Make change checks more fine-grained

Author: yolken-segment

cmd/kubeapply-lambda/main.go

@@ -22,9 +22,11 @@ var (
 	sess        *session.Session
 	statsClient stats.StatsClient
 
-	automerge   bool
-	debug       bool
-	strictCheck bool
+	automerge       bool
+	debug           bool
+	strictCheck     bool
+	greenCIRequired bool
+	reviewRequired  bool
 
 	logsURL = getLogsURL()
 )
@@ -75,6 +77,15 @@ var (
 	// Optional, defaults to false.
 	strictCheckStr = os.Getenv("KUBEAPPLY_STRICT_CHECK")
 
+	// Whether a green CI is required to apply. Ideally, should be set to "true",
+	// but based on how long the CI takes, might be easier to have it be "false" in
+	// non-production environments.
+	greenCIRequiredStr = os.Getenv("KUBEAPPLY_GREEN_CI_REQUIRED")
+
+	// Whether a review is required to apply. Generally "true" in production and
+	// otherwise "false".
+	reviewRequiredStr = os.Getenv("KUBEAPPLY_REVIEW_REQUIRED")
+
 	// SSM parameter used for fetching webhook secret.
 	webhookSecretSSMParam = os.Getenv("KUBEAPPLY_WEBHOOK_SECRET_SSM_PARAM")
 )
@@ -154,6 +165,14 @@ func init() {
 		strictCheck = true
 	}
 
+	if strings.ToLower(greenCIRequiredStr) == "true" {
+		greenCIRequired = true
+	}
+
+	if strings.ToLower(reviewRequiredStr) == "true" {
+		reviewRequired = true
+	}
+
 	if strings.ToLower(automergeStr) == "true" {
 		automerge = true
 	}
@@ -223,6 +242,8 @@ func handleRequest(
 			Env:                   env,
 			Version:               version.Version,
 			StrictCheck:           strictCheck,
+			GreenCIRequired:       greenCIRequired,
+			ReviewRequired:        reviewRequired,
 			Automerge:             automerge,
 			UseLocks:              true,
 			ApplyConsistencyCheck: false,

cmd/kubeapply-server/main.go

@@ -41,8 +41,12 @@ type Config struct {
 	Env           string `conf:"env"            help:"only consider changes for this environment"`
 	GithubToken   string `conf:"github-token"   help:"token for Github API access"`
 	LogsURL       string `conf:"logs-url"       help:"url for logs; used as link for status checks"`
-	StrictCheck   bool   `conf:"strict-check"   help:"ensure green status and approval before apply"`
 	WebhookSecret string `conf:"webhook-secret" help:"shared secret set in Github webhooks"`
+
+	// TODO: Deprecate StrictCheck since it's covered by the parameters below that.
+	StrictCheck     bool `conf:"strict-check"      help:"ensure green status and approval before apply"`
+	GreenCIRequired bool `conf:"green-ci-required" help:"require green CI before applying"`
+	ReviewRequired  bool `conf:"review-required"   help:"require review before applying:"`
 }
 
 var config = Config{
@@ -121,6 +125,8 @@ func webhookHTTPHandler(
 			ApplyConsistencyCheck: false,
 			Automerge:             config.Automerge,
 			StrictCheck:           config.StrictCheck,
+			GreenCIRequired:       config.GreenCIRequired,
+			ReviewRequired:        config.ReviewRequired,
 			Debug:                 config.Debug,
 		},
 	)

cmd/kubeapply/subcmd/pull_request.go

@@ -56,8 +56,16 @@ type pullRequestFlags struct {
 	// Full name of the repo, in [owner]/[name] format
 	repo string
 
-	// Whether to be strict about checking for approvals and green github status
+	// Whether to be strict about checking for approvals and green github status.
+	//
+	// Deprecated, to be replaced by the values below.
 	strictCheck bool
+
+	// Whether green CI is required to apply
+	greenCIRequired bool
+
+	// Whether a review is required to apply
+	reviewRequired bool
 }
 
 var pullRequestFlagValues pullRequestFlags
@@ -111,6 +119,12 @@ func init() {
 		"",
 		"Installation ID for github app",
 	)
+	pullRequestCmd.Flags().BoolVar(
+		&pullRequestFlagValues.greenCIRequired,
+		"green-ci-required",
+		false,
+		"Whether a green CI is required to apply",
+	)
 	pullRequestCmd.Flags().IntVar(
 		&pullRequestFlagValues.pullRequestNum,
 		"pull-request",
@@ -123,6 +137,12 @@ func init() {
 		"",
 		"Repo to post comment in, in format [owner]/[name]",
 	)
+	pullRequestCmd.Flags().BoolVar(
+		&pullRequestFlagValues.reviewRequired,
+		"review-required",
+		false,
+		"Whether a review is required to apply",
+	)
 	pullRequestCmd.Flags().BoolVar(
 		&pullRequestFlagValues.strictCheck,
 		"strict-check",
@@ -265,6 +285,8 @@ func pullRequestRun(cmd *cobra.Command, args []string) error {
 			ApplyConsistencyCheck: false,
 			Automerge:             pullRequestFlagValues.automerge,
 			StrictCheck:           pullRequestFlagValues.strictCheck,
+			GreenCIRequired:       pullRequestFlagValues.greenCIRequired,
+			ReviewRequired:        pullRequestFlagValues.reviewRequired,
 			Debug:                 debug,
 		},
 	)

pkg/config/config.go

@@ -76,6 +76,12 @@ type ClusterConfig struct {
 	// Optional, defaults to false.
 	GithubIgnore bool `json:"ignore"`
 
+	// ReviewOptional indicates that reviews should not be required for changes in this
+	// cluster even if strict mode is on.
+	//
+	// Optional, and only applicable to webhooks mode.
+	GithubReviewOptional bool `json:"reviewOptional"`
+
 	// VersionConstraint is a string version constraint against with the kubeapply binary
 	// will be checked. See https://github.com/Masterminds/semver for details on the expected
 	// format.

pkg/events/handler.go

@@ -51,8 +51,16 @@ type WebhookHandlerSettings struct {
 
 	// StrictCheck indicates whether we should block applies on having an approval and all
 	// green statuses.
+	//
+	// Note: To be deprecated and replaced with CICheck and ReviewRequired below.
 	StrictCheck bool
 
+	// GreenCIRequired indicates whether CI must be green before allowing applies.
+	GreenCIRequired bool
+
+	// ReviewRequired indicates whether a review is required before allowing applies.
+	ReviewRequired bool
+
 	// UseLocks indicates whether we should use locking to prevent overlapping handler calls
 	// for a cluster.
 	UseLocks bool
@@ -381,14 +389,29 @@ func (whh *WebhookHandler) runApply(
 		Env:               whh.settings.Env,
 	}
 
-	if whh.settings.StrictCheck && !statusOK {
+	overrideReviewRequired := true
+
+	for _, clusterClient := range clusterClients {
+		if !clusterClient.Config().GithubReviewOptional {
+			overrideReviewRequired = false
+			break
+		}
+	}
+	if overrideReviewRequired {
+		log.Info(
+			"Overriding review required because all clusters in this change have review optional",
+		)
+	}
+
+	if (whh.settings.StrictCheck || whh.settings.GreenCIRequired) && !statusOK {
 		applyErr = multilineError(
-			"Cannot run apply because strict-check is set to true and commit status is not green.",
+			"Cannot run apply because green-ci-required is set to true and commit status is not green.",
 			"Please fix status and try again.",
 		)
-	} else if whh.settings.StrictCheck && !approved {
+	} else if (whh.settings.StrictCheck || whh.settings.ReviewRequired) &&
+		!approved && !overrideReviewRequired {
 		applyErr = multilineError(
-			"Cannot run apply because strict-check is set to true and request is not approved.",
+			"Cannot run apply because review-required is set to true and request is not approved.",
 			"Please get at least one approval and try again.",
 		)
 	} else if behindBy > 0 {