DVO-110: Move disable checks to configmap

.gitignore

@@ -24,3 +24,6 @@ build/_output/*
 
 # go
 vendor
+
+# local dev default configmap
+config/deployment-validation-operator-config.yaml

README.md

@@ -99,29 +99,27 @@ The metrics generated by DVO can be scraped by anything that understands prometh
 oc process --local NAMESPACE='some-namespace' -f deploy/openshift/network-policies.yaml | oc create -f -
 ```
 
-## Enabling Checks
+## Configuring Checks
 
-In the validation engine file is a function called getDisabledChecks(). Comment out the check you would like to have available to the user base of DVO and push the change.
+DVO performs validation checks using kube-linter. The checks configuration is mirrored to the one for the kube-linter project. More information on configuration options can be found [here](https://github.com/stackrox/kube-linter/blob/main/docs/configuring-kubelinter.md), and a list of available checks  can be found [here](https://github.com/stackrox/kube-linter/blob/main/docs/generated/checks.md).
 
-```
-Location of file
-../pkg/validations/validation_engine.go 
-```
+To configure DVO with a different set of checks, create a ConfigMap in the cluster with the new checks configuration. An example of a configuration ConfigMap can be found [here](./deploy/openshift/configmap.yaml).
 
-Documentation of currently allowed checks can be found [here](./docs/checks.md)
+**constraint**: Currently, the configuration isn't continuously monitored and is only checked at startup. If a new set of checks is configured in a ConfigMap, the pod running DVO will need to be rebooted.
 
-## Disabling Checks
+### Enabling checks
 
-In the current state of DVO, all validation checks are performed with [kube-linter](https://github.com/stackrox/kube-linter).
+To enable all checks, set the `addAllBuiltIn` property to `true`. If you only want to enable individual checks, include them as a collection in the `include` property and leave `addAllBuiltIn` with a value of `false`.
 
-kube-linter supports functionality for [ignoring violations for a particular kubernetes object](https://github.com/stackrox/kube-linter/blob/main/docs/configuring-kubelinter.md#ignoring-violations-for-specific-cases). This functionality is also supported through DVO.
+The `include` property can work together with `doNotAutoAddDefaults` set to `true` in a whitelisting way. Only the checks collection passed in `include` will be executed.
 
-In the validation engine file is a function called getDisabledChecks(). Comment the check you would like to have disabled to the user base of DVO and push the change.
+### Disabling checks
 
-```
-Location of file 
-../pkg/validations/validation_engine.go 
-```
+To disable all checks, set the `doNotAutoAddDefaults` property to `true`. If you only want to disable individual checks, include them as a collection in the `exclude` property and leave `doNotAutoAddDefaults` with a value of `false`
+
+The `exclude` property takes precedence over the `include` property. If a particular check is in both collections, it will be excluded by default.
+
+The `exclude` property can work in conjunction with `addAllBuiltIn` set to `true` in a blacklisting fashion. All checks will be triggered and only the checks passed in `exclude` will be ignored.
 
 ## Tests
 

config.example.yaml

@@ -6,3 +6,11 @@ checks:
   # explicitly opt-out of checks that are not relevant using Exclude.
   # Takes precedence over doNotAutoAddDefaults, if both are set.
   addAllBuiltIn: true
+
+  # exclude will remove a set of checks from the validations.
+  # It is used when addAllBuiltIn is set to true.
+  exclude: ["check1", "check2"]
+
+  # include will add a set of checks to the validations.
+  # It is used when addAllBuiltIn is set to false.
+  include: ["check1", "check2"]

deploy/openshift/configmap.yaml

@@ -15,3 +15,5 @@ data:
       # explicitly opt-out of checks that are not relevant using Exclude.
       # Takes precedence over doNotAutoAddDefaults, if both are set.
       addAllBuiltIn: true
+
+      exclude: ["access-to-create-pods", "access-to-secrets", "cluster-admin-role-binding", "default-service-account", "deprecated-service-account-field", "docker-sock", "drop-net-raw-capability", "env-var-secret", "exposed-services", "latest-tag", "mismatching-selector", "no-extensions-v1beta", "no-liveness-probe", "no-read-only-root-fs", "no-readiness-probe", "no-rolling-update-strategy", "privileged-ports", "read-secret-from-env-var", "required-annotation-email", "required-label-owner", "sensitive-host-mounts", "ssh-port", "unsafe-proc-mount", "use-namespace", "wildcard-in-rules", "writable-host-mount"]

deploy/openshift/deployment-validation-operator-olm.yaml

@@ -50,6 +50,8 @@ objects:
         # explicitly opt-out of checks that are not relevant using Exclude.
         # Takes precedence over doNotAutoAddDefaults, if both are set.
         addAllBuiltIn: true
+
+        exclude: ["access-to-create-pods", "access-to-secrets", "cluster-admin-role-binding", "default-service-account", "deprecated-service-account-field", "docker-sock", "drop-net-raw-capability", "env-var-secret", "exposed-services", "latest-tag", "mismatching-selector", "no-extensions-v1beta", "no-liveness-probe", "no-read-only-root-fs", "no-readiness-probe", "no-rolling-update-strategy", "privileged-ports", "read-secret-from-env-var", "required-annotation-email", "required-label-owner", "sensitive-host-mounts", "ssh-port", "unsafe-proc-mount", "use-namespace", "wildcard-in-rules", "writable-host-mount"]
 - apiVersion: v1
   kind: Service
   metadata:

hack/olm-registry/olm-artifacts-template.yaml

@@ -150,3 +150,5 @@ objects:
                 # explicitly opt-out of checks that are not relevant using Exclude.
                 # Takes precedence over doNotAutoAddDefaults, if both are set.
                 addAllBuiltIn: true
+
+                exclude: ["access-to-create-pods", "access-to-secrets", "cluster-admin-role-binding", "default-service-account", "deprecated-service-account-field", "docker-sock", "drop-net-raw-capability", "env-var-secret", "exposed-services", "latest-tag", "mismatching-selector", "no-extensions-v1beta", "no-liveness-probe", "no-read-only-root-fs", "no-readiness-probe", "no-rolling-update-strategy", "privileged-ports", "read-secret-from-env-var", "required-annotation-email", "required-label-owner", "sensitive-host-mounts", "ssh-port", "unsafe-proc-mount", "use-namespace", "wildcard-in-rules", "writable-host-mount"]

pkg/controller/configmap_watcher.go

@@ -3,24 +3,44 @@ package controller
 import (
 	"context"
 	"fmt"
-	"strings"
 	"time"
 
+	"golang.stackrox.io/kube-linter/pkg/config"
+	"gopkg.in/yaml.v3"
+
+	"github.com/go-logr/logr"
 	apicorev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
+// this structure mirrors Kube-Linter configuration structure
+// it is used as a bridge to unmarshall ConfigMap data
+// doc: https://pkg.go.dev/golang.stackrox.io/kube-linter/pkg/config#Config
+type KubeLinterChecks struct {
+	Checks struct {
+		AddAllBuiltIn        bool     `yaml:"addAllBuiltIn,omitempty"`
+		DoNotAutoAddDefaults bool     `yaml:"doNotAutoAddDefaults,omitempty"`
+		Exclude              []string `yaml:"exclude,omitempty"`
+		Include              []string `yaml:"include,omitempty"`
+		IgnorePaths          []string `yaml:"ignorePaths,omitempty"`
+	} `yaml:"checks"`
+}
+
 type ConfigMapWatcher struct {
 	clientset kubernetes.Interface
-	ch        chan struct{} // TODO - TBD configmap struct
+	checks    KubeLinterChecks
+	ch        chan config.Config
+	logger    logr.Logger
 }
 
 var configMapName = "deployment-validation-operator-config"
 var configMapNamespace = "deployment-validation-operator"
+var configMapDataAccess = "deployment-validation-operator-config.yaml"
 
 // NewConfigMapWatcher returns a watcher that can be used both:
 // basic: with GetStaticDisabledChecks method, it returns an existent ConfigMap data's disabled check
@@ -33,19 +53,19 @@ func NewConfigMapWatcher(cfg *rest.Config) (ConfigMapWatcher, error) {
 
 	return ConfigMapWatcher{
 		clientset: clientset,
+		logger:    log.Log.WithName("ConfigMapWatcher"),
 	}, nil
 }
 
-// GetStaticDisabledChecks returns an existent ConfigMap data's disabled checks, if they exist
-func (cmw *ConfigMapWatcher) GetStaticDisabledChecks(ctx context.Context) ([]string, error) {
+// GetStaticKubelinterConfig returns the ConfigMap's checks configuration
+func (cmw *ConfigMapWatcher) GetStaticKubelinterConfig(ctx context.Context) (config.Config, error) {
 	cm, err := cmw.clientset.CoreV1().
 		ConfigMaps(configMapNamespace).Get(ctx, configMapName, v1.GetOptions{})
 	if err != nil {
-		return []string{}, fmt.Errorf("gathering starting configmap: %w", err)
+		return config.Config{}, fmt.Errorf("getting initial configuration: %w", err)
 	}
 
-	// TODO - Fix dummy return based on data being check1,check2,check3...
-	return strings.Split(cm.Data["disabled-checks"], ","), nil
+	return cmw.getKubeLinterConfig(cm.Data[configMapDataAccess])
 }
 
 // StartInformer will update the channel structure with new configuration data from ConfigMap update event
@@ -57,14 +77,17 @@ func (cmw *ConfigMapWatcher) StartInformer(ctx context.Context) error {
 
 	informer.AddEventHandler(cache.ResourceEventHandlerFuncs{ // nolint:errcheck
 		UpdateFunc: func(oldObj, newObj interface{}) {
-			oldCm := oldObj.(*apicorev1.ConfigMap)
 			newCm := newObj.(*apicorev1.ConfigMap)
 
-			fmt.Printf("oldCm: %v\n", oldCm)
-			fmt.Printf("ConfigMap updated: %s/%s\n", newCm.Namespace, newCm.Name)
+			cmw.logger.Info("ConfigMap has been updated")
 
-			// TODO - Validate new configmap
-			cmw.ch <- struct{}{}
+			cfg, err := cmw.getKubeLinterConfig(newCm.Data[configMapDataAccess])
+			if err != nil {
+				cmw.logger.Error(err, "ConfigMap data format")
+				return
+			}
+
+			cmw.ch <- cfg
 		},
 	})
 
@@ -74,6 +97,21 @@ func (cmw *ConfigMapWatcher) StartInformer(ctx context.Context) error {
 }
 
 // ConfigChanged receives push notifications when the configuration is updated
-func (cmw *ConfigMapWatcher) ConfigChanged() <-chan struct{} {
+func (cmw *ConfigMapWatcher) ConfigChanged() <-chan config.Config {
 	return cmw.ch
 }
+
+// getKubeLinterConfig returns a valid Kube-linter Config structure
+// based on the checks received by the string
+func (cmw *ConfigMapWatcher) getKubeLinterConfig(data string) (config.Config, error) {
+	var cfg config.Config
+
+	err := yaml.Unmarshal([]byte(data), &cmw.checks)
+	if err != nil {
+		return cfg, fmt.Errorf("unmarshalling configmap data: %w", err)
+	}
+
+	cfg.Checks = config.ChecksConfig(cmw.checks.Checks)
+
+	return cfg, nil
+}

pkg/controller/configmap_watcher_test.go

@@ -5,27 +5,67 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"golang.stackrox.io/kube-linter/pkg/config"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kubefake "k8s.io/client-go/kubernetes/fake"
 )
 
 func TestStaticConfigMapWatcher(t *testing.T) {
-	// Given
-	cm := &corev1.ConfigMap{
-		ObjectMeta: metav1.ObjectMeta{Namespace: configMapNamespace, Name: configMapName},
-		Data:       map[string]string{"disabled-checks": "check,check2"},
-	}
-	client := kubefake.NewSimpleClientset([]runtime.Object{cm}...)
-	mock := ConfigMapWatcher{
-		clientset: client,
+	testCases := []struct {
+		name   string
+		data   string
+		checks config.ChecksConfig
+	}{
+		{
+			name: "kube-linter 'doNotAutoAddDefaults' is gathered from configuration",
+			data: "checks:\n  doNotAutoAddDefaults: true",
+			checks: config.ChecksConfig{
+				DoNotAutoAddDefaults: true,
+			},
+		},
+		{
+			name: "kube-linter 'addAllBuiltIn' is gathered from configuration",
+			data: "checks:\n  addAllBuiltIn: true",
+			checks: config.ChecksConfig{
+				AddAllBuiltIn: true,
+			},
+		},
+		{
+			name: "kube-linter 'exclude' is gathered from configuration",
+			data: "checks:\n  exclude: [\"check1\", \"check2\"]",
+			checks: config.ChecksConfig{
+				Exclude: []string{"check1", "check2"},
+			},
+		},
+		{
+			name: "kube-linter 'include' is gathered from configuration",
+			data: "checks:\n  include: [\"check1\", \"check2\"]",
+			checks: config.ChecksConfig{
+				Include: []string{"check1", "check2"},
+			},
+		},
 	}
 
-	// When
-	test, err := mock.GetStaticDisabledChecks(context.Background())
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			// Given
+			cm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{Namespace: configMapNamespace, Name: configMapName},
+				Data: map[string]string{
+					"deployment-validation-operator-config.yaml": testCase.data,
+				},
+			}
+			client := kubefake.NewSimpleClientset([]runtime.Object{cm}...)
+			mock := ConfigMapWatcher{clientset: client}
+
+			// When
+			test, err := mock.GetStaticKubelinterConfig(context.Background())
 
-	// Assert
-	assert.NoError(t, err)
-	assert.Equal(t, []string{"check", "check2"}, test)
+			// Assert
+			assert.NoError(t, err)
+			assert.Equal(t, testCase.checks, test.Checks)
+		})
+	}
 }

pkg/validations/base_test.go

@@ -225,12 +225,11 @@ func TestIncompatibleChecksAreDisabled(t *testing.T) {
 	}
 
 	badChecks := getIncompatibleChecks()
-	disabledChecks := getDisabledChecks()
 	allKubeLinterChecks, err := getAllBuiltInKubeLinterChecks()
 	if err != nil {
 		t.Fatalf("Got unexpected error while getting all checks built-into kube-linter: %v", err)
 	}
-	expectedNumChecks := (len(allKubeLinterChecks) - len(badChecks)) - len(disabledChecks)
+	expectedNumChecks := (len(allKubeLinterChecks) - len(badChecks))
 
 	enabledChecks := engine.EnabledChecks()
 	if len(enabledChecks) != expectedNumChecks {

pkg/validations/validation_engine.go

@@ -52,13 +52,17 @@ func fileExists(filename string) bool {
 }
 
 func (ve *validationEngine) LoadConfig(path string) error {
-	v := viper.New()
-
 	if !fileExists(path) {
 		log.Info(fmt.Sprintf("config file %s does not exist. Use default configuration", path))
-		path = ""
+		// legacy disabled checks
+		ve.config.Checks.Exclude = getDisabledChecks()
+		ve.config.Checks.AddAllBuiltIn = true
+
+		return nil
 	}
 
+	v := viper.New()
+
 	// Load Configuration
 	config, err := config.Load(v, path)
 	if err != nil {
@@ -76,7 +80,6 @@ type PrometheusRegistry interface {
 
 func (ve *validationEngine) InitRegistry(promReg PrometheusRegistry) error {
 	disableIncompatibleChecks(&ve.config)
-	disableChecks(&ve.config)
 
 	registry := checkregistry.New()
 	if err := builtinchecks.LoadInto(registry); err != nil {
@@ -166,16 +169,19 @@ func InitializeValidationEngine(configPath string, reg PrometheusRegistry) error
 	ve := validationEngine{}
 
 	err := ve.LoadConfig(configPath)
-	if err == nil {
-		err = ve.InitRegistry(reg)
+	if err != nil {
+		return err
 	}
 
-	// Only replace the exisiting engine if no errors occurred
-	if err == nil {
-		engine = ve
+	err = ve.InitRegistry(reg)
+	if err != nil {
+		return err
 	}
 
-	return err
+	// Only replace the exisiting engine if no errors occurred
+	engine = ve
+
+	return nil
 }
 
 func (ve *validationEngine) GetCheckByName(name string) (config.Check, error) {
@@ -207,12 +213,6 @@ func getIncompatibleChecks() []string {
 	}
 }
 
-// disableChecks will forcibly update a kube-linter config
-// to disable checks that do not have supporting openshift documentation
-func disableChecks(c *config.Config) {
-	c.Checks.Exclude = append(c.Checks.Exclude, getDisabledChecks()...)
-}
-
 // getDisabledChecks returns an array of kube-linter check names that are disabled for DVO
 // These checks are disabled as they do not have supporting Openshift documentation
 // 38 checks... 47 checks according to kube-linter website
@@ -227,31 +227,20 @@ func getDisabledChecks() []string {
 		"drop-net-raw-capability",
 		"env-var-secret",
 		"exposed-services",
-		// "host-ipc",
-		// "host-network",
-		// "host-pid",
 		"latest-tag",
-		// "minimum-three-replicas",
 		"mismatching-selector",
-		// "no-anti-affinity",
 		"no-extensions-v1beta",
 		"no-liveness-probe",
 		"no-read-only-root-fs",
 		"no-readiness-probe",
 		"no-rolling-update-strategy",
-		// "privilege-escalation-container",
-		// "privileged-container",
 		"privileged-ports",
 		"read-secret-from-env-var",
 		"required-annotation-email",
 		"required-label-owner",
-		// "run-as-non-root",
 		"sensitive-host-mounts",
 		"ssh-port",
 		"unsafe-proc-mount",
-		// "unsafe-sysctls",
-		// "unset-cpu-requirements",
-		// "unset-memory-requirements",
 		"use-namespace",
 		"wildcard-in-rules",
 		"writable-host-mount",