Merge pull request #124 from turkenh/sync-upstream-release-1.15

Sync upstream release 1.15 for v1.15.1

apis/apiextensions/v1alpha1/usage_types.go

@@ -68,6 +68,9 @@ type UsageSpec struct {
 	// Reason is the reason for blocking deletion of the resource.
 	// +optional
 	Reason *string `json:"reason,omitempty"`
+	// ReplayDeletion will trigger a deletion on the used resource during the deletion of the usage itself, if it was attempted to be deleted at least once.
+	// +optional
+	ReplayDeletion *bool `json:"replayDeletion,omitempty"`
 }
 
 // UsageStatus defines the observed state of Usage.

cluster/charts/crossplane/README.md

@@ -103,8 +103,8 @@ and their default values.
 | `registryCaBundleConfig.key` | The ConfigMap key containing a custom CA bundle to enable fetching packages from registries with unknown or untrusted certificates. | `""` |
 | `registryCaBundleConfig.name` | The ConfigMap name containing a custom CA bundle to enable fetching packages from registries with unknown or untrusted certificates. | `""` |
 | `replicas` | The number of Crossplane pod `replicas` to deploy. | `1` |
-| `resourcesCrossplane.limits.cpu` | CPU resource limits for the Crossplane pod. | `"100m"` |
-| `resourcesCrossplane.limits.memory` | Memory resource limits for the Crossplane pod. | `"512Mi"` |
+| `resourcesCrossplane.limits.cpu` | CPU resource limits for the Crossplane pod. | `"500m"` |
+| `resourcesCrossplane.limits.memory` | Memory resource limits for the Crossplane pod. | `"1024Mi"` |
 | `resourcesCrossplane.requests.cpu` | CPU resource requests for the Crossplane pod. | `"100m"` |
 | `resourcesCrossplane.requests.memory` | Memory resource requests for the Crossplane pod. | `"256Mi"` |
 | `resourcesRBACManager.limits.cpu` | CPU resource limits for the RBAC Manager pod. | `"100m"` |

cluster/charts/crossplane/values.yaml

@@ -85,9 +85,9 @@ priorityClassName: ""
 resourcesCrossplane:
   limits:
     # -- CPU resource limits for the Crossplane pod.
-    cpu: 100m
+    cpu: 500m
     # -- Memory resource limits for the Crossplane pod.
-    memory: 512Mi
+    memory: 1024Mi
   requests:
     # -- CPU resource requests for the Crossplane pod.
     cpu: 100m

cluster/crds/apiextensions.crossplane.io_usages.yaml

@@ -138,6 +138,11 @@ spec:
               reason:
                 description: Reason is the reason for blocking deletion of the resource.
                 type: string
+              replayDeletion:
+                description: ReplayDeletion will trigger a deletion on the used resource
+                  during the deletion of the usage itself, if it was attempted to
+                  be deleted at least once.
+                type: boolean
             required:
             - of
             type: object

cmd/crossplane/core/core.go

@@ -95,7 +95,7 @@ type startCommand struct {
 
 	SyncInterval     time.Duration `short:"s" help:"How often all resources will be double-checked for drift from the desired state." default:"1h"`
 	PollInterval     time.Duration `help:"How often individual resources will be checked for drift from the desired state." default:"1m"`
-	MaxReconcileRate int           `help:"The global maximum rate per second at which resources may checked for drift from the desired state." default:"10"`
+	MaxReconcileRate int           `help:"The global maximum rate per second at which resources may checked for drift from the desired state." default:"100"`
 
 	WebhookEnabled bool `help:"Enable webhook configuration." default:"true" env:"WEBHOOK_ENABLED"`
 

internal/controller/apiextensions/composite/composition_render.go

@@ -32,8 +32,8 @@ const (
 	errMarshalProtoStruct = "cannot marshal protobuf Struct to JSON"
 	errSetControllerRef   = "cannot set controller reference"
 
-	errFmtKindOrGroupChanged = "cannot change the kind or group of a composed resource from %s to %s (possible composed resource template mismatch)"
-	errFmtNamePrefixLabel    = "cannot find top-level composite resource name label %q in composite resource metadata"
+	errFmtKindChanged     = "cannot change the kind of a composed resource from %s to %s (possible composed resource template mismatch)"
+	errFmtNamePrefixLabel = "cannot find top-level composite resource name label %q in composite resource metadata"
 
 	// TODO(negz): Include more detail such as field paths if they exist.
 	// Perhaps require each patch type to have a String() method to help
@@ -61,16 +61,18 @@ func RenderFromJSON(o resource.Object, data []byte) error {
 	o.SetName(name)
 	o.SetNamespace(namespace)
 
-	// This resource already had a GK (probably because it already exists), but
+	// This resource already had a Kind (probably because it already exists), but
 	// when we rendered its template it changed. This shouldn't happen. Either
-	// someone changed the kind or group in the template, or we're trying to use the
+	// someone changed the kind in the template, or we're trying to use the
 	// wrong template (e.g. because the order of an array of anonymous templates
 	// changed).
 	// Please note, we don't check for version changes, as versions can change. For example,
 	// if a composed resource was created with a template that has a version of "v1alpha1",
 	// and then the template is updated to "v1beta1", the composed resource will still be valid.
-	if !gvk.Empty() && o.GetObjectKind().GroupVersionKind().GroupKind() != gvk.GroupKind() {
-		return errors.Errorf(errFmtKindOrGroupChanged, gvk, o.GetObjectKind().GroupVersionKind())
+	// We also don't check for group changes, as groups can change during
+	// migrations.
+	if !gvk.Empty() && o.GetObjectKind().GroupVersionKind().Kind != gvk.Kind {
+		return errors.Errorf(errFmtKindChanged, gvk, o.GetObjectKind().GroupVersionKind())
 	}
 
 	return nil

internal/controller/apiextensions/composite/composition_render_test.go

@@ -62,38 +62,37 @@ func TestRenderFromJSON(t *testing.T) {
 				err: errors.Wrap(errInvalidChar, errUnmarshalJSON),
 			},
 		},
-		"ExistingGroupChanged": {
-			reason: "We should return an error if unmarshalling the base template changed the composed resource's group.",
+		"ExistingKindChanged": {
+			reason: "We should return an error if unmarshalling the base template changed the composed resource's kind.",
 			args: args{
 				o: composed.New(composed.FromReference(corev1.ObjectReference{
 					APIVersion: "example.org/v1",
 					Kind:       "Potato",
 				})),
-				data: []byte(`{"apiVersion": "foo.io/v1", "kind": "Potato"}`),
+				data: []byte(`{"apiVersion": "example.org/v1", "kind": "Different"}`),
 			},
 			want: want{
 				o: composed.New(composed.FromReference(corev1.ObjectReference{
-					APIVersion: "foo.io/v1",
-					Kind:       "Potato",
+					APIVersion: "example.org/v1",
+					Kind:       "Different",
 				})),
-				err: errors.Errorf(errFmtKindOrGroupChanged, "example.org/v1, Kind=Potato", "foo.io/v1, Kind=Potato"),
+				err: errors.Errorf(errFmtKindChanged, "example.org/v1, Kind=Potato", "example.org/v1, Kind=Different"),
 			},
 		},
-		"ExistingKindChanged": {
-			reason: "We should return an error if unmarshalling the base template changed the composed resource's kind.",
+		"GroupCanChange": {
+			reason: "We should accept group changes in the base template.",
 			args: args{
 				o: composed.New(composed.FromReference(corev1.ObjectReference{
 					APIVersion: "example.org/v1",
 					Kind:       "Potato",
 				})),
-				data: []byte(`{"apiVersion": "example.org/v1", "kind": "Different"}`),
+				data: []byte(`{"apiVersion": "foo.io/v1", "kind": "Potato"}`),
 			},
 			want: want{
 				o: composed.New(composed.FromReference(corev1.ObjectReference{
-					APIVersion: "example.org/v1",
-					Kind:       "Different",
+					APIVersion: "foo.io/v1",
+					Kind:       "Potato",
 				})),
-				err: errors.Errorf(errFmtKindOrGroupChanged, "example.org/v1, Kind=Potato", "example.org/v1, Kind=Different"),
 			},
 		},
 		"VersionCanChange": {

internal/controller/apiextensions/usage/reconciler.go

@@ -86,6 +86,7 @@ const (
 	reasonRemoveInUseLabel event.Reason = "RemoveInUseLabel"
 	reasonAddFinalizer     event.Reason = "AddFinalizer"
 	reasonRemoveFinalizer  event.Reason = "RemoveFinalizer"
+	reasonReplayDeletion   event.Reason = "ReplayDeletion"
 
 	reasonUsageConfigured event.Reason = "UsageConfigured"
 	reasonWaitUsing       event.Reason = "WaitingUsingDeleted"
@@ -316,6 +317,23 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 			}
 		}
 
+		if u.Spec.ReplayDeletion != nil && *u.Spec.ReplayDeletion && used.GetAnnotations() != nil {
+			if policy, ok := used.GetAnnotations()[usage.AnnotationKeyDeletionAttempt]; ok {
+				// We have already recorded a deletion attempt and want to replay deletion, let's delete the used resource.
+				go func() {
+					// We do the deletion async and after some delay to make sure the usage is deleted before the
+					// deletion attempt. We remove the finalizer on this Usage right below, so, we know it will disappear
+					// very soon.
+					time.Sleep(2 * time.Second)
+					log.Info("Replaying deletion of the used resource", "apiVersion", used.GetAPIVersion(), "kind", used.GetKind(), "name", used.GetName(), "policy", policy)
+					// We cannot use the context from the reconcile function since it will be cancelled after the reconciliation.
+					if err = r.client.Delete(context.Background(), used, client.PropagationPolicy(policy)); err != nil {
+						log.Info("Error when replaying deletion of the used resource", "apiVersion", used.GetAPIVersion(), "kind", used.GetKind(), "name", used.GetName(), "err", err)
+					}
+				}()
+			}
+		}
+
 		// Remove the finalizer from the usage
 		if err = r.usage.RemoveFinalizer(ctx, u); err != nil {
 			log.Debug(errRemoveFinalizer, "error", err)

internal/controller/apiextensions/usage/reconciler_test.go

@@ -26,6 +26,7 @@ import (
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -38,6 +39,7 @@ import (
 	"github.com/crossplane/crossplane-runtime/pkg/test"
 
 	"github.com/crossplane/crossplane/apis/apiextensions/v1alpha1"
+	"github.com/crossplane/crossplane/internal/usage"
 	"github.com/crossplane/crossplane/internal/xcrd"
 )
 
@@ -723,6 +725,58 @@ func TestReconcile(t *testing.T) {
 				r: reconcile.Result{},
 			},
 		},
+		"SuccessfulDeleteWithReplayDeletion": {
+			reason: "We should replay deletion after usage is gone and replayDeletion is true.",
+			args: args{
+				mgr: &fake.Manager{},
+				opts: []ReconcilerOption{
+					WithClientApplicator(xpresource.ClientApplicator{
+						Client: &test.MockClient{
+							MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
+								if o, ok := obj.(*v1alpha1.Usage); ok {
+									o.SetDeletionTimestamp(&now)
+									o.Spec.ReplayDeletion = ptr.To(true)
+									o.Spec.Of.ResourceRef = &v1alpha1.ResourceRef{Name: "cool"}
+									return nil
+								}
+								if o, ok := obj.(*composed.Unstructured); ok {
+									o.SetAnnotations(map[string]string{usage.AnnotationKeyDeletionAttempt: string(metav1.DeletePropagationBackground)})
+									o.SetLabels(map[string]string{inUseLabelKey: "true"})
+									return nil
+								}
+								return errors.New("unexpected object type")
+							}),
+							MockList: test.NewMockListFn(nil, func(_ client.ObjectList) error {
+								return nil
+							}),
+							MockUpdate: test.NewMockUpdateFn(nil, func(obj client.Object) error {
+								if o, ok := obj.(*composed.Unstructured); ok {
+									if o.GetLabels()[inUseLabelKey] != "" {
+										t.Errorf("expected in use label to be removed")
+									}
+									return nil
+								}
+								return errors.New("unexpected object type")
+							}),
+							MockDelete: func(_ context.Context, _ client.Object, _ ...client.DeleteOption) error {
+								return nil
+							},
+						},
+					}),
+					WithSelectorResolver(fakeSelectorResolver{
+						resourceSelectorFn: func(_ context.Context, _ *v1alpha1.Usage) error {
+							return nil
+						},
+					}),
+					WithFinalizer(xpresource.FinalizerFns{RemoveFinalizerFn: func(_ context.Context, _ xpresource.Object) error {
+						return nil
+					}}),
+				},
+			},
+			want: want{
+				r: reconcile.Result{},
+			},
+		},
 		"SuccessfulWaitWhenUsingStillThere": {
 			reason: "We should wait until the using resource is deleted.",
 			args: args{

internal/usage/handler.go

@@ -30,10 +30,12 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+	"sigs.k8s.io/yaml"
 
 	"github.com/crossplane/crossplane-runtime/pkg/controller"
 	"github.com/crossplane/crossplane-runtime/pkg/errors"
 	"github.com/crossplane/crossplane-runtime/pkg/logging"
+	xpmeta "github.com/crossplane/crossplane-runtime/pkg/meta"
 	xpunstructured "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured"
 
 	"github.com/crossplane/crossplane/apis/apiextensions/v1alpha1"
@@ -44,6 +46,11 @@ const (
 	// indexing and retrieving needed CRDs
 	InUseIndexKey = "inuse.apiversion.kind.name"
 
+	// AnnotationKeyDeletionAttempt is the annotation key used to record whether
+	// a deletion attempt was made and blocked by the Usage. The value stored is
+	// the propagation policy used with the deletion attempt.
+	AnnotationKeyDeletionAttempt = "usage.crossplane.io/deletion-attempt-with-policy"
+
 	// Error strings.
 	errFmtUnexpectedOp = "unexpected operation %q, expected \"DELETE\""
 )
@@ -87,7 +94,7 @@ func SetupWebhookWithManager(mgr ctrl.Manager, options controller.Options) error
 
 // Handler implements the admission Handler for Composition.
 type Handler struct {
-	reader client.Reader
+	client client.Client
 	log    logging.Logger
 }
 
@@ -102,9 +109,9 @@ func WithLogger(l logging.Logger) HandlerOption {
 }
 
 // NewHandler returns a new Handler.
-func NewHandler(reader client.Reader, opts ...HandlerOption) *Handler {
+func NewHandler(client client.Client, opts ...HandlerOption) *Handler {
 	h := &Handler{
-		reader: reader,
+		client: client,
 		log:    logging.NewNopLogger(),
 	}
 
@@ -126,22 +133,44 @@ func (h *Handler) Handle(ctx context.Context, request admission.Request) admissi
 		if err := u.UnmarshalJSON(request.OldObject.Raw); err != nil {
 			return admission.Errored(http.StatusBadRequest, err)
 		}
-		return h.validateNoUsages(ctx, u)
+		opts := &metav1.DeleteOptions{}
+		if err := yaml.Unmarshal(request.Options.Raw, opts); err != nil {
+			return admission.Errored(http.StatusBadRequest, err)
+		}
+		return h.validateNoUsages(ctx, u, opts)
 	default:
 		return admission.Errored(http.StatusBadRequest, errors.Errorf(errFmtUnexpectedOp, request.Operation))
 	}
 }
 
-func (h *Handler) validateNoUsages(ctx context.Context, u *unstructured.Unstructured) admission.Response {
-	h.log.Debug("Validating no usages", "apiVersion", u.GetAPIVersion(), "kind", u.GetKind(), "name", u.GetName())
+func (h *Handler) validateNoUsages(ctx context.Context, u *unstructured.Unstructured, opts *metav1.DeleteOptions) admission.Response {
+	h.log.Debug("Validating no usages", "apiVersion", u.GetAPIVersion(), "kind", u.GetKind(), "name", u.GetName(), "policy", opts.PropagationPolicy)
 	usageList := &v1alpha1.UsageList{}
-	if err := h.reader.List(ctx, usageList, client.MatchingFields{InUseIndexKey: IndexValueForObject(u)}); err != nil {
+	if err := h.client.List(ctx, usageList, client.MatchingFields{InUseIndexKey: IndexValueForObject(u)}); err != nil {
 		h.log.Debug("Error when getting Usages", "apiVersion", u.GetAPIVersion(), "kind", u.GetKind(), "name", u.GetName(), "err", err)
 		return admission.Errored(http.StatusInternalServerError, err)
 	}
 	if len(usageList.Items) > 0 {
 		msg := inUseMessage(usageList)
 		h.log.Debug("Usage found, deletion not allowed", "apiVersion", u.GetAPIVersion(), "kind", u.GetKind(), "name", u.GetName(), "msg", msg)
+
+		// Use the default propagation policy if not provided
+		policy := metav1.DeletePropagationBackground
+		if opts.PropagationPolicy != nil {
+			policy = *opts.PropagationPolicy
+		}
+		// If the resource is being deleted, we want to record the first deletion attempt
+		// so that we can track whether a deletion was attempted at least once.
+		if u.GetAnnotations() == nil || u.GetAnnotations()[AnnotationKeyDeletionAttempt] != string(policy) {
+			orig := u.DeepCopy()
+			xpmeta.AddAnnotations(u, map[string]string{AnnotationKeyDeletionAttempt: string(policy)})
+			// Patch the resource to add the deletion attempt annotation
+			if err := h.client.Patch(ctx, u, client.MergeFrom(orig)); err != nil {
+				h.log.Debug("Error when patching the resource to add the deletion attempt annotation", "apiVersion", u.GetAPIVersion(), "kind", u.GetKind(), "name", u.GetName(), "err", err)
+				return admission.Errored(http.StatusInternalServerError, err)
+			}
+		}
+
 		return admission.Response{
 			AdmissionResponse: admissionv1.AdmissionResponse{
 				Allowed: false,