Merge branch 'main' into finalizer-bug

Makefile

@@ -50,7 +50,7 @@ lint: tools format ## Concurrently runs a whole bunch of static analysis tools
 test: generate
 test: test-unit+kube-envtest ## Run all tests. You can also select a category by running e.g. make test-unit or make test-kube-envtest
 
-ENVTEST_K8S_VERSION = 1.26 # refers to the version of kubebuilder assets to be downloaded by envtest binary.
+ENVTEST_K8S_VERSION = 1.28 # refers to the version of kubebuilder assets to be downloaded by envtest binary.
 test-%:
 	$(eval test-type:=$(subst +,||,$(subst test-,,$@)))
 	KUBEBUILDER_ASSETS="$(shell $(LOCALBIN)/setup-envtest use $(ENVTEST_K8S_VERSION) -p path)" \
@@ -157,12 +157,12 @@ $(LOCALBIN)/kustomize:
 	tar xzvf /tmp/kustomize.tar.gz -C $(LOCALBIN)
 	chmod +x $(LOCALBIN)/kustomize
 
-CONTROLLER_TOOLS_VERSION?=$(call go-mod-version,'controller-tools')
+CONTROLLER_TOOLS_VERSION?=v0.16.1 # Cannot be pulled as dependency from go.mod, as it will bump go requirements to 1.22
 $(LOCALBIN)/controller-gen:
 	$(call header,"Installing $(notdir $@)")
 	$(call go-get-tool,controller-gen,sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION))
 
-ENVTEST_PACKAGE_VERSION = v0.0.0-20240320141353-395cfc7486e6
+ENVTEST_PACKAGE_VERSION = v0.0.0-20240813183042-b901db121e1f
 $(LOCALBIN)/setup-envtest:
 	$(call header,"Installing $(notdir $@)")
 	GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@$(ENVTEST_PACKAGE_VERSION)

config/rbac/role.yaml

@@ -2,9 +2,14 @@
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
 metadata:
-  creationTimestamp: null
   name: manager-role
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - services
+  verbs:
+  - '*'
 - apiGroups:
   - authorino.kuadrant.io
   resources:
@@ -21,17 +26,7 @@ rules:
   - networking.istio.io
   resources:
   - destinationrules
-  verbs:
-  - '*'
-- apiGroups:
-  - networking.istio.io
-  resources:
   - gateways
-  verbs:
-  - '*'
-- apiGroups:
-  - networking.istio.io
-  resources:
   - virtualservices
   verbs:
   - '*'

controllers/authzctrl/controller_test.go

@@ -138,6 +138,8 @@ var _ = Describe("Checking Authorization Resource Creation", test.EnvTest(), fun
 			}
 
 			Expect(createdAuthPolicy.Spec.GetAction()).To(Equal(v1beta1.AuthorizationPolicy_CUSTOM))
+			// WorkloadSelector expresssion defined in suite_test
+			Expect(createdAuthPolicy.Spec.GetSelector().GetMatchLabels()).To(HaveKeyWithValue("component", resourceName))
 
 			return nil
 		}, test.DefaultTimeout, test.DefaultPolling).Should(Succeed())

controllers/authzctrl/reconcile_authpolicy.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"reflect"
 
+	"github.com/opendatahub-io/odh-platform/pkg/config"
 	"github.com/opendatahub-io/odh-platform/pkg/metadata"
 	"github.com/opendatahub-io/odh-platform/pkg/metadata/labels"
 	"istio.io/api/security/v1beta1"
@@ -19,35 +20,36 @@ import (
 )
 
 func (r *Controller) reconcileAuthPolicy(ctx context.Context, target *unstructured.Unstructured) error {
-	desired := createAuthzPolicy(r.authComponent.Ports, r.authComponent.WorkloadSelector, r.config.ProviderName, target)
+	resolvedSelectors, errResolve := config.ResolveSelectors(r.authComponent.WorkloadSelector, target)
+	if errResolve != nil {
+		return fmt.Errorf("could not resolve WorkloadSelectors err: %w", errResolve)
+	}
+
+	desired := createAuthzPolicy(r.authComponent.Ports, resolvedSelectors, r.config.ProviderName, target)
 	found := &istiosecurityv1beta1.AuthorizationPolicy{}
 	justCreated := false
 
-	err := r.Get(ctx, types.NamespacedName{
+	typeName := types.NamespacedName{
 		Name:      desired.Name,
-		Namespace: desired.Namespace,
-	}, found)
-	if err != nil {
-		if k8serr.IsNotFound(err) {
+		Namespace: desired.Namespace}
+	if errGet := r.Get(ctx, typeName, found); errGet != nil {
+		if k8serr.IsNotFound(errGet) {
 			errCreate := r.Create(ctx, desired)
 			if client.IgnoreAlreadyExists(errCreate) != nil {
 				return fmt.Errorf("unable to create AuthorizationPolicy: %w", errCreate)
 			}
 
 			justCreated = true
 		} else {
-			return fmt.Errorf("unable to fetch AuthorizationPolicy: %w", err)
+			return fmt.Errorf("unable to fetch AuthorizationPolicy: %w", errGet)
 		}
 	}
 
 	// Reconcile the Istio AuthorizationPolicy if it has been manually modified
 	if !justCreated && !CompareAuthPolicies(desired, found) {
-		if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
-			if err := r.Get(ctx, types.NamespacedName{
-				Name:      desired.Name,
-				Namespace: desired.Namespace,
-			}, found); err != nil {
-				return fmt.Errorf("failed getting AuthorizationPolicy %s in namespace %s: %w", desired.Name, desired.Namespace, err)
+		if errConflict := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+			if errGet := r.Get(ctx, typeName, found); errGet != nil {
+				return fmt.Errorf("failed getting AuthorizationPolicy %s in namespace %s: %w", desired.Name, desired.Namespace, errGet)
 			}
 
 			found.Spec = *desired.Spec.DeepCopy()
@@ -58,8 +60,8 @@ func (r *Controller) reconcileAuthPolicy(ctx context.Context, target *unstructur
 			}
 
 			return nil
-		}); err != nil {
-			return fmt.Errorf("unable to reconcile the AuthorizationPolicy: %w", err)
+		}); errConflict != nil {
+			return fmt.Errorf("unable to reconcile the AuthorizationPolicy: %w", errConflict)
 		}
 	}
 

controllers/authzctrl/suite_test.go

@@ -43,9 +43,11 @@ var _ = SynchronizedBeforeSuite(func(ctx context.Context) {
 						},
 						Resources: "components",
 					},
-					WorkloadSelector: map[string]string{},
-					Ports:            []string{},
-					HostPaths:        []string{"spec.host"},
+					WorkloadSelector: map[string]string{
+						"component": "{{.metadata.name}}",
+					},
+					Ports:     []string{},
+					HostPaths: []string{"spec.host"},
 				},
 			},
 			authzctrl.PlatformAuthorizationConfig{

controllers/routingctrl/controller.go

@@ -54,6 +54,7 @@ type Controller struct {
 // +kubebuilder:rbac:groups="networking.istio.io",resources=virtualservices,verbs=*
 // +kubebuilder:rbac:groups="networking.istio.io",resources=gateways,verbs=*
 // +kubebuilder:rbac:groups="networking.istio.io",resources=destinationrules,verbs=*
+// +kubebuilder:rbac:groups="",resources=services,verbs=*
 
 // Reconcile ensures that the component has all required resources needed to use routing capability of the platform.
 func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
@@ -63,7 +64,10 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, nil
 	}
 
-	reconcilers := []platformctrl.SubReconcileFunc{r.reconcileResources}
+	reconcilers := []platformctrl.SubReconcileFunc{
+		r.removeUnusedRoutingResources,
+		r.createRoutingResources,
+	}
 
 	sourceRes := &unstructured.Unstructured{}
 	sourceRes.SetGroupVersionKind(r.component.ObjectReference.GroupVersionKind)
@@ -78,12 +82,12 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, fmt.Errorf("failed getting resource: %w", err)
 	}
 
-	if !sourceRes.GetDeletionTimestamp().IsZero() {
-		return r.HandleResourceDeletion(ctx, sourceRes)
-	}
-
 	r.log.Info("triggered routing reconcile", "namespace", req.Namespace, "name", req.Name)
 
+	if unstruct.IsMarkedForDeletion(sourceRes) {
+		return ctrl.Result{}, r.handleResourceDeletion(ctx, sourceRes)
+	}
+
 	var errs []error
 
 	if errFinalizer := r.ensureResourceHasFinalizer(ctx, sourceRes); errFinalizer != nil {