From 78e7c5dc181ceea746f8b171c29a8075edc1f4fa 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 17:53:49 +0200 Subject: [PATCH] fix: move ur controller filtering in reconciler (#3964) (#3994) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: use BackgroundProcessingEnabled method Signed-off-by: Charles-Edouard Brétéché * refactor: webhooks metrics reporting Signed-off-by: Charles-Edouard Brétéché * refactor: metrics package Signed-off-by: Charles-Edouard Brétéché * fix: move ur controller filtering in reconciler (#3964) * fix: move ur controller filtering in reconciler Signed-off-by: Charles-Edouard Brétéché * fix: mark ur retry on conflict Signed-off-by: Charles-Edouard Brétéché * fix: test data Signed-off-by: Charles-Edouard Brétéché * fix: add filter back in update ur handler Signed-off-by: Charles-Edouard Brétéché * fix: added some logs about attempts and increased backoff Signed-off-by: Charles-Edouard Brétéché * fix: reconciliation logic Signed-off-by: Charles-Edouard Brétéché * fix: Test_Generate_Synchronize_Flag Signed-off-by: Charles-Edouard Brétéché * fix: small nits Signed-off-by: Charles-Edouard Brétéché (cherry picked from commit 1936d86623445538d6f0b6561a9ee004d547bb10) Signed-off-by: Charles-Edouard Brétéché * fix: conflicts Signed-off-by: Charles-Edouard Brétéché --- cmd/initContainer/main.go | 9 +- cmd/kyverno/main.go | 3 +- pkg/background/common/status.go | 97 ++----- pkg/background/common/util.go | 70 ++++- pkg/background/generate/generate.go | 17 +- pkg/background/log.go | 5 + pkg/background/mutate/mutate.go | 13 +- pkg/background/request_process.go | 75 ----- pkg/background/update_request_controller.go | 300 ++++++++++---------- pkg/policy/policy_controller.go | 37 ++- pkg/policy/updaterequest.go | 10 +- pkg/webhooks/generation.go | 56 ++-- pkg/webhooks/updaterequest/generator.go | 48 ++-- test/e2e/generate/resources.go | 3 + 14 files changed, 329 insertions(+), 414 deletions(-) create mode 100644 pkg/background/log.go delete mode 100644 pkg/background/request_process.go diff --git a/cmd/initContainer/main.go b/cmd/initContainer/main.go index 98e5bf5bb8..496098b1d7 100644 --- a/cmd/initContainer/main.go +++ b/cmd/initContainer/main.go @@ -503,7 +503,7 @@ func convertGR(pclient kyvernoclient.Interface) error { }, } - new, err := pclient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).Create(context.TODO(), ur, metav1.CreateOptions{}) + _, err := pclient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).Create(context.TODO(), ur, metav1.CreateOptions{}) if err != nil { logger.Info("failed to create UpdateRequest", "GR namespace", gr.GetNamespace(), "GR name", gr.GetName(), "err", err.Error()) errors = append(errors, err) @@ -512,13 +512,6 @@ func convertGR(pclient kyvernoclient.Interface) error { logger.Info("successfully created UpdateRequest", "GR namespace", gr.GetNamespace(), "GR name", gr.GetName()) } - new.Status.State = urkyverno.Pending - if _, err := pclient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).UpdateStatus(context.TODO(), new, metav1.UpdateOptions{}); err != nil { - logger.Error(err, "failed to set UpdateRequest state to Pending") - errors = append(errors, err) - continue - } - if err := pclient.KyvernoV1().GenerateRequests(config.KyvernoNamespace).Delete(context.TODO(), gr.GetName(), metav1.DeleteOptions{}); err != nil { errors = append(errors, err) logger.Error(err, "failed to delete GR") diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index ac7a14dd2d..e24b47a303 100755 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -293,7 +293,6 @@ func main() { kubeInformer.Core().V1().Pods(), eventGenerator, kubeInformer.Core().V1().Namespaces(), - log.Log.WithName("BackgroundController"), configuration, ) @@ -416,7 +415,7 @@ func main() { cleanUp, log.Log.WithName("WebhookServer"), openAPIController, - urc, + &urc, promConfig, ) diff --git a/pkg/background/common/status.go b/pkg/background/common/status.go index 585a059ffc..a34e4af37d 100644 --- a/pkg/background/common/status.go +++ b/pkg/background/common/status.go @@ -1,96 +1,43 @@ package common import ( - kyverno "github.com/kyverno/kyverno/api/kyverno/v1" - urkyverno "github.com/kyverno/kyverno/api/kyverno/v1beta1" + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" + kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" kyvernoclient "github.com/kyverno/kyverno/pkg/client/clientset/versioned" - jsonutils "github.com/kyverno/kyverno/pkg/utils/json" - "k8s.io/apimachinery/pkg/api/errors" - "sigs.k8s.io/controller-runtime/pkg/log" + kyvernov1beta1listers "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1beta1" ) //StatusControlInterface provides interface to update status subresource type StatusControlInterface interface { - Failed(ur urkyverno.UpdateRequest, message string, genResources []kyverno.ResourceSpec) error - Success(ur urkyverno.UpdateRequest, genResources []kyverno.ResourceSpec) error - Skip(ur urkyverno.UpdateRequest, genResources []kyverno.ResourceSpec) error + Failed(name string, message string, genResources []kyvernov1.ResourceSpec) (*kyvernov1beta1.UpdateRequest, error) + Success(name string, genResources []kyvernov1.ResourceSpec) (*kyvernov1beta1.UpdateRequest, error) + Skip(name string, genResources []kyvernov1.ResourceSpec) (*kyvernov1beta1.UpdateRequest, error) } -// StatusControl is default implementaation of GRStatusControlInterface -type StatusControl struct { - Client kyvernoclient.Interface +// statusControl is default implementaation of GRStatusControlInterface +type statusControl struct { + client kyvernoclient.Interface + urLister kyvernov1beta1listers.UpdateRequestNamespaceLister } -//Failed sets ur status.state to failed with message -func (sc StatusControl) Failed(ur urkyverno.UpdateRequest, message string, genResources []kyverno.ResourceSpec) error { - genR := &urkyverno.UpdateRequestStatus{ - State: urkyverno.Failed, - Message: message, - } - if genResources != nil { - genR.GeneratedResources = genResources +func NewStatusControl(client kyvernoclient.Interface, urLister kyvernov1beta1listers.UpdateRequestNamespaceLister) StatusControlInterface { + return &statusControl{ + client: client, + urLister: urLister, } +} - patch := jsonutils.NewPatch( - "/status", - "replace", - genR, - ) - _, err := PatchUpdateRequest(&ur, patch, sc.Client, "status") - if err != nil && !errors.IsNotFound(err) { - log.Log.Error(err, "failed to patch update request status", "name", ur.Name) - return err - } - log.Log.V(3).Info("updated update request status", "name", ur.Name, "status", string(kyverno.Failed)) - return nil +// Failed sets ur status.state to failed with message +func (sc *statusControl) Failed(name, message string, genResources []kyvernov1.ResourceSpec) (*kyvernov1beta1.UpdateRequest, error) { + return UpdateStatus(sc.client, sc.urLister, name, kyvernov1beta1.Failed, message, genResources) } // Success sets the ur status.state to completed and clears message -func (sc StatusControl) Success(ur urkyverno.UpdateRequest, genResources []kyverno.ResourceSpec) error { - genR := &urkyverno.UpdateRequestStatus{ - State: urkyverno.Completed, - Message: "", - } - - if genResources != nil { - genR.GeneratedResources = genResources - } - - patch := jsonutils.NewPatch( - "/status", - "replace", - genR, - ) - _, err := PatchUpdateRequest(&ur, patch, sc.Client, "status") - if err != nil && !errors.IsNotFound(err) { - log.Log.Error(err, "failed to patch update request status", "name", ur.Name) - return err - } - log.Log.V(3).Info("updated update request status", "name", ur.Name, "status", string(urkyverno.Completed)) - return nil +func (sc *statusControl) Success(name string, genResources []kyvernov1.ResourceSpec) (*kyvernov1beta1.UpdateRequest, error) { + return UpdateStatus(sc.client, sc.urLister, name, kyvernov1beta1.Completed, "", genResources) } // Success sets the ur status.state to completed and clears message -func (sc StatusControl) Skip(ur urkyverno.UpdateRequest, genResources []kyverno.ResourceSpec) error { - genR := &urkyverno.UpdateRequestStatus{ - State: urkyverno.Skip, - Message: "", - } - - if genResources != nil { - genR.GeneratedResources = genResources - } - - patch := jsonutils.NewPatch( - "/status", - "replace", - genR, - ) - _, err := PatchUpdateRequest(&ur, patch, sc.Client, "status") - if err != nil && !errors.IsNotFound(err) { - log.Log.Error(err, "failed to update UR status", "name", ur.Name) - return err - } - log.Log.V(3).Info("updated UR status", "name", ur.Name, "status", string(kyverno.Skip)) - return nil +func (sc *statusControl) Skip(name string, genResources []kyvernov1.ResourceSpec) (*kyvernov1beta1.UpdateRequest, error) { + return UpdateStatus(sc.client, sc.urLister, name, kyvernov1beta1.Skip, "", genResources) } diff --git a/pkg/background/common/util.go b/pkg/background/common/util.go index 5117752e92..49e67706e6 100644 --- a/pkg/background/common/util.go +++ b/pkg/background/common/util.go @@ -4,31 +4,73 @@ import ( "context" "time" - urkyverno "github.com/kyverno/kyverno/api/kyverno/v1beta1" + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" + kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" kyvernoclient "github.com/kyverno/kyverno/pkg/client/clientset/versioned" + kyvernov1beta1listers "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1beta1" "github.com/kyverno/kyverno/pkg/config" - jsonutils "github.com/kyverno/kyverno/pkg/utils/json" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/util/retry" + "sigs.k8s.io/controller-runtime/pkg/log" ) var DefaultRetry = wait.Backoff{ - Steps: 5, - Duration: 30 * time.Millisecond, + Steps: 15, + Duration: 100 * time.Millisecond, Factor: 1.0, Jitter: 0.1, } -// PatchUpdateRequest patches a update request object -func PatchUpdateRequest(ur *urkyverno.UpdateRequest, patch jsonutils.Patch, client kyvernoclient.Interface, subresources ...string) (*urkyverno.UpdateRequest, error) { - data, err := patch.ToPatchBytes() - if nil != err { - return ur, err - } - newUR, err := client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).Patch(context.TODO(), ur.Name, types.JSONPatchType, data, metav1.PatchOptions{}, subresources...) +func Update(client kyvernoclient.Interface, urLister kyvernov1beta1listers.UpdateRequestNamespaceLister, name string, mutator func(*kyvernov1beta1.UpdateRequest)) (*kyvernov1beta1.UpdateRequest, error) { + var ur *kyvernov1beta1.UpdateRequest + err := retry.RetryOnConflict(DefaultRetry, func() error { + ur, err := urLister.Get(name) + if err != nil { + log.Log.Error(err, "[ATTEMPT] failed to fetch update request", "name", name) + return err + } + ur = ur.DeepCopy() + mutator(ur) + _, err = client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).Update(context.TODO(), ur, metav1.UpdateOptions{}) + if err != nil { + log.Log.Error(err, "[ATTEMPT] failed to update update request", "name", name) + } + return err + }) if err != nil { - return ur, err + log.Log.Error(err, "failed to update update request", "name", name) + } else { + log.Log.V(3).Info("updated update request", "name", name, "status") } - return newUR, nil + return ur, err +} + +func UpdateStatus(client kyvernoclient.Interface, urLister kyvernov1beta1listers.UpdateRequestNamespaceLister, name string, state kyvernov1beta1.UpdateRequestState, message string, genResources []kyvernov1.ResourceSpec) (*kyvernov1beta1.UpdateRequest, error) { + var ur *kyvernov1beta1.UpdateRequest + err := retry.RetryOnConflict(DefaultRetry, func() error { + ur, err := urLister.Get(name) + if err != nil { + log.Log.Error(err, "[ATTEMPT] failed to fetch update request", "name", name) + return err + } + ur = ur.DeepCopy() + ur.Status.State = state + ur.Status.Message = message + if genResources != nil { + ur.Status.GeneratedResources = genResources + } + _, err = client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).UpdateStatus(context.TODO(), ur, metav1.UpdateOptions{}) + if err != nil { + log.Log.Error(err, "[ATTEMPT] failed to update update request status", "name", name) + return err + } + return err + }) + if err != nil { + log.Log.Error(err, "failed to update update request status", "name", name) + } else { + log.Log.V(3).Info("updated update request status", "name", name, "status", string(state)) + } + return ur, err } diff --git a/pkg/background/generate/generate.go b/pkg/background/generate/generate.go index ef9350be28..864c2856ff 100644 --- a/pkg/background/generate/generate.go +++ b/pkg/background/generate/generate.go @@ -90,7 +90,7 @@ func NewGenerateController( urLister: urLister, } - c.statusControl = common.StatusControl{Client: kyvernoClient} + c.statusControl = common.NewStatusControl(kyvernoClient, urLister) c.nsLister = nsLister return &c, nil @@ -301,12 +301,19 @@ func (c *GenerateController) getPolicySpec(ur urkyverno.UpdateRequest) (kyverno. func updateStatus(statusControl common.StatusControlInterface, ur urkyverno.UpdateRequest, err error, genResources []kyverno.ResourceSpec, precreatedResource bool) error { if err != nil { - return statusControl.Failed(ur, err.Error(), genResources) + if _, err := statusControl.Failed(ur.GetName(), err.Error(), genResources); err != nil { + return err + } } else if precreatedResource { - return statusControl.Skip(ur, genResources) + if _, err := statusControl.Skip(ur.GetName(), genResources); err != nil { + return err + } + } else { + if _, err := statusControl.Success(ur.GetName(), genResources); err != nil { + return err + } } - - return statusControl.Success(ur, genResources) + return nil } func (c *GenerateController) applyGeneratePolicy(log logr.Logger, policyContext *engine.PolicyContext, ur urkyverno.UpdateRequest, applicableRules []string) (genResources []kyverno.ResourceSpec, processExisting bool, err error) { diff --git a/pkg/background/log.go b/pkg/background/log.go new file mode 100644 index 0000000000..c8d0aa7c8e --- /dev/null +++ b/pkg/background/log.go @@ -0,0 +1,5 @@ +package background + +import "sigs.k8s.io/controller-runtime/pkg/log" + +var logger = log.Log.WithName("background") diff --git a/pkg/background/mutate/mutate.go b/pkg/background/mutate/mutate.go index b5a8b06b8b..0554c22277 100644 --- a/pkg/background/mutate/mutate.go +++ b/pkg/background/mutate/mutate.go @@ -74,7 +74,7 @@ func NewMutateExistingController( Config: dynamicConfig, } - c.statusControl = common.StatusControl{Client: kyvernoClient} + c.statusControl = common.NewStatusControl(kyvernoClient, urLister) return &c, nil } @@ -184,10 +184,15 @@ func (c *MutateExistingController) report(err error, policy, rule string, target func updateURStatus(statusControl common.StatusControlInterface, ur urkyverno.UpdateRequest, err error) error { if err != nil { - return statusControl.Failed(ur, err.Error(), nil) + if _, err := statusControl.Failed(ur.GetName(), err.Error(), nil); err != nil { + return err + } + } else { + if _, err := statusControl.Success(ur.GetName(), nil); err != nil { + return err + } } - - return statusControl.Success(ur, nil) + return nil } func addAnnotation(policy kyvernov1.PolicyInterface, patched *unstructured.Unstructured, r response.RuleResponse) (patchedNew *unstructured.Unstructured, err error) { diff --git a/pkg/background/request_process.go b/pkg/background/request_process.go deleted file mode 100644 index 42a98fb731..0000000000 --- a/pkg/background/request_process.go +++ /dev/null @@ -1,75 +0,0 @@ -package background - -import ( - "context" - - kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" - "github.com/kyverno/kyverno/pkg/background/common" - "github.com/kyverno/kyverno/pkg/background/generate" - "github.com/kyverno/kyverno/pkg/background/mutate" - "github.com/kyverno/kyverno/pkg/config" - jsonutils "github.com/kyverno/kyverno/pkg/utils/json" - "github.com/pkg/errors" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/util/retry" -) - -func (c *Controller) processUR(ur *kyvernov1beta1.UpdateRequest) error { - switch ur.Spec.Type { - case kyvernov1beta1.Mutate: - ctrl, _ := mutate.NewMutateExistingController(c.kyvernoClient, c.client, - c.policyLister, c.npolicyLister, c.urLister, c.eventGen, c.log, c.Config) - return ctrl.ProcessUR(ur) - - case kyvernov1beta1.Generate: - ctrl, _ := generate.NewGenerateController(c.kyvernoClient, c.client, - c.policyLister, c.npolicyLister, c.urLister, c.eventGen, c.nsLister, c.log, c.Config, - ) - return ctrl.ProcessUR(ur) - } - return nil -} - -func (c *Controller) markUR(ur *kyvernov1beta1.UpdateRequest) (*kyvernov1beta1.UpdateRequest, bool, error) { - ur = ur.DeepCopy() - if ur.Status.Handler != "" { - return ur, ur.Status.Handler == config.KyvernoPodName, nil - } - err := retry.RetryOnConflict(common.DefaultRetry, func() error { - var retryError error - ur.Status.Handler = config.KyvernoPodName - ur, retryError = c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).UpdateStatus(context.TODO(), ur, metav1.UpdateOptions{}) - return retryError - }) - return ur, true, err -} - -func (c *Controller) unmarkUR(ur *kyvernov1beta1.UpdateRequest) error { - if _, err := c.patchHandler(ur, ""); err != nil { - return err - } - - if ur.Spec.Type == kyvernov1beta1.Mutate && ur.Status.State == kyvernov1beta1.Completed { - return c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).Delete(context.TODO(), ur.GetName(), metav1.DeleteOptions{}) - } - return nil -} - -func (c *Controller) patchHandler(ur *kyvernov1beta1.UpdateRequest, val string) (*kyvernov1beta1.UpdateRequest, error) { - patch := jsonutils.NewPatch( - "/status/handler", - "replace", - val, - ) - - updateUR, err := common.PatchUpdateRequest(ur, patch, c.kyvernoClient, "status") - if err != nil && !apierrors.IsNotFound(err) { - c.log.Error(err, "failed to patch UpdateRequest: %v", patch) - if val == "" { - return nil, errors.Wrapf(err, "failed to patch UpdateRequest to clear /status/handler") - } - return nil, errors.Wrapf(err, "failed to patch UpdateRequest to update /status/handler to %s", val) - } - return updateUR, nil -} diff --git a/pkg/background/update_request_controller.go b/pkg/background/update_request_controller.go index 5fc5e330b3..2b8cd6ac38 100644 --- a/pkg/background/update_request_controller.go +++ b/pkg/background/update_request_controller.go @@ -3,31 +3,31 @@ package background import ( "context" "fmt" - "reflect" "time" - "github.com/go-logr/logr" - kyverno "github.com/kyverno/kyverno/api/kyverno/v1" - urkyverno "github.com/kyverno/kyverno/api/kyverno/v1beta1" - "github.com/kyverno/kyverno/pkg/autogen" + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" + kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" common "github.com/kyverno/kyverno/pkg/background/common" + "github.com/kyverno/kyverno/pkg/background/generate" + "github.com/kyverno/kyverno/pkg/background/mutate" kyvernoclient "github.com/kyverno/kyverno/pkg/client/clientset/versioned" kyvernoinformer "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v1" urkyvernoinformer "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v1beta1" - kyvernolister "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1" - urlister "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1beta1" + kyvernov1listers "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1" + kyvernov1beta1listers "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1beta1" "github.com/kyverno/kyverno/pkg/config" dclient "github.com/kyverno/kyverno/pkg/dclient" "github.com/kyverno/kyverno/pkg/event" admissionv1 "k8s.io/api/admission/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" - coreinformers "k8s.io/client-go/informers/core/v1" + corev1informers "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" - corelister "k8s.io/client-go/listers/core/v1" + corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" + "k8s.io/client-go/util/retry" "k8s.io/client-go/util/workqueue" ) @@ -35,42 +35,28 @@ const ( maxRetries = 10 ) -// Controller manages the life-cycle for Generate-Requests and applies generate rule -type Controller struct { - // dynamic client implementation - client dclient.Interface +type Controller interface { + Run(int, <-chan struct{}) +} - // typed client for Kyverno CRDs +// controller manages the life-cycle for Generate-Requests and applies generate rule +type controller struct { + // clients + client dclient.Interface kyvernoClient kyvernoclient.Interface - policyInformer kyvernoinformer.ClusterPolicyInformer + // listers + policyLister kyvernov1listers.ClusterPolicyLister + npolicyLister kyvernov1listers.PolicyLister + urLister kyvernov1beta1listers.UpdateRequestNamespaceLister + nsLister corev1listers.NamespaceLister + podLister corev1listers.PodLister - // event generator interface - eventGen event.Interface - - // urStatusControl is used to update UR status - statusControl common.StatusControlInterface - - // UR that need to be synced + // queue queue workqueue.RateLimitingInterface - // policyLister can list/get cluster policy from the shared informer's store - policyLister kyvernolister.ClusterPolicyLister - - // policyLister can list/get Namespace policy from the shared informer's store - npolicyLister kyvernolister.PolicyLister - - // urLister can list/get update request from the shared informer's store - urLister urlister.UpdateRequestNamespaceLister - - // nsLister can list/get namespaces from the shared informer's store - nsLister corelister.NamespaceLister - - // nsLister can list/get pods from the shared informer's store - podLister corelister.PodLister - log logr.Logger - - Config config.Configuration + eventGen event.Interface + configuration config.Configuration } //NewController returns an instance of the Generate-Request Controller @@ -81,49 +67,44 @@ func NewController( policyInformer kyvernoinformer.ClusterPolicyInformer, npolicyInformer kyvernoinformer.PolicyInformer, urInformer urkyvernoinformer.UpdateRequestInformer, - podInformer coreinformers.PodInformer, + podInformer corev1informers.PodInformer, eventGen event.Interface, - namespaceInformer coreinformers.NamespaceInformer, - log logr.Logger, + namespaceInformer corev1informers.NamespaceInformer, dynamicConfig config.Configuration, -) *Controller { - c := Controller{ - client: client, - kyvernoClient: kyvernoClient, - policyInformer: policyInformer, - eventGen: eventGen, - queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "generate-request"), - log: log, - Config: dynamicConfig, - statusControl: common.StatusControl{Client: kyvernoClient}, - policyLister: policyInformer.Lister(), - npolicyLister: npolicyInformer.Lister(), - urLister: urInformer.Lister().UpdateRequests(config.KyvernoNamespace), - nsLister: namespaceInformer.Lister(), - podLister: podInformer.Lister(), +) Controller { + urLister := urInformer.Lister().UpdateRequests(config.KyvernoNamespace) + c := controller{ + client: client, + kyvernoClient: kyvernoClient, + policyLister: policyInformer.Lister(), + npolicyLister: npolicyInformer.Lister(), + urLister: urLister, + nsLister: namespaceInformer.Lister(), + podLister: podInformer.Lister(), + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "generate-request"), + eventGen: eventGen, + configuration: dynamicConfig, } urInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: c.addUR, UpdateFunc: c.updateUR, DeleteFunc: c.deleteUR, }) + policyInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + UpdateFunc: c.updatePolicy, // We only handle updates to policy + // Deletion of policy will be handled by cleanup controller + }) return &c } // Run starts workers -func (c *Controller) Run(workers int, stopCh <-chan struct{}) { - logger := c.log - defer utilruntime.HandleCrash() +func (c *controller) Run(workers int, stopCh <-chan struct{}) { + defer runtime.HandleCrash() defer c.queue.ShutDown() logger.Info("starting") defer logger.Info("shutting down") - c.policyInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ - UpdateFunc: c.updatePolicy, // We only handle updates to policy - // Deletion of policy will be handled by cleanup controller - }) - for i := 0; i < workers; i++ { go wait.Until(c.worker, time.Second, stopCh) } @@ -133,12 +114,12 @@ func (c *Controller) Run(workers int, stopCh <-chan struct{}) { // worker runs a worker thread that just dequeues items, processes them, and marks them done. // It enforces that the syncHandler is never invoked concurrently with the same key. -func (c *Controller) worker() { +func (c *controller) worker() { for c.processNextWorkItem() { } } -func (c *Controller) processNextWorkItem() bool { +func (c *controller) processNextWorkItem() bool { key, quit := c.queue.Get() if quit { return false @@ -150,8 +131,7 @@ func (c *Controller) processNextWorkItem() bool { return true } -func (c *Controller) handleErr(err error, key interface{}) { - logger := c.log +func (c *controller) handleErr(err error, key interface{}) { if err == nil { c.queue.Forget(key) return @@ -173,9 +153,7 @@ func (c *Controller) handleErr(err error, key interface{}) { c.queue.Forget(key) } -func (c *Controller) syncUpdateRequest(key string) error { - logger := c.log - var err error +func (c *controller) syncUpdateRequest(key string) error { startTime := time.Now() logger.V(4).Info("started sync", "key", key, "startTime", startTime) defer func() { @@ -187,12 +165,15 @@ func (c *Controller) syncUpdateRequest(key string) error { } ur, err := c.urLister.Get(urName) if err != nil { - if apierrors.IsNotFound(err) { - return nil - } - return fmt.Errorf("failed to fetch update request %s: %v", key, err) + return err + } + // if not in any state, try to set it to pending + if ur.Status.State == "" { + ur = ur.DeepCopy() + ur.Status.State = kyvernov1beta1.Pending + _, err := c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).UpdateStatus(context.TODO(), ur, metav1.UpdateOptions{}) + return err } - // if it was acquired by a pod that is gone, release it if ur.Status.Handler != "" { _, err = c.podLister.Pods(config.KyvernoNamespace).Get(ur.Status.Handler) @@ -205,58 +186,46 @@ func (c *Controller) syncUpdateRequest(key string) error { return err } } - - ur, ok, err := c.markUR(ur) + // if in pending state, try to acquire ur and eventually process it + if ur.Status.State == kyvernov1beta1.Pending { + ur, ok, err := c.acquireUR(ur) + if err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return fmt.Errorf("failed to mark handler for UR %s: %v", key, err) + } + if !ok { + logger.V(3).Info("another instance is handling the UR", "handler", ur.Status.Handler) + return nil + } + logger.V(3).Info("UR is marked successfully", "ur", ur.GetName(), "resourceVersion", ur.GetResourceVersion()) + if err := c.processUR(ur); err != nil { + return fmt.Errorf("failed to process UR %s: %v", key, err) + } + } + ur, err = c.releaseUR(ur) if err != nil { - return fmt.Errorf("failed to mark handler for UR %s: %v", key, err) - } - if !ok { - logger.V(3).Info("another instance is handling the UR", "handler", ur.Status.Handler) - return nil - } - logger.V(3).Info("UR is marked successfully", "ur", ur.GetName(), "resourceVersion", ur.GetResourceVersion()) - if err := c.processUR(ur); err != nil { - return fmt.Errorf("failed to process UR %s: %v", key, err) - } - if err = c.unmarkUR(ur); err != nil { return fmt.Errorf("failed to unmark UR %s: %v", key, err) } - return nil + err = c.cleanUR(ur) + return err } -func (c *Controller) enqueueUpdateRequest(obj interface{}) { +func (c *controller) enqueueUpdateRequest(obj interface{}) { key, err := cache.MetaNamespaceKeyFunc(obj) if err != nil { - c.log.Error(err, "failed to extract name") + logger.Error(err, "failed to extract name") return } - - c.log.V(5).Info("enqueued update request", "ur", key) + logger.V(5).Info("enqueued update request", "ur", key) c.queue.Add(key) } -func (c *Controller) updatePolicy(old, cur interface{}) { - logger := c.log - oldP := old.(*kyverno.ClusterPolicy) - curP := cur.(*kyverno.ClusterPolicy) +func (c *controller) updatePolicy(old, cur interface{}) { + oldP := old.(*kyvernov1.ClusterPolicy) + curP := cur.(*kyvernov1.ClusterPolicy) if oldP.ResourceVersion == curP.ResourceVersion { - // Periodic resync will send update events for all known Namespace. - // Two different versions of the same replica set will always have different RVs. - return - } - - var policyHasGenerate bool - for _, rule := range autogen.ComputeRules(curP) { - if rule.HasGenerate() { - policyHasGenerate = true - } - } - - if reflect.DeepEqual(curP.Spec, oldP.Spec) { - policyHasGenerate = false - } - - if !policyHasGenerate { return } @@ -275,54 +244,89 @@ func (c *Controller) updatePolicy(old, cur interface{}) { } } -func (c *Controller) addUR(obj interface{}) { - ur := obj.(*urkyverno.UpdateRequest) - if ur.Status.Handler != "" { - return - } +func (c *controller) addUR(obj interface{}) { + ur := obj.(*kyvernov1beta1.UpdateRequest) c.enqueueUpdateRequest(ur) } -func (c *Controller) updateUR(old, cur interface{}) { - oldUr := old.(*urkyverno.UpdateRequest) - curUr := cur.(*urkyverno.UpdateRequest) - if oldUr.ResourceVersion == curUr.ResourceVersion { - // Periodic resync will send update events for all known Namespace. - // Two different versions of the same replica set will always have different RVs. - return - } - // only process the ones that are in "Pending"/"Completed" state - // if the UPDATE Request fails due to incorrect policy, it will be requeued during policy update - if curUr.Status.State != urkyverno.Pending { - return - } - - if curUr.Status.Handler != "" { - return - } +func (c *controller) updateUR(_, cur interface{}) { + curUr := cur.(*kyvernov1beta1.UpdateRequest) c.enqueueUpdateRequest(curUr) } -func (c *Controller) deleteUR(obj interface{}) { - logger := c.log - ur, ok := obj.(*urkyverno.UpdateRequest) +func (c *controller) deleteUR(obj interface{}) { + ur, ok := obj.(*kyvernov1beta1.UpdateRequest) if !ok { tombstone, ok := obj.(cache.DeletedFinalStateUnknown) if !ok { logger.Info("Couldn't get object from tombstone", "obj", obj) return } - ur, ok = tombstone.Obj.(*urkyverno.UpdateRequest) + ur, ok = tombstone.Obj.(*kyvernov1beta1.UpdateRequest) if !ok { logger.Info("tombstone contained object that is not a Update Request CR", "obj", obj) return } } - - if ur.Status.Handler != "" { - return - } - // sync Handler will remove it from the queue c.enqueueUpdateRequest(ur) } + +func (c *controller) processUR(ur *kyvernov1beta1.UpdateRequest) error { + switch ur.Spec.Type { + case kyvernov1beta1.Mutate: + ctrl, _ := mutate.NewMutateExistingController(c.kyvernoClient, c.client, + c.policyLister, c.npolicyLister, c.urLister, c.eventGen, logger, c.configuration) + return ctrl.ProcessUR(ur) + + case kyvernov1beta1.Generate: + ctrl, _ := generate.NewGenerateController(c.kyvernoClient, c.client, + c.policyLister, c.npolicyLister, c.urLister, c.eventGen, c.nsLister, logger, c.configuration, + ) + return ctrl.ProcessUR(ur) + } + return nil +} + +func (c *controller) acquireUR(ur *kyvernov1beta1.UpdateRequest) (*kyvernov1beta1.UpdateRequest, bool, error) { + err := retry.RetryOnConflict(common.DefaultRetry, func() error { + var err error + ur, err = c.urLister.Get(ur.GetName()) + if err != nil { + return err + } + if ur.Status.Handler != "" { + return nil + } + ur = ur.DeepCopy() + ur.Status.Handler = config.KyvernoPodName + ur, err = c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).UpdateStatus(context.TODO(), ur, metav1.UpdateOptions{}) + return err + }) + return ur, ur.Status.Handler == config.KyvernoPodName, err +} + +func (c *controller) releaseUR(ur *kyvernov1beta1.UpdateRequest) (*kyvernov1beta1.UpdateRequest, error) { + err := retry.RetryOnConflict(common.DefaultRetry, func() error { + var err error + ur, err = c.urLister.Get(ur.GetName()) + if err != nil { + return err + } + if ur.Status.Handler != config.KyvernoPodName { + return nil + } + ur = ur.DeepCopy() + ur.Status.Handler = "" + ur, err = c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).UpdateStatus(context.TODO(), ur, metav1.UpdateOptions{}) + return err + }) + return ur, err +} + +func (c *controller) cleanUR(ur *kyvernov1beta1.UpdateRequest) error { + if ur.Spec.Type == kyvernov1beta1.Mutate && ur.Status.State == kyvernov1beta1.Completed { + return c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).Delete(context.TODO(), ur.GetName(), metav1.DeleteOptions{}) + } + return nil +} diff --git a/pkg/policy/policy_controller.go b/pkg/policy/policy_controller.go index b668f29c1d..180ef2e042 100644 --- a/pkg/policy/policy_controller.go +++ b/pkg/policy/policy_controller.go @@ -11,14 +11,16 @@ import ( "github.com/go-logr/logr" kyverno "github.com/kyverno/kyverno/api/kyverno/v1" - urkyverno "github.com/kyverno/kyverno/api/kyverno/v1beta1" + kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" utilscommon "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/common" "github.com/kyverno/kyverno/pkg/autogen" + common "github.com/kyverno/kyverno/pkg/background/common" kyvernoclient "github.com/kyverno/kyverno/pkg/client/clientset/versioned" "github.com/kyverno/kyverno/pkg/client/clientset/versioned/scheme" kyvernoinformer "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v1" urkyvernoinformer "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v1beta1" kyvernolister "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1" + kyvernov1beta1listers "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1beta1" urkyvernolister "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1beta1" "github.com/kyverno/kyverno/pkg/config" client "github.com/kyverno/kyverno/pkg/dclient" @@ -520,7 +522,7 @@ func generateTriggers(client client.Interface, rule kyverno.Rule, log logr.Logge return convertlist(list.Items) } -func deleteUR(kyvernoClient kyvernoclient.Interface, policyKey string, grList []*urkyverno.UpdateRequest, logger logr.Logger) { +func deleteUR(kyvernoClient kyvernoclient.Interface, policyKey string, grList []*kyvernov1beta1.UpdateRequest, logger logr.Logger) { for _, v := range grList { if policyKey == v.Spec.Policy { err := kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).Delete(context.TODO(), v.GetName(), metav1.DeleteOptions{}) @@ -531,29 +533,26 @@ func deleteUR(kyvernoClient kyvernoclient.Interface, policyKey string, grList [] } } -func updateUR(kyvernoClient kyvernoclient.Interface, policyKey string, urList []*urkyverno.UpdateRequest, logger logr.Logger) { +func updateUR(kyvernoClient kyvernoclient.Interface, urLister kyvernov1beta1listers.UpdateRequestNamespaceLister, policyKey string, urList []*kyvernov1beta1.UpdateRequest, logger logr.Logger) { for _, ur := range urList { if policyKey == ur.Spec.Policy { - urLabels := ur.Labels - if len(urLabels) == 0 { - urLabels = make(map[string]string) - } - - nBig, err := rand.Int(rand.Reader, big.NewInt(100000)) - if err != nil { - logger.Error(err, "failed to generate random interger") - } - urLabels["policy-update"] = fmt.Sprintf("revision-count-%d", nBig.Int64()) - ur.SetLabels(urLabels) - - new, err := kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).Update(context.TODO(), ur, metav1.UpdateOptions{}) + _, err := common.Update(kyvernoClient, urLister, ur.GetName(), func(ur *kyvernov1beta1.UpdateRequest) { + urLabels := ur.Labels + if len(urLabels) == 0 { + urLabels = make(map[string]string) + } + nBig, err := rand.Int(rand.Reader, big.NewInt(100000)) + if err != nil { + logger.Error(err, "failed to generate random interger") + } + urLabels["policy-update"] = fmt.Sprintf("revision-count-%d", nBig.Int64()) + ur.SetLabels(urLabels) + }) if err != nil { logger.Error(err, "failed to update gr", "name", ur.GetName()) continue } - - new.Status.State = urkyverno.Pending - if _, err := kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).UpdateStatus(context.TODO(), new, metav1.UpdateOptions{}); err != nil { + if _, err := common.UpdateStatus(kyvernoClient, urLister, ur.GetName(), kyvernov1beta1.Pending, "", nil); err != nil { logger.Error(err, "failed to set UpdateRequest state to Pending") } } diff --git a/pkg/policy/updaterequest.go b/pkg/policy/updaterequest.go index 5643bc018b..f9b4f39111 100644 --- a/pkg/policy/updaterequest.go +++ b/pkg/policy/updaterequest.go @@ -32,7 +32,7 @@ func (pc *PolicyController) updateUR(policyKey string, policy kyverno.PolicyInte var errors []error mutateURs := pc.listMutateURs(policyKey, nil) generateURs := pc.listGenerateURs(policyKey, nil) - updateUR(pc.kyvernoClient, policyKey, append(mutateURs, generateURs...), pc.log.WithName("updateUR")) + updateUR(pc.kyvernoClient, pc.urLister.UpdateRequests(config.KyvernoNamespace), policyKey, append(mutateURs, generateURs...), pc.log.WithName("updateUR")) for _, rule := range policy.GetSpec().Rules { var ruleType urkyverno.RequestType @@ -116,16 +116,10 @@ func (pc *PolicyController) handleUpdateRequest(ur *urkyverno.UpdateRequest, tri } pc.log.Info("creating new UR for generate") - new, err := pc.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).Create(context.TODO(), ur, metav1.CreateOptions{}) + _, err := pc.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).Create(context.TODO(), ur, metav1.CreateOptions{}) if err != nil { return false, err } - - new.Status.State = urkyverno.Pending - if _, err := pc.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).UpdateStatus(context.TODO(), new, metav1.UpdateOptions{}); err != nil { - pc.log.Error(err, "failed to set UpdateRequest state to Pending") - return false, err - } } return false, err } diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go index 98bbdf936d..a0ce189c8c 100644 --- a/pkg/webhooks/generation.go +++ b/pkg/webhooks/generation.go @@ -10,7 +10,7 @@ import ( "github.com/gardener/controller-manager-library/pkg/logger" "github.com/go-logr/logr" kyverno "github.com/kyverno/kyverno/api/kyverno/v1" - urkyverno "github.com/kyverno/kyverno/api/kyverno/v1beta1" + kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" "github.com/kyverno/kyverno/pkg/autogen" gencommon "github.com/kyverno/kyverno/pkg/background/common" gen "github.com/kyverno/kyverno/pkg/background/generate" @@ -23,7 +23,6 @@ import ( enginutils "github.com/kyverno/kyverno/pkg/engine/utils" "github.com/kyverno/kyverno/pkg/engine/variables" "github.com/kyverno/kyverno/pkg/event" - jsonutils "github.com/kyverno/kyverno/pkg/utils/json" "github.com/kyverno/kyverno/pkg/webhooks/updaterequest" admissionv1 "k8s.io/api/admission/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -75,7 +74,7 @@ func (ws *WebhookServer) handleGenerate( go ws.registerPolicyExecutionDurationMetricGenerate(logger, string(request.Operation), policy, *engineResponse) } - if failedResponse := applyUpdateRequest(request, urkyverno.Generate, ws.urGenerator, policyContext.AdmissionInfo, request.Operation, engineResponses...); failedResponse != nil { + if failedResponse := applyUpdateRequest(request, kyvernov1beta1.Generate, ws.urGenerator, policyContext.AdmissionInfo, request.Operation, engineResponses...); failedResponse != nil { // report failure event for _, failedUR := range failedResponse { err := fmt.Errorf("failed to create Update Request: %v", failedUR.err) @@ -133,12 +132,12 @@ func (ws *WebhookServer) handleUpdateGenerateSourceResource(resLabels map[string } } else { selector := labels.SelectorFromSet(labels.Set(map[string]string{ - urkyverno.URGeneratePolicyLabel: policyName, + kyvernov1beta1.URGeneratePolicyLabel: policyName, })) urList, err := ws.urLister.List(selector) if err != nil { - logger.Error(err, "failed to get update request for the resource", "label", urkyverno.URGeneratePolicyLabel) + logger.Error(err, "failed to get update request for the resource", "label", kyvernov1beta1.URGeneratePolicyLabel) return } @@ -152,31 +151,22 @@ func (ws *WebhookServer) handleUpdateGenerateSourceResource(resLabels map[string // updateAnnotationInUR - function used to update UR annotation // updating UR will trigger reprocessing of UR and recreation/updation of generated resource -func (ws *WebhookServer) updateAnnotationInUR(ur *urkyverno.UpdateRequest, logger logr.Logger) { - urAnnotations := ur.Annotations - if len(urAnnotations) == 0 { - urAnnotations = make(map[string]string) - } - ws.mu.Lock() - urAnnotations["generate.kyverno.io/updation-time"] = time.Now().String() - ur.SetAnnotations(urAnnotations) - ws.mu.Unlock() - - patch := jsonutils.NewPatch( - "/metadata/annotations", - "replace", - ur.Annotations, - ) - - new, err := gencommon.PatchUpdateRequest(ur, patch, ws.kyvernoClient) - if err != nil { +func (ws *WebhookServer) updateAnnotationInUR(ur *kyvernov1beta1.UpdateRequest, logger logr.Logger) { + if _, err := gencommon.Update(ws.kyvernoClient, ws.urLister, ur.GetName(), func(ur *kyvernov1beta1.UpdateRequest) { + urAnnotations := ur.Annotations + if len(urAnnotations) == 0 { + urAnnotations = make(map[string]string) + } + urAnnotations["generate.kyverno.io/updation-time"] = time.Now().String() + ur.SetAnnotations(urAnnotations) + }); err != nil { logger.Error(err, "failed to update update request update-time annotations for the resource", "update request", ur.Name) return } - new.Status.State = urkyverno.Pending - if _, err := ws.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).UpdateStatus(contextdefault.TODO(), new, metav1.UpdateOptions{}); err != nil { + if _, err := gencommon.UpdateStatus(ws.kyvernoClient, ws.urLister, ur.GetName(), kyvernov1beta1.Pending, "", nil); err != nil { logger.Error(err, "failed to set UpdateRequest state to Pending", "update request", ur.Name) } + } //handleUpdateGenerateTargetResource - handles update of target resource for generate policy @@ -354,7 +344,7 @@ func (ws *WebhookServer) handleDelete(request *admissionv1.AdmissionRequest) { return } - if ur.Spec.Type == urkyverno.Mutate { + if ur.Spec.Type == kyvernov1beta1.Mutate { return } ws.updateAnnotationInUR(ur, logger) @@ -364,7 +354,7 @@ func (ws *WebhookServer) handleDelete(request *admissionv1.AdmissionRequest) { func (ws *WebhookServer) deleteGR(logger logr.Logger, engineResponse *response.EngineResponse) { logger.V(4).Info("querying all update requests") selector := labels.SelectorFromSet(labels.Set(map[string]string{ - urkyverno.URGeneratePolicyLabel: engineResponse.PolicyResponse.Policy.Name, + kyvernov1beta1.URGeneratePolicyLabel: engineResponse.PolicyResponse.Policy.Name, "generate.kyverno.io/resource-name": engineResponse.PolicyResponse.Resource.Name, "generate.kyverno.io/resource-kind": engineResponse.PolicyResponse.Resource.Kind, "generate.kyverno.io/resource-namespace": engineResponse.PolicyResponse.Resource.Namespace, @@ -384,14 +374,14 @@ func (ws *WebhookServer) deleteGR(logger logr.Logger, engineResponse *response.E } } -func applyUpdateRequest(request *admissionv1.AdmissionRequest, ruleType urkyverno.RequestType, grGenerator updaterequest.Interface, userRequestInfo urkyverno.RequestInfo, +func applyUpdateRequest(request *admissionv1.AdmissionRequest, ruleType kyvernov1beta1.RequestType, grGenerator updaterequest.Interface, userRequestInfo kyvernov1beta1.RequestInfo, action admissionv1.Operation, engineResponses ...*response.EngineResponse) (failedUpdateRequest []updateRequestResponse) { requestBytes, err := json.Marshal(request) if err != nil { logger.Error(err, "error loading request into context") } - admissionRequestInfo := urkyverno.AdmissionRequestInfoObject{ + admissionRequestInfo := kyvernov1beta1.AdmissionRequestInfoObject{ AdmissionRequest: string(requestBytes), Operation: action, } @@ -406,7 +396,7 @@ func applyUpdateRequest(request *admissionv1.AdmissionRequest, ruleType urkyvern return } -func transform(admissionRequestInfo urkyverno.AdmissionRequestInfoObject, userRequestInfo urkyverno.RequestInfo, er *response.EngineResponse, ruleType urkyverno.RequestType) urkyverno.UpdateRequestSpec { +func transform(admissionRequestInfo kyvernov1beta1.AdmissionRequestInfoObject, userRequestInfo kyvernov1beta1.RequestInfo, er *response.EngineResponse, ruleType kyvernov1beta1.RequestType) kyvernov1beta1.UpdateRequestSpec { var PolicyNameNamespaceKey string if er.PolicyResponse.Policy.Namespace != "" { PolicyNameNamespaceKey = er.PolicyResponse.Policy.Namespace + "/" + er.PolicyResponse.Policy.Name @@ -414,7 +404,7 @@ func transform(admissionRequestInfo urkyverno.AdmissionRequestInfoObject, userRe PolicyNameNamespaceKey = er.PolicyResponse.Policy.Name } - ur := urkyverno.UpdateRequestSpec{ + ur := kyvernov1beta1.UpdateRequestSpec{ Type: ruleType, Policy: PolicyNameNamespaceKey, Resource: kyverno.ResourceSpec{ @@ -423,7 +413,7 @@ func transform(admissionRequestInfo urkyverno.AdmissionRequestInfoObject, userRe Name: er.PolicyResponse.Resource.Name, APIVersion: er.PolicyResponse.Resource.APIVersion, }, - Context: urkyverno.UpdateRequestSpecContext{ + Context: kyvernov1beta1.UpdateRequestSpecContext{ UserRequestInfo: userRequestInfo, AdmissionRequestInfo: admissionRequestInfo, }, @@ -433,6 +423,6 @@ func transform(admissionRequestInfo urkyverno.AdmissionRequestInfoObject, userRe } type updateRequestResponse struct { - ur urkyverno.UpdateRequestSpec + ur kyvernov1beta1.UpdateRequestSpec err error } diff --git a/pkg/webhooks/updaterequest/generator.go b/pkg/webhooks/updaterequest/generator.go index f527459fab..cd4632a532 100644 --- a/pkg/webhooks/updaterequest/generator.go +++ b/pkg/webhooks/updaterequest/generator.go @@ -7,7 +7,8 @@ import ( backoff "github.com/cenkalti/backoff" "github.com/gardener/controller-manager-library/pkg/logger" "github.com/go-logr/logr" - urkyverno "github.com/kyverno/kyverno/api/kyverno/v1beta1" + kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" + "github.com/kyverno/kyverno/pkg/background/common" kyvernoclient "github.com/kyverno/kyverno/pkg/client/clientset/versioned" urkyvernoinformer "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v1beta1" urkyvernolister "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1beta1" @@ -17,16 +18,17 @@ import ( "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 { - Apply(gr urkyverno.UpdateRequestSpec, action admissionv1.Operation) error + Apply(gr kyvernov1beta1.UpdateRequestSpec, action admissionv1.Operation) error } // info object stores message data to create update request type info struct { - spec urkyverno.UpdateRequestSpec + spec kyvernov1beta1.UpdateRequestSpec action admissionv1.Operation } @@ -51,7 +53,7 @@ func NewGenerator(client kyvernoclient.Interface, urInformer urkyvernoinformer.U } // Apply creates update request resource -func (g *Generator) Apply(ur urkyverno.UpdateRequestSpec, action admissionv1.Operation) error { +func (g *Generator) Apply(ur kyvernov1beta1.UpdateRequestSpec, action admissionv1.Operation) error { logger := g.log logger.V(4).Info("reconcile Update Request", "request", ur) @@ -89,10 +91,10 @@ func (g *Generator) generate(i info) error { return nil } -func retryApplyResource(client kyvernoclient.Interface, urSpec urkyverno.UpdateRequestSpec, +func retryApplyResource(client kyvernoclient.Interface, urSpec kyvernov1beta1.UpdateRequestSpec, log logr.Logger, action admissionv1.Operation, urLister urkyvernolister.UpdateRequestNamespaceLister) error { - if action == admissionv1.Delete && urSpec.Type == urkyverno.Generate { + if action == admissionv1.Delete && urSpec.Type == kyvernov1beta1.Generate { return nil } @@ -105,17 +107,17 @@ func retryApplyResource(client kyvernoclient.Interface, urSpec urkyverno.UpdateR } applyResource := func() error { - ur := urkyverno.UpdateRequest{ + ur := kyvernov1beta1.UpdateRequest{ Spec: urSpec, - Status: urkyverno.UpdateRequestStatus{ - State: urkyverno.Pending, + Status: kyvernov1beta1.UpdateRequestStatus{ + State: kyvernov1beta1.Pending, }, } queryLabels := make(map[string]string) - if ur.Spec.Type == urkyverno.Mutate { + if ur.Spec.Type == kyvernov1beta1.Mutate { queryLabels := map[string]string{ - urkyverno.URMutatePolicyLabel: ur.Spec.Policy, + 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, @@ -124,9 +126,9 @@ func retryApplyResource(client kyvernoclient.Interface, urSpec urkyverno.UpdateR if ur.Spec.Resource.APIVersion != "" { queryLabels["mutate.updaterequest.kyverno.io/trigger-apiversion"] = ur.Spec.Resource.APIVersion } - } else if ur.Spec.Type == urkyverno.Generate { + } else if ur.Spec.Type == kyvernov1beta1.Generate { queryLabels = labels.Set(map[string]string{ - urkyverno.URGeneratePolicyLabel: policyName, + 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, @@ -159,13 +161,19 @@ func retryApplyResource(client kyvernoclient.Interface, urSpec urkyverno.UpdateR } else { log.V(4).Info("successfully updated UpdateRequest", "retryCount", i, "name", ur.GetName(), "namespace", ur.GetNamespace()) } - - new.Status.State = urkyverno.Pending - if _, err := client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).UpdateStatus(context.TODO(), new, metav1.UpdateOptions{}); err != nil { + 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 } @@ -183,12 +191,6 @@ func retryApplyResource(client kyvernoclient.Interface, urSpec urkyverno.UpdateR } else { log.V(4).Info("successfully created UpdateRequest", "retryCount", i, "name", new.GetName(), "namespace", ur.GetNamespace()) } - - new.Status.State = urkyverno.Pending - if _, err := client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace).UpdateStatus(context.TODO(), new, metav1.UpdateOptions{}); err != nil { - log.Error(err, "failed to set UpdateRequest state to Pending") - return err - } } return nil diff --git a/test/e2e/generate/resources.go b/test/e2e/generate/resources.go index ff1f2d4296..7764f39cb7 100644 --- a/test/e2e/generate/resources.go +++ b/test/e2e/generate/resources.go @@ -255,6 +255,7 @@ metadata: name: add-networkpolicy spec: background: true + generateExistingOnPolicyUpdate: true rules: - name: allow-dns match: @@ -294,6 +295,7 @@ metadata: name: add-networkpolicy spec: background: true + generateExistingOnPolicyUpdate: true rules: - name: allow-dns match: @@ -333,6 +335,7 @@ metadata: name: add-networkpolicy spec: background: true + generateExistingOnPolicyUpdate: true rules: - name: allow-dns match: