Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]: github_rest_api Data Source returns null values for body and headers #2109

Closed
1 task done
srgustafson8 opened this issue Jan 17, 2024 · 4 comments · Fixed by #2110
Closed
1 task done

[BUG]: github_rest_api Data Source returns null values for body and headers #2109

srgustafson8 opened this issue Jan 17, 2024 · 4 comments · Fixed by #2110
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented

Comments

@srgustafson8
Copy link
Contributor

srgustafson8 commented Jan 17, 2024

Expected Behavior

I am looking to use the github_rest_api data source to get information about the rate limit of the authenticated user/app. I would expect that given;

terraform {
  required_providers {
    github = {
      version = "< 6.0.0"
      source  = "integrations/github"
    }
  }
}

provider "github" {
  owner = "myorg"
}

data "github_rest_api" "test_call" {
    endpoint = "rate_limit"
}

output "result" {
  value = data.github_rest_api.test_call
}

I would get:

> terraform apply --auto-approve

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

result = {
  "body" = {
    "resources" = {
      "core" = {
        "limit" = 5000
        "used" = 3
...etc...etc...
      }
    }
  }  
  "code" = 200
  "endpoint" = "rate_limit"
  "headers" = {
    " X-Oauth-Scopes" = [
      "delete:packages",
      "gist",
      "notifications"
    ]
    "X-Ratelimit-Limit" = 5000
....etc....etc
  }
  "id" = "1234:123445:ABCDEF:ABCDEF:ABCDEFG"
  "status" = "200 OK"
}

Actual Behavior

Instead the result shows partially, but the body and headers attributes are null.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

result = {
  "body" = tomap(null) /* of string */
  "code" = 200
  "endpoint" = "rate_limit"
  "headers" = tomap(null) /* of string */
  "id" = "9D52:27E4D9:624A602:634FC6D:65A7DDFF"
  "status" = "200 OK"
}

This is partially replicated with other API calls, for example, changing the endpoint to repos/integrations/terraform-provider-github/git/refs/heads/main returns:

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

result = {
  "body" = tomap({
    "node_id" = "MDM6UmVmOTM0NDYwOTk6cmVmcy9oZWFkcy9tYWlu"
    "ref" = "refs/heads/main"
    "url" = "https://api.github.com/repos/integrations/terraform-provider-github/git/refs/heads/main"
  })
  "code" = 200
  "endpoint" = "repos/integrations/terraform-provider-github/git/refs/heads/main"
  "headers" = tomap(null) /* of string */
  "id" = "DFAE:220AD1:601548B:611DD1E:65A7DF15"
  "status" = "200 OK"
}

Partially managing to decode the body, but failing to decode the headers. This is also unstable, with changes to what manages to get decoded differing each time you run an apply.

Deduction Time

Attempting to debug this, I recreated the data resource locally to run:

package main

import (
	"context"

	"github.com/google/go-github/v58/github"
	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
)

func dataSourceGithubRestApi() *schema.Resource {
	return &schema.Resource{
		Read: dataSourceGithubRestApiRead,

		Schema: map[string]*schema.Schema{
			"endpoint": {
				Type:     schema.TypeString,
				Required: true,
				ForceNew: true,
			},
			"code": {
				Type:     schema.TypeInt,
				Computed: true,
			},
			"status": {
				Type:     schema.TypeString,
				Computed: true,
			},
			"headers": {
				Type:     schema.TypeMap, // map[string]string
				Computed: true,
			},
			"body": {
				Type:     schema.TypeMap,
				Computed: true,
			},
		},
	}
}

func dataSourceGithubRestApiRead(d *schema.ResourceData, meta interface{}) error {
	u := "rate_limit"

	client := github.NewClient(nil)
	ctx := context.Background()

	var body map[string]interface{}

	req, err := client.NewRequest("GET", u, nil)
	if err != nil {
		return err
	}

	resp, _ := client.Do(ctx, req, &body)

	d.SetId(resp.Header.Get("x-github-request-id"))
	d.Set("code", resp.StatusCode)
	d.Set("status", resp.Status)
	d.Set("headers", resp.Header)
	d.Set("body", body)

	return nil
}

func main() {
	var meta interface{}
	meta = "true"
	// ctx := context.Background()
	d := dataSourceGithubRestApi().TestResourceData()
	e := dataSourceGithubRestApi().Read(d, meta)
	if e != nil {
		println("it died" + e.Error())
	}
	println("done")
}

Which would run without error, but the resourceData fields for body and headers would not be populated, following the behaviour of the data source.

Purely by accident, I also tried this same code with the v2 of the plugin sdk schema module, which throws errors:

❯ go run main.go
2024/01/17 14:29:54 [ERROR] setting state: headers.Content-Length: '' expected type 'string', got unconvertible type '[]string', value: '[467]'
2024/01/17 14:29:54 [ERROR] setting state: body.resources: '' expected type 'string', got unconvertible type 'map[string]interface {}', value: 'map[core:map[limit:60 remaining:44 reset:1.705502521e+09 resource:core used:16] graphql:map[limit:0 remaining:0 reset:1.705505394e+09 resource:graphql used:0] integration_manifest:map[limit:5000 remaining:5000 reset:1.705505394e+09 resource:integration_manifest used:0] search:map[limit:10 remaining:10 reset:1.705501854e+09 resource:search used:0]]'
done

So I then tried to use the 6.0.0-alpha version of the provider which utilises the v2 sdk, and got the following:

❯ terraform apply --auto-approve
data.github_rest_api.test_call: Reading...
╷
│ Error: headers.X-Ratelimit-Reset: '' expected type 'string', got unconvertible type '[]string', value: '[1705503178]'
│ 
│   with data.github_rest_api.test_call,
│   on testing.tf line 14, in data "github_rest_api" "test_call":
│   14: data "github_rest_api" "test_call" {
│ 
╵

My final diagnosis is that the error stems from the fact that the body and headers attributes are given schema.TypeMap, which I assume Terraform defaults to assuming means map[string]string, and all map elements must be the same type. The GitHub API calls return a variety of schemas with mostly differing types which can't be normalised. This never showed up when using the v1 sdk as the error seems to be never generated or swallowed.

Proposed Fix

I don't think it would be possible to come up with a schema type that allows for the variety of API responses that can be returned. I believe this resource should be changed to return body and headers as a (JSON) string, and users can use jsondecode() to get to the underlying values returned, with knowledge of the API call they are making and it's schema.

I'm happy to make these changes if no-one else wants to pick them up.

Terraform Version

Terraform v1.5.7
on darwin_arm64

  • provider registry.terraform.io/integrations/github v5.44.0 / 6.0.0-alpha

Affected Resource(s)

  • data_source_github_rest_api [link]

Terraform Configuration Files

No response

Steps to Reproduce

No response

Debug Output

No response

Panic Output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@srgustafson8 srgustafson8 added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented labels Jan 17, 2024
@nickfloyd nickfloyd added Status: Up for grabs Issues that are ready to be worked on by anyone and removed Status: Triage This is being looked at and prioritized labels Jan 19, 2024
@nickfloyd nickfloyd moved this from 🆕 Triage to 🔥 Backlog in 🧰 Octokit Active Jan 19, 2024
@nickfloyd
Copy link
Contributor

Hey @srgustafson8 thanks for tracking this down. Let us know if you'd be interested in fixing the issue and submitting a PR. For now I have labeled this as "Up For Grabs" so that the community can take a stab at getting this fixed as well. ❤️

@srgustafson8
Copy link
Contributor Author

Hi @nickfloyd - way ahead of you 😉 submitted #2110 earlier

@nickfloyd
Copy link
Contributor

Hi @nickfloyd - way ahead of you 😉 submitted #2110 earlier

💥 Awesome. Thank you so much! We'll have a look! ❤

@github-project-automation github-project-automation bot moved this from 🔥 Backlog to ✅ Done in 🧰 Octokit Active Jan 22, 2024
@riezebosch
Copy link
Contributor

riezebosch commented Feb 16, 2024

@srgustafson8 shouldn't this be resp.Body instead: https://github.com/integrations/terraform-provider-github/blame/main/github/data_source_github_rest_api.go#L60

Looks like the test could also be expanded to test for some or actual value instead of only presence.

I fail to get the tests run locally and also don't seem to get running the provider locally 🤷🏻

Update: I think I solved it: #2152

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented
Projects
None yet
3 participants