From 1ffe54946cdf4c4650004d86a6204a4d56d57013 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Fri, 14 Jul 2023 18:31:31 +0200 Subject: [PATCH 01/41] Add filter for other ConfigMaps within the same namespace --- pkg/controller/configmap_watcher.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/controller/configmap_watcher.go b/pkg/controller/configmap_watcher.go index 31b574ab..568e9a4b 100644 --- a/pkg/controller/configmap_watcher.go +++ b/pkg/controller/configmap_watcher.go @@ -68,8 +68,8 @@ func (cmw *ConfigMapWatcher) GetStaticKubelinterConfig(ctx context.Context) (con return cmw.getKubeLinterConfig(cm.Data[configMapDataAccess]) } -// StartInformer will update the channel structure with new configuration data from ConfigMap update event -func (cmw *ConfigMapWatcher) StartInformer(ctx context.Context) error { +// Start will update the channel structure with new configuration data from ConfigMap update event +func (cmw ConfigMapWatcher) Start(ctx context.Context) error { factory := informers.NewSharedInformerFactoryWithOptions( cmw.clientset, time.Second*30, informers.WithNamespace(configMapNamespace), ) @@ -79,6 +79,10 @@ func (cmw *ConfigMapWatcher) StartInformer(ctx context.Context) error { UpdateFunc: func(oldObj, newObj interface{}) { newCm := newObj.(*apicorev1.ConfigMap) + if configMapName != newCm.ObjectMeta.Name { + return + } + cmw.logger.Info("ConfigMap has been updated") cfg, err := cmw.getKubeLinterConfig(newCm.Data[configMapDataAccess]) From 12ad75b6dbf2117983bfe0478eb7d428b73c56ab Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Fri, 14 Jul 2023 18:32:12 +0200 Subject: [PATCH 02/41] Add validationEngine and watcher to reconcilier --- pkg/controller/generic_reconciler.go | 29 ++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/pkg/controller/generic_reconciler.go b/pkg/controller/generic_reconciler.go index c0ea0748..1b512e18 100644 --- a/pkg/controller/generic_reconciler.go +++ b/pkg/controller/generic_reconciler.go @@ -8,6 +8,7 @@ import ( "sort" "strconv" + "golang.stackrox.io/kube-linter/pkg/checkregistry" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" @@ -28,6 +29,11 @@ var ( _ manager.Runnable = &GenericReconciler{} ) +type ValidationEngine interface { + CheckRegistry() checkregistry.CheckRegistry + RunForObjects(objects []client.Object, namespaceUID string) (validations.ValidationOutcome, error) +} + // GenericReconciler watches a defined object type GenericReconciler struct { listLimit int64 @@ -37,10 +43,13 @@ type GenericReconciler struct { client client.Client discovery discovery.DiscoveryInterface logger logr.Logger + validationEngine ValidationEngine + configWatcher *ConfigMapWatcher } // NewGenericReconciler returns a GenericReconciler struct -func NewGenericReconciler(client client.Client, discovery discovery.DiscoveryInterface) (*GenericReconciler, error) { +// func NewGenericReconciler(client client.Client, discovery discovery.DiscoveryInterface) (*GenericReconciler, error) { +func NewGenericReconciler(client client.Client, discovery discovery.DiscoveryInterface, ve ValidationEngine, cmw *ConfigMapWatcher) (*GenericReconciler, error) { listLimit, err := getListLimit() if err != nil { return nil, err @@ -54,6 +63,8 @@ func NewGenericReconciler(client client.Client, discovery discovery.DiscoveryInt objectValidationCache: newValidationCache(), currentObjects: newValidationCache(), logger: ctrl.Log.WithName("reconcile"), + validationEngine: ve, + configWatcher: cmw, }, nil } @@ -94,6 +105,8 @@ func (gr *GenericReconciler) AddToManager(mgr manager.Manager) error { // Start validating the given object kind every interval. func (gr *GenericReconciler) Start(ctx context.Context) error { + go gr.ConfigChanged(ctx) + for { select { case <-ctx.Done(): @@ -264,7 +277,8 @@ func (gr *GenericReconciler) reconcileGroupOfObjects(ctx context.Context, cliObjects = append(cliObjects, typedClientObject) } - outcome, err := validations.RunValidationsForObjects(cliObjects, namespaceUID) + // outcome, err := validations.RunValidationsForObjects(cliObjects, namespaceUID) + outcome, err := gr.validationEngine.RunForObjects(cliObjects, namespaceUID) if err != nil { return fmt.Errorf("running validations: %w", err) } @@ -346,3 +360,14 @@ func (gr GenericReconciler) getNamespacedResourcesGVK(resources []metav1.APIReso } return namespacedResources } + +func (gr *GenericReconciler) ConfigChanged(ctx context.Context) { + for { + select { + case cfg := <-gr.configWatcher.ConfigChanged(): + fmt.Println("//// GOT CFG ", cfg) + case <-ctx.Done(): + return + } + } +} From bfbed743da13b918986f3c6e58fcf4dc86604fdc Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Fri, 14 Jul 2023 18:32:49 +0200 Subject: [PATCH 03/41] Add initializer and accessible function from struct --- pkg/validations/validation_engine.go | 54 ++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index 14e91729..a68fa507 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -8,13 +8,17 @@ import ( "strings" // Import checks from DVO + "github.com/app-sre/deployment-validation-operator/pkg/utils" _ "github.com/app-sre/deployment-validation-operator/pkg/validations/all" + "sigs.k8s.io/controller-runtime/pkg/client" "golang.stackrox.io/kube-linter/pkg/builtinchecks" "golang.stackrox.io/kube-linter/pkg/checkregistry" "golang.stackrox.io/kube-linter/pkg/config" "golang.stackrox.io/kube-linter/pkg/configresolver" "golang.stackrox.io/kube-linter/pkg/diagnostic" + "golang.stackrox.io/kube-linter/pkg/lintcontext" + "golang.stackrox.io/kube-linter/pkg/run" // Import and initialize all check templates from kube-linter _ "golang.stackrox.io/kube-linter/pkg/templates/all" @@ -34,6 +38,22 @@ type validationEngine struct { metrics map[string]*prometheus.GaugeVec } +func NewEngine(configPath string, reg PrometheusRegistry) (*validationEngine, error) { + ve := &validationEngine{} + + err := ve.LoadConfig(configPath) + if err != nil { + return nil, err + } + + err = ve.InitRegistry(reg) + if err != nil { + return nil, err + } + + return ve, nil +} + func (ve *validationEngine) CheckRegistry() checkregistry.CheckRegistry { return ve.registry } @@ -42,6 +62,40 @@ func (ve *validationEngine) EnabledChecks() []string { return ve.enabledChecks } +func (ve *validationEngine) RunForObjects(objects []client.Object, namespaceUID string) (ValidationOutcome, error) { + lintCtx := &lintContextImpl{} + for _, obj := range objects { + // Only run checks against an object with no owners. This should be + // the object that controls the configuration + if !utils.IsOwner(obj) { + continue + } + // If controller has no replicas clear do not run any validations + if isControllersWithNoReplicas(obj) { + continue + } + lintCtx.addObjects(lintcontext.Object{K8sObject: obj}) + } + lintCtxs := []lintcontext.LintContext{lintCtx} + if len(lintCtxs) == 0 { + return ObjectValidationIgnored, nil + } + result, err := run.Run(lintCtxs, ve.CheckRegistry(), ve.EnabledChecks()) + if err != nil { + log.Error(err, "error running validations") + return "", fmt.Errorf("error running validations: %v", err) + } + + // Clear labels from past run to ensure only results from this run + // are reflected in the metrics + for _, o := range objects { + req := NewRequestFromObject(o) + req.NamespaceUID = namespaceUID + ve.ClearMetrics(result.Reports, req.ToPromLabels()) + } + return processResult(result, namespaceUID) +} + // Get info on config file if it exists func fileExists(filename string) bool { info, err := os.Stat(filename) From db7ff749b2c28f1287d6ee339890fb523687d2f7 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Fri, 14 Jul 2023 18:33:09 +0200 Subject: [PATCH 04/41] Fix Manager setup to account for validationEngine and watcher --- main.go | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/main.go b/main.go index dce83e60..c87a3b7e 100644 --- a/main.go +++ b/main.go @@ -120,15 +120,6 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("initializing discovery client: %w", err) } - gr, err := controller.NewGenericReconciler(mgr.GetClient(), discoveryClient) - if err != nil { - return nil, fmt.Errorf("initializing generic reconciler: %w", err) - } - - if err = gr.AddToManager(mgr); err != nil { - return nil, fmt.Errorf("adding generic reconciler to manager: %w", err) - } - log.Info("Initializing Prometheus Registry") reg := prometheus.NewRegistry() @@ -145,11 +136,35 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error } log.Info("Initializing Validation Engine") - - if err := validations.InitializeValidationEngine(opts.ConfigFile, reg); err != nil { + validationEngine, err := validations.NewEngine(opts.ConfigFile, reg) + if err != nil { return nil, fmt.Errorf("initializing validation engine: %w", err) } + cmWatcher, err := controller.NewConfigMapWatcher(cfg) + if err != nil { + return nil, fmt.Errorf("initializing configmap watcher: %w", err) + } + + if err := mgr.Add(cmWatcher); err != nil { + return nil, fmt.Errorf("adding configmap watcher to manager: %w", err) + } + + gr, err := controller.NewGenericReconciler(mgr.GetClient(), discoveryClient, validationEngine, &cmWatcher) + if err != nil { + return nil, fmt.Errorf("initializing generic reconciler: %w", err) + } + + if err = gr.AddToManager(mgr); err != nil { + return nil, fmt.Errorf("adding generic reconciler to manager: %w", err) + } + + // log.Info("Initializing Validation Engine") + // + // if err := validations.InitializeValidationEngine(opts.ConfigFile, reg); err != nil { + // return nil, fmt.Errorf("initializing validation engine: %w", err) + // } + return mgr, nil } From 36f192b3fc3aa8d0eda33909d01ee0203137c6a5 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Wed, 9 Aug 2023 16:42:30 +0200 Subject: [PATCH 05/41] Fix missing channel --- pkg/controller/configmap_watcher.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/controller/configmap_watcher.go b/pkg/controller/configmap_watcher.go index 568e9a4b..ac065487 100644 --- a/pkg/controller/configmap_watcher.go +++ b/pkg/controller/configmap_watcher.go @@ -54,6 +54,7 @@ func NewConfigMapWatcher(cfg *rest.Config) (ConfigMapWatcher, error) { return ConfigMapWatcher{ clientset: clientset, logger: log.Log.WithName("ConfigMapWatcher"), + ch: make(chan config.Config), }, nil } From 2d169d18fd38cd64590c53390fc5818d72e99c64 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Wed, 9 Aug 2023 16:54:25 +0200 Subject: [PATCH 06/41] Decouple Prometheus registry initialization --- main.go | 12 +++------ pkg/prometheus/prometheus.go | 48 ++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/main.go b/main.go index c87a3b7e..4c67e966 100644 --- a/main.go +++ b/main.go @@ -19,7 +19,6 @@ import ( dvo_prom "github.com/app-sre/deployment-validation-operator/pkg/prometheus" "github.com/app-sre/deployment-validation-operator/pkg/validations" "github.com/app-sre/deployment-validation-operator/version" - "github.com/prometheus/client_golang/prometheus" "github.com/go-logr/logr" osappsv1 "github.com/openshift/api/apps/v1" @@ -122,7 +121,10 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error log.Info("Initializing Prometheus Registry") - reg := prometheus.NewRegistry() + reg, err := dvo_prom.GetRegistry() + if err != nil { + return nil, fmt.Errorf("initializing Prometheus server: %w", err) + } log.Info(fmt.Sprintf("Initializing Prometheus metrics endpoint on %q", opts.MetricsEndpoint())) @@ -159,12 +161,6 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("adding generic reconciler to manager: %w", err) } - // log.Info("Initializing Validation Engine") - // - // if err := validations.InitializeValidationEngine(opts.ConfigFile, reg); err != nil { - // return nil, fmt.Errorf("initializing validation engine: %w", err) - // } - return mgr, nil } diff --git a/pkg/prometheus/prometheus.go b/pkg/prometheus/prometheus.go index 45932251..61555a13 100644 --- a/pkg/prometheus/prometheus.go +++ b/pkg/prometheus/prometheus.go @@ -7,10 +7,12 @@ import ( "strings" "time" + "github.com/app-sre/deployment-validation-operator/pkg/validations" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/collectors" "github.com/prometheus/client_golang/prometheus/promhttp" dto "github.com/prometheus/client_model/go" + "golang.stackrox.io/kube-linter/pkg/checkregistry" ) type Registry interface { @@ -80,6 +82,52 @@ func getRouter(registry Registry, path string) (*http.ServeMux, error) { return mux, nil } +// GetRegistry returns a fully configured Prometheus registry with metrics based on Kube Linter validations +func GetRegistry() (*prometheus.Registry, error) { + prom := prometheus.NewRegistry() + + reg, err := validations.GetKubeLinterRegistry() + if err != nil { + return nil, err + } + + checks, err := validations.GetAllNamesFromRegistry(reg) + if err != nil { + return nil, err + } + + for _, checkName := range checks { + metric, err := setupMetric(reg, checkName) + if err != nil { + return nil, fmt.Errorf("unable to create metric for check %s", checkName) + } + + if err := prom.Register(metric); err != nil { + return nil, fmt.Errorf("registering metric for check %q: %w", checkName, err) + } + } + + return prom, nil +} + +// setupMetric uses registered validations to return the correct metric for a Prometheus registry +func setupMetric(reg checkregistry.CheckRegistry, name string) (*prometheus.GaugeVec, error) { + check := reg.Load(name) + if check == nil { + return nil, fmt.Errorf("unable to create metric for check %s", name) + } + + return prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Name: strings.ReplaceAll(check.Spec.Name, "-", "_"), + Help: fmt.Sprintf("Description: %s ; Remediation: %s", check.Spec.Description, check.Spec.Remediation), + ConstLabels: prometheus.Labels{ + "check_description": check.Spec.Description, + "check_remediation": check.Spec.Remediation, + }, + }, []string{"namespace_uid", "namespace", "uid", "name", "kind"}), nil +} + type Server struct { s *http.Server } From 5496b73e523715da113354c3c1c4c3b473e95264 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Wed, 9 Aug 2023 16:54:43 +0200 Subject: [PATCH 07/41] Fix missing interface change --- pkg/controller/generic_reconciler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/generic_reconciler_test.go b/pkg/controller/generic_reconciler_test.go index dfc5fa35..0a02cf84 100644 --- a/pkg/controller/generic_reconciler_test.go +++ b/pkg/controller/generic_reconciler_test.go @@ -966,5 +966,5 @@ func createTestReconciler(scheme *runtime.Scheme, objects []client.Object) (*Gen } client := cliBuilder.Build() cli := kubefake.NewSimpleClientset() - return NewGenericReconciler(client, cli.Discovery()) + return NewGenericReconciler(client, cli.Discovery(), nil, nil) } From 878979fd26cc8597bb410cead4db7423ef8ba630 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Wed, 9 Aug 2023 16:55:20 +0200 Subject: [PATCH 08/41] Add new utils functions for Prometheus registry decoupling --- pkg/validations/utils.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/pkg/validations/utils.go b/pkg/validations/utils.go index 823aeeea..846ac631 100644 --- a/pkg/validations/utils.go +++ b/pkg/validations/utils.go @@ -5,6 +5,10 @@ import ( "strings" "github.com/app-sre/deployment-validation-operator/config" + "golang.stackrox.io/kube-linter/pkg/builtinchecks" + "golang.stackrox.io/kube-linter/pkg/checkregistry" + klConfig "golang.stackrox.io/kube-linter/pkg/config" + "golang.stackrox.io/kube-linter/pkg/configresolver" "github.com/prometheus/client_golang/prometheus" ) @@ -25,3 +29,34 @@ func newGaugeVecMetric( labelNames, ) } + +// GetKubeLinterRegistry sets up a Kube Linter registry with builtin default validations +func GetKubeLinterRegistry() (checkregistry.CheckRegistry, error) { + registry := checkregistry.New() + if err := builtinchecks.LoadInto(registry); err != nil { + log.Error(err, "failed to load kube-linter built-in validations") + return nil, err + } + + return registry, nil +} + +// GetAllNamesFromRegistry returns a slice with the names of all valid Kube Linter checks. +// Since any check can be configured ad hoc, we return all valid checks. +func GetAllNamesFromRegistry(reg checkregistry.CheckRegistry) ([]string, error) { + // Get all checks except for incompatible ones + cfg := klConfig.Config{ + Checks: klConfig.ChecksConfig{ + AddAllBuiltIn: true, + }, + } + disableIncompatibleChecks(&cfg) + + checks, err := configresolver.GetEnabledChecksAndValidate(&cfg, reg) + if err != nil { + log.Error(err, "error getting enabled validations") + return nil, err + } + + return checks, nil +} From 833c15afbcd7c7328921e5f682fea407c7e803d8 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Wed, 9 Aug 2023 18:43:35 +0200 Subject: [PATCH 09/41] Fix global variable missleading --- pkg/controller/generic_reconciler.go | 11 ++++-- pkg/validations/validation_engine.go | 59 +++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/pkg/controller/generic_reconciler.go b/pkg/controller/generic_reconciler.go index 1b512e18..051ea95a 100644 --- a/pkg/controller/generic_reconciler.go +++ b/pkg/controller/generic_reconciler.go @@ -8,7 +8,7 @@ import ( "sort" "strconv" - "golang.stackrox.io/kube-linter/pkg/checkregistry" + "golang.stackrox.io/kube-linter/pkg/config" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" @@ -30,7 +30,7 @@ var ( ) type ValidationEngine interface { - CheckRegistry() checkregistry.CheckRegistry + UpdateRegistry(newconfig config.Config) error RunForObjects(objects []client.Object, namespaceUID string) (validations.ValidationOutcome, error) } @@ -365,7 +365,12 @@ func (gr *GenericReconciler) ConfigChanged(ctx context.Context) { for { select { case cfg := <-gr.configWatcher.ConfigChanged(): - fmt.Println("//// GOT CFG ", cfg) + err := gr.validationEngine.UpdateRegistry(cfg) + if err != nil { + fmt.Printf("error updating configuration from ConfigMap: %v\n", cfg) + return + } + case <-ctx.Done(): return } diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index a68fa507..33f77f36 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -46,11 +46,16 @@ func NewEngine(configPath string, reg PrometheusRegistry) (*validationEngine, er return nil, err } + // TODO - Remove Prometheus registry err = ve.InitRegistry(reg) if err != nil { return nil, err } + // keeping the validation engine glogal scoped for compatibility + // it would be a good idea to remove the global variable use in the long run + engine = *ve + return ve, nil } @@ -184,9 +189,55 @@ func (ve *validationEngine) InitRegistry(promReg PrometheusRegistry) error { }, ) - if err := promReg.Register(metric); err != nil { - return fmt.Errorf("registering metric for check %q: %w", check.Spec.Name, err) + validationMetrics[checkName] = metric + } + + ve.registry = registry + ve.enabledChecks = enabledChecks + ve.metrics = validationMetrics + ve.registeredChecks = registeredChecks + + return nil +} + +func (ve *validationEngine) UpdateRegistry(newconfig config.Config) error { + ve.config = newconfig + + registry := checkregistry.New() + if err := builtinchecks.LoadInto(registry); err != nil { + log.Error(err, "failed to load built-in validations") + return err + } + + if err := configresolver.LoadCustomChecksInto(&ve.config, registry); err != nil { + log.Error(err, "failed to load custom checks") + return err + } + + enabledChecks, err := configresolver.GetEnabledChecksAndValidate(&ve.config, registry) + if err != nil { + log.Error(err, "error finding enabled validations") + return err + } + + validationMetrics := map[string]*prometheus.GaugeVec{} + registeredChecks := map[string]config.Check{} + for _, checkName := range enabledChecks { + check := registry.Load(checkName) + if check == nil { + return fmt.Errorf("unable to create metric for check %s", checkName) } + registeredChecks[check.Spec.Name] = check.Spec + metric := newGaugeVecMetric( + strings.ReplaceAll(check.Spec.Name, "-", "_"), + fmt.Sprintf("Description: %s ; Remediation: %s", + check.Spec.Description, check.Spec.Remediation), + []string{"namespace_uid", "namespace", "uid", "name", "kind"}, + prometheus.Labels{ + "check_description": check.Spec.Description, + "check_remediation": check.Spec.Remediation, + }, + ) validationMetrics[checkName] = metric } @@ -196,6 +247,10 @@ func (ve *validationEngine) InitRegistry(promReg PrometheusRegistry) error { ve.metrics = validationMetrics ve.registeredChecks = registeredChecks + // keeping the validation engine glogal scoped for compatibility + // it would be a good idea to remove the global variable use in the long run + engine = *ve + return nil } From e38100b2447602b61d1c9bf6430454842085fa04 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Fri, 11 Aug 2023 18:33:14 +0200 Subject: [PATCH 10/41] Refactor validation engine package WIP --- pkg/validations/base_test.go | 2 +- pkg/validations/validation_engine.go | 96 ++++------------------------ 2 files changed, 13 insertions(+), 85 deletions(-) diff --git a/pkg/validations/base_test.go b/pkg/validations/base_test.go index b79dfdf5..c70d4524 100644 --- a/pkg/validations/base_test.go +++ b/pkg/validations/base_test.go @@ -32,7 +32,7 @@ func newEngine(c config.Config) (validationEngine, error) { ve := validationEngine{ config: c, } - loadErr := ve.InitRegistry(prometheus.NewRegistry()) + loadErr := ve.InitRegistry() if loadErr != nil { return validationEngine{}, loadErr } diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index 33f77f36..1a495d56 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -12,7 +12,6 @@ import ( _ "github.com/app-sre/deployment-validation-operator/pkg/validations/all" "sigs.k8s.io/controller-runtime/pkg/client" - "golang.stackrox.io/kube-linter/pkg/builtinchecks" "golang.stackrox.io/kube-linter/pkg/checkregistry" "golang.stackrox.io/kube-linter/pkg/config" "golang.stackrox.io/kube-linter/pkg/configresolver" @@ -46,16 +45,11 @@ func NewEngine(configPath string, reg PrometheusRegistry) (*validationEngine, er return nil, err } - // TODO - Remove Prometheus registry - err = ve.InitRegistry(reg) + err = ve.InitRegistry() if err != nil { return nil, err } - // keeping the validation engine glogal scoped for compatibility - // it would be a good idea to remove the global variable use in the long run - engine = *ve - return ve, nil } @@ -141,6 +135,8 @@ func (ve *validationEngine) LoadConfig(path string) error { log.Error(err, "failed to load config") return err } + + disableIncompatibleChecks(&config) ve.config = config return nil @@ -150,62 +146,9 @@ type PrometheusRegistry interface { Register(prometheus.Collector) error } -func (ve *validationEngine) InitRegistry(promReg PrometheusRegistry) error { - disableIncompatibleChecks(&ve.config) - - registry := checkregistry.New() - if err := builtinchecks.LoadInto(registry); err != nil { - log.Error(err, "failed to load built-in validations") - return err - } - - if err := configresolver.LoadCustomChecksInto(&ve.config, registry); err != nil { - log.Error(err, "failed to load custom checks") - return err - } - - enabledChecks, err := configresolver.GetEnabledChecksAndValidate(&ve.config, registry) +func (ve *validationEngine) InitRegistry() error { + registry, err := GetKubeLinterRegistry() if err != nil { - log.Error(err, "error finding enabled validations") - return err - } - - validationMetrics := map[string]*prometheus.GaugeVec{} - registeredChecks := map[string]config.Check{} - for _, checkName := range enabledChecks { - check := registry.Load(checkName) - if check == nil { - return fmt.Errorf("unable to create metric for check %s", checkName) - } - registeredChecks[check.Spec.Name] = check.Spec - metric := newGaugeVecMetric( - strings.ReplaceAll(check.Spec.Name, "-", "_"), - fmt.Sprintf("Description: %s ; Remediation: %s", - check.Spec.Description, check.Spec.Remediation), - []string{"namespace_uid", "namespace", "uid", "name", "kind"}, - prometheus.Labels{ - "check_description": check.Spec.Description, - "check_remediation": check.Spec.Remediation, - }, - ) - - validationMetrics[checkName] = metric - } - - ve.registry = registry - ve.enabledChecks = enabledChecks - ve.metrics = validationMetrics - ve.registeredChecks = registeredChecks - - return nil -} - -func (ve *validationEngine) UpdateRegistry(newconfig config.Config) error { - ve.config = newconfig - - registry := checkregistry.New() - if err := builtinchecks.LoadInto(registry); err != nil { - log.Error(err, "failed to load built-in validations") return err } @@ -220,6 +163,7 @@ func (ve *validationEngine) UpdateRegistry(newconfig config.Config) error { return err } + // TODO - Use same approach than in prometheus package validationMetrics := map[string]*prometheus.GaugeVec{} registeredChecks := map[string]config.Check{} for _, checkName := range enabledChecks { @@ -284,28 +228,6 @@ func (ve *validationEngine) ClearMetrics(reports []diagnostic.WithContext, label } } -// InitializeValidationEngine will initialize the validation engine from scratch. -// If an existing engine exists, it will not be replaced with the new one unless all -// initialization steps succeed. -func InitializeValidationEngine(configPath string, reg PrometheusRegistry) error { - ve := validationEngine{} - - err := ve.LoadConfig(configPath) - if err != nil { - return err - } - - err = ve.InitRegistry(reg) - if err != nil { - return err - } - - // Only replace the exisiting engine if no errors occurred - engine = ve - - return nil -} - func (ve *validationEngine) GetCheckByName(name string) (config.Check, error) { check, ok := ve.registeredChecks[name] if !ok { @@ -314,6 +236,12 @@ func (ve *validationEngine) GetCheckByName(name string) (config.Check, error) { return check, nil } +// UpdateConfig TODO - doc +func (ve *validationEngine) UpdateConfig(newconfig config.Config) { + disableIncompatibleChecks(&newconfig) + ve.config = newconfig +} + // disableIncompatibleChecks will forcibly update a kube-linter config // to disable checks that are incompatible with DVO. // the same check name may end up in the exclude list multiple times as a result of this; this is OK. From 7c572b7b77c7294abff59d7d257c8ca6b4946cc3 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Fri, 11 Aug 2023 18:33:34 +0200 Subject: [PATCH 11/41] Refactor re-use existing methods on VE --- pkg/controller/generic_reconciler.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/controller/generic_reconciler.go b/pkg/controller/generic_reconciler.go index 051ea95a..adbf366d 100644 --- a/pkg/controller/generic_reconciler.go +++ b/pkg/controller/generic_reconciler.go @@ -30,7 +30,8 @@ var ( ) type ValidationEngine interface { - UpdateRegistry(newconfig config.Config) error + InitRegistry() error + UpdateConfig(newconfig config.Config) RunForObjects(objects []client.Object, namespaceUID string) (validations.ValidationOutcome, error) } @@ -365,7 +366,8 @@ func (gr *GenericReconciler) ConfigChanged(ctx context.Context) { for { select { case cfg := <-gr.configWatcher.ConfigChanged(): - err := gr.validationEngine.UpdateRegistry(cfg) + gr.validationEngine.UpdateConfig(cfg) + err := gr.validationEngine.InitRegistry() if err != nil { fmt.Printf("error updating configuration from ConfigMap: %v\n", cfg) return From c94677ca19e07eb6fab57867cb169277b9f6b389 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Mon, 14 Aug 2023 18:21:39 +0200 Subject: [PATCH 12/41] Refactor minor changes on initializacion --- main.go | 4 ++-- pkg/prometheus/prometheus.go | 4 ++-- pkg/validations/validation_engine.go | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/main.go b/main.go index 4c67e966..ad5d4a63 100644 --- a/main.go +++ b/main.go @@ -121,7 +121,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error log.Info("Initializing Prometheus Registry") - reg, err := dvo_prom.GetRegistry() + reg, err := dvo_prom.SetupRegistry() if err != nil { return nil, fmt.Errorf("initializing Prometheus server: %w", err) } @@ -138,7 +138,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error } log.Info("Initializing Validation Engine") - validationEngine, err := validations.NewEngine(opts.ConfigFile, reg) + validationEngine, err := validations.NewEngine(opts.ConfigFile) if err != nil { return nil, fmt.Errorf("initializing validation engine: %w", err) } diff --git a/pkg/prometheus/prometheus.go b/pkg/prometheus/prometheus.go index 61555a13..3d04186e 100644 --- a/pkg/prometheus/prometheus.go +++ b/pkg/prometheus/prometheus.go @@ -82,8 +82,8 @@ func getRouter(registry Registry, path string) (*http.ServeMux, error) { return mux, nil } -// GetRegistry returns a fully configured Prometheus registry with metrics based on Kube Linter validations -func GetRegistry() (*prometheus.Registry, error) { +// SetupRegistry returns a fully configured Prometheus registry with metrics based on Kube Linter validations +func SetupRegistry() (*prometheus.Registry, error) { prom := prometheus.NewRegistry() reg, err := validations.GetKubeLinterRegistry() diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index 1a495d56..da3421bf 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -37,7 +37,7 @@ type validationEngine struct { metrics map[string]*prometheus.GaugeVec } -func NewEngine(configPath string, reg PrometheusRegistry) (*validationEngine, error) { +func NewEngine(configPath string) (*validationEngine, error) { ve := &validationEngine{} err := ve.LoadConfig(configPath) @@ -136,7 +136,6 @@ func (ve *validationEngine) LoadConfig(path string) error { return err } - disableIncompatibleChecks(&config) ve.config = config return nil @@ -147,6 +146,8 @@ type PrometheusRegistry interface { } func (ve *validationEngine) InitRegistry() error { + disableIncompatibleChecks(&ve.config) + registry, err := GetKubeLinterRegistry() if err != nil { return err @@ -238,7 +239,6 @@ func (ve *validationEngine) GetCheckByName(name string) (config.Check, error) { // UpdateConfig TODO - doc func (ve *validationEngine) UpdateConfig(newconfig config.Config) { - disableIncompatibleChecks(&newconfig) ve.config = newconfig } From 9642fa1dde221b5c33fc973ad3029489cc216eb9 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 17 Aug 2023 14:03:51 +0200 Subject: [PATCH 13/41] WIP Move configmap watcher to its own package --- pkg/{controller => configmap}/configmap_watcher.go | 2 +- pkg/{controller => configmap}/configmap_watcher_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename pkg/{controller => configmap}/configmap_watcher.go (99%) rename pkg/{controller => configmap}/configmap_watcher_test.go (98%) diff --git a/pkg/controller/configmap_watcher.go b/pkg/configmap/configmap_watcher.go similarity index 99% rename from pkg/controller/configmap_watcher.go rename to pkg/configmap/configmap_watcher.go index ac065487..37a18fce 100644 --- a/pkg/controller/configmap_watcher.go +++ b/pkg/configmap/configmap_watcher.go @@ -1,4 +1,4 @@ -package controller +package configmap import ( "context" diff --git a/pkg/controller/configmap_watcher_test.go b/pkg/configmap/configmap_watcher_test.go similarity index 98% rename from pkg/controller/configmap_watcher_test.go rename to pkg/configmap/configmap_watcher_test.go index 22ded7b2..fc500271 100644 --- a/pkg/controller/configmap_watcher_test.go +++ b/pkg/configmap/configmap_watcher_test.go @@ -1,4 +1,4 @@ -package controller +package configmap import ( "context" From 706daeda71c6475b010169958dc7b9536b7ea2e4 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 17 Aug 2023 17:13:20 +0200 Subject: [PATCH 14/41] Refactor main script to improve readability --- main.go | 90 +++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 55 insertions(+), 35 deletions(-) diff --git a/main.go b/main.go index ad5d4a63..ce319fa2 100644 --- a/main.go +++ b/main.go @@ -15,6 +15,7 @@ import ( apis "github.com/app-sre/deployment-validation-operator/api" dvconfig "github.com/app-sre/deployment-validation-operator/config" "github.com/app-sre/deployment-validation-operator/internal/options" + "github.com/app-sre/deployment-validation-operator/pkg/configmap" "github.com/app-sre/deployment-validation-operator/pkg/controller" dvo_prom "github.com/app-sre/deployment-validation-operator/pkg/prometheus" "github.com/app-sre/deployment-validation-operator/pkg/validations" @@ -85,48 +86,23 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("getting config: %w", err) } - log.Info("Initialize Scheme") - - scheme, err := initializeScheme() - if err != nil { - return nil, fmt.Errorf("initializing scheme: %w", err) - } - log.Info("Initialize Manager") - mgrOpts, err := getManagerOptions(scheme, opts) - if err != nil { - return nil, fmt.Errorf("getting manager options: %w", err) - } - - mgr, err := manager.New(cfg, mgrOpts) + mgr, err := initManager(log, opts, cfg) if err != nil { return nil, fmt.Errorf("initializing manager: %w", err) } - if err := mgr.AddHealthzCheck("health", healthz.Ping); err != nil { - return nil, fmt.Errorf("adding healthz check: %w", err) - } - - if err := mgr.AddReadyzCheck("check", healthz.Ping); err != nil { - return nil, fmt.Errorf("adding readyz check: %w", err) - } - log.Info("Registering Components") - discoveryClient, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig()) - if err != nil { - return nil, fmt.Errorf("initializing discovery client: %w", err) - } - - log.Info("Initializing Prometheus Registry") + log.Info("-> Initialize Prometheus Registry") reg, err := dvo_prom.SetupRegistry() if err != nil { return nil, fmt.Errorf("initializing Prometheus server: %w", err) } - log.Info(fmt.Sprintf("Initializing Prometheus metrics endpoint on %q", opts.MetricsEndpoint())) + log.Info(fmt.Sprintf("-> Initialize Prometheus metrics endpoint on %q", opts.MetricsEndpoint())) srv, err := dvo_prom.NewServer(reg, opts.MetricsPath, fmt.Sprintf(":%d", opts.MetricsPort)) if err != nil { @@ -137,13 +113,9 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("adding metrics server to manager: %w", err) } - log.Info("Initializing Validation Engine") - validationEngine, err := validations.NewEngine(opts.ConfigFile) - if err != nil { - return nil, fmt.Errorf("initializing validation engine: %w", err) - } + log.Info("-> Initialize ConfigMap watcher") - cmWatcher, err := controller.NewConfigMapWatcher(cfg) + cmWatcher, err := configmap.NewConfigMapWatcher(cfg) if err != nil { return nil, fmt.Errorf("initializing configmap watcher: %w", err) } @@ -152,7 +124,25 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("adding configmap watcher to manager: %w", err) } - gr, err := controller.NewGenericReconciler(mgr.GetClient(), discoveryClient, validationEngine, &cmWatcher) + log.Info("-> Initialize Validation Engine") + + validationEngine, err := validations.NewEngine(opts.ConfigFile, cmWatcher) + if err != nil { + return nil, fmt.Errorf("initializing validation engine: %w", err) + } + + if err := mgr.Add(validationEngine); err != nil { + return nil, fmt.Errorf("adding validationEngine to manager: %w", err) + } + + log.Info("-> Initialize Reconciler") + + discoveryClient, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig()) + if err != nil { + return nil, fmt.Errorf("initializing discovery client: %w", err) + } + + gr, err := controller.NewGenericReconciler(mgr.GetClient(), discoveryClient) // TODO - deprecated validationengine param if err != nil { return nil, fmt.Errorf("initializing generic reconciler: %w", err) } @@ -246,3 +236,33 @@ func kubeClientQPS() (float32, error) { qps = float32(val) return qps, err } + +func initManager(log logr.Logger, opts options.Options, cfg *rest.Config) (manager.Manager, error) { + log.Info("-> Initialize Scheme") + scheme, err := initializeScheme() + if err != nil { + return nil, fmt.Errorf("initializing scheme: %w", err) + } + + log.Info("-> Getting Manager Options") + mgrOpts, err := getManagerOptions(scheme, opts) + if err != nil { + return nil, fmt.Errorf("getting manager options: %w", err) + } + + mgr, err := manager.New(cfg, mgrOpts) + if err != nil { + return nil, fmt.Errorf("getting new manager: %w", err) + } + + log.Info("-> Adding Healthz and Readyz checks") + if err := mgr.AddHealthzCheck("health", healthz.Ping); err != nil { + return nil, fmt.Errorf("adding healthz check: %w", err) + } + + if err := mgr.AddReadyzCheck("check", healthz.Ping); err != nil { + return nil, fmt.Errorf("adding readyz check: %w", err) + } + + return mgr, nil +} From ed1d8b8f036d1d5f2de107d33b4e03df2b1aad9f Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 17 Aug 2023 17:14:28 +0200 Subject: [PATCH 15/41] Fix redundant system trigger of the informer --- pkg/configmap/configmap_watcher.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/configmap/configmap_watcher.go b/pkg/configmap/configmap_watcher.go index 37a18fce..6434b452 100644 --- a/pkg/configmap/configmap_watcher.go +++ b/pkg/configmap/configmap_watcher.go @@ -3,6 +3,7 @@ package configmap import ( "context" "fmt" + "reflect" "time" "golang.stackrox.io/kube-linter/pkg/config" @@ -80,7 +81,8 @@ func (cmw ConfigMapWatcher) Start(ctx context.Context) error { UpdateFunc: func(oldObj, newObj interface{}) { newCm := newObj.(*apicorev1.ConfigMap) - if configMapName != newCm.ObjectMeta.Name { + // This is sometimes triggered even if no change was due to the ConfigMap + if configMapName != newCm.ObjectMeta.Name || reflect.DeepEqual(oldObj, newObj) { return } From aef2a12489c0897f0c45ce366b05ea20bc1662a0 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 17 Aug 2023 17:16:28 +0200 Subject: [PATCH 16/41] Rollback VE and CMW dependencies and interface --- pkg/controller/generic_reconciler.go | 36 ++--------------------- pkg/controller/generic_reconciler_test.go | 2 +- 2 files changed, 3 insertions(+), 35 deletions(-) diff --git a/pkg/controller/generic_reconciler.go b/pkg/controller/generic_reconciler.go index adbf366d..c0ea0748 100644 --- a/pkg/controller/generic_reconciler.go +++ b/pkg/controller/generic_reconciler.go @@ -8,7 +8,6 @@ import ( "sort" "strconv" - "golang.stackrox.io/kube-linter/pkg/config" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" @@ -29,12 +28,6 @@ var ( _ manager.Runnable = &GenericReconciler{} ) -type ValidationEngine interface { - InitRegistry() error - UpdateConfig(newconfig config.Config) - RunForObjects(objects []client.Object, namespaceUID string) (validations.ValidationOutcome, error) -} - // GenericReconciler watches a defined object type GenericReconciler struct { listLimit int64 @@ -44,13 +37,10 @@ type GenericReconciler struct { client client.Client discovery discovery.DiscoveryInterface logger logr.Logger - validationEngine ValidationEngine - configWatcher *ConfigMapWatcher } // NewGenericReconciler returns a GenericReconciler struct -// func NewGenericReconciler(client client.Client, discovery discovery.DiscoveryInterface) (*GenericReconciler, error) { -func NewGenericReconciler(client client.Client, discovery discovery.DiscoveryInterface, ve ValidationEngine, cmw *ConfigMapWatcher) (*GenericReconciler, error) { +func NewGenericReconciler(client client.Client, discovery discovery.DiscoveryInterface) (*GenericReconciler, error) { listLimit, err := getListLimit() if err != nil { return nil, err @@ -64,8 +54,6 @@ func NewGenericReconciler(client client.Client, discovery discovery.DiscoveryInt objectValidationCache: newValidationCache(), currentObjects: newValidationCache(), logger: ctrl.Log.WithName("reconcile"), - validationEngine: ve, - configWatcher: cmw, }, nil } @@ -106,8 +94,6 @@ func (gr *GenericReconciler) AddToManager(mgr manager.Manager) error { // Start validating the given object kind every interval. func (gr *GenericReconciler) Start(ctx context.Context) error { - go gr.ConfigChanged(ctx) - for { select { case <-ctx.Done(): @@ -278,8 +264,7 @@ func (gr *GenericReconciler) reconcileGroupOfObjects(ctx context.Context, cliObjects = append(cliObjects, typedClientObject) } - // outcome, err := validations.RunValidationsForObjects(cliObjects, namespaceUID) - outcome, err := gr.validationEngine.RunForObjects(cliObjects, namespaceUID) + outcome, err := validations.RunValidationsForObjects(cliObjects, namespaceUID) if err != nil { return fmt.Errorf("running validations: %w", err) } @@ -361,20 +346,3 @@ func (gr GenericReconciler) getNamespacedResourcesGVK(resources []metav1.APIReso } return namespacedResources } - -func (gr *GenericReconciler) ConfigChanged(ctx context.Context) { - for { - select { - case cfg := <-gr.configWatcher.ConfigChanged(): - gr.validationEngine.UpdateConfig(cfg) - err := gr.validationEngine.InitRegistry() - if err != nil { - fmt.Printf("error updating configuration from ConfigMap: %v\n", cfg) - return - } - - case <-ctx.Done(): - return - } - } -} diff --git a/pkg/controller/generic_reconciler_test.go b/pkg/controller/generic_reconciler_test.go index 0a02cf84..dfc5fa35 100644 --- a/pkg/controller/generic_reconciler_test.go +++ b/pkg/controller/generic_reconciler_test.go @@ -966,5 +966,5 @@ func createTestReconciler(scheme *runtime.Scheme, objects []client.Object) (*Gen } client := cliBuilder.Build() cli := kubefake.NewSimpleClientset() - return NewGenericReconciler(client, cli.Discovery(), nil, nil) + return NewGenericReconciler(client, cli.Discovery()) } From 08cc8880f162b0b6a5a79a97761cb8022e822b01 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 17 Aug 2023 17:17:54 +0200 Subject: [PATCH 17/41] Move CMW control to VE package --- pkg/validations/validation_engine.go | 65 ++++++++++------------------ 1 file changed, 22 insertions(+), 43 deletions(-) diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index da3421bf..c52f836a 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -2,22 +2,20 @@ package validations import ( // Used to embed yamls by kube-linter + "context" _ "embed" "fmt" "os" "strings" // Import checks from DVO - "github.com/app-sre/deployment-validation-operator/pkg/utils" + "github.com/app-sre/deployment-validation-operator/pkg/configmap" _ "github.com/app-sre/deployment-validation-operator/pkg/validations/all" - "sigs.k8s.io/controller-runtime/pkg/client" "golang.stackrox.io/kube-linter/pkg/checkregistry" "golang.stackrox.io/kube-linter/pkg/config" "golang.stackrox.io/kube-linter/pkg/configresolver" "golang.stackrox.io/kube-linter/pkg/diagnostic" - "golang.stackrox.io/kube-linter/pkg/lintcontext" - "golang.stackrox.io/kube-linter/pkg/run" // Import and initialize all check templates from kube-linter _ "golang.stackrox.io/kube-linter/pkg/templates/all" @@ -35,10 +33,13 @@ type validationEngine struct { enabledChecks []string registeredChecks map[string]config.Check metrics map[string]*prometheus.GaugeVec + cmWatcher *configmap.ConfigMapWatcher } -func NewEngine(configPath string) (*validationEngine, error) { - ve := &validationEngine{} +func NewEngine(configPath string, cmw configmap.ConfigMapWatcher) (*validationEngine, error) { + ve := &validationEngine{ + cmWatcher: &cmw, + } err := ve.LoadConfig(configPath) if err != nil { @@ -61,40 +62,6 @@ func (ve *validationEngine) EnabledChecks() []string { return ve.enabledChecks } -func (ve *validationEngine) RunForObjects(objects []client.Object, namespaceUID string) (ValidationOutcome, error) { - lintCtx := &lintContextImpl{} - for _, obj := range objects { - // Only run checks against an object with no owners. This should be - // the object that controls the configuration - if !utils.IsOwner(obj) { - continue - } - // If controller has no replicas clear do not run any validations - if isControllersWithNoReplicas(obj) { - continue - } - lintCtx.addObjects(lintcontext.Object{K8sObject: obj}) - } - lintCtxs := []lintcontext.LintContext{lintCtx} - if len(lintCtxs) == 0 { - return ObjectValidationIgnored, nil - } - result, err := run.Run(lintCtxs, ve.CheckRegistry(), ve.EnabledChecks()) - if err != nil { - log.Error(err, "error running validations") - return "", fmt.Errorf("error running validations: %v", err) - } - - // Clear labels from past run to ensure only results from this run - // are reflected in the metrics - for _, o := range objects { - req := NewRequestFromObject(o) - req.NamespaceUID = namespaceUID - ve.ClearMetrics(result.Reports, req.ToPromLabels()) - } - return processResult(result, namespaceUID) -} - // Get info on config file if it exists func fileExists(filename string) bool { info, err := os.Stat(filename) @@ -237,9 +204,21 @@ func (ve *validationEngine) GetCheckByName(name string) (config.Check, error) { return check, nil } -// UpdateConfig TODO - doc -func (ve *validationEngine) UpdateConfig(newconfig config.Config) { - ve.config = newconfig +// Start TODO - doc +func (ve *validationEngine) Start(ctx context.Context) error { + for { + select { + case cfg := <-ve.cmWatcher.ConfigChanged(): + ve.config = cfg + err := ve.InitRegistry() + if err != nil { + fmt.Printf("error updating configuration from ConfigMap: %v\n", cfg) + } + + case <-ctx.Done(): + return nil + } + } } // disableIncompatibleChecks will forcibly update a kube-linter config From cab232af5f2b2d00cf2c785b995188876b9eac73 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 17 Aug 2023 18:11:09 +0200 Subject: [PATCH 18/41] Update comments --- main.go | 2 +- pkg/validations/validation_engine.go | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/main.go b/main.go index ce319fa2..ac1cc7c7 100644 --- a/main.go +++ b/main.go @@ -142,7 +142,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("initializing discovery client: %w", err) } - gr, err := controller.NewGenericReconciler(mgr.GetClient(), discoveryClient) // TODO - deprecated validationengine param + gr, err := controller.NewGenericReconciler(mgr.GetClient(), discoveryClient) if err != nil { return nil, fmt.Errorf("initializing generic reconciler: %w", err) } diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index c52f836a..8dd7298b 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -159,8 +159,6 @@ func (ve *validationEngine) InitRegistry() error { ve.metrics = validationMetrics ve.registeredChecks = registeredChecks - // keeping the validation engine glogal scoped for compatibility - // it would be a good idea to remove the global variable use in the long run engine = *ve return nil @@ -204,7 +202,7 @@ func (ve *validationEngine) GetCheckByName(name string) (config.Check, error) { return check, nil } -// Start TODO - doc +// Start will overwrite validations if no error appears with the new config func (ve *validationEngine) Start(ctx context.Context) error { for { select { From 4efd6f4a0b0d3d1b5ae4a784781cef9496fa0468 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Mon, 21 Aug 2023 13:19:33 +0200 Subject: [PATCH 19/41] Fix linting recommendations --- main.go | 6 ++--- pkg/configmap/configmap_watcher.go | 16 ++++++------ pkg/configmap/configmap_watcher_test.go | 2 +- pkg/prometheus/prometheus.go | 6 ++++- pkg/validations/base_test.go | 8 +++--- pkg/validations/validation_engine.go | 34 ++++++++++++------------- 6 files changed, 38 insertions(+), 34 deletions(-) diff --git a/main.go b/main.go index ac1cc7c7..eeb260df 100644 --- a/main.go +++ b/main.go @@ -17,7 +17,7 @@ import ( "github.com/app-sre/deployment-validation-operator/internal/options" "github.com/app-sre/deployment-validation-operator/pkg/configmap" "github.com/app-sre/deployment-validation-operator/pkg/controller" - dvo_prom "github.com/app-sre/deployment-validation-operator/pkg/prometheus" + dvoProm "github.com/app-sre/deployment-validation-operator/pkg/prometheus" "github.com/app-sre/deployment-validation-operator/pkg/validations" "github.com/app-sre/deployment-validation-operator/version" @@ -97,14 +97,14 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error log.Info("-> Initialize Prometheus Registry") - reg, err := dvo_prom.SetupRegistry() + reg, err := dvoProm.SetupRegistry() if err != nil { return nil, fmt.Errorf("initializing Prometheus server: %w", err) } log.Info(fmt.Sprintf("-> Initialize Prometheus metrics endpoint on %q", opts.MetricsEndpoint())) - srv, err := dvo_prom.NewServer(reg, opts.MetricsPath, fmt.Sprintf(":%d", opts.MetricsPort)) + srv, err := dvoProm.NewServer(reg, opts.MetricsPath, fmt.Sprintf(":%d", opts.MetricsPort)) if err != nil { return nil, fmt.Errorf("initializing metrics server: %w", err) } diff --git a/pkg/configmap/configmap_watcher.go b/pkg/configmap/configmap_watcher.go index 6434b452..a8091087 100644 --- a/pkg/configmap/configmap_watcher.go +++ b/pkg/configmap/configmap_watcher.go @@ -32,7 +32,7 @@ type KubeLinterChecks struct { } `yaml:"checks"` } -type ConfigMapWatcher struct { +type Watcher struct { clientset kubernetes.Interface checks KubeLinterChecks ch chan config.Config @@ -46,13 +46,13 @@ 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 // dynamic: with StartInformer it sets an Informer that will be triggered on ConfigMap update -func NewConfigMapWatcher(cfg *rest.Config) (ConfigMapWatcher, error) { +func NewConfigMapWatcher(cfg *rest.Config) (Watcher, error) { clientset, err := kubernetes.NewForConfig(cfg) if err != nil { - return ConfigMapWatcher{}, fmt.Errorf("initializing clientset: %w", err) + return Watcher{}, fmt.Errorf("initializing clientset: %w", err) } - return ConfigMapWatcher{ + return Watcher{ clientset: clientset, logger: log.Log.WithName("ConfigMapWatcher"), ch: make(chan config.Config), @@ -60,7 +60,7 @@ func NewConfigMapWatcher(cfg *rest.Config) (ConfigMapWatcher, error) { } // GetStaticKubelinterConfig returns the ConfigMap's checks configuration -func (cmw *ConfigMapWatcher) GetStaticKubelinterConfig(ctx context.Context) (config.Config, error) { +func (cmw *Watcher) GetStaticKubelinterConfig(ctx context.Context) (config.Config, error) { cm, err := cmw.clientset.CoreV1(). ConfigMaps(configMapNamespace).Get(ctx, configMapName, v1.GetOptions{}) if err != nil { @@ -71,7 +71,7 @@ func (cmw *ConfigMapWatcher) GetStaticKubelinterConfig(ctx context.Context) (con } // Start will update the channel structure with new configuration data from ConfigMap update event -func (cmw ConfigMapWatcher) Start(ctx context.Context) error { +func (cmw Watcher) Start(ctx context.Context) error { factory := informers.NewSharedInformerFactoryWithOptions( cmw.clientset, time.Second*30, informers.WithNamespace(configMapNamespace), ) @@ -104,13 +104,13 @@ func (cmw ConfigMapWatcher) Start(ctx context.Context) error { } // ConfigChanged receives push notifications when the configuration is updated -func (cmw *ConfigMapWatcher) ConfigChanged() <-chan config.Config { +func (cmw *Watcher) 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) { +func (cmw *Watcher) getKubeLinterConfig(data string) (config.Config, error) { var cfg config.Config err := yaml.Unmarshal([]byte(data), &cmw.checks) diff --git a/pkg/configmap/configmap_watcher_test.go b/pkg/configmap/configmap_watcher_test.go index fc500271..7d30989e 100644 --- a/pkg/configmap/configmap_watcher_test.go +++ b/pkg/configmap/configmap_watcher_test.go @@ -58,7 +58,7 @@ func TestStaticConfigMapWatcher(t *testing.T) { }, } client := kubefake.NewSimpleClientset([]runtime.Object{cm}...) - mock := ConfigMapWatcher{clientset: client} + mock := Watcher{clientset: client} // When test, err := mock.GetStaticKubelinterConfig(context.Background()) diff --git a/pkg/prometheus/prometheus.go b/pkg/prometheus/prometheus.go index 3d04186e..a9a38f20 100644 --- a/pkg/prometheus/prometheus.go +++ b/pkg/prometheus/prometheus.go @@ -120,7 +120,11 @@ func setupMetric(reg checkregistry.CheckRegistry, name string) (*prometheus.Gaug return prometheus.NewGaugeVec( prometheus.GaugeOpts{ Name: strings.ReplaceAll(check.Spec.Name, "-", "_"), - Help: fmt.Sprintf("Description: %s ; Remediation: %s", check.Spec.Description, check.Spec.Remediation), + Help: fmt.Sprintf( + "Description: %s ; Remediation: %s", + check.Spec.Description, + check.Spec.Remediation, + ), ConstLabels: prometheus.Labels{ "check_description": check.Spec.Description, "check_remediation": check.Spec.Remediation, diff --git a/pkg/validations/base_test.go b/pkg/validations/base_test.go index c70d4524..a4fe3cd0 100644 --- a/pkg/validations/base_test.go +++ b/pkg/validations/base_test.go @@ -28,13 +28,13 @@ const ( customCheckTemplate = "minimum-replicas" ) -func newEngine(c config.Config) (validationEngine, error) { - ve := validationEngine{ +func newEngine(c config.Config) (ValidationEngine, error) { + ve := ValidationEngine{ config: c, } loadErr := ve.InitRegistry() if loadErr != nil { - return validationEngine{}, loadErr + return ValidationEngine{}, loadErr } return ve, nil } @@ -333,7 +333,7 @@ func stringInSlice(a string, list []string) bool { // getAllBuiltInKubeLinterChecks returns every check built-into kube-linter (including checks that DVO disables) func getAllBuiltInKubeLinterChecks() ([]string, error) { - ve := validationEngine{ + ve := ValidationEngine{ config: newEngineConfigWithAllChecks(), } registry := checkregistry.New() diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index 8dd7298b..37131690 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -3,14 +3,14 @@ package validations import ( // Used to embed yamls by kube-linter "context" - _ "embed" + _ "embed" // nolint:golint "fmt" "os" "strings" // Import checks from DVO "github.com/app-sre/deployment-validation-operator/pkg/configmap" - _ "github.com/app-sre/deployment-validation-operator/pkg/validations/all" + _ "github.com/app-sre/deployment-validation-operator/pkg/validations/all" // nolint:golint "golang.stackrox.io/kube-linter/pkg/checkregistry" "golang.stackrox.io/kube-linter/pkg/config" @@ -18,26 +18,26 @@ import ( "golang.stackrox.io/kube-linter/pkg/diagnostic" // Import and initialize all check templates from kube-linter - _ "golang.stackrox.io/kube-linter/pkg/templates/all" + _ "golang.stackrox.io/kube-linter/pkg/templates/all" // nolint:golint "github.com/prometheus/client_golang/prometheus" "github.com/spf13/viper" ) -var engine validationEngine +var engine ValidationEngine -type validationEngine struct { +type ValidationEngine struct { config config.Config registry checkregistry.CheckRegistry enabledChecks []string registeredChecks map[string]config.Check metrics map[string]*prometheus.GaugeVec - cmWatcher *configmap.ConfigMapWatcher + cmWatcher *configmap.Watcher } -func NewEngine(configPath string, cmw configmap.ConfigMapWatcher) (*validationEngine, error) { - ve := &validationEngine{ +func NewEngine(configPath string, cmw configmap.Watcher) (*ValidationEngine, error) { + ve := &ValidationEngine{ cmWatcher: &cmw, } @@ -54,11 +54,11 @@ func NewEngine(configPath string, cmw configmap.ConfigMapWatcher) (*validationEn return ve, nil } -func (ve *validationEngine) CheckRegistry() checkregistry.CheckRegistry { +func (ve *ValidationEngine) CheckRegistry() checkregistry.CheckRegistry { return ve.registry } -func (ve *validationEngine) EnabledChecks() []string { +func (ve *ValidationEngine) EnabledChecks() []string { return ve.enabledChecks } @@ -71,7 +71,7 @@ func fileExists(filename string) bool { return !info.IsDir() } -func (ve *validationEngine) LoadConfig(path string) error { +func (ve *ValidationEngine) LoadConfig(path string) error { if !fileExists(path) { log.Info(fmt.Sprintf("config file %s does not exist. Use default configuration", path)) // TODO - This hardcode will be removed when a ConfigMap is set by default in regular installation @@ -112,7 +112,7 @@ type PrometheusRegistry interface { Register(prometheus.Collector) error } -func (ve *validationEngine) InitRegistry() error { +func (ve *ValidationEngine) InitRegistry() error { disableIncompatibleChecks(&ve.config) registry, err := GetKubeLinterRegistry() @@ -164,7 +164,7 @@ func (ve *validationEngine) InitRegistry() error { return nil } -func (ve *validationEngine) GetMetric(name string) *prometheus.GaugeVec { +func (ve *ValidationEngine) GetMetric(name string) *prometheus.GaugeVec { m, ok := ve.metrics[name] if !ok { return nil @@ -172,13 +172,13 @@ func (ve *validationEngine) GetMetric(name string) *prometheus.GaugeVec { return m } -func (ve *validationEngine) DeleteMetrics(labels prometheus.Labels) { +func (ve *ValidationEngine) DeleteMetrics(labels prometheus.Labels) { for _, vector := range ve.metrics { vector.Delete(labels) } } -func (ve *validationEngine) ClearMetrics(reports []diagnostic.WithContext, labels prometheus.Labels) { +func (ve *ValidationEngine) ClearMetrics(reports []diagnostic.WithContext, labels prometheus.Labels) { // Create a list of validation names for use to delete the labels from any // metric which isn't in the report but for which there is a metric reportValidationNames := map[string]struct{}{} @@ -194,7 +194,7 @@ func (ve *validationEngine) ClearMetrics(reports []diagnostic.WithContext, label } } -func (ve *validationEngine) GetCheckByName(name string) (config.Check, error) { +func (ve *ValidationEngine) GetCheckByName(name string) (config.Check, error) { check, ok := ve.registeredChecks[name] if !ok { return config.Check{}, fmt.Errorf("check '%s' is not registered", name) @@ -203,7 +203,7 @@ func (ve *validationEngine) GetCheckByName(name string) (config.Check, error) { } // Start will overwrite validations if no error appears with the new config -func (ve *validationEngine) Start(ctx context.Context) error { +func (ve *ValidationEngine) Start(ctx context.Context) error { for { select { case cfg := <-ve.cmWatcher.ConfigChanged(): From 32884631c2c6840250951735b0b2252e842953f5 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Mon, 21 Aug 2023 19:22:37 +0200 Subject: [PATCH 20/41] WIP Refactor metrics used on Prom registry to be used in VE struct --- main.go | 8 +++++--- pkg/prometheus/prometheus.go | 18 ++++++++++-------- pkg/validations/utils.go | 17 ----------------- pkg/validations/validation_engine.go | 19 ++----------------- 4 files changed, 17 insertions(+), 45 deletions(-) diff --git a/main.go b/main.go index eeb260df..716402c1 100644 --- a/main.go +++ b/main.go @@ -20,6 +20,7 @@ import ( dvoProm "github.com/app-sre/deployment-validation-operator/pkg/prometheus" "github.com/app-sre/deployment-validation-operator/pkg/validations" "github.com/app-sre/deployment-validation-operator/version" + "github.com/prometheus/client_golang/prometheus" "github.com/go-logr/logr" osappsv1 "github.com/openshift/api/apps/v1" @@ -97,9 +98,10 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error log.Info("-> Initialize Prometheus Registry") - reg, err := dvoProm.SetupRegistry() + reg := prometheus.NewRegistry() + metrics, err := dvoProm.PreloadMetrics(reg) if err != nil { - return nil, fmt.Errorf("initializing Prometheus server: %w", err) + return nil, fmt.Errorf("preloading kube-linter metrics: %w", err) } log.Info(fmt.Sprintf("-> Initialize Prometheus metrics endpoint on %q", opts.MetricsEndpoint())) @@ -126,7 +128,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error log.Info("-> Initialize Validation Engine") - validationEngine, err := validations.NewEngine(opts.ConfigFile, cmWatcher) + validationEngine, err := validations.NewEngine(opts.ConfigFile, cmWatcher, metrics) if err != nil { return nil, fmt.Errorf("initializing validation engine: %w", err) } diff --git a/pkg/prometheus/prometheus.go b/pkg/prometheus/prometheus.go index a9a38f20..657fe79e 100644 --- a/pkg/prometheus/prometheus.go +++ b/pkg/prometheus/prometheus.go @@ -82,32 +82,34 @@ func getRouter(registry Registry, path string) (*http.ServeMux, error) { return mux, nil } -// SetupRegistry returns a fully configured Prometheus registry with metrics based on Kube Linter validations -func SetupRegistry() (*prometheus.Registry, error) { - prom := prometheus.NewRegistry() +// PreloadMetrics TODO : Doc this +func PreloadMetrics(pr *prometheus.Registry) (map[string]*prometheus.GaugeVec, error) { + preloadedMetrics := make(map[string]*prometheus.GaugeVec) - reg, err := validations.GetKubeLinterRegistry() + klr, err := validations.GetKubeLinterRegistry() if err != nil { return nil, err } - checks, err := validations.GetAllNamesFromRegistry(reg) + checks, err := validations.GetAllNamesFromRegistry(klr) if err != nil { return nil, err } for _, checkName := range checks { - metric, err := setupMetric(reg, checkName) + metric, err := setupMetric(klr, checkName) if err != nil { return nil, fmt.Errorf("unable to create metric for check %s", checkName) } - if err := prom.Register(metric); err != nil { + if err := pr.Register(metric); err != nil { return nil, fmt.Errorf("registering metric for check %q: %w", checkName, err) } + + preloadedMetrics[checkName] = metric } - return prom, nil + return preloadedMetrics, nil } // setupMetric uses registered validations to return the correct metric for a Prometheus registry diff --git a/pkg/validations/utils.go b/pkg/validations/utils.go index 846ac631..7e603d8f 100644 --- a/pkg/validations/utils.go +++ b/pkg/validations/utils.go @@ -1,10 +1,6 @@ package validations import ( - "fmt" - "strings" - - "github.com/app-sre/deployment-validation-operator/config" "golang.stackrox.io/kube-linter/pkg/builtinchecks" "golang.stackrox.io/kube-linter/pkg/checkregistry" klConfig "golang.stackrox.io/kube-linter/pkg/config" @@ -17,19 +13,6 @@ func DeleteMetrics(labels prometheus.Labels) { engine.DeleteMetrics(labels) } -func newGaugeVecMetric( - name, help string, labelNames []string, constLabels prometheus.Labels, -) *prometheus.GaugeVec { - return prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: fmt.Sprintf("%s_%s", strings.ReplaceAll(config.OperatorName, "-", "_"), name), - Help: help, - ConstLabels: constLabels, - }, - labelNames, - ) -} - // GetKubeLinterRegistry sets up a Kube Linter registry with builtin default validations func GetKubeLinterRegistry() (checkregistry.CheckRegistry, error) { registry := checkregistry.New() diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index 37131690..3e11b706 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -6,7 +6,6 @@ import ( _ "embed" // nolint:golint "fmt" "os" - "strings" // Import checks from DVO "github.com/app-sre/deployment-validation-operator/pkg/configmap" @@ -36,9 +35,10 @@ type ValidationEngine struct { cmWatcher *configmap.Watcher } -func NewEngine(configPath string, cmw configmap.Watcher) (*ValidationEngine, error) { +func NewEngine(configPath string, cmw configmap.Watcher, metrics map[string]*prometheus.GaugeVec) (*ValidationEngine, error) { ve := &ValidationEngine{ cmWatcher: &cmw, + metrics: metrics, } err := ve.LoadConfig(configPath) @@ -131,8 +131,6 @@ func (ve *ValidationEngine) InitRegistry() error { return err } - // TODO - Use same approach than in prometheus package - validationMetrics := map[string]*prometheus.GaugeVec{} registeredChecks := map[string]config.Check{} for _, checkName := range enabledChecks { check := registry.Load(checkName) @@ -140,23 +138,10 @@ func (ve *ValidationEngine) InitRegistry() error { return fmt.Errorf("unable to create metric for check %s", checkName) } registeredChecks[check.Spec.Name] = check.Spec - metric := newGaugeVecMetric( - strings.ReplaceAll(check.Spec.Name, "-", "_"), - fmt.Sprintf("Description: %s ; Remediation: %s", - check.Spec.Description, check.Spec.Remediation), - []string{"namespace_uid", "namespace", "uid", "name", "kind"}, - prometheus.Labels{ - "check_description": check.Spec.Description, - "check_remediation": check.Spec.Remediation, - }, - ) - - validationMetrics[checkName] = metric } ve.registry = registry ve.enabledChecks = enabledChecks - ve.metrics = validationMetrics ve.registeredChecks = registeredChecks engine = *ve From 0d9235d8a2ccb7f618a34cac4bb9099a8cdcb654 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Tue, 22 Aug 2023 13:09:40 +0200 Subject: [PATCH 21/41] Update documentation --- pkg/prometheus/prometheus.go | 14 ++++++++++++-- pkg/validations/utils.go | 19 ++++++++++++++++--- pkg/validations/validation_engine.go | 12 ++++++++++++ 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/pkg/prometheus/prometheus.go b/pkg/prometheus/prometheus.go index 657fe79e..3169a3bf 100644 --- a/pkg/prometheus/prometheus.go +++ b/pkg/prometheus/prometheus.go @@ -82,7 +82,16 @@ func getRouter(registry Registry, path string) (*http.ServeMux, error) { return mux, nil } -// PreloadMetrics TODO : Doc this +// PreloadMetrics preloads metrics related to predefined checks into the provided Prometheus registry. +// It retrieves predefined checks from the linter registry, sets up corresponding GaugeVec metrics, +// and registers them in the Prometheus registry. +// +// Parameters: +// - pr: A pointer to a Prometheus registry where the metrics will be registered. +// +// Returns: +// - A map of check names to corresponding GaugeVec metrics. +// - An error if any error occurs during metric setup or registration. func PreloadMetrics(pr *prometheus.Registry) (map[string]*prometheus.GaugeVec, error) { preloadedMetrics := make(map[string]*prometheus.GaugeVec) @@ -112,7 +121,8 @@ func PreloadMetrics(pr *prometheus.Registry) (map[string]*prometheus.GaugeVec, e return preloadedMetrics, nil } -// setupMetric uses registered validations to return the correct metric for a Prometheus registry +// setupMetric sets up a Prometheus GaugeVec metric based on the provided check name and information from a CheckRegistry. +// The metric is created with the formatted name, description, and remediation information from the check specification. func setupMetric(reg checkregistry.CheckRegistry, name string) (*prometheus.GaugeVec, error) { check := reg.Load(name) if check == nil { diff --git a/pkg/validations/utils.go b/pkg/validations/utils.go index 7e603d8f..1342e3b1 100644 --- a/pkg/validations/utils.go +++ b/pkg/validations/utils.go @@ -13,7 +13,13 @@ func DeleteMetrics(labels prometheus.Labels) { engine.DeleteMetrics(labels) } -// GetKubeLinterRegistry sets up a Kube Linter registry with builtin default validations +// GetKubeLinterRegistry returns a CheckRegistry containing kube-linter built-in validations. +// It initializes a new CheckRegistry, loads the built-in validations into the registry, +// and returns the resulting registry if successful. +// +// Returns: +// - A CheckRegistry containing kube-linter built-in validations if successful. +// - An error if the built-in validations fail to load into the registry. func GetKubeLinterRegistry() (checkregistry.CheckRegistry, error) { registry := checkregistry.New() if err := builtinchecks.LoadInto(registry); err != nil { @@ -24,8 +30,15 @@ func GetKubeLinterRegistry() (checkregistry.CheckRegistry, error) { return registry, nil } -// GetAllNamesFromRegistry returns a slice with the names of all valid Kube Linter checks. -// Since any check can be configured ad hoc, we return all valid checks. +// GetAllNamesFromRegistry retrieves the names of all enabled checks from the provided CheckRegistry. +// It fetches the names of checks that are enabled based on a specified configuration, excluding incompatible ones. +// +// Parameters: +// - reg: A CheckRegistry containing predefined checks and their specifications. +// +// Returns: +// - A slice of strings containing the names of all enabled checks if successful. +// - An error if there's an issue while fetching the enabled check names or validating the configuration. func GetAllNamesFromRegistry(reg checkregistry.CheckRegistry) ([]string, error) { // Get all checks except for incompatible ones cfg := klConfig.Config{ diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index 3e11b706..5330cdd9 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -35,6 +35,18 @@ type ValidationEngine struct { cmWatcher *configmap.Watcher } +// NewEngine creates a new ValidationEngine instance with the provided configuration path, configmap.Watcher, and metrics. +// It initializes a ValidationEngine with the provided watcher for configmap changes and a set of preloaded metrics. +// The engine's configuration is loaded from the specified configuration path, and its check registry is initialized. +// +// Parameters: +// - configPath: The path to the configuration file for the ValidationEngine. +// - cmw: A configmap.Watcher for monitoring changes to configmaps. +// - metrics: A map of preloaded Prometheus GaugeVec metrics. +// +// Returns: +// - A pointer to a new ValidationEngine instance if successful. +// - An error if there's an issue loading the configuration or initializing the check registry. func NewEngine(configPath string, cmw configmap.Watcher, metrics map[string]*prometheus.GaugeVec) (*ValidationEngine, error) { ve := &ValidationEngine{ cmWatcher: &cmw, From 9fa2b714a1a1ac569cca3f3247acc9aa7f3e9ea1 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Tue, 22 Aug 2023 13:13:02 +0200 Subject: [PATCH 22/41] Fix loglines prefixes --- main.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/main.go b/main.go index 716402c1..72d2b1ad 100644 --- a/main.go +++ b/main.go @@ -96,7 +96,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error log.Info("Registering Components") - log.Info("-> Initialize Prometheus Registry") + log.Info("Initialize Prometheus Registry") reg := prometheus.NewRegistry() metrics, err := dvoProm.PreloadMetrics(reg) @@ -104,7 +104,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("preloading kube-linter metrics: %w", err) } - log.Info(fmt.Sprintf("-> Initialize Prometheus metrics endpoint on %q", opts.MetricsEndpoint())) + log.Info(fmt.Sprintf("Initialize Prometheus metrics endpoint on %q", opts.MetricsEndpoint())) srv, err := dvoProm.NewServer(reg, opts.MetricsPath, fmt.Sprintf(":%d", opts.MetricsPort)) if err != nil { @@ -115,7 +115,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("adding metrics server to manager: %w", err) } - log.Info("-> Initialize ConfigMap watcher") + log.Info("Initialize ConfigMap watcher") cmWatcher, err := configmap.NewConfigMapWatcher(cfg) if err != nil { @@ -126,7 +126,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("adding configmap watcher to manager: %w", err) } - log.Info("-> Initialize Validation Engine") + log.Info("Initialize Validation Engine") validationEngine, err := validations.NewEngine(opts.ConfigFile, cmWatcher, metrics) if err != nil { @@ -137,7 +137,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("adding validationEngine to manager: %w", err) } - log.Info("-> Initialize Reconciler") + log.Info("Initialize Reconciler") discoveryClient, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig()) if err != nil { @@ -240,13 +240,13 @@ func kubeClientQPS() (float32, error) { } func initManager(log logr.Logger, opts options.Options, cfg *rest.Config) (manager.Manager, error) { - log.Info("-> Initialize Scheme") + log.Info("Initialize Scheme") scheme, err := initializeScheme() if err != nil { return nil, fmt.Errorf("initializing scheme: %w", err) } - log.Info("-> Getting Manager Options") + log.Info("Getting Manager Options") mgrOpts, err := getManagerOptions(scheme, opts) if err != nil { return nil, fmt.Errorf("getting manager options: %w", err) @@ -257,7 +257,7 @@ func initManager(log logr.Logger, opts options.Options, cfg *rest.Config) (manag return nil, fmt.Errorf("getting new manager: %w", err) } - log.Info("-> Adding Healthz and Readyz checks") + log.Info("Adding Healthz and Readyz checks") if err := mgr.AddHealthzCheck("health", healthz.Ping); err != nil { return nil, fmt.Errorf("adding healthz check: %w", err) } From 2a35cf6a01a1c58e498291f0e132bf52fc585711 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Tue, 22 Aug 2023 13:22:05 +0200 Subject: [PATCH 23/41] Fix linter issues --- pkg/prometheus/prometheus.go | 2 +- pkg/validations/validation_engine.go | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/prometheus/prometheus.go b/pkg/prometheus/prometheus.go index 3169a3bf..24bc674f 100644 --- a/pkg/prometheus/prometheus.go +++ b/pkg/prometheus/prometheus.go @@ -121,7 +121,7 @@ func PreloadMetrics(pr *prometheus.Registry) (map[string]*prometheus.GaugeVec, e return preloadedMetrics, nil } -// setupMetric sets up a Prometheus GaugeVec metric based on the provided check name and information from a CheckRegistry. +// setupMetric sets up a Prometheus metric based on the provided checkname and information from a CheckRegistry. // The metric is created with the formatted name, description, and remediation information from the check specification. func setupMetric(reg checkregistry.CheckRegistry, name string) (*prometheus.GaugeVec, error) { check := reg.Load(name) diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index 5330cdd9..c84d3643 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -35,7 +35,7 @@ type ValidationEngine struct { cmWatcher *configmap.Watcher } -// NewEngine creates a new ValidationEngine instance with the provided configuration path, configmap.Watcher, and metrics. +// NewEngine creates a new ValidationEngine instance with the provided configuration path, a watcher, and metrics. // It initializes a ValidationEngine with the provided watcher for configmap changes and a set of preloaded metrics. // The engine's configuration is loaded from the specified configuration path, and its check registry is initialized. // @@ -47,7 +47,11 @@ type ValidationEngine struct { // Returns: // - A pointer to a new ValidationEngine instance if successful. // - An error if there's an issue loading the configuration or initializing the check registry. -func NewEngine(configPath string, cmw configmap.Watcher, metrics map[string]*prometheus.GaugeVec) (*ValidationEngine, error) { +func NewEngine( + configPath string, + cmw configmap.Watcher, + metrics map[string]*prometheus.GaugeVec, +) (*ValidationEngine, error) { ve := &ValidationEngine{ cmWatcher: &cmw, metrics: metrics, From 3bea7dfeb9cac17315c89687f2783cbebc645652 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Tue, 22 Aug 2023 19:20:50 +0200 Subject: [PATCH 24/41] Fix missing prefix on metrics --- pkg/prometheus/prometheus.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/prometheus/prometheus.go b/pkg/prometheus/prometheus.go index 24bc674f..6a14d81b 100644 --- a/pkg/prometheus/prometheus.go +++ b/pkg/prometheus/prometheus.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/app-sre/deployment-validation-operator/config" "github.com/app-sre/deployment-validation-operator/pkg/validations" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/collectors" @@ -131,7 +132,9 @@ func setupMetric(reg checkregistry.CheckRegistry, name string) (*prometheus.Gaug return prometheus.NewGaugeVec( prometheus.GaugeOpts{ - Name: strings.ReplaceAll(check.Spec.Name, "-", "_"), + Name: strings.ReplaceAll( + fmt.Sprintf("%s_%s", config.OperatorName, check.Spec.Name), + "-", "_"), Help: fmt.Sprintf( "Description: %s ; Remediation: %s", check.Spec.Description, From b570cdb6e9a0450876c4ecc51f87a25ba152852c Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Tue, 22 Aug 2023 19:22:17 +0200 Subject: [PATCH 25/41] Fix unit tests breaking due to metrics preload --- pkg/validations/base_test.go | 6 ++++++ pkg/validations/utils.go | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/pkg/validations/base_test.go b/pkg/validations/base_test.go index a4fe3cd0..04e6de73 100644 --- a/pkg/validations/base_test.go +++ b/pkg/validations/base_test.go @@ -36,6 +36,12 @@ func newEngine(c config.Config) (ValidationEngine, error) { if loadErr != nil { return ValidationEngine{}, loadErr } + // checks now are preloaded, adding them after Registry init + ve.metrics = make(map[string]*prometheus.GaugeVec) + for _, checkName := range ve.EnabledChecks() { + check := ve.registeredChecks[checkName] + ve.metrics[checkName] = newGaugeVecMetric(check) + } return ve, nil } diff --git a/pkg/validations/utils.go b/pkg/validations/utils.go index 1342e3b1..59a261ae 100644 --- a/pkg/validations/utils.go +++ b/pkg/validations/utils.go @@ -1,11 +1,15 @@ package validations import ( + "fmt" + "strings" + "golang.stackrox.io/kube-linter/pkg/builtinchecks" "golang.stackrox.io/kube-linter/pkg/checkregistry" klConfig "golang.stackrox.io/kube-linter/pkg/config" "golang.stackrox.io/kube-linter/pkg/configresolver" + "github.com/app-sre/deployment-validation-operator/config" "github.com/prometheus/client_golang/prometheus" ) @@ -56,3 +60,19 @@ func GetAllNamesFromRegistry(reg checkregistry.CheckRegistry) ([]string, error) return checks, nil } + +func newGaugeVecMetric(check klConfig.Check) *prometheus.GaugeVec { + return prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Name: strings.ReplaceAll( + fmt.Sprintf("%s_%s", config.OperatorName, check.Name), + "-", "_"), + Help: fmt.Sprintf( + "Description: %s ; Remediation: %s", check.Description, check.Remediation, + ), + ConstLabels: prometheus.Labels{ + "check_description": check.Description, + "check_remediation": check.Remediation, + }, + }, []string{"namespace_uid", "namespace", "uid", "name", "kind"}) +} From 2a3abeeb2511d73945ddf0325698df37b1930745 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 24 Aug 2023 17:37:11 +0200 Subject: [PATCH 26/41] Fix nit recommendations --- main.go | 2 +- pkg/validations/utils.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/main.go b/main.go index 72d2b1ad..758d1c95 100644 --- a/main.go +++ b/main.go @@ -117,7 +117,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error log.Info("Initialize ConfigMap watcher") - cmWatcher, err := configmap.NewConfigMapWatcher(cfg) + cmWatcher, err := configmap.NewWatcher(cfg) if err != nil { return nil, fmt.Errorf("initializing configmap watcher: %w", err) } diff --git a/pkg/validations/utils.go b/pkg/validations/utils.go index 59a261ae..dc0b9007 100644 --- a/pkg/validations/utils.go +++ b/pkg/validations/utils.go @@ -62,11 +62,11 @@ func GetAllNamesFromRegistry(reg checkregistry.CheckRegistry) ([]string, error) } func newGaugeVecMetric(check klConfig.Check) *prometheus.GaugeVec { + metricName := strings.ReplaceAll(fmt.Sprintf("%s_%s", config.OperatorName, check.Name), "-", "_") + return prometheus.NewGaugeVec( prometheus.GaugeOpts{ - Name: strings.ReplaceAll( - fmt.Sprintf("%s_%s", config.OperatorName, check.Name), - "-", "_"), + Name: metricName, Help: fmt.Sprintf( "Description: %s ; Remediation: %s", check.Description, check.Remediation, ), From 99e05d2d251224c153e168a3b480db225f54ebdf Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 24 Aug 2023 17:37:44 +0200 Subject: [PATCH 27/41] Add dynamic namespace setting on watcher --- pkg/configmap/configmap_watcher.go | 67 ++++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 7 deletions(-) diff --git a/pkg/configmap/configmap_watcher.go b/pkg/configmap/configmap_watcher.go index a8091087..1b9221ed 100644 --- a/pkg/configmap/configmap_watcher.go +++ b/pkg/configmap/configmap_watcher.go @@ -6,6 +6,8 @@ import ( "reflect" "time" + dvoConfig "github.com/app-sre/deployment-validation-operator/config" + "golang.stackrox.io/kube-linter/pkg/config" "gopkg.in/yaml.v3" @@ -37,32 +39,46 @@ type Watcher struct { checks KubeLinterChecks ch chan config.Config logger logr.Logger + namespace string } 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 -// dynamic: with StartInformer it sets an Informer that will be triggered on ConfigMap update -func NewConfigMapWatcher(cfg *rest.Config) (Watcher, error) { +// NewWatcher creates a new Watcher instance for observing changes to a ConfigMap. +// +// Parameters: +// - cfg: A pointer to a rest.Config representing the Kubernetes client configuration. +// +// Returns: +// - A Watcher instance for monitoring changes to DVO ConfigMap resource if the initialization is successful. +// - An error if there's an issue while initializing the Kubernetes clientset. +func NewWatcher(cfg *rest.Config) (Watcher, error) { clientset, err := kubernetes.NewForConfig(cfg) if err != nil { return Watcher{}, fmt.Errorf("initializing clientset: %w", err) } + // the Informer will use this to monitor the namespace for the ConfigMap. + namespace, err := getDeploymentNamespace(clientset) + if err != nil { + return Watcher{}, fmt.Errorf("getting namespace: %w", err) + } + + // fmt.Printf("namespaces: %v\n", deployments) return Watcher{ clientset: clientset, logger: log.Log.WithName("ConfigMapWatcher"), ch: make(chan config.Config), + namespace: namespace, }, nil } // GetStaticKubelinterConfig returns the ConfigMap's checks configuration func (cmw *Watcher) GetStaticKubelinterConfig(ctx context.Context) (config.Config, error) { cm, err := cmw.clientset.CoreV1(). - ConfigMaps(configMapNamespace).Get(ctx, configMapName, v1.GetOptions{}) + ConfigMaps(cmw.namespace).Get(ctx, configMapName, v1.GetOptions{}) if err != nil { return config.Config{}, fmt.Errorf("getting initial configuration: %w", err) } @@ -73,16 +89,33 @@ func (cmw *Watcher) GetStaticKubelinterConfig(ctx context.Context) (config.Confi // 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( - cmw.clientset, time.Second*30, informers.WithNamespace(configMapNamespace), + cmw.clientset, time.Second*30, informers.WithNamespace(cmw.namespace), ) informer := factory.Core().V1().ConfigMaps().Informer() informer.AddEventHandler(cache.ResourceEventHandlerFuncs{ // nolint:errcheck + AddFunc: func(obj interface{}) { + newCm := obj.(*apicorev1.ConfigMap) + + if configMapName != newCm.GetName() { + return + } + + cmw.logger.Info("ConfigMap has been created") + + cfg, err := cmw.getKubeLinterConfig(newCm.Data[configMapDataAccess]) + if err != nil { + cmw.logger.Error(err, "ConfigMap data format") + return + } + + cmw.ch <- cfg + }, UpdateFunc: func(oldObj, newObj interface{}) { newCm := newObj.(*apicorev1.ConfigMap) // This is sometimes triggered even if no change was due to the ConfigMap - if configMapName != newCm.ObjectMeta.Name || reflect.DeepEqual(oldObj, newObj) { + if configMapName != newCm.GetName() || reflect.DeepEqual(oldObj, newObj) { return } @@ -122,3 +155,23 @@ func (cmw *Watcher) getKubeLinterConfig(data string) (config.Config, error) { return cfg, nil } + +// getDeploymentNamespace retrieves the namespace of the Deployment associated with DVO. +// If found, it returns the namespace of the Deployment. +// If not found, it returns the default namespace value from `dvoConfig.OperatorNamespace`. +func getDeploymentNamespace(clientset *kubernetes.Clientset) (string, error) { + deployments, err := clientset.AppsV1().Deployments(v1.NamespaceAll). + List(context.Background(), v1.ListOptions{}) + if err != nil { + return "", fmt.Errorf("getting deployments from clientset: %w", err) + } + + for _, deployment := range deployments.Items { + if deployment.GetName() == dvoConfig.OperatorName { + return deployment.GetNamespace(), nil + } + } + + // set 'deployment-validation-operator' as default value + return dvoConfig.OperatorNamespace, nil +} From dc54e41cb0a7236e39cc688645eac6c3e2536927 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Fri, 25 Aug 2023 17:24:02 +0200 Subject: [PATCH 28/41] Fix unit tests --- pkg/configmap/configmap_watcher_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/configmap/configmap_watcher_test.go b/pkg/configmap/configmap_watcher_test.go index 7d30989e..1c64d6cb 100644 --- a/pkg/configmap/configmap_watcher_test.go +++ b/pkg/configmap/configmap_watcher_test.go @@ -58,7 +58,7 @@ func TestStaticConfigMapWatcher(t *testing.T) { }, } client := kubefake.NewSimpleClientset([]runtime.Object{cm}...) - mock := Watcher{clientset: client} + mock := Watcher{clientset: client, namespace: configMapNamespace} // When test, err := mock.GetStaticKubelinterConfig(context.Background()) From 134b73cdc13ce7e6f229a036ebb4ab40d4085110 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Tue, 29 Aug 2023 13:40:27 +0200 Subject: [PATCH 29/41] Use Pod instead of deployment for namespace --- pkg/configmap/configmap_watcher.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/configmap/configmap_watcher.go b/pkg/configmap/configmap_watcher.go index 1b9221ed..97f9444c 100644 --- a/pkg/configmap/configmap_watcher.go +++ b/pkg/configmap/configmap_watcher.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "reflect" + "strings" "time" dvoConfig "github.com/app-sre/deployment-validation-operator/config" @@ -156,22 +157,21 @@ func (cmw *Watcher) getKubeLinterConfig(data string) (config.Config, error) { return cfg, nil } -// getDeploymentNamespace retrieves the namespace of the Deployment associated with DVO. -// If found, it returns the namespace of the Deployment. -// If not found, it returns the default namespace value from `dvoConfig.OperatorNamespace`. +// getDeploymentNamespace TODO - refactor name and doc func getDeploymentNamespace(clientset *kubernetes.Clientset) (string, error) { - deployments, err := clientset.AppsV1().Deployments(v1.NamespaceAll). - List(context.Background(), v1.ListOptions{}) + pods, err := clientset.CoreV1().Pods(v1.NamespaceAll). + List(context.Background(), v1.ListOptions{ + LabelSelector: "app=deployment-validation-operator", + }) if err != nil { - return "", fmt.Errorf("getting deployments from clientset: %w", err) + return "", fmt.Errorf("getting DVO pod from clientset: %w", err) } - for _, deployment := range deployments.Items { - if deployment.GetName() == dvoConfig.OperatorName { - return deployment.GetNamespace(), nil + for i := range pods.Items { + if strings.Contains(pods.Items[i].GetName(), dvoConfig.OperatorNamespace) { + return pods.Items[i].GetNamespace(), nil } } - // set 'deployment-validation-operator' as default value - return dvoConfig.OperatorNamespace, nil + return "", fmt.Errorf("could not find DVO pod") } From f51630d389b3dcda750d8a8dd935c659c65522b5 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Tue, 29 Aug 2023 13:42:12 +0200 Subject: [PATCH 30/41] Move back Watcher to reconcilier --- pkg/controller/generic_reconciler.go | 36 ++++++++++++++++++++++- pkg/controller/generic_reconciler_test.go | 3 +- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/pkg/controller/generic_reconciler.go b/pkg/controller/generic_reconciler.go index c0ea0748..1c96f6f3 100644 --- a/pkg/controller/generic_reconciler.go +++ b/pkg/controller/generic_reconciler.go @@ -15,6 +15,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/discovery" + "github.com/app-sre/deployment-validation-operator/pkg/configmap" "github.com/app-sre/deployment-validation-operator/pkg/utils" "github.com/app-sre/deployment-validation-operator/pkg/validations" "github.com/go-logr/logr" @@ -37,10 +38,15 @@ type GenericReconciler struct { client client.Client discovery discovery.DiscoveryInterface logger logr.Logger + cmWatcher *configmap.Watcher } // NewGenericReconciler returns a GenericReconciler struct -func NewGenericReconciler(client client.Client, discovery discovery.DiscoveryInterface) (*GenericReconciler, error) { +func NewGenericReconciler( + client client.Client, + discovery discovery.DiscoveryInterface, + cmw configmap.Watcher, +) (*GenericReconciler, error) { listLimit, err := getListLimit() if err != nil { return nil, err @@ -54,6 +60,7 @@ func NewGenericReconciler(client client.Client, discovery discovery.DiscoveryInt objectValidationCache: newValidationCache(), currentObjects: newValidationCache(), logger: ctrl.Log.WithName("reconcile"), + cmWatcher: &cmw, }, nil } @@ -94,6 +101,8 @@ func (gr *GenericReconciler) AddToManager(mgr manager.Manager) error { // Start validating the given object kind every interval. func (gr *GenericReconciler) Start(ctx context.Context) error { + go gr.LookForConfigUpdates(ctx) + for { select { case <-ctx.Done(): @@ -112,6 +121,31 @@ func (gr *GenericReconciler) Start(ctx context.Context) error { } } +func (gr *GenericReconciler) LookForConfigUpdates(ctx context.Context) { + for { + select { + case cfg := <-gr.cmWatcher.ConfigChanged(): + //ve.config = cfg + validations.UpdateConfig(cfg) // save previous configuration in case of rollback + //err := ve.InitRegistry() + err := validations.InitRegistry() + if err == nil { + gr.objectValidationCache.drain() + validations.ResetMetrics() + + } else { + gr.logger.Error( + err, + fmt.Sprintf("error updating configuration from ConfigMap: %v\n", cfg), + ) + } + + case <-ctx.Done(): + return + } + } +} + func (gr *GenericReconciler) reconcileEverything(ctx context.Context) error { apiResources, err := reconcileResourceList(gr.discovery, gr.client.Scheme()) if err != nil { diff --git a/pkg/controller/generic_reconciler_test.go b/pkg/controller/generic_reconciler_test.go index dfc5fa35..91464f19 100644 --- a/pkg/controller/generic_reconciler_test.go +++ b/pkg/controller/generic_reconciler_test.go @@ -6,6 +6,7 @@ import ( "os" "testing" + "github.com/app-sre/deployment-validation-operator/pkg/configmap" "github.com/app-sre/deployment-validation-operator/pkg/validations" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" @@ -966,5 +967,5 @@ func createTestReconciler(scheme *runtime.Scheme, objects []client.Object) (*Gen } client := cliBuilder.Build() cli := kubefake.NewSimpleClientset() - return NewGenericReconciler(client, cli.Discovery()) + return NewGenericReconciler(client, cli.Discovery(), configmap.Watcher{}) } From c32a514e267a82f70a37448ce17720d93c2f7fed Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Tue, 29 Aug 2023 13:43:01 +0200 Subject: [PATCH 31/41] Add accesses for reconcilier --- pkg/validations/utils.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/validations/utils.go b/pkg/validations/utils.go index dc0b9007..ce41910a 100644 --- a/pkg/validations/utils.go +++ b/pkg/validations/utils.go @@ -76,3 +76,17 @@ func newGaugeVecMetric(check klConfig.Check) *prometheus.GaugeVec { }, }, []string{"namespace_uid", "namespace", "uid", "name", "kind"}) } + +func UpdateConfig(cfg klConfig.Config) { + engine.config = cfg +} + +func InitRegistry() error { + return engine.InitRegistry() +} + +func ResetMetrics() { + for _, metric := range engine.metrics { + metric.Reset() + } +} From dc542f1e2b2c13d93b2ab1166ed2d9b7816a4759 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Tue, 29 Aug 2023 13:45:56 +0200 Subject: [PATCH 32/41] Remove Watcher from VE and fix main script --- main.go | 8 ++---- pkg/validations/validation_engine.go | 40 ++++++---------------------- 2 files changed, 10 insertions(+), 38 deletions(-) diff --git a/main.go b/main.go index 758d1c95..cace9eb2 100644 --- a/main.go +++ b/main.go @@ -128,15 +128,11 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error log.Info("Initialize Validation Engine") - validationEngine, err := validations.NewEngine(opts.ConfigFile, cmWatcher, metrics) + err = validations.InitEngine(opts.ConfigFile, metrics) if err != nil { return nil, fmt.Errorf("initializing validation engine: %w", err) } - if err := mgr.Add(validationEngine); err != nil { - return nil, fmt.Errorf("adding validationEngine to manager: %w", err) - } - log.Info("Initialize Reconciler") discoveryClient, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig()) @@ -144,7 +140,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("initializing discovery client: %w", err) } - gr, err := controller.NewGenericReconciler(mgr.GetClient(), discoveryClient) + gr, err := controller.NewGenericReconciler(mgr.GetClient(), discoveryClient, cmWatcher) if err != nil { return nil, fmt.Errorf("initializing generic reconciler: %w", err) } diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index c84d3643..c8424e13 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -2,13 +2,13 @@ package validations import ( // Used to embed yamls by kube-linter - "context" + _ "embed" // nolint:golint "fmt" "os" // Import checks from DVO - "github.com/app-sre/deployment-validation-operator/pkg/configmap" + _ "github.com/app-sre/deployment-validation-operator/pkg/validations/all" // nolint:golint "golang.stackrox.io/kube-linter/pkg/checkregistry" @@ -32,10 +32,9 @@ type ValidationEngine struct { enabledChecks []string registeredChecks map[string]config.Check metrics map[string]*prometheus.GaugeVec - cmWatcher *configmap.Watcher } -// NewEngine creates a new ValidationEngine instance with the provided configuration path, a watcher, and metrics. +// InitEngine creates a new ValidationEngine instance with the provided configuration path, a watcher, and metrics. // It initializes a ValidationEngine with the provided watcher for configmap changes and a set of preloaded metrics. // The engine's configuration is loaded from the specified configuration path, and its check registry is initialized. // @@ -45,29 +44,23 @@ type ValidationEngine struct { // - metrics: A map of preloaded Prometheus GaugeVec metrics. // // Returns: -// - A pointer to a new ValidationEngine instance if successful. // - An error if there's an issue loading the configuration or initializing the check registry. -func NewEngine( - configPath string, - cmw configmap.Watcher, - metrics map[string]*prometheus.GaugeVec, -) (*ValidationEngine, error) { +func InitEngine(configPath string, metrics map[string]*prometheus.GaugeVec) error { ve := &ValidationEngine{ - cmWatcher: &cmw, - metrics: metrics, + metrics: metrics, } err := ve.LoadConfig(configPath) if err != nil { - return nil, err + return err } err = ve.InitRegistry() if err != nil { - return nil, err + return err } - return ve, nil + return nil } func (ve *ValidationEngine) CheckRegistry() checkregistry.CheckRegistry { @@ -203,23 +196,6 @@ func (ve *ValidationEngine) GetCheckByName(name string) (config.Check, error) { return check, nil } -// Start will overwrite validations if no error appears with the new config -func (ve *ValidationEngine) Start(ctx context.Context) error { - for { - select { - case cfg := <-ve.cmWatcher.ConfigChanged(): - ve.config = cfg - err := ve.InitRegistry() - if err != nil { - fmt.Printf("error updating configuration from ConfigMap: %v\n", cfg) - } - - case <-ctx.Done(): - return nil - } - } -} - // disableIncompatibleChecks will forcibly update a kube-linter config // to disable checks that are incompatible with DVO. // the same check name may end up in the exclude list multiple times as a result of this; this is OK. From df2b040fa5038f186d40c3d778d8dcf273eaad8b Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Tue, 29 Aug 2023 18:29:41 +0200 Subject: [PATCH 33/41] Refactor loglines and pod namespace gather function --- pkg/configmap/configmap_watcher.go | 39 +++++++++++++----------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/pkg/configmap/configmap_watcher.go b/pkg/configmap/configmap_watcher.go index 97f9444c..69d1a3ce 100644 --- a/pkg/configmap/configmap_watcher.go +++ b/pkg/configmap/configmap_watcher.go @@ -3,12 +3,10 @@ package configmap import ( "context" "fmt" + "os" "reflect" - "strings" "time" - dvoConfig "github.com/app-sre/deployment-validation-operator/config" - "golang.stackrox.io/kube-linter/pkg/config" "gopkg.in/yaml.v3" @@ -62,12 +60,11 @@ func NewWatcher(cfg *rest.Config) (Watcher, error) { } // the Informer will use this to monitor the namespace for the ConfigMap. - namespace, err := getDeploymentNamespace(clientset) + namespace, err := getPodNamespace() if err != nil { return Watcher{}, fmt.Errorf("getting namespace: %w", err) } - // fmt.Printf("namespaces: %v\n", deployments) return Watcher{ clientset: clientset, logger: log.Log.WithName("ConfigMapWatcher"), @@ -102,7 +99,11 @@ func (cmw Watcher) Start(ctx context.Context) error { return } - cmw.logger.Info("ConfigMap has been created") + cmw.logger.Info( + "a ConfigMap has been created under watched namespace", + "name", newCm.GetName(), + "namespace", newCm.GetNamespace(), + ) cfg, err := cmw.getKubeLinterConfig(newCm.Data[configMapDataAccess]) if err != nil { @@ -120,7 +121,11 @@ func (cmw Watcher) Start(ctx context.Context) error { return } - cmw.logger.Info("ConfigMap has been updated") + cmw.logger.Info( + "a ConfigMap has been updated under watched namespace", + "name", newCm.GetName(), + "namespace", newCm.GetNamespace(), + ) cfg, err := cmw.getKubeLinterConfig(newCm.Data[configMapDataAccess]) if err != nil { @@ -157,21 +162,11 @@ func (cmw *Watcher) getKubeLinterConfig(data string) (config.Config, error) { return cfg, nil } -// getDeploymentNamespace TODO - refactor name and doc -func getDeploymentNamespace(clientset *kubernetes.Clientset) (string, error) { - pods, err := clientset.CoreV1().Pods(v1.NamespaceAll). - List(context.Background(), v1.ListOptions{ - LabelSelector: "app=deployment-validation-operator", - }) - if err != nil { - return "", fmt.Errorf("getting DVO pod from clientset: %w", err) - } - - for i := range pods.Items { - if strings.Contains(pods.Items[i].GetName(), dvoConfig.OperatorNamespace) { - return pods.Items[i].GetNamespace(), nil - } +func getPodNamespace() (string, error) { + namespace, exists := os.LookupEnv("POD_NAMESPACE") + if !exists { + return "", fmt.Errorf("could not find DVO pod") } - return "", fmt.Errorf("could not find DVO pod") + return namespace, nil } From 5fbf0c6f4296005f0730d216bf06dfa93bdcd882 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 31 Aug 2023 17:24:44 +0200 Subject: [PATCH 34/41] Add new logic for removing unexistent checks from configuration --- pkg/validations/validation_engine.go | 49 +++++++++++- pkg/validations/validation_engine_test.go | 92 +++++++++++++++++++++++ 2 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 pkg/validations/validation_engine_test.go diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index c8424e13..a91e4f3f 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -6,6 +6,7 @@ import ( _ "embed" // nolint:golint "fmt" "os" + "regexp" // Import checks from DVO @@ -134,7 +135,7 @@ func (ve *ValidationEngine) InitRegistry() error { return err } - enabledChecks, err := configresolver.GetEnabledChecksAndValidate(&ve.config, registry) + enabledChecks, err := ve.getValidChecks(registry) if err != nil { log.Error(err, "error finding enabled validations") return err @@ -196,6 +197,52 @@ func (ve *ValidationEngine) GetCheckByName(name string) (config.Check, error) { return check, nil } +// getValidChecks function fetches and validates the list of enabled checks from the ValidationEngine's +// configuration. It uses the provided check registry to validate the enabled checks against available checks. +// If any checks are found to be invalid (not present in the check registry), they are removed from the configuration. +// The function then recursively calls itself to fetch a new list of valid checks without the invalid ones. +func (ve *ValidationEngine) getValidChecks(registry checkregistry.CheckRegistry) ([]string, error) { + enabledChecks, err := configresolver.GetEnabledChecksAndValidate(&ve.config, registry) + if err != nil { + // error format from configresolver: + // "enabled checks validation error: [check \"check name\" not found, ...]"} + re := regexp.MustCompile(`check \"([^,]*)\" not found`) + if matches := re.FindAllStringSubmatch(err.Error(), -1); matches != nil { + for i := range matches { + log.Info("entered ConfigMap check was not validated and is ignored", + "validation name", matches[i][1], + ) + ve.removeCheckFromConfig(matches[i][1]) + } + return ve.getValidChecks(registry) + } + return []string{}, err + } + + return enabledChecks, nil +} + +// removeCheckFromConfig function searches for the given check name in both the "Include" and "Exclude" lists +// of checks in the ValidationEngine's configuration. If the check is found in either list, it is removed by updating +// the respective list. +func (ve *ValidationEngine) removeCheckFromConfig(check string) { + include := ve.config.Checks.Include + for i := 0; i < len(include); i++ { + if include[i] == check { + ve.config.Checks.Include = append(include[:i], include[i+1:]...) + return + } + } + + exclude := ve.config.Checks.Exclude + for i := 0; i < len(exclude); i++ { + if exclude[i] == check { + ve.config.Checks.Exclude = append(exclude[:i], exclude[i+1:]...) + return + } + } +} + // disableIncompatibleChecks will forcibly update a kube-linter config // to disable checks that are incompatible with DVO. // the same check name may end up in the exclude list multiple times as a result of this; this is OK. diff --git a/pkg/validations/validation_engine_test.go b/pkg/validations/validation_engine_test.go new file mode 100644 index 00000000..0ae57e96 --- /dev/null +++ b/pkg/validations/validation_engine_test.go @@ -0,0 +1,92 @@ +package validations + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "golang.stackrox.io/kube-linter/pkg/config" +) + +func TestGetValidChecks(t *testing.T) { + testCases := []struct { + name string + included []string + expected []string + }{ + { + name: "it returns only included checks", + included: []string{"host-network", "host-pid"}, + expected: []string{"host-network", "host-pid"}, + }, + { + name: "it returns only validated checks, and not the misspelled", + included: []string{"host-network", "host-pid", "misspelled", "wrong_format"}, + expected: []string{"host-network", "host-pid"}, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + // Given + mock := ValidationEngine{ + config: config.Config{ + Checks: config.ChecksConfig{ + DoNotAutoAddDefaults: true, + Include: testCase.included, + }, + }, + } + registry, _ := GetKubeLinterRegistry() + + // When + test, err := mock.getValidChecks(registry) + + // Assert + assert.Equal(t, testCase.expected, test) + assert.NoError(t, err) + }) + } +} + +func TestRemoveCheckFromConfig(t *testing.T) { + testCases := []struct { + name string + check string + cfg config.ChecksConfig + expected config.ChecksConfig + }{ + { + name: "function removes misspelled check from Include list", + check: "unset-something", + cfg: config.ChecksConfig{ + Include: []string{"host-network", "host-pid", "unset-something"}, + }, + expected: config.ChecksConfig{ + Include: []string{"host-network", "host-pid"}, + }, + }, + { + name: "function removes misspelled check from Exclude list", + check: "unset-something", + cfg: config.ChecksConfig{ + Exclude: []string{"host-network", "host-pid", "unset-something"}, + }, + expected: config.ChecksConfig{ + Exclude: []string{"host-network", "host-pid"}, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + // Given + mock := ValidationEngine{config: config.Config{Checks: testCase.cfg}} + + // When + mock.removeCheckFromConfig(testCase.check) + + // Assert + assert.Equal(t, mock.config.Checks, testCase.expected) + }) + } +} From 9e2dfeb7f35ed08871c7e68841249501dc50dec2 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 31 Aug 2023 17:38:17 +0200 Subject: [PATCH 35/41] Minor refactoring --- main.go | 2 +- pkg/controller/generic_reconciler.go | 4 ++-- pkg/validations/validation_engine.go | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/main.go b/main.go index cace9eb2..658c89f6 100644 --- a/main.go +++ b/main.go @@ -140,7 +140,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("initializing discovery client: %w", err) } - gr, err := controller.NewGenericReconciler(mgr.GetClient(), discoveryClient, cmWatcher) + gr, err := controller.NewGenericReconciler(mgr.GetClient(), discoveryClient, &cmWatcher) if err != nil { return nil, fmt.Errorf("initializing generic reconciler: %w", err) } diff --git a/pkg/controller/generic_reconciler.go b/pkg/controller/generic_reconciler.go index 1c96f6f3..779024dd 100644 --- a/pkg/controller/generic_reconciler.go +++ b/pkg/controller/generic_reconciler.go @@ -45,7 +45,7 @@ type GenericReconciler struct { func NewGenericReconciler( client client.Client, discovery discovery.DiscoveryInterface, - cmw configmap.Watcher, + cmw *configmap.Watcher, ) (*GenericReconciler, error) { listLimit, err := getListLimit() if err != nil { @@ -60,7 +60,7 @@ func NewGenericReconciler( objectValidationCache: newValidationCache(), currentObjects: newValidationCache(), logger: ctrl.Log.WithName("reconcile"), - cmWatcher: &cmw, + cmWatcher: cmw, }, nil } diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index a91e4f3f..3da4d21a 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -38,6 +38,7 @@ type ValidationEngine struct { // InitEngine creates a new ValidationEngine instance with the provided configuration path, a watcher, and metrics. // It initializes a ValidationEngine with the provided watcher for configmap changes and a set of preloaded metrics. // The engine's configuration is loaded from the specified configuration path, and its check registry is initialized. +// InitRegistry sets this instance in the package scope in engine variable. // // Parameters: // - configPath: The path to the configuration file for the ValidationEngine. From d5af598e9bad48ef6eec03c8434cd1d8cbaf4499 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 31 Aug 2023 17:44:44 +0200 Subject: [PATCH 36/41] Add missing documentation and fix tests --- pkg/controller/generic_reconciler.go | 4 +--- pkg/controller/generic_reconciler_test.go | 2 +- pkg/validations/utils.go | 3 +++ 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/controller/generic_reconciler.go b/pkg/controller/generic_reconciler.go index 779024dd..05f7c1a1 100644 --- a/pkg/controller/generic_reconciler.go +++ b/pkg/controller/generic_reconciler.go @@ -125,9 +125,7 @@ func (gr *GenericReconciler) LookForConfigUpdates(ctx context.Context) { for { select { case cfg := <-gr.cmWatcher.ConfigChanged(): - //ve.config = cfg - validations.UpdateConfig(cfg) // save previous configuration in case of rollback - //err := ve.InitRegistry() + validations.UpdateConfig(cfg) err := validations.InitRegistry() if err == nil { gr.objectValidationCache.drain() diff --git a/pkg/controller/generic_reconciler_test.go b/pkg/controller/generic_reconciler_test.go index 91464f19..f1b73c20 100644 --- a/pkg/controller/generic_reconciler_test.go +++ b/pkg/controller/generic_reconciler_test.go @@ -967,5 +967,5 @@ func createTestReconciler(scheme *runtime.Scheme, objects []client.Object) (*Gen } client := cliBuilder.Build() cli := kubefake.NewSimpleClientset() - return NewGenericReconciler(client, cli.Discovery(), configmap.Watcher{}) + return NewGenericReconciler(client, cli.Discovery(), &configmap.Watcher{}) } diff --git a/pkg/validations/utils.go b/pkg/validations/utils.go index ce41910a..9867012c 100644 --- a/pkg/validations/utils.go +++ b/pkg/validations/utils.go @@ -77,14 +77,17 @@ func newGaugeVecMetric(check klConfig.Check) *prometheus.GaugeVec { }, []string{"namespace_uid", "namespace", "uid", "name", "kind"}) } +// UpdateConfig provides an access to setup new configuration for the generic reconciler func UpdateConfig(cfg klConfig.Config) { engine.config = cfg } +// InitRegistry forces Validation Engine to initialize a new registry func InitRegistry() error { return engine.InitRegistry() } +// ResetMetrics resets all the metrics registered in the Validation Engine func ResetMetrics() { for _, metric := range engine.metrics { metric.Reset() From 9eeda44cf4c9f5217f9a8c445661ac091be03594 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 31 Aug 2023 17:47:27 +0200 Subject: [PATCH 37/41] Unexport validation engine structure as now it's not handled outside of the package --- pkg/validations/base_test.go | 8 +++---- pkg/validations/validation_engine.go | 26 +++++++++++------------ pkg/validations/validation_engine_test.go | 4 ++-- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/validations/base_test.go b/pkg/validations/base_test.go index 04e6de73..06ef24ae 100644 --- a/pkg/validations/base_test.go +++ b/pkg/validations/base_test.go @@ -28,13 +28,13 @@ const ( customCheckTemplate = "minimum-replicas" ) -func newEngine(c config.Config) (ValidationEngine, error) { - ve := ValidationEngine{ +func newEngine(c config.Config) (validationEngine, error) { + ve := validationEngine{ config: c, } loadErr := ve.InitRegistry() if loadErr != nil { - return ValidationEngine{}, loadErr + return validationEngine{}, loadErr } // checks now are preloaded, adding them after Registry init ve.metrics = make(map[string]*prometheus.GaugeVec) @@ -339,7 +339,7 @@ func stringInSlice(a string, list []string) bool { // getAllBuiltInKubeLinterChecks returns every check built-into kube-linter (including checks that DVO disables) func getAllBuiltInKubeLinterChecks() ([]string, error) { - ve := ValidationEngine{ + ve := validationEngine{ config: newEngineConfigWithAllChecks(), } registry := checkregistry.New() diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index 3da4d21a..b38fe11d 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -25,9 +25,9 @@ import ( "github.com/spf13/viper" ) -var engine ValidationEngine +var engine validationEngine -type ValidationEngine struct { +type validationEngine struct { config config.Config registry checkregistry.CheckRegistry enabledChecks []string @@ -48,7 +48,7 @@ type ValidationEngine struct { // Returns: // - An error if there's an issue loading the configuration or initializing the check registry. func InitEngine(configPath string, metrics map[string]*prometheus.GaugeVec) error { - ve := &ValidationEngine{ + ve := &validationEngine{ metrics: metrics, } @@ -65,11 +65,11 @@ func InitEngine(configPath string, metrics map[string]*prometheus.GaugeVec) erro return nil } -func (ve *ValidationEngine) CheckRegistry() checkregistry.CheckRegistry { +func (ve *validationEngine) CheckRegistry() checkregistry.CheckRegistry { return ve.registry } -func (ve *ValidationEngine) EnabledChecks() []string { +func (ve *validationEngine) EnabledChecks() []string { return ve.enabledChecks } @@ -82,7 +82,7 @@ func fileExists(filename string) bool { return !info.IsDir() } -func (ve *ValidationEngine) LoadConfig(path string) error { +func (ve *validationEngine) LoadConfig(path string) error { if !fileExists(path) { log.Info(fmt.Sprintf("config file %s does not exist. Use default configuration", path)) // TODO - This hardcode will be removed when a ConfigMap is set by default in regular installation @@ -123,7 +123,7 @@ type PrometheusRegistry interface { Register(prometheus.Collector) error } -func (ve *ValidationEngine) InitRegistry() error { +func (ve *validationEngine) InitRegistry() error { disableIncompatibleChecks(&ve.config) registry, err := GetKubeLinterRegistry() @@ -160,7 +160,7 @@ func (ve *ValidationEngine) InitRegistry() error { return nil } -func (ve *ValidationEngine) GetMetric(name string) *prometheus.GaugeVec { +func (ve *validationEngine) GetMetric(name string) *prometheus.GaugeVec { m, ok := ve.metrics[name] if !ok { return nil @@ -168,13 +168,13 @@ func (ve *ValidationEngine) GetMetric(name string) *prometheus.GaugeVec { return m } -func (ve *ValidationEngine) DeleteMetrics(labels prometheus.Labels) { +func (ve *validationEngine) DeleteMetrics(labels prometheus.Labels) { for _, vector := range ve.metrics { vector.Delete(labels) } } -func (ve *ValidationEngine) ClearMetrics(reports []diagnostic.WithContext, labels prometheus.Labels) { +func (ve *validationEngine) ClearMetrics(reports []diagnostic.WithContext, labels prometheus.Labels) { // Create a list of validation names for use to delete the labels from any // metric which isn't in the report but for which there is a metric reportValidationNames := map[string]struct{}{} @@ -190,7 +190,7 @@ func (ve *ValidationEngine) ClearMetrics(reports []diagnostic.WithContext, label } } -func (ve *ValidationEngine) GetCheckByName(name string) (config.Check, error) { +func (ve *validationEngine) GetCheckByName(name string) (config.Check, error) { check, ok := ve.registeredChecks[name] if !ok { return config.Check{}, fmt.Errorf("check '%s' is not registered", name) @@ -202,7 +202,7 @@ func (ve *ValidationEngine) GetCheckByName(name string) (config.Check, error) { // configuration. It uses the provided check registry to validate the enabled checks against available checks. // If any checks are found to be invalid (not present in the check registry), they are removed from the configuration. // The function then recursively calls itself to fetch a new list of valid checks without the invalid ones. -func (ve *ValidationEngine) getValidChecks(registry checkregistry.CheckRegistry) ([]string, error) { +func (ve *validationEngine) getValidChecks(registry checkregistry.CheckRegistry) ([]string, error) { enabledChecks, err := configresolver.GetEnabledChecksAndValidate(&ve.config, registry) if err != nil { // error format from configresolver: @@ -226,7 +226,7 @@ func (ve *ValidationEngine) getValidChecks(registry checkregistry.CheckRegistry) // removeCheckFromConfig function searches for the given check name in both the "Include" and "Exclude" lists // of checks in the ValidationEngine's configuration. If the check is found in either list, it is removed by updating // the respective list. -func (ve *ValidationEngine) removeCheckFromConfig(check string) { +func (ve *validationEngine) removeCheckFromConfig(check string) { include := ve.config.Checks.Include for i := 0; i < len(include); i++ { if include[i] == check { diff --git a/pkg/validations/validation_engine_test.go b/pkg/validations/validation_engine_test.go index 0ae57e96..f7983f22 100644 --- a/pkg/validations/validation_engine_test.go +++ b/pkg/validations/validation_engine_test.go @@ -28,7 +28,7 @@ func TestGetValidChecks(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { // Given - mock := ValidationEngine{ + mock := validationEngine{ config: config.Config{ Checks: config.ChecksConfig{ DoNotAutoAddDefaults: true, @@ -80,7 +80,7 @@ func TestRemoveCheckFromConfig(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { // Given - mock := ValidationEngine{config: config.Config{Checks: testCase.cfg}} + mock := validationEngine{config: config.Config{Checks: testCase.cfg}} // When mock.removeCheckFromConfig(testCase.check) From 994c69002f28b4d36324c05169b9118f0825a83a Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Mon, 11 Sep 2023 17:18:26 +0200 Subject: [PATCH 38/41] Add missing operator.yaml fix to get new namespace ENV variable --- deploy/openshift/operator.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/deploy/openshift/operator.yaml b/deploy/openshift/operator.yaml index 8f32e554..d869b7ea 100644 --- a/deploy/openshift/operator.yaml +++ b/deploy/openshift/operator.yaml @@ -86,6 +86,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace volumeMounts: - name: dvo-config mountPath: /config From 862556034971a4840575cd5530d648a45bb2d953 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Mon, 11 Sep 2023 17:51:33 +0200 Subject: [PATCH 39/41] Add delete scenario in the watcher --- pkg/configmap/configmap_watcher.go | 6 ++++++ pkg/validations/utils.go | 21 +++++++++++++++++++++ pkg/validations/validation_engine.go | 17 +---------------- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/pkg/configmap/configmap_watcher.go b/pkg/configmap/configmap_watcher.go index 69d1a3ce..2ef325d1 100644 --- a/pkg/configmap/configmap_watcher.go +++ b/pkg/configmap/configmap_watcher.go @@ -10,6 +10,7 @@ import ( "golang.stackrox.io/kube-linter/pkg/config" "gopkg.in/yaml.v3" + "github.com/app-sre/deployment-validation-operator/pkg/validations" "github.com/go-logr/logr" apicorev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -135,6 +136,11 @@ func (cmw Watcher) Start(ctx context.Context) error { cmw.ch <- cfg }, + DeleteFunc: func(_ interface{}) { + cmw.ch <- config.Config{ + Checks: validations.GetDefaultChecks(), + } + }, }) factory.Start(ctx.Done()) diff --git a/pkg/validations/utils.go b/pkg/validations/utils.go index 9867012c..f7bb9b53 100644 --- a/pkg/validations/utils.go +++ b/pkg/validations/utils.go @@ -93,3 +93,24 @@ func ResetMetrics() { metric.Reset() } } + +// GetDefaultChecks provides a default set of checks usable in case there is no custom ConfigMap +func GetDefaultChecks() klConfig.ChecksConfig { + return klConfig.ChecksConfig{ + DoNotAutoAddDefaults: true, + Include: []string{ + "host-ipc", + "host-network", + "host-pid", + "non-isolated-pod", + "pdb-max-unavailable", + "pdb-min-available", + "privilege-escalation-container", + "privileged-container", + "run-as-non-root", + "unsafe-sysctls", + "unset-cpu-requirements", + "unset-memory-requirements", + }, + } +} diff --git a/pkg/validations/validation_engine.go b/pkg/validations/validation_engine.go index b38fe11d..f5a5cefd 100644 --- a/pkg/validations/validation_engine.go +++ b/pkg/validations/validation_engine.go @@ -85,22 +85,7 @@ func fileExists(filename string) bool { func (ve *validationEngine) LoadConfig(path string) error { if !fileExists(path) { log.Info(fmt.Sprintf("config file %s does not exist. Use default configuration", path)) - // TODO - This hardcode will be removed when a ConfigMap is set by default in regular installation - ve.config.Checks.DoNotAutoAddDefaults = true - ve.config.Checks.Include = []string{ - "host-ipc", - "host-network", - "host-pid", - "non-isolated-pod", - "pdb-max-unavailable", - "pdb-min-available", - "privilege-escalation-container", - "privileged-container", - "run-as-non-root", - "unsafe-sysctls", - "unset-cpu-requirements", - "unset-memory-requirements", - } + ve.config.Checks = GetDefaultChecks() return nil } From 043a69f12fb7bbf283fdbdbb6f5700657dbd067a Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Mon, 11 Sep 2023 18:10:45 +0200 Subject: [PATCH 40/41] Minor refactoring --- main.go | 2 +- pkg/configmap/configmap_watcher.go | 21 ++++++++++++++------- pkg/configmap/configmap_watcher_test.go | 2 ++ 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/main.go b/main.go index 658c89f6..cace9eb2 100644 --- a/main.go +++ b/main.go @@ -140,7 +140,7 @@ func setupManager(log logr.Logger, opts options.Options) (manager.Manager, error return nil, fmt.Errorf("initializing discovery client: %w", err) } - gr, err := controller.NewGenericReconciler(mgr.GetClient(), discoveryClient, &cmWatcher) + gr, err := controller.NewGenericReconciler(mgr.GetClient(), discoveryClient, cmWatcher) if err != nil { return nil, fmt.Errorf("initializing generic reconciler: %w", err) } diff --git a/pkg/configmap/configmap_watcher.go b/pkg/configmap/configmap_watcher.go index 2ef325d1..263cdb10 100644 --- a/pkg/configmap/configmap_watcher.go +++ b/pkg/configmap/configmap_watcher.go @@ -43,7 +43,6 @@ type Watcher struct { } var configMapName = "deployment-validation-operator-config" -var configMapNamespace = "deployment-validation-operator" var configMapDataAccess = "deployment-validation-operator-config.yaml" // NewWatcher creates a new Watcher instance for observing changes to a ConfigMap. @@ -52,21 +51,21 @@ var configMapDataAccess = "deployment-validation-operator-config.yaml" // - cfg: A pointer to a rest.Config representing the Kubernetes client configuration. // // Returns: -// - A Watcher instance for monitoring changes to DVO ConfigMap resource if the initialization is successful. +// - A pointer to a Watcher instance for monitoring changes to DVO ConfigMap resource. // - An error if there's an issue while initializing the Kubernetes clientset. -func NewWatcher(cfg *rest.Config) (Watcher, error) { +func NewWatcher(cfg *rest.Config) (*Watcher, error) { clientset, err := kubernetes.NewForConfig(cfg) if err != nil { - return Watcher{}, fmt.Errorf("initializing clientset: %w", err) + return nil, fmt.Errorf("initializing clientset: %w", err) } // the Informer will use this to monitor the namespace for the ConfigMap. namespace, err := getPodNamespace() if err != nil { - return Watcher{}, fmt.Errorf("getting namespace: %w", err) + return nil, fmt.Errorf("getting namespace: %w", err) } - return Watcher{ + return &Watcher{ clientset: clientset, logger: log.Log.WithName("ConfigMapWatcher"), ch: make(chan config.Config), @@ -136,7 +135,15 @@ func (cmw Watcher) Start(ctx context.Context) error { cmw.ch <- cfg }, - DeleteFunc: func(_ interface{}) { + DeleteFunc: func(oldObj interface{}) { + cm := oldObj.(*apicorev1.ConfigMap) + + cmw.logger.Info( + "a ConfigMap has been deleted under watched namespace", + "name", cm.GetName(), + "namespace", cm.GetNamespace(), + ) + cmw.ch <- config.Config{ Checks: validations.GetDefaultChecks(), } diff --git a/pkg/configmap/configmap_watcher_test.go b/pkg/configmap/configmap_watcher_test.go index 1c64d6cb..56daf852 100644 --- a/pkg/configmap/configmap_watcher_test.go +++ b/pkg/configmap/configmap_watcher_test.go @@ -13,6 +13,8 @@ import ( ) func TestStaticConfigMapWatcher(t *testing.T) { + var configMapNamespace = "deployment-validation-operator" + testCases := []struct { name string data string From 4f6c8fe64acd561b5041b2695f83c087a4064bde Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Tue, 12 Sep 2023 16:14:52 +0200 Subject: [PATCH 41/41] Bundle regenerated with missing env variable --- bundle.Dockerfile | 2 +- ...loyment-validation-operator.clusterserviceversion.yaml | 8 ++++++-- bundle/metadata/annotations.yaml | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/bundle.Dockerfile b/bundle.Dockerfile index 4ea891e8..d9c0a88e 100644 --- a/bundle.Dockerfile +++ b/bundle.Dockerfile @@ -6,7 +6,7 @@ LABEL operators.operatorframework.io.bundle.manifests.v1=manifests/ LABEL operators.operatorframework.io.bundle.metadata.v1=metadata/ LABEL operators.operatorframework.io.bundle.package.v1=deployment-validation-operator LABEL operators.operatorframework.io.bundle.channels.v1=alpha -LABEL operators.operatorframework.io.metrics.builder=operator-sdk-v1.28.1 +LABEL operators.operatorframework.io.metrics.builder=operator-sdk-v1.31.0+git LABEL operators.operatorframework.io.metrics.mediatype.v1=metrics+v1 LABEL operators.operatorframework.io.metrics.project_layout=unknown diff --git a/bundle/manifests/deployment-validation-operator.clusterserviceversion.yaml b/bundle/manifests/deployment-validation-operator.clusterserviceversion.yaml index 24507bb2..f91d9e6a 100644 --- a/bundle/manifests/deployment-validation-operator.clusterserviceversion.yaml +++ b/bundle/manifests/deployment-validation-operator.clusterserviceversion.yaml @@ -4,8 +4,8 @@ metadata: annotations: alm-examples: '[]' capabilities: Basic Install - createdAt: "2023-08-24T07:58:38Z" - operators.operatorframework.io/builder: operator-sdk-v1.28.1 + createdAt: "2023-09-12T14:13:09Z" + operators.operatorframework.io/builder: operator-sdk-v1.31.0+git operators.operatorframework.io/project_layout: unknown name: deployment-validation-operator.v0.0.0 namespace: placeholder @@ -82,6 +82,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace image: quay.io/deployment-validation-operator/dv-operator:latest imagePullPolicy: Always livenessProbe: diff --git a/bundle/metadata/annotations.yaml b/bundle/metadata/annotations.yaml index a6ce8a57..7831d323 100644 --- a/bundle/metadata/annotations.yaml +++ b/bundle/metadata/annotations.yaml @@ -5,6 +5,6 @@ annotations: operators.operatorframework.io.bundle.metadata.v1: metadata/ operators.operatorframework.io.bundle.package.v1: deployment-validation-operator operators.operatorframework.io.bundle.channels.v1: alpha - operators.operatorframework.io.metrics.builder: operator-sdk-v1.28.1 + operators.operatorframework.io.metrics.builder: operator-sdk-v1.31.0+git operators.operatorframework.io.metrics.mediatype.v1: metrics+v1 operators.operatorframework.io.metrics.project_layout: unknown