feat: Add new gh client implementation (#3448)

* feat: Add new gh client implementation

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>

* fixup! feat: Add new gh client implementation

* fixup! feat: Add new gh client implementation

* fixup! feat: Add new gh client implementation

* fixup! feat: Add new gh client implementation

* fixup! feat: Add new gh client implementation

* fixup! feat: Add new gh client implementation

* fixup! feat: Add new gh client implementation

* fixup! feat: Add new gh client implementation

* fixup! feat: Add new gh client implementation

---------

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>

Author: stevehipwell

.github/workflows/dotcom-acceptance-tests.yaml

@@ -109,6 +109,7 @@ jobs:
           GITHUB_OWNER: ${{ case(matrix.mode == 'individual', vars.GH_TEST_LOGIN, matrix.mode == 'organization', vars.GH_TEST_ORG_NAME, '') }}
           GITHUB_USERNAME: ${{ vars.GH_TEST_LOGIN }}
           GITHUB_ENTERPRISE_SLUG: ${{ vars.GH_TEST_ENTERPRISE_SLUG }}
+          GITHUB_LEGACY_CLIENT: "false"
           GH_TEST_AUTH_MODE: ${{ matrix.mode }}
           GH_TEST_USER_REPOSITORY: ${{ vars.GH_TEST_USER_REPOSITORY }}
           GH_TEST_ORG_USER: ${{ vars.GH_TEST_ORG_USER }}

.github/workflows/ghes-acceptance-tests.yaml

@@ -120,6 +120,7 @@ jobs:
           GITHUB_OWNER: ""
           GITHUB_USERNAME: ""
           GITHUB_ENTERPRISE_SLUG: ""
+          GITHUB_LEGACY_CLIENT: "false"
           GH_TEST_AUTH_MODE: enterprise
         run: |
           set -eou pipefail

CONTRIBUTING.md

@@ -107,6 +107,7 @@ Once you have the repository cloned, there's a couple of additional steps you'll
   export GITHUB_OWNER="<name of an organization>"
   export GITHUB_USERNAME="<username of the user who created the token>"
   export GITHUB_TOKEN="<token of a user with an organization account>"
+  export GITHUB_LEGACY_CLIENT="false"
   ```
 
 - Build the project with `make build`
@@ -239,6 +240,9 @@ export TF_ACC="1"
 # Configure the URL override for GHES.
 export GITHUB_BASE_URL=
 
+# Use the modern client for testing.
+export GITHUB_LEGACY_CLIENT="false"
+
 # Configure acceptance testing mode; one of anonymous, individual, organization, team or enterprise. If not set will default to anonymous
 export GH_TEST_AUTH_MODE=
 
@@ -286,6 +290,7 @@ To run acceptance tests the `TF_ACC` environment variable must be set. Below is
     "GITHUB_ENTERPRISE_SLUG": "",
     "GITHUB_OWNER": "<ORGANIZATION>",
     "GITHUB_USERNAME": "<USERNAME>",
+    "GITHUB_LEGACY_CLIENT": "false",
     "GH_TEST_AUTH_MODE": "organization",
     "GH_TEST_USER_REPOSITORY": "",
     "GH_TEST_ORG_USER": "",

docs/index.md

@@ -121,17 +121,19 @@ provider "github" {
 
 - `app_auth` (Block List, Max: 1) Authenticate using a GitHub App. (see [below for nested schema](#nestedblock--app_auth))
 - `base_url` (String) The base URL for the GitHub API; this defaults to the GitHub API URL. If you are using GitHub Enterprise Server (GHES) or GitHub Enterprise Cloud with Data Residency (GHEC-DR), this is required. This can also be set by the `GITHUB_BASE_URL` environment variable.
-- `insecure` (Boolean) Allow insecure server connections when using SSL.
+- `cache_path` (String) The path to the cache directory for persisting GitHub API requests between runs; if not set there will be no caching between runs. This can also be set by the `GITHUB_CACHE_PATH` environment variable.
+- `insecure` (Boolean, Deprecated) Allow insecure server connections when using SSL.
+- `legacy_client` (Boolean) Use the legacy GitHub client implementation; if set to `false`, the new client implementation is used. This can also be set by the `GITHUB_LEGACY_CLIENT` environment variable.
 - `max_per_page` (Number) The maximum number of results per page for paginated API requests; this defaults to `100`. This can also be set by the `GITHUB_MAX_PER_PAGE` environment variable.
 - `max_retries` (Number) The maximum number of retries for failed requests; this defaults to `3`.
 - `organization` (String, Deprecated) GitHub organization to manage. This can also be set by the `GITHUB_ORGANIZATION` environment variable.
 - `owner` (String) GitHub organization or user account to manage; this is required when authenticating using a GitHub App. If the owner is not provided and a token is provided, the provider will attempt to auto-detect the owner associated with the token. This can also be set by the `GITHUB_OWNER` environment variable.
-- `parallel_requests` (Boolean) Allow the provider to make parallel API calls; this is experimental and may cause concurrency and rate limiting issues.
-- `read_delay_ms` (Number) The delay in milliseconds between read operations; this defaults to `0`. This can be used to mitigate rate limiting issues when performing a large number of read operations.
+- `parallel_requests` (Boolean) Allow the provider to make parallel API calls; this is experimental and may cause concurrency and rate limiting issues. This is ignored for the REST API when `legacy_client` is `false` since the new client implementation is designed to safely handle parallel requests.
+- `read_delay_ms` (Number) The delay in milliseconds between read operations; this defaults to `0`. This can be used to mitigate rate limiting issues when performing a large number of read operations. This is ignored for the REST API when `legacy_client` is `false` since the new client implementation is GitHub rate limit aware.
 - `retry_delay_ms` (Number) The delay in milliseconds between retry attempts; this defaults to `1000`. This setting only applies when `max_retries` is greater than `0`.
-- `retryable_errors` (List of Number) List of HTTP status codes that should be retried; if not set this uses the provider defaults. This setting only applies when `max_retries` is greater than `0`.
+- `retryable_errors` (List of Number) List of HTTP status codes that should be retried; if not set this uses the provider defaults. This setting only applies when `max_retries` is greater than `0`. This is ignored for the REST API when `legacy_client` is `false` since the new client implementation handles the retry logic.
 - `token` (String) GitHub OAuth or Personal Access Token (PAT) to use for authentication. This can also be set by the `GITHUB_TOKEN` environment variable.
-- `write_delay_ms` (Number) The delay in milliseconds between write operations; this defaults to `1000`. This is used to mitigate the GitHub API's abuse rate limits when writing. Note that **ALL** requests to the GraphQL API are implemented as `POST` requests under the hood, so this setting affects those calls as well.
+- `write_delay_ms` (Number) The delay in milliseconds between write operations; this defaults to `1000`. This is used to mitigate the GitHub API's abuse rate limits when writing. Note that **ALL** requests to the GraphQL API are implemented as `POST` requests under the hood, so this setting affects those calls as well. This is ignored for the REST API when `legacy_client` is `false` since the new client implementation is GitHub rate limit aware.
 
 <a id="nestedblock--app_auth"></a>
 ### Nested Schema for `app_auth`

github/acc_test.go

@@ -34,13 +34,17 @@ var (
 
 type testAccConfig struct {
 	// Target configuration
-	baseURL *url.URL
+	legacyClient bool
+	baseURL      *url.URL
 
 	// Auth configuration
-	authMode testMode
-	owner    string
-	username string
-	token    string
+	authMode          testMode
+	owner             string
+	username          string
+	token             string
+	appID             string
+	appInstallationID string
+	appPEM            string
 
 	// Enterprise configuration
 	enterpriseSlug  string
@@ -112,13 +116,12 @@ func TestMain(m *testing.M) {
 	}
 
 	config := testAccConfig{
-		baseURL:                   baseURL,
-		authMode:                  authMode,
-		testPublicRepository:      "terraform-provider-github",
-		testPublicRepositoryOwner: "integrations",
-		testPublicReleaseId:       186531906,
-		// The terraform-provider-github_6.4.0_manifest.json asset ID from
-		// https://github.com/integrations/terraform-provider-github/releases/tag/v6.4.0
+		legacyClient:                      os.Getenv("GITHUB_LEGACY_CLIENT") != "false",
+		baseURL:                           baseURL,
+		authMode:                          authMode,
+		testPublicRepository:              "terraform-provider-github",
+		testPublicRepositoryOwner:         "integrations",
+		testPublicReleaseId:               186531906, // The terraform-provider-github_6.4.0_manifest.json asset ID from https://github.com/integrations/terraform-provider-github/releases/tag/v6.4.0
 		testPublicRelaseAssetId:           "207956097",
 		testPublicRelaseAssetName:         "terraform-provider-github_6.4.0_manifest.json",
 		testPublicReleaseAssetContent:     "{\n  \"version\": 1,\n  \"metadata\": {\n    \"protocol_versions\": [\n      \"5.0\"\n    ]\n  }\n}",
@@ -135,28 +138,46 @@ func TestMain(m *testing.M) {
 		testExternalUser2:                 os.Getenv("GH_TEST_EXTERNAL_USER2"),
 		testAdvancedSecurity:              os.Getenv("GH_TEST_ADVANCED_SECURITY") == "true",
 		testRepositoryVisibility:          "public",
-		enterpriseIsEMU:                   authMode == enterprise && os.Getenv("GH_TEST_ENTERPRISE_IS_EMU") == "true",
 	}
 
 	if config.authMode != anonymous {
 		config.owner = os.Getenv("GITHUB_OWNER")
 		config.username = os.Getenv("GITHUB_USERNAME")
 		config.token = os.Getenv("GITHUB_TOKEN")
+		config.appID = os.Getenv("GITHUB_APP_ID")
+		config.appInstallationID = os.Getenv("GITHUB_APP_INSTALLATION_ID")
+		config.appPEM = os.Getenv("GITHUB_APP_PEM_FILE")
 
 		if len(config.owner) == 0 {
 			fmt.Println("GITHUB_OWNER environment variable not set")
 			os.Exit(1)
 		}
 
-		if len(config.username) == 0 {
+		if config.authMode == individual && len(config.username) == 0 {
 			fmt.Println("GITHUB_USERNAME environment variable not set")
 			os.Exit(1)
 		}
 
-		if len(config.token) == 0 {
-			fmt.Println("GITHUB_TOKEN environment variable not set")
+		if config.token == "" && config.appID == "" {
+			fmt.Println("authentication not configured")
 			os.Exit(1)
 		}
+
+		if config.token != "" && config.appID != "" {
+			fmt.Println("Both token and app auth configured")
+			os.Exit(1)
+		}
+
+		if config.appID != "" && (config.appInstallationID == "" || config.appPEM == "") {
+			fmt.Println("App auth configured without all required parameters")
+			os.Exit(1)
+		}
+	}
+
+	if config.authMode != anonymous && config.authMode != individual {
+		if i, err := strconv.Atoi(os.Getenv("GH_TEST_ORG_APP_INSTALLATION_ID")); err == nil {
+			config.testOrgAppInstallationId = i
+		}
 	}
 
 	if config.authMode == enterprise {
@@ -167,19 +188,14 @@ func TestMain(m *testing.M) {
 			os.Exit(1)
 		}
 
-		i, err := strconv.Atoi(os.Getenv("GH_TEST_ENTERPRISE_EMU_GROUP_ID"))
-		if err == nil {
-			config.testEnterpriseEMUGroupId = i
-		}
-
-		if config.enterpriseIsEMU {
+		if os.Getenv("GH_TEST_ENTERPRISE_IS_EMU") == "true" {
+			config.enterpriseIsEMU = true
 			config.testRepositoryVisibility = "private"
-		}
-	}
 
-	i, err := strconv.Atoi(os.Getenv("GH_TEST_ORG_APP_INSTALLATION_ID"))
-	if err == nil {
-		config.testOrgAppInstallationId = i
+			if i, err := strconv.Atoi(os.Getenv("GH_TEST_ENTERPRISE_EMU_GROUP_ID")); err == nil {
+				config.testEnterpriseEMUGroupId = i
+			}
+		}
 	}
 
 	testAccConf = &config
@@ -189,19 +205,48 @@ func TestMain(m *testing.M) {
 	resource.TestMain(m)
 }
 
-func getTestMeta() (*Owner, error) {
-	config := Config{
-		Token:   testAccConf.token,
-		Owner:   testAccConf.owner,
-		BaseURL: testAccConf.baseURL,
+func getTestAppToken() (string, error) {
+	if testAccConf.appID == "" || testAccConf.appInstallationID == "" || testAccConf.appPEM == "" {
+		return "", fmt.Errorf("app auth not configured")
 	}
 
-	meta, err := config.Meta()
+	appToken, err := GenerateOAuthTokenFromApp(testAccConf.baseURL, testAccConf.appID, testAccConf.appInstallationID, testAccConf.appPEM)
 	if err != nil {
-		return nil, fmt.Errorf("error getting GitHub meta parameter")
+		return "", err
 	}
 
-	return meta.(*Owner), nil
+	return appToken, nil
+}
+
+func getTestToken() (string, error) {
+	if testAccConf.token != "" {
+		return testAccConf.token, nil
+	}
+
+	return getTestAppToken()
+}
+
+func getTestMeta() (*Owner, error) {
+	config := &Config{
+		GraphQLAPIPath: "graphql",
+		LegacyClient:   testAccConf.legacyClient,
+		BaseURL:        testAccConf.baseURL,
+		Owner:          testAccConf.owner,
+	}
+
+	if config.LegacyClient || testAccConf.appID == "" {
+		token, err := getTestToken()
+		if err != nil {
+			return nil, fmt.Errorf("error getting test token: %w", err)
+		}
+		config.Token = token
+	} else {
+		config.AppID = &testAccConf.appID
+		config.AppInstallationID = &testAccConf.appInstallationID
+		config.AppPEM = []byte(testAccConf.appPEM)
+	}
+
+	return configureProviderMeta(context.Background(), config)
 }
 
 func configureSweepers() {
@@ -292,6 +337,12 @@ func skipUnauthenticated(t *testing.T) {
 	}
 }
 
+func skipApp(t *testing.T) {
+	if testAccConf.appID != "" {
+		t.Skip("Skipping as app not configured")
+	}
+}
+
 func skipUnlessHasOrgs(t *testing.T) {
 	if !slices.Contains(orgTestModes, testAccConf.authMode) {
 		t.Skip("Skipping as test mode doesn't have orgs")

github/config.go

@@ -16,17 +16,23 @@ import (
 )
 
 type Config struct {
-	Token            string
-	Owner            string
-	BaseURL          *url.URL
-	IsGHES           bool
-	Insecure         bool
-	WriteDelay       time.Duration
-	ReadDelay        time.Duration
-	RetryDelay       time.Duration
-	RetryableErrors  map[int]bool
-	MaxRetries       int
-	ParallelRequests bool
+	AppID             *string
+	AppInstallationID *string
+	AppPEM            []byte
+	BaseURL           *url.URL
+	CachePath         *string
+	GraphQLAPIPath    string
+	Insecure          bool
+	LegacyClient      bool
+	MaxRetries        int
+	Owner             string
+	ParallelRequests  bool
+	ReadDelay         time.Duration
+	RESTAPIPath       string
+	RetryableErrors   map[int]bool
+	RetryDelay        time.Duration
+	Token             string
+	WriteDelay        time.Duration
 }
 
 type Owner struct {
@@ -47,6 +53,8 @@ const (
 	DotComAPIHost = "api.github.com"
 	// GHESRESTAPISuffix is the rest api suffix for GitHub Enterprise Server.
 	GHESRESTAPIPath = "api/v3/"
+	// GHESGraphQLAPISuffix is the GraphQL api suffix for GitHub Enterprise Server.
+	GHESGraphQLAPIPath = "api/graphql"
 )
 
 var (
@@ -83,7 +91,7 @@ func (c *Config) AuthenticatedHTTPClient() *http.Client {
 }
 
 func (c *Config) Anonymous() bool {
-	return c.Token == ""
+	return c.AppID == nil && c.Token == ""
 }
 
 func (c *Config) AnonymousHTTPClient() *http.Client {
@@ -92,30 +100,19 @@ func (c *Config) AnonymousHTTPClient() *http.Client {
 }
 
 func (c *Config) NewGraphQLClient(client *http.Client) (*githubv4.Client, error) {
-	var path string
-	if c.IsGHES {
-		path = "api/graphql"
-	} else {
-		path = "graphql"
-	}
-
-	return githubv4.NewEnterpriseClient(c.BaseURL.JoinPath(path).String(), client), nil
+	return githubv4.NewEnterpriseClient(c.BaseURL.JoinPath(c.GraphQLAPIPath).String(), client), nil
 }
 
 func (c *Config) NewRESTClient(client *http.Client) (*github.Client, error) {
-	path := ""
-	if c.IsGHES {
-		path = GHESRESTAPIPath
-	}
-
-	v3client, err := github.NewClient(github.WithHTTPClient(client), github.WithURLs(new(c.BaseURL.JoinPath(path).String()), nil))
+	v3client, err := github.NewClient(github.WithHTTPClient(client), github.WithURLs(new(c.BaseURL.JoinPath(c.RESTAPIPath).String()), nil))
 	if err != nil {
 		return nil, err
 	}
 
 	return v3client, nil
 }
 
+// Deprecated: This is no longer required as [configureProviderMeta] is now used to configure the provider meta parameter with the necessary clients and owner information. Use [configureProviderMeta] instead.
 func (c *Config) ConfigureOwner(owner *Owner) (*Owner, error) {
 	ctx := context.Background()
 	owner.name = c.Owner
@@ -144,34 +141,9 @@ func (c *Config) ConfigureOwner(owner *Owner) (*Owner, error) {
 
 // Meta returns the meta parameter that is passed into subsequent resources
 // https://godoc.org/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#ConfigureFunc
+// Deprecated: Use [configureProviderMeta] instead.
 func (c *Config) Meta() (any, error) {
-	var client *http.Client
-	if c.Anonymous() {
-		client = c.AnonymousHTTPClient()
-	} else {
-		client = c.AuthenticatedHTTPClient()
-	}
-
-	v3client, err := c.NewRESTClient(client)
-	if err != nil {
-		return nil, err
-	}
-
-	v4client, err := c.NewGraphQLClient(client)
-	if err != nil {
-		return nil, err
-	}
-
-	var owner Owner
-	owner.v4client = v4client
-	owner.v3client = v3client
-	owner.StopContext = context.Background()
-
-	_, err = c.ConfigureOwner(&owner)
-	if err != nil {
-		return &owner, err
-	}
-	return &owner, nil
+	return configureProviderMeta(context.Background(), c)
 }
 
 type previewHeaderInjectorTransport struct {
@@ -205,8 +177,8 @@ func (injector *previewHeaderInjectorTransport) RoundTrip(req *http.Request) (*h
 
 // getBaseURL returns a correctly configured base URL and a bool as to if this is GitHub Enterprise Server.
 func getBaseURL(s string) (*url.URL, bool, error) {
-	if len(s) == 0 {
-		s = DotComAPIURL
+	if s == "" {
+		return nil, false, fmt.Errorf("base url must not be empty")
 	}
 
 	u, err := url.Parse(s)