From e788bf0c95f71c2a6c59cda92079780194cebd32 Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Tue, 26 Sep 2023 10:49:33 +0000 Subject: [PATCH] feat: remove the creation of cronjobs in cleanup controller (#8526) (#8528) * feat: remove the creation of cronjobs in cleanup controller * fix: use lastExecutionTime instead of nextExecutionTime --------- Signed-off-by: Mariam Fahmy Co-authored-by: Mariam Fahmy --- api/kyverno/v2alpha1/cleanup_policy_types.go | 3 +- api/kyverno/v2alpha1/zz_generated.deepcopy.go | 1 + charts/kyverno/templates/crds/crds.yaml | 6 + .../handlers/cleanup/handlers.go | 262 ----------- cmd/cleanup-controller/main.go | 92 ++-- cmd/cleanup-controller/server.go | 26 -- config/crds/kyverno.io_cleanuppolicies.yaml | 3 + .../kyverno.io_clustercleanuppolicies.yaml | 3 + config/install-latest-testing.yaml | 6 + docs/user/crd/index.html | 12 + go.mod | 1 + go.sum | 2 + .../kyverno/v2alpha1/cleanuppolicystatus.go | 11 +- .../controllers}/cleanup/condition.go | 0 .../controllers}/cleanup/condition_test.go | 0 pkg/controllers/cleanup/controller.go | 424 +++++++++++------- 16 files changed, 354 insertions(+), 498 deletions(-) delete mode 100644 cmd/cleanup-controller/handlers/cleanup/handlers.go rename {cmd/cleanup-controller/handlers => pkg/controllers}/cleanup/condition.go (100%) rename {cmd/cleanup-controller/handlers => pkg/controllers}/cleanup/condition_test.go (100%) diff --git a/api/kyverno/v2alpha1/cleanup_policy_types.go b/api/kyverno/v2alpha1/cleanup_policy_types.go index 93150c8a8a7e..e18ff2337185 100644 --- a/api/kyverno/v2alpha1/cleanup_policy_types.go +++ b/api/kyverno/v2alpha1/cleanup_policy_types.go @@ -184,7 +184,8 @@ type CleanupPolicySpec struct { // CleanupPolicyStatus stores the status of the policy. type CleanupPolicyStatus struct { - Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` + LastExecutionTime metav1.Time `json:"lastExecutionTime,omitempty"` } // Validate implements programmatic validation diff --git a/api/kyverno/v2alpha1/zz_generated.deepcopy.go b/api/kyverno/v2alpha1/zz_generated.deepcopy.go index f4a9c5f924d8..0a312751950b 100644 --- a/api/kyverno/v2alpha1/zz_generated.deepcopy.go +++ b/api/kyverno/v2alpha1/zz_generated.deepcopy.go @@ -133,6 +133,7 @@ func (in *CleanupPolicyStatus) DeepCopyInto(out *CleanupPolicyStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + in.LastExecutionTime.DeepCopyInto(&out.LastExecutionTime) return } diff --git a/charts/kyverno/templates/crds/crds.yaml b/charts/kyverno/templates/crds/crds.yaml index 400c4380acdf..e407627fb70c 100644 --- a/charts/kyverno/templates/crds/crds.yaml +++ b/charts/kyverno/templates/crds/crds.yaml @@ -1896,6 +1896,9 @@ spec: - type type: object type: array + lastExecutionTime: + format: date-time + type: string type: object required: - spec @@ -3802,6 +3805,9 @@ spec: - type type: object type: array + lastExecutionTime: + format: date-time + type: string type: object required: - spec diff --git a/cmd/cleanup-controller/handlers/cleanup/handlers.go b/cmd/cleanup-controller/handlers/cleanup/handlers.go deleted file mode 100644 index 87d77ae18be8..000000000000 --- a/cmd/cleanup-controller/handlers/cleanup/handlers.go +++ /dev/null @@ -1,262 +0,0 @@ -package cleanup - -import ( - "context" - "time" - - "github.com/go-logr/logr" - kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" - kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" - kyvernov2alpha1 "github.com/kyverno/kyverno/api/kyverno/v2alpha1" - kyvernov2alpha1listers "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v2alpha1" - "github.com/kyverno/kyverno/pkg/clients/dclient" - "github.com/kyverno/kyverno/pkg/config" - engineapi "github.com/kyverno/kyverno/pkg/engine/api" - enginecontext "github.com/kyverno/kyverno/pkg/engine/context" - "github.com/kyverno/kyverno/pkg/engine/factories" - "github.com/kyverno/kyverno/pkg/engine/jmespath" - "github.com/kyverno/kyverno/pkg/event" - "github.com/kyverno/kyverno/pkg/metrics" - controllerutils "github.com/kyverno/kyverno/pkg/utils/controller" - "github.com/kyverno/kyverno/pkg/utils/match" - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/metric" - "go.uber.org/multierr" - "k8s.io/apimachinery/pkg/util/sets" - corev1listers "k8s.io/client-go/listers/core/v1" - "k8s.io/client-go/tools/cache" -) - -type handlers struct { - client dclient.Interface - cpolLister kyvernov2alpha1listers.ClusterCleanupPolicyLister - polLister kyvernov2alpha1listers.CleanupPolicyLister - nsLister corev1listers.NamespaceLister - cmResolver engineapi.ConfigmapResolver - eventGen event.Interface - jp jmespath.Interface - metrics cleanupMetrics -} - -type cleanupMetrics struct { - deletedObjectsTotal metric.Int64Counter - cleanupFailuresTotal metric.Int64Counter -} - -func newCleanupMetrics(logger logr.Logger) cleanupMetrics { - meter := otel.GetMeterProvider().Meter(metrics.MeterName) - deletedObjectsTotal, err := meter.Int64Counter( - "kyverno_cleanup_controller_deletedobjects", - metric.WithDescription("can be used to track number of deleted objects."), - ) - if err != nil { - logger.Error(err, "Failed to create instrument, cleanup_controller_deletedobjects_total") - } - cleanupFailuresTotal, err := meter.Int64Counter( - "kyverno_cleanup_controller_errors", - metric.WithDescription("can be used to track number of cleanup failures."), - ) - if err != nil { - logger.Error(err, "Failed to create instrument, cleanup_controller_errors_total") - } - return cleanupMetrics{ - deletedObjectsTotal: deletedObjectsTotal, - cleanupFailuresTotal: cleanupFailuresTotal, - } -} - -func New( - logger logr.Logger, - client dclient.Interface, - cpolLister kyvernov2alpha1listers.ClusterCleanupPolicyLister, - polLister kyvernov2alpha1listers.CleanupPolicyLister, - nsLister corev1listers.NamespaceLister, - cmResolver engineapi.ConfigmapResolver, - jp jmespath.Interface, - eventGen event.Interface, -) *handlers { - return &handlers{ - client: client, - cpolLister: cpolLister, - polLister: polLister, - nsLister: nsLister, - cmResolver: cmResolver, - eventGen: eventGen, - metrics: newCleanupMetrics(logger), - jp: jp, - } -} - -func (h *handlers) Cleanup(ctx context.Context, logger logr.Logger, name string, _ time.Time, cfg config.Configuration) error { - logger.Info("cleaning up...") - defer logger.Info("done") - namespace, name, err := cache.SplitMetaNamespaceKey(name) - if err != nil { - return err - } - policy, err := h.lookupPolicy(namespace, name) - if err != nil { - return err - } - return h.executePolicy(ctx, logger, policy, cfg) -} - -func (h *handlers) lookupPolicy(namespace, name string) (kyvernov2alpha1.CleanupPolicyInterface, error) { - if namespace == "" { - return h.cpolLister.Get(name) - } else { - return h.polLister.CleanupPolicies(namespace).Get(name) - } -} - -func (h *handlers) executePolicy( - ctx context.Context, - logger logr.Logger, - policy kyvernov2alpha1.CleanupPolicyInterface, - cfg config.Configuration, -) error { - spec := policy.GetSpec() - kinds := sets.New(spec.MatchResources.GetKinds()...) - debug := logger.V(4) - var errs []error - - enginectx := enginecontext.NewContext(h.jp) - ctxFactory := factories.DefaultContextLoaderFactory(h.cmResolver) - - loader := ctxFactory(nil, kyvernov1.Rule{}) - if err := loader.Load( - ctx, - h.jp, - h.client, - nil, - spec.Context, - enginectx, - ); err != nil { - return err - } - - for kind := range kinds { - commonLabels := []attribute.KeyValue{ - attribute.String("policy_type", policy.GetKind()), - attribute.String("policy_namespace", policy.GetNamespace()), - attribute.String("policy_name", policy.GetName()), - attribute.String("resource_kind", kind), - } - debug := debug.WithValues("kind", kind) - debug.Info("processing...") - list, err := h.client.ListResource(ctx, "", kind, policy.GetNamespace(), nil) - if err != nil { - debug.Error(err, "failed to list resources") - errs = append(errs, err) - if h.metrics.cleanupFailuresTotal != nil { - h.metrics.cleanupFailuresTotal.Add(ctx, 1, metric.WithAttributes(commonLabels...)) - } - } else { - for i := range list.Items { - resource := list.Items[i] - namespace := resource.GetNamespace() - name := resource.GetName() - debug := debug.WithValues("name", name, "namespace", namespace) - if !controllerutils.IsManagedByKyverno(&resource) { - var nsLabels map[string]string - if namespace != "" { - ns, err := h.nsLister.Get(namespace) - if err != nil { - debug.Error(err, "failed to get namespace labels") - errs = append(errs, err) - } - nsLabels = ns.GetLabels() - } - // match namespaces - if err := match.CheckNamespace(policy.GetNamespace(), resource); err != nil { - debug.Info("resource namespace didn't match policy namespace", "result", err) - } - // match resource with match/exclude clause - matched := match.CheckMatchesResources( - resource, - spec.MatchResources, - nsLabels, - // TODO(eddycharly): we don't have user info here, we should check that - // we don't have user conditions in the policy rule - kyvernov1beta1.RequestInfo{}, - resource.GroupVersionKind(), - "", - ) - if matched != nil { - debug.Info("resource/match didn't match", "result", matched) - continue - } - if spec.ExcludeResources != nil { - excluded := match.CheckMatchesResources( - resource, - *spec.ExcludeResources, - nsLabels, - // TODO(eddycharly): we don't have user info here, we should check that - // we don't have user conditions in the policy rule - kyvernov1beta1.RequestInfo{}, - resource.GroupVersionKind(), - "", - ) - if excluded == nil { - debug.Info("resource/exclude matched") - continue - } else { - debug.Info("resource/exclude didn't match", "result", excluded) - } - } - // check conditions - if spec.Conditions != nil { - enginectx.Reset() - if err := enginectx.SetTargetResource(resource.Object); err != nil { - debug.Error(err, "failed to add resource in context") - errs = append(errs, err) - continue - } - if err := enginectx.AddNamespace(resource.GetNamespace()); err != nil { - debug.Error(err, "failed to add namespace in context") - errs = append(errs, err) - continue - } - if err := enginectx.AddImageInfos(&resource, cfg); err != nil { - debug.Error(err, "failed to add image infos in context") - errs = append(errs, err) - continue - } - passed, err := checkAnyAllConditions(logger, enginectx, *spec.Conditions) - if err != nil { - debug.Error(err, "failed to check condition") - errs = append(errs, err) - continue - } - if !passed { - debug.Info("conditions did not pass") - continue - } - } - var labels []attribute.KeyValue - labels = append(labels, commonLabels...) - labels = append(labels, attribute.String("resource_namespace", namespace)) - logger.WithValues("name", name, "namespace", namespace).Info("resource matched, it will be deleted...") - if err := h.client.DeleteResource(ctx, resource.GetAPIVersion(), resource.GetKind(), namespace, name, false); err != nil { - if h.metrics.cleanupFailuresTotal != nil { - h.metrics.cleanupFailuresTotal.Add(ctx, 1, metric.WithAttributes(labels...)) - } - debug.Error(err, "failed to delete resource") - errs = append(errs, err) - e := event.NewCleanupPolicyEvent(policy, resource, err) - h.eventGen.Add(e) - } else { - if h.metrics.deletedObjectsTotal != nil { - h.metrics.deletedObjectsTotal.Add(ctx, 1, metric.WithAttributes(labels...)) - } - debug.Info("deleted") - e := event.NewCleanupPolicyEvent(policy, resource, nil) - h.eventGen.Add(e) - } - } - } - } - } - return multierr.Combine(errs...) -} diff --git a/cmd/cleanup-controller/main.go b/cmd/cleanup-controller/main.go index 0f0cca43ec19..7b00aad9015f 100644 --- a/cmd/cleanup-controller/main.go +++ b/cmd/cleanup-controller/main.go @@ -11,7 +11,6 @@ import ( "github.com/kyverno/kyverno/api/kyverno" policyhandlers "github.com/kyverno/kyverno/cmd/cleanup-controller/handlers/admission/policy" resourcehandlers "github.com/kyverno/kyverno/cmd/cleanup-controller/handlers/admission/resource" - cleanuphandlers "github.com/kyverno/kyverno/cmd/cleanup-controller/handlers/cleanup" "github.com/kyverno/kyverno/cmd/internal" "github.com/kyverno/kyverno/pkg/auth/checker" kyvernoinformer "github.com/kyverno/kyverno/pkg/client/informers/externalversions" @@ -111,6 +110,38 @@ func main() { os.Exit(1) } checker := checker.NewSelfChecker(setup.KubeClient.AuthorizationV1().SelfSubjectAccessReviews()) + // informer factories + kubeInformer := kubeinformers.NewSharedInformerFactoryWithOptions(setup.KubeClient, resyncPeriod) + kyvernoInformer := kyvernoinformer.NewSharedInformerFactory(setup.KyvernoClient, resyncPeriod) + // listers + nsLister := kubeInformer.Core().V1().Namespaces().Lister() + // log policy changes + genericloggingcontroller.NewController( + setup.Logger.WithName("cleanup-policy"), + "CleanupPolicy", + kyvernoInformer.Kyverno().V2alpha1().CleanupPolicies(), + genericloggingcontroller.CheckGeneration, + ) + genericloggingcontroller.NewController( + setup.Logger.WithName("cluster-cleanup-policy"), + "ClusterCleanupPolicy", + kyvernoInformer.Kyverno().V2alpha1().ClusterCleanupPolicies(), + genericloggingcontroller.CheckGeneration, + ) + eventGenerator := event.NewEventCleanupGenerator( + setup.KyvernoDynamicClient, + kyvernoInformer.Kyverno().V2alpha1().ClusterCleanupPolicies(), + kyvernoInformer.Kyverno().V2alpha1().CleanupPolicies(), + maxQueuedEvents, + logging.WithName("EventGenerator"), + ) + // start informers and wait for cache sync + if !internal.StartInformersAndWaitForCacheSync(ctx, setup.Logger, kubeInformer, kyvernoInformer) { + os.Exit(1) + } + // start event generator + var wg sync.WaitGroup + go eventGenerator.Run(ctx, 3, &wg) // setup leader election le, err := leaderelection.New( setup.Logger.WithName("leader-election"), @@ -124,6 +155,9 @@ func main() { // informer factories kubeInformer := kubeinformers.NewSharedInformerFactoryWithOptions(setup.KubeClient, resyncPeriod) kyvernoInformer := kyvernoinformer.NewSharedInformerFactory(setup.KyvernoClient, resyncPeriod) + + cmResolver := internal.NewConfigMapResolver(ctx, setup.Logger, setup.KubeClient, resyncPeriod) + // controllers renewer := tls.NewCertRenewer( setup.KubeClient.CoreV1().Secrets(config.KyvernoNamespace()), @@ -226,11 +260,15 @@ func main() { cleanupController := internal.NewController( cleanup.ControllerName, cleanup.NewController( - setup.KubeClient, + setup.KyvernoDynamicClient, + setup.KyvernoClient, kyvernoInformer.Kyverno().V2alpha1().ClusterCleanupPolicies(), kyvernoInformer.Kyverno().V2alpha1().CleanupPolicies(), - kubeInformer.Batch().V1().CronJobs(), - "https://"+config.KyvernoServiceName()+"."+config.KyvernoNamespace()+".svc", + nsLister, + setup.Configuration, + cmResolver, + setup.Jp, + eventGenerator, ), cleanup.Workers, ) @@ -264,54 +302,9 @@ func main() { setup.Logger.Error(err, "failed to initialize leader election") os.Exit(1) } - // informer factories - kubeInformer := kubeinformers.NewSharedInformerFactoryWithOptions(setup.KubeClient, resyncPeriod) - kyvernoInformer := kyvernoinformer.NewSharedInformerFactory(setup.KyvernoClient, resyncPeriod) - // listers - cpolLister := kyvernoInformer.Kyverno().V2alpha1().ClusterCleanupPolicies().Lister() - polLister := kyvernoInformer.Kyverno().V2alpha1().CleanupPolicies().Lister() - nsLister := kubeInformer.Core().V1().Namespaces().Lister() - // log policy changes - genericloggingcontroller.NewController( - setup.Logger.WithName("cleanup-policy"), - "CleanupPolicy", - kyvernoInformer.Kyverno().V2alpha1().CleanupPolicies(), - genericloggingcontroller.CheckGeneration, - ) - genericloggingcontroller.NewController( - setup.Logger.WithName("cluster-cleanup-policy"), - "ClusterCleanupPolicy", - kyvernoInformer.Kyverno().V2alpha1().ClusterCleanupPolicies(), - genericloggingcontroller.CheckGeneration, - ) - eventGenerator := event.NewEventCleanupGenerator( - setup.KyvernoDynamicClient, - kyvernoInformer.Kyverno().V2alpha1().ClusterCleanupPolicies(), - kyvernoInformer.Kyverno().V2alpha1().CleanupPolicies(), - maxQueuedEvents, - logging.WithName("EventGenerator"), - ) - // start informers and wait for cache sync - if !internal.StartInformersAndWaitForCacheSync(ctx, setup.Logger, kubeInformer, kyvernoInformer) { - os.Exit(1) - } - // start event generator - var wg sync.WaitGroup - go eventGenerator.Run(ctx, 3, &wg) // create handlers policyHandlers := policyhandlers.New(setup.KyvernoDynamicClient) resourceHandlers := resourcehandlers.New(checker) - cmResolver := internal.NewConfigMapResolver(ctx, setup.Logger, setup.KubeClient, resyncPeriod) - cleanupHandlers := cleanuphandlers.New( - setup.Logger.WithName("cleanup-handler"), - setup.KyvernoDynamicClient, - cpolLister, - polLister, - nsLister, - cmResolver, - setup.Jp, - eventGenerator, - ) // create server server := NewServer( func() ([]byte, []byte, error) { @@ -323,7 +316,6 @@ func main() { }, policyHandlers.Validate, resourceHandlers.Validate, - cleanupHandlers.Cleanup, setup.MetricsManager, webhooks.DebugModeOptions{ DumpPayload: dumpPayload, diff --git a/cmd/cleanup-controller/server.go b/cmd/cleanup-controller/server.go index 0d225cfcc18e..b2a5e39f4044 100644 --- a/cmd/cleanup-controller/server.go +++ b/cmd/cleanup-controller/server.go @@ -9,12 +9,10 @@ import ( "github.com/go-logr/logr" "github.com/julienschmidt/httprouter" "github.com/kyverno/kyverno/pkg/config" - "github.com/kyverno/kyverno/pkg/controllers/cleanup" "github.com/kyverno/kyverno/pkg/logging" "github.com/kyverno/kyverno/pkg/metrics" "github.com/kyverno/kyverno/pkg/webhooks" "github.com/kyverno/kyverno/pkg/webhooks/handlers" - apierrors "k8s.io/apimachinery/pkg/api/errors" ) type Server interface { @@ -45,7 +43,6 @@ func NewServer( tlsProvider TlsProvider, validationHandler ValidationHandler, labelValidationHandler LabelValidationHandler, - cleanupHandler CleanupHandler, metricsConfig metrics.MetricsConfigManager, debugModeOpts webhooks.DebugModeOptions, probes Probes, @@ -53,21 +50,6 @@ func NewServer( ) Server { policyLogger := logging.WithName("cleanup-policy") labelLogger := logging.WithName("ttl-label") - cleanupLogger := logging.WithName("cleanup") - cleanupHandlerFunc := func(w http.ResponseWriter, r *http.Request) { - policy := r.URL.Query().Get("policy") - logger := cleanupLogger.WithValues("policy", policy) - err := cleanupHandler(r.Context(), logger, policy, time.Now(), cfg) - if err == nil { - w.WriteHeader(http.StatusOK) - } else { - if apierrors.IsNotFound(err) { - w.WriteHeader(http.StatusNotFound) - } else { - w.WriteHeader(http.StatusInternalServerError) - } - } - } mux := httprouter.New() mux.HandlerFunc( "POST", @@ -89,14 +71,6 @@ func NewServer( WithAdmission(labelLogger.WithName("validate")). ToHandlerFunc("VALIDATE"), ) - mux.HandlerFunc( - "GET", - cleanup.CleanupServicePath, - handlers.HttpHandler(cleanupHandlerFunc). - WithMetrics(policyLogger). - WithTrace("CLEANUP"). - ToHandlerFunc("CLEANUP"), - ) mux.HandlerFunc("GET", config.LivenessServicePath, handlers.Probe(probes.IsLive)) mux.HandlerFunc("GET", config.ReadinessServicePath, handlers.Probe(probes.IsReady)) return &server{ diff --git a/config/crds/kyverno.io_cleanuppolicies.yaml b/config/crds/kyverno.io_cleanuppolicies.yaml index c4a2d6ed75d9..06bbeebc269e 100644 --- a/config/crds/kyverno.io_cleanuppolicies.yaml +++ b/config/crds/kyverno.io_cleanuppolicies.yaml @@ -1248,6 +1248,9 @@ spec: - type type: object type: array + lastExecutionTime: + format: date-time + type: string type: object required: - spec diff --git a/config/crds/kyverno.io_clustercleanuppolicies.yaml b/config/crds/kyverno.io_clustercleanuppolicies.yaml index 8b26f32697b1..610761185ae4 100644 --- a/config/crds/kyverno.io_clustercleanuppolicies.yaml +++ b/config/crds/kyverno.io_clustercleanuppolicies.yaml @@ -1248,6 +1248,9 @@ spec: - type type: object type: array + lastExecutionTime: + format: date-time + type: string type: object required: - spec diff --git a/config/install-latest-testing.yaml b/config/install-latest-testing.yaml index 1da64be891a9..ae73fdf2637c 100644 --- a/config/install-latest-testing.yaml +++ b/config/install-latest-testing.yaml @@ -2099,6 +2099,9 @@ spec: - type type: object type: array + lastExecutionTime: + format: date-time + type: string type: object required: - spec @@ -4005,6 +4008,9 @@ spec: - type type: object type: array + lastExecutionTime: + format: date-time + type: string type: object required: - spec diff --git a/docs/user/crd/index.html b/docs/user/crd/index.html index 18d230da0792..03feef985de3 100644 --- a/docs/user/crd/index.html +++ b/docs/user/crd/index.html @@ -5940,6 +5940,18 @@

CleanupPolicyStatus + + +lastExecutionTime
+ + +Kubernetes meta/v1.Time + + + + + +
diff --git a/go.mod b/go.mod index 202674e379a7..d09669935fd9 100644 --- a/go.mod +++ b/go.mod @@ -145,6 +145,7 @@ require ( github.com/alibabacloud-go/tea-xml v1.1.3 // indirect github.com/aliyun/credentials-go v1.3.1 // indirect github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230305170008-8188dc5388df // indirect + github.com/aptible/supercronic v0.2.26 github.com/aws/aws-sdk-go-v2 v1.21.0 // indirect github.com/aws/aws-sdk-go-v2/config v1.18.40 // indirect github.com/aws/aws-sdk-go-v2/credentials v1.13.38 // indirect diff --git a/go.sum b/go.sum index 4fa9d4c89b12..45bcbecd50f7 100644 --- a/go.sum +++ b/go.sum @@ -235,6 +235,8 @@ github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230305170008-8188dc5388df/g github.com/aokoli/goutils v1.0.1/go.mod h1:SijmP0QR8LtwsmDs8Yii5Z/S4trXFGFC2oO5g9DP+DQ= github.com/apache/thrift v0.12.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ= github.com/apache/thrift v0.13.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ= +github.com/aptible/supercronic v0.2.26 h1:dPid6awDpM1mINOqOdpDLeiSj8WwjbSn5idBt6gMlaw= +github.com/aptible/supercronic v0.2.26/go.mod h1:xK+y2E7w0eTSgCoEoPvUSmzwGoboEe4G6ZX97l4VyqI= github.com/aquilax/truncate v1.0.0 h1:UgIGS8U/aZ4JyOJ2h3xcF5cSQ06+gGBnjxH2RUHJe0U= github.com/aquilax/truncate v1.0.0/go.mod h1:BeMESIDMlvlS3bmg4BVvBbbZUNwWtS8uzYPAKXwwhLw= github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0 h1:jfIu9sQUG6Ig+0+Ap1h4unLjW6YQJpKZVmUzxsD4E/Q= diff --git a/pkg/client/applyconfigurations/kyverno/v2alpha1/cleanuppolicystatus.go b/pkg/client/applyconfigurations/kyverno/v2alpha1/cleanuppolicystatus.go index bc342aea9911..53c13a1b1130 100644 --- a/pkg/client/applyconfigurations/kyverno/v2alpha1/cleanuppolicystatus.go +++ b/pkg/client/applyconfigurations/kyverno/v2alpha1/cleanuppolicystatus.go @@ -25,7 +25,8 @@ import ( // CleanupPolicyStatusApplyConfiguration represents an declarative configuration of the CleanupPolicyStatus type for use // with apply. type CleanupPolicyStatusApplyConfiguration struct { - Conditions []v1.Condition `json:"conditions,omitempty"` + Conditions []v1.Condition `json:"conditions,omitempty"` + LastExecutionTime *v1.Time `json:"lastExecutionTime,omitempty"` } // CleanupPolicyStatusApplyConfiguration constructs an declarative configuration of the CleanupPolicyStatus type for use with @@ -43,3 +44,11 @@ func (b *CleanupPolicyStatusApplyConfiguration) WithConditions(values ...v1.Cond } return b } + +// WithLastExecutionTime sets the LastExecutionTime field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the LastExecutionTime field is set to the value of the last call. +func (b *CleanupPolicyStatusApplyConfiguration) WithLastExecutionTime(value v1.Time) *CleanupPolicyStatusApplyConfiguration { + b.LastExecutionTime = &value + return b +} diff --git a/cmd/cleanup-controller/handlers/cleanup/condition.go b/pkg/controllers/cleanup/condition.go similarity index 100% rename from cmd/cleanup-controller/handlers/cleanup/condition.go rename to pkg/controllers/cleanup/condition.go diff --git a/cmd/cleanup-controller/handlers/cleanup/condition_test.go b/pkg/controllers/cleanup/condition_test.go similarity index 100% rename from cmd/cleanup-controller/handlers/cleanup/condition_test.go rename to pkg/controllers/cleanup/condition_test.go diff --git a/pkg/controllers/cleanup/controller.go b/pkg/controllers/cleanup/controller.go index 9f1422ce1d46..c8f0a0efe04f 100644 --- a/pkg/controllers/cleanup/controller.go +++ b/pkg/controllers/cleanup/controller.go @@ -2,47 +2,64 @@ package cleanup import ( "context" - "fmt" "time" + "github.com/aptible/supercronic/cronexpr" "github.com/go-logr/logr" + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" + kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" kyvernov2alpha1 "github.com/kyverno/kyverno/api/kyverno/v2alpha1" + "github.com/kyverno/kyverno/pkg/client/clientset/versioned" kyvernov2alpha1informers "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v2alpha1" kyvernov2alpha1listers "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v2alpha1" + "github.com/kyverno/kyverno/pkg/clients/dclient" "github.com/kyverno/kyverno/pkg/config" "github.com/kyverno/kyverno/pkg/controllers" + engineapi "github.com/kyverno/kyverno/pkg/engine/api" + enginecontext "github.com/kyverno/kyverno/pkg/engine/context" + "github.com/kyverno/kyverno/pkg/engine/factories" + "github.com/kyverno/kyverno/pkg/engine/jmespath" + "github.com/kyverno/kyverno/pkg/event" + "github.com/kyverno/kyverno/pkg/logging" + "github.com/kyverno/kyverno/pkg/metrics" controllerutils "github.com/kyverno/kyverno/pkg/utils/controller" - batchv1 "k8s.io/api/batch/v1" - corev1 "k8s.io/api/core/v1" + "github.com/kyverno/kyverno/pkg/utils/match" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" + "go.uber.org/multierr" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - batchv1informers "k8s.io/client-go/informers/batch/v1" - "k8s.io/client-go/kubernetes" - batchv1listers "k8s.io/client-go/listers/batch/v1" - "k8s.io/client-go/tools/cache" + "k8s.io/apimachinery/pkg/util/sets" + corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/util/workqueue" ) -const ( - // CleanupServicePath is the path for triggering cleanup - CleanupServicePath = "/cleanup" -) - type controller struct { // clients - client kubernetes.Interface + client dclient.Interface + kyvernoClient versioned.Interface // listers cpolLister kyvernov2alpha1listers.ClusterCleanupPolicyLister polLister kyvernov2alpha1listers.CleanupPolicyLister - cjLister batchv1listers.CronJobLister + nsLister corev1listers.NamespaceLister // queue queue workqueue.RateLimitingInterface enqueue controllerutils.EnqueueFuncT[kyvernov2alpha1.CleanupPolicyInterface] // config - cleanupService string + configuration config.Configuration + cmResolver engineapi.ConfigmapResolver + eventGen event.Interface + jp jmespath.Interface + metrics cleanupMetrics +} + +type cleanupMetrics struct { + deletedObjectsTotal metric.Int64Counter + cleanupFailuresTotal metric.Int64Counter } const ( @@ -52,11 +69,15 @@ const ( ) func NewController( - client kubernetes.Interface, + client dclient.Interface, + kyvernoClient versioned.Interface, cpolInformer kyvernov2alpha1informers.ClusterCleanupPolicyInformer, polInformer kyvernov2alpha1informers.CleanupPolicyInformer, - cjInformer batchv1informers.CronJobInformer, - cleanupService string, + nsLister corev1listers.NamespaceLister, + configuration config.Configuration, + cmResolver engineapi.ConfigmapResolver, + jp jmespath.Interface, + eventGen event.Interface, ) controllers.Controller { queue := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), ControllerName) keyFunc := controllerutils.MetaNamespaceKeyT[kyvernov2alpha1.CleanupPolicyInterface] @@ -77,13 +98,18 @@ func NewController( } } c := &controller{ - client: client, - cpolLister: cpolInformer.Lister(), - polLister: polInformer.Lister(), - cjLister: cjInformer.Lister(), - queue: queue, - cleanupService: cleanupService, - enqueue: baseEnqueueFunc, + client: client, + kyvernoClient: kyvernoClient, + cpolLister: cpolInformer.Lister(), + polLister: polInformer.Lister(), + nsLister: nsLister, + queue: queue, + enqueue: baseEnqueueFunc, + configuration: configuration, + cmResolver: cmResolver, + eventGen: eventGen, + metrics: newCleanupMetrics(logger), + jp: jp, } if _, err := controllerutils.AddEventHandlersT( cpolInformer.Informer(), @@ -101,46 +127,33 @@ func NewController( ); err != nil { logger.Error(err, "failed to register event handlers") } - if _, err := controllerutils.AddEventHandlersT( - cjInformer.Informer(), - func(n *batchv1.CronJob) { c.enqueueCronJob(n) }, - func(o *batchv1.CronJob, n *batchv1.CronJob) { c.enqueueCronJob(o) }, - func(n *batchv1.CronJob) { c.enqueueCronJob(n) }, - ); err != nil { - logger.Error(err, "failed to register event handlers") - } return c } -func (c *controller) Run(ctx context.Context, workers int) { - controllerutils.Run(ctx, logger.V(3), ControllerName, time.Second, c.queue, workers, maxRetries, c.reconcile) +func newCleanupMetrics(logger logr.Logger) cleanupMetrics { + meter := otel.GetMeterProvider().Meter(metrics.MeterName) + deletedObjectsTotal, err := meter.Int64Counter( + "kyverno_cleanup_controller_deletedobjects", + metric.WithDescription("can be used to track number of deleted objects."), + ) + if err != nil { + logger.Error(err, "Failed to create instrument, cleanup_controller_deletedobjects_total") + } + cleanupFailuresTotal, err := meter.Int64Counter( + "kyverno_cleanup_controller_errors", + metric.WithDescription("can be used to track number of cleanup failures."), + ) + if err != nil { + logger.Error(err, "Failed to create instrument, cleanup_controller_errors_total") + } + return cleanupMetrics{ + deletedObjectsTotal: deletedObjectsTotal, + cleanupFailuresTotal: cleanupFailuresTotal, + } } -func (c *controller) enqueueCronJob(n *batchv1.CronJob) { - if len(n.OwnerReferences) == 1 { - if n.OwnerReferences[0].Kind == "ClusterCleanupPolicy" { - cpol := &kyvernov2alpha1.ClusterCleanupPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: n.OwnerReferences[0].Name, - }, - } - err := c.enqueue(cpol) - if err != nil { - logger.Error(err, "failed to enqueue ClusterCleanupPolicy object", cpol) - } - } else if n.OwnerReferences[0].Kind == "CleanupPolicy" { - pol := &kyvernov2alpha1.CleanupPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: n.OwnerReferences[0].Name, - Namespace: n.Namespace, - }, - } - err := c.enqueue(pol) - if err != nil { - logger.Error(err, "failed to enqueue CleanupPolicy object", pol) - } - } - } +func (c *controller) Run(ctx context.Context, workers int) { + controllerutils.Run(ctx, logger.V(3), ControllerName, time.Second, c.queue, workers, maxRetries, c.reconcile) } func (c *controller) getPolicy(namespace, name string) (kyvernov2alpha1.CleanupPolicyInterface, error) { @@ -159,84 +172,150 @@ func (c *controller) getPolicy(namespace, name string) (kyvernov2alpha1.CleanupP } } -func (c *controller) getCronjob(namespace, name string) (*batchv1.CronJob, error) { - cj, err := c.cjLister.CronJobs(namespace).Get(name) - if err != nil { - return nil, err - } - return cj, nil -} +func (c *controller) cleanup(ctx context.Context, logger logr.Logger, policy kyvernov2alpha1.CleanupPolicyInterface) error { + spec := policy.GetSpec() + kinds := sets.New(spec.MatchResources.GetKinds()...) + debug := logger.V(4) + var errs []error -func (c *controller) buildCronJob(cronJob *batchv1.CronJob, pol kyvernov2alpha1.CleanupPolicyInterface) error { - // TODO: find a better way to do that, it looks like resources returned by WATCH don't have the GVK - apiVersion := "kyverno.io/v2alpha1" - kind := "CleanupPolicy" - if pol.GetNamespace() == "" { - kind = "ClusterCleanupPolicy" - } - policyName, err := cache.MetaNamespaceKeyFunc(pol) - if err != nil { + enginectx := enginecontext.NewContext(c.jp) + ctxFactory := factories.DefaultContextLoaderFactory(c.cmResolver) + + loader := ctxFactory(nil, kyvernov1.Rule{}) + if err := loader.Load( + ctx, + c.jp, + c.client, + nil, + spec.Context, + enginectx, + ); err != nil { return err } - // set owner reference - cronJob.OwnerReferences = []metav1.OwnerReference{ - { - APIVersion: apiVersion, - Kind: kind, - Name: pol.GetName(), - UID: pol.GetUID(), - }, - } - var successfulJobsHistoryLimit int32 = 0 - var failedJobsHistoryLimit int32 = 1 - var boolFalse bool = false - var boolTrue bool = true - var int1000 int64 = 1000 - // set spec - cronJob.Spec = batchv1.CronJobSpec{ - Schedule: pol.GetSpec().Schedule, - SuccessfulJobsHistoryLimit: &successfulJobsHistoryLimit, - FailedJobsHistoryLimit: &failedJobsHistoryLimit, - ConcurrencyPolicy: batchv1.ForbidConcurrent, - JobTemplate: batchv1.JobTemplateSpec{ - Spec: batchv1.JobSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyOnFailure, - Containers: []corev1.Container{ - { - Name: "cleanup", - Image: "curlimages/curl:7.86.0", - Args: []string{ - "-k", - // TODO: ca - // "--cacert", - // "/tmp/ca.crt", - fmt.Sprintf("%s%s?policy=%s", c.cleanupService, CleanupServicePath, policyName), - }, - SecurityContext: &corev1.SecurityContext{ - AllowPrivilegeEscalation: &boolFalse, - RunAsNonRoot: &boolTrue, - RunAsUser: &int1000, - SeccompProfile: &corev1.SeccompProfile{ - Type: corev1.SeccompProfileTypeRuntimeDefault, - }, - Capabilities: &corev1.Capabilities{ - Drop: []corev1.Capability{"ALL"}, - }, - }, - }, - }, - }, - }, - }, - }, + + for kind := range kinds { + commonLabels := []attribute.KeyValue{ + attribute.String("policy_type", policy.GetKind()), + attribute.String("policy_namespace", policy.GetNamespace()), + attribute.String("policy_name", policy.GetName()), + attribute.String("resource_kind", kind), + } + debug := debug.WithValues("kind", kind) + debug.Info("processing...") + list, err := c.client.ListResource(ctx, "", kind, policy.GetNamespace(), nil) + if err != nil { + debug.Error(err, "failed to list resources") + errs = append(errs, err) + if c.metrics.cleanupFailuresTotal != nil { + c.metrics.cleanupFailuresTotal.Add(ctx, 1, metric.WithAttributes(commonLabels...)) + } + } else { + for i := range list.Items { + resource := list.Items[i] + namespace := resource.GetNamespace() + name := resource.GetName() + debug := debug.WithValues("name", name, "namespace", namespace) + if !controllerutils.IsManagedByKyverno(&resource) { + var nsLabels map[string]string + if namespace != "" { + ns, err := c.nsLister.Get(namespace) + if err != nil { + debug.Error(err, "failed to get namespace labels") + errs = append(errs, err) + } + nsLabels = ns.GetLabels() + } + // match namespaces + if err := match.CheckNamespace(policy.GetNamespace(), resource); err != nil { + debug.Info("resource namespace didn't match policy namespace", "result", err) + } + // match resource with match/exclude clause + matched := match.CheckMatchesResources( + resource, + spec.MatchResources, + nsLabels, + // TODO(eddycharly): we don't have user info here, we should check that + // we don't have user conditions in the policy rule + kyvernov1beta1.RequestInfo{}, + resource.GroupVersionKind(), + "", + ) + if matched != nil { + debug.Info("resource/match didn't match", "result", matched) + continue + } + if spec.ExcludeResources != nil { + excluded := match.CheckMatchesResources( + resource, + *spec.ExcludeResources, + nsLabels, + // TODO(eddycharly): we don't have user info here, we should check that + // we don't have user conditions in the policy rule + kyvernov1beta1.RequestInfo{}, + resource.GroupVersionKind(), + "", + ) + if excluded == nil { + debug.Info("resource/exclude matched") + continue + } else { + debug.Info("resource/exclude didn't match", "result", excluded) + } + } + // check conditions + if spec.Conditions != nil { + enginectx.Reset() + if err := enginectx.SetTargetResource(resource.Object); err != nil { + debug.Error(err, "failed to add resource in context") + errs = append(errs, err) + continue + } + if err := enginectx.AddNamespace(resource.GetNamespace()); err != nil { + debug.Error(err, "failed to add namespace in context") + errs = append(errs, err) + continue + } + if err := enginectx.AddImageInfos(&resource, c.configuration); err != nil { + debug.Error(err, "failed to add image infos in context") + errs = append(errs, err) + continue + } + passed, err := checkAnyAllConditions(logger, enginectx, *spec.Conditions) + if err != nil { + debug.Error(err, "failed to check condition") + errs = append(errs, err) + continue + } + if !passed { + debug.Info("conditions did not pass") + continue + } + } + var labels []attribute.KeyValue + labels = append(labels, commonLabels...) + labels = append(labels, attribute.String("resource_namespace", namespace)) + logger.WithValues("name", name, "namespace", namespace).Info("resource matched, it will be deleted...") + if err := c.client.DeleteResource(ctx, resource.GetAPIVersion(), resource.GetKind(), namespace, name, false); err != nil { + if c.metrics.cleanupFailuresTotal != nil { + c.metrics.cleanupFailuresTotal.Add(ctx, 1, metric.WithAttributes(labels...)) + } + debug.Error(err, "failed to delete resource") + errs = append(errs, err) + e := event.NewCleanupPolicyEvent(policy, resource, err) + c.eventGen.Add(e) + } else { + if c.metrics.deletedObjectsTotal != nil { + c.metrics.deletedObjectsTotal.Add(ctx, 1, metric.WithAttributes(labels...)) + } + debug.Info("deleted") + e := event.NewCleanupPolicyEvent(policy, resource, nil) + c.eventGen.Add(e) + } + } + } + } } - // set labels - controllerutils.SetManagedByKyvernoLabel(cronJob) - controllerutils.SetManagedByKyvernoLabel(&cronJob.Spec.JobTemplate) - controllerutils.SetManagedByKyvernoLabel(&cronJob.Spec.JobTemplate.Spec.Template) - return nil + return multierr.Combine(errs...) } func (c *controller) reconcile(ctx context.Context, logger logr.Logger, key, namespace, name string) error { @@ -248,33 +327,62 @@ func (c *controller) reconcile(ctx context.Context, logger logr.Logger, key, nam logger.Error(err, "unable to get the policy from policy informer") return err } - cronjobNs := namespace - if namespace == "" { - cronjobNs = config.KyvernoNamespace() - } - observed, err := c.getCronjob(cronjobNs, string(policy.GetUID())) + spec := policy.GetSpec() + cronExpr, err := cronexpr.Parse(spec.Schedule) if err != nil { - if !apierrors.IsNotFound(err) { - return err - } - observed = &batchv1.CronJob{ - ObjectMeta: metav1.ObjectMeta{ - Name: string(policy.GetUID()), - Namespace: cronjobNs, - }, - } + logger.Error(err, "unable to parse the schedule") + return err } - if observed.ResourceVersion == "" { - err := c.buildCronJob(observed, policy) - if err != nil { - return err + + status := policy.GetStatus() + creationTime := policy.GetCreationTimestamp().Time + firstExecutionTime := cronExpr.Next(creationTime) + + var nextExecutionTime time.Time + // In case it isn't the first execution of the cleanup policy. + if firstExecutionTime.Before(time.Now()) { + var executionTime time.Time + if status.LastExecutionTime.IsZero() { + executionTime = firstExecutionTime + } else { + executionTime = cronExpr.Next(status.LastExecutionTime.Time) + } + // In case it is the time to do the cleanup process + if time.Now().After(executionTime) { + err := c.cleanup(ctx, logger, policy) + if err != nil { + return err + } + c.updateCleanupPolicyStatus(ctx, policy, namespace, executionTime) + nextExecutionTime = cronExpr.Next(executionTime) + } else { + nextExecutionTime = executionTime } - _, err = c.client.BatchV1().CronJobs(cronjobNs).Create(ctx, observed, metav1.CreateOptions{}) - return err } else { - _, err = controllerutils.Update(ctx, observed, c.client.BatchV1().CronJobs(cronjobNs), func(observed *batchv1.CronJob) error { - return c.buildCronJob(observed, policy) - }) - return err + // In case it is the first execution of the cleanup policy. + nextExecutionTime = firstExecutionTime + } + + // calculate the remaining time until deletion. + timeRemaining := time.Until(nextExecutionTime) + // add the item back to the queue after the remaining time. + c.queue.AddAfter(key, timeRemaining) + return nil +} + +func (c *controller) updateCleanupPolicyStatus(ctx context.Context, policy kyvernov2alpha1.CleanupPolicyInterface, namespace string, time time.Time) { + switch obj := policy.(type) { + case *kyvernov2alpha1.ClusterCleanupPolicy: + latest := obj.DeepCopy() + latest.Status.LastExecutionTime.Time = time + + new, _ := c.kyvernoClient.KyvernoV2alpha1().ClusterCleanupPolicies().UpdateStatus(ctx, latest, metav1.UpdateOptions{}) + logging.V(3).Info("updated cluster cleanup policy status", "name", policy.GetName(), "status", new.Status) + case *kyvernov2alpha1.CleanupPolicy: + latest := obj.DeepCopy() + latest.Status.LastExecutionTime.Time = time + + new, _ := c.kyvernoClient.KyvernoV2alpha1().CleanupPolicies(namespace).UpdateStatus(ctx, latest, metav1.UpdateOptions{}) + logging.V(3).Info("updated cleanup policy status", "name", policy.GetName(), "namespace", policy.GetNamespace(), "status", new.Status) } }