chore: simplifies resource loading (maistra#68)

Route resources loader has many unused parameters being part of the
interface definition.

This change streamlines the abstraction to only rely on a route data
structure and type based on which particular resources are loaded.

Data structure holding routing information has been renamed to
RoutingData and new constructor func has been introduced to encapsulate
logic of creating service-related fields.

controllers/routingctrl/reconcile_resources.go

@@ -15,7 +15,6 @@ import (
 	"github.com/opendatahub-io/odh-platform/pkg/unstruct"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
@@ -69,22 +68,13 @@ 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.RoutingTemplateData{
-		PlatformRoutingConfiguration: r.config,
-		PublicServiceName:            exportedSvc.GetName() + "-" + exportedSvc.GetNamespace(),
-		ServiceName:                  exportedSvc.GetName(),
-		ServiceNamespace:             exportedSvc.GetNamespace(),
-		ServiceTargetPort:            exportedSvc.Spec.Ports[0].TargetPort.String(),
-		Domain:                       domain,
-	}
+	templateData := spi.NewRoutingData(r.config, exportedSvc, domain)
 
 	// To establish ownership for watched component
 	ownershipLabels := append(labels.AsOwner(target), labels.AppManagedBy("odh-routing-controller"))
 
-	targetKey := client.ObjectKeyFromObject(target)
-
 	for _, exportMode := range exportModes {
-		resources, err := r.templateLoader.Load(ctx, exportMode, targetKey, templateData)
+		resources, err := r.templateLoader.Load(templateData, exportMode)
 		if err != nil {
 			return fmt.Errorf("could not load templates for type %s: %w", exportMode, err)
 		}
@@ -98,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.RoutingTemplateData) error {
+func (r *Controller) propagateHostsToWatchedCR(target *unstructured.Unstructured, data *spi.RoutingData) error {
 	exportModes := r.extractExportModes(target)
 
 	// Remove all existing routing addresses

pkg/metadata/labels/funcs.go

@@ -38,7 +38,12 @@ func StandardLabelsFrom(source metav1.Object) []metadata.Option {
 // AppendStandardLabelsFrom appends standard labels found in source object but only
 // when they are not already present in the target object.
 func AppendStandardLabelsFrom(source metav1.Object) *LabelAppender {
-	return &LabelAppender{labels: standardLabelsFrom(source)}
+	return AppendLabels(standardLabelsFrom(source)...)
+}
+
+// AppendLabels appends provided labels to the target object but only when they are not already present.
+func AppendLabels(labels ...Label) *LabelAppender {
+	return &LabelAppender{labels: labels}
 }
 
 // MatchingLabels returns a client.MatchingLabels selector for the provided labels.

pkg/routing/routing.go

@@ -2,7 +2,6 @@ package routing
 
 import (
 	"bytes"
-	"context"
 	_ "embed" // needed for go:embed directive
 	"fmt"
 	"strings"
@@ -11,7 +10,6 @@ import (
 	"github.com/opendatahub-io/odh-platform/pkg/schema"
 	"github.com/opendatahub-io/odh-platform/pkg/spi"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/types"
 )
 
 //go:embed template/routing_public.yaml
@@ -27,7 +25,7 @@ func NewStaticTemplateLoader() spi.RoutingTemplateLoader {
 	return &staticTemplateLoader{}
 }
 
-func (s *staticTemplateLoader) Load(_ context.Context, routeType spi.RouteType, key types.NamespacedName, data spi.RoutingTemplateData) ([]*unstructured.Unstructured, error) {
+func (s *staticTemplateLoader) Load(data *spi.RoutingData, routeType spi.RouteType) ([]*unstructured.Unstructured, error) {
 	var resources []*unstructured.Unstructured
 
 	var templateContent []byte
@@ -60,7 +58,7 @@ func (s *staticTemplateLoader) Load(_ context.Context, routeType spi.RouteType,
 	return resources, nil
 }
 
-func (s *staticTemplateLoader) resolveTemplate(tmpl []byte, data spi.RoutingTemplateData) ([]byte, error) {
+func (s *staticTemplateLoader) resolveTemplate(tmpl []byte, data *spi.RoutingData) ([]byte, error) {
 	engine, err := template.New("routing").Parse(string(tmpl))
 	if err != nil {
 		return []byte{}, fmt.Errorf("could not create template engine: %w", err)

pkg/routing/routing_test.go

@@ -1,40 +1,53 @@
 package routing_test
 
 import (
-	"context"
-
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/opendatahub-io/odh-platform/pkg/routing"
 	"github.com/opendatahub-io/odh-platform/pkg/spi"
 	"github.com/opendatahub-io/odh-platform/test"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
 )
 
 var _ = Describe("Resource functions", test.Unit(), func() {
 
 	Context("Template Loader", func() {
 
-		data := spi.RoutingTemplateData{
-			PlatformRoutingConfiguration: spi.PlatformRoutingConfiguration{
-				GatewayNamespace:     "opendatahub",
-				IngressSelectorLabel: "istio",
-				IngressSelectorValue: "rhoai-gateway",
-				IngressService:       "rhoai-router-ingress",
-			},
-			PublicServiceName: "registry-office",
-			ServiceName:       "registry",
-			ServiceNamespace:  "office",
-			Domain:            "app-crc.testing",
+		config := spi.PlatformRoutingConfiguration{
+			GatewayNamespace:     "opendatahub",
+			IngressSelectorLabel: "istio",
+			IngressSelectorValue: "rhoai-gateway",
+			IngressService:       "rhoai-router-ingress",
 		}
 
+		data := spi.NewRoutingData(config,
+			&corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "registry",
+					Namespace: "office",
+				},
+				Spec: corev1.ServiceSpec{
+					Ports: []corev1.ServicePort{
+						{
+							Name:        "http-api",
+							Port:        80,
+							AppProtocol: ptr.To("http"),
+						},
+					},
+				},
+			},
+			"app-crc.testing",
+		)
+
 		It("should load public resources", func() {
 			// given
 			// data^
 
 			// when
-			res, err := routing.NewStaticTemplateLoader().Load(context.Background(), spi.PublicRoute, types.NamespacedName{}, data)
+			res, err := routing.NewStaticTemplateLoader().Load(data, spi.PublicRoute)
 
 			// then
 			Expect(err).ShouldNot(HaveOccurred())
@@ -46,7 +59,7 @@ var _ = Describe("Resource functions", test.Unit(), func() {
 			// data^
 
 			// when
-			res, err := routing.NewStaticTemplateLoader().Load(context.Background(), spi.ExternalRoute, types.NamespacedName{}, data)
+			res, err := routing.NewStaticTemplateLoader().Load(data, spi.ExternalRoute)
 
 			// then
 			Expect(err).ShouldNot(HaveOccurred())

pkg/spi/types.go

@@ -9,6 +9,7 @@ import (
 
 	authorinov1beta2 "github.com/kuadrant/authorino/api/v1beta2"
 	"github.com/opendatahub-io/odh-platform/pkg/platform"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 )
@@ -130,8 +131,7 @@ type PlatformRoutingConfiguration struct {
 	GatewayNamespace string
 }
 
-// TODO(mvp) revise the stuct name - is it only for templates?
-type RoutingTemplateData struct {
+type RoutingData struct {
 	PlatformRoutingConfiguration
 
 	PublicServiceName string // [service-name]-[service-namespace]
@@ -143,10 +143,18 @@ type RoutingTemplateData struct {
 	Domain string
 }
 
-// RoutingTemplateLoader provides a way to differentiate the Route template used based on
-//   - RouteType
-//   - Namespace / Resource name
-//   - Loader source
+func NewRoutingData(config PlatformRoutingConfiguration, svc *corev1.Service, domain string) *RoutingData {
+	return &RoutingData{
+		PlatformRoutingConfiguration: config,
+		PublicServiceName:            svc.GetName() + "-" + svc.GetNamespace(),
+		ServiceName:                  svc.GetName(),
+		ServiceNamespace:             svc.GetNamespace(),
+		ServiceTargetPort:            svc.Spec.Ports[0].TargetPort.String(),
+		Domain:                       domain,
+	}
+}
+
+// RoutingTemplateLoader provides a way to differentiate the Route resource templates used based on its types.
 type RoutingTemplateLoader interface {
-	Load(ctx context.Context, routeType RouteType, key types.NamespacedName, data RoutingTemplateData) ([]*unstructured.Unstructured, error)
+	Load(data *RoutingData, routeType RouteType) ([]*unstructured.Unstructured, error)
 }