From 8ae774ba2eafa80e6b25b714cfec247b5aab3a9e Mon Sep 17 00:00:00 2001 From: Ross Strickland Date: Tue, 31 Jan 2023 10:06:49 -0600 Subject: [PATCH] Initial work. --- server/core/config/raw/policies.go | 30 +++++-- server/core/config/valid/policies.go | 18 +++-- server/core/db/boltdb.go | 10 ++- server/core/runtime/policy/conftest_client.go | 80 +++++++++++++------ server/events/command/project_result.go | 27 ++++--- server/events/markdown_renderer.go | 27 +++++-- server/events/models/models.go | 26 ++++-- server/events/project_command_runner.go | 50 ++++++++++-- 8 files changed, 191 insertions(+), 77 deletions(-) diff --git a/server/core/config/raw/policies.go b/server/core/config/raw/policies.go index 270e314a41..a951cbdb16 100644 --- a/server/core/config/raw/policies.go +++ b/server/core/config/raw/policies.go @@ -2,15 +2,16 @@ package raw import ( validation "github.com/go-ozzo/ozzo-validation" - "github.com/hashicorp/go-version" + version "github.com/hashicorp/go-version" "github.com/runatlantis/atlantis/server/core/config/valid" ) // PolicySets is the raw schema for repo-level atlantis.yaml config. type PolicySets struct { - Version *string `yaml:"conftest_version,omitempty" json:"conftest_version,omitempty"` - Owners PolicyOwners `yaml:"owners,omitempty" json:"owners,omitempty"` - PolicySets []PolicySet `yaml:"policy_sets" json:"policy_sets"` + Version *string `yaml:"conftest_version,omitempty" json:"conftest_version,omitempty"` + Owners PolicyOwners `yaml:"owners,omitempty" json:"owners,omitempty"` + PolicySets []PolicySet `yaml:"policy_sets" json:"policy_sets"` + ReviewCount int `yaml:"review_count,omitempty" json:"review_count,omitempty"` } func (p PolicySets) Validate() error { @@ -27,10 +28,20 @@ func (p PolicySets) ToValid() valid.PolicySets { policySets.Version, _ = version.NewVersion(*p.Version) } + // Default number of required reviews for all policy sets should be 1. + policySets.ReviewCount = p.ReviewCount + if policySets.ReviewCount == 0 { + policySets.ReviewCount = 1 + } + policySets.Owners = p.Owners.ToValid() validPolicySets := make([]valid.PolicySet, 0) for _, rawPolicySet := range p.PolicySets { + // Default to top-level review count if not specified. + if rawPolicySet.ReviewCount == 0 { + rawPolicySet.ReviewCount = policySets.ReviewCount + } validPolicySets = append(validPolicySets, rawPolicySet.ToValid()) } policySets.PolicySets = validPolicySets @@ -57,16 +68,18 @@ func (o PolicyOwners) ToValid() valid.PolicyOwners { } type PolicySet struct { - Path string `yaml:"path" json:"path"` - Source string `yaml:"source" json:"source"` - Name string `yaml:"name" json:"name"` - Owners PolicyOwners `yaml:"owners,omitempty" json:"owners,omitempty"` + Path string `yaml:"path" json:"path"` + Source string `yaml:"source" json:"source"` + Name string `yaml:"name" json:"name"` + Owners PolicyOwners `yaml:"owners,omitempty" json:"owners,omitempty"` + ReviewCount int `yaml:"review_count,omitempty" json:"review_count,omitempty"` } func (p PolicySet) Validate() error { return validation.ValidateStruct(&p, validation.Field(&p.Name, validation.Required.Error("is required")), validation.Field(&p.Owners), + validation.Field(&p.ReviewCount), validation.Field(&p.Path, validation.Required.Error("is required")), validation.Field(&p.Source, validation.In(valid.LocalPolicySet, valid.GithubPolicySet).Error("only 'local' and 'github' source types are supported")), ) @@ -78,6 +91,7 @@ func (p PolicySet) ToValid() valid.PolicySet { policySet.Name = p.Name policySet.Path = p.Path policySet.Source = p.Source + policySet.ReviewCount = p.ReviewCount policySet.Owners = p.Owners.ToValid() return policySet diff --git a/server/core/config/valid/policies.go b/server/core/config/valid/policies.go index 48d166cea2..307f3ba387 100644 --- a/server/core/config/valid/policies.go +++ b/server/core/config/valid/policies.go @@ -3,7 +3,7 @@ package valid import ( "strings" - "github.com/hashicorp/go-version" + version "github.com/hashicorp/go-version" ) const ( @@ -15,9 +15,10 @@ const ( // PolicySet objects. PolicySets struct is used by PolicyCheck workflow to build // context to enforce policies. type PolicySets struct { - Version *version.Version - Owners PolicyOwners - PolicySets []PolicySet + Version *version.Version + Owners PolicyOwners + ReviewCount int + PolicySets []PolicySet } type PolicyOwners struct { @@ -26,10 +27,11 @@ type PolicyOwners struct { } type PolicySet struct { - Source string - Path string - Name string - Owners PolicyOwners + Source string + Path string + Name string + ReviewCount int + Owners PolicyOwners } func (p *PolicySets) HasPolicies() bool { diff --git a/server/core/db/boltdb.go b/server/core/db/boltdb.go index 14641bf0af..658e8d5244 100644 --- a/server/core/db/boltdb.go +++ b/server/core/db/boltdb.go @@ -357,6 +357,7 @@ func (b *BoltDB) UpdatePullWithResults(pull models.PullRequest, newResults []com res.ProjectName == proj.ProjectName { proj.Status = res.PlanStatus() + proj.PolicyStatus = res.PolicyCheckApprovals updatedExisting = true break } @@ -483,9 +484,10 @@ func (b *BoltDB) writePullToBucket(bucket *bolt.Bucket, key []byte, pull models. func (b *BoltDB) projectResultToProject(p command.ProjectResult) models.ProjectStatus { return models.ProjectStatus{ - Workspace: p.Workspace, - RepoRelDir: p.RepoRelDir, - ProjectName: p.ProjectName, - Status: p.PlanStatus(), + Workspace: p.Workspace, + RepoRelDir: p.RepoRelDir, + ProjectName: p.ProjectName, + PolicyStatus: p.PolicyCheckApprovals, + Status: p.PlanStatus(), } } diff --git a/server/core/runtime/policy/conftest_client.go b/server/core/runtime/policy/conftest_client.go index 2c21226c2c..654bff6008 100644 --- a/server/core/runtime/policy/conftest_client.go +++ b/server/core/runtime/policy/conftest_client.go @@ -17,6 +17,10 @@ import ( "github.com/runatlantis/atlantis/server/logging" "golang.org/x/text/cases" "golang.org/x/text/language" + "github.com/hashicorp/go-multierror" + "github.com/runatlantis/atlantis/server/events/models" + "encoding/json" + "regexp" ) const ( @@ -163,46 +167,64 @@ func NewConfTestExecutorWorkflow(log logging.SimpleLogging, versionRootDir strin } func (c *ConfTestExecutorWorkflow) Run(ctx command.ProjectContext, executablePath string, envs map[string]string, workdir string, extraArgs []string) (string, error) { - policyArgs := []Arg{} - policySetNames := []string{} ctx.Log.Debug("policy sets, %s ", ctx.PolicySets) + + inputFile := filepath.Join(workdir, ctx.GetShowResultFileName()) + var policySetResults []models.PolicySetResult + var combinedErr error + + for _, policySet := range ctx.PolicySets.PolicySets { - path, err := c.SourceResolver.Resolve(policySet) + path, resolveErr := c.SourceResolver.Resolve(policySet) // Let's not fail the whole step because of a single failure. Log and fail silently - if err != nil { - ctx.Log.Err("Error resolving policyset %s. err: %s", policySet.Name, err.Error()) + if resolveErr != nil { + ctx.Log.Err("Error resolving policyset %s. err: %s", policySet.Name, resolveErr.Error()) continue } - policyArg := NewPolicyArg(path) - policyArgs = append(policyArgs, policyArg) - - policySetNames = append(policySetNames, policySet.Name) - } - - inputFile := filepath.Join(workdir, ctx.GetShowResultFileName()) + args := ConftestTestCommandArgs{ + PolicyArgs: []Arg{NewPolicyArg(path)}, + ExtraArgs: extraArgs, + InputFile: inputFile, + Command: executablePath, + } - args := ConftestTestCommandArgs{ - PolicyArgs: policyArgs, - ExtraArgs: extraArgs, - InputFile: inputFile, - Command: executablePath, + serializedArgs, _ := args.build() + cmdOutput, cmdErr := c.Exec.CombinedOutput(serializedArgs, envs, workdir) + + passed := true + if cmdErr != nil { + // Since we're running conftest for each policyset, individual command errors should be concatenated. + if isValidConftestOutput(cmdOutput) { + combinedErr = multierror.Append(combinedErr, errors.New(fmt.Sprintf("policy_set: %s: conftest: %s", policySet.Name, "Some policies failed."))) + } else { + combinedErr = multierror.Append(combinedErr, errors.New(fmt.Sprintf("policy_set: %s: conftest: %s", policySet.Name, cmdOutput))) + } + passed = false + } + + policySetResults = append(policySetResults, models.PolicySetResult{ + PolicySetName: policySet.Name, + PolicySetOutput: cmdOutput, + Passed: passed, + }) } - serializedArgs, err := args.build() - - if err != nil { - ctx.Log.Warn("No policies have been configured") + if policySetResults == nil { + ctx.Log.Warn("No policies have been configured.") return "", nil // TODO: enable when we can pass policies in otherwise e2e tests with policy checks fail // return "", errors.Wrap(err, "building args") } - initialOutput := fmt.Sprintf("Checking plan against the following policies: \n %s\n", strings.Join(policySetNames, "\n ")) - cmdOutput, err := c.Exec.CombinedOutput(serializedArgs, envs, workdir) + marshaledStatus, err := json.Marshal(policySetResults) + if err != nil { + return "", errors.New(fmt.Sprintf("Cannot marshal data into []PolicySetResult. Error: %w Data: %w", err, policySetResults)) + } + output := string(marshaledStatus) - return c.sanitizeOutput(inputFile, initialOutput+cmdOutput), err + return c.sanitizeOutput(inputFile, output), combinedErr } @@ -255,3 +277,13 @@ func getDefaultVersion() (*version.Version, error) { } return wrappedVersion, nil } + +// Checks if output from conftest is a valid output. +func isValidConftestOutput(output string) bool { + + r := regexp.MustCompile(`^(WARN|FAIL|\[)`) + if match := r.FindString(output); match != "" { + return true + } + return false +} \ No newline at end of file diff --git a/server/events/command/project_result.go b/server/events/command/project_result.go index 9191d86f8a..08a13f00fc 100644 --- a/server/events/command/project_result.go +++ b/server/events/command/project_result.go @@ -6,19 +6,20 @@ import ( // ProjectResult is the result of executing a plan/policy_check/apply for a specific project. type ProjectResult struct { - Command Name - SubCommand string - RepoRelDir string - Workspace string - Error error - Failure string - PlanSuccess *models.PlanSuccess - PolicyCheckSuccess *models.PolicyCheckSuccess - ApplySuccess string - VersionSuccess string - ImportSuccess *models.ImportSuccess - StateRmSuccess *models.StateRmSuccess - ProjectName string + Command Name + SubCommand string + RepoRelDir string + Workspace string + Error error + Failure string + PlanSuccess *models.PlanSuccess + PolicyCheckSuccess *models.PolicyCheckSuccess + PolicyCheckApprovals []models.PolicySetApproval + ApplySuccess string + VersionSuccess string + ImportSuccess *models.ImportSuccess + StateRmSuccess *models.StateRmSuccess + ProjectName string } // CommitStatus returns the vcs commit status of this project result. diff --git a/server/events/markdown_renderer.go b/server/events/markdown_renderer.go index 5d8b6441dc..680e5b26ae 100644 --- a/server/events/markdown_renderer.go +++ b/server/events/markdown_renderer.go @@ -78,7 +78,14 @@ type errData struct { commonData } -// failureData is data about a failure response. +// policyFailureData is data about a failing +type policyCheckFailureData struct { + Failure string + commonData + models.PolicyCheckSuccess +} + +// failureData is data about a generic failure response. type failureData struct { Failure string commonData @@ -189,7 +196,11 @@ func (m *MarkdownRenderer) renderProjectResults(results []command.ProjectResult, } resultData.Rendered = m.renderTemplateTrimSpace(tmpl, errData{result.Error.Error(), common}) } else if result.Failure != "" { - resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("failure"), failureData{result.Failure, common}) + if common.Command == "PolicyCheck" { + resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("failure"), policyCheckFailureData{result.Failure, common, *result.PolicyCheckSuccess}) + } else { + resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("failure"), failureData{result.Failure, common}) + } } else if result.PlanSuccess != nil { result.PlanSuccess.TerraformOutput = strings.TrimSpace(result.PlanSuccess.TerraformOutput) if m.shouldUseWrappedTmpl(vcsHost, result.PlanSuccess.TerraformOutput) { @@ -199,12 +210,12 @@ func (m *MarkdownRenderer) renderProjectResults(results []command.ProjectResult, } numPlanSuccesses++ } else if result.PolicyCheckSuccess != nil { - result.PolicyCheckSuccess.PolicyCheckOutput = strings.TrimSpace(result.PolicyCheckSuccess.PolicyCheckOutput) - if m.shouldUseWrappedTmpl(vcsHost, result.PolicyCheckSuccess.PolicyCheckOutput) { - resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("policyCheckSuccessWrapped"), policyCheckSuccessData{PolicyCheckSuccess: *result.PolicyCheckSuccess, PolicyCheckSummary: result.PolicyCheckSuccess.Summary()}) - } else { - resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("policyCheckSuccessUnwrapped"), policyCheckSuccessData{PolicyCheckSuccess: *result.PolicyCheckSuccess}) - } +// result.PolicyCheckSuccess.PolicySetResults = strings.TrimSpace(string(result.PolicyCheckSuccess.PolicySetResults)) +// if m.shouldUseWrappedTmpl(vcsHost, result.PolicyCheckSuccess.PolicySetResults) { +// resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("policyCheckSuccessWrapped"), policyCheckSuccessData{PolicyCheckSuccess: *result.PolicyCheckSuccess, PolicyCheckSummary: result.PolicyCheckSuccess.Summary()}) +// } else { + resultData.Rendered = m.renderTemplateTrimSpace(templates.Lookup("policyCheckSuccessUnwrapped"), policyCheckSuccessData{PolicyCheckSuccess: *result.PolicyCheckSuccess}) +// } numPolicyCheckSuccesses++ } else if result.ApplySuccess != "" { output := strings.TrimSpace(result.ApplySuccess) diff --git a/server/events/models/models.go b/server/events/models/models.go index 6c45795be8..1e44724ef9 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -366,6 +366,12 @@ type PlanSuccess struct { HasDiverged bool } +type PolicySetResult struct { + PolicySetName string + PolicySetOutput string + Passed bool +} + // Summary regexes var ( reChangesOutside = regexp.MustCompile(`Note: Objects have changed outside of Terraform`) @@ -409,7 +415,7 @@ func (p PlanSuccess) DiffMarkdownFormattedTerraformOutput() string { // PolicyCheckSuccess is the result of a successful policy check run. type PolicyCheckSuccess struct { // PolicyCheckOutput is the output from policy check binary(conftest|opa) - PolicyCheckOutput string + PolicySetResults []PolicySetResult // LockURL is the full URL to the lock held by this policy check. LockURL string // RePlanCmd is the command that users should run to re-plan this project. @@ -441,11 +447,11 @@ type StateRmSuccess struct { // Summary extracts one line summary of policy check. func (p *PolicyCheckSuccess) Summary() string { note := "" - - r := regexp.MustCompile(`\d+ tests?, \d+ passed, \d+ warnings?, \d+ failures?, \d+ exceptions?(, \d skipped)?`) - if match := r.FindString(p.PolicyCheckOutput); match != "" { - return note + match - } + // + //r := regexp.MustCompile(`\d+ tests?, \d+ passed, \d+ warnings?, \d+ failures?, \d+ exceptions?(, \d skipped)?`) + //if match := r.FindString(p.PolicyCheckOutput); match != "" { + // return note + match + //} return note } @@ -472,11 +478,19 @@ func (p PullStatus) StatusCount(status ProjectPlanStatus) int { return c } +// PolicySetApprovalStatus tracks the number of approvals a given PolicySet has received. +type PolicySetApproval struct { + PolicySetName string + Approvals int +} + // ProjectStatus is the status of a specific project. type ProjectStatus struct { Workspace string RepoRelDir string ProjectName string + // PolicySetApprovals tracks the approval status of every PolicySet for a Project. + PolicyStatus []PolicySetApproval // Status is the status of where this project is at in the planning cycle. Status ProjectPlanStatus } diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go index ef1bd55f6d..c70afedc9c 100644 --- a/server/events/project_command_runner.go +++ b/server/events/project_command_runner.go @@ -26,6 +26,8 @@ import ( "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/webhooks" "github.com/runatlantis/atlantis/server/logging" + "github.com/hashicorp/go-multierror" + "encoding/json" ) const OperationComplete = true @@ -234,9 +236,22 @@ func (p *DefaultProjectCommandRunner) Plan(ctx command.ProjectContext) command.P // PolicyCheck evaluates policies defined with Rego for the project described by ctx. func (p *DefaultProjectCommandRunner) PolicyCheck(ctx command.ProjectContext) command.ProjectResult { policySuccess, failure, err := p.doPolicyCheck(ctx) + var policyCheckApprovals []models.PolicySetApproval + for _, ps := range policySuccess.PolicySetResults { + approvals := 0 + if !ps.Passed { + for _, cps := range ctx.PolicySets.PolicySets { + if ps.PolicySetName == cps.Name { + approvals = 0 - cps.ReviewCount + } + } + } + policyCheckApprovals = append(policyCheckApprovals, models.PolicySetApproval{PolicySetName: ps.PolicySetName, Approvals: approvals}) + } return command.ProjectResult{ Command: command.PolicyCheck, PolicyCheckSuccess: policySuccess, + PolicyCheckApprovals: policyCheckApprovals, Error: err, Failure: failure, RepoRelDir: ctx.RepoRelDir, @@ -320,7 +335,7 @@ func (p *DefaultProjectCommandRunner) doApprovePolicies(ctx command.ProjectConte // without checking some sort of state that the policy check has indeed passed this is likely to cause issues return &models.PolicyCheckSuccess{ - PolicyCheckOutput: "Policies approved", +// PolicyCheckOutput: "Policies approved", }, "", nil } @@ -376,23 +391,46 @@ func (p *DefaultProjectCommandRunner) doPolicyCheck(ctx command.ProjectContext) return nil, "", DirNotExistErr{RepoRelDir: ctx.RepoRelDir} } + var failures []string outputs, err := p.runSteps(ctx.Steps, ctx, absPath) if err != nil { - // Note: we are explicitly not unlocking the pr here since a failing policy check will require - // approval - return nil, "", fmt.Errorf("%s\n%s", err, strings.Join(outputs, "\n")) + stepErr := err + err = nil + for { + stepErr = errors.Unwrap(stepErr) + if stepErr == nil { + break + } + if strings.Contains(stepErr.Error(), "Some policies failed.") { + failures = append(failures, stepErr.Error()) + } else { + err = multierror.Append(err, stepErr) + } + } + + if err != nil { + // Note: we are explicitly not unlocking the pr here since a failing policy check will require + // approval + return nil, "", err + } + } + + var policySetResults []models.PolicySetResult + err = json.Unmarshal([]byte(strings.Join(outputs, "\n")), &policySetResults) + if err != nil { + return nil, "", err } return &models.PolicyCheckSuccess{ LockURL: p.LockURLGenerator.GenerateLockURL(lockAttempt.LockKey), - PolicyCheckOutput: strings.Join(outputs, "\n"), + PolicySetResults: policySetResults, RePlanCmd: ctx.RePlanCmd, ApplyCmd: ctx.ApplyCmd, // set this to false right now because we don't have this information // TODO: refactor the templates in a sane way so we don't need this HasDiverged: false, - }, "", nil + }, strings.Join(failures, "\n"), nil } func (p *DefaultProjectCommandRunner) doPlan(ctx command.ProjectContext) (*models.PlanSuccess, string, error) {