Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added UnlockedApplyRequirement to support 🔒 and 🔓 emoji for atlantis apply #90

Merged
merged 13 commits into from
Aug 5, 2021
5 changes: 5 additions & 0 deletions cmd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ const (
RepoWhitelistFlag = "repo-whitelist"
RepoAllowlistFlag = "repo-allowlist"
RequireApprovalFlag = "require-approval"
RequireSQUnlockedFlag = "require-unlocked"
RequireMergeableFlag = "require-mergeable"
SilenceNoProjectsFlag = "silence-no-projects"
SilenceForkPRErrorsFlag = "silence-fork-pr-errors"
Expand Down Expand Up @@ -343,6 +344,10 @@ var boolFlags = map[string]boolFlag{
defaultValue: false,
hidden: true,
},
RequireSQUnlockedFlag: {
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,
Expand Down
2 changes: 1 addition & 1 deletion server/events/yaml/parser_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: []`,
Expand Down
30 changes: 27 additions & 3 deletions server/events/yaml/raw/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,16 @@ const (
ApprovedApplyRequirement = "approved"
MergeableApplyRequirement = "mergeable"
UnDivergedApplyRequirement = "undiverged"
UnlockedApplyRequirement = "unlocked"
)

var applyRequirements = [...]string{
ApprovedApplyRequirement,
MergeableApplyRequirement,
UnDivergedApplyRequirement,
UnlockedApplyRequirement,
}

type Project struct {
Name *string `yaml:"name,omitempty"`
Dir *string `yaml:"dir,omitempty"`
Expand Down Expand Up @@ -106,10 +114,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 {
return fmt.Errorf("%q is not a valid apply_requirement, only %q, %q and %q are supported", r, ApprovedApplyRequirement, MergeableApplyRequirement, UnDivergedApplyRequirement)
for _, req := range reqs {
isValid := false
for _, applyRequirement := range applyRequirements {
if req == applyRequirement {
isValid = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just use a map instead of a list, this would become one line of code then?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm that's true! I'll change it to a map.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like using a map makes the buildValidApplyRequirementsString() unpredictable causing the tests to fail occasionally. Should I just revert back to using a slice or should I change how we're generating the test statement?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably just account for that failure by sorting the result.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

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()
}
2 changes: 1 addition & 1 deletion server/events/yaml/raw/project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 7 additions & 2 deletions server/events/yaml/valid/global_cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
const MergeableApplyReq = "mergeable"
const ApprovedApplyReq = "approved"
const UnDivergedApplyReq = "undiverged"
const SQUnlockedApplyReq = "unlocked"
const PoliciesPassedApplyReq = "policies_passed"
const ApplyRequirementsKey = "apply_requirements"
const PreWorkflowHooksKey = "pre_workflow_hooks"
Expand Down Expand Up @@ -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, sqUnLockedReq bool, preWorkflowHooks []*PreWorkflowHook) GlobalCfg {
return NewGlobalCfgFromArgs(GlobalCfgArgs{
AllowRepoCfg: allowRepoCfg,
MergeableReq: mergeableReq,
ApprovedReq: approvedReq,
UnDivergedReq: unDivergedReq,
SQUnLockedReq: sqUnLockedReq,
PreWorkflowHooks: preWorkflowHooks,
})
}
Expand All @@ -139,6 +141,7 @@ type GlobalCfgArgs struct {
MergeableReq bool
ApprovedReq bool
UnDivergedReq bool
SQUnLockedReq bool
PolicyCheckEnabled bool
PreWorkflowHooks []*PreWorkflowHook
}
Expand All @@ -164,7 +167,9 @@ func NewGlobalCfgFromArgs(args GlobalCfgArgs) GlobalCfg {
if args.UnDivergedReq {
applyReqs = append(applyReqs, UnDivergedApplyReq)
}

if args.SQUnLockedReq {
applyReqs = append(applyReqs, SQUnlockedApplyReq)
}
if args.PolicyCheckEnabled {
applyReqs = append(applyReqs, PoliciesPassedApplyReq)
}
Expand Down
1 change: 1 addition & 0 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
MergeableReq: userConfig.RequireMergeable,
ApprovedReq: userConfig.RequireApproval,
UnDivergedReq: userConfig.RequireUnDiverged,
SQUnLockedReq: userConfig.RequireSQUnlocked,
PolicyCheckEnabled: userConfig.EnablePolicyChecksFlag,
})
if userConfig.RepoConfig != "" {
Expand Down
5 changes: 4 additions & 1 deletion server/user_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ 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"`
RequireUnDiverged bool `mapstructure:"require-undiverged"`
// RequireSQUnlocked is whether to require pull requests to be unlocked before running
// terraform apply.
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.
Expand Down