From 1df2c4ad6985053e12bca8e0641095fde95c1a45 Mon Sep 17 00:00:00 2001 From: Aslak Knutsen Date: Mon, 26 Aug 2024 13:18:13 +0200 Subject: [PATCH] feat: add configurable serviceSelector instead of hardcoded labels (#57) Allow a component to configure the label selectors used to select the services that should be exposable. This allows Platform Routing to operator on an unchanged target. ServiceSelectors also allow templating of the key and value in the configuration to extract data from the Target CR resource. Example: "routing.opendatahub.io/{{.kind}}": "{{.metadata.name}}", // > "routing.opendatahub.io/Service": "MyService" > [!CAUTION] > the target CR is represented as a map[string]any with lowercase > so in contrast to the Object representation {{ .Name }} it's required to > write the full object path as {{ .metadata.name }} --------- Co-authored-by: Bartosz Majsak --- controllers/routingctrl/controller_test.go | 1 - .../routingctrl/exported_svc_locator.go | 9 +-- controllers/routingctrl/fixtures_test.go | 10 ++- .../routingctrl/reconcile_resources.go | 8 ++- controllers/routingctrl/suite_test.go | 5 ++ pkg/config/selectors.go | 70 +++++++++++++++++++ pkg/config/selectors_test.go | 51 ++++++++++++++ pkg/metadata/labels/types.go | 18 ----- pkg/platform/types.go | 6 ++ 9 files changed, 145 insertions(+), 33 deletions(-) create mode 100644 pkg/config/selectors.go create mode 100644 pkg/config/selectors_test.go diff --git a/controllers/routingctrl/controller_test.go b/controllers/routingctrl/controller_test.go index f66af8f..080363f 100644 --- a/controllers/routingctrl/controller_test.go +++ b/controllers/routingctrl/controller_test.go @@ -92,7 +92,6 @@ var _ = Describe("Platform routing setup for the component", test.EnvTest(), fun toRemove = append(toRemove, component) // required labels for the exported service: - // routing.opendatahub.io/exported: "true" // platform.opendatahub.io/owner-name: test-component // platform.opendatahub.io/owner-kind: Component addRoutingRequirementsToSvc(ctx, svc, component) diff --git a/controllers/routingctrl/exported_svc_locator.go b/controllers/routingctrl/exported_svc_locator.go index 4bde0ab..7403144 100644 --- a/controllers/routingctrl/exported_svc_locator.go +++ b/controllers/routingctrl/exported_svc_locator.go @@ -5,21 +5,16 @@ import ( "errors" "fmt" - "github.com/opendatahub-io/odh-platform/pkg/metadata/labels" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" ) -func getExportedServices(ctx context.Context, cli client.Client, target *unstructured.Unstructured) ([]corev1.Service, error) { +func getExportedServices(ctx context.Context, cli client.Client, labels map[string]string, target *unstructured.Unstructured) ([]corev1.Service, error) { listOpts := []client.ListOption{ client.InNamespace(target.GetNamespace()), - labels.MatchingLabels( - labels.RoutingExported("true"), - labels.OwnerName(target.GetName()), - labels.OwnerKind(target.GetObjectKind().GroupVersionKind().Kind), - ), + client.MatchingLabels(labels), } var exportedSvcList *corev1.ServiceList diff --git a/controllers/routingctrl/fixtures_test.go b/controllers/routingctrl/fixtures_test.go index cd8c880..0fbb30d 100644 --- a/controllers/routingctrl/fixtures_test.go +++ b/controllers/routingctrl/fixtures_test.go @@ -31,20 +31,18 @@ func getClusterDomain(ctx context.Context, cli client.Client) string { return domain } -// addRoutingRequirementsToSvc adds routing-related metadata to the Service being exported. -// It adds the "routing.opendatahub.io/exported" label to indicate that the service is exported, -// and it also sets labels for the owner component's name and kind, using -// "platform.opendatahub.io/owner-name" and "platform.opendatahub.io/owner-kind" respectively. +// addRoutingRequirementsToSvc adds routing-related metadata to the Service being exported to match the +// serviceSelector defined in the suite_test. func addRoutingRequirementsToSvc(ctx context.Context, exportedSvc *corev1.Service, owningComponent *unstructured.Unstructured) { - exportedLabel := labels.RoutingExported("true") ownerName := labels.OwnerName(owningComponent.GetName()) ownerKind := labels.OwnerKind(owningComponent.GetObjectKind().GroupVersionKind().Kind) _, errExportSvc := controllerutil.CreateOrUpdate(ctx, envTest.Client, exportedSvc, func() error { - metadata.ApplyMetaOptions(exportedSvc, exportedLabel, ownerName, ownerKind) + metadata.ApplyMetaOptions(exportedSvc, ownerName, ownerKind) return nil }) + Expect(errExportSvc).ToNot(HaveOccurred()) } diff --git a/controllers/routingctrl/reconcile_resources.go b/controllers/routingctrl/reconcile_resources.go index db83f20..b79f991 100644 --- a/controllers/routingctrl/reconcile_resources.go +++ b/controllers/routingctrl/reconcile_resources.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/opendatahub-io/odh-platform/pkg/cluster" + "github.com/opendatahub-io/odh-platform/pkg/config" "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" @@ -26,7 +27,12 @@ func (r *Controller) reconcileResources(ctx context.Context, target *unstructure r.log.Info("Reconciling resources for target", "target", target) - exportedServices, errSvcGet := getExportedServices(ctx, r.Client, target) + renderedSelectors, errLables := config.ResolveSelectors(r.component.ServiceSelector, target) + if errLables != nil { + return fmt.Errorf("could not render labels for ServiceSelector %v. Error %w", r.component.ServiceSelector, errLables) + } + + exportedServices, errSvcGet := getExportedServices(ctx, r.Client, renderedSelectors, target) if errSvcGet != nil { if errors.Is(errSvcGet, &ExportedServiceNotFoundError{}) { r.log.Info("no exported services found for target", "target", target) diff --git a/controllers/routingctrl/suite_test.go b/controllers/routingctrl/suite_test.go index d3ff11b..0761beb 100644 --- a/controllers/routingctrl/suite_test.go +++ b/controllers/routingctrl/suite_test.go @@ -7,6 +7,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "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/test" @@ -37,6 +38,9 @@ var _ = SynchronizedBeforeSuite(func() { return } + ownerName := labels.OwnerName("{{.metadata.name}}") + ownerKind := labels.OwnerKind("{{.kind}}") + routingCtrl := routingctrl.New( nil, ctrl.Log.WithName("controllers").WithName("platform"), @@ -49,6 +53,7 @@ var _ = SynchronizedBeforeSuite(func() { Kind: "Component", }, }, + ServiceSelector: labels.MatchingLabels(ownerName, ownerKind), }, }, routingConfiguration, diff --git a/pkg/config/selectors.go b/pkg/config/selectors.go new file mode 100644 index 0000000..c8de34c --- /dev/null +++ b/pkg/config/selectors.go @@ -0,0 +1,70 @@ +package config + +import ( + "bytes" + "fmt" + "strings" + "text/template" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +// ResolveSelectors uses golang template engine to resolve the expressions in the `selectorExpressions` map using +// `source` as a data input. Both the keys and values are resolved against the source data. +// +// Note: expressions are resolved against the source using lowercase keys +// +// Example source: +// +// kind: Service +// metadata: +// name: MyService +// +// Example selectorExpressions: +// +// map[string]{string} { +// "routing.opendatahub.io/{{.kind}}": "{{.metadata.name}}", // > "routing.opendatahub.io/Service": "MyService" +// } +func ResolveSelectors(selectorExpressions map[string]string, source *unstructured.Unstructured) (map[string]string, error) { + resolved := make(map[string]string, len(selectorExpressions)) + mainTemplate := template.New("unused_name").Option("missingkey=error") + + for key, val := range selectorExpressions { + var err error + + resolvedKey := key + if strings.Contains(key, "{{") { + resolvedKey, err = resolve(mainTemplate, key, source) + if err != nil { + return nil, fmt.Errorf("could not resolve key %s: %w", key, err) + } + } + + resolvedVal := val + if strings.Contains(val, "{{") { + resolvedVal, err = resolve(mainTemplate, val, source) + if err != nil { + return nil, fmt.Errorf("could not resolve value %s: %w", val, err) + } + } + + resolved[resolvedKey] = resolvedVal + } + + return resolved, nil +} + +func resolve(templ *template.Template, textTemplate string, source *unstructured.Unstructured) (string, error) { + tmpl, err := templ.Parse(textTemplate) + if err != nil { + return "", fmt.Errorf("could not parse template: %w", err) + } + + var buff bytes.Buffer + + if err := tmpl.Execute(&buff, source.Object); err != nil { + return "", fmt.Errorf("could not execute template: %w", err) + } + + return buff.String(), nil +} diff --git a/pkg/config/selectors_test.go b/pkg/config/selectors_test.go new file mode 100644 index 0000000..50f454b --- /dev/null +++ b/pkg/config/selectors_test.go @@ -0,0 +1,51 @@ +package config_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/opendatahub-io/odh-platform/pkg/config" + "github.com/opendatahub-io/odh-platform/test" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +var _ = Describe("Templated selectors", test.Unit(), func() { + + Context("simple expressions", func() { + + It("should resolve simple expressions for both key and value", func() { + labels := map[string]string{ + "A.{{.kind}}": "{{.metadata.name}}", + "B": "{{.kind}}", + } + + target := unstructured.Unstructured{ + Object: map[string]any{}, + } + target.SetName("X") + target.SetKind("Y") + + renderedLabels, err := config.ResolveSelectors(labels, &target) + Expect(err).ToNot(HaveOccurred()) + + Expect(renderedLabels["A.Y"]).To(Equal("X")) + Expect(renderedLabels["B"]).To(Equal("Y")) + }) + + It("should fail on missing expression", func() { + labels := map[string]string{ + "A": "{{.metadata.name}}", + } + + target := unstructured.Unstructured{ + Object: map[string]any{}, + } + target.SetKind("Y") + + _, err := config.ResolveSelectors(labels, &target) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("could not execute template")) + Expect(err.Error()).To(ContainSubstring("could not resolve value")) + }) + }) + +}) diff --git a/pkg/metadata/labels/types.go b/pkg/metadata/labels/types.go index c2fb5cf..4c2231e 100644 --- a/pkg/metadata/labels/types.go +++ b/pkg/metadata/labels/types.go @@ -131,24 +131,6 @@ func (o OwnerUID) Value() string { return string(o) } -// RoutingExported is a Label to mark resources that are exported by the routing capability. -// It is intended to be set by enrolled component to mark resources that should be used to -// configure routing capability by the Platform. This can be a Kubernetes Service or Istio -// VirtualService from which settings like hosts and ports are extracted. -type RoutingExported string - -func (r RoutingExported) ApplyToMeta(obj metav1.Object) { - addLabel(r, obj) -} - -func (r RoutingExported) Key() string { - return "routing.opendatahub.io/exported" -} - -func (r RoutingExported) Value() string { - return string(r) -} - func addLabel(label Label, obj metav1.Object) { existingLabels := obj.GetLabels() if existingLabels == nil { diff --git a/pkg/platform/types.go b/pkg/platform/types.go index 5221e8c..3807c3d 100644 --- a/pkg/platform/types.go +++ b/pkg/platform/types.go @@ -16,6 +16,12 @@ type ObjectReference struct { type RoutingTarget struct { // ObjectReference provides reference details to the associated object. ObjectReference `json:"ref,omitempty"` + // ServiceSelector is a LabelSelector definition to locate the Service(s) to expose to Routing for the given ObjectReference. + // All provided label selectors must be present on the Service to find a match. + // + // go expressions are handled in the selector key and value to set dynamic values from the current ObjectReference; + // e.g. "routing.opendatahub.io/{{.kind}}": "{{.metadata.name}}", // > "routing.opendatahub.io/Service": "MyService" + ServiceSelector map[string]string `json:"serviceSelector,omitempty"` } // ProtectedResource holds references and configuration details necessary for