diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 8eb89439f2..bc8e68622d 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -1328,7 +1328,7 @@ func (t *testGithubClient) GetRepoStatuses(repo models.Repo, pull models.PullReq return []*github.RepoStatus{}, nil } -func (t *testGithubClient) GetRepoChecks(repo models.Repo, pull models.PullRequest) ([]*github.CheckRun, error) { +func (t *testGithubClient) GetRepoChecks(repo models.Repo, commitSHA string) ([]*github.CheckRun, error) { return []*github.CheckRun{}, nil } diff --git a/server/events/vcs/instrumented_client.go b/server/events/vcs/instrumented_client.go index 450db2e37e..a6d97840e5 100644 --- a/server/events/vcs/instrumented_client.go +++ b/server/events/vcs/instrumented_client.go @@ -50,7 +50,7 @@ type IGithubClient interface { GetContents(owner, repo, branch, path string) ([]byte, error) GetRepoStatuses(repo models.Repo, pull models.PullRequest) ([]*github.RepoStatus, error) - GetRepoChecks(repo models.Repo, pull models.PullRequest) ([]*github.CheckRun, error) + GetRepoChecks(repo models.Repo, commitSHA string) ([]*github.CheckRun, error) } // InstrumentedGithubClient should delegate to the underlying InstrumentedClient for vcs provider-agnostic @@ -118,7 +118,7 @@ func (c *InstrumentedGithubClient) GetPullRequestFromName(repoName string, repoO return pull, err } -func (c *InstrumentedGithubClient) GetRepoChecks(repo models.Repo, pull models.PullRequest) ([]*github.CheckRun, error) { +func (c *InstrumentedGithubClient) GetRepoChecks(repo models.Repo, commitSHA string) ([]*github.CheckRun, error) { scope := c.StatsScope.SubScope("get_repo_checks") executionTime := scope.Timer(metrics.ExecutionTimeMetric).Start() @@ -127,7 +127,7 @@ func (c *InstrumentedGithubClient) GetRepoChecks(repo models.Repo, pull models.P executionSuccess := scope.Counter(metrics.ExecutionSuccessMetric) executionError := scope.Counter(metrics.ExecutionErrorMetric) - statuses, err := c.GhClient.GetRepoChecks(repo, pull.HeadCommit) + statuses, err := c.GhClient.GetRepoChecks(repo, commitSHA) if err != nil { executionError.Inc(1) @@ -137,7 +137,7 @@ func (c *InstrumentedGithubClient) GetRepoChecks(repo models.Repo, pull models.P executionSuccess.Inc(1) //TODO: thread context and use related logging methods. - c.Logger.Info("fetched vcs repo checks", fields.PullRequest(pull)) + c.Logger.Info("fetched vcs repo checks", map[string]interface{}{"commitSHA": commitSHA}) return statuses, err } diff --git a/server/events/vcs/lyft/pull_status_fetcher.go b/server/events/vcs/lyft/pull_status_fetcher.go index ce44f51672..3a04bcac2a 100644 --- a/server/events/vcs/lyft/pull_status_fetcher.go +++ b/server/events/vcs/lyft/pull_status_fetcher.go @@ -14,7 +14,7 @@ const LockValue = "lock" type PullClient interface { GetPullRequest(repo models.Repo, pullNum int) (*github.PullRequest, error) GetRepoStatuses(repo models.Repo, pull models.PullRequest) ([]*github.RepoStatus, error) - GetRepoChecks(repo models.Repo, pull models.PullRequest) ([]*github.CheckRun, error) + GetRepoChecks(repo models.Repo, commitSHA string) ([]*github.CheckRun, error) PullIsApproved(repo models.Repo, pull models.PullRequest) (models.ApprovalStatus, error) } @@ -48,7 +48,7 @@ func (s *SQBasedPullStatusFetcher) FetchPullStatus(repo models.Repo, pull models return pullStatus, errors.Wrap(err, "fetching repo statuses") } - checks, err := s.client.GetRepoChecks(repo, pull) + checks, err := s.client.GetRepoChecks(repo, pull.HeadCommit) if err != nil { return pullStatus, errors.Wrap(err, "fetching repo checks") } diff --git a/server/events/vcs/mocks/matchers/ptr_to_github_pullrequest.go b/server/events/vcs/mocks/matchers/ptr_to_github_pullrequest.go index 0ac17c2a2c..d7cc48f64b 100644 --- a/server/events/vcs/mocks/matchers/ptr_to_github_pullrequest.go +++ b/server/events/vcs/mocks/matchers/ptr_to_github_pullrequest.go @@ -2,9 +2,8 @@ package matchers import ( - "reflect" - "github.com/petergtz/pegomock" + "reflect" github "github.com/google/go-github/v31/github" ) diff --git a/server/events/vcs/mocks/matchers/slice_of_ptr_to_github_checkrun.go b/server/events/vcs/mocks/matchers/slice_of_ptr_to_github_checkrun.go index 11852576dc..52d7451ce7 100644 --- a/server/events/vcs/mocks/matchers/slice_of_ptr_to_github_checkrun.go +++ b/server/events/vcs/mocks/matchers/slice_of_ptr_to_github_checkrun.go @@ -2,9 +2,8 @@ package matchers import ( - "reflect" - "github.com/petergtz/pegomock" + "reflect" github "github.com/google/go-github/v31/github" ) diff --git a/server/events/vcs/mocks/matchers/slice_of_ptr_to_github_repostatus.go b/server/events/vcs/mocks/matchers/slice_of_ptr_to_github_repostatus.go index e9f6fd3c21..17c2141302 100644 --- a/server/events/vcs/mocks/matchers/slice_of_ptr_to_github_repostatus.go +++ b/server/events/vcs/mocks/matchers/slice_of_ptr_to_github_repostatus.go @@ -2,9 +2,8 @@ package matchers import ( - "reflect" - "github.com/petergtz/pegomock" + "reflect" github "github.com/google/go-github/v31/github" ) diff --git a/server/events/vcs/mocks/mock_IGithub_client.go b/server/events/vcs/mocks/mock_IGithub_client.go index 3024f6b717..8eab6d739c 100644 --- a/server/events/vcs/mocks/mock_IGithub_client.go +++ b/server/events/vcs/mocks/mock_IGithub_client.go @@ -4,12 +4,13 @@ package mocks import ( - "reflect" - "time" - + context "context" github "github.com/google/go-github/v31/github" pegomock "github.com/petergtz/pegomock" models "github.com/runatlantis/atlantis/server/events/models" + types "github.com/runatlantis/atlantis/server/events/vcs/types" + "reflect" + "time" ) type MockIGithubClient struct { @@ -141,7 +142,7 @@ func (mock *MockIGithubClient) GetPullRequestFromName(_param0 string, _param1 st return ret0, ret1 } -func (mock *MockIGithubClient) GetRepoChecks(_param0 models.Repo, _param1 models.PullRequest) ([]*github.CheckRun, error) { +func (mock *MockIGithubClient) GetRepoChecks(_param0 models.Repo, _param1 string) ([]*github.CheckRun, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockIGithubClient().") } @@ -266,11 +267,11 @@ func (mock *MockIGithubClient) SupportsSingleFileDownload(_param0 models.Repo) b return ret0 } -func (mock *MockIGithubClient) UpdateStatus(_param0 models.Repo, _param1 models.PullRequest, _param2 models.CommitStatus, _param3 string, _param4 string, _param5 string) error { +func (mock *MockIGithubClient) UpdateStatus(_param0 context.Context, _param1 types.UpdateStatusRequest) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockIGithubClient().") } - params := []pegomock.Param{_param0, _param1, _param2, _param3, _param4, _param5} + params := []pegomock.Param{_param0, _param1} result := pegomock.GetGenericMockFrom(mock).Invoke("UpdateStatus", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) var ret0 error if len(result) != 0 { @@ -520,7 +521,7 @@ func (c *MockIGithubClient_GetPullRequestFromName_OngoingVerification) GetAllCap return } -func (verifier *VerifierMockIGithubClient) GetRepoChecks(_param0 models.Repo, _param1 models.PullRequest) *MockIGithubClient_GetRepoChecks_OngoingVerification { +func (verifier *VerifierMockIGithubClient) GetRepoChecks(_param0 models.Repo, _param1 string) *MockIGithubClient_GetRepoChecks_OngoingVerification { params := []pegomock.Param{_param0, _param1} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GetRepoChecks", params, verifier.timeout) return &MockIGithubClient_GetRepoChecks_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} @@ -531,21 +532,21 @@ type MockIGithubClient_GetRepoChecks_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockIGithubClient_GetRepoChecks_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest) { +func (c *MockIGithubClient_GetRepoChecks_OngoingVerification) GetCapturedArguments() (models.Repo, string) { _param0, _param1 := c.GetAllCapturedArguments() return _param0[len(_param0)-1], _param1[len(_param1)-1] } -func (c *MockIGithubClient_GetRepoChecks_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest) { +func (c *MockIGithubClient_GetRepoChecks_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]models.Repo, len(c.methodInvocations)) for u, param := range params[0] { _param0[u] = param.(models.Repo) } - _param1 = make([]models.PullRequest, len(c.methodInvocations)) + _param1 = make([]string, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) + _param1[u] = param.(string) } } return @@ -733,8 +734,8 @@ func (c *MockIGithubClient_SupportsSingleFileDownload_OngoingVerification) GetAl return } -func (verifier *VerifierMockIGithubClient) UpdateStatus(_param0 models.Repo, _param1 models.PullRequest, _param2 models.CommitStatus, _param3 string, _param4 string, _param5 string) *MockIGithubClient_UpdateStatus_OngoingVerification { - params := []pegomock.Param{_param0, _param1, _param2, _param3, _param4, _param5} +func (verifier *VerifierMockIGithubClient) UpdateStatus(_param0 context.Context, _param1 types.UpdateStatusRequest) *MockIGithubClient_UpdateStatus_OngoingVerification { + params := []pegomock.Param{_param0, _param1} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "UpdateStatus", params, verifier.timeout) return &MockIGithubClient_UpdateStatus_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -744,37 +745,21 @@ type MockIGithubClient_UpdateStatus_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockIGithubClient_UpdateStatus_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest, models.CommitStatus, string, string, string) { - _param0, _param1, _param2, _param3, _param4, _param5 := c.GetAllCapturedArguments() - return _param0[len(_param0)-1], _param1[len(_param1)-1], _param2[len(_param2)-1], _param3[len(_param3)-1], _param4[len(_param4)-1], _param5[len(_param5)-1] +func (c *MockIGithubClient_UpdateStatus_OngoingVerification) GetCapturedArguments() (context.Context, types.UpdateStatusRequest) { + _param0, _param1 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1] } -func (c *MockIGithubClient_UpdateStatus_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest, _param2 []models.CommitStatus, _param3 []string, _param4 []string, _param5 []string) { +func (c *MockIGithubClient_UpdateStatus_OngoingVerification) GetAllCapturedArguments() (_param0 []context.Context, _param1 []types.UpdateStatusRequest) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]context.Context, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.Repo) + _param0[u] = param.(context.Context) } - _param1 = make([]models.PullRequest, len(c.methodInvocations)) + _param1 = make([]types.UpdateStatusRequest, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.PullRequest) - } - _param2 = make([]models.CommitStatus, len(c.methodInvocations)) - for u, param := range params[2] { - _param2[u] = param.(models.CommitStatus) - } - _param3 = make([]string, len(c.methodInvocations)) - for u, param := range params[3] { - _param3[u] = param.(string) - } - _param4 = make([]string, len(c.methodInvocations)) - for u, param := range params[4] { - _param4[u] = param.(string) - } - _param5 = make([]string, len(c.methodInvocations)) - for u, param := range params[5] { - _param5[u] = param.(string) + _param1[u] = param.(types.UpdateStatusRequest) } } return diff --git a/server/server.go b/server/server.go index df98844102..1c05af1958 100644 --- a/server/server.go +++ b/server/server.go @@ -175,6 +175,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { var bitbucketCloudClient *bitbucketcloud.Client var bitbucketServerClient *bitbucketserver.Client var azuredevopsClient *vcs.AzureDevopsClient + var featureAllocator feature.Allocator mergeabilityChecker := vcs.NewLyftPullMergeabilityChecker(userConfig.VCSStatusName) @@ -204,18 +205,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { return nil, errors.Wrapf(err, "instantiating metrics scope") } - featureAllocator, err := feature.NewGHSourcedAllocator( - feature.RepoConfig{ - Owner: userConfig.FFOwner, - Repo: userConfig.FFRepo, - Branch: userConfig.FFBranch, - Path: userConfig.FFPath, - }, githubClient, ctxLogger) - - if err != nil { - return nil, errors.Wrap(err, "initializing feature allocator") - } - if userConfig.GithubUser != "" || userConfig.GithubAppID != 0 { supportedVCSHosts = append(supportedVCSHosts, models.Github) if userConfig.GithubUser != "" { @@ -252,6 +241,18 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { return nil, err } + featureAllocator, err = feature.NewGHSourcedAllocator( + feature.RepoConfig{ + Owner: userConfig.FFOwner, + Repo: userConfig.FFRepo, + Branch: userConfig.FFBranch, + Path: userConfig.FFPath, + }, rawGithubClient, ctxLogger) + + if err != nil { + return nil, errors.Wrap(err, "initializing feature allocator") + } + // [WENGINES-4643] TODO: Remove this wrapped client once github checks is stable checksWrapperGhClient := &lyft_checks.ChecksClientWrapper{ FeatureAllocator: featureAllocator,