From e53187c7487e672722ec4e874944c7cb7d3d44b4 Mon Sep 17 00:00:00 2001 From: Samra Belachew Date: Fri, 30 Jun 2023 14:01:25 -0700 Subject: [PATCH] remove --- .../legacy/lyft/gateway/events_controller.go | 5 +- .../event/closed_pull_request_handler.go | 15 +- .../event/closed_pull_request_handler_test.go | 91 ----------- .../neptune/gateway/event/comment_handler.go | 21 +-- .../gateway/event/comment_handler_test.go | 146 +++++------------- .../event/modified_pull_request_handler.go | 18 +-- .../modified_pull_request_handler_test.go | 3 - .../event/pull_request_review_handler.go | 15 +- .../event/pull_request_review_handler_test.go | 43 ------ 9 files changed, 42 insertions(+), 315 deletions(-) diff --git a/server/legacy/lyft/gateway/events_controller.go b/server/legacy/lyft/gateway/events_controller.go index b05d9d882..fbe2f0b27 100644 --- a/server/legacy/lyft/gateway/events_controller.go +++ b/server/legacy/lyft/gateway/events_controller.go @@ -71,10 +71,9 @@ func NewVCSEventsController( } prSignaler := &pr.WorkflowSignaler{TemporalClient: temporalClient, DefaultTFVersion: defaultTFVersion} prRequirementChecker := requirement.NewPRAggregate(globalCfg) - modifiedPullHandler := gateway_handlers.NewModifiedPullHandler(logger, asyncScheduler, rootConfigBuilder, globalCfg, prRequirementChecker, prSignaler, legacyHandler, featureAllocator) + modifiedPullHandler := gateway_handlers.NewModifiedPullHandler(logger, asyncScheduler, rootConfigBuilder, globalCfg, prRequirementChecker, prSignaler, legacyHandler) closedPullHandler := &gateway_handlers.ClosedPullRequestHandler{ WorkerProxy: pullEventSNSProxy, - Allocator: featureAllocator, Logger: logger, PRCloseSignaler: prSignaler, Scope: scope.SubScope("pull.closed"), @@ -121,7 +120,6 @@ func NewVCSEventsController( logger, snsWriter, asyncScheduler, - featureAllocator, prSignaler, deploySignaler, vcsClient, @@ -160,7 +158,6 @@ func NewVCSEventsController( SnsWriter: snsWriter, Logger: logger, CheckRunFetcher: checkRunFetcher, - Allocator: featureAllocator, WorkflowSignaler: prSignaler, Scope: scope.SubScope("pull.review"), RootConfigBuilder: rootConfigBuilder, diff --git a/server/neptune/gateway/event/closed_pull_request_handler.go b/server/neptune/gateway/event/closed_pull_request_handler.go index 6f33ad2b9..bff466937 100644 --- a/server/neptune/gateway/event/closed_pull_request_handler.go +++ b/server/neptune/gateway/event/closed_pull_request_handler.go @@ -6,7 +6,6 @@ import ( "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/legacy/http" "github.com/runatlantis/atlantis/server/logging" - "github.com/runatlantis/atlantis/server/neptune/lyft/feature" "github.com/uber-go/tally/v4" "go.temporal.io/api/serviceerror" ) @@ -17,7 +16,6 @@ type prCloseSignaler interface { type ClosedPullRequestHandler struct { WorkerProxy workerProxy - Allocator feature.Allocator Logger logging.Logger PRCloseSignaler prCloseSignaler Scope tally.Scope @@ -35,18 +33,7 @@ func (c *ClosedPullRequestHandler) Handle(ctx context.Context, request *http.Buf } func (c *ClosedPullRequestHandler) handlePlatformMode(ctx context.Context, event PullRequest) error { - shouldAllocate, err := c.Allocator.ShouldAllocate(feature.LegacyDeprecation, feature.FeatureContext{ - RepoName: event.Pull.HeadRepo.FullName, - }) - if err != nil { - c.Logger.ErrorContext(ctx, "unable to allocate pr mode") - return nil - } - if !shouldAllocate { - c.Logger.InfoContext(ctx, "handler not configured for allocation") - return nil - } - err = c.PRCloseSignaler.SendCloseSignal(ctx, event.Pull.HeadRepo.FullName, event.Pull.Num) + err := c.PRCloseSignaler.SendCloseSignal(ctx, event.Pull.HeadRepo.FullName, event.Pull.Num) var workflowNotFoundErr *serviceerror.NotFound if errors.As(err, &workflowNotFoundErr) { diff --git a/server/neptune/gateway/event/closed_pull_request_handler_test.go b/server/neptune/gateway/event/closed_pull_request_handler_test.go index c9f4d88e7..fbf9dfc17 100644 --- a/server/neptune/gateway/event/closed_pull_request_handler_test.go +++ b/server/neptune/gateway/event/closed_pull_request_handler_test.go @@ -9,21 +9,12 @@ import ( "github.com/runatlantis/atlantis/server/logging" "github.com/runatlantis/atlantis/server/models" "github.com/runatlantis/atlantis/server/neptune/gateway/event" - "github.com/runatlantis/atlantis/server/neptune/lyft/feature" "github.com/stretchr/testify/assert" "github.com/uber-go/tally/v4" "go.temporal.io/api/serviceerror" ) func TestClosedPullHandler_Handle(t *testing.T) { - allocator := &testAllocator{ - expectedAllocation: true, - expectedFeatureID: feature.LegacyDeprecation, - expectedFeatureCtx: feature.FeatureContext{ - RepoName: repoFullName, - }, - t: t, - } workerProxy := &mockWorkerProxy{} signaler := &testCloseSignaler{ t: t, @@ -31,7 +22,6 @@ func TestClosedPullHandler_Handle(t *testing.T) { expectedPullNum: 1, } pullHandler := event.ClosedPullRequestHandler{ - Allocator: allocator, Logger: logging.NewNoopCtxLogger(t), WorkerProxy: workerProxy, PRCloseSignaler: signaler, @@ -51,78 +41,7 @@ func TestClosedPullHandler_Handle(t *testing.T) { assert.NoError(t, err) } -func TestClosedPullHandler_Handle_AllocationError(t *testing.T) { - allocator := &testAllocator{ - expectedError: assert.AnError, - expectedFeatureID: feature.LegacyDeprecation, - expectedFeatureCtx: feature.FeatureContext{ - RepoName: repoFullName, - }, - t: t, - } - workerProxy := &mockWorkerProxy{} - signaler := &testCloseSignaler{} - pullHandler := event.ClosedPullRequestHandler{ - Allocator: allocator, - Logger: logging.NewNoopCtxLogger(t), - WorkerProxy: workerProxy, - PRCloseSignaler: signaler, - } - pr := event.PullRequest{ - Pull: models.PullRequest{ - BaseRepo: testRepo, - HeadRepo: testRepo, - HeadBranch: "somebranch", - HeadCommit: "1234", - Num: 1, - }, - } - err := pullHandler.Handle(context.Background(), &http.BufferedRequest{}, pr) - assert.False(t, signaler.called) - assert.True(t, workerProxy.called) - assert.NoError(t, err) -} - -func TestClosedPullHandler_Handle_AllocationFail(t *testing.T) { - allocator := &testAllocator{ - expectedFeatureID: feature.LegacyDeprecation, - expectedFeatureCtx: feature.FeatureContext{ - RepoName: repoFullName, - }, - t: t, - } - workerProxy := &mockWorkerProxy{} - signaler := &testCloseSignaler{} - pullHandler := event.ClosedPullRequestHandler{ - Allocator: allocator, - Logger: logging.NewNoopCtxLogger(t), - WorkerProxy: workerProxy, - PRCloseSignaler: signaler, - } - pr := event.PullRequest{ - Pull: models.PullRequest{ - BaseRepo: testRepo, - HeadRepo: testRepo, - HeadBranch: "somebranch", - HeadCommit: "1234", - Num: 1, - }, - } - err := pullHandler.Handle(context.Background(), &http.BufferedRequest{}, pr) - assert.False(t, signaler.called) - assert.True(t, workerProxy.called) - assert.NoError(t, err) -} - func TestClosedPullHandler_Handle_SignalError(t *testing.T) { - allocator := &testAllocator{ - expectedAllocation: true, - expectedFeatureID: feature.LegacyDeprecation, - expectedFeatureCtx: feature.FeatureContext{ - RepoName: repoFullName, - }, - t: t, - } workerProxy := &mockWorkerProxy{} signaler := &testCloseSignaler{ t: t, @@ -131,7 +50,6 @@ func TestClosedPullHandler_Handle_SignalError(t *testing.T) { expectedPullNum: 1, } pullHandler := event.ClosedPullRequestHandler{ - Allocator: allocator, Logger: logging.NewNoopCtxLogger(t), WorkerProxy: workerProxy, PRCloseSignaler: signaler, @@ -152,14 +70,6 @@ func TestClosedPullHandler_Handle_SignalError(t *testing.T) { } func TestClosedPullHandler_Handle_SignalNotFoundError(t *testing.T) { - allocator := &testAllocator{ - expectedAllocation: true, - expectedFeatureID: feature.LegacyDeprecation, - expectedFeatureCtx: feature.FeatureContext{ - RepoName: repoFullName, - }, - t: t, - } workerProxy := &mockWorkerProxy{} signaler := &testCloseSignaler{ t: t, @@ -168,7 +78,6 @@ func TestClosedPullHandler_Handle_SignalNotFoundError(t *testing.T) { err: errors.Wrap(serviceerror.NewNotFound(""), "error wrapping"), } pullHandler := event.ClosedPullRequestHandler{ - Allocator: allocator, Logger: logging.NewNoopCtxLogger(t), WorkerProxy: workerProxy, PRCloseSignaler: signaler, diff --git a/server/neptune/gateway/event/comment_handler.go b/server/neptune/gateway/event/comment_handler.go index 8ca0f5ecb..c7c4ab728 100644 --- a/server/neptune/gateway/event/comment_handler.go +++ b/server/neptune/gateway/event/comment_handler.go @@ -6,9 +6,6 @@ import ( "time" "github.com/hashicorp/go-multierror" - "github.com/runatlantis/atlantis/server/neptune/gateway/pr" - "github.com/runatlantis/atlantis/server/neptune/lyft/feature" - "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/config/valid" "github.com/runatlantis/atlantis/server/legacy/events/command" @@ -17,6 +14,7 @@ import ( "github.com/runatlantis/atlantis/server/models" "github.com/runatlantis/atlantis/server/neptune/gateway/config" "github.com/runatlantis/atlantis/server/neptune/gateway/deploy" + "github.com/runatlantis/atlantis/server/neptune/gateway/pr" "github.com/runatlantis/atlantis/server/neptune/gateway/requirement" "github.com/runatlantis/atlantis/server/neptune/sync" "github.com/runatlantis/atlantis/server/neptune/workflows" @@ -69,7 +67,7 @@ func (c Comment) GetRepo() models.Repo { return c.BaseRepo } -func NewCommentEventWorkerProxy(logger logging.Logger, snsWriter Writer, scheduler scheduler, allocator feature.Allocator, prSignaler prSignaler, deploySignaler deploySignaler, commentCreator commentCreator, vcsStatusUpdater statusUpdater, globalCfg valid.GlobalCfg, rootConfigBuilder rootConfigBuilder, legacyErrorHandler errorHandler, neptuneErrorHandler errorHandler, requirementChecker requirementChecker) *CommentEventWorkerProxy { +func NewCommentEventWorkerProxy(logger logging.Logger, snsWriter Writer, scheduler scheduler, prSignaler prSignaler, deploySignaler deploySignaler, commentCreator commentCreator, vcsStatusUpdater statusUpdater, globalCfg valid.GlobalCfg, rootConfigBuilder rootConfigBuilder, legacyErrorHandler errorHandler, neptuneErrorHandler errorHandler, requirementChecker requirementChecker) *CommentEventWorkerProxy { return &CommentEventWorkerProxy{ logger: logger, scheduler: scheduler, @@ -84,7 +82,6 @@ func NewCommentEventWorkerProxy(logger logging.Logger, snsWriter Writer, schedul deploySignaler: deploySignaler, commentCreator: commentCreator, requirementChecker: requirementChecker, - allocator: allocator, prSignaler: prSignaler, }, vcsStatusUpdater: vcsStatusUpdater, @@ -99,7 +96,6 @@ type NeptuneWorkerProxy struct { deploySignaler deploySignaler commentCreator commentCreator requirementChecker requirementChecker - allocator feature.Allocator prSignaler prSignaler } @@ -107,19 +103,6 @@ func (p *NeptuneWorkerProxy) Handle(ctx context.Context, event Comment, cmd *com if cmd.Name == command.Apply { return p.handleApplies(ctx, event, cmd, roots) } - // TODO: remove when we begin in-depth testing and rollout of pr mode - // feature allocator is only temporary while we continue building out implementation - shouldAllocate, err := p.allocator.ShouldAllocate(feature.LegacyDeprecation, feature.FeatureContext{ - RepoName: event.Pull.HeadRepo.FullName, - }) - if err != nil { - p.logger.ErrorContext(ctx, "unable to allocate pr mode") - return nil - } - if !shouldAllocate { - p.logger.InfoContext(ctx, "handler not configured for allocation") - return nil - } prRequest := pr.Request{ Number: event.Pull.Num, Revision: event.Pull.HeadCommit, diff --git a/server/neptune/gateway/event/comment_handler_test.go b/server/neptune/gateway/event/comment_handler_test.go index 5c20897c9..8afff396f 100644 --- a/server/neptune/gateway/event/comment_handler_test.go +++ b/server/neptune/gateway/event/comment_handler_test.go @@ -5,9 +5,6 @@ import ( "fmt" "testing" - "github.com/runatlantis/atlantis/server/neptune/gateway/pr" - "github.com/runatlantis/atlantis/server/neptune/lyft/feature" - "github.com/runatlantis/atlantis/server/config/valid" "github.com/runatlantis/atlantis/server/legacy/events/command" "github.com/runatlantis/atlantis/server/logging" @@ -15,6 +12,7 @@ import ( "github.com/runatlantis/atlantis/server/neptune/gateway/config" "github.com/runatlantis/atlantis/server/neptune/gateway/deploy" "github.com/runatlantis/atlantis/server/neptune/gateway/event" + "github.com/runatlantis/atlantis/server/neptune/gateway/pr" "github.com/runatlantis/atlantis/server/neptune/gateway/requirement" "github.com/runatlantis/atlantis/server/neptune/sync" "github.com/runatlantis/atlantis/server/neptune/workflows" @@ -172,14 +170,10 @@ func TestCommentEventWorkerProxy_HandleForceApply(t *testing.T) { } statusUpdater := &mockStatusUpdater{} cfg := valid.NewGlobalCfg("somedir") - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - } prSignaler := &mockPRSignaler{ expectedT: t, } - commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, allocator, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) + commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) bufReq := buildRequest(t) cmd := &command.Comment{ Name: command.Apply, @@ -224,10 +218,6 @@ func TestCommentEventWorkerProxy_HandleApplyComment_RequirementsFailed(t *testin InstallationToken: 123, } testSignaler := &testDeploySignaler{} - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - } prSignaler := &mockPRSignaler{ expectedT: t, } @@ -237,7 +227,7 @@ func TestCommentEventWorkerProxy_HandleApplyComment_RequirementsFailed(t *testin commentCreator := &mockCommentCreator{} statusUpdater := &mockStatusUpdater{} cfg := valid.NewGlobalCfg("somedir") - commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, allocator, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{ + commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{ err: assert.AnError, }) bufReq := buildRequest(t) @@ -313,14 +303,10 @@ func TestCommentEventWorkerProxy_HandleApplyComment(t *testing.T) { commentCreator := &mockCommentCreator{} statusUpdater := &mockStatusUpdater{} cfg := valid.NewGlobalCfg("somedir") - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - } prSignaler := &mockPRSignaler{ expectedT: t, } - commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, allocator, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) + commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) bufReq := buildRequest(t) cmd := &command.Comment{ Name: command.Apply, @@ -388,14 +374,10 @@ func TestCommentEventWorkerProxy_HandlePlanComment_NoCmds(t *testing.T) { }, } cfg := valid.NewGlobalCfg("somedir") - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - } prSignaler := &mockPRSignaler{ expectedT: t, } - commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, allocator, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) + commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) bufReq := buildRequest(t) cmd := &command.Comment{ Name: command.Plan, @@ -447,14 +429,10 @@ func TestCommentEventWorkerProxy_HandleApplyComment_NoCmds(t *testing.T) { }, } cfg := valid.NewGlobalCfg("somedir") - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - } prSignaler := &mockPRSignaler{ expectedT: t, } - commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, allocator, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) + commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) bufReq := buildRequest(t) cmd := &command.Comment{ Name: command.Apply, @@ -469,6 +447,27 @@ func TestCommentEventWorkerProxy_HandleApplyComment_NoCmds(t *testing.T) { func TestCommentEventWorkerProxy_HandlePlanComment(t *testing.T) { logger := logging.NewNoopCtxLogger(t) + roots := []*valid.MergedProjectCfg{ + { + Name: "root2", + }, + } + token := int64(123) + prRequest := pr.Request{ + Number: testPull.Num, + Revision: testPull.HeadCommit, + Repo: testPull.HeadRepo, + InstallationToken: token, + Branch: testPull.HeadBranch, + ValidateEnvs: []pr.ValidateEnvs{ + { + PullNum: testPull.Num, + PullAuthor: testPull.Author, + HeadCommit: testPull.HeadCommit, + Username: "someuser", + }, + }, + } rootConfigBuilder := &mockRootConfigBuilder{ expectedT: t, expectedCommit: &config.RepoCommit{ @@ -478,13 +477,9 @@ func TestCommentEventWorkerProxy_HandlePlanComment(t *testing.T) { OptionalPRNum: testPull.Num, }, expectedToken: 123, - rootConfigs: []*valid.MergedProjectCfg{ - { - Name: "root2", - }, - }, + rootConfigs: roots, } - testSignaler := &testDeploySignaler{} + deploySignaler := &testDeploySignaler{} commentEvent := event.Comment{ Pull: testPull, PullNum: testPull.Num, @@ -507,14 +502,12 @@ func TestCommentEventWorkerProxy_HandlePlanComment(t *testing.T) { expectedT: t, } cfg := valid.NewGlobalCfg("somedir") - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - } prSignaler := &mockPRSignaler{ - expectedT: t, + expectedT: t, + expectedRoots: roots, + expectedPRRequest: prRequest, } - commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, allocator, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) + commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, deploySignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) bufReq := buildRequest(t) cmd := &command.Comment{ Name: command.Plan, @@ -523,7 +516,8 @@ func TestCommentEventWorkerProxy_HandlePlanComment(t *testing.T) { assert.NoError(t, err) assert.True(t, statusUpdater.isCalled) assert.False(t, commentCreator.isCalled) - assert.False(t, testSignaler.called) + assert.False(t, deploySignaler.called) + assert.True(t, prSignaler.called) assert.True(t, writer.isCalled) } @@ -584,17 +578,12 @@ func TestCommentEventWorkerProxy_HandlePlanCommentAllocatorEnabled(t *testing.T) expectedT: t, } cfg := valid.NewGlobalCfg("somedir") - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - expectedAllocation: true, - } prSignaler := &mockPRSignaler{ expectedT: t, expectedRoots: roots, expectedPRRequest: prRequest, } - commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, allocator, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) + commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) bufReq := buildRequest(t) cmd := &command.Comment{ Name: command.Plan, @@ -608,69 +597,6 @@ func TestCommentEventWorkerProxy_HandlePlanCommentAllocatorEnabled(t *testing.T) assert.True(t, prSignaler.called) } -func TestCommentEventWorkerProxy_WriteError(t *testing.T) { - logger := logging.NewNoopCtxLogger(t) - rootConfigBuilder := &mockRootConfigBuilder{ - expectedT: t, - expectedCommit: &config.RepoCommit{ - Repo: testRepo, - Branch: testPull.HeadBranch, - Sha: testPull.HeadCommit, - OptionalPRNum: testPull.Num, - }, - expectedToken: 123, - rootConfigs: []*valid.MergedProjectCfg{ - { - Name: "root2", - }, - }, - } - testSignaler := &testDeploySignaler{} - writer := &mockSnsWriter{ - err: assert.AnError, - } - scheduler := &sync.SynchronousScheduler{Logger: logger} - rootDeployer := &mockRootDeployer{} - commentCreator := &mockCommentCreator{} - statusUpdater := &mockStatusUpdater{ - expectedRepo: testRepo, - expectedPull: testPull, - expectedVCSStatus: models.QueuedVCSStatus, - expectedCmd: command.Plan.String(), - expectedBody: "Request received. Adding to the queue...", - expectedT: t, - } - cfg := valid.NewGlobalCfg("somedir") - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - } - prSignaler := &mockPRSignaler{ - expectedT: t, - } - commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, allocator, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{}) - bufReq := buildRequest(t) - commentEvent := event.Comment{ - Pull: testPull, - PullNum: testPull.Num, - BaseRepo: testRepo, - HeadRepo: testRepo, - User: models.User{ - Username: "someuser", - }, - InstallationToken: 123, - } - cmd := &command.Comment{ - Name: command.Plan, - } - err := commentEventWorkerProxy.Handle(context.Background(), bufReq, commentEvent, cmd) - assert.Error(t, err) - assert.True(t, statusUpdater.isCalled) - assert.False(t, commentCreator.isCalled) - assert.False(t, rootDeployer.isCalled) - assert.True(t, writer.isCalled) -} - type mockCommentCreator struct { isCalled bool expectedT *testing.T diff --git a/server/neptune/gateway/event/modified_pull_request_handler.go b/server/neptune/gateway/event/modified_pull_request_handler.go index 67fc6e7ca..5280b3b4d 100644 --- a/server/neptune/gateway/event/modified_pull_request_handler.go +++ b/server/neptune/gateway/event/modified_pull_request_handler.go @@ -9,7 +9,6 @@ import ( "github.com/runatlantis/atlantis/server/neptune/gateway/config" "github.com/runatlantis/atlantis/server/neptune/gateway/pr" "github.com/runatlantis/atlantis/server/neptune/gateway/requirement" - "github.com/runatlantis/atlantis/server/neptune/lyft/feature" "github.com/runatlantis/atlantis/server/vcs/provider/github" "go.temporal.io/sdk/client" @@ -34,7 +33,6 @@ type ModifiedPullHandler struct { GlobalCfg valid.GlobalCfg RequirementChecker requirementChecker LegacyHandler legacyHandler - Allocator feature.Allocator PRSignaler prSignaler } @@ -47,7 +45,7 @@ type PullRequest struct { InstallationToken int64 } -func NewModifiedPullHandler(logger logging.Logger, scheduler scheduler, rootConfigBuilder rootConfigBuilder, globalCfg valid.GlobalCfg, requirementChecker requirementChecker, prSignaler prSignaler, legacyHandler legacyHandler, allocator feature.Allocator) *ModifiedPullHandler { +func NewModifiedPullHandler(logger logging.Logger, scheduler scheduler, rootConfigBuilder rootConfigBuilder, globalCfg valid.GlobalCfg, requirementChecker requirementChecker, prSignaler prSignaler, legacyHandler legacyHandler) *ModifiedPullHandler { return &ModifiedPullHandler{ Logger: logger, Scheduler: scheduler, @@ -56,7 +54,6 @@ func NewModifiedPullHandler(logger logging.Logger, scheduler scheduler, rootConf RequirementChecker: requirementChecker, LegacyHandler: legacyHandler, PRSignaler: prSignaler, - Allocator: allocator, } } @@ -124,19 +121,6 @@ func (p *ModifiedPullHandler) handlePlatformMode(ctx context.Context, request *h if len(roots) == 0 { return nil } - // TODO: remove when we begin in-depth testing and rollout of pr mode - // feature allocator is only temporary while we continue building out implementation - shouldAllocate, err := p.Allocator.ShouldAllocate(feature.LegacyDeprecation, feature.FeatureContext{ - RepoName: event.Pull.HeadRepo.FullName, - }) - if err != nil { - p.Logger.ErrorContext(ctx, "unable to allocate pr mode") - return nil - } - if !shouldAllocate { - p.Logger.InfoContext(ctx, "handler not configured for allocation") - return nil - } prRequest := pr.Request{ Number: event.Pull.Num, Revision: event.Pull.HeadCommit, diff --git a/server/neptune/gateway/event/modified_pull_request_handler_test.go b/server/neptune/gateway/event/modified_pull_request_handler_test.go index 0df7da3b9..85f5643bb 100644 --- a/server/neptune/gateway/event/modified_pull_request_handler_test.go +++ b/server/neptune/gateway/event/modified_pull_request_handler_test.go @@ -81,7 +81,6 @@ func TestModifiedPullHandler_Handle_SignalerFailure(t *testing.T) { expectedT: t, expectedPRRequest: prRequest, }, - Allocator: &testFeatureAllocator{Enabled: true}, } err := pullHandler.Handle(context.Background(), &http.BufferedRequest{}, event.PullRequest{}) assert.ErrorContains(t, err, assert.AnError.Error()) @@ -156,7 +155,6 @@ func TestModifiedPullHandler_Handle_BranchStrategy(t *testing.T) { }, LegacyHandler: legacyHandler, PRSignaler: signaler, - Allocator: &testFeatureAllocator{Enabled: true}, } err := pullHandler.Handle(context.Background(), &http.BufferedRequest{}, pull) assert.NoError(t, err) @@ -225,7 +223,6 @@ func TestModifiedPullHandler_Handle_MergeStrategy(t *testing.T) { }, LegacyHandler: legacyHandler, PRSignaler: signaler, - Allocator: &testFeatureAllocator{Enabled: true}, } err := pullHandler.Handle(context.Background(), &http.BufferedRequest{}, pr) assert.NoError(t, err) diff --git a/server/neptune/gateway/event/pull_request_review_handler.go b/server/neptune/gateway/event/pull_request_review_handler.go index 370028ef3..6b7b85a5d 100644 --- a/server/neptune/gateway/event/pull_request_review_handler.go +++ b/server/neptune/gateway/event/pull_request_review_handler.go @@ -9,7 +9,6 @@ import ( "time" "github.com/runatlantis/atlantis/server/neptune/gateway/pr" - "github.com/runatlantis/atlantis/server/neptune/lyft/feature" "github.com/uber-go/tally/v4" "go.temporal.io/api/serviceerror" @@ -50,7 +49,6 @@ type PullRequestReviewWorkerProxy struct { SnsWriter Writer Logger logging.Logger CheckRunFetcher fetcher - Allocator feature.Allocator WorkflowSignaler workflowSignaler Scope tally.Scope RootConfigBuilder rootConfigBuilder @@ -109,18 +107,7 @@ func (p *PullRequestReviewWorkerProxy) handlePlatformMode(ctx context.Context, r if event.State != Approved && event.State != ChangesRequested { return nil } - shouldAllocate, err := p.Allocator.ShouldAllocate(feature.LegacyDeprecation, feature.FeatureContext{ - RepoName: event.Repo.FullName, - }) - if err != nil { - p.Logger.ErrorContext(ctx, "unable to allocate pr mode") - return nil - } - if !shouldAllocate { - p.Logger.InfoContext(ctx, "prr handler not configured for allocation") - return nil - } - + var err error switch event.State { case ChangesRequested: err = p.handleChangesRequestedEvent(ctx, event) diff --git a/server/neptune/gateway/event/pull_request_review_handler_test.go b/server/neptune/gateway/event/pull_request_review_handler_test.go index c79669d5d..3d3fdcc18 100644 --- a/server/neptune/gateway/event/pull_request_review_handler_test.go +++ b/server/neptune/gateway/event/pull_request_review_handler_test.go @@ -14,7 +14,6 @@ import ( "github.com/runatlantis/atlantis/server/logging" "github.com/runatlantis/atlantis/server/models" "github.com/runatlantis/atlantis/server/neptune/gateway/event" - "github.com/runatlantis/atlantis/server/neptune/lyft/feature" "github.com/runatlantis/atlantis/server/neptune/sync" "github.com/stretchr/testify/assert" "github.com/uber-go/tally/v4" @@ -40,12 +39,6 @@ func TestPullRequestReviewWorkerProxy_HandleApprovalWithFailedPolicies(t *testin failedPolicies: []string{"failed policy"}, } logger := logging.NewNoopCtxLogger(t) - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - expectedFeatureCtx: feature.FeatureContext{RepoName: repoFullName}, - expectedAllocation: true, - } signaler := &reviewSignaler{ t: t, expectedRepoName: "repo", @@ -57,7 +50,6 @@ func TestPullRequestReviewWorkerProxy_HandleApprovalWithFailedPolicies(t *testin SnsWriter: writer, Logger: logger, CheckRunFetcher: mockFetcher, - Allocator: allocator, WorkflowSignaler: signaler, Scope: tally.NewTestScope("", map[string]string{}), } @@ -75,12 +67,6 @@ func TestPullRequestReviewWorkerProxy_HandleApprovalWithFailedPolicies(t *testin func TestPullRequestReviewWorkerProxy_HandleChangesRequestedWithFailedPolicies(t *testing.T) { logger := logging.NewNoopCtxLogger(t) - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - expectedFeatureCtx: feature.FeatureContext{RepoName: repoFullName}, - expectedAllocation: true, - } signaler := &reviewSignaler{ t: t, expectedRepoName: "repo", @@ -104,7 +90,6 @@ func TestPullRequestReviewWorkerProxy_HandleChangesRequestedWithFailedPolicies(t proxy := event.PullRequestReviewWorkerProxy{ Scheduler: &sync.SynchronousScheduler{Logger: logger}, Logger: logger, - Allocator: allocator, WorkflowSignaler: signaler, Scope: tally.NewTestScope("", map[string]string{}), RootConfigBuilder: rootConfigBuilder, @@ -133,12 +118,6 @@ func TestPullRequestReviewWorkerProxy_HandleSuccessNoFailedPolicies(t *testing.T writer := &mockSnsWriter{} mockFetcher := &mockCheckRunFetcher{} logger := logging.NewNoopCtxLogger(t) - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - expectedFeatureCtx: feature.FeatureContext{RepoName: repoFullName}, - expectedAllocation: true, - } signaler := &reviewSignaler{ t: t, expectedRepoName: "repo", @@ -150,7 +129,6 @@ func TestPullRequestReviewWorkerProxy_HandleSuccessNoFailedPolicies(t *testing.T SnsWriter: writer, Logger: logger, CheckRunFetcher: mockFetcher, - Allocator: allocator, WorkflowSignaler: signaler, Scope: tally.NewTestScope("", map[string]string{}), } @@ -196,12 +174,6 @@ func TestPullRequestReviewWorkerProxy_FetcherError(t *testing.T) { err: assert.AnError, } logger := logging.NewNoopCtxLogger(t) - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - expectedFeatureCtx: feature.FeatureContext{RepoName: repoFullName}, - expectedAllocation: true, - } signaler := &reviewSignaler{ t: t, expectedRepoName: "repo", @@ -213,7 +185,6 @@ func TestPullRequestReviewWorkerProxy_FetcherError(t *testing.T) { SnsWriter: writer, Logger: logger, CheckRunFetcher: mockFetcher, - Allocator: allocator, WorkflowSignaler: signaler, Scope: tally.NewTestScope("", map[string]string{}), } @@ -235,12 +206,6 @@ func TestPullRequestReviewWorkerProxy_SNSError(t *testing.T) { failedPolicies: []string{"failed policy"}, } logger := logging.NewNoopCtxLogger(t) - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - expectedFeatureCtx: feature.FeatureContext{RepoName: repoFullName}, - expectedAllocation: true, - } signaler := &reviewSignaler{ t: t, expectedRepoName: "repo", @@ -252,7 +217,6 @@ func TestPullRequestReviewWorkerProxy_SNSError(t *testing.T) { SnsWriter: writer, Logger: logger, CheckRunFetcher: mockFetcher, - Allocator: allocator, WorkflowSignaler: signaler, Scope: tally.NewTestScope("", map[string]string{}), } @@ -275,12 +239,6 @@ func TestPullRequestReviewWorkerProxy_SignalerError(t *testing.T) { failedPolicies: []string{"failed policy"}, } logger := logging.NewNoopCtxLogger(t) - allocator := &testAllocator{ - t: t, - expectedFeatureID: feature.LegacyDeprecation, - expectedFeatureCtx: feature.FeatureContext{RepoName: repoFullName}, - expectedAllocation: true, - } signaler := &reviewSignaler{ t: t, expectedRepoName: "repo", @@ -293,7 +251,6 @@ func TestPullRequestReviewWorkerProxy_SignalerError(t *testing.T) { SnsWriter: writer, Logger: logger, CheckRunFetcher: mockFetcher, - Allocator: allocator, WorkflowSignaler: signaler, Scope: tally.NewTestScope("", map[string]string{}), }