From e4ddfb411c26ea6884d188db7a401965cf3d938b Mon Sep 17 00:00:00 2001
From: Alex Goodman <wagoodman@users.noreply.github.com>
Date: Fri, 26 Jul 2019 10:27:00 -0400
Subject: [PATCH] Expose CI validation options to the CLI (#212)

* expose ci config to cli switches; addresses #198

* added/updated tests for ci configuration
---
 .gitignore                   |   3 +
 cmd/analyze.go               |  17 ++++--
 cmd/build.go                 |   2 +
 cmd/ci.go                    |  39 ++++++++++++
 cmd/root.go                  |  19 +++++-
 runtime/ci/evaluator.go      | 112 ++++++++++++++++++++++-------------
 runtime/ci/evaluator_test.go |  19 +++---
 runtime/ci/rules.go          |  99 +++++++++++++++++++++++++++----
 runtime/ci/types.go          |  47 ---------------
 runtime/run.go               |  14 +----
 runtime/types.go             |  13 ++--
 11 files changed, 256 insertions(+), 128 deletions(-)
 create mode 100644 cmd/ci.go
 delete mode 100644 runtime/ci/types.go

diff --git a/.gitignore b/.gitignore
index 149cdc3..33ce974 100644
--- a/.gitignore
+++ b/.gitignore
@@ -21,3 +21,6 @@
 /dist
 .cover
 coverage.txt
+
+# ignore the binary
+dive
diff --git a/cmd/analyze.go b/cmd/analyze.go
index 937378a..8061de9 100644
--- a/cmd/analyze.go
+++ b/cmd/analyze.go
@@ -12,6 +12,7 @@ import (
 // image analysis to the screen
 func doAnalyzeCmd(cmd *cobra.Command, args []string) {
 	defer utils.Cleanup()
+
 	if len(args) == 0 {
 		printVersionFlag, err := cmd.PersistentFlags().GetBool("version")
 		if err == nil && printVersionFlag {
@@ -20,22 +21,28 @@ func doAnalyzeCmd(cmd *cobra.Command, args []string) {
 		}
 
 		fmt.Println("No image argument given")
-		_ = cmd.Help()
 		utils.Exit(1)
 	}
 
 	userImage := args[0]
 	if userImage == "" {
 		fmt.Println("No image argument given")
-		_ = cmd.Help()
 		utils.Exit(1)
 	}
 
 	initLogging()
 
+	isCi, ciConfig, err := configureCi()
+
+	if err != nil {
+		fmt.Printf("ci configuration error: %v\n", err)
+		utils.Exit(1)
+	}
+
 	runtime.Run(runtime.Options{
-		ImageId:      userImage,
-		ExportFile:   exportFile,
-		CiConfigFile: ciConfigFile,
+		Ci:         isCi,
+		ImageId:    userImage,
+		ExportFile: exportFile,
+		CiConfig:   ciConfig,
 	})
 }
diff --git a/cmd/build.go b/cmd/build.go
index a29d8a4..1ff2220 100644
--- a/cmd/build.go
+++ b/cmd/build.go
@@ -25,7 +25,9 @@ func doBuildCmd(cmd *cobra.Command, args []string) {
 	initLogging()
 
 	runtime.Run(runtime.Options{
+		Ci:         isCi,
 		BuildArgs:  args,
 		ExportFile: exportFile,
+		CiConfig:   ciConfig,
 	})
 }
diff --git a/cmd/ci.go b/cmd/ci.go
new file mode 100644
index 0000000..28a0867
--- /dev/null
+++ b/cmd/ci.go
@@ -0,0 +1,39 @@
+package cmd
+
+import (
+	"bytes"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"strconv"
+
+	"github.com/spf13/viper"
+)
+
+func configureCi() (bool, *viper.Viper, error) {
+
+	isCiFromEnv, _ := strconv.ParseBool(os.Getenv("CI"))
+	isCi = isCi || isCiFromEnv
+
+	if isCi {
+		ciConfig.SetConfigType("yaml")
+
+		if _, err := os.Stat(ciConfigFile); !os.IsNotExist(err) {
+			fmt.Printf("  Using CI config: %s\n", ciConfigFile)
+
+			fileBytes, err := ioutil.ReadFile(ciConfigFile)
+			if err != nil {
+				return isCi, nil, err
+			}
+
+			err = ciConfig.ReadConfig(bytes.NewBuffer(fileBytes))
+			if err != nil {
+				return isCi, nil, err
+			}
+		} else {
+			fmt.Println("  Using default CI config")
+		}
+	}
+
+	return isCi, ciConfig, nil
+}
diff --git a/cmd/root.go b/cmd/root.go
index e360cbf..0edc27b 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -18,6 +18,8 @@ import (
 var cfgFile string
 var exportFile string
 var ciConfigFile string
+var ciConfig = viper.New()
+var isCi bool
 
 // rootCmd represents the base command when called without any subcommands
 var rootCmd = &cobra.Command{
@@ -39,13 +41,26 @@ func Execute() {
 }
 
 func init() {
+	initCli()
 	cobra.OnInitialize(initConfig)
+}
 
+func initCli() {
 	rootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default is $HOME/.dive.yaml, ~/.config/dive/*.yaml, or $XDG_CONFIG_HOME/dive.yaml)")
 	rootCmd.PersistentFlags().BoolP("version", "v", false, "display version number")
-
+	rootCmd.Flags().BoolVar(&isCi, "ci", false, "Skip the interactive TUI and validate against CI rules (same as env var CI=true)")
 	rootCmd.Flags().StringVarP(&exportFile, "json", "j", "", "Skip the interactive TUI and write the layer analysis statistics to a given file.")
 	rootCmd.Flags().StringVar(&ciConfigFile, "ci-config", ".dive-ci", "If CI=true in the environment, use the given yaml to drive validation rules.")
+
+	rootCmd.Flags().String("lowestEfficiency", "0.9", "(only valid with --ci given) lowest allowable image efficiency, otherwise CI validation will fail.")
+	rootCmd.Flags().String("highestWastedBytes", "disabled", "(only valid with --ci given) highest allowable bytes wasted, otherwise CI validation will fail.")
+	rootCmd.Flags().String("highestUserWastedPercent", "0.1", "(only valid with --ci given) highest allowable percentage of bytes wasted, otherwise CI validation will fail.")
+
+	for _, key := range []string{"lowestEfficiency", "highestWastedBytes", "highestUserWastedPercent"} {
+		if err := ciConfig.BindPFlag(fmt.Sprintf("rules.%s", key), rootCmd.Flags().Lookup(key)); err != nil {
+			log.Fatal("Unable to bind flag:", err)
+		}
+	}
 }
 
 // initConfig reads in config file and ENV variables if set.
@@ -55,7 +70,7 @@ func initConfig() {
 
 	viper.SetDefault("log.level", log.InfoLevel.String())
 	viper.SetDefault("log.path", "./dive.log")
-	viper.SetDefault("log.enabled", true)
+	viper.SetDefault("log.enabled", false)
 	// keybindings: status view / global
 	viper.SetDefault("keybinding.quit", "ctrl+c")
 	viper.SetDefault("keybinding.toggle-view", "tab")
diff --git a/runtime/ci/evaluator.go b/runtime/ci/evaluator.go
index 8a95440..9e1a7f9 100644
--- a/runtime/ci/evaluator.go
+++ b/runtime/ci/evaluator.go
@@ -1,76 +1,101 @@
 package ci
 
 import (
-	"bytes"
 	"fmt"
-	"io/ioutil"
 	"sort"
 	"strings"
 
-	"github.com/logrusorgru/aurora"
 	"github.com/spf13/viper"
+
+	"github.com/logrusorgru/aurora"
 	"github.com/wagoodman/dive/image"
 )
 
-func NewEvaluator() *Evaluator {
-	ciConfig := viper.New()
-	ciConfig.SetConfigType("yaml")
+type Evaluator struct {
+	Rules         []Rule
+	Results       map[string]RuleResult
+	Tally         ResultTally
+	Pass          bool
+	Misconfigured bool
+}
 
-	ciConfig.SetDefault("rules.lowestEfficiency", 0.9)
-	ciConfig.SetDefault("rules.highestWastedBytes", "disabled")
-	ciConfig.SetDefault("rules.highestUserWastedPercent", 0.1)
+type ResultTally struct {
+	Pass  int
+	Fail  int
+	Skip  int
+	Warn  int
+	Total int
+}
 
+func NewEvaluator(config *viper.Viper) *Evaluator {
 	return &Evaluator{
-		Config:  ciConfig,
-		Rules:   loadCiRules(),
+		Rules:   loadCiRules(config),
 		Results: make(map[string]RuleResult),
 		Pass:    true,
 	}
 }
 
-func (ci *Evaluator) LoadConfig(configFile string) error {
-	fileBytes, err := ioutil.ReadFile(configFile)
-	if err != nil {
-		return err
-	}
-
-	err = ci.Config.ReadConfig(bytes.NewBuffer(fileBytes))
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
 func (ci *Evaluator) isRuleEnabled(rule Rule) bool {
-	value := ci.Config.GetString(rule.Key())
-	return value != "disabled"
+	return rule.Configuration() != "disabled"
 }
 
 func (ci *Evaluator) Evaluate(analysis *image.AnalysisResult) bool {
+	canEvaluate := true
 	for _, rule := range ci.Rules {
-		if ci.isRuleEnabled(rule) {
-
-			value := ci.Config.GetString(rule.Key())
-			status, message := rule.Evaluate(analysis, value)
-
-			if _, exists := ci.Results[rule.Key()]; exists {
-				panic(fmt.Errorf("CI rule result recorded twice: %s", rule.Key()))
-			}
-
-			if status == RuleFailed {
-				ci.Pass = false
-			}
-
+		if !ci.isRuleEnabled(rule) {
 			ci.Results[rule.Key()] = RuleResult{
-				status:  status,
-				message: message,
+				status:  RuleConfigured,
+				message: "rule disabled",
 			}
+			continue
+		}
+
+		err := rule.Validate()
+		if err != nil {
+			ci.Results[rule.Key()] = RuleResult{
+				status:  RuleMisconfigured,
+				message: err.Error(),
+			}
+			canEvaluate = false
 		} else {
+			ci.Results[rule.Key()] = RuleResult{
+				status:  RuleConfigured,
+				message: "test",
+			}
+		}
+
+	}
+
+	if !canEvaluate {
+		ci.Pass = false
+		ci.Misconfigured = true
+		return ci.Pass
+	}
+
+	for _, rule := range ci.Rules {
+		if !ci.isRuleEnabled(rule) {
 			ci.Results[rule.Key()] = RuleResult{
 				status:  RuleDisabled,
 				message: "rule disabled",
 			}
+			continue
 		}
+
+		status, message := rule.Evaluate(analysis)
+
+		if value, exists := ci.Results[rule.Key()]; exists && value.status != RuleConfigured && value.status != RuleMisconfigured {
+			panic(fmt.Errorf("CI rule result recorded twice: %s", rule.Key()))
+		}
+
+		if status == RuleFailed {
+			ci.Pass = false
+		}
+
+		ci.Results[rule.Key()] = RuleResult{
+			status:  status,
+			message: message,
+		}
+
 	}
 
 	ci.Tally.Total = len(ci.Results)
@@ -115,6 +140,11 @@ func (ci *Evaluator) Report() {
 		}
 	}
 
+	if ci.Misconfigured {
+		fmt.Println(aurora.Red("CI Misconfigured"))
+		return
+	}
+
 	summary := fmt.Sprintf("Result:%s [Total:%d] [Passed:%d] [Failed:%d] [Warn:%d] [Skipped:%d]", status, ci.Tally.Total, ci.Tally.Pass, ci.Tally.Fail, ci.Tally.Warn, ci.Tally.Skip)
 	if ci.Pass {
 		fmt.Println(aurora.Green(summary))
diff --git a/runtime/ci/evaluator_test.go b/runtime/ci/evaluator_test.go
index 0090523..f5ffa4e 100644
--- a/runtime/ci/evaluator_test.go
+++ b/runtime/ci/evaluator_test.go
@@ -22,34 +22,37 @@ func Test_Evaluator(t *testing.T) {
 		expectedPass   bool
 		expectedResult map[string]RuleStatus
 	}{
-		"allFail":     {"0.99", "1B", "0.01", false, map[string]RuleStatus{"lowestEfficiency": RuleFailed, "highestWastedBytes": RuleFailed, "highestUserWastedPercent": RuleFailed}},
-		"allPass":     {"0.9", "50kB", "0.1", true, map[string]RuleStatus{"lowestEfficiency": RulePassed, "highestWastedBytes": RulePassed, "highestUserWastedPercent": RulePassed}},
-		"allDisabled": {"disabled", "disabled", "disabled", true, map[string]RuleStatus{"lowestEfficiency": RuleDisabled, "highestWastedBytes": RuleDisabled, "highestUserWastedPercent": RuleDisabled}},
+		"allFail":           {"0.99", "1B", "0.01", false, map[string]RuleStatus{"lowestEfficiency": RuleFailed, "highestWastedBytes": RuleFailed, "highestUserWastedPercent": RuleFailed}},
+		"allPass":           {"0.9", "50kB", "0.1", true, map[string]RuleStatus{"lowestEfficiency": RulePassed, "highestWastedBytes": RulePassed, "highestUserWastedPercent": RulePassed}},
+		"allDisabled":       {"disabled", "disabled", "disabled", true, map[string]RuleStatus{"lowestEfficiency": RuleDisabled, "highestWastedBytes": RuleDisabled, "highestUserWastedPercent": RuleDisabled}},
+		"misconfiguredHigh": {"1.1", "1BB", "10", false, map[string]RuleStatus{"lowestEfficiency": RuleMisconfigured, "highestWastedBytes": RuleMisconfigured, "highestUserWastedPercent": RuleMisconfigured}},
+		"misconfiguredLow":  {"-9", "-1BB", "-0.1", false, map[string]RuleStatus{"lowestEfficiency": RuleMisconfigured, "highestWastedBytes": RuleMisconfigured, "highestUserWastedPercent": RuleMisconfigured}},
 	}
 
-	for _, test := range table {
-		evaluator := NewEvaluator()
-
+	for name, test := range table {
 		ciConfig := viper.New()
 		ciConfig.SetDefault("rules.lowestEfficiency", test.efficiency)
 		ciConfig.SetDefault("rules.highestWastedBytes", test.wastedBytes)
 		ciConfig.SetDefault("rules.highestUserWastedPercent", test.wastedPercent)
-		evaluator.Config = ciConfig
+
+		evaluator := NewEvaluator(ciConfig)
 
 		pass := evaluator.Evaluate(result)
 
 		if test.expectedPass != pass {
+			t.Logf("Test: %s", name)
 			t.Errorf("Test_Evaluator: expected pass=%v, got %v", test.expectedPass, pass)
 		}
 
 		if len(test.expectedResult) != len(evaluator.Results) {
+			t.Logf("Test: %s", name)
 			t.Errorf("Test_Evaluator: expected %v results, got %v", len(test.expectedResult), len(evaluator.Results))
 		}
 
 		for rule, actualResult := range evaluator.Results {
 			expectedStatus := test.expectedResult[strings.TrimPrefix(rule, "rules.")]
 			if expectedStatus != actualResult.status {
-				t.Errorf("   %v: expected %v rule failures, got %v", rule, expectedStatus, actualResult.status)
+				t.Errorf("   %v: expected %v rule failures, got %v: %v", rule, expectedStatus, actualResult.status, actualResult)
 			}
 		}
 
diff --git a/runtime/ci/rules.go b/runtime/ci/rules.go
index 4eeb5bf..a2e1c37 100644
--- a/runtime/ci/rules.go
+++ b/runtime/ci/rules.go
@@ -4,15 +4,50 @@ import (
 	"fmt"
 	"strconv"
 
+	"github.com/spf13/viper"
+
 	"github.com/dustin/go-humanize"
 	"github.com/logrusorgru/aurora"
 	"github.com/wagoodman/dive/image"
 )
 
-func newGenericCiRule(key string, evaluator func(*image.AnalysisResult, string) (RuleStatus, string)) *GenericCiRule {
+const (
+	RuleUnknown = iota
+	RulePassed
+	RuleFailed
+	RuleWarning
+	RuleDisabled
+	RuleMisconfigured
+	RuleConfigured
+)
+
+type Rule interface {
+	Key() string
+	Configuration() string
+	Validate() error
+	Evaluate(*image.AnalysisResult) (RuleStatus, string)
+}
+
+type GenericCiRule struct {
+	key             string
+	configValue     string
+	configValidator func(string) error
+	evaluator       func(*image.AnalysisResult, string) (RuleStatus, string)
+}
+
+type RuleStatus int
+
+type RuleResult struct {
+	status  RuleStatus
+	message string
+}
+
+func newGenericCiRule(key string, configValue string, validator func(string) error, evaluator func(*image.AnalysisResult, string) (RuleStatus, string)) *GenericCiRule {
 	return &GenericCiRule{
-		key:       key,
-		evaluator: evaluator,
+		key:             key,
+		configValue:     configValue,
+		configValidator: validator,
+		evaluator:       evaluator,
 	}
 }
 
@@ -20,8 +55,16 @@ func (rule *GenericCiRule) Key() string {
 	return rule.key
 }
 
-func (rule *GenericCiRule) Evaluate(result *image.AnalysisResult, value string) (RuleStatus, string) {
-	return rule.evaluator(result, value)
+func (rule *GenericCiRule) Configuration() string {
+	return rule.configValue
+}
+
+func (rule *GenericCiRule) Validate() error {
+	return rule.configValidator(rule.configValue)
+}
+
+func (rule *GenericCiRule) Evaluate(result *image.AnalysisResult) (RuleStatus, string) {
+	return rule.evaluator(result, rule.configValue)
 }
 
 func (status RuleStatus) String() string {
@@ -34,16 +77,31 @@ func (status RuleStatus) String() string {
 		return aurora.Blue("WARN").String()
 	case RuleDisabled:
 		return aurora.Blue("SKIP").String()
+	case RuleMisconfigured:
+		return aurora.Bold(aurora.Inverse(aurora.Red("MISCONFIGURED"))).String()
+	case RuleConfigured:
+		return "CONFIGURED   "
 	default:
 		return aurora.Inverse("Unknown").String()
 	}
 }
 
-func loadCiRules() []Rule {
+func loadCiRules(config *viper.Viper) []Rule {
 	var rules = make([]Rule, 0)
-
+	var ruleKey = "lowestEfficiency"
 	rules = append(rules, newGenericCiRule(
-		"rules.lowestEfficiency",
+		ruleKey,
+		config.GetString(fmt.Sprintf("rules.%s", ruleKey)),
+		func(value string) error {
+			lowestEfficiency, err := strconv.ParseFloat(value, 64)
+			if err != nil {
+				return fmt.Errorf("invalid config value ('%v'): %v", value, err)
+			}
+			if lowestEfficiency < 0 || lowestEfficiency > 1 {
+				return fmt.Errorf("lowestEfficiency config value is outside allowed range (0-1), given '%s'", value)
+			}
+			return nil
+		},
 		func(analysis *image.AnalysisResult, value string) (RuleStatus, string) {
 			lowestEfficiency, err := strconv.ParseFloat(value, 64)
 			if err != nil {
@@ -56,8 +114,17 @@ func loadCiRules() []Rule {
 		},
 	))
 
+	ruleKey = "highestWastedBytes"
 	rules = append(rules, newGenericCiRule(
-		"rules.highestWastedBytes",
+		ruleKey,
+		config.GetString(fmt.Sprintf("rules.%s", ruleKey)),
+		func(value string) error {
+			_, err := humanize.ParseBytes(value)
+			if err != nil {
+				return fmt.Errorf("invalid config value ('%v'): %v", value, err)
+			}
+			return nil
+		},
 		func(analysis *image.AnalysisResult, value string) (RuleStatus, string) {
 			highestWastedBytes, err := humanize.ParseBytes(value)
 			if err != nil {
@@ -70,8 +137,20 @@ func loadCiRules() []Rule {
 		},
 	))
 
+	ruleKey = "highestUserWastedPercent"
 	rules = append(rules, newGenericCiRule(
-		"rules.highestUserWastedPercent",
+		ruleKey,
+		config.GetString(fmt.Sprintf("rules.%s", ruleKey)),
+		func(value string) error {
+			highestUserWastedPercent, err := strconv.ParseFloat(value, 64)
+			if err != nil {
+				return fmt.Errorf("invalid config value ('%v'): %v", value, err)
+			}
+			if highestUserWastedPercent < 0 || highestUserWastedPercent > 1 {
+				return fmt.Errorf("highestUserWastedPercent config value is outside allowed range (0-1), given '%s'", value)
+			}
+			return nil
+		},
 		func(analysis *image.AnalysisResult, value string) (RuleStatus, string) {
 			highestUserWastedPercent, err := strconv.ParseFloat(value, 64)
 			if err != nil {
diff --git a/runtime/ci/types.go b/runtime/ci/types.go
deleted file mode 100644
index 4910c29..0000000
--- a/runtime/ci/types.go
+++ /dev/null
@@ -1,47 +0,0 @@
-package ci
-
-import (
-	"github.com/spf13/viper"
-	"github.com/wagoodman/dive/image"
-)
-
-type RuleStatus int
-
-type RuleResult struct {
-	status  RuleStatus
-	message string
-}
-
-const (
-	RuleUnknown = iota
-	RulePassed
-	RuleFailed
-	RuleWarning
-	RuleDisabled
-)
-
-type Rule interface {
-	Key() string
-	Evaluate(*image.AnalysisResult, string) (RuleStatus, string)
-}
-
-type GenericCiRule struct {
-	key       string
-	evaluator func(*image.AnalysisResult, string) (RuleStatus, string)
-}
-
-type Evaluator struct {
-	Config  *viper.Viper
-	Rules   []Rule
-	Results map[string]RuleResult
-	Tally   ResultTally
-	Pass    bool
-}
-
-type ResultTally struct {
-	Pass  int
-	Fail  int
-	Skip  int
-	Warn  int
-	Total int
-}
diff --git a/runtime/run.go b/runtime/run.go
index 8def88a..e6bd46e 100644
--- a/runtime/run.go
+++ b/runtime/run.go
@@ -5,7 +5,6 @@ import (
 	"io/ioutil"
 	"log"
 	"os"
-	"strconv"
 
 	"github.com/dustin/go-humanize"
 	"github.com/logrusorgru/aurora"
@@ -25,15 +24,9 @@ func runCi(analysis *image.AnalysisResult, options Options) {
 	fmt.Printf("  wastedBytes: %d bytes (%s)\n", analysis.WastedBytes, humanize.Bytes(analysis.WastedBytes))
 	fmt.Printf("  userWastedPercent: %2.4f %%\n", analysis.WastedUserPercent*100)
 
-	fmt.Println(title("Run CI Validations..."))
-	evaluator := ci.NewEvaluator()
+	evaluator := ci.NewEvaluator(options.CiConfig)
 
-	err := evaluator.LoadConfig(options.CiConfigFile)
-	if err != nil {
-		fmt.Println("  Using default CI config")
-	} else {
-		fmt.Printf("  Using CI config: %s\n", options.CiConfigFile)
-	}
+	fmt.Println(title("Run CI Validations..."))
 
 	pass := evaluator.Evaluate(analysis)
 	evaluator.Report()
@@ -71,7 +64,6 @@ func runBuild(buildArgs []string) string {
 func Run(options Options) {
 	doExport := options.ExportFile != ""
 	doBuild := len(options.BuildArgs) > 0
-	isCi, _ := strconv.ParseBool(os.Getenv("CI"))
 
 	if doBuild {
 		fmt.Println(title("Building image..."))
@@ -115,7 +107,7 @@ func Run(options Options) {
 		}
 	}
 
-	if isCi {
+	if options.Ci {
 		runCi(result, options)
 	} else {
 		if doExport {
diff --git a/runtime/types.go b/runtime/types.go
index 002fc85..02f64ca 100644
--- a/runtime/types.go
+++ b/runtime/types.go
@@ -1,10 +1,15 @@
 package runtime
 
+import (
+	"github.com/spf13/viper"
+)
+
 type Options struct {
-	ImageId      string
-	ExportFile   string
-	CiConfigFile string
-	BuildArgs    []string
+	Ci         bool
+	ImageId    string
+	ExportFile string
+	CiConfig   *viper.Viper
+	BuildArgs  []string
 }
 
 type export struct {