diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 779b0b73c0..369d2ed3e4 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -5,11 +5,12 @@ import ( "encoding/json" "errors" "fmt" - "github.com/kyverno/kyverno/pkg/engine/response" "reflect" "strings" "time" + "github.com/kyverno/kyverno/pkg/engine/response" + "github.com/go-logr/logr" kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1" pkgcommon "github.com/kyverno/kyverno/pkg/common" @@ -34,7 +35,7 @@ func (c *Controller) processGR(gr *kyverno.GenerateRequest) error { var genResources []kyverno.ResourceSpec // 1 - Check if the resource exists - resource, err = getResource(c.client, gr.Spec.Resource) + resource, err = getResource(c.client, gr.Spec.Resource, c.log) if err != nil { // Don't update status logger.V(3).Info("resource does not exist or is pending creation, re-queueing", "details", err.Error()) diff --git a/pkg/generate/resource.go b/pkg/generate/resource.go index 0972bd8d56..748370d40e 100644 --- a/pkg/generate/resource.go +++ b/pkg/generate/resource.go @@ -1,23 +1,42 @@ package generate import ( + "time" + + logr "github.com/go-logr/logr" kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1" + "github.com/kyverno/kyverno/pkg/common" dclient "github.com/kyverno/kyverno/pkg/dclient" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -func getResource(client *dclient.Client, resourceSpec kyverno.ResourceSpec) (*unstructured.Unstructured, error) { - if resourceSpec.Kind == "Namespace" { - resourceSpec.Namespace = "" +func getResource(client *dclient.Client, resourceSpec kyverno.ResourceSpec, log logr.Logger) (*unstructured.Unstructured, error) { + + get := func() (*unstructured.Unstructured, error) { + if resourceSpec.Kind == "Namespace" { + resourceSpec.Namespace = "" + } + resource, err := client.GetResource(resourceSpec.APIVersion, resourceSpec.Kind, resourceSpec.Namespace, resourceSpec.Name) + if err != nil { + return nil, err + } + + if resource.GetDeletionTimestamp() != nil { + return nil, nil + } + + return resource, nil } - resource, err := client.GetResource(resourceSpec.APIVersion, resourceSpec.Kind, resourceSpec.Namespace, resourceSpec.Name) - if err != nil { + + retry := func() error { + _, err := get() + return err + } + + f := common.RetryFunc(time.Second, 30*time.Second, retry, log.WithName("getResource")) + if err := f(); err != nil { return nil, err } - if resource.GetDeletionTimestamp() != nil { - return nil, nil - } - - return resource, nil + return get() } diff --git a/pkg/webhookconfig/configmanager.go b/pkg/webhookconfig/configmanager.go index ca3a3aca6c..3f0c1f49cb 100644 --- a/pkg/webhookconfig/configmanager.go +++ b/pkg/webhookconfig/configmanager.go @@ -373,8 +373,10 @@ func (m *webhookConfigManager) reconcileWebhook(namespace, name string) error { return err } + ready := true if err := m.updateWebhookConfig(webhooks); err != nil { - return errors.Wrapf(err, "failed to update webhook configurations for policy %s/%s", namespace, name) + ready = false + logger.Error(err, "failed to update webhook configurations for policy") } // DELETION of the policy @@ -382,11 +384,13 @@ func (m *webhookConfigManager) reconcileWebhook(namespace, name string) error { return nil } - if err := m.updateStatus(policy); err != nil { + if err := m.updateStatus(policy, ready); err != nil { return errors.Wrapf(err, "failed to update policy status %s/%s", namespace, name) } - logger.Info("policy is ready to serve admission requests") + if ready { + logger.Info("policy is ready to serve admission requests") + } return nil } @@ -469,17 +473,17 @@ func (m *webhookConfigManager) buildWebhooks(namespace string) (res []*webhook, for _, p := range policies { if p.HasValidate() || p.HasGenerate() { if p.Spec.FailurePolicy != nil && *p.Spec.FailurePolicy == kyverno.Ignore { - m.mergeWebhook(validateIgnore, p) + m.mergeWebhook(validateIgnore, p, true) } else { - m.mergeWebhook(validateFail, p) + m.mergeWebhook(validateFail, p, true) } } if p.HasMutate() || p.HasGenerate() { if p.Spec.FailurePolicy != nil && *p.Spec.FailurePolicy == kyverno.Ignore { - m.mergeWebhook(mutateIgnore, p) + m.mergeWebhook(mutateIgnore, p, false) } else { - m.mergeWebhook(mutateFail, p) + m.mergeWebhook(mutateFail, p, false) } } } @@ -635,9 +639,9 @@ func (m *webhookConfigManager) compareAndUpdateWebhook(webhookKind, webhookName return nil } -func (m *webhookConfigManager) updateStatus(policy *kyverno.ClusterPolicy) error { +func (m *webhookConfigManager) updateStatus(policy *kyverno.ClusterPolicy, status bool) error { policyCopy := policy.DeepCopy() - policyCopy.Status.Ready = true + policyCopy.Status.Ready = status if policy.GetNamespace() == "" { _, err := m.kyvernoClient.KyvernoV1().ClusterPolicies().UpdateStatus(context.TODO(), policyCopy, v1.UpdateOptions{}) return err @@ -648,12 +652,19 @@ func (m *webhookConfigManager) updateStatus(policy *kyverno.ClusterPolicy) error } // mergeWebhook merges the matching kinds of the policy to webhook.rule -func (m *webhookConfigManager) mergeWebhook(dst *webhook, policy *kyverno.ClusterPolicy) { +func (m *webhookConfigManager) mergeWebhook(dst *webhook, policy *kyverno.ClusterPolicy, updateValidate bool) { matchedGVK := make([]string, 0) for _, rule := range policy.Spec.Rules { - matchedGVK = append(matchedGVK, rule.MatchKinds()...) + // matching kinds in generate policies need to be added to both webhook if rule.HasGenerate() { + matchedGVK = append(matchedGVK, rule.MatchKinds()...) matchedGVK = append(matchedGVK, rule.Generation.ResourceSpec.Kind) + continue + } + + if (updateValidate && rule.HasValidate()) || + (!updateValidate && rule.HasMutate()) { + matchedGVK = append(matchedGVK, rule.MatchKinds()...) } } diff --git a/test/e2e/generate/generate_test.go b/test/e2e/generate/generate_test.go index b89db4636f..c2ad92b0e4 100644 --- a/test/e2e/generate/generate_test.go +++ b/test/e2e/generate/generate_test.go @@ -1102,8 +1102,15 @@ func Test_Generate_Policy_Deletion_for_Clone(t *testing.T) { // ======= Check Generated Resources ======= By(fmt.Sprintf("Checking the generated resource (Configmap) in namespace : %s", tests.ResourceNamespace)) - _, err = e2eClient.GetNamespacedResource(cmGVR, tests.ResourceNamespace, tests.ConfigMapName) + err = e2e.GetWithRetry(1*time.Second, 15, func() error { + _, err := e2eClient.GetNamespacedResource(cmGVR, tests.ResourceNamespace, tests.ConfigMapName) + if err != nil { + return err + } + return nil + }) Expect(err).NotTo(HaveOccurred()) + // =========================================== // test: the generated resource is not updated if the source resource is updated