From c39c096de8fe4c8431ed9fc0752ccecb5c952fe0 Mon Sep 17 00:00:00 2001 From: mprahl Date: Wed, 4 Jan 2023 11:07:26 -0500 Subject: [PATCH] Use a single dynamic client in the reconciler This makes the code simpler and removes the overhead of creating separate copies of API configs and dynamic clients on each object being evaluated. Signed-off-by: mprahl --- controllers/configurationpolicy_controller.go | 93 ++++++++----------- main.go | 23 +++-- 2 files changed, 52 insertions(+), 64 deletions(-) diff --git a/controllers/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go index fd9cbd6b..22c1016f 100644 --- a/controllers/configurationpolicy_controller.go +++ b/controllers/configurationpolicy_controller.go @@ -107,8 +107,9 @@ type ConfigurationPolicyReconciler struct { InstanceName string // The Kubernetes client to use when evaluating/enforcing policies. Most times, this will be the same cluster // where the controller is running. - TargetK8sClient kubernetes.Interface - TargetK8sConfig *rest.Config + TargetK8sClient kubernetes.Interface + TargetK8sDynamicClient dynamic.Interface + TargetK8sConfig *rest.Config // Whether custom metrics collection is enabled EnableMetrics bool discoveryInfo @@ -471,8 +472,6 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(plc policyv1.Configu continue } - dclient, gvr := r.getResourceAndDynamicClient(mapping) - namespaced := object.Object.Metadata.Namespace != "" // determine whether object should be deleted @@ -482,8 +481,9 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(plc policyv1.Configu namespaced, object.Object.Metadata.Namespace, object.Object.Metadata.Name, - gvr, - dclient) + mapping.Resource, + r.TargetK8sDynamicClient, + ) // object does not exist, no deletion logic needed if existing == nil { @@ -524,9 +524,9 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(plc policyv1.Configu var res dynamic.ResourceInterface if namespaced { - res = dclient.Resource(gvr).Namespace(object.Object.Metadata.Namespace) + res = r.TargetK8sDynamicClient.Resource(mapping.Resource).Namespace(object.Object.Metadata.Namespace) } else { - res = dclient.Resource(gvr) + res = r.TargetK8sDynamicClient.Resource(mapping.Resource) } if completed, err := deleteObject(res, object.Object.Metadata.Name, @@ -540,8 +540,9 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(plc policyv1.Configu namespaced, object.Object.Metadata.Namespace, object.Object.Metadata.Name, - gvr, - dclient) + mapping.Resource, + r.TargetK8sDynamicClient, + ) if obj != nil { log.Error(err, "Error: tried to delete object, but delete is hanging") @@ -1251,8 +1252,6 @@ func (r *ConfigurationPolicyReconciler) handleObjects( namespace, objDetails.namespace)) } - dclient, rsrc := r.getResourceAndDynamicClient(mapping) - if objDetails.isNamespaced && namespace == "" { objName := objDetails.name kindWithoutNS := objDetails.kind @@ -1286,7 +1285,9 @@ func (r *ConfigurationPolicyReconciler) handleObjects( if objDetails.name != "" { // named object, so checking just for the existence of the specific object // If the object couldn't be retrieved, this will be handled later on. - object, _ = getObject(objDetails.isNamespaced, namespace, objDetails.name, rsrc, dclient) + object, _ = getObject( + objDetails.isNamespaced, namespace, objDetails.name, mapping.Resource, r.TargetK8sDynamicClient, + ) exists = object != nil @@ -1295,8 +1296,17 @@ func (r *ConfigurationPolicyReconciler) handleObjects( log.V(1).Info( "The object template does not specify a name. Will search for matching objects in the namespace.", ) - objNames = append(objNames, getNamesOfKind(unstruct, rsrc, objDetails.isNamespaced, - namespace, dclient, strings.ToLower(string(objectT.ComplianceType)))...) + objNames = append( + objNames, + getNamesOfKind( + unstruct, + mapping.Resource, + objDetails.isNamespaced, + namespace, + r.TargetK8sDynamicClient, + strings.ToLower(string(objectT.ComplianceType)), + )..., + ) // we do not support enforce on unnamed templates if !strings.EqualFold(string(remediation), "inform") { @@ -1313,13 +1323,13 @@ func (r *ConfigurationPolicyReconciler) handleObjects( } objShouldExist := !strings.EqualFold(string(objectT.ComplianceType), string(policyv1.MustNotHave)) - rsrcKind = rsrc.Resource + rsrcKind = mapping.Resource.Resource if len(objNames) == 1 { name := objNames[0] singObj := singleObject{ policy: policy, - gvr: rsrc, + gvr: mapping.Resource, object: object, name: name, namespace: namespace, @@ -1334,14 +1344,14 @@ func (r *ConfigurationPolicyReconciler) handleObjects( var creationInfo *policyv1.ObjectProperties objNames, compliant, rsrcKind, statusUpdateNeeded, creationInfo = r.handleSingleObj( - singObj, remediation, exists, dclient, objectT, + singObj, remediation, exists, objectT, ) // The message string for single objects is different than for multiple reason = generateSingleObjReason(objShouldExist, compliant, exists) // Enforce could clear the objNames array so use name instead relatedObjects = addRelatedObjects( compliant, - rsrc, + mapping.Resource, objDetails.kind, namespace, objDetails.isNamespaced, @@ -1370,7 +1380,7 @@ func (r *ConfigurationPolicyReconciler) handleObjects( relatedObjects = addRelatedObjects( compliant, - rsrc, + mapping.Resource, objDetails.kind, namespace, objDetails.isNamespaced, @@ -1424,7 +1434,6 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( obj singleObject, remediation policyv1.RemediationAction, exists bool, - dclient dynamic.Interface, objectT *policyv1.ObjectTemplate, ) ( objNameList []string, @@ -1448,7 +1457,7 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( // it is a musthave and it does not exist, so it must be created if strings.EqualFold(string(remediation), string(policyv1.Enforce)) { var uid string - statusUpdateNeeded, uid, err = r.enforceByCreatingOrDeleting(obj, dclient) + statusUpdateNeeded, uid, err = r.enforceByCreatingOrDeleting(obj) if err != nil { // violation created for handling error @@ -1468,7 +1477,7 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( if exists && !obj.shouldExist { // it is a mustnothave but it exist, so it must be deleted if strings.EqualFold(string(remediation), string(policyv1.Enforce)) { - statusUpdateNeeded, _, err = r.enforceByCreatingOrDeleting(obj, dclient) + statusUpdateNeeded, _, err = r.enforceByCreatingOrDeleting(obj) if err != nil { objLog.Error(err, "Could not handle existing mustnothave object") } @@ -1502,7 +1511,7 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( before := time.Now().UTC() - throwSpecViolation, msg, pErr := r.checkAndUpdateResource(obj, compType, mdCompType, remediation, dclient) + throwSpecViolation, msg, pErr := r.checkAndUpdateResource(obj, compType, mdCompType, remediation) duration := time.Now().UTC().Sub(before) seconds := float64(duration) / float64(time.Second) @@ -1609,29 +1618,6 @@ func (r *ConfigurationPolicyReconciler) isObjectNamespaced( return false } -// getResourceAndDynamicClient creates a dynamic client to query resources and pulls the groupVersionResource -// for an object from its mapping -func (r *ConfigurationPolicyReconciler) getResourceAndDynamicClient(mapping *meta.RESTMapping) ( - dclient dynamic.Interface, rsrc schema.GroupVersionResource, -) { - restconfig := rest.CopyConfig(r.TargetK8sConfig) - restconfig.GroupVersion = &schema.GroupVersion{ - Group: mapping.GroupVersionKind.Group, - Version: mapping.GroupVersionKind.Version, - } - - dclient, err := dynamic.NewForConfig(restconfig) - if err != nil { - log.Error(err, "Could not get dynamic client from config", "config", restconfig) - os.Exit(1) - } - - // check all resources in the list of resources on the cluster to get a match for the mapping - rsrc = mapping.Resource - - return dclient, rsrc -} - // getMapping takes in a raw object, decodes it, and maps it to an existing group/kind func (r *ConfigurationPolicyReconciler) getMapping( ext runtime.RawExtension, @@ -1817,9 +1803,7 @@ func getNamesOfKind( // completely missing (as opposed to existing, but not matching the desired state), or where a // mustnothave object does exist. Eg, it does not handle the case where a targeted update would need // to be made to an object. -func (r *ConfigurationPolicyReconciler) enforceByCreatingOrDeleting( - obj singleObject, dclient dynamic.Interface, -) ( +func (r *ConfigurationPolicyReconciler) enforceByCreatingOrDeleting(obj singleObject) ( result bool, uid string, erro error, ) { log := log.WithValues( @@ -1832,9 +1816,9 @@ func (r *ConfigurationPolicyReconciler) enforceByCreatingOrDeleting( var res dynamic.ResourceInterface if obj.namespaced { - res = dclient.Resource(obj.gvr).Namespace(obj.namespace) + res = r.TargetK8sDynamicClient.Resource(obj.gvr).Namespace(obj.namespace) } else { - res = dclient.Resource(obj.gvr) + res = r.TargetK8sDynamicClient.Resource(obj.gvr) } var completed bool @@ -2338,7 +2322,6 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( complianceType string, mdComplianceType string, remediation policyv1.RemediationAction, - dclient dynamic.Interface, ) (throwSpecViolation bool, message string, processingErr bool) { log := log.WithValues( "policy", obj.policy.Name, "name", obj.name, "namespace", obj.namespace, "resource", obj.gvr.Resource, @@ -2352,9 +2335,9 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( var res dynamic.ResourceInterface if obj.namespaced { - res = dclient.Resource(obj.gvr).Namespace(obj.namespace) + res = r.TargetK8sDynamicClient.Resource(obj.gvr).Namespace(obj.namespace) } else { - res = dclient.Resource(obj.gvr) + res = r.TargetK8sDynamicClient.Resource(obj.gvr) } var err error diff --git a/main.go b/main.go index ada205f7..52a1f2ae 100644 --- a/main.go +++ b/main.go @@ -22,6 +22,7 @@ import ( extensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" k8sruntime "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" clientgoscheme "k8s.io/client-go/kubernetes/scheme" _ "k8s.io/client-go/plugin/pkg/client/auth" @@ -225,11 +226,13 @@ func main() { } var targetK8sClient kubernetes.Interface + var targetK8sDynamicClient dynamic.Interface var targetK8sConfig *rest.Config if targetKubeConfig == "" { targetK8sConfig = cfg targetK8sClient = kubernetes.NewForConfigOrDie(targetK8sConfig) + targetK8sDynamicClient = dynamic.NewForConfigOrDie(targetK8sConfig) } else { var err error @@ -240,6 +243,7 @@ func main() { } targetK8sClient = kubernetes.NewForConfigOrDie(targetK8sConfig) + targetK8sDynamicClient = dynamic.NewForConfigOrDie(targetK8sConfig) log.Info( "Overrode the target Kubernetes cluster for policy evaluation and enforcement", "path", targetKubeConfig, @@ -249,15 +253,16 @@ func main() { instanceName, _ := os.Hostname() // on an error, instanceName will be empty, which is ok reconciler := controllers.ConfigurationPolicyReconciler{ - Client: mgr.GetClient(), - DecryptionConcurrency: decryptionConcurrency, - EvaluationConcurrency: evaluationConcurrency, - Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor(controllers.ControllerName), - InstanceName: instanceName, - TargetK8sClient: targetK8sClient, - TargetK8sConfig: targetK8sConfig, - EnableMetrics: enableMetrics, + Client: mgr.GetClient(), + DecryptionConcurrency: decryptionConcurrency, + EvaluationConcurrency: evaluationConcurrency, + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor(controllers.ControllerName), + InstanceName: instanceName, + TargetK8sClient: targetK8sClient, + TargetK8sDynamicClient: targetK8sDynamicClient, + TargetK8sConfig: targetK8sConfig, + EnableMetrics: enableMetrics, } if err = reconciler.SetupWithManager(mgr); err != nil { log.Error(err, "Unable to create controller", "controller", "ConfigurationPolicy")