From 54646650c7ef9d4551d22cd54bf8b4e5330aa001 Mon Sep 17 00:00:00 2001 From: decentral1se Date: Sun, 22 Jan 2023 12:24:09 +0100 Subject: [PATCH] fix!: disable traefik linting when DOMAIN isn't present Also reformats the linting output to be more readable. Closes https://git.coopcloud.tech/coop-cloud/organising/issues/319. --- cli/recipe/lint.go | 60 ++++++++++++++++++++++++++----------- pkg/lint/recipe.go | 75 +++++++++++++++++++++++++++++++++++++++------- 2 files changed, 107 insertions(+), 28 deletions(-) diff --git a/cli/recipe/lint.go b/cli/recipe/lint.go index 180597dc..802088e4 100644 --- a/cli/recipe/lint.go +++ b/cli/recipe/lint.go @@ -30,38 +30,62 @@ var recipeLintCommand = cli.Command{ logrus.Fatal(err) } - tableCol := []string{"ref", "rule", "satisfied", "severity", "resolve"} + tableCol := []string{"ref", "rule", "severity", "satisfied", "skipped", "resolve"} table := formatter.CreateTable(tableCol) hasError := false bar := formatter.CreateProgressbar(-1, "running recipe lint rules...") for level := range lint.LintRules { for _, rule := range lint.LintRules[level] { - ok, err := rule.Function(recipe) - if err != nil { - logrus.Warn(err) + if internal.OnlyErrors && rule.Level != "error" { + logrus.Debugf("skipping %s, does not have level \"error\"", rule.Ref) + continue } - if !ok && rule.Level == "error" { - hasError = true + skipped := false + if rule.Skip(recipe) { + skipped = true } - var result string - if ok { - result = "yes" - } else { - result = "NO" + skippedOutput := "-" + if skipped { + skippedOutput = "yes" } - if internal.OnlyErrors { - if !ok && rule.Level == "error" { - table.Append([]string{rule.Ref, rule.Description, result, rule.Level, rule.HowToResolve}) - bar.Add(1) + satisfied := false + if !skipped { + ok, err := rule.Function(recipe) + if err != nil { + logrus.Warn(err) + } + + if !ok && rule.Level == "error" { + hasError = true + } + + if ok { + satisfied = true } - } else { - table.Append([]string{rule.Ref, rule.Description, result, rule.Level, rule.HowToResolve}) - bar.Add(1) } + + satisfiedOutput := "yes" + if !satisfied { + satisfiedOutput = "NO" + if skipped { + satisfiedOutput = "-" + } + } + + table.Append([]string{ + rule.Ref, + rule.Description, + rule.Level, + satisfiedOutput, + skippedOutput, + rule.HowToResolve, + }) + + bar.Add(1) } } diff --git a/pkg/lint/recipe.go b/pkg/lint/recipe.go index b0759cf8..ee9a15c1 100644 --- a/pkg/lint/recipe.go +++ b/pkg/lint/recipe.go @@ -19,12 +19,40 @@ var Critical = "critical" type LintFunction func(recipe.Recipe) (bool, error) +// SkipFunction determines whether the LintFunction is run or not. It should +// not take the lint rule level into account because some rules are always an +// error but may depend on some additional context of the recipe configuration. +// This function aims to cover those additional cases. +type SkipFunction func(recipe.Recipe) (bool, error) + +// LintRule is a linting rule which helps a recipe maintainer avoid common +// problems in their recipe configurations. We aim to highlight things that +// might result in critical errors or hours lost in debugging obscure +// Docker-isms. Humans make the final call on these rules, please raise an +// issue if you disagree. type LintRule struct { - Ref string - Level string - Description string - HowToResolve string - Function LintFunction + Ref string // Reference of the linting rule + Level string // Level of the warning + Description string // Description of the issue + HowToResolve string // Documentation for recipe maintainer + Function LintFunction // Rule implementation + SkipCondition SkipFunction // Whether or not to execute the lint rule +} + +// Skip implements the SkipFunction for the lint rule. +func (l LintRule) Skip(recipe recipe.Recipe) bool { + if l.SkipCondition != nil { + ok, err := l.SkipCondition(recipe) + if err != nil { + logrus.Debugf("%s: skip condition: %s", l.Ref, err) + } + if ok { + logrus.Debugf("skipping %s based on skip condition", l.Ref) + return true + } + } + + return false } var LintRules = map[string][]LintRule{ @@ -102,11 +130,12 @@ var LintRules = map[string][]LintRule{ Function: LintAppService, }, { - Ref: "R010", - Level: "error", - Description: "traefik routing enabled", - HowToResolve: "include \"traefik.enable=true\" deploy label", - Function: LintTraefikEnabled, + Ref: "R010", + Level: "error", + Description: "traefik routing enabled", + HowToResolve: "include \"traefik.enable=true\" deploy label", + Function: LintTraefikEnabled, + SkipCondition: LintTraefikEnabledSkipCondition, }, { Ref: "R011", @@ -125,6 +154,9 @@ var LintRules = map[string][]LintRule{ }, } +// LintForErrors lints specifically for errors and not other levels. This is +// used in code paths such as "app deploy" to avoid nasty surprises but not for +// the typical linting commands, which do handle other levels. func LintForErrors(recipe recipe.Recipe) error { logrus.Debugf("linting for critical errors in %s configs", recipe.Name) @@ -132,7 +164,12 @@ func LintForErrors(recipe recipe.Recipe) error { if level != "error" { continue } + for _, rule := range LintRules[level] { + if rule.Skip(recipe) { + continue + } + ok, err := rule.Function(recipe) if err != nil { return err @@ -175,6 +212,24 @@ func LintAppService(recipe recipe.Recipe) (bool, error) { return false, nil } +// LintTraefikEnabledSkipCondition signals a skip for this linting rule if it +// confirms that there is no "DOMAIN=..." in the .env.sample configuration of +// the recipe. This typically means that no domain is required to deploy and +// therefore no matching traefik deploy label will be present. +func LintTraefikEnabledSkipCondition(recipe recipe.Recipe) (bool, error) { + envSamplePath := path.Join(config.RECIPES_DIR, recipe.Name, ".env.sample") + sampleEnv, err := config.ReadEnv(envSamplePath) + if err != nil { + return false, fmt.Errorf("Unable to discover .env.sample for %s", recipe.Name) + } + + if _, ok := sampleEnv["DOMAIN"]; !ok { + return true, nil + } + + return false, nil +} + func LintTraefikEnabled(recipe recipe.Recipe) (bool, error) { for _, service := range recipe.Config.Services { for label := range service.Deploy.Labels {