From caa769fb1db5fcad6a8c3621df06a1fbc48ef52a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Mon, 23 May 2022 16:39:12 +0200 Subject: [PATCH] refactor: clean updaterequest generator (#3949) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: clean updaterequest generator Signed-off-by: Charles-Edouard Brétéché * refactor: clean updaterequest generator Signed-off-by: Charles-Edouard Brétéché Co-authored-by: shuting --- cmd/kyverno/main.go | 14 +- pkg/webhooks/resource/handlers.go | 4 +- pkg/webhooks/resource/utils.go | 2 +- pkg/webhooks/updaterequest/generator.go | 264 +++++++++--------------- pkg/webhooks/updaterequest/log.go | 5 + 5 files changed, 114 insertions(+), 175 deletions(-) create mode 100644 pkg/webhooks/updaterequest/log.go diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index eb7d5cd3bf..d173c1678a 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -154,6 +154,7 @@ func main() { // utils kyvernoV1 := kyvernoInformer.Kyverno().V1() + kyvernoV1beta1 := kyvernoInformer.Kyverno().V1beta1() kyvernoV1alpha2 := kyvernoInformer.Kyverno().V1alpha2() // load image registry secrets @@ -264,7 +265,7 @@ func main() { dynamicClient, kyvernoV1.ClusterPolicies(), kyvernoV1.Policies(), - kyvernoInformer.Kyverno().V1beta1().UpdateRequests(), + kyvernoV1beta1.UpdateRequests(), configuration, eventGenerator, reportReqGen, @@ -279,10 +280,7 @@ func main() { os.Exit(1) } - urgen := webhookgenerate.NewGenerator(kyvernoClient, - kyvernoInformer.Kyverno().V1beta1().UpdateRequests(), - stopCh, - log.Log.WithName("UpdateRequestGenerator")) + urgen := webhookgenerate.NewGenerator(kyvernoClient, kyvernoV1beta1.UpdateRequests()) urc := background.NewController( kubeClient, @@ -290,7 +288,7 @@ func main() { dynamicClient, kyvernoV1.ClusterPolicies(), kyvernoV1.Policies(), - kyvernoInformer.Kyverno().V1beta1().UpdateRequests(), + kyvernoV1beta1.UpdateRequests(), kubeInformer.Core().V1().Namespaces(), kubeInformer.Core().V1().Pods(), eventGenerator, @@ -303,7 +301,7 @@ func main() { dynamicClient, kyvernoV1.ClusterPolicies(), kyvernoV1.Policies(), - kyvernoInformer.Kyverno().V1beta1().UpdateRequests(), + kyvernoV1beta1.UpdateRequests(), kubeInformer.Core().V1().Namespaces(), log.Log.WithName("GenerateCleanUpController"), ) @@ -413,7 +411,7 @@ func main() { kubeInformer.Core().V1().Namespaces().Lister(), kubeInformer.Rbac().V1().RoleBindings().Lister(), kubeInformer.Rbac().V1().ClusterRoleBindings().Lister(), - kyvernoInformer.Kyverno().V1beta1().UpdateRequests().Lister().UpdateRequests(config.KyvernoNamespace()), + kyvernoV1beta1.UpdateRequests().Lister().UpdateRequests(config.KyvernoNamespace()), reportReqGen, urgen, eventGenerator, diff --git a/pkg/webhooks/resource/handlers.go b/pkg/webhooks/resource/handlers.go index 350d117a61..44fc588e42 100644 --- a/pkg/webhooks/resource/handlers.go +++ b/pkg/webhooks/resource/handlers.go @@ -56,7 +56,7 @@ type handlers struct { urLister kyvernov1beta1listers.UpdateRequestNamespaceLister prGenerator policyreport.GeneratorInterface - urGenerator webhookgenerate.Interface + urGenerator webhookgenerate.Generator eventGen event.Interface auditHandler AuditHandler openAPIController *openapi.Controller @@ -73,7 +73,7 @@ func NewHandlers( crbLister rbacv1listers.ClusterRoleBindingLister, urLister kyvernov1beta1listers.UpdateRequestNamespaceLister, prGenerator policyreport.GeneratorInterface, - urGenerator webhookgenerate.Interface, + urGenerator webhookgenerate.Generator, eventGen event.Interface, auditHandler AuditHandler, openAPIController *openapi.Controller, diff --git a/pkg/webhooks/resource/utils.go b/pkg/webhooks/resource/utils.go index ed3b9ade4f..8b73e536cc 100644 --- a/pkg/webhooks/resource/utils.go +++ b/pkg/webhooks/resource/utils.go @@ -315,7 +315,7 @@ func stripNonPolicyFields(obj, newRes map[string]interface{}, logger logr.Logger return obj, newRes } -func applyUpdateRequest(request *admissionv1.AdmissionRequest, ruleType kyvernov1beta1.RequestType, grGenerator updaterequest.Interface, userRequestInfo kyvernov1beta1.RequestInfo, +func applyUpdateRequest(request *admissionv1.AdmissionRequest, ruleType kyvernov1beta1.RequestType, grGenerator updaterequest.Generator, userRequestInfo kyvernov1beta1.RequestInfo, action admissionv1.Operation, engineResponses ...*response.EngineResponse, ) (failedUpdateRequest []updateRequestResponse) { requestBytes, err := json.Marshal(request) diff --git a/pkg/webhooks/updaterequest/generator.go b/pkg/webhooks/updaterequest/generator.go index 0c870ba92a..d37583a7ce 100644 --- a/pkg/webhooks/updaterequest/generator.go +++ b/pkg/webhooks/updaterequest/generator.go @@ -5,10 +5,7 @@ import ( "time" backoff "github.com/cenkalti/backoff" - "github.com/gardener/controller-manager-library/pkg/logger" - "github.com/go-logr/logr" kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" - "github.com/kyverno/kyverno/pkg/background/common" kyvernoclient "github.com/kyverno/kyverno/pkg/client/clientset/versioned" kyvernov1beta1informers "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v1beta1" kyvernov1beta1listers "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1beta1" @@ -16,190 +13,46 @@ import ( admissionv1 "k8s.io/api/admission/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/util/retry" ) -// UpdateRequest provides interface to manage update requests -type Interface interface { +// Generator provides interface to manage update requests +type Generator interface { Apply(gr kyvernov1beta1.UpdateRequestSpec, action admissionv1.Operation) error } -// info object stores message data to create update request -type info struct { - spec kyvernov1beta1.UpdateRequestSpec - action admissionv1.Operation -} - -// Generator defines the implementation to mange update request resource -type Generator struct { +// generator defines the implementation to manage update request resource +type generator struct { + // clients client kyvernoclient.Interface - stopCh <-chan struct{} - log logr.Logger + // listers urLister kyvernov1beta1listers.UpdateRequestNamespaceLister } // NewGenerator returns a new instance of UpdateRequest resource generator -func NewGenerator(client kyvernoclient.Interface, urInformer kyvernov1beta1informers.UpdateRequestInformer, stopCh <-chan struct{}, log logr.Logger) *Generator { - gen := &Generator{ +func NewGenerator(client kyvernoclient.Interface, urInformer kyvernov1beta1informers.UpdateRequestInformer) Generator { + return &generator{ client: client, - stopCh: stopCh, - log: log, urLister: urInformer.Lister().UpdateRequests(config.KyvernoNamespace()), } - return gen } // Apply creates update request resource -func (g *Generator) Apply(ur kyvernov1beta1.UpdateRequestSpec, action admissionv1.Operation) error { - logger := g.log +func (g *generator) Apply(ur kyvernov1beta1.UpdateRequestSpec, action admissionv1.Operation) error { logger.V(4).Info("reconcile Update Request", "request", ur) - - message := info{ - action: action, - spec: ur, - } - go g.processApply(message) - return nil -} - -// Run starts the update request spec -func (g *Generator) Run(workers int, stopCh <-chan struct{}) { - logger := g.log - defer utilruntime.HandleCrash() - - logger.V(4).Info("starting") - defer func() { - logger.V(4).Info("shutting down") - }() - - <-g.stopCh -} - -func (g *Generator) processApply(i info) { - if err := g.generate(i); err != nil { - logger.Error(err, "failed to update request CR") - } -} - -func (g *Generator) generate(i info) error { - if err := retryApplyResource(g.client, i.spec, g.log, i.action, g.urLister); err != nil { - return err - } - return nil -} - -func retryApplyResource( - client kyvernoclient.Interface, - urSpec kyvernov1beta1.UpdateRequestSpec, - log logr.Logger, - action admissionv1.Operation, - urLister kyvernov1beta1listers.UpdateRequestNamespaceLister, -) error { - if action == admissionv1.Delete && urSpec.Type == kyvernov1beta1.Generate { + if action == admissionv1.Delete && ur.Type == kyvernov1beta1.Generate { return nil } - - var i int - var err error - - _, policyName, err := cache.SplitMetaNamespaceKey(urSpec.Policy) + _, policyName, err := cache.SplitMetaNamespaceKey(ur.Policy) if err != nil { return err } + go g.applyResource(policyName, ur) + return nil +} - applyResource := func() error { - ur := kyvernov1beta1.UpdateRequest{ - Spec: urSpec, - Status: kyvernov1beta1.UpdateRequestStatus{ - State: kyvernov1beta1.Pending, - }, - } - - queryLabels := make(map[string]string) - if ur.Spec.Type == kyvernov1beta1.Mutate { - queryLabels := map[string]string{ - kyvernov1beta1.URMutatePolicyLabel: ur.Spec.Policy, - "mutate.updaterequest.kyverno.io/trigger-name": ur.Spec.Resource.Name, - "mutate.updaterequest.kyverno.io/trigger-namespace": ur.Spec.Resource.Namespace, - "mutate.updaterequest.kyverno.io/trigger-kind": ur.Spec.Resource.Kind, - } - - if ur.Spec.Resource.APIVersion != "" { - queryLabels["mutate.updaterequest.kyverno.io/trigger-apiversion"] = ur.Spec.Resource.APIVersion - } - } else if ur.Spec.Type == kyvernov1beta1.Generate { - queryLabels = labels.Set(map[string]string{ - kyvernov1beta1.URGeneratePolicyLabel: policyName, - "generate.kyverno.io/resource-name": urSpec.Resource.Name, - "generate.kyverno.io/resource-kind": urSpec.Resource.Kind, - "generate.kyverno.io/resource-namespace": urSpec.Resource.Namespace, - }) - } - - ur.SetNamespace(config.KyvernoNamespace()) - isExist := false - log.V(4).Info("apply UpdateRequest", "ruleType", ur.Spec.Type) - - urList, err := urLister.List(labels.SelectorFromSet(queryLabels)) - if err != nil { - log.Error(err, "failed to get update request for the resource", "kind", urSpec.Resource.Kind, "name", urSpec.Resource.Name, "namespace", urSpec.Resource.Namespace) - return err - } - - for _, v := range urList { - log.V(4).Info("updating existing update request", "name", v.GetName()) - - v.Spec.Context = ur.Spec.Context - v.Spec.Policy = ur.Spec.Policy - v.Spec.Resource = ur.Spec.Resource - v.Status.Message = "" - - new, err := client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Update(context.TODO(), v, metav1.UpdateOptions{}) - if err != nil { - log.V(4).Info("failed to update UpdateRequest, retrying", "retryCount", i, "name", ur.GetName(), "namespace", ur.GetNamespace(), "err", err.Error()) - i++ - return err - } else { - log.V(4).Info("successfully updated UpdateRequest", "retryCount", i, "name", ur.GetName(), "namespace", ur.GetNamespace()) - } - err = retry.RetryOnConflict(common.DefaultRetry, func() error { - ur, err := client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Get(context.TODO(), new.GetName(), metav1.GetOptions{}) - if err != nil { - return err - } - ur.Status.State = kyvernov1beta1.Pending - _, err = client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).UpdateStatus(context.TODO(), ur, metav1.UpdateOptions{}) - return err - }) - if err != nil { - log.Error(err, "failed to set UpdateRequest state to Pending") - return err - } - isExist = true - } - - if !isExist { - log.V(4).Info("creating new UpdateRequest", "type", ur.Spec.Type) - - ur.SetGenerateName("ur-") - ur.SetLabels(queryLabels) - - new, err := client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Create(context.TODO(), &ur, metav1.CreateOptions{}) - if err != nil { - log.V(4).Info("failed to create UpdateRequest, retrying", "retryCount", i, "name", ur.GetGenerateName(), "namespace", ur.GetNamespace(), "err", err.Error()) - i++ - return err - } else { - log.V(4).Info("successfully created UpdateRequest", "retryCount", i, "name", new.GetName(), "namespace", ur.GetNamespace()) - } - } - - return nil - } - +func (g *generator) applyResource(policyName string, urSpec kyvernov1beta1.UpdateRequestSpec) { exbackoff := &backoff.ExponentialBackOff{ InitialInterval: 500 * time.Millisecond, RandomizationFactor: 0.5, @@ -208,13 +61,96 @@ func retryApplyResource( MaxElapsedTime: 3 * time.Second, Clock: backoff.SystemClock, } - exbackoff.Reset() - err = backoff.Retry(applyResource, exbackoff) + if err := backoff.Retry(func() error { return g.tryApplyResource(policyName, urSpec) }, exbackoff); err != nil { + logger.Error(err, "failed to update request CR") + } +} +func (g *generator) tryApplyResource(policyName string, urSpec kyvernov1beta1.UpdateRequestSpec) error { + ur := kyvernov1beta1.UpdateRequest{ + Spec: urSpec, + Status: kyvernov1beta1.UpdateRequestStatus{ + State: kyvernov1beta1.Pending, + }, + } + + queryLabels := make(map[string]string) + if ur.Spec.Type == kyvernov1beta1.Mutate { + queryLabels := map[string]string{ + kyvernov1beta1.URMutatePolicyLabel: ur.Spec.Policy, + "mutate.updaterequest.kyverno.io/trigger-name": ur.Spec.Resource.Name, + "mutate.updaterequest.kyverno.io/trigger-namespace": ur.Spec.Resource.Namespace, + "mutate.updaterequest.kyverno.io/trigger-kind": ur.Spec.Resource.Kind, + } + + if ur.Spec.Resource.APIVersion != "" { + queryLabels["mutate.updaterequest.kyverno.io/trigger-apiversion"] = ur.Spec.Resource.APIVersion + } + } else if ur.Spec.Type == kyvernov1beta1.Generate { + queryLabels = labels.Set(map[string]string{ + kyvernov1beta1.URGeneratePolicyLabel: policyName, + "generate.kyverno.io/resource-name": urSpec.Resource.Name, + "generate.kyverno.io/resource-kind": urSpec.Resource.Kind, + "generate.kyverno.io/resource-namespace": urSpec.Resource.Namespace, + }) + } + + ur.SetNamespace(config.KyvernoNamespace()) + isExist := false + logger.V(4).Info("apply UpdateRequest", "ruleType", ur.Spec.Type) + + urList, err := g.urLister.List(labels.SelectorFromSet(queryLabels)) if err != nil { + logger.Error(err, "failed to get update request for the resource", "kind", urSpec.Resource.Kind, "name", urSpec.Resource.Name, "namespace", urSpec.Resource.Namespace) return err } + for _, v := range urList { + logger.V(4).Info("updating existing update request", "name", v.GetName()) + + v.Spec.Context = ur.Spec.Context + v.Spec.Policy = ur.Spec.Policy + v.Spec.Resource = ur.Spec.Resource + v.Status.Message = "" + + new, err := g.client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Update(context.TODO(), v, metav1.UpdateOptions{}) + if err != nil { + logger.V(4).Info("failed to update UpdateRequest, retrying", "name", ur.GetName(), "namespace", ur.GetNamespace(), "err", err.Error()) + return err + } else { + logger.V(4).Info("successfully updated UpdateRequest", "name", ur.GetName(), "namespace", ur.GetNamespace()) + } + + new.Status.State = kyvernov1beta1.Pending + if _, err := g.client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).UpdateStatus(context.TODO(), new, metav1.UpdateOptions{}); err != nil { + logger.Error(err, "failed to set UpdateRequest state to Pending") + return err + } + + isExist = true + } + + if !isExist { + logger.V(4).Info("creating new UpdateRequest", "type", ur.Spec.Type) + + ur.SetGenerateName("ur-") + ur.SetLabels(queryLabels) + + new, err := g.client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Create(context.TODO(), &ur, metav1.CreateOptions{}) + if err != nil { + logger.V(4).Info("failed to create UpdateRequest, retrying", "name", ur.GetGenerateName(), "namespace", ur.GetNamespace(), "err", err.Error()) + return err + } else { + logger.V(4).Info("successfully created UpdateRequest", "name", new.GetName(), "namespace", ur.GetNamespace()) + } + + new.Status.State = kyvernov1beta1.Pending + if _, err := g.client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).UpdateStatus(context.TODO(), new, metav1.UpdateOptions{}); err != nil { + logger.Error(err, "failed to set UpdateRequest state to Pending") + return err + } + } + return nil } diff --git a/pkg/webhooks/updaterequest/log.go b/pkg/webhooks/updaterequest/log.go new file mode 100644 index 0000000000..b0dc6feca2 --- /dev/null +++ b/pkg/webhooks/updaterequest/log.go @@ -0,0 +1,5 @@ +package updaterequest + +import "sigs.k8s.io/controller-runtime/pkg/log" + +var logger = log.Log.WithName("updaterequest-generator")