refactor: splits routing and authz types (#70)

`spi/types.go` mixes routing and authz specific types and interfaces
which belong to their respective packages.

This change moves them to respective packages and also fixes ireturn
where funcs return structs satisfying the interfaces, and consumers
(such as controllers) take interface as fields or func parameters.

.golangci.yml

@@ -83,8 +83,6 @@ linters-settings:
       - stdlib
       # also allow generics
       - generic
-      # we want to return spi.Interfaces in routing.go and authconfig.go
-      - spi
       # for custom Gomega matchers
       - types.GomegaMatcher
   varnamelen: 

controllers/authzctrl/controller.go

@@ -12,7 +12,7 @@ import (
 	"github.com/opendatahub-io/odh-platform/pkg/authorization"
 	"github.com/opendatahub-io/odh-platform/pkg/metadata"
 	"github.com/opendatahub-io/odh-platform/pkg/metadata/annotations"
-	"github.com/opendatahub-io/odh-platform/pkg/routing"
+	"github.com/opendatahub-io/odh-platform/pkg/platform"
 	"github.com/opendatahub-io/odh-platform/pkg/spi"
 	istiosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
 	k8serr "k8s.io/apimachinery/pkg/api/errors"
@@ -26,21 +26,21 @@ import (
 const name = "authorization"
 
 func New(cli client.Client, log logr.Logger,
-	component spi.AuthorizationComponent, config PlatformAuthorizationConfig) *Controller {
+	protectedResource platform.ProtectedResource, config PlatformAuthorizationConfig) *Controller {
 	return &Controller{
 		active: true,
 		Client: cli,
 		log: log.WithValues(
 			"controller", name,
-			"component", component.ObjectReference.Kind,
+			"component", protectedResource.ResourceReference.Kind,
 		),
-		config:        config,
-		authComponent: component,
-		typeDetector:  authorization.NewAnnotationAuthTypeDetector(annotations.AuthEnabled("").Key()),
+		config:            config,
+		protectedResource: protectedResource,
+		typeDetector:      authorization.NewAnnotationAuthTypeDetector(annotations.AuthEnabled("").Key()),
 		// TODO: Evaluate passing in the hostExtractor to avoid coupling the authorizaiton/routing packages
-		hostExtractor: authorization.UnifiedHostExtractor(
-			authorization.NewPathExpressionExtractor(component.HostPaths),
-			routing.NewAnnotationHostExtractor(";", metadata.Keys(annotations.RoutingAddressesExternal(""), annotations.RoutingAddressesPublic(""))...)),
+		hostExtractor: spi.UnifiedHostExtractor(
+			spi.NewPathExpressionExtractor(protectedResource.HostPaths),
+			spi.NewAnnotationHostExtractor(";", metadata.Keys(annotations.RoutingAddressesExternal(""), annotations.RoutingAddressesPublic(""))...)),
 		templateLoader: authorization.NewConfigMapTemplateLoader(cli, authorization.NewStaticTemplateLoader(config.Audiences)),
 	}
 }
@@ -57,13 +57,13 @@ type PlatformAuthorizationConfig struct {
 // Controller holds the authorization controller configuration.
 type Controller struct {
 	client.Client
-	active         bool
-	log            logr.Logger
-	config         PlatformAuthorizationConfig
-	authComponent  spi.AuthorizationComponent
-	typeDetector   spi.AuthTypeDetector
-	hostExtractor  spi.HostExtractor
-	templateLoader spi.AuthConfigTemplateLoader
+	active            bool
+	log               logr.Logger
+	config            PlatformAuthorizationConfig
+	protectedResource platform.ProtectedResource
+	typeDetector      authorization.AuthTypeDetector
+	hostExtractor     spi.HostExtractor
+	templateLoader    authorization.AuthConfigTemplateLoader
 }
 
 // +kubebuilder:rbac:groups=authorino.kuadrant.io,resources=authconfigs,verbs=get;list;watch;create;update;patch;delete
@@ -82,7 +82,7 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	reconcilers := []platformctrl.SubReconcileFunc{r.reconcileAuthConfig, r.reconcileAuthPolicy}
 
 	sourceRes := &unstructured.Unstructured{}
-	sourceRes.SetGroupVersionKind(r.authComponent.ObjectReference.GroupVersionKind)
+	sourceRes.SetGroupVersionKind(r.protectedResource.ResourceReference.GroupVersionKind)
 
 	if err := r.Client.Get(ctx, req.NamespacedName, sourceRes); err != nil {
 		if k8serr.IsNotFound(err) {
@@ -105,7 +105,7 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 }
 
 func (r *Controller) Name() string {
-	return name + "-" + strings.ToLower(r.authComponent.ObjectReference.Kind)
+	return name + "-" + strings.ToLower(r.protectedResource.ResourceReference.Kind)
 }
 
 func (r *Controller) SetupWithManager(mgr ctrl.Manager) error {
@@ -120,8 +120,8 @@ func (r *Controller) SetupWithManager(mgr ctrl.Manager) error {
 		Named(r.Name()).
 		For(&metav1.PartialObjectMetadata{
 			TypeMeta: metav1.TypeMeta{
-				APIVersion: r.authComponent.ObjectReference.GroupVersion().String(),
-				Kind:       r.authComponent.ObjectReference.Kind,
+				APIVersion: r.protectedResource.ResourceReference.GroupVersion().String(),
+				Kind:       r.protectedResource.ResourceReference.Kind,
 			},
 		}, builder.OnlyMetadata).
 		Owns(&authorinov1beta2.AuthConfig{}).

controllers/authzctrl/reconcile_authpolicy.go

@@ -20,12 +20,12 @@ import (
 )
 
 func (r *Controller) reconcileAuthPolicy(ctx context.Context, target *unstructured.Unstructured) error {
-	resolvedSelectors, errResolve := config.ResolveSelectors(r.authComponent.WorkloadSelector, target)
+	resolvedSelectors, errResolve := config.ResolveSelectors(r.protectedResource.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)
+	desired := createAuthzPolicy(r.protectedResource.Ports, resolvedSelectors, r.config.ProviderName, target)
 	found := &istiosecurityv1beta1.AuthorizationPolicy{}
 	justCreated := false
 

controllers/authzctrl/suite_test.go

@@ -9,7 +9,6 @@ import (
 	"github.com/opendatahub-io/odh-platform/controllers/authzctrl"
 	"github.com/opendatahub-io/odh-platform/pkg/config"
 	"github.com/opendatahub-io/odh-platform/pkg/platform"
-	"github.com/opendatahub-io/odh-platform/pkg/spi"
 	"github.com/opendatahub-io/odh-platform/test"
 	"github.com/opendatahub-io/odh-platform/test/k8senvtest"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -33,22 +32,20 @@ var _ = SynchronizedBeforeSuite(func(ctx context.Context) {
 		authzctrl.New(
 			nil,
 			ctrl.Log.WithName("controllers").WithName("platform"),
-			spi.AuthorizationComponent{
-				ProtectedResource: platform.ProtectedResource{
-					ObjectReference: platform.ObjectReference{
-						GroupVersionKind: schema.GroupVersionKind{
-							Version: "v1",
-							Group:   "opendatahub.io",
-							Kind:    "Component",
-						},
-						Resources: "components",
+			platform.ProtectedResource{
+				ResourceReference: platform.ResourceReference{
+					GroupVersionKind: schema.GroupVersionKind{
+						Version: "v1",
+						Group:   "opendatahub.io",
+						Kind:    "Component",
 					},
-					WorkloadSelector: map[string]string{
-						"component": "{{.metadata.name}}",
-					},
-					Ports:     []string{},
-					HostPaths: []string{"spec.host"},
+					Resources: "components",
+				},
+				WorkloadSelector: map[string]string{
+					"component": "{{.metadata.name}}",
 				},
+				Ports:     []string{},
+				HostPaths: []string{"spec.host"},
 			},
 			authzctrl.PlatformAuthorizationConfig{
 				Label:        config.GetAuthorinoLabel(),

controllers/routingctrl/controller.go

@@ -8,8 +8,8 @@ import (
 
 	"github.com/go-logr/logr"
 	platformctrl "github.com/opendatahub-io/odh-platform/controllers"
+	"github.com/opendatahub-io/odh-platform/pkg/platform"
 	"github.com/opendatahub-io/odh-platform/pkg/routing"
-	"github.com/opendatahub-io/odh-platform/pkg/spi"
 	"github.com/opendatahub-io/odh-platform/pkg/unstruct"
 	openshiftroutev1 "github.com/openshift/api/route/v1"
 	istionetworkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1"
@@ -26,15 +26,15 @@ const (
 	finalizerName = "routing.opendatahub.io/finalizer"
 )
 
-func New(cli client.Client, log logr.Logger, component spi.RoutingComponent, config spi.PlatformRoutingConfiguration) *Controller {
+func New(cli client.Client, log logr.Logger, target platform.RoutingTarget, config routing.PlatformRoutingConfiguration) *Controller {
 	return &Controller{
 		active: true,
 		Client: cli,
 		log: log.WithValues(
 			"controller", name,
-			"component", component.ObjectReference.Kind,
+			"component", target.ResourceReference.Kind,
 		),
-		component:      component,
+		component:      target,
 		config:         config,
 		templateLoader: routing.NewStaticTemplateLoader(),
 	}
@@ -45,9 +45,9 @@ type Controller struct {
 	client.Client
 	active         bool
 	log            logr.Logger
-	component      spi.RoutingComponent
-	templateLoader spi.RoutingTemplateLoader
-	config         spi.PlatformRoutingConfiguration
+	component      platform.RoutingTarget
+	templateLoader routing.TemplateLoader
+	config         routing.PlatformRoutingConfiguration
 }
 
 // +kubebuilder:rbac:groups="route.openshift.io",resources=routes,verbs=*
@@ -70,7 +70,7 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	}
 
 	sourceRes := &unstructured.Unstructured{}
-	sourceRes.SetGroupVersionKind(r.component.ObjectReference.GroupVersionKind)
+	sourceRes.SetGroupVersionKind(r.component.ResourceReference.GroupVersionKind)
 
 	if err := r.Client.Get(ctx, req.NamespacedName, sourceRes); err != nil {
 		if k8serr.IsNotFound(err) {
@@ -104,7 +104,7 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 }
 
 func (r *Controller) Name() string {
-	return name + "-" + strings.ToLower(r.component.ObjectReference.Kind)
+	return name + "-" + strings.ToLower(r.component.ResourceReference.Kind)
 }
 
 func (r *Controller) SetupWithManager(mgr ctrl.Manager) error {
@@ -119,8 +119,8 @@ func (r *Controller) SetupWithManager(mgr ctrl.Manager) error {
 		Named(r.Name()).
 		For(&metav1.PartialObjectMetadata{
 			TypeMeta: metav1.TypeMeta{
-				APIVersion: r.component.ObjectReference.GroupVersion().String(),
-				Kind:       r.component.ObjectReference.Kind,
+				APIVersion: r.component.ResourceReference.GroupVersion().String(),
+				Kind:       r.component.ResourceReference.Kind,
 			},
 		}, builder.OnlyMetadata).
 		Owns(&istionetworkingv1beta1.DestinationRule{}).

controllers/routingctrl/delete_resources.go

@@ -5,7 +5,7 @@ import (
 	"fmt"
 
 	"github.com/opendatahub-io/odh-platform/pkg/metadata/labels"
-	"github.com/opendatahub-io/odh-platform/pkg/spi"
+	"github.com/opendatahub-io/odh-platform/pkg/routing"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	k8slabels "k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -16,7 +16,7 @@ import (
 
 func (r *Controller) removeUnusedRoutingResources(ctx context.Context, target *unstructured.Unstructured) error {
 	exportModes := r.extractExportModes(target)
-	unusedRouteTypes := spi.UnusedRouteTypes(exportModes)
+	unusedRouteTypes := routing.UnusedRouteTypes(exportModes)
 
 	if len(unusedRouteTypes) == 0 {
 		// no unused route types to remove resources for
@@ -49,7 +49,7 @@ func (r *Controller) handleResourceDeletion(ctx context.Context, sourceRes *unst
 
 func (r *Controller) deleteOwnedResources(ctx context.Context,
 	target *unstructured.Unstructured,
-	exportModes []spi.RouteType,
+	exportModes []routing.RouteType,
 	gvks []schema.GroupVersionKind) error {
 	exportTypeValues := make([]string, len(exportModes))
 	for i, mode := range exportModes {

controllers/routingctrl/gvk.go

@@ -1,7 +1,7 @@
 package routingctrl
 
 import (
-	"github.com/opendatahub-io/odh-platform/pkg/spi"
+	"github.com/opendatahub-io/odh-platform/pkg/routing"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
@@ -19,17 +19,17 @@ var publicGVKs = []schema.GroupVersionKind{
 	{Group: "networking.istio.io", Version: "v1beta1", Kind: "DestinationRule"},
 }
 
-func routingResourceGVKs(exportModes ...spi.RouteType) []schema.GroupVersionKind {
+func routingResourceGVKs(exportModes ...routing.RouteType) []schema.GroupVersionKind {
 	// use map just to handle possible duplication of gvks
 	gvkSet := make(map[schema.GroupVersionKind]struct{})
 
 	for _, exportMode := range exportModes {
 		var gvks []schema.GroupVersionKind
 
 		switch exportMode {
-		case spi.ExternalRoute:
+		case routing.ExternalRoute:
 			gvks = externalGVKs
-		case spi.PublicRoute:
+		case routing.PublicRoute:
 			gvks = publicGVKs
 		}
 

controllers/routingctrl/reconcile_resources.go

@@ -11,7 +11,7 @@ import (
 	"github.com/opendatahub-io/odh-platform/pkg/metadata"
 	"github.com/opendatahub-io/odh-platform/pkg/metadata/annotations"
 	"github.com/opendatahub-io/odh-platform/pkg/metadata/labels"
-	"github.com/opendatahub-io/odh-platform/pkg/spi"
+	"github.com/opendatahub-io/odh-platform/pkg/routing"
 	"github.com/opendatahub-io/odh-platform/pkg/unstruct"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -68,7 +68,7 @@ func (r *Controller) createRoutingResources(ctx context.Context, target *unstruc
 func (r *Controller) exportService(ctx context.Context, target *unstructured.Unstructured, exportedSvc *corev1.Service, domain string) error {
 	exportModes := r.extractExportModes(target)
 
-	templateData := spi.NewRoutingData(r.config, exportedSvc, domain)
+	templateData := routing.NewExposedServiceConfig(r.config, exportedSvc, domain)
 
 	// To establish ownership for watched component
 	ownershipLabels := append(labels.AsOwner(target), labels.AppManagedBy("odh-routing-controller"))
@@ -88,7 +88,7 @@ func (r *Controller) exportService(ctx context.Context, target *unstructured.Uns
 	return r.propagateHostsToWatchedCR(target, templateData)
 }
 
-func (r *Controller) propagateHostsToWatchedCR(target *unstructured.Unstructured, data *spi.RoutingData) error {
+func (r *Controller) propagateHostsToWatchedCR(target *unstructured.Unstructured, data *routing.ExposedServiceConfig) error {
 	exportModes := r.extractExportModes(target)
 
 	// Remove all existing routing addresses
@@ -100,10 +100,10 @@ func (r *Controller) propagateHostsToWatchedCR(target *unstructured.Unstructured
 	// TODO(mvp): put the logic of creating host names into a single place
 	for _, exportMode := range exportModes {
 		switch exportMode {
-		case spi.ExternalRoute:
+		case routing.ExternalRoute:
 			externalAddress := annotations.RoutingAddressesExternal(fmt.Sprintf("%s-%s.%s", data.ServiceName, data.ServiceNamespace, data.Domain))
 			metaOptions = append(metaOptions, externalAddress)
-		case spi.PublicRoute:
+		case routing.PublicRoute:
 			publicAddresses := annotations.RoutingAddressesPublic(fmt.Sprintf("%[1]s.%[2]s;%[1]s.%[2]s.svc;%[1]s.%[2]s.svc.cluster.local", data.PublicServiceName, data.GatewayNamespace))
 			metaOptions = append(metaOptions, publicAddresses)
 		}
@@ -124,18 +124,18 @@ func (r *Controller) ensureResourceHasFinalizer(ctx context.Context, target *uns
 	return nil
 }
 
-func (r *Controller) extractExportModes(target *unstructured.Unstructured) []spi.RouteType {
+func (r *Controller) extractExportModes(target *unstructured.Unstructured) []routing.RouteType {
 	exportModes, exportModeFound := target.GetAnnotations()[annotations.RoutingExportMode("").Key()]
 	if !exportModeFound {
 		return nil
 	}
 
 	exportModesSplit := strings.Split(exportModes, ";")
-	validRouteTypes := make([]spi.RouteType, 0, len(exportModesSplit))
+	validRouteTypes := make([]routing.RouteType, 0, len(exportModesSplit))
 
 	for _, exportMode := range exportModesSplit {
-		routeType := spi.RouteType(strings.TrimSpace(exportMode))
-		if spi.IsValidRouteType(routeType) {
+		routeType := routing.RouteType(strings.TrimSpace(exportMode))
+		if routing.IsValidRouteType(routeType) {
 			validRouteTypes = append(validRouteTypes, routeType)
 		} else {
 			r.log.Info("Invalid route type found",

controllers/routingctrl/suite_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/opendatahub-io/odh-platform/controllers/routingctrl"
 	"github.com/opendatahub-io/odh-platform/pkg/metadata/labels"
 	"github.com/opendatahub-io/odh-platform/pkg/platform"
-	"github.com/opendatahub-io/odh-platform/pkg/spi"
+	"github.com/opendatahub-io/odh-platform/pkg/routing"
 	"github.com/opendatahub-io/odh-platform/test"
 	"github.com/opendatahub-io/odh-platform/test/k8senvtest"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -18,7 +18,7 @@ import (
 
 var (
 	envTest              *k8senvtest.Client
-	routingConfiguration = spi.PlatformRoutingConfiguration{
+	routingConfiguration = routing.PlatformRoutingConfiguration{
 		IngressService:       "odh-router",
 		GatewayNamespace:     "odh-gateway",
 		IngressSelectorLabel: "istio",
@@ -44,17 +44,15 @@ var _ = SynchronizedBeforeSuite(func() {
 	routingCtrl := routingctrl.New(
 		nil,
 		ctrl.Log.WithName("controllers").WithName("platform"),
-		spi.RoutingComponent{
-			RoutingTarget: platform.RoutingTarget{
-				ObjectReference: platform.ObjectReference{
-					GroupVersionKind: schema.GroupVersionKind{
-						Version: "v1",
-						Group:   "opendatahub.io",
-						Kind:    "Component",
-					},
+		platform.RoutingTarget{
+			ResourceReference: platform.ResourceReference{
+				GroupVersionKind: schema.GroupVersionKind{
+					Version: "v1",
+					Group:   "opendatahub.io",
+					Kind:    "Component",
 				},
-				ServiceSelector: labels.MatchingLabels(ownerName, ownerKind),
 			},
+			ServiceSelector: labels.MatchingLabels(ownerName, ownerKind),
 		},
 		routingConfiguration,
 	)

go.mod

@@ -15,7 +15,6 @@ replace (
 	k8s.io/component-base => k8s.io/component-base v0.28.4
 	sigs.k8s.io/controller-runtime => sigs.k8s.io/controller-runtime v0.17.5
 	sigs.k8s.io/controller-tools => sigs.k8s.io/controller-tools v0.14.0
-
 )
 
 require (