Add GraphQL Client

[patrickmarabeas]

Initialize a client for Github GraphQL API calls. This will allow for resources and data sources to be uplifted on a case by case basis.

• Add shurcool/graphqlv4
• Initialize a GraphQL client (v4client)
• Rename client to v3client
• Update client usage to v3client
• Update vendor files

[kpfleming]

I know this will sound really picky, but I'd prefer to see them named 'v3client' and 'v4client'; the API protocol is just an artifact, it's not really the differentiating factor between them the API version is. Imagine what will happen if GitHub releases as V5 API which is also GraphQL based :-)

[patrickmarabeas]

Makes sense.

[patrickmarabeas]

• Clients renamed as per @kpfleming request
• Vendor files updated for CI to pass

[jcudit]

@patrickmarabeas - I've got this queued to remove conflicts early next week. Happy to merge if you beat me to it 😄

[patrickmarabeas]

Was hoping to get to it today but failed miserably.

[patrickmarabeas]

@jcudit rebased and conflicts resolved.

[jcudit]

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x100 pc=0x1c945c7]

goroutine 15 [running]:
github.com/terraform-providers/terraform-provider-github/github.(*Config).Clients(0xc000019a28, 0x20469a6, 0x9, 0x1debba0, 0x2d84d60)
	/Users/jcudit/terraform-providers/terraform-provider-github/github/config.go:106 +0x5a7
github.com/terraform-providers/terraform-provider-github/github.providerConfigure.func1(0xc00043c700, 0x0, 0xc0002995a0, 0xc00043c700, 0x0)
	/Users/jcudit/terraform-providers/terraform-provider-github/github/provider.go:125 +0x2bc
github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Provider).Configure(0xc000044e80, 0xc0001b02d0, 0x1e5b4c0, 0xc0001b0240)
	/Users/jcudit/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk@v1.7.0/helper/schema/provider.go:275 +0xf6
github.com/hashicorp/terraform-plugin-sdk/internal/helper/plugin.(*GRPCProviderServer).Configure(0xc0007b45e8, 0x23b1a60, 0xc0004d1c80, 0xc0004671c0, 0xc0007b45e8, 0xc0004d1c80, 0xc00045abd0)
	/Users/jcudit/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk@v1.7.0/internal/helper/plugin/grpc_provider.go:487 +0x2e6
github.com/hashicorp/terraform-plugin-sdk/internal/tfplugin5._Provider_Configure_Handler(0x1fed500, 0xc0007b45e8, 0x23b1a60, 0xc0004d1c80, 0xc0001587e0, 0x0, 0x23b1a60, 0xc0004d1c80, 0xc0005a43c0, 0x51)
	/Users/jcudit/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk@v1.7.0/internal/tfplugin5/tfplugin5.pb.go:3135 +0x23e
google.golang.org/grpc.(*Server).processUnaryRPC(0xc000444840, 0x23c0ea0, 0xc00055af00, 0xc000452600, 0xc000222300, 0x2df05d8, 0x0, 0x0, 0x0)
	/Users/jcudit/go/pkg/mod/google.golang.org/grpc@v1.23.0/server.go:995 +0x466
google.golang.org/grpc.(*Server).handleStream(0xc000444840, 0x23c0ea0, 0xc00055af00, 0xc000452600, 0x0)
	/Users/jcudit/go/pkg/mod/google.golang.org/grpc@v1.23.0/server.go:1275 +0xda6
google.golang.org/grpc.(*Server).serveStreams.func1.1(0xc0007b8070, 0xc000444840, 0x23c0ea0, 0xc00055af00, 0xc000452600)
	/Users/jcudit/go/pkg/mod/google.golang.org/grpc@v1.23.0/server.go:710 +0x9f
created by google.golang.org/grpc.(*Server).serveStreams.func1
	/Users/jcudit/go/pkg/mod/google.golang.org/grpc@v1.23.0/server.go:708 +0xa1
FAIL	github.com/terraform-providers/terraform-provider-github/github	0.524s
?   	github.com/terraform-providers/terraform-provider-github	[no test files]

Caught a bunch of these when running acceptance tests locally ☝️
Are you getting similar results?

[patrickmarabeas]

Soz, assumed CI was running acceptance tests.

Tests no longer panicking, but I am getting some failures - most look to be because I have previously run the tests and things are already in the state it is trying to put them into.

The only error looks to be the following, which is related to my addition of adding v3/ to the BaseURL if it does not match https://api.github.com/ - which is to facilitate a) enterprise installs b) the graphql endpoint (graphql is added).

=== RUN   TestProvider_insecure
--- FAIL: TestProvider_insecure (0.07s)
    testing.go:654: Step 1 error: errors during refresh:

        Error: GET https://localhost:64499/v3/orgs/patrickmarabeas-org: 404  []

          on /var/folders/wq/ztvbmnls1f50rsgvn4z4wtdr0000gn/T/tf-test093003891/main.tf line 1:
          (source code not available)

Is it possible to update the mock API URL to include the /v3/ path?

I'll continue to dig into it later tonight if you don't beat me to it.

[patrickmarabeas]

I'm encountering the above error (sans v3/) on master - so unsure what I'm missing.

[jcudit]

--- FAIL: TestProvider_insecure (0.07s)

Adding something like the following to this function should get the test passing:

	mux.HandleFunc("/v3/users/hashibot", testRespondJson(userResponseBody))
	mux.HandleFunc("/v3/users/hashibot/gpg_keys", testRespondJson(gpgKeysResponseBody))
	mux.HandleFunc("/v3/users/hashibot/keys", testRespondJson(keysResponseBody))
	mux.HandleFunc("/v3/orgs/", testRespondJson("{}"))

baseURL := os.Getenv("GITHUB_BASE_URL")

I had to set this in my environment or a panic occurs during TestAccGithubRepository_defaultBranch. Can we set a sane default somewhere or update the acceptance testing documentation to include this variable?

---

Can't spot much else. I think this one is pretty close to shipping.

[patrickmarabeas]

I don't think that's it. I get the same error on master. Are you seeing similar errors on your end? Prepending /v3 did not resolve the issue.

• Added update to README re. GITHUB_BASE_URL
• Squash / rebased

Is it possible to get acceptance tests running on this PR? I notice they seem to run on some, but not others...

[anGie44]

@patrickmarabeas I gotcha! i just added the acceptance test label - let's see if/how they run :)

ah i see the tests erroring out b/c the ENV GITHUB_TOKEN isn't getting set apparently 😞 ..might be related to https://help.github.com/en/actions/configuring-and-managing-workflows/authenticating-with-the-github_token#permissions-for-the-github_token as I've seen the tests run when a PR isn't from a forked repo

[patrickmarabeas]

Ahh. of course.

[jcudit]

I propose we track the failing TestProvider_insecure test elsewhere and move forward with these changes as-is since master looks to be failing similar to how this branch fails. Other feature work can then proceed with the foundation this work lays and we can revisit additional functionality when a better design emerges.

I'll leave this approval around until you confirm this is an acceptable approach and hope to merge by end of week.