From e5ec74dca5499d0378ba7a9bc03220492da69f05 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 28 Jul 2021 16:52:58 -0700 Subject: [PATCH 01/13] Added Unlocked Apply Requirement. --- cmd/server.go | 5 +++++ server/events/yaml/raw/project.go | 5 +++-- server/events/yaml/valid/global_cfg.go | 9 +++++++-- server/server.go | 1 + server/user_config.go | 3 +++ 5 files changed, 19 insertions(+), 4 deletions(-) diff --git a/cmd/server.go b/cmd/server.go index afa272f19..ea861272b 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -102,6 +102,7 @@ const ( VCSStatusName = "vcs-status-name" TFEHostnameFlag = "tfe-hostname" TFETokenFlag = "tfe-token" + UnlockedReq = "require-unlocked" WriteGitCredsFlag = "write-git-creds" // NOTE: Must manually set these as defaults in the setDefaults function. @@ -377,6 +378,10 @@ var boolFlags = map[string]boolFlag{ description: "Skips cloning the PR repo if there are no projects were changed in the PR.", defaultValue: false, }, + UnlockedReq: { + description: "Allows a user to lock a PR", + defaultValue: true, + }, } var intFlags = map[string]intFlag{ ParallelPoolSize: { diff --git a/server/events/yaml/raw/project.go b/server/events/yaml/raw/project.go index 4fe32bacb..06fc11c2d 100644 --- a/server/events/yaml/raw/project.go +++ b/server/events/yaml/raw/project.go @@ -17,6 +17,7 @@ const ( ApprovedApplyRequirement = "approved" MergeableApplyRequirement = "mergeable" UnDivergedApplyRequirement = "undiverged" + UnLockedApplyRequirement = "unlocked" ) type Project struct { @@ -107,8 +108,8 @@ func validProjectName(name string) bool { func validApplyReq(value interface{}) error { reqs := value.([]string) for _, r := range reqs { - if r != ApprovedApplyRequirement && r != MergeableApplyRequirement && r != UnDivergedApplyRequirement { - return fmt.Errorf("%q is not a valid apply_requirement, only %q, %q and %q are supported", r, ApprovedApplyRequirement, MergeableApplyRequirement, UnDivergedApplyRequirement) + if r != ApprovedApplyRequirement && r != MergeableApplyRequirement && r != UnDivergedApplyRequirement && r!= UnLockedApplyRequirement { + return fmt.Errorf("%q is not a valid apply_requirement, only %q, %q, %q and %q are supported", r, ApprovedApplyRequirement, MergeableApplyRequirement, UnDivergedApplyRequirement, UnLockedApplyRequirement) } } return nil diff --git a/server/events/yaml/valid/global_cfg.go b/server/events/yaml/valid/global_cfg.go index c2c4ced7d..419b557bb 100644 --- a/server/events/yaml/valid/global_cfg.go +++ b/server/events/yaml/valid/global_cfg.go @@ -12,6 +12,7 @@ import ( const MergeableApplyReq = "mergeable" const ApprovedApplyReq = "approved" const UnDivergedApplyReq = "undiverged" +const UnlockedApplyReq = "unlocked" const PoliciesPassedApplyReq = "policies_passed" const ApplyRequirementsKey = "apply_requirements" const PreWorkflowHooksKey = "pre_workflow_hooks" @@ -109,12 +110,13 @@ var DefaultPlanStage = Stage{ } // Deprecated: use NewGlobalCfgFromArgs -func NewGlobalCfgWithHooks(allowRepoCfg bool, mergeableReq bool, approvedReq bool, unDivergedReq bool, preWorkflowHooks []*PreWorkflowHook) GlobalCfg { +func NewGlobalCfgWithHooks(allowRepoCfg bool, mergeableReq bool, approvedReq bool, unDivergedReq bool, unlockedReq bool, preWorkflowHooks []*PreWorkflowHook) GlobalCfg { return NewGlobalCfgFromArgs(GlobalCfgArgs{ AllowRepoCfg: allowRepoCfg, MergeableReq: mergeableReq, ApprovedReq: approvedReq, UnDivergedReq: unDivergedReq, + UnLockedReq: unlockedReq, PreWorkflowHooks: preWorkflowHooks, }) } @@ -139,6 +141,7 @@ type GlobalCfgArgs struct { MergeableReq bool ApprovedReq bool UnDivergedReq bool + UnLockedReq bool PolicyCheckEnabled bool PreWorkflowHooks []*PreWorkflowHook } @@ -164,7 +167,9 @@ func NewGlobalCfgFromArgs(args GlobalCfgArgs) GlobalCfg { if args.UnDivergedReq { applyReqs = append(applyReqs, UnDivergedApplyReq) } - + if args.UnLockedReq { + applyReqs = append(applyReqs, UnlockedApplyReq) + } if args.PolicyCheckEnabled { applyReqs = append(applyReqs, PoliciesPassedApplyReq) } diff --git a/server/server.go b/server/server.go index 33a40497e..5dac1ff89 100644 --- a/server/server.go +++ b/server/server.go @@ -370,6 +370,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { MergeableReq: userConfig.RequireMergeable, ApprovedReq: userConfig.RequireApproval, UnDivergedReq: userConfig.RequireUnDiverged, + UnLockedReq: userConfig.RequireUnlocked, PolicyCheckEnabled: userConfig.EnablePolicyChecksFlag, }) if userConfig.RepoConfig != "" { diff --git a/server/user_config.go b/server/user_config.go index 0b23a0318..c39d53787 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -66,6 +66,9 @@ type UserConfig struct { // RequireUnDiverged is whether to require pull requests to rebase default branch before // allowing terraform apply's to run. RequireUnDiverged bool `mapstructure:"require-undiverged"` + // RequireUnlocked is where to require pull requests to be unlocked before running + // terraform apply. + RequireUnlocked bool `mapstructure:"require-unlocked"` SilenceForkPRErrors bool `mapstructure:"silence-fork-pr-errors"` // SilenceVCSStatusNoPlans is whether autoplan should set commit status if no plans // are found. From 5c390f0e93983efef8d7ae8f3d45c0d5529f63ca Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Mon, 2 Aug 2021 12:07:31 -0700 Subject: [PATCH 02/13] Renaming flags for consistency. --- cmd/server.go | 10 +- server/events/mocks/mock_apply_handler.go | 121 ++++++++++++++++++++++ server/events/yaml/valid/global_cfg.go | 4 +- server/user_config.go | 6 +- 4 files changed, 131 insertions(+), 10 deletions(-) create mode 100644 server/events/mocks/mock_apply_handler.go diff --git a/cmd/server.go b/cmd/server.go index ea861272b..20ce5bdd3 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -87,6 +87,7 @@ const ( RepoWhitelistFlag = "repo-whitelist" RepoAllowlistFlag = "repo-allowlist" RequireApprovalFlag = "require-approval" + RequireUnlockedFlag = "require-unlocked" RequireMergeableFlag = "require-mergeable" SilenceNoProjectsFlag = "silence-no-projects" SilenceForkPRErrorsFlag = "silence-fork-pr-errors" @@ -102,7 +103,6 @@ const ( VCSStatusName = "vcs-status-name" TFEHostnameFlag = "tfe-hostname" TFETokenFlag = "tfe-token" - UnlockedReq = "require-unlocked" WriteGitCredsFlag = "write-git-creds" // NOTE: Must manually set these as defaults in the setDefaults function. @@ -344,6 +344,10 @@ var boolFlags = map[string]boolFlag{ defaultValue: false, hidden: true, }, + RequireUnlockedFlag: { + description: "Require pull requests to be \"Unlocked\" before allowing the apply command to be run.", + defaultValue: false, + }, SilenceNoProjectsFlag: { description: "Silences Atlants from responding to PRs when it finds no projects.", defaultValue: false, @@ -378,10 +382,6 @@ var boolFlags = map[string]boolFlag{ description: "Skips cloning the PR repo if there are no projects were changed in the PR.", defaultValue: false, }, - UnlockedReq: { - description: "Allows a user to lock a PR", - defaultValue: true, - }, } var intFlags = map[string]intFlag{ ParallelPoolSize: { diff --git a/server/events/mocks/mock_apply_handler.go b/server/events/mocks/mock_apply_handler.go new file mode 100644 index 000000000..e541f6a15 --- /dev/null +++ b/server/events/mocks/mock_apply_handler.go @@ -0,0 +1,121 @@ +// Code generated by pegomock. DO NOT EDIT. +// Source: github.com/runatlantis/atlantis/server/events (interfaces: ApplyRequirementHandler) + +package mocks + +import ( + pegomock "github.com/petergtz/pegomock" + models "github.com/runatlantis/atlantis/server/events/models" + "reflect" + "time" +) + +type MockApplyRequirementHandler struct { + fail func(message string, callerSkip ...int) +} + +func NewMockApplyRequirementHandler(options ...pegomock.Option) *MockApplyRequirementHandler { + mock := &MockApplyRequirementHandler{} + for _, option := range options { + option.Apply(mock) + } + return mock +} + +func (mock *MockApplyRequirementHandler) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } +func (mock *MockApplyRequirementHandler) FailHandler() pegomock.FailHandler { return mock.fail } + +func (mock *MockApplyRequirementHandler) HandleApplyRequirements(_param0 string, _param1 models.ProjectCommandContext) (string, string, bool, error) { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockApplyRequirementHandler().") + } + params := []pegomock.Param{_param0, _param1} + result := pegomock.GetGenericMockFrom(mock).Invoke("HandleApplyRequirements", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem(), reflect.TypeOf((*string)(nil)).Elem(), reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) + var ret0 string + var ret1 string + var ret2 bool + var ret3 error + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(string) + } + if result[1] != nil { + ret1 = result[1].(string) + } + if result[2] != nil { + ret2 = result[2].(bool) + } + if result[3] != nil { + ret3 = result[3].(error) + } + } + return ret0, ret1, ret2, ret3 +} + +func (mock *MockApplyRequirementHandler) VerifyWasCalledOnce() *VerifierMockApplyRequirementHandler { + return &VerifierMockApplyRequirementHandler{ + mock: mock, + invocationCountMatcher: pegomock.Times(1), + } +} + +func (mock *MockApplyRequirementHandler) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockApplyRequirementHandler { + return &VerifierMockApplyRequirementHandler{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + } +} + +func (mock *MockApplyRequirementHandler) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockApplyRequirementHandler { + return &VerifierMockApplyRequirementHandler{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + inOrderContext: inOrderContext, + } +} + +func (mock *MockApplyRequirementHandler) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockApplyRequirementHandler { + return &VerifierMockApplyRequirementHandler{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + timeout: timeout, + } +} + +type VerifierMockApplyRequirementHandler struct { + mock *MockApplyRequirementHandler + invocationCountMatcher pegomock.InvocationCountMatcher + inOrderContext *pegomock.InOrderContext + timeout time.Duration +} + +func (verifier *VerifierMockApplyRequirementHandler) HandleApplyRequirements(_param0 string, _param1 models.ProjectCommandContext) *MockApplyRequirementHandler_HandleApplyRequirements_OngoingVerification { + params := []pegomock.Param{_param0, _param1} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "HandleApplyRequirements", params, verifier.timeout) + return &MockApplyRequirementHandler_HandleApplyRequirements_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockApplyRequirementHandler_HandleApplyRequirements_OngoingVerification struct { + mock *MockApplyRequirementHandler + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockApplyRequirementHandler_HandleApplyRequirements_OngoingVerification) GetCapturedArguments() (string, models.ProjectCommandContext) { + _param0, _param1 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1] +} + +func (c *MockApplyRequirementHandler_HandleApplyRequirements_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []models.ProjectCommandContext) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]string, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(string) + } + _param1 = make([]models.ProjectCommandContext, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(models.ProjectCommandContext) + } + } + return +} diff --git a/server/events/yaml/valid/global_cfg.go b/server/events/yaml/valid/global_cfg.go index 419b557bb..5b3046d89 100644 --- a/server/events/yaml/valid/global_cfg.go +++ b/server/events/yaml/valid/global_cfg.go @@ -110,13 +110,13 @@ var DefaultPlanStage = Stage{ } // Deprecated: use NewGlobalCfgFromArgs -func NewGlobalCfgWithHooks(allowRepoCfg bool, mergeableReq bool, approvedReq bool, unDivergedReq bool, unlockedReq bool, preWorkflowHooks []*PreWorkflowHook) GlobalCfg { +func NewGlobalCfgWithHooks(allowRepoCfg bool, mergeableReq bool, approvedReq bool, unDivergedReq bool, unLockedReq bool, preWorkflowHooks []*PreWorkflowHook) GlobalCfg { return NewGlobalCfgFromArgs(GlobalCfgArgs{ AllowRepoCfg: allowRepoCfg, MergeableReq: mergeableReq, ApprovedReq: approvedReq, UnDivergedReq: unDivergedReq, - UnLockedReq: unlockedReq, + UnLockedReq: unLockedReq, PreWorkflowHooks: preWorkflowHooks, }) } diff --git a/server/user_config.go b/server/user_config.go index c39d53787..feb245c5f 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -65,9 +65,9 @@ type UserConfig struct { SilenceNoProjects bool `mapstructure:"silence-no-projects"` // RequireUnDiverged is whether to require pull requests to rebase default branch before // allowing terraform apply's to run. - RequireUnDiverged bool `mapstructure:"require-undiverged"` - // RequireUnlocked is where to require pull requests to be unlocked before running - // terraform apply. + RequireUnDiverged bool `mapstructure:"require-undiverged"` + // RequireUnlocked is whether to require pull requests to be unlocked before running + // terraform apply. RequireUnlocked bool `mapstructure:"require-unlocked"` SilenceForkPRErrors bool `mapstructure:"silence-fork-pr-errors"` // SilenceVCSStatusNoPlans is whether autoplan should set commit status if no plans From 744353e0967c7af4c82ce65e0edd5a8bf11e676e Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Mon, 2 Aug 2021 12:09:36 -0700 Subject: [PATCH 03/13] Removed irrelevant mock file. --- server/events/mocks/mock_apply_handler.go | 121 ---------------------- 1 file changed, 121 deletions(-) delete mode 100644 server/events/mocks/mock_apply_handler.go diff --git a/server/events/mocks/mock_apply_handler.go b/server/events/mocks/mock_apply_handler.go deleted file mode 100644 index e541f6a15..000000000 --- a/server/events/mocks/mock_apply_handler.go +++ /dev/null @@ -1,121 +0,0 @@ -// Code generated by pegomock. DO NOT EDIT. -// Source: github.com/runatlantis/atlantis/server/events (interfaces: ApplyRequirementHandler) - -package mocks - -import ( - pegomock "github.com/petergtz/pegomock" - models "github.com/runatlantis/atlantis/server/events/models" - "reflect" - "time" -) - -type MockApplyRequirementHandler struct { - fail func(message string, callerSkip ...int) -} - -func NewMockApplyRequirementHandler(options ...pegomock.Option) *MockApplyRequirementHandler { - mock := &MockApplyRequirementHandler{} - for _, option := range options { - option.Apply(mock) - } - return mock -} - -func (mock *MockApplyRequirementHandler) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } -func (mock *MockApplyRequirementHandler) FailHandler() pegomock.FailHandler { return mock.fail } - -func (mock *MockApplyRequirementHandler) HandleApplyRequirements(_param0 string, _param1 models.ProjectCommandContext) (string, string, bool, error) { - if mock == nil { - panic("mock must not be nil. Use myMock := NewMockApplyRequirementHandler().") - } - params := []pegomock.Param{_param0, _param1} - result := pegomock.GetGenericMockFrom(mock).Invoke("HandleApplyRequirements", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem(), reflect.TypeOf((*string)(nil)).Elem(), reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) - var ret0 string - var ret1 string - var ret2 bool - var ret3 error - if len(result) != 0 { - if result[0] != nil { - ret0 = result[0].(string) - } - if result[1] != nil { - ret1 = result[1].(string) - } - if result[2] != nil { - ret2 = result[2].(bool) - } - if result[3] != nil { - ret3 = result[3].(error) - } - } - return ret0, ret1, ret2, ret3 -} - -func (mock *MockApplyRequirementHandler) VerifyWasCalledOnce() *VerifierMockApplyRequirementHandler { - return &VerifierMockApplyRequirementHandler{ - mock: mock, - invocationCountMatcher: pegomock.Times(1), - } -} - -func (mock *MockApplyRequirementHandler) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockApplyRequirementHandler { - return &VerifierMockApplyRequirementHandler{ - mock: mock, - invocationCountMatcher: invocationCountMatcher, - } -} - -func (mock *MockApplyRequirementHandler) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockApplyRequirementHandler { - return &VerifierMockApplyRequirementHandler{ - mock: mock, - invocationCountMatcher: invocationCountMatcher, - inOrderContext: inOrderContext, - } -} - -func (mock *MockApplyRequirementHandler) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockApplyRequirementHandler { - return &VerifierMockApplyRequirementHandler{ - mock: mock, - invocationCountMatcher: invocationCountMatcher, - timeout: timeout, - } -} - -type VerifierMockApplyRequirementHandler struct { - mock *MockApplyRequirementHandler - invocationCountMatcher pegomock.InvocationCountMatcher - inOrderContext *pegomock.InOrderContext - timeout time.Duration -} - -func (verifier *VerifierMockApplyRequirementHandler) HandleApplyRequirements(_param0 string, _param1 models.ProjectCommandContext) *MockApplyRequirementHandler_HandleApplyRequirements_OngoingVerification { - params := []pegomock.Param{_param0, _param1} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "HandleApplyRequirements", params, verifier.timeout) - return &MockApplyRequirementHandler_HandleApplyRequirements_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} -} - -type MockApplyRequirementHandler_HandleApplyRequirements_OngoingVerification struct { - mock *MockApplyRequirementHandler - methodInvocations []pegomock.MethodInvocation -} - -func (c *MockApplyRequirementHandler_HandleApplyRequirements_OngoingVerification) GetCapturedArguments() (string, models.ProjectCommandContext) { - _param0, _param1 := c.GetAllCapturedArguments() - return _param0[len(_param0)-1], _param1[len(_param1)-1] -} - -func (c *MockApplyRequirementHandler_HandleApplyRequirements_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []models.ProjectCommandContext) { - params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) - if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) - for u, param := range params[0] { - _param0[u] = param.(string) - } - _param1 = make([]models.ProjectCommandContext, len(c.methodInvocations)) - for u, param := range params[1] { - _param1[u] = param.(models.ProjectCommandContext) - } - } - return -} From d50f9beb0625e32894b5f673fac4b86131f72244 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Mon, 2 Aug 2021 12:15:17 -0700 Subject: [PATCH 04/13] Fix failing tests --- server/events/yaml/parser_validator_test.go | 2 +- server/events/yaml/raw/project_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/events/yaml/parser_validator_test.go b/server/events/yaml/parser_validator_test.go index a722585e0..0e0deae25 100644 --- a/server/events/yaml/parser_validator_test.go +++ b/server/events/yaml/parser_validator_test.go @@ -1230,7 +1230,7 @@ func TestParseGlobalCfg(t *testing.T) { input: `repos: - id: /.*/ apply_requirements: [invalid]`, - expErr: "repos: (0: (apply_requirements: \"invalid\" is not a valid apply_requirement, only \"approved\", \"mergeable\" and \"undiverged\" are supported.).).", + expErr: "repos: (0: (apply_requirements: \"invalid\" is not a valid apply_requirement, only \"approved\", \"mergeable\", \"undiverged\" and \"unlocked\" are supported.).).", }, "no workflows key": { input: `repos: []`, diff --git a/server/events/yaml/raw/project_test.go b/server/events/yaml/raw/project_test.go index fce646a20..30b16b39b 100644 --- a/server/events/yaml/raw/project_test.go +++ b/server/events/yaml/raw/project_test.go @@ -101,7 +101,7 @@ func TestProject_Validate(t *testing.T) { Dir: String("."), ApplyRequirements: []string{"unsupported"}, }, - expErr: "apply_requirements: \"unsupported\" is not a valid apply_requirement, only \"approved\", \"mergeable\" and \"undiverged\" are supported.", + expErr: "apply_requirements: \"unsupported\" is not a valid apply_requirement, only \"approved\", \"mergeable\", \"undiverged\" and \"unlocked\" are supported.", }, { description: "apply reqs with approved requirement", From 3450fd36b101288ddc3766a25c9497aff59a6726 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Mon, 2 Aug 2021 12:31:27 -0700 Subject: [PATCH 05/13] Fixed linting --- server/events/models/models.go | 2 +- server/events/yaml/raw/project.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/events/models/models.go b/server/events/models/models.go index e897130dd..1724248b9 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -423,7 +423,7 @@ func (p ProjectCommandContext) ProjectCloneDir() string { // SetScope sets the scope of the stats object field. Note: we deliberately set this on the value // instead of a pointer since we want scopes to mirror our function stack func (p ProjectCommandContext) SetScope(scope string) { - p.Scope = p.Scope.Scope(scope) + p.Scope = p.Scope.Scope(scope) //nolint } // GetShowResultFileName returns the filename (not the path) to store the tf show result diff --git a/server/events/yaml/raw/project.go b/server/events/yaml/raw/project.go index 06fc11c2d..16e7e03c4 100644 --- a/server/events/yaml/raw/project.go +++ b/server/events/yaml/raw/project.go @@ -108,7 +108,7 @@ func validProjectName(name string) bool { func validApplyReq(value interface{}) error { reqs := value.([]string) for _, r := range reqs { - if r != ApprovedApplyRequirement && r != MergeableApplyRequirement && r != UnDivergedApplyRequirement && r!= UnLockedApplyRequirement { + if r != ApprovedApplyRequirement && r != MergeableApplyRequirement && r != UnDivergedApplyRequirement && r != UnLockedApplyRequirement { return fmt.Errorf("%q is not a valid apply_requirement, only %q, %q, %q and %q are supported", r, ApprovedApplyRequirement, MergeableApplyRequirement, UnDivergedApplyRequirement, UnLockedApplyRequirement) } } From 4a930af95462b9a6676030d7800f06da3fd4de82 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 4 Aug 2021 10:06:38 -0700 Subject: [PATCH 06/13] Addressing PR comments. --- cmd/server.go | 4 +-- server/events/yaml/parser_validator_test.go | 2 +- server/events/yaml/raw/project.go | 30 +++++++++++++++------ server/events/yaml/raw/project_test.go | 2 +- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/cmd/server.go b/cmd/server.go index 20ce5bdd3..62f2f4b2b 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -87,7 +87,7 @@ const ( RepoWhitelistFlag = "repo-whitelist" RepoAllowlistFlag = "repo-allowlist" RequireApprovalFlag = "require-approval" - RequireUnlockedFlag = "require-unlocked" + RequireSQUnlockedFlag = "require-unlocked" RequireMergeableFlag = "require-mergeable" SilenceNoProjectsFlag = "silence-no-projects" SilenceForkPRErrorsFlag = "silence-fork-pr-errors" @@ -344,7 +344,7 @@ var boolFlags = map[string]boolFlag{ defaultValue: false, hidden: true, }, - RequireUnlockedFlag: { + RequireSQUnlockedFlag: { description: "Require pull requests to be \"Unlocked\" before allowing the apply command to be run.", defaultValue: false, }, diff --git a/server/events/yaml/parser_validator_test.go b/server/events/yaml/parser_validator_test.go index 0e0deae25..00b8d23e2 100644 --- a/server/events/yaml/parser_validator_test.go +++ b/server/events/yaml/parser_validator_test.go @@ -1230,7 +1230,7 @@ func TestParseGlobalCfg(t *testing.T) { input: `repos: - id: /.*/ apply_requirements: [invalid]`, - expErr: "repos: (0: (apply_requirements: \"invalid\" is not a valid apply_requirement, only \"approved\", \"mergeable\", \"undiverged\" and \"unlocked\" are supported.).).", + expErr: "repos: (0: (apply_requirements: \"invalid\" is not a valid apply_requirement, only \"approved\", \"mergeable\", \"undiverged\", and \"unlocked\" are supported.).).", }, "no workflows key": { input: `repos: []`, diff --git a/server/events/yaml/raw/project.go b/server/events/yaml/raw/project.go index 16e7e03c4..d4b129cf4 100644 --- a/server/events/yaml/raw/project.go +++ b/server/events/yaml/raw/project.go @@ -13,13 +13,11 @@ import ( ) const ( - DefaultWorkspace = "default" - ApprovedApplyRequirement = "approved" - MergeableApplyRequirement = "mergeable" - UnDivergedApplyRequirement = "undiverged" - UnLockedApplyRequirement = "unlocked" + DefaultWorkspace = "default" ) +var applyRequirements = [...]string{"approved", "mergeable", "undiverged", "unlocked"} + type Project struct { Name *string `yaml:"name,omitempty"` Dir *string `yaml:"dir,omitempty"` @@ -107,10 +105,26 @@ func validProjectName(name string) bool { func validApplyReq(value interface{}) error { reqs := value.([]string) - for _, r := range reqs { - if r != ApprovedApplyRequirement && r != MergeableApplyRequirement && r != UnDivergedApplyRequirement && r != UnLockedApplyRequirement { - return fmt.Errorf("%q is not a valid apply_requirement, only %q, %q, %q and %q are supported", r, ApprovedApplyRequirement, MergeableApplyRequirement, UnDivergedApplyRequirement, UnLockedApplyRequirement) + for _, req := range reqs { + isValid := false + for _, applyRequirement := range applyRequirements { + if req == applyRequirement { + isValid = true + break + } + } + if !isValid { + return fmt.Errorf("%q is not a valid apply_requirement, only %s are supported", req, buildValidApplyRequirementsString()) } } return nil } + +func buildValidApplyRequirementsString() string { + var returnString strings.Builder + for _, applyReq := range applyRequirements[:len(applyRequirements)-1] { + returnString.WriteString(fmt.Sprintf("%q, ", applyReq)) + } + returnString.WriteString(fmt.Sprintf("and %q", applyRequirements[len(applyRequirements)-1])) + return returnString.String() +} diff --git a/server/events/yaml/raw/project_test.go b/server/events/yaml/raw/project_test.go index 30b16b39b..677cc625d 100644 --- a/server/events/yaml/raw/project_test.go +++ b/server/events/yaml/raw/project_test.go @@ -101,7 +101,7 @@ func TestProject_Validate(t *testing.T) { Dir: String("."), ApplyRequirements: []string{"unsupported"}, }, - expErr: "apply_requirements: \"unsupported\" is not a valid apply_requirement, only \"approved\", \"mergeable\", \"undiverged\" and \"unlocked\" are supported.", + expErr: "apply_requirements: \"unsupported\" is not a valid apply_requirement, only \"approved\", \"mergeable\", \"undiverged\", and \"unlocked\" are supported.", }, { description: "apply reqs with approved requirement", From 043e84ea731099eb19f441c9fa46f53f39aceac2 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 4 Aug 2021 10:14:43 -0700 Subject: [PATCH 07/13] Fix failing tests --- server/events/models/models.go | 2 +- server/events/yaml/raw/project.go | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/server/events/models/models.go b/server/events/models/models.go index 1724248b9..e897130dd 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -423,7 +423,7 @@ func (p ProjectCommandContext) ProjectCloneDir() string { // SetScope sets the scope of the stats object field. Note: we deliberately set this on the value // instead of a pointer since we want scopes to mirror our function stack func (p ProjectCommandContext) SetScope(scope string) { - p.Scope = p.Scope.Scope(scope) //nolint + p.Scope = p.Scope.Scope(scope) } // GetShowResultFileName returns the filename (not the path) to store the tf show result diff --git a/server/events/yaml/raw/project.go b/server/events/yaml/raw/project.go index d4b129cf4..4c668c925 100644 --- a/server/events/yaml/raw/project.go +++ b/server/events/yaml/raw/project.go @@ -13,10 +13,19 @@ import ( ) const ( - DefaultWorkspace = "default" + DefaultWorkspace = "default" + ApprovedApplyRequirement = "approved" + MergeableApplyRequirement = "mergeable" + UnDivergedApplyRequirement = "undiverged" + UnlockedApplyRequirement = "unlocked" ) -var applyRequirements = [...]string{"approved", "mergeable", "undiverged", "unlocked"} +var applyRequirements = [...]string{ + ApprovedApplyRequirement, + MergeableApplyRequirement, + UnDivergedApplyRequirement, + UnlockedApplyRequirement, +} type Project struct { Name *string `yaml:"name,omitempty"` From 5caa1a8916489f6e13c68c5355217ed89ad508f4 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 4 Aug 2021 10:23:07 -0700 Subject: [PATCH 08/13] Renaming symbols --- server/events/yaml/valid/global_cfg.go | 12 ++++++------ server/server.go | 2 +- server/user_config.go | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/server/events/yaml/valid/global_cfg.go b/server/events/yaml/valid/global_cfg.go index 5b3046d89..6b887680a 100644 --- a/server/events/yaml/valid/global_cfg.go +++ b/server/events/yaml/valid/global_cfg.go @@ -12,7 +12,7 @@ import ( const MergeableApplyReq = "mergeable" const ApprovedApplyReq = "approved" const UnDivergedApplyReq = "undiverged" -const UnlockedApplyReq = "unlocked" +const SQUnlockedApplyReq = "unlocked" const PoliciesPassedApplyReq = "policies_passed" const ApplyRequirementsKey = "apply_requirements" const PreWorkflowHooksKey = "pre_workflow_hooks" @@ -110,13 +110,13 @@ var DefaultPlanStage = Stage{ } // Deprecated: use NewGlobalCfgFromArgs -func NewGlobalCfgWithHooks(allowRepoCfg bool, mergeableReq bool, approvedReq bool, unDivergedReq bool, unLockedReq bool, preWorkflowHooks []*PreWorkflowHook) GlobalCfg { +func NewGlobalCfgWithHooks(allowRepoCfg bool, mergeableReq bool, approvedReq bool, unDivergedReq bool, sqUnLockedReq bool, preWorkflowHooks []*PreWorkflowHook) GlobalCfg { return NewGlobalCfgFromArgs(GlobalCfgArgs{ AllowRepoCfg: allowRepoCfg, MergeableReq: mergeableReq, ApprovedReq: approvedReq, UnDivergedReq: unDivergedReq, - UnLockedReq: unLockedReq, + SQUnLockedReq: sqUnLockedReq, PreWorkflowHooks: preWorkflowHooks, }) } @@ -141,7 +141,7 @@ type GlobalCfgArgs struct { MergeableReq bool ApprovedReq bool UnDivergedReq bool - UnLockedReq bool + SQUnLockedReq bool PolicyCheckEnabled bool PreWorkflowHooks []*PreWorkflowHook } @@ -167,8 +167,8 @@ func NewGlobalCfgFromArgs(args GlobalCfgArgs) GlobalCfg { if args.UnDivergedReq { applyReqs = append(applyReqs, UnDivergedApplyReq) } - if args.UnLockedReq { - applyReqs = append(applyReqs, UnlockedApplyReq) + if args.SQUnLockedReq { + applyReqs = append(applyReqs, SQUnlockedApplyReq) } if args.PolicyCheckEnabled { applyReqs = append(applyReqs, PoliciesPassedApplyReq) diff --git a/server/server.go b/server/server.go index 5dac1ff89..dc87552cf 100644 --- a/server/server.go +++ b/server/server.go @@ -370,7 +370,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { MergeableReq: userConfig.RequireMergeable, ApprovedReq: userConfig.RequireApproval, UnDivergedReq: userConfig.RequireUnDiverged, - UnLockedReq: userConfig.RequireUnlocked, + SQUnLockedReq: userConfig.RequireSQUnlocked, PolicyCheckEnabled: userConfig.EnablePolicyChecksFlag, }) if userConfig.RepoConfig != "" { diff --git a/server/user_config.go b/server/user_config.go index feb245c5f..ddbfaa07c 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -66,9 +66,9 @@ type UserConfig struct { // RequireUnDiverged is whether to require pull requests to rebase default branch before // allowing terraform apply's to run. RequireUnDiverged bool `mapstructure:"require-undiverged"` - // RequireUnlocked is whether to require pull requests to be unlocked before running + // RequireSQUnlocked is whether to require pull requests to be unlocked before running // terraform apply. - RequireUnlocked bool `mapstructure:"require-unlocked"` + RequireSQUnlocked bool `mapstructure:"require-unlocked"` SilenceForkPRErrors bool `mapstructure:"silence-fork-pr-errors"` // SilenceVCSStatusNoPlans is whether autoplan should set commit status if no plans // are found. From 823bfe432c3ba2ea954c840f3836248a84c80032 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 4 Aug 2021 13:32:16 -0700 Subject: [PATCH 09/13] Changed applyRequirements from slice to map --- server/events/yaml/raw/project.go | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/server/events/yaml/raw/project.go b/server/events/yaml/raw/project.go index 4c668c925..f2386a930 100644 --- a/server/events/yaml/raw/project.go +++ b/server/events/yaml/raw/project.go @@ -20,11 +20,11 @@ const ( UnlockedApplyRequirement = "unlocked" ) -var applyRequirements = [...]string{ - ApprovedApplyRequirement, - MergeableApplyRequirement, - UnDivergedApplyRequirement, - UnlockedApplyRequirement, +var applyRequirements = map[string]bool{ + ApprovedApplyRequirement: true, + MergeableApplyRequirement: true, + UnDivergedApplyRequirement: true, + UnlockedApplyRequirement: true, } type Project struct { @@ -115,14 +115,7 @@ func validProjectName(name string) bool { func validApplyReq(value interface{}) error { reqs := value.([]string) for _, req := range reqs { - isValid := false - for _, applyRequirement := range applyRequirements { - if req == applyRequirement { - isValid = true - break - } - } - if !isValid { + if _, ok := applyRequirements[req]; !ok { return fmt.Errorf("%q is not a valid apply_requirement, only %s are supported", req, buildValidApplyRequirementsString()) } } @@ -131,9 +124,14 @@ func validApplyReq(value interface{}) error { func buildValidApplyRequirementsString() string { var returnString strings.Builder - for _, applyReq := range applyRequirements[:len(applyRequirements)-1] { + counter := 0 + for applyReq := range applyRequirements { + if counter == len(applyRequirements)-1 { + returnString.WriteString(fmt.Sprintf("and %q", applyReq)) + break + } returnString.WriteString(fmt.Sprintf("%q, ", applyReq)) + counter += 1 } - returnString.WriteString(fmt.Sprintf("and %q", applyRequirements[len(applyRequirements)-1])) return returnString.String() } From 4de376367d54926c3a2fdbbd89041a458cca3ae0 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 4 Aug 2021 15:31:15 -0700 Subject: [PATCH 10/13] Sorting apply requirements for consistent error log messages. --- server/events/yaml/raw/project.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/server/events/yaml/raw/project.go b/server/events/yaml/raw/project.go index f2386a930..80adb11f7 100644 --- a/server/events/yaml/raw/project.go +++ b/server/events/yaml/raw/project.go @@ -4,6 +4,7 @@ import ( "fmt" "net/url" "path/filepath" + "sort" "strings" validation "github.com/go-ozzo/ozzo-validation" @@ -123,9 +124,16 @@ func validApplyReq(value interface{}) error { } func buildValidApplyRequirementsString() string { + // Sort keys. + applyRequirementsList := make([]string, 0, len(applyRequirements)) + for applyReq := range applyRequirements { + applyRequirementsList = append(applyRequirementsList, applyReq) + } + sort.Strings(applyRequirementsList) + var returnString strings.Builder counter := 0 - for applyReq := range applyRequirements { + for _, applyReq := range applyRequirementsList { if counter == len(applyRequirements)-1 { returnString.WriteString(fmt.Sprintf("and %q", applyReq)) break From 3539248d334a61708f6117e0fffafdad209909c9 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 4 Aug 2021 15:39:24 -0700 Subject: [PATCH 11/13] Fixing linter error --- server/events/yaml/raw/project.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/events/yaml/raw/project.go b/server/events/yaml/raw/project.go index 80adb11f7..f75010af8 100644 --- a/server/events/yaml/raw/project.go +++ b/server/events/yaml/raw/project.go @@ -139,7 +139,7 @@ func buildValidApplyRequirementsString() string { break } returnString.WriteString(fmt.Sprintf("%q, ", applyReq)) - counter += 1 + counter++ } return returnString.String() } From 7b2601442f1e750b0173ad011a56a388a30f836e Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 4 Aug 2021 16:26:08 -0700 Subject: [PATCH 12/13] Updated invalid apply requirement message --- server/events/yaml/parser_validator_test.go | 2 +- server/events/yaml/raw/project.go | 25 +++++---------------- server/events/yaml/raw/project_test.go | 2 +- 3 files changed, 7 insertions(+), 22 deletions(-) diff --git a/server/events/yaml/parser_validator_test.go b/server/events/yaml/parser_validator_test.go index 00b8d23e2..ba37ed6dc 100644 --- a/server/events/yaml/parser_validator_test.go +++ b/server/events/yaml/parser_validator_test.go @@ -1230,7 +1230,7 @@ func TestParseGlobalCfg(t *testing.T) { input: `repos: - id: /.*/ apply_requirements: [invalid]`, - expErr: "repos: (0: (apply_requirements: \"invalid\" is not a valid apply_requirement, only \"approved\", \"mergeable\", \"undiverged\", and \"unlocked\" are supported.).).", + expErr: "repos: (0: (apply_requirements: \"invalid\" is not a valid apply_requirement, supported apply requirements are: \"approved\", \"mergeable\", \"undiverged\", \"unlocked\".).).", }, "no workflows key": { input: `repos: []`, diff --git a/server/events/yaml/raw/project.go b/server/events/yaml/raw/project.go index f75010af8..6d086b6b4 100644 --- a/server/events/yaml/raw/project.go +++ b/server/events/yaml/raw/project.go @@ -114,32 +114,17 @@ func validProjectName(name string) bool { } func validApplyReq(value interface{}) error { - reqs := value.([]string) - for _, req := range reqs { - if _, ok := applyRequirements[req]; !ok { - return fmt.Errorf("%q is not a valid apply_requirement, only %s are supported", req, buildValidApplyRequirementsString()) - } - } - return nil -} - -func buildValidApplyRequirementsString() string { - // Sort keys. applyRequirementsList := make([]string, 0, len(applyRequirements)) for applyReq := range applyRequirements { applyRequirementsList = append(applyRequirementsList, applyReq) } sort.Strings(applyRequirementsList) - var returnString strings.Builder - counter := 0 - for _, applyReq := range applyRequirementsList { - if counter == len(applyRequirements)-1 { - returnString.WriteString(fmt.Sprintf("and %q", applyReq)) - break + reqs := value.([]string) + for _, req := range reqs { + if _, ok := applyRequirements[req]; !ok { + return fmt.Errorf("%q is not a valid apply_requirement, supported apply requirements are: %s", req, "\""+strings.Join(applyRequirementsList, "\", \"")+"\"") } - returnString.WriteString(fmt.Sprintf("%q, ", applyReq)) - counter++ } - return returnString.String() + return nil } diff --git a/server/events/yaml/raw/project_test.go b/server/events/yaml/raw/project_test.go index 677cc625d..962d8ee6f 100644 --- a/server/events/yaml/raw/project_test.go +++ b/server/events/yaml/raw/project_test.go @@ -101,7 +101,7 @@ func TestProject_Validate(t *testing.T) { Dir: String("."), ApplyRequirements: []string{"unsupported"}, }, - expErr: "apply_requirements: \"unsupported\" is not a valid apply_requirement, only \"approved\", \"mergeable\", \"undiverged\", and \"unlocked\" are supported.", + expErr: "apply_requirements: \"unsupported\" is not a valid apply_requirement, supported apply requirements are: \"approved\", \"mergeable\", \"undiverged\", \"unlocked\".", }, { description: "apply reqs with approved requirement", From 1c52a2b688b9252c4c77aa038d34637a975b7137 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 4 Aug 2021 16:35:20 -0700 Subject: [PATCH 13/13] Added supportedApplyReqs as a global var --- server/events/yaml/raw/project.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/server/events/yaml/raw/project.go b/server/events/yaml/raw/project.go index 6d086b6b4..fe0aca246 100644 --- a/server/events/yaml/raw/project.go +++ b/server/events/yaml/raw/project.go @@ -28,6 +28,8 @@ var applyRequirements = map[string]bool{ UnlockedApplyRequirement: true, } +var supportedApplyReqs = buildSupportedApplyReqs() + type Project struct { Name *string `yaml:"name,omitempty"` Dir *string `yaml:"dir,omitempty"` @@ -123,8 +125,19 @@ func validApplyReq(value interface{}) error { reqs := value.([]string) for _, req := range reqs { if _, ok := applyRequirements[req]; !ok { - return fmt.Errorf("%q is not a valid apply_requirement, supported apply requirements are: %s", req, "\""+strings.Join(applyRequirementsList, "\", \"")+"\"") + return fmt.Errorf("%q is not a valid apply_requirement, supported apply requirements are: %s", req, supportedApplyReqs) } } return nil } + +func buildSupportedApplyReqs() string { + // Sort keys. + applyRequirementsList := make([]string, 0, len(applyRequirements)) + for applyReq := range applyRequirements { + applyRequirementsList = append(applyRequirementsList, applyReq) + } + sort.Strings(applyRequirementsList) + + return "\"" + strings.Join(applyRequirementsList, "\", \"") + "\"" +}