diff --git a/controllers/webhooks/component_webhook.go b/controllers/webhooks/component_webhook.go index f77f5f097..fa5771d24 100644 --- a/controllers/webhooks/component_webhook.go +++ b/controllers/webhooks/component_webhook.go @@ -65,22 +65,40 @@ func (r *ComponentWebhook) Default(ctx context.Context, obj runtime.Object) erro if len(component.OwnerReferences) == 0 && component.DeletionTimestamp.IsZero() { // Get the Application CR + // Use the background context to ensure the operator's kubeconfig is used hasApplication := appstudiov1alpha1.Application{} - err := r.client.Get(ctx, types.NamespacedName{Name: component.Spec.Application, Namespace: component.Namespace}, &hasApplication) + err := r.client.Get(context.Background(), types.NamespacedName{Name: component.Spec.Application, Namespace: component.Namespace}, &hasApplication) if err != nil { // Don't block if the Application doesn't exist yet - this will retrigger whenever the resource is modified err = fmt.Errorf("unable to get the Application %s for Component %s, ignoring for now", component.Spec.Application, compName) componentlog.Error(err, "skip setting owner reference on component") } else { - ownerReference := metav1.OwnerReference{ - APIVersion: hasApplication.APIVersion, - Kind: hasApplication.Kind, - Name: hasApplication.Name, - UID: hasApplication.UID, + // Update the Component's owner ref's - retry on conflict + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { + var curComp appstudiov1alpha1.Component + // Get the Component to update using the operator's kubeconfig so that there aren't any permissions issues setting the owner reference + // Use the background context to ensure the operator's kubeconfig is used + err := r.client.Get(context.Background(), types.NamespacedName{Name: compName, Namespace: component.Namespace}, &curComp) + if err != nil { + componentlog.Error(err, "unable to get current component, so skip setting owner reference") + return nil + } + + ownerReference := metav1.OwnerReference{ + APIVersion: hasApplication.APIVersion, + Kind: hasApplication.Kind, + Name: hasApplication.Name, + UID: hasApplication.UID, + } + curComp.SetOwnerReferences(append(curComp.GetOwnerReferences(), ownerReference)) + err = r.client.Update(ctx, &curComp) + return err + }) + if err != nil { + componentlog.Error(err, "error setting owner-references") } - component.SetOwnerReferences(append(component.GetOwnerReferences(), ownerReference)) - } + } } return nil diff --git a/controllers/webhooks/component_webhook_unit_test.go b/controllers/webhooks/component_webhook_unit_test.go index 624013b85..aa72146df 100644 --- a/controllers/webhooks/component_webhook_unit_test.go +++ b/controllers/webhooks/component_webhook_unit_test.go @@ -66,7 +66,7 @@ func TestComponentDefaultingWebhook(t *testing.T) { client: fakeClient, comp: appstudiov1alpha1.Component{ ObjectMeta: v1.ObjectMeta{ - Name: "1-test-component", + Name: "1-test-component-owner-ref", Namespace: "default", }, Spec: appstudiov1alpha1.ComponentSpec{ @@ -80,7 +80,7 @@ func TestComponentDefaultingWebhook(t *testing.T) { client: fakeClient, comp: appstudiov1alpha1.Component{ ObjectMeta: v1.ObjectMeta{ - Name: "1-test-component", + Name: "1-test-component-not-found", Namespace: "default", }, Spec: appstudiov1alpha1.ComponentSpec{ @@ -92,6 +92,8 @@ func TestComponentDefaultingWebhook(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { + err := test.client.Create(context.Background(), &test.comp) + assert.Nil(t, err) compWebhook := ComponentWebhook{ client: test.client, log: zap.New(zap.UseFlagOptions(&zap.Options{ @@ -99,16 +101,20 @@ func TestComponentDefaultingWebhook(t *testing.T) { TimeEncoder: zapcore.ISO8601TimeEncoder, })), } - err := compWebhook.Default(context.Background(), &test.comp) + err = compWebhook.Default(context.Background(), &test.comp) // Defaulting webhook should not return an error assert.Nil(t, err) // Ensure that the component had its owner reference set correctly - if len(test.comp.OwnerReferences) != 1 && test.name != "application not found" { + // Get the updated component + var updatedComp appstudiov1alpha1.Component + test.client.Get(context.Background(), types.NamespacedName{Namespace: "default", Name: test.comp.Name}, &updatedComp) + + if len(updatedComp.OwnerReferences) != 1 && test.name != "application not found" { t.Error("expected component to have owner reference set") } else if test.name != "application not found" { - if test.comp.OwnerReferences[0].Name != "application1" { + if updatedComp.OwnerReferences[0].Name != "application1" { t.Errorf("expected component to have owner reference set to application %s, got %s", "application1", test.comp.OwnerReferences[0].Name) } }