[KFLUXBUGS-1299] Use HAS kubeconfig to set ownerref (#478)

* [KFLUXBUGS-1299] Use HAS kubeconfig to set ownerref Signed-off-by: John Collier <[email protected]> * Add comment Signed-off-by: John Collier <[email protected]> * Add another comment Signed-off-by: John Collier <[email protected]> --------- Signed-off-by: John Collier <[email protected]>
redhat-appstudio · May 21, 2024 · 4affbb7 · 4affbb7
1 parent ee2fc4b
commit 4affbb7
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 13 deletions.
diff --git 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
@@ -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,23 +92,29 @@ 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{
 					Development: true,
 					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)
 				}
 			}