From 3e7430ae48bb7fea365ad7fa1e23a427de6db4a6 Mon Sep 17 00:00:00 2001 From: Francesco Ilario Date: Fri, 17 Nov 2023 15:59:02 +0100 Subject: [PATCH] fix: forward context in Github and REST clients (#342) --- pkg/client/github_client.go | 6 +++--- pkg/client/rest_client.go | 4 ++-- pkg/client/rest_client_test.go | 5 +++-- pkg/status/versionchecks.go | 4 ++-- pkg/status/versionchecks_test.go | 23 ++++++++++++----------- pkg/test/github_client.go | 3 ++- 6 files changed, 24 insertions(+), 21 deletions(-) diff --git a/pkg/client/github_client.go b/pkg/client/github_client.go index a1347253..b665cf31 100644 --- a/pkg/client/github_client.go +++ b/pkg/client/github_client.go @@ -12,7 +12,7 @@ import ( const GitHubAPICallDelay = 1 * time.Minute // GetGitHubClientFunc a func that returns a GitHub client instance -type GetGitHubClientFunc func(string) *github.Client +type GetGitHubClientFunc func(context.Context, string) *github.Client type GitHubRepository struct { Org, Name, Branch, DeployedCommitSHA string @@ -21,11 +21,11 @@ type GitHubRepository struct { // NewGitHubClient return a client that interacts with GitHub and has rate limiter configured. // With authenticated GitHub api you can make 5,000 requests per hour. // see: https://github.com/google/go-github#rate-limiting -func NewGitHubClient(accessToken string) *github.Client { +func NewGitHubClient(ctx context.Context, accessToken string) *github.Client { ts := oauth2.StaticTokenSource( &oauth2.Token{AccessToken: accessToken}, ) - tc := oauth2.NewClient(context.TODO(), ts) + tc := oauth2.NewClient(ctx, ts) return github.NewClient(tc) } diff --git a/pkg/client/rest_client.go b/pkg/client/rest_client.go index 104fccc4..e1f45627 100644 --- a/pkg/client/rest_client.go +++ b/pkg/client/rest_client.go @@ -12,7 +12,7 @@ import ( // CreateTokenRequest creates a TokenRequest for a service account using given expiration in seconds. // Returns the token string and nil if everything went fine, otherwise an empty string and an error is returned in case something went wrong. -func CreateTokenRequest(restClient *rest.RESTClient, namespacedName types.NamespacedName, expirationInSeconds int) (string, error) { +func CreateTokenRequest(ctx context.Context, restClient *rest.RESTClient, namespacedName types.NamespacedName, expirationInSeconds int) (string, error) { tokenRequest := &authv1.TokenRequest{ Spec: authv1.TokenRequestSpec{ ExpirationSeconds: pointer.Int64(int64(expirationInSeconds)), @@ -22,7 +22,7 @@ func CreateTokenRequest(restClient *rest.RESTClient, namespacedName types.Namesp if err := restClient.Post(). AbsPath(fmt.Sprintf("api/v1/namespaces/%s/serviceaccounts/%s/token", namespacedName.Namespace, namespacedName.Name)). Body(tokenRequest). - Do(context.TODO()). + Do(ctx). Into(result); err != nil { return "", err } diff --git a/pkg/client/rest_client_test.go b/pkg/client/rest_client_test.go index 745ee5fb..d131769f 100644 --- a/pkg/client/rest_client_test.go +++ b/pkg/client/rest_client_test.go @@ -1,6 +1,7 @@ package client_test import ( + "context" "encoding/json" "net/http" "testing" @@ -26,7 +27,7 @@ func TestCreateTokenRequest(t *testing.T) { // when require.NoError(t, err) - token, err := restclient.CreateTokenRequest(cl, types.NamespacedName{ + token, err := restclient.CreateTokenRequest(context.TODO(), cl, types.NamespacedName{ Namespace: "jane-env", Name: "jane", }, 1) @@ -49,7 +50,7 @@ func TestCreateTokenRequest(t *testing.T) { // when require.NoError(t, err) - token, err := restclient.CreateTokenRequest(cl, types.NamespacedName{ + token, err := restclient.CreateTokenRequest(context.TODO(), cl, types.NamespacedName{ Namespace: "jane-env", Name: "jane", }, 1) diff --git a/pkg/status/versionchecks.go b/pkg/status/versionchecks.go index 69353bdd..50966f23 100644 --- a/pkg/status/versionchecks.go +++ b/pkg/status/versionchecks.go @@ -30,7 +30,7 @@ type VersionCheckManager struct { // CheckDeployedVersionIsUpToDate verifies if there is a match between the latest commit in GitHub for a given repo and branch matches the provided commit SHA. // There is some preconfigured delay/threshold that we keep in account before returning an `error condition`. -func (m *VersionCheckManager) CheckDeployedVersionIsUpToDate(isProd bool, accessTokenKey string, alreadyExistingConditions []toolchainv1alpha1.Condition, githubRepo client.GitHubRepository) *toolchainv1alpha1.Condition { +func (m *VersionCheckManager) CheckDeployedVersionIsUpToDate(ctx context.Context, isProd bool, accessTokenKey string, alreadyExistingConditions []toolchainv1alpha1.Condition, githubRepo client.GitHubRepository) *toolchainv1alpha1.Condition { // the first two checks are pretty much the same for all components if !isProd { cond := NewComponentReadyCondition(toolchainv1alpha1.ToolchainStatusDeploymentRevisionCheckDisabledReason) @@ -57,7 +57,7 @@ func (m *VersionCheckManager) CheckDeployedVersionIsUpToDate(isProd bool, access return &previouslySet } m.LastGHCallsPerRepo[githubRepo.Name] = time.Now() - githubClient := m.GetGithubClientFunc(accessTokenKey) + githubClient := m.GetGithubClientFunc(ctx, accessTokenKey) // get the latest commit from given repository and branch latestCommit, commitResponse, err := githubClient.Repositories.GetCommit(context.TODO(), githubRepo.Org, githubRepo.Name, githubRepo.Branch, &github.ListOptions{}) defer commitResponse.Body.Close() diff --git a/pkg/status/versionchecks_test.go b/pkg/status/versionchecks_test.go index 25adc78f..9857328f 100644 --- a/pkg/status/versionchecks_test.go +++ b/pkg/status/versionchecks_test.go @@ -1,6 +1,7 @@ package status import ( + "context" "net/http" "testing" "time" @@ -37,7 +38,7 @@ func TestCheckDeployedVersionIsUpToDate(t *testing.T) { } // when - conditions := versionCheckMgr.CheckDeployedVersionIsUpToDate(false, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) + conditions := versionCheckMgr.CheckDeployedVersionIsUpToDate(context.TODO(), false, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) // then test.AssertConditionsMatchAndRecentTimestamps(t, []toolchainv1alpha1.Condition{*conditions}, expected) @@ -52,7 +53,7 @@ func TestCheckDeployedVersionIsUpToDate(t *testing.T) { } // when - conditions := versionCheckMgr.CheckDeployedVersionIsUpToDate(true, "", []toolchainv1alpha1.Condition{}, githubRepo) + conditions := versionCheckMgr.CheckDeployedVersionIsUpToDate(context.TODO(), true, "", []toolchainv1alpha1.Condition{}, githubRepo) // then test.AssertConditionsMatchAndRecentTimestamps(t, []toolchainv1alpha1.Condition{*conditions}, expected) @@ -77,7 +78,7 @@ func TestCheckDeployedVersionIsUpToDate(t *testing.T) { } // when - conditions := versionCheckMgrLastCAll.CheckDeployedVersionIsUpToDate(true, "githubToken", alreadyExistingConditions, githubRepo) + conditions := versionCheckMgrLastCAll.CheckDeployedVersionIsUpToDate(context.TODO(), true, "githubToken", alreadyExistingConditions, githubRepo) // then test.AssertConditionsMatchAndRecentTimestamps(t, []toolchainv1alpha1.Condition{*conditions}, expected) @@ -93,7 +94,7 @@ func TestCheckDeployedVersionIsUpToDate(t *testing.T) { } // when - conditions := versionCheckMgr.CheckDeployedVersionIsUpToDate(true, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) // deployed commit matches latest commit SHA in github + conditions := versionCheckMgr.CheckDeployedVersionIsUpToDate(context.TODO(), true, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) // deployed commit matches latest commit SHA in github // then test.AssertConditionsMatchAndRecentTimestamps(t, []toolchainv1alpha1.Condition{*conditions}, expected) @@ -116,7 +117,7 @@ func TestCheckDeployedVersionIsUpToDate(t *testing.T) { githubRepo.DeployedCommitSHA = "5678efgh" // deployed SHA is still at previous commit // when - conditions := versionCheckMgrThreshold.CheckDeployedVersionIsUpToDate(true, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) + conditions := versionCheckMgrThreshold.CheckDeployedVersionIsUpToDate(context.TODO(), true, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) // then test.AssertConditionsMatchAndRecentTimestamps(t, []toolchainv1alpha1.Condition{*conditions}, expected) @@ -138,7 +139,7 @@ func TestCheckDeployedVersionIsUpToDate(t *testing.T) { githubRepo.DeployedCommitSHA = "5678efgh" // deployed SHA is still at previous commit // when - conditions := versionCheckMgrThresholdExpired.CheckDeployedVersionIsUpToDate(true, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) + conditions := versionCheckMgrThresholdExpired.CheckDeployedVersionIsUpToDate(context.TODO(), true, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) // when test.AssertConditionsMatchAndRecentTimestamps(t, []toolchainv1alpha1.Condition{*conditions}, expected) @@ -152,7 +153,7 @@ func TestCheckDeployedVersionIsUpToDate(t *testing.T) { t.Run("internal server error from github", func(t *testing.T) { // given versionCheckMgrError := VersionCheckManager{ - GetGithubClientFunc: func(string) *github.Client { + GetGithubClientFunc: func(context.Context, string) *github.Client { mockedHTTPClient := mock.NewMockedHTTPClient( mock.WithRequestMatchHandler( test.GetReposCommitsByOwnerByRepoByRef, @@ -177,7 +178,7 @@ func TestCheckDeployedVersionIsUpToDate(t *testing.T) { } // when - conditions := versionCheckMgrError.CheckDeployedVersionIsUpToDate(true, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) + conditions := versionCheckMgrError.CheckDeployedVersionIsUpToDate(context.TODO(), true, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) // then test.AssertConditionsMatchAndRecentTimestamps(t, []toolchainv1alpha1.Condition{*conditions}, expected) @@ -186,7 +187,7 @@ func TestCheckDeployedVersionIsUpToDate(t *testing.T) { t.Run("response with no commits", func(t *testing.T) { // given versionCheckMgrError := VersionCheckManager{ - GetGithubClientFunc: func(string) *github.Client { + GetGithubClientFunc: func(context.Context, string) *github.Client { mockedHTTPClient := test.MockGithubRepositoryCommit(nil) return github.NewClient(mockedHTTPClient) }, @@ -200,7 +201,7 @@ func TestCheckDeployedVersionIsUpToDate(t *testing.T) { } // when - conditions := versionCheckMgrError.CheckDeployedVersionIsUpToDate(true, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) + conditions := versionCheckMgrError.CheckDeployedVersionIsUpToDate(context.TODO(), true, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) // then test.AssertConditionsMatchAndRecentTimestamps(t, []toolchainv1alpha1.Condition{*conditions}, expected) @@ -221,7 +222,7 @@ func TestCheckDeployedVersionIsUpToDate(t *testing.T) { } // when - conditions := versionCheckMgrNoCond.CheckDeployedVersionIsUpToDate(true, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) + conditions := versionCheckMgrNoCond.CheckDeployedVersionIsUpToDate(context.TODO(), true, "githubToken", []toolchainv1alpha1.Condition{}, githubRepo) // then test.AssertConditionsMatchAndRecentTimestamps(t, []toolchainv1alpha1.Condition{*conditions}, expected) diff --git a/pkg/test/github_client.go b/pkg/test/github_client.go index 39e52d00..828f0ca1 100644 --- a/pkg/test/github_client.go +++ b/pkg/test/github_client.go @@ -1,6 +1,7 @@ package test import ( + "context" "net/http" "time" @@ -20,7 +21,7 @@ func MockGitHubClientForRepositoryCommits(githubCommitSHA string, commitTimestam NewMockedGithubCommit(githubCommitSHA, commitTimestamp), ) mockedGitHubClient := github.NewClient(mockedHTTPClient) - return func(string) *github.Client { + return func(context.Context, string) *github.Client { return mockedGitHubClient } }