[KFLUXBUGS-1290] Handle ownerrefs in webhook (#475)

* [KFLUXBUGS-1290] Handle ownerrefs in webhook Signed-off-by: John Collier <[email protected]> * DeletionTimestamp check Signed-off-by: John Collier <[email protected]> * Fix typo Signed-off-by: John Collier <[email protected]> * Fix gosec Signed-off-by: John Collier <[email protected]> --------- Signed-off-by: John Collier <[email protected]>
redhat-appstudio · May 16, 2024 · bfa086d · bfa086d
1 parent 55667ac
commit bfa086d
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 3 deletions.
diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml
@@ -101,7 +101,7 @@ jobs:
         uses: codecov/[email protected]
       - name: Run Gosec Security Scanner
         run: |
-          go install github.com/securego/gosec/v2/cmd/gosec@latest
+          go install github.com/securego/gosec/v2/cmd/gosec@v2.19.0
           make gosec
           if [[ $? != 0 ]]
           then

diff --git a/Makefile b/Makefile
@@ -137,7 +137,7 @@ vet: ## Run go vet against code.
 .PHONY: gosec
 gosec:
 	# Run this command to install gosec, if not installed:
-	# go install github.com/securego/gosec/v2/cmd/gosec@latest
+	# go install github.com/securego/gosec/v2/cmd/gosec@v2.19.0
 	gosec -no-fail -fmt=sarif -out=gosec.sarif  ./...
 
 lint:

diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -44,6 +44,7 @@ webhooks:
     - UPDATE
     resources:
     - components
+    - components/status
   sideEffects: None
 ---
 apiVersion: admissionregistration.k8s.io/v1

diff --git a/controllers/webhooks/component_webhook.go b/controllers/webhooks/component_webhook.go
@@ -27,6 +27,7 @@ import (
 
 	"github.com/go-logr/logr"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/validation"
@@ -54,10 +55,34 @@ func (w *ComponentWebhook) Register(mgr ctrl.Manager, log *logr.Logger) error {
 
 // EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
 
-// +kubebuilder:webhook:path=/mutate-appstudio-redhat-com-v1alpha1-component,mutating=true,failurePolicy=fail,sideEffects=None,groups=appstudio.redhat.com,resources=components,verbs=create;update,versions=v1alpha1,name=mcomponent.kb.io,admissionReviewVersions=v1
+// +kubebuilder:webhook:path=/mutate-appstudio-redhat-com-v1alpha1-component,mutating=true,failurePolicy=fail,sideEffects=None,groups=appstudio.redhat.com,resources=components;components/status,verbs=create;update,versions=v1alpha1,name=mcomponent.kb.io,admissionReviewVersions=v1
 
 // Default implements webhook.Defaulter so a webhook will be registered for the type
 func (r *ComponentWebhook) Default(ctx context.Context, obj runtime.Object) error {
+	component := obj.(*appstudiov1alpha1.Component)
+	compName := component.Name
+	componentlog := r.log.WithValues("controllerKind", "Component").WithValues("name", compName).WithValues("namespace", component.Namespace)
+
+	if len(component.OwnerReferences) == 0 && component.DeletionTimestamp.IsZero() {
+		// Get the Application CR
+		hasApplication := appstudiov1alpha1.Application{}
+		err := r.client.Get(ctx, 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,
+			}
+			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
@@ -35,6 +35,87 @@ import (
 	"k8s.io/client-go/kubernetes/scheme"
 )
 
+func TestComponentDefaultingWebhook(t *testing.T) {
+
+	fakeClient := setUpComponents(t)
+
+	app := appstudiov1alpha1.Application{
+		ObjectMeta: v1.ObjectMeta{
+			Name:      "application1",
+			Namespace: "default",
+		},
+		TypeMeta: v1.TypeMeta{
+			APIVersion: "appstudio.redhat.com/v1alpha1",
+			Kind:       "Component",
+		},
+		Spec: appstudiov1alpha1.ApplicationSpec{
+			DisplayName: "app",
+			Description: "Description",
+		},
+	}
+	err := fakeClient.Create(context.Background(), &app)
+	require.NoError(t, err)
+
+	tests := []struct {
+		name   string
+		client client.Client
+		comp   appstudiov1alpha1.Component
+	}{
+		{
+			name:   "component has owner ref set",
+			client: fakeClient,
+			comp: appstudiov1alpha1.Component{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "1-test-component",
+					Namespace: "default",
+				},
+				Spec: appstudiov1alpha1.ComponentSpec{
+					ComponentName: "component1",
+					Application:   "application1",
+				},
+			},
+		},
+		{
+			name:   "application not found",
+			client: fakeClient,
+			comp: appstudiov1alpha1.Component{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "1-test-component",
+					Namespace: "default",
+				},
+				Spec: appstudiov1alpha1.ComponentSpec{
+					ComponentName: "component1",
+					Application:   "application-not-found",
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			compWebhook := ComponentWebhook{
+				client: test.client,
+				log: zap.New(zap.UseFlagOptions(&zap.Options{
+					Development: true,
+					TimeEncoder: zapcore.ISO8601TimeEncoder,
+				})),
+			}
+			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" {
+				t.Error("expected component to have owner reference set")
+			} else if test.name != "application not found" {
+				if test.comp.OwnerReferences[0].Name != "application1" {
+					t.Errorf("expected component to have owner reference set to application %s, got %s", "application1", test.comp.OwnerReferences[0].Name)
+				}
+			}
+		})
+	}
+}
+
 func TestComponentCreateValidatingWebhook(t *testing.T) {
 
 	fakeClient := setUpComponents(t)