fix!: disable traefik linting when DOMAIN isn't present

Also reformats the linting output to be more readable.

Closes coop-cloud/organising#319.

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)
 			}
 		}
 

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 {