DVO-146: Improve config YAML reading & some tests (#278)

Author: tremes

Diff for: pkg/configmap/configmap_watcher.go

@@ -8,35 +8,20 @@ import (
 	"time"
 
 	"golang.stackrox.io/kube-linter/pkg/config"
-	"gopkg.in/yaml.v3"
 
 	"github.com/app-sre/deployment-validation-operator/pkg/validations"
+	"github.com/ghodss/yaml"
 	"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 Watcher struct {
 	clientset kubernetes.Interface
-	checks    KubeLinterChecks
 	cfg       config.Config
 	ch        chan struct{}
 	logger    logr.Logger
@@ -74,17 +59,6 @@ func NewWatcher(cfg *rest.Config) (*Watcher, error) {
 	}, nil
 }
 
-// GetStaticKubelinterConfig returns the ConfigMap's checks configuration
-func (cmw *Watcher) GetStaticKubelinterConfig(ctx context.Context) (config.Config, error) {
-	cm, err := cmw.clientset.CoreV1().
-		ConfigMaps(cmw.namespace).Get(ctx, configMapName, v1.GetOptions{})
-	if err != nil {
-		return config.Config{}, fmt.Errorf("getting initial configuration: %w", err)
-	}
-
-	return cmw.getKubeLinterConfig(cm.Data[configMapDataAccess])
-}
-
 // Start will update the channel structure with new configuration data from ConfigMap update event
 func (cmw *Watcher) Start(ctx context.Context) error {
 	factory := informers.NewSharedInformerFactoryWithOptions(
@@ -106,7 +80,7 @@ func (cmw *Watcher) Start(ctx context.Context) error {
 				"namespace", newCm.GetNamespace(),
 			)
 
-			cfg, err := cmw.getKubeLinterConfig(newCm.Data[configMapDataAccess])
+			cfg, err := readConfig(newCm.Data[configMapDataAccess])
 			if err != nil {
 				cmw.logger.Error(err, "ConfigMap data format")
 				return
@@ -130,7 +104,7 @@ func (cmw *Watcher) Start(ctx context.Context) error {
 				"namespace", newCm.GetNamespace(),
 			)
 
-			cfg, err := cmw.getKubeLinterConfig(newCm.Data[configMapDataAccess])
+			cfg, err := readConfig(newCm.Data[configMapDataAccess])
 			if err != nil {
 				cmw.logger.Error(err, "ConfigMap data format")
 				return
@@ -172,18 +146,16 @@ func (cmw *Watcher) GetConfig() config.Config {
 	return cmw.cfg
 }
 
-// getKubeLinterConfig returns a valid Kube-linter Config structure
+// readConfig returns a valid Kube-linter Config structure
 // based on the checks received by the string
-func (cmw *Watcher) getKubeLinterConfig(data string) (config.Config, error) {
+func readConfig(data string) (config.Config, error) {
 	var cfg config.Config
 
-	err := yaml.Unmarshal([]byte(data), &cmw.checks)
+	err := yaml.Unmarshal([]byte(data), &cfg, yaml.DisallowUnknownFields)
 	if err != nil {
 		return cfg, fmt.Errorf("unmarshalling configmap data: %w", err)
 	}
 
-	cfg.Checks = config.ChecksConfig(cmw.checks.Checks)
-
 	return cfg, nil
 }
 

Diff for: pkg/configmap/configmap_watcher_test.go

@@ -1,73 +1,125 @@
 package configmap
 
 import (
-	"context"
+	"fmt"
 	"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) {
-	var configMapNamespace = "deployment-validation-operator"
-
-	testCases := []struct {
-		name   string
-		data   string
-		checks config.ChecksConfig
+func TestReadConfig(t *testing.T) {
+	tests := []struct {
+		name           string
+		configData     string
+		expectedConfig config.Config
+		expectedError  error
 	}{
 		{
-			name: "kube-linter 'doNotAutoAddDefaults' is gathered from configuration",
-			data: "checks:\n  doNotAutoAddDefaults: true",
-			checks: config.ChecksConfig{
-				DoNotAutoAddDefaults: true,
+			name: "Basic valid config",
+			configData: `
+checks:
+  doNotAutoAddDefaults: false
+  addAllBuiltIn: true
+  include:
+  - "unset-memory-requirements"
+  - "unset-cpu-requirements"`,
+			expectedConfig: config.Config{
+				Checks: config.ChecksConfig{
+					AddAllBuiltIn:        true,
+					DoNotAutoAddDefaults: false,
+					Include:              []string{"unset-memory-requirements", "unset-cpu-requirements"}, // nolint: lll
+				},
 			},
+			expectedError: nil,
 		},
 		{
-			name: "kube-linter 'addAllBuiltIn' is gathered from configuration",
-			data: "checks:\n  addAllBuiltIn: true",
-			checks: config.ChecksConfig{
-				AddAllBuiltIn: true,
+			name: "Invalid config field \"doNotAutoAddDefaultsAAA\"",
+			configData: `
+checks:
+  doNotAutoAddDefaultsAAA: false
+  addAllBuiltIn: true
+  include:
+  - "unset-memory-requirements"
+  - "unset-cpu-requirements"`,
+			expectedError: fmt.Errorf("unmarshalling configmap data: error unmarshaling JSON: while decoding JSON: json: unknown field \"doNotAutoAddDefaultsAAA\""), // nolint: lll
+			expectedConfig: config.Config{
+				Checks: config.ChecksConfig{
+					AddAllBuiltIn:        true,
+					DoNotAutoAddDefaults: false,
+					Include:              []string{"unset-memory-requirements", "unset-cpu-requirements"}, // nolint: lll
+				},
 			},
 		},
 		{
-			name: "kube-linter 'exclude' is gathered from configuration",
-			data: "checks:\n  exclude: [\"check1\", \"check2\"]",
-			checks: config.ChecksConfig{
-				Exclude: []string{"check1", "check2"},
+			name: "Invalid config field \"include\"",
+			configData: `
+checks:
+  doNotAutoAddDefaults: false
+  addAllBuiltIn: true
+  includeaa:
+  - "unset-memory-requirements"
+  - "unset-cpu-requirements"`,
+			expectedError: fmt.Errorf("unmarshalling configmap data: error unmarshaling JSON: while decoding JSON: json: unknown field \"includeaa\""), // nolint: lll
+			expectedConfig: config.Config{
+				Checks: config.ChecksConfig{
+					AddAllBuiltIn:        true,
+					DoNotAutoAddDefaults: false,
+				},
 			},
 		},
 		{
-			name: "kube-linter 'include' is gathered from configuration",
-			data: "checks:\n  include: [\"check1\", \"check2\"]",
-			checks: config.ChecksConfig{
-				Include: []string{"check1", "check2"},
+			name: "Valid config with custom check",
+			configData: `
+checks:
+  doNotAutoAddDefaults: false
+  addAllBuiltIn: true
+  include:
+  - "unset-memory-requirements"
+customChecks:
+  - name: test-minimum-replicas
+    description: "some description"
+    remediation: "some remediation"
+    template: minimum-replicas
+    params:
+      minReplicas: 3
+    scope:
+      objectKinds:
+        - DeploymentLike`,
+			expectedError: nil,
+			expectedConfig: config.Config{
+				Checks: config.ChecksConfig{
+					AddAllBuiltIn:        true,
+					DoNotAutoAddDefaults: false,
+					Include:              []string{"unset-memory-requirements"},
+				},
+				CustomChecks: []config.Check{
+					{
+						Name:        "test-minimum-replicas",
+						Description: "some description",
+						Remediation: "some remediation",
+						Template:    "minimum-replicas",
+						Params: map[string]interface{}{
+							"minReplicas": float64(3),
+						},
+						Scope: &config.ObjectKindsDesc{
+							ObjectKinds: []string{"DeploymentLike"},
+						},
+					},
+				},
 			},
 		},
 	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
 
-	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,
-				},
+			cfg, err := readConfig(tt.configData)
+			if tt.expectedError != nil {
+				assert.Equal(t, tt.expectedError.Error(), err.Error())
+			} else {
+				assert.NoError(t, err)
 			}
-			client := kubefake.NewSimpleClientset([]runtime.Object{cm}...)
-			mock := Watcher{clientset: client, namespace: configMapNamespace}
-
-			// When
-			test, err := mock.GetStaticKubelinterConfig(context.Background())
-
-			// Assert
-			assert.NoError(t, err)
-			assert.Equal(t, testCase.checks, test.Checks)
+			assert.Equal(t, tt.expectedConfig, cfg)
 		})
 	}
 }