Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions helm/templates/validating-admission-policy.yaml
Original file line number Diff line number Diff line change
@@ -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 }}
10 changes: 10 additions & 0 deletions helm/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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]'
57 changes: 11 additions & 46 deletions pkg/controller/resourcegraphdefinition/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say we do want to reconcile RGDs when the CRD they manage gets deleted so we re-create them

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would agree, will confirm the general impl in todays comm call

},
}), builder.OnlyMetadata).
Complete(reconcile.AsReconciler[*v1alpha1.ResourceGraphDefinition](mgr.GetClient(), r))
}

func (r *ResourceGraphDefinitionReconciler) Reconcile(ctx context.Context, o *v1alpha1.ResourceGraphDefinition) (ctrl.Result, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
19 changes: 19 additions & 0 deletions test/integration/suites/core/crd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})

Expand Down
33 changes: 33 additions & 0 deletions website/docs/docs/concepts/00-resource-group-definitions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down