Skip to content

Commit

Permalink
fix: forward context in Github and REST clients (#342)
Browse files Browse the repository at this point in the history
  • Loading branch information
filariow authored Nov 17, 2023
1 parent 2bf1822 commit 3e7430a
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 21 deletions.
6 changes: 3 additions & 3 deletions pkg/client/github_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/client/rest_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand All @@ -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
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/client/rest_client_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package client_test

import (
"context"
"encoding/json"
"net/http"
"testing"
Expand All @@ -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)
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions pkg/status/versionchecks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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()
Expand Down
23 changes: 12 additions & 11 deletions pkg/status/versionchecks_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package status

import (
"context"
"net/http"
"testing"
"time"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -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)
},
Expand All @@ -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)
Expand All @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion pkg/test/github_client.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package test

import (
"context"
"net/http"
"time"

Expand All @@ -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
}
}
Expand Down

0 comments on commit 3e7430a

Please sign in to comment.