diff --git a/helm/templates/validating-admission-policy.yaml b/helm/templates/validating-admission-policy.yaml new file mode 100644 index 000000000..9e5eb227b --- /dev/null +++ b/helm/templates/validating-admission-policy.yaml @@ -0,0 +1,33 @@ +{{- if not .Values.config.allowCRDDeletion }} +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicy +metadata: + name: {{ include "kro.fullname" . }}-crd-protection-policy + labels: + {{- include "kro.labels" . | nindent 4 }} +spec: + matchConstraints: + resourceRules: + - apiGroups: ["kro.run"] + apiVersions: ["v1alpha1"] + operations: ["DELETE"] + resources: ["resourcegraphdefinitions"] + validations: + - expression: | + has(oldObject.metadata.annotations) && + oldObject.metadata.annotations.exists(w, w == '{{ .Values.validation.admission.policy.rgd.protectionKey }}') && + oldObject.metadata.annotations['{{ .Values.validation.admission.policy.rgd.annotation.key }}'] == 'true' + reason: Invalid + message: | + Deletion denied. To proceed, set annotation '{{ .Values.validation.admission.policy.rgd.annotation.key }}: "true"'. Note: removing an RGD also deletes its CustomResourceDefinition and may cause data loss. +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicyBinding +metadata: + name: {{ include "kro.fullname" . }}-crd-protection-policy-binding + labels: + {{- include "kro.labels" . | nindent 4 }} +spec: + policyName: {{ include "kro.fullname" . }}-crd-protection-policy + validationActions: {{ .Values.validation.admission.policy.rgd.actions }} +{{- end }} \ No newline at end of file diff --git a/helm/values.yaml b/helm/values.yaml index 1167bec1f..f9314ea7b 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -153,3 +153,13 @@ metrics: # - action: replace # replacement: my-cluster # targetLabel: cluster + +validation: + admission: + policy: + rgd: + annotation: + # The Key on the RGD to use for the validation of delete + key: kro.run/allow-delete + # The value of the annotation to use for the validation of delete operations on RGDs + actions: '[Deny]' \ No newline at end of file diff --git a/pkg/controller/resourcegraphdefinition/controller.go b/pkg/controller/resourcegraphdefinition/controller.go index 33767d5e7..003fdfc24 100644 --- a/pkg/controller/resourcegraphdefinition/controller.go +++ b/pkg/controller/resourcegraphdefinition/controller.go @@ -20,14 +20,11 @@ import ( "github.com/go-logr/logr" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" ctrlrtcontroller "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -109,50 +106,18 @@ func (r *ResourceGraphDefinitionReconciler) SetupWithManager(mgr ctrl.Manager) e MaxConcurrentReconciles: r.maxConcurrentReconciles, }, ). - WatchesMetadata( - &extv1.CustomResourceDefinition{}, - handler.EnqueueRequestsFromMapFunc(r.findRGDsForCRD), - builder.WithPredicates(predicate.Funcs{ - UpdateFunc: func(e event.UpdateEvent) bool { - return true - }, - CreateFunc: func(e event.CreateEvent) bool { - return false - }, - DeleteFunc: func(e event.DeleteEvent) bool { - return false - }, - }), - ). - Complete(reconcile.AsReconciler[*v1alpha1.ResourceGraphDefinition](mgr.GetClient(), r)) -} - -// findRGDsForCRD returns a list of reconcile requests for the ResourceGraphDefinition -// that owns the given CRD. It is used to trigger reconciliation when a CRD is updated. -func (r *ResourceGraphDefinitionReconciler) findRGDsForCRD(ctx context.Context, obj client.Object) []reconcile.Request { - mobj, err := meta.Accessor(obj) - if err != nil { - return nil - } - - // Check if the CRD is owned by a ResourceGraphDefinition - if !metadata.IsKROOwned(mobj) { - return nil - } - - rgdName, ok := mobj.GetLabels()[metadata.ResourceGraphDefinitionNameLabel] - if !ok { - return nil - } - - // Return a reconcile request for the corresponding RGD - return []reconcile.Request{ - { - NamespacedName: types.NamespacedName{ - Name: rgdName, + Owns(&extv1.CustomResourceDefinition{}, builder.WithPredicates(predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + return true }, - }, - } + CreateFunc: func(e event.CreateEvent) bool { + return false + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return false + }, + }), builder.OnlyMetadata). + Complete(reconcile.AsReconciler[*v1alpha1.ResourceGraphDefinition](mgr.GetClient(), r)) } func (r *ResourceGraphDefinitionReconciler) Reconcile(ctx context.Context, o *v1alpha1.ResourceGraphDefinition) (ctrl.Result, error) { diff --git a/pkg/controller/resourcegraphdefinition/controller_reconcile.go b/pkg/controller/resourcegraphdefinition/controller_reconcile.go index c174e66b0..3794ee156 100644 --- a/pkg/controller/resourcegraphdefinition/controller_reconcile.go +++ b/pkg/controller/resourcegraphdefinition/controller_reconcile.go @@ -59,6 +59,10 @@ func (r *ResourceGraphDefinitionReconciler) reconcileResourceGraphDefinition( crd := processedRGD.Instance.GetCRD() graphExecLabeler.ApplyLabels(&crd.ObjectMeta) + if err := ctrl.SetControllerReference(rgd, crd, r.Scheme()); err != nil { + mark.KindUnready(err.Error()) + return nil, nil, fmt.Errorf("failed to set controller reference of CRD: %w", err) + } // Ensure CRD exists and is up to date log.V(1).Info("reconciling resource graph definition CRD") diff --git a/test/integration/suites/core/crd_test.go b/test/integration/suites/core/crd_test.go index 6ac5ec13a..7516fa275 100644 --- a/test/integration/suites/core/crd_test.go +++ b/test/integration/suites/core/crd_test.go @@ -104,6 +104,25 @@ var _ = Describe("CRD", func() { g.Expect(props["spec"].Properties["field2"].Default.Raw).To(Equal([]byte("42"))) }, 10*time.Second, time.Second).WithContext(ctx).Should(Succeed()) + // Check ownership relationship + owners := crd.GetOwnerReferences() + ownerRefExists := false + for _, owner := range owners { + if owner.Controller != nil && *owner.Controller { + ownerRefExists = true + Expect(owner.Name).To(Equal(rgd.Name)) + Expect(owner.Kind).To(Equal("ResourceGraphDefinition")) + Expect(owner.APIVersion).To(Equal(krov1alpha1.GroupVersion.String())) + Expect(owner.UID).To(Equal(rgd.UID)) + Expect(owner.BlockOwnerDeletion).ToNot(BeNil()) + Expect(*owner.BlockOwnerDeletion).To(BeTrue()) + Expect(owner.Controller).ToNot(BeNil()) + Expect(*owner.Controller).To(BeTrue()) + break + } + } + Expect(ownerRefExists).To(BeTrue(), "CRD should be owned by ResourceGraphDefinition") + Expect(env.Client.Delete(ctx, rgd)).To(Succeed()) }) diff --git a/website/docs/docs/concepts/00-resource-group-definitions.md b/website/docs/docs/concepts/00-resource-group-definitions.md index 569274e35..b3e850d0b 100644 --- a/website/docs/docs/concepts/00-resource-group-definitions.md +++ b/website/docs/docs/concepts/00-resource-group-definitions.md @@ -164,6 +164,39 @@ etc.) automatically. kro continuously monitors your ResourceGraphDefinition for changes, updating the API and its behavior accordingly. +## Deletion of ResourceGraphDefinitions + +Because **ResourceGraphDefinitions** are owners of **CustomResourceDefinitions** that get created to support Instances, +they are subject to the same lifecycle as CRDs, which makes them vulnerable to updates and accidental deletion. +When you delete a **ResourceGraphDefinition**, kro deletes the corresponding CRD and controller. + +That's why by default, **ResourceGraphDefinitions** are protected by a **ValidatingAdmissionPolicy** +that will prevent you from accidentally deleting them. To allow deletion anyhow, you can set the `kro.run/allow-delete` annotation to `true` on the **ResourceGraphDefinition**. + +This may cause errors like below: + +```shell +kubectl delete rgd simple-deployment-for-deletion-rgd +> The resourcegraphdefinitions "simple-deployment-for-deletion-rgd" is invalid: + ValidatingAdmissionPolicy 'allow-delete-resourcegraphdefinitions-with-annotation' with + binding 'allow-delete-resourcegraphdefinitions-with-annotation-binding' denied request: Deletion denied. + To proceed, set annotation 'kro.run/allow-delete: "true"'. + Note: removing an RGD also deletes its CustomResourceDefinition and may cause data loss. +``` + +:::info + +When installing via HELM, one can customize the key of the annotation with `validation.admission.policy.rgd.annotation.key` +You can disable the default ValidatingAdmissionPolicy by setting `config.allowCRDDeletion` to `true` + +::: + +:::warning + +When disabling this protection, you must ensure that you do not delete the CRD or controller manually without ensuring safe instance deletion. + +::: + ## Instance Example After the **ResourceGraphDefinition** is validated and registered in the cluster, users