Implement Target Metadata

cmd/api/main.go

@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"net/url"
 	"os"
-	"strings"
 	"time"
 
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
@@ -59,6 +58,7 @@ import (
 	ngrokcontroller "github.com/ngrok/ngrok-operator/internal/controller/ngrok"
 	"github.com/ngrok/ngrok-operator/internal/ngrokapi"
 	"github.com/ngrok/ngrok-operator/internal/store"
+	"github.com/ngrok/ngrok-operator/internal/util"
 	"github.com/ngrok/ngrok-operator/internal/version"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	//+kubebuilder:scaffold:imports
@@ -107,8 +107,10 @@ type managerOpts struct {
 	enableFeatureBindings bool
 
 	bindings struct {
-		allowedURLs []string
-		name        string
+		allowedURLs        []string
+		name               string
+		serviceAnnotations string
+		serviceLabels      string
 	}
 
 	// env vars
@@ -149,6 +151,8 @@ func cmd() *cobra.Command {
 	c.Flags().BoolVar(&opts.enableFeatureBindings, "enable-feature-bindings", false, "Enables the Endpoint Bindings controller")
 	c.Flags().StringSliceVar(&opts.bindings.allowedURLs, "bindings-allowed-urls", []string{"*"}, "Allowed URLs for Endpoint Bindings")
 	c.Flags().StringVar(&opts.bindings.name, "bindings-name", "default", "Name of the Endpoint Binding Configuration")
+	c.Flags().StringVar(&opts.bindings.serviceAnnotations, "bindings-service-annotations", "", "Service Annotations to propagate to the target service")
+	c.Flags().StringVar(&opts.bindings.serviceLabels, "bindings-service-labels", "", "Service Labels to propagate to the target service")
 
 	opts.zapOpts = &zap.Options{}
 	goFlagSet := flag.NewFlagSet("manager", flag.ContinueOnError)
@@ -312,17 +316,9 @@ func getK8sResourceDriver(ctx context.Context, mgr manager.Manager, options mana
 		store.WithClusterDomain(options.clusterDomain),
 	)
 	if options.ngrokMetadata != "" {
-		metadata := strings.TrimSuffix(options.ngrokMetadata, ",")
-		// metadata is a comma separated list of key=value pairs.
-		// e.g. "foo=bar,baz=qux"
-		customMetadata := make(map[string]string)
-		pairs := strings.Split(metadata, ",")
-		for _, pair := range pairs {
-			kv := strings.Split(pair, "=")
-			if len(kv) != 2 {
-				return nil, fmt.Errorf("invalid metadata pair: %q", pair)
-			}
-			customMetadata[kv[0]] = kv[1]
+		customMetadata, err := util.ParseHelmDictionary(options.ngrokMetadata)
+		if err != nil {
+			return nil, fmt.Errorf("unable to parse ngrokMetadata: %w", err)
 		}
 		d.WithNgrokMetadata(customMetadata)
 	}
@@ -471,7 +467,19 @@ func enableGatewayFeatureSet(_ context.Context, _ managerOpts, mgr ctrl.Manager,
 }
 
 // enableBindingsFeatureSet enables the Bindings feature set for the operator
-func enableBindingsFeatureSet(_ context.Context, opts managerOpts, mgr ctrl.Manager, _ *store.Driver, _ ngrokapi.Clientset) error {
+func enableBindingsFeatureSet(ctx context.Context, opts managerOpts, mgr ctrl.Manager, _ *store.Driver, _ ngrokapi.Clientset) error {
+	targetServiceAnnotations, err := util.ParseHelmDictionary(opts.bindings.serviceAnnotations)
+	if err != nil {
+		setupLog.WithValues("serviceAnnotations", opts.bindings.serviceAnnotations).Error(err, "unable to parse service annotations")
+		targetServiceAnnotations = make(map[string]string)
+	}
+
+	targetServiceLabels, err := util.ParseHelmDictionary(opts.bindings.serviceLabels)
+	if err != nil {
+		setupLog.WithValues("serviceLabels", opts.bindings.serviceLabels).Error(err, "unable to parse service labels")
+		targetServiceLabels = make(map[string]string)
+	}
+
 	// BoundEndpoints
 	if err := (&bindingscontroller.BoundEndpointReconciler{
 		Client:        mgr.GetClient(),
@@ -495,6 +503,8 @@ func enableBindingsFeatureSet(_ context.Context, opts managerOpts, mgr ctrl.Mana
 		Recorder:                     mgr.GetEventRecorderFor("endpoint-binding-poller"),
 		Namespace:                    opts.namespace,
 		KubernetesOperatorConfigName: opts.releaseName,
+		TargetServiceAnnotations:     targetServiceAnnotations,
+		TargetServiceLabels:          targetServiceLabels,
 		PollingInterval:              5 * time.Minute,
 		// NOTE: This range must stay static for the current implementation.
 		PortRange: bindingscontroller.PortRangeConfig{Min: 10000, Max: 65535},

helm/ngrok-operator/templates/controller-deployment.yaml

@@ -66,6 +66,20 @@ spec:
         {{- if .Values.bindings.enabled }}
         - --bindings-name={{ .Values.bindings.name }}
         - --bindings-allowed-urls={{ join "," .Values.bindings.allowedURLs }}
+        {{- if .Values.bindings.serviceAnnotations }}
+        - --bindings-service-annotations={{- $serviceAnnotations := list -}}
+          {{- range $key, $value := .Values.bindings.serviceAnnotations }}
+          {{- $serviceAnnotations = append $serviceAnnotations (printf "%s=%s" $key $value) -}}
+          {{- end }}
+          {{- $serviceAnnotations | join "," }}
+        {{- end }}
+        {{- if .Values.bindings.serviceLabels }}
+        - --bindings-service-labels={{- $serviceLabels := list -}}
+          {{- range $key, $value := .Values.bindings.serviceLabels }}
+          {{- $serviceLabels = append $serviceLabels (printf "%s=%s" $key $value) -}}
+          {{- end }}
+          {{- $serviceLabels | join "," }}
+        {{- end }}
         {{- end }}
         {{- if .Values.description }}
         - --description={{ .Values.description | quote }}

helm/ngrok-operator/tests/controller-deployment_test.yaml

@@ -173,6 +173,25 @@ tests:
   - contains:
       path: spec.template.spec.containers[0].args
       content: --ngrokMetadata=metaDataKey1=metaDataValue1,metaDataKey2=metaDataValue2
+- it: Should pass bindings.serviceAnnotations and bindings.serviceLabels to the controller
+  set:
+    bindings:
+      enabled: true
+      serviceAnnotations:
+        serviceAnnotationKey1: serviceAnnotationValue1
+        serviceAnnotationKey2: serviceAnnotationValue2
+      serviceLabels:
+        serviceLabelKey1: serviceLabelValue1
+        serviceLabelKey2: serviceLabelValue2
+  template: controller-deployment.yaml
+  documentIndex: 0 # Document 0 is the deployment since its the first template
+  asserts:
+  - contains:
+      path: spec.template.spec.containers[0].args
+      content: --bindings-service-annotations=serviceAnnotationKey1=serviceAnnotationValue1,serviceAnnotationKey2=serviceAnnotationValue2
+  - contains:
+      path: spec.template.spec.containers[0].args
+      content: --bindings-service-labels=serviceLabelKey1=serviceLabelValue1,serviceLabelKey2=serviceLabelValue2
 - it: Should pass through extra volumes and extra volume mounts
   set:
     extraVolumes:

internal/controller/bindings/boundendpoint_controller.go

@@ -415,7 +415,6 @@ func (r *BoundEndpointReconciler) convertBoundEndpointToServices(boundEndpoint *
 
 	upstreamLabels := util.MergeMaps(commonBoundEndpointLabels, thisBindingLabels)
 	upstreamAnnotations := map[string]string{
-		// TODO(hkatz) Implement Metadata
 		LabelEndpointURL: endpointURL,
 	}
 	// upstreamService represents the Pod Forwarders as a Service

internal/controller/bindings/boundendpoint_poller.go

@@ -50,6 +50,12 @@ type BoundEndpointPoller struct {
 	// PortRange is the allocatable port range for the Service definitions to Pod Forwarders
 	PortRange PortRangeConfig
 
+	// TargetServiceAnnotations is a map of key/value pairs to attach to the BoundEndpoint's Target Service
+	TargetServiceAnnotations map[string]string
+
+	// TargetServiceAnnotations is a map of key/value pairs to attach to the BoundEndpoint's Target Service
+	TargetServiceLabels map[string]string
+
 	// portAllocator manages the unique port allocations
 	portAllocator *portBitmap
 
@@ -315,7 +321,6 @@ func (r *BoundEndpointPoller) filterBoundEndpointActions(ctx context.Context, ex
 	return toCreate, toUpdate, toDelete
 }
 
-// TODO: Metadata
 func (r *BoundEndpointPoller) createBinding(ctx context.Context, desired bindingsv1alpha1.BoundEndpoint) error {
 	log := ctrl.LoggerFrom(ctx)
 
@@ -346,6 +351,10 @@ func (r *BoundEndpointPoller) createBinding(ctx context.Context, desired binding
 				Namespace: desired.Spec.Target.Namespace,
 				Service:   desired.Spec.Target.Service,
 				Port:      desired.Spec.Target.Port,
+				Metadata: bindingsv1alpha1.TargetMetadata{
+					Annotations: r.TargetServiceAnnotations,
+					Labels:      r.TargetServiceLabels,
+				},
 			},
 		},
 	}
@@ -408,6 +417,10 @@ func (r *BoundEndpointPoller) updateBinding(ctx context.Context, desired binding
 
 	desiredName := hashURI(desired.Spec.EndpointURI)
 
+	// Attach the metadata fields to the desired boundendpoint
+	desired.Spec.Target.Metadata.Annotations = r.TargetServiceAnnotations
+	desired.Spec.Target.Metadata.Labels = r.TargetServiceLabels
+
 	var existing bindingsv1alpha1.BoundEndpoint
 	err := r.Get(ctx, client.ObjectKey{Namespace: r.Namespace, Name: desiredName}, &existing)
 	if err != nil {
@@ -526,8 +539,6 @@ func boundEndpointNeedsUpdate(existing bindingsv1alpha1.BoundEndpoint, desired b
 	}
 
 	return false
-
-	// TODO: Compare Metadata labels and annotations between configured values and existing values
 }
 
 // hashURI hashes a URI to a unique string that can be used as BoundEndpoint.metadata.name

internal/controller/bindings/boundendpoint_poller_test.go

@@ -193,6 +193,40 @@ func Test_BoundEndpointPoller_boundEndpointNeedsUpdate(t *testing.T) {
 		},
 	}
 
+	epdExample1NewMetadata := bindingsv1alpha1.BoundEndpoint{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: hashURI(uriExample1),
+		},
+		Spec: bindingsv1alpha1.BoundEndpointSpec{
+			EndpointURI: uriExample1,
+			Target: bindingsv1alpha1.EndpointTarget{
+				Namespace: "namespace1",
+				Service:   "service1",
+				Port:      8080,
+				Protocol:  "TCP",
+				Metadata: bindingsv1alpha1.TargetMetadata{
+					Annotations: map[string]string{
+						"key": "value",
+					},
+					Labels: map[string]string{
+						"key": "value",
+					},
+				},
+			},
+		},
+		Status: bindingsv1alpha1.BoundEndpointStatus{
+			HashedName: hashURI(uriExample1),
+			Endpoints: []bindingsv1alpha1.BindingEndpoint{
+				{
+					Ref:          v6.Ref{ID: "ep_abc123", URI: "example-uri"},
+					Status:       bindingsv1alpha1.StatusProvisioning,
+					ErrorCode:    "",
+					ErrorMessage: "",
+				},
+			},
+		},
+	}
+
 	uriExample2 := "https://service2.namespace2:443"
 	epdExample2 := bindingsv1alpha1.BoundEndpoint{
 		ObjectMeta: metav1.ObjectMeta{
@@ -282,6 +316,12 @@ func Test_BoundEndpointPoller_boundEndpointNeedsUpdate(t *testing.T) {
 			desired:  epdExample2NewStatus,
 			want:     true,
 		},
+		{
+			name:     "different metadata",
+			existing: epdExample1,
+			desired:  epdExample1NewMetadata,
+			want:     true,
+		},
 	}
 
 	for _, test := range tests {

internal/util/helm.go

@@ -0,0 +1,37 @@
+package util
+
+import (
+	"fmt"
+	"strings"
+)
+
+// ParseHelmDictionary parses a string in the format of a Helm dictionary and returns a map
+// - Keys and Values must be strings
+// - Format must be key=val,key=val
+func ParseHelmDictionary(dict string) (map[string]string, error) {
+	final := make(map[string]string)
+
+	if len(dict) == 0 {
+		return final, nil
+	}
+
+	dict = strings.TrimSpace(dict)
+	dict = strings.TrimSuffix(dict, ",")
+
+	pairs := strings.Split(dict, ",")
+
+	if len(pairs) == 0 {
+		return nil, fmt.Errorf("invalid metadata dictionary: %q", dict)
+	}
+
+	for _, pair := range pairs {
+		kv := strings.Split(pair, "=")
+		if len(kv) != 2 {
+			return nil, fmt.Errorf("invalid metadata pair: %q", pair)
+		}
+
+		final[kv[0]] = kv[1]
+	}
+
+	return final, nil
+}

internal/util/helm_test.go

@@ -0,0 +1,63 @@
+package util
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_ParseHelmDictionary(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		dict string
+		want map[string]string
+		err  bool
+	}{
+		{
+			name: "empty",
+			dict: "",
+			want: map[string]string{},
+			err:  false,
+		},
+		{
+			name: "single",
+			dict: "key=val",
+			want: map[string]string{"key": "val"},
+			err:  false,
+		},
+		{
+			name: "multiple",
+			dict: "key1=val1,key2=val2",
+			want: map[string]string{"key1": "val1", "key2": "val2"},
+			err:  false,
+		},
+		{
+			name: "invalid",
+			dict: "key1=val1,key2",
+			want: nil,
+			err:  true,
+		},
+		{
+			name: "invalid2",
+			dict: "key1-no-equal-sign",
+			want: nil,
+			err:  true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			assert := assert.New(t)
+			got, err := ParseHelmDictionary(tt.dict)
+			if tt.err {
+				assert.Error(err)
+			} else {
+				assert.NoError(err)
+				assert.Equal(tt.want, got)
+			}
+		})
+	}
+}