From 92788ae784410acc531eab74ac2590463cb2f481 Mon Sep 17 00:00:00 2001 From: Harrison Katz Date: Tue, 29 Oct 2024 14:18:25 -0400 Subject: [PATCH 01/11] Add clusterId and checks for KubernetesOperators registration --- .../v1alpha1/kubernetesoperator_types.go | 2 ++ cmd/api/main.go | 6 +++++ helm/ngrok-operator/CHANGELOG.md | 4 ++++ .../templates/controller-deployment.yaml | 1 + ...rok.k8s.ngrok.com_kubernetesoperators.yaml | 4 ++++ .../controller-deployment_test.yaml.snap | 2 ++ .../tests/controller-deployment_test.yaml | 2 ++ helm/ngrok-operator/values.yaml | 2 ++ .../ngrok/kubernetesoperator_controller.go | 22 ++++++++++++++----- 9 files changed, 40 insertions(+), 5 deletions(-) diff --git a/api/ngrok/v1alpha1/kubernetesoperator_types.go b/api/ngrok/v1alpha1/kubernetesoperator_types.go index 96d1ffd3..04102a1e 100644 --- a/api/ngrok/v1alpha1/kubernetesoperator_types.go +++ b/api/ngrok/v1alpha1/kubernetesoperator_types.go @@ -42,6 +42,8 @@ type ngrokAPICommon struct { // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. type KubernetesOperatorDeployment struct { + // Cluster is the name of the k8s cluster in which the operator is deployed + Cluster string `json:"cluster,omitempty"` // Name is the name of the k8s deployment for the operator Name string `json:"name,omitempty"` // The namespace in which the operator is deployed diff --git a/cmd/api/main.go b/cmd/api/main.go index 1d74c577..f31ee832 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -91,6 +91,7 @@ func main() { type managerOpts struct { // flags + clusterId string releaseName string metricsAddr string electionID string @@ -143,6 +144,7 @@ func cmd() *cobra.Command { }, } + c.Flags().StringVar(&opts.clusterId, "cluster-id", "", "Cluster ID where the ngrok-operator is installed") c.Flags().StringVar(&opts.releaseName, "release-name", "ngrok-operator", "Helm Release name for the deployed operator") c.Flags().StringVar(&opts.metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to") c.Flags().StringVar(&opts.probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") @@ -170,6 +172,9 @@ func cmd() *cobra.Command { c.Flags().StringVar(&opts.bindings.serviceLabels, "bindings-service-labels", "", "Service Labels to propagate to the target service") c.Flags().StringVar(&opts.bindings.ingressEndpoint, "bindings-ingress-endpoint", "", "The endpoint the bindings forwarder connects to") + _ = c.MarkFlagRequired("cluster-id") + _ = c.MarkFlagRequired("release-name") + opts.zapOpts = &zap.Options{} goFlagSet := flag.NewFlagSet("manager", flag.ContinueOnError) opts.zapOpts.BindFlags(goFlagSet) @@ -652,6 +657,7 @@ func createKubernetesOperator(ctx context.Context, client client.Client, opts ma _, err := controllerutil.CreateOrUpdate(ctx, client, k8sOperator, func() error { k8sOperator.Spec = ngrokv1alpha1.KubernetesOperatorSpec{ Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ + Cluster: opts.clusterId, Name: opts.releaseName, Namespace: opts.namespace, Version: version.GetVersion(), diff --git a/helm/ngrok-operator/CHANGELOG.md b/helm/ngrok-operator/CHANGELOG.md index ee9137c1..fec09731 100644 --- a/helm/ngrok-operator/CHANGELOG.md +++ b/helm/ngrok-operator/CHANGELOG.md @@ -141,6 +141,10 @@ The `ngrok-operator` helm chart now includes a `schema.json` file that can be us - Generate and commit schema.json file by @alex-bezek in [#472](https://github.com/ngrok/ngrok-operator/pull/472) +### Added + +- Required `.Values.clusterId` for the ngrok-operator to register with the ngrok API + ### Changed #### Traffic Policy diff --git a/helm/ngrok-operator/templates/controller-deployment.yaml b/helm/ngrok-operator/templates/controller-deployment.yaml index 9bcf32c1..aaf192db 100644 --- a/helm/ngrok-operator/templates/controller-deployment.yaml +++ b/helm/ngrok-operator/templates/controller-deployment.yaml @@ -59,6 +59,7 @@ spec: - /api-manager args: - --release-name={{ .Release.Name }} + - --cluster-id={{ .Values.clusterId | required "Missing required .Values.clusterId!"}} {{- include "ngrok-operator.manager.cliFeatureFlags" . | nindent 8 }} {{- if .Values.oneClickDemoMode }} - --one-click-demo-mode diff --git a/helm/ngrok-operator/templates/crds/ngrok.k8s.ngrok.com_kubernetesoperators.yaml b/helm/ngrok-operator/templates/crds/ngrok.k8s.ngrok.com_kubernetesoperators.yaml index 115e3bc1..ac2517d2 100644 --- a/helm/ngrok-operator/templates/crds/ngrok.k8s.ngrok.com_kubernetesoperators.yaml +++ b/helm/ngrok-operator/templates/crds/ngrok.k8s.ngrok.com_kubernetesoperators.yaml @@ -98,6 +98,10 @@ spec: deployment: description: Deployment information of this Kubernetes Operator properties: + cluster: + description: Cluster is the name of the k8s cluster in which the + operator is deployed + type: string name: description: Name is the name of the k8s deployment for the operator type: string diff --git a/helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap b/helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap index 0143e8b8..a8fd4f84 100644 --- a/helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap +++ b/helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap @@ -53,6 +53,7 @@ Should match all-options snapshot: containers: - args: - --release-name=RELEASE-NAME + - --cluster-id=test-cluster - --enable-feature-ingress=true - --enable-feature-gateway=false - --description="The official ngrok Kubernetes Operator." @@ -718,6 +719,7 @@ Should match default snapshot: containers: - args: - --release-name=RELEASE-NAME + - --cluster-id=test-cluster - --enable-feature-ingress=true - --enable-feature-gateway=false - --description="The official ngrok Kubernetes Operator." diff --git a/helm/ngrok-operator/tests/controller-deployment_test.yaml b/helm/ngrok-operator/tests/controller-deployment_test.yaml index 05ff0356..5c56e5f6 100644 --- a/helm/ngrok-operator/tests/controller-deployment_test.yaml +++ b/helm/ngrok-operator/tests/controller-deployment_test.yaml @@ -1,4 +1,6 @@ suite: test controller-deployment +set: + clusterId: "test-cluster" templates: - controller-deployment.yaml # The following included templates are needed due to the way helm unittest works. diff --git a/helm/ngrok-operator/values.yaml b/helm/ngrok-operator/values.yaml index 5bf1cecc..7247a06c 100644 --- a/helm/ngrok-operator/values.yaml +++ b/helm/ngrok-operator/values.yaml @@ -3,12 +3,14 @@ ## ## @param nameOverride String to partially override generated resource names ## @param fullnameOverride String to fully override generated resource names +## @param clusterId String to identify the kubernetes cluster in the ngrok dashboard (must be unique per Helm release per cluster) ## @param description ngrok-operator description that will appear in the ngrok dashboard ## @param commonLabels Labels to add to all deployed objects ## @param commonAnnotations Annotations to add to all deployed objects ## @param oneClickDemoMode If true, then the operator will startup without required fields or API registration, become Ready, but not actually be running nameOverride: "" fullnameOverride: "" +clusterId: "" description: "The official ngrok Kubernetes Operator." commonLabels: {} commonAnnotations: {} diff --git a/internal/controller/ngrok/kubernetesoperator_controller.go b/internal/controller/ngrok/kubernetesoperator_controller.go index 5e0cf20e..1af2f6a5 100644 --- a/internal/controller/ngrok/kubernetesoperator_controller.go +++ b/internal/controller/ngrok/kubernetesoperator_controller.go @@ -137,6 +137,8 @@ func (r *KubernetesOperatorReconciler) create(ctx context.Context, ko *ngrokv1al EnabledFeatures: calculateFeaturesEnabled(ko), Region: ko.Spec.Region, Deployment: ngrok.KubernetesOperatorDeployment{ + // TODO(hkatz) clusterId + // Cluster: ko.Spec.Deployment.Cluster, Name: ko.Spec.Deployment.Name, Namespace: ko.Spec.Deployment.Namespace, Version: ko.Spec.Deployment.Version, @@ -290,6 +292,7 @@ func (r *KubernetesOperatorReconciler) findExisting(ctx context.Context, ko *ngr namespaceUID, err := getNamespaceUID(ctx, r.Client, ko.GetNamespace()) if err != nil { + log.V(3).Error(err, "failed to get namespace UID") return nil, nil } @@ -305,9 +308,15 @@ func (r *KubernetesOperatorReconciler) findExisting(ctx context.Context, ko *ngr iterLogger.V(5).Info("checking if KubernetesOperator matches") + // TODO(hkatz) clusterId + // if item.Deployment.Cluster != ko.Spec.Deployment.Cluster { + // continue + // } + if item.Deployment.Name != ko.Spec.Deployment.Name { continue } + if item.Deployment.Namespace != ko.GetNamespace() { continue } @@ -320,18 +329,21 @@ func (r *KubernetesOperatorReconciler) findExisting(ctx context.Context, ko *ngr uid, err := extractNamespaceUIDFromMetadata(metadata) // In case the metadata is not a JSON object or we can't extract it, // we'll ignore it and continue our search - if err != nil || uid == "" { - continue - } - if uid != string(namespaceUID) { + if err != nil || uid == "" || uid != string(namespaceUID) { + // namespace UID does not match, but deployment information do match + // warn the user that this is an unexpected case and their ngrok-operator + // will not be able to register with the API + iterLogger.Error(err, "Namespace UID mismatch between ngrok-operator deployment and ngrok API kubernetes_operators, operator will not register!", "expected", string(namespaceUID), "actual", uid) + r.Recorder.Event(ko, v1.EventTypeWarning, "NamespaceMismatch", "Namespace UID mismatch between ngrok-operator deployment and ngrok API kubernetes_operators, operator will not register!") continue } } - iterLogger.V(3).Info("found matching KubernetesOperator") + iterLogger.V(3).Info("found matching KubernetesOperator", "id", item.ID) return item, nil } + log.V(3).Info("no matching KubernetesOperator found") return nil, iter.Err() } From 3a0631bd68cf816ae2ec01acc50bd5f05115f232 Mon Sep 17 00:00:00 2001 From: Harrison Katz Date: Thu, 31 Oct 2024 16:00:47 -0400 Subject: [PATCH 02/11] Compare binding name too --- internal/controller/ngrok/kubernetesoperator_controller.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/controller/ngrok/kubernetesoperator_controller.go b/internal/controller/ngrok/kubernetesoperator_controller.go index 1af2f6a5..81d8852b 100644 --- a/internal/controller/ngrok/kubernetesoperator_controller.go +++ b/internal/controller/ngrok/kubernetesoperator_controller.go @@ -321,6 +321,13 @@ func (r *KubernetesOperatorReconciler) findExisting(ctx context.Context, ko *ngr continue } + // bindings are enabled, check that the binding name matches + if slices.Contains(item.EnabledFeatures, featureMap[ngrokv1alpha1.KubernetesOperatorFeatureBindings]) { + if item.Binding.Name != ko.Spec.Binding.Name { + continue // possibly the same k8sop, but not the same binding + } + } + // In case the KubernetesOperator already exists in the ngrok API, check if it's the namespace // UID is the same as the one we are trying to create. If it is, use the existing one since we // get conflicts if we try to create a new one. From 348bc9bfcd059876492cb206ad011884664e8227 Mon Sep 17 00:00:00 2001 From: Harrison Katz Date: Fri, 1 Nov 2024 10:01:18 -0400 Subject: [PATCH 03/11] Add required clusterId to each helm deploy in Makefile --- Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index 6b10c9a9..487e3b69 100644 --- a/Makefile +++ b/Makefile @@ -136,6 +136,7 @@ endif KUBE_NAMESPACE ?= ngrok-operator HELM_RELEASE_NAME ?= ngrok-operator KUBE_DEPLOYMENT_NAME ?= ngrok-operator-manager +KUBE_CLUSTER_NAME ?= development .PHONY: release release: @@ -146,6 +147,7 @@ deploy: _deploy-check-env-vars docker-build manifests kustomize _helm_setup ## D helm upgrade $(HELM_RELEASE_NAME) $(HELM_CHART_DIR) --install \ --namespace $(KUBE_NAMESPACE) \ --create-namespace \ + --set clusterId=$(KUBE_CLUSTER_NAME) \ --set image.repository=$(IMG) \ --set image.tag="latest" \ --set podAnnotations."k8s\.ngrok\.com/test"="\{\"env\": \"local\"\}" \ @@ -162,6 +164,7 @@ deploy_gateway: _deploy-check-env-vars docker-build manifests kustomize _helm_se helm upgrade $(HELM_RELEASE_NAME) $(HELM_CHART_DIR) --install \ --namespace $(KUBE_NAMESPACE) \ --create-namespace \ + --set clusterId=$(KUBE_CLUSTER_NAME) \ --set image.repository=$(IMG) \ --set image.tag="latest" \ --set podAnnotations."k8s\.ngrok\.com/test"="\{\"env\": \"local\"\}" \ @@ -179,6 +182,7 @@ deploy_with_bindings: _deploy-check-env-vars docker-build manifests kustomize _h helm upgrade $(HELM_RELEASE_NAME) $(HELM_CHART_DIR) --install \ --namespace $(KUBE_NAMESPACE) \ --create-namespace \ + --set clusterId=$(KUBE_CLUSTER_NAME) \ --set image.repository=$(IMG) \ --set image.tag="latest" \ --set podAnnotations."k8s\.ngrok\.com/test"="\{\"env\": \"local\"\}" \ From 1a5e8f079fda96079d9999c9af0218ca97a33f8f Mon Sep 17 00:00:00 2001 From: hjkatz Date: Fri, 1 Nov 2024 14:01:40 +0000 Subject: [PATCH 04/11] Update README.md and values.schema.json with readme-generator-for-helm --- helm/ngrok-operator/values.schema.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/helm/ngrok-operator/values.schema.json b/helm/ngrok-operator/values.schema.json index ba3c6557..5e51ffa9 100644 --- a/helm/ngrok-operator/values.schema.json +++ b/helm/ngrok-operator/values.schema.json @@ -12,6 +12,11 @@ "description": "String to fully override generated resource names", "default": "" }, + "clusterId": { + "type": "string", + "description": "String to identify the kubernetes cluster in the ngrok dashboard (must be unique per Helm release per cluster)", + "default": "" + }, "description": { "type": "string", "description": "ngrok-operator description that will appear in the ngrok dashboard", From b9d6201b5363e98d2a6e4d5c3d51ae4c14fa041d Mon Sep 17 00:00:00 2001 From: Harrison Katz Date: Tue, 12 Nov 2024 14:40:39 -0500 Subject: [PATCH 05/11] Update helm templates for clusterId and oneClickDemoMode interplay --- .../templates/controller-deployment.yaml | 4 ++++ .../tests/controller-deployment_test.yaml | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/helm/ngrok-operator/templates/controller-deployment.yaml b/helm/ngrok-operator/templates/controller-deployment.yaml index aaf192db..0916752d 100644 --- a/helm/ngrok-operator/templates/controller-deployment.yaml +++ b/helm/ngrok-operator/templates/controller-deployment.yaml @@ -59,7 +59,11 @@ spec: - /api-manager args: - --release-name={{ .Release.Name }} + {{- if .Values.oneClickDemoMode }} + - --cluster-id=one-click-demo-mode + {{- else }} - --cluster-id={{ .Values.clusterId | required "Missing required .Values.clusterId!"}} + {{- end }} {{- include "ngrok-operator.manager.cliFeatureFlags" . | nindent 8 }} {{- if .Values.oneClickDemoMode }} - --one-click-demo-mode diff --git a/helm/ngrok-operator/tests/controller-deployment_test.yaml b/helm/ngrok-operator/tests/controller-deployment_test.yaml index 5c56e5f6..869b3e70 100644 --- a/helm/ngrok-operator/tests/controller-deployment_test.yaml +++ b/helm/ngrok-operator/tests/controller-deployment_test.yaml @@ -246,6 +246,22 @@ tests: - contains: path: spec.template.spec.containers[0].args content: --one-click-demo-mode +- it: Should pass cluster-id + 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: --cluster-id=test-cluster +- it: Should pass cluster-id as demo mode when enabled + template: controller-deployment.yaml + documentIndex: 0 # Document 0 is the deployment since its the first template + set: + oneClickDemoMode: true + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --cluster-id=one-click-demo-mode - it: Should pass log format argument if set set: log: From 5e401da88bb8d3c31b1e3195b3192a2aa2345d76 Mon Sep 17 00:00:00 2001 From: Harrison Katz Date: Thu, 14 Nov 2024 13:28:38 -0500 Subject: [PATCH 06/11] Add logging for k8sop created --- cmd/api/main.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/cmd/api/main.go b/cmd/api/main.go index f31ee832..b8c2cb41 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -18,6 +18,7 @@ package main import ( "context" + "encoding/json" "errors" "flag" "fmt" @@ -259,6 +260,7 @@ func runNormalMode(ctx context.Context, opts managerOpts, k8sClient client.Clien return fmt.Errorf("Unable to load ngrokClientSet: %w", err) } + // TODO(hkatz) for now we are hiding the k8sop API regstration behind the bindings feature flag if opts.enableFeatureBindings { // register the k8sop in the ngrok API if err := createKubernetesOperator(ctx, k8sClient, opts); err != nil { @@ -687,7 +689,12 @@ func createKubernetesOperator(ctx context.Context, client client.Client, opts ma } k8sOperator.Spec.EnabledFeatures = features - setupLog.Info("created KubernetesOperator", "name", k8sOperator.Name, "namespace", k8sOperator.Namespace, "op", fmt.Sprintf("%+v", k8sOperator.Spec.Binding)) + if data, err := json.Marshal(k8sOperator); err == nil { + setupLog.Info("created KubernetesOperator", "name", k8sOperator.Name, "namespace", k8sOperator.Namespace, "op", string(data)) + } else { + setupLog.Info("created KubernetesOperator", "name", k8sOperator.Name, "namespace", k8sOperator.Namespace, "op", fmt.Sprintf("%+v", k8sOperator)) + } + return nil }) return err From 1f4057484758832da1b3a5cb85c7078bb1e2b856 Mon Sep 17 00:00:00 2001 From: Harrison Katz Date: Thu, 14 Nov 2024 13:28:55 -0500 Subject: [PATCH 07/11] Add better matching functions --- .../ngrok/kubernetesoperator_controller.go | 144 ++++------- .../kubernetesoperator_controller_test.go | 226 ++++++++++++++++++ 2 files changed, 270 insertions(+), 100 deletions(-) create mode 100644 internal/controller/ngrok/kubernetesoperator_controller_test.go diff --git a/internal/controller/ngrok/kubernetesoperator_controller.go b/internal/controller/ngrok/kubernetesoperator_controller.go index 81d8852b..32f3c3f8 100644 --- a/internal/controller/ngrok/kubernetesoperator_controller.go +++ b/internal/controller/ngrok/kubernetesoperator_controller.go @@ -31,7 +31,6 @@ import ( "crypto/rand" "crypto/x509" "crypto/x509/pkix" - "encoding/json" "encoding/pem" "errors" "slices" @@ -132,7 +131,7 @@ func (r *KubernetesOperatorReconciler) create(ctx context.Context, ko *ngrokv1al // Not found, so we'll create the KubernetesOperator createParams := &ngrok.KubernetesOperatorCreate{ - Metadata: r.tryMergeMetadata(ctx, ko), + Metadata: ko.Spec.Metadata, Description: ko.Spec.Description, EnabledFeatures: calculateFeaturesEnabled(ko), Region: ko.Spec.Region, @@ -183,6 +182,13 @@ func (r *KubernetesOperatorReconciler) update(ctx context.Context, ko *ngrokv1al return r.updateStatus(ctx, ko, nil, err) } + // confirm that the ngrokKo we recieve matches our given ko we're updating + // otherwise we need to create a new ngrokKo with the new information and ID + if !ngrokK8sopMatchesKubernetesOperator(ngrokKo, ko) { + log.V(3).Info("existing KubernetesOperator does not match, creating new k8sop") + return r.create(ctx, ko) // create will find or create + } + return r._update(ctx, ko, ngrokKo) } @@ -250,7 +256,7 @@ func (r *KubernetesOperatorReconciler) _update(ctx context.Context, ko *ngrokv1a updateParams := &ngrok.KubernetesOperatorUpdate{ ID: ngrokKo.ID, Description: ptr.To(ko.Spec.Description), - Metadata: ptr.To(r.tryMergeMetadata(ctx, ko)), + Metadata: ptr.To(ko.Spec.Metadata), EnabledFeatures: calculateFeaturesEnabled(ko), Region: ptr.To(ko.Spec.Region), } @@ -290,12 +296,6 @@ func (r *KubernetesOperatorReconciler) _update(ctx context.Context, ko *ngrokv1a func (r *KubernetesOperatorReconciler) findExisting(ctx context.Context, ko *ngrokv1alpha1.KubernetesOperator) (*ngrok.KubernetesOperator, error) { log := ctrl.LoggerFrom(ctx) - namespaceUID, err := getNamespaceUID(ctx, r.Client, ko.GetNamespace()) - if err != nil { - log.V(3).Error(err, "failed to get namespace UID") - return nil, nil - } - iter := r.NgrokClientset.KubernetesOperators().List(&ngrok.Paging{}) for iter.Next(ctx) { item := iter.Item() @@ -307,45 +307,11 @@ func (r *KubernetesOperatorReconciler) findExisting(ctx context.Context, ko *ngr ) iterLogger.V(5).Info("checking if KubernetesOperator matches") - - // TODO(hkatz) clusterId - // if item.Deployment.Cluster != ko.Spec.Deployment.Cluster { - // continue - // } - - if item.Deployment.Name != ko.Spec.Deployment.Name { + if !ngrokK8sopMatchesKubernetesOperator(item, ko) { + iterLogger.V(5).Info("KubernetesOperator does not match") continue } - if item.Deployment.Namespace != ko.GetNamespace() { - continue - } - - // bindings are enabled, check that the binding name matches - if slices.Contains(item.EnabledFeatures, featureMap[ngrokv1alpha1.KubernetesOperatorFeatureBindings]) { - if item.Binding.Name != ko.Spec.Binding.Name { - continue // possibly the same k8sop, but not the same binding - } - } - - // In case the KubernetesOperator already exists in the ngrok API, check if it's the namespace - // UID is the same as the one we are trying to create. If it is, use the existing one since we - // get conflicts if we try to create a new one. - metadata := item.Metadata - if metadata != "" { - uid, err := extractNamespaceUIDFromMetadata(metadata) - // In case the metadata is not a JSON object or we can't extract it, - // we'll ignore it and continue our search - if err != nil || uid == "" || uid != string(namespaceUID) { - // namespace UID does not match, but deployment information do match - // warn the user that this is an unexpected case and their ngrok-operator - // will not be able to register with the API - iterLogger.Error(err, "Namespace UID mismatch between ngrok-operator deployment and ngrok API kubernetes_operators, operator will not register!", "expected", string(namespaceUID), "actual", uid) - r.Recorder.Event(ko, v1.EventTypeWarning, "NamespaceMismatch", "Namespace UID mismatch between ngrok-operator deployment and ngrok API kubernetes_operators, operator will not register!") - continue - } - } - iterLogger.V(3).Info("found matching KubernetesOperator", "id", item.ID) return item, nil } @@ -354,6 +320,39 @@ func (r *KubernetesOperatorReconciler) findExisting(ctx context.Context, ko *ngr return nil, iter.Err() } +// ngrokK8sopMatchesKubernetesOperator checks if the KubernetesOperator in the ngrok API matches the KubernetesOperator CRD +func ngrokK8sopMatchesKubernetesOperator(k8sop *ngrok.KubernetesOperator, ko *ngrokv1alpha1.KubernetesOperator) bool { + if k8sop == nil || ko == nil { + return false + } + + // TODO(hkatz) clusterId + // if item.Deployment.Cluster != ko.Spec.Deployment.Cluster { + // continue + // } + + if k8sop.Deployment.Name != ko.Spec.Deployment.Name { + return false + } + + if k8sop.Deployment.Namespace != ko.Spec.Deployment.Namespace { + return false + } + + // bindings enabled on the CRD + if slices.Contains(ko.Spec.EnabledFeatures, ngrokv1alpha1.KubernetesOperatorFeatureBindings) { + // bindings enabled in the API + if slices.Contains(k8sop.EnabledFeatures, featureMap[ngrokv1alpha1.KubernetesOperatorFeatureBindings]) { + // names must match + if k8sop.Binding.Name != ko.Spec.Binding.Name { + return false + } + } + } + + return true +} + func calculateFeaturesEnabled(ko *ngrokv1alpha1.KubernetesOperator) []string { features := []string{} @@ -442,52 +441,6 @@ func (r *KubernetesOperatorReconciler) updateTLSSecretCert(ctx context.Context, return r.Client.Patch(ctx, newSecret, client.MergeFrom(secret)) } -// Try merging the user-provided metadata in the KubernetesOperator spec with the namespace UID. -// This is done to see if we can adopt an existing KubernetesOperator in the ngrok API going forward. -// If there are any errors, the original metadata is returned. -func (r *KubernetesOperatorReconciler) tryMergeMetadata(ctx context.Context, ko *ngrokv1alpha1.KubernetesOperator) string { - namespaceUID, err := getNamespaceUID(ctx, r.Client, ko.GetNamespace()) - if err != nil { - return ko.Spec.Metadata - } - - metadata, err := mergeMetadata(ko.Spec.Metadata, namespaceUID) - if err != nil { - return ko.Spec.Metadata - } - - return metadata -} - -const UIDNamespaceMetadataKey = "namespace.uid" - -// mergeMetadata merges the UID of the namespace of the kubernetes operator with the metadata -// provided by the user. -func mergeMetadata(metadata string, namespaceUID string) (string, error) { - m := map[string]any{} - if err := json.Unmarshal([]byte(metadata), &m); err != nil { - return "", err - } - m[UIDNamespaceMetadataKey] = namespaceUID - metadataBytes, err := json.Marshal(m) - if err != nil { - return "", err - } - return string(metadataBytes), nil -} - -func extractNamespaceUIDFromMetadata(metadata string) (string, error) { - m := map[string]any{} - if err := json.Unmarshal([]byte(metadata), &m); err != nil { - return "", err - } - uid, ok := m[UIDNamespaceMetadataKey].(string) - if !ok { - return "", nil - } - return uid, nil -} - // nolint:unused func generateCSR(privKey *ecdsa.PrivateKey) ([]byte, error) { subj := pkix.Name{} @@ -508,12 +461,3 @@ func generateCSR(privKey *ecdsa.PrivateKey) ([]byte, error) { } return buf.Bytes(), nil } - -func getNamespaceUID(ctx context.Context, r client.Reader, namespace string) (string, error) { - ns := &v1.Namespace{} - err := r.Get(ctx, client.ObjectKey{Namespace: namespace, Name: namespace}, ns) - if err != nil { - return "", err - } - return string(ns.UID), nil -} diff --git a/internal/controller/ngrok/kubernetesoperator_controller_test.go b/internal/controller/ngrok/kubernetesoperator_controller_test.go new file mode 100644 index 00000000..71c0c35c --- /dev/null +++ b/internal/controller/ngrok/kubernetesoperator_controller_test.go @@ -0,0 +1,226 @@ +package ngrok + +import ( + "testing" + + "github.com/ngrok/ngrok-api-go/v6" + ngrokv1alpha1 "github.com/ngrok/ngrok-operator/api/ngrok/v1alpha1" + "github.com/stretchr/testify/assert" +) + +func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + ngrokK8sop *ngrok.KubernetesOperator + koK8sop *ngrokv1alpha1.KubernetesOperator + want bool + }{ + { + name: "both nil", + want: false, + koK8sop: nil, + ngrokK8sop: nil, + }, + { + name: "basic deployment", + want: true, + ngrokK8sop: &ngrok.KubernetesOperator{ + EnabledFeatures: []string{"Ingress"}, // API returns title cased features + Deployment: ngrok.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + }, + koK8sop: &ngrokv1alpha1.KubernetesOperator{ + Spec: ngrokv1alpha1.KubernetesOperatorSpec{ + Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress}, + }, + }, + }, + { + name: "different namespace", + want: false, + ngrokK8sop: &ngrok.KubernetesOperator{ + EnabledFeatures: []string{"Ingress"}, // API returns title cased features + Deployment: ngrok.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + }, + koK8sop: &ngrokv1alpha1.KubernetesOperator{ + Spec: ngrokv1alpha1.KubernetesOperatorSpec{ + Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "different-namespace", + }, + EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress}, + }, + }, + }, + { + name: "different name", + want: false, + ngrokK8sop: &ngrok.KubernetesOperator{ + EnabledFeatures: []string{"Ingress"}, // API returns title cased features + Deployment: ngrok.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + }, + koK8sop: &ngrokv1alpha1.KubernetesOperator{ + Spec: ngrokv1alpha1.KubernetesOperatorSpec{ + Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ + Name: "different-name", + Namespace: "ngrok-operator", + }, + EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress}, + }, + }, + }, + { + name: "bindings: same features, same name", + want: true, + ngrokK8sop: &ngrok.KubernetesOperator{ + EnabledFeatures: []string{"Ingress", "Bindings"}, // API returns title cased features + Deployment: ngrok.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + Binding: &ngrok.KubernetesOperatorBinding{ + Name: "example", + }, + }, + koK8sop: &ngrokv1alpha1.KubernetesOperator{ + Spec: ngrokv1alpha1.KubernetesOperatorSpec{ + Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress, ngrokv1alpha1.KubernetesOperatorFeatureBindings}, + Binding: &ngrokv1alpha1.KubernetesOperatorBinding{ + Name: "example", + }, + }, + }, + }, + { + name: "bindings: same features, different name", + want: false, + ngrokK8sop: &ngrok.KubernetesOperator{ + EnabledFeatures: []string{"Ingress", "Bindings"}, // API returns title cased features + Deployment: ngrok.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + Binding: &ngrok.KubernetesOperatorBinding{ + Name: "example", + }, + }, + koK8sop: &ngrokv1alpha1.KubernetesOperator{ + Spec: ngrokv1alpha1.KubernetesOperatorSpec{ + Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress, ngrokv1alpha1.KubernetesOperatorFeatureBindings}, + Binding: &ngrokv1alpha1.KubernetesOperatorBinding{ + Name: "different-name", + }, + }, + }, + }, + { + name: "bindings: different features, enabled -> disabled", + want: true, + ngrokK8sop: &ngrok.KubernetesOperator{ + EnabledFeatures: []string{"Ingress", "Bindings"}, // API returns title cased features + Deployment: ngrok.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + Binding: &ngrok.KubernetesOperatorBinding{ + Name: "example", + }, + }, + koK8sop: &ngrokv1alpha1.KubernetesOperator{ + Spec: ngrokv1alpha1.KubernetesOperatorSpec{ + Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress}, + }, + }, + }, + { + name: "bindings: different features, disabled -> enabled (same name)", + want: true, + ngrokK8sop: &ngrok.KubernetesOperator{ + EnabledFeatures: []string{"Ingress"}, // API returns title cased features + Deployment: ngrok.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + Binding: &ngrok.KubernetesOperatorBinding{ + Name: "example", + }, + }, + koK8sop: &ngrokv1alpha1.KubernetesOperator{ + Spec: ngrokv1alpha1.KubernetesOperatorSpec{ + Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress, ngrokv1alpha1.KubernetesOperatorFeatureBindings}, + Binding: &ngrokv1alpha1.KubernetesOperatorBinding{ + Name: "example", + }, + }, + }, + }, + { + name: "bindings: different features, disabled -> enabled (different name)", + // here we've redeployed the ngrok-op with the same name, but we're enabling the bindings feature + // even after it may have been enabled in the ngrok API previously + // now the binding name may be different, but we still want to adopt this matching k8sop because + // we're _enabling_ the feature and declaring the binding name + want: true, + ngrokK8sop: &ngrok.KubernetesOperator{ + EnabledFeatures: []string{"Ingress"}, // API returns title cased features + Deployment: ngrok.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + Binding: &ngrok.KubernetesOperatorBinding{ + Name: "example", + }, + }, + koK8sop: &ngrokv1alpha1.KubernetesOperator{ + Spec: ngrokv1alpha1.KubernetesOperatorSpec{ + Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ + Name: "example", + Namespace: "ngrok-operator", + }, + EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress, ngrokv1alpha1.KubernetesOperatorFeatureBindings}, + Binding: &ngrokv1alpha1.KubernetesOperatorBinding{ + Name: "different-name", + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + assert := assert.New(t) + assert.Equal(test.want, ngrokK8sopMatchesKubernetesOperator(test.ngrokK8sop, test.koK8sop)) + }) + } +} From 7faac52f2c24e6b1adacabd4d2d36f53469e2dcc Mon Sep 17 00:00:00 2001 From: hjkatz Date: Thu, 14 Nov 2024 18:30:02 +0000 Subject: [PATCH 08/11] Update README.md and values.schema.json with readme-generator-for-helm --- helm/ngrok-operator/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/helm/ngrok-operator/README.md b/helm/ngrok-operator/README.md index 492f8fa2..4e6aa4ab 100644 --- a/helm/ngrok-operator/README.md +++ b/helm/ngrok-operator/README.md @@ -39,6 +39,7 @@ To uninstall the chart: | ------------------- | ------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------- | | `nameOverride` | String to partially override generated resource names | `""` | | `fullnameOverride` | String to fully override generated resource names | `""` | +| `clusterId` | String to identify the kubernetes cluster in the ngrok dashboard (must be unique per Helm release per cluster) | `""` | | `description` | ngrok-operator description that will appear in the ngrok dashboard | `The official ngrok Kubernetes Operator.` | | `commonLabels` | Labels to add to all deployed objects | `{}` | | `commonAnnotations` | Annotations to add to all deployed objects | `{}` | From 62264f936bb770386abfd8fff540f2dda2cb01fa Mon Sep 17 00:00:00 2001 From: Harrison Katz Date: Thu, 19 Dec 2024 13:53:30 -0500 Subject: [PATCH 09/11] Rename clusterId -> clusterName --- Makefile | 6 +++--- cmd/api/main.go | 8 ++++---- helm/ngrok-operator/CHANGELOG.md | 2 +- .../templates/controller-deployment.yaml | 4 ++-- .../__snapshot__/controller-deployment_test.yaml.snap | 4 ++-- .../tests/controller-deployment_test.yaml | 10 +++++----- helm/ngrok-operator/values.yaml | 4 ++-- .../controller/ngrok/kubernetesoperator_controller.go | 4 ++-- 8 files changed, 21 insertions(+), 21 deletions(-) diff --git a/Makefile b/Makefile index 487e3b69..2c768899 100644 --- a/Makefile +++ b/Makefile @@ -147,7 +147,7 @@ deploy: _deploy-check-env-vars docker-build manifests kustomize _helm_setup ## D helm upgrade $(HELM_RELEASE_NAME) $(HELM_CHART_DIR) --install \ --namespace $(KUBE_NAMESPACE) \ --create-namespace \ - --set clusterId=$(KUBE_CLUSTER_NAME) \ + --set clusterName=$(KUBE_CLUSTER_NAME) \ --set image.repository=$(IMG) \ --set image.tag="latest" \ --set podAnnotations."k8s\.ngrok\.com/test"="\{\"env\": \"local\"\}" \ @@ -164,7 +164,7 @@ deploy_gateway: _deploy-check-env-vars docker-build manifests kustomize _helm_se helm upgrade $(HELM_RELEASE_NAME) $(HELM_CHART_DIR) --install \ --namespace $(KUBE_NAMESPACE) \ --create-namespace \ - --set clusterId=$(KUBE_CLUSTER_NAME) \ + --set clusterName=$(KUBE_CLUSTER_NAME) \ --set image.repository=$(IMG) \ --set image.tag="latest" \ --set podAnnotations."k8s\.ngrok\.com/test"="\{\"env\": \"local\"\}" \ @@ -182,7 +182,7 @@ deploy_with_bindings: _deploy-check-env-vars docker-build manifests kustomize _h helm upgrade $(HELM_RELEASE_NAME) $(HELM_CHART_DIR) --install \ --namespace $(KUBE_NAMESPACE) \ --create-namespace \ - --set clusterId=$(KUBE_CLUSTER_NAME) \ + --set clusterName=$(KUBE_CLUSTER_NAME) \ --set image.repository=$(IMG) \ --set image.tag="latest" \ --set podAnnotations."k8s\.ngrok\.com/test"="\{\"env\": \"local\"\}" \ diff --git a/cmd/api/main.go b/cmd/api/main.go index b8c2cb41..6426860b 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -92,7 +92,7 @@ func main() { type managerOpts struct { // flags - clusterId string + clusterName string releaseName string metricsAddr string electionID string @@ -145,7 +145,7 @@ func cmd() *cobra.Command { }, } - c.Flags().StringVar(&opts.clusterId, "cluster-id", "", "Cluster ID where the ngrok-operator is installed") + c.Flags().StringVar(&opts.clusterName, "cluster-name", "", "Cluster Name where the ngrok-operator is installed") c.Flags().StringVar(&opts.releaseName, "release-name", "ngrok-operator", "Helm Release name for the deployed operator") c.Flags().StringVar(&opts.metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to") c.Flags().StringVar(&opts.probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") @@ -173,7 +173,7 @@ func cmd() *cobra.Command { c.Flags().StringVar(&opts.bindings.serviceLabels, "bindings-service-labels", "", "Service Labels to propagate to the target service") c.Flags().StringVar(&opts.bindings.ingressEndpoint, "bindings-ingress-endpoint", "", "The endpoint the bindings forwarder connects to") - _ = c.MarkFlagRequired("cluster-id") + _ = c.MarkFlagRequired("cluster-name") _ = c.MarkFlagRequired("release-name") opts.zapOpts = &zap.Options{} @@ -659,7 +659,7 @@ func createKubernetesOperator(ctx context.Context, client client.Client, opts ma _, err := controllerutil.CreateOrUpdate(ctx, client, k8sOperator, func() error { k8sOperator.Spec = ngrokv1alpha1.KubernetesOperatorSpec{ Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ - Cluster: opts.clusterId, + Cluster: opts.clusterName, Name: opts.releaseName, Namespace: opts.namespace, Version: version.GetVersion(), diff --git a/helm/ngrok-operator/CHANGELOG.md b/helm/ngrok-operator/CHANGELOG.md index fec09731..e8cd84e2 100644 --- a/helm/ngrok-operator/CHANGELOG.md +++ b/helm/ngrok-operator/CHANGELOG.md @@ -143,7 +143,7 @@ The `ngrok-operator` helm chart now includes a `schema.json` file that can be us ### Added -- Required `.Values.clusterId` for the ngrok-operator to register with the ngrok API +- Required `.Values.clusterName` for the ngrok-operator to register with the ngrok API ### Changed diff --git a/helm/ngrok-operator/templates/controller-deployment.yaml b/helm/ngrok-operator/templates/controller-deployment.yaml index 0916752d..377c7e4b 100644 --- a/helm/ngrok-operator/templates/controller-deployment.yaml +++ b/helm/ngrok-operator/templates/controller-deployment.yaml @@ -60,9 +60,9 @@ spec: args: - --release-name={{ .Release.Name }} {{- if .Values.oneClickDemoMode }} - - --cluster-id=one-click-demo-mode + - --cluster-name=one-click-demo-mode {{- else }} - - --cluster-id={{ .Values.clusterId | required "Missing required .Values.clusterId!"}} + - --cluster-name={{ .Values.clusterName | required "Missing required .Values.clusterName!"}} {{- end }} {{- include "ngrok-operator.manager.cliFeatureFlags" . | nindent 8 }} {{- if .Values.oneClickDemoMode }} diff --git a/helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap b/helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap index a8fd4f84..50416705 100644 --- a/helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap +++ b/helm/ngrok-operator/tests/__snapshot__/controller-deployment_test.yaml.snap @@ -53,7 +53,7 @@ Should match all-options snapshot: containers: - args: - --release-name=RELEASE-NAME - - --cluster-id=test-cluster + - --cluster-name=test-cluster - --enable-feature-ingress=true - --enable-feature-gateway=false - --description="The official ngrok Kubernetes Operator." @@ -719,7 +719,7 @@ Should match default snapshot: containers: - args: - --release-name=RELEASE-NAME - - --cluster-id=test-cluster + - --cluster-name=test-cluster - --enable-feature-ingress=true - --enable-feature-gateway=false - --description="The official ngrok Kubernetes Operator." diff --git a/helm/ngrok-operator/tests/controller-deployment_test.yaml b/helm/ngrok-operator/tests/controller-deployment_test.yaml index 869b3e70..b6549721 100644 --- a/helm/ngrok-operator/tests/controller-deployment_test.yaml +++ b/helm/ngrok-operator/tests/controller-deployment_test.yaml @@ -1,6 +1,6 @@ suite: test controller-deployment set: - clusterId: "test-cluster" + clusterName: "test-cluster" templates: - controller-deployment.yaml # The following included templates are needed due to the way helm unittest works. @@ -246,14 +246,14 @@ tests: - contains: path: spec.template.spec.containers[0].args content: --one-click-demo-mode -- it: Should pass cluster-id +- it: Should pass cluster-name 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: --cluster-id=test-cluster -- it: Should pass cluster-id as demo mode when enabled + content: --cluster-name=test-cluster +- it: Should pass cluster-name as demo mode when enabled template: controller-deployment.yaml documentIndex: 0 # Document 0 is the deployment since its the first template set: @@ -261,7 +261,7 @@ tests: asserts: - contains: path: spec.template.spec.containers[0].args - content: --cluster-id=one-click-demo-mode + content: --cluster-name=one-click-demo-mode - it: Should pass log format argument if set set: log: diff --git a/helm/ngrok-operator/values.yaml b/helm/ngrok-operator/values.yaml index 7247a06c..e1968a3c 100644 --- a/helm/ngrok-operator/values.yaml +++ b/helm/ngrok-operator/values.yaml @@ -3,14 +3,14 @@ ## ## @param nameOverride String to partially override generated resource names ## @param fullnameOverride String to fully override generated resource names -## @param clusterId String to identify the kubernetes cluster in the ngrok dashboard (must be unique per Helm release per cluster) +## @param clusterName String to identify the kubernetes cluster in the ngrok dashboard (must be unique per Helm release per cluster) ## @param description ngrok-operator description that will appear in the ngrok dashboard ## @param commonLabels Labels to add to all deployed objects ## @param commonAnnotations Annotations to add to all deployed objects ## @param oneClickDemoMode If true, then the operator will startup without required fields or API registration, become Ready, but not actually be running nameOverride: "" fullnameOverride: "" -clusterId: "" +clusterName: "" description: "The official ngrok Kubernetes Operator." commonLabels: {} commonAnnotations: {} diff --git a/internal/controller/ngrok/kubernetesoperator_controller.go b/internal/controller/ngrok/kubernetesoperator_controller.go index 32f3c3f8..1dacd463 100644 --- a/internal/controller/ngrok/kubernetesoperator_controller.go +++ b/internal/controller/ngrok/kubernetesoperator_controller.go @@ -136,7 +136,7 @@ func (r *KubernetesOperatorReconciler) create(ctx context.Context, ko *ngrokv1al EnabledFeatures: calculateFeaturesEnabled(ko), Region: ko.Spec.Region, Deployment: ngrok.KubernetesOperatorDeployment{ - // TODO(hkatz) clusterId + // TODO(hkatz) clusterName // Cluster: ko.Spec.Deployment.Cluster, Name: ko.Spec.Deployment.Name, Namespace: ko.Spec.Deployment.Namespace, @@ -326,7 +326,7 @@ func ngrokK8sopMatchesKubernetesOperator(k8sop *ngrok.KubernetesOperator, ko *ng return false } - // TODO(hkatz) clusterId + // TODO(hkatz) clusterName // if item.Deployment.Cluster != ko.Spec.Deployment.Cluster { // continue // } From 913d6cb2dd8322085fdaaeee9f78ed936caef465 Mon Sep 17 00:00:00 2001 From: Harrison Katz Date: Thu, 19 Dec 2024 14:34:35 -0500 Subject: [PATCH 10/11] Enable Deployment.ClusterName --- .../v1alpha1/kubernetesoperator_types.go | 4 +- cmd/api/main.go | 17 +-- ...rok.k8s.ngrok.com_kubernetesoperators.yaml | 6 +- .../ngrok/kubernetesoperator_controller.go | 16 +-- .../kubernetesoperator_controller_test.go | 124 +++++++++++++----- 5 files changed, 111 insertions(+), 56 deletions(-) diff --git a/api/ngrok/v1alpha1/kubernetesoperator_types.go b/api/ngrok/v1alpha1/kubernetesoperator_types.go index 04102a1e..f8c68731 100644 --- a/api/ngrok/v1alpha1/kubernetesoperator_types.go +++ b/api/ngrok/v1alpha1/kubernetesoperator_types.go @@ -42,8 +42,8 @@ type ngrokAPICommon struct { // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. type KubernetesOperatorDeployment struct { - // Cluster is the name of the k8s cluster in which the operator is deployed - Cluster string `json:"cluster,omitempty"` + // ClusterName is the name of the k8s cluster in which the operator is deployed + ClusterName string `json:"clusterName,omitempty"` // Name is the name of the k8s deployment for the operator Name string `json:"name,omitempty"` // The namespace in which the operator is deployed diff --git a/cmd/api/main.go b/cmd/api/main.go index 6426860b..197a578d 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -260,12 +260,9 @@ func runNormalMode(ctx context.Context, opts managerOpts, k8sClient client.Clien return fmt.Errorf("Unable to load ngrokClientSet: %w", err) } - // TODO(hkatz) for now we are hiding the k8sop API regstration behind the bindings feature flag - if opts.enableFeatureBindings { - // register the k8sop in the ngrok API - if err := createKubernetesOperator(ctx, k8sClient, opts); err != nil { - return fmt.Errorf("unable to create KubernetesOperator: %w", err) - } + // register the k8sop in the ngrok API + if err := createKubernetesOperator(ctx, k8sClient, opts); err != nil { + return fmt.Errorf("unable to create KubernetesOperator: %w", err) } // k8sResourceDriver is the driver that will be used to interact with the k8s resources for all controllers @@ -659,10 +656,10 @@ func createKubernetesOperator(ctx context.Context, client client.Client, opts ma _, err := controllerutil.CreateOrUpdate(ctx, client, k8sOperator, func() error { k8sOperator.Spec = ngrokv1alpha1.KubernetesOperatorSpec{ Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ - Cluster: opts.clusterName, - Name: opts.releaseName, - Namespace: opts.namespace, - Version: version.GetVersion(), + ClusterName: opts.clusterName, + Name: opts.releaseName, + Namespace: opts.namespace, + Version: version.GetVersion(), }, Region: opts.region, } diff --git a/helm/ngrok-operator/templates/crds/ngrok.k8s.ngrok.com_kubernetesoperators.yaml b/helm/ngrok-operator/templates/crds/ngrok.k8s.ngrok.com_kubernetesoperators.yaml index ac2517d2..62786619 100644 --- a/helm/ngrok-operator/templates/crds/ngrok.k8s.ngrok.com_kubernetesoperators.yaml +++ b/helm/ngrok-operator/templates/crds/ngrok.k8s.ngrok.com_kubernetesoperators.yaml @@ -98,9 +98,9 @@ spec: deployment: description: Deployment information of this Kubernetes Operator properties: - cluster: - description: Cluster is the name of the k8s cluster in which the - operator is deployed + clusterName: + description: ClusterName is the name of the k8s cluster in which + the operator is deployed type: string name: description: Name is the name of the k8s deployment for the operator diff --git a/internal/controller/ngrok/kubernetesoperator_controller.go b/internal/controller/ngrok/kubernetesoperator_controller.go index 1dacd463..4b925231 100644 --- a/internal/controller/ngrok/kubernetesoperator_controller.go +++ b/internal/controller/ngrok/kubernetesoperator_controller.go @@ -136,11 +136,10 @@ func (r *KubernetesOperatorReconciler) create(ctx context.Context, ko *ngrokv1al EnabledFeatures: calculateFeaturesEnabled(ko), Region: ko.Spec.Region, Deployment: ngrok.KubernetesOperatorDeployment{ - // TODO(hkatz) clusterName - // Cluster: ko.Spec.Deployment.Cluster, - Name: ko.Spec.Deployment.Name, - Namespace: ko.Spec.Deployment.Namespace, - Version: ko.Spec.Deployment.Version, + ClusterName: ko.Spec.Deployment.ClusterName, + Name: ko.Spec.Deployment.Name, + Namespace: ko.Spec.Deployment.Namespace, + Version: ko.Spec.Deployment.Version, }, } @@ -326,10 +325,9 @@ func ngrokK8sopMatchesKubernetesOperator(k8sop *ngrok.KubernetesOperator, ko *ng return false } - // TODO(hkatz) clusterName - // if item.Deployment.Cluster != ko.Spec.Deployment.Cluster { - // continue - // } + if k8sop.Deployment.ClusterName != ko.Spec.Deployment.ClusterName { + return false + } if k8sop.Deployment.Name != ko.Spec.Deployment.Name { return false diff --git a/internal/controller/ngrok/kubernetesoperator_controller_test.go b/internal/controller/ngrok/kubernetesoperator_controller_test.go index 71c0c35c..a2851ee2 100644 --- a/internal/controller/ngrok/kubernetesoperator_controller_test.go +++ b/internal/controller/ngrok/kubernetesoperator_controller_test.go @@ -29,15 +29,39 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { ngrokK8sop: &ngrok.KubernetesOperator{ EnabledFeatures: []string{"Ingress"}, // API returns title cased features Deployment: ngrok.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, }, koK8sop: &ngrokv1alpha1.KubernetesOperator{ Spec: ngrokv1alpha1.KubernetesOperatorSpec{ Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", + }, + EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress}, + }, + }, + }, + { + name: "different clustername", + want: false, + ngrokK8sop: &ngrok.KubernetesOperator{ + EnabledFeatures: []string{"Ingress"}, // API returns title cased features + Deployment: ngrok.KubernetesOperatorDeployment{ + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", + }, + }, + koK8sop: &ngrokv1alpha1.KubernetesOperator{ + Spec: ngrokv1alpha1.KubernetesOperatorSpec{ + Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ + ClusterName: "different-cluster", + Name: "example", + Namespace: "ngrok-operator", }, EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress}, }, @@ -49,15 +73,17 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { ngrokK8sop: &ngrok.KubernetesOperator{ EnabledFeatures: []string{"Ingress"}, // API returns title cased features Deployment: ngrok.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, }, koK8sop: &ngrokv1alpha1.KubernetesOperator{ Spec: ngrokv1alpha1.KubernetesOperatorSpec{ Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "different-namespace", + ClusterName: "my-cluster", + Name: "example", + Namespace: "different-namespace", }, EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress}, }, @@ -69,15 +95,39 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { ngrokK8sop: &ngrok.KubernetesOperator{ EnabledFeatures: []string{"Ingress"}, // API returns title cased features Deployment: ngrok.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", + }, + }, + koK8sop: &ngrokv1alpha1.KubernetesOperator{ + Spec: ngrokv1alpha1.KubernetesOperatorSpec{ + Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ + ClusterName: "my-cluster", + Name: "different-name", + Namespace: "ngrok-operator", + }, + EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress}, + }, + }, + }, + { + name: "same name, different clustername", + want: false, + ngrokK8sop: &ngrok.KubernetesOperator{ + EnabledFeatures: []string{"Ingress"}, // API returns title cased features + Deployment: ngrok.KubernetesOperatorDeployment{ + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, }, koK8sop: &ngrokv1alpha1.KubernetesOperator{ Spec: ngrokv1alpha1.KubernetesOperatorSpec{ Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ - Name: "different-name", - Namespace: "ngrok-operator", + ClusterName: "different-cluster", + Name: "example", + Namespace: "ngrok-operator", }, EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress}, }, @@ -89,8 +139,9 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { ngrokK8sop: &ngrok.KubernetesOperator{ EnabledFeatures: []string{"Ingress", "Bindings"}, // API returns title cased features Deployment: ngrok.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, Binding: &ngrok.KubernetesOperatorBinding{ Name: "example", @@ -99,8 +150,9 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { koK8sop: &ngrokv1alpha1.KubernetesOperator{ Spec: ngrokv1alpha1.KubernetesOperatorSpec{ Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress, ngrokv1alpha1.KubernetesOperatorFeatureBindings}, Binding: &ngrokv1alpha1.KubernetesOperatorBinding{ @@ -115,8 +167,9 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { ngrokK8sop: &ngrok.KubernetesOperator{ EnabledFeatures: []string{"Ingress", "Bindings"}, // API returns title cased features Deployment: ngrok.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, Binding: &ngrok.KubernetesOperatorBinding{ Name: "example", @@ -125,8 +178,9 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { koK8sop: &ngrokv1alpha1.KubernetesOperator{ Spec: ngrokv1alpha1.KubernetesOperatorSpec{ Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress, ngrokv1alpha1.KubernetesOperatorFeatureBindings}, Binding: &ngrokv1alpha1.KubernetesOperatorBinding{ @@ -141,8 +195,9 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { ngrokK8sop: &ngrok.KubernetesOperator{ EnabledFeatures: []string{"Ingress", "Bindings"}, // API returns title cased features Deployment: ngrok.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, Binding: &ngrok.KubernetesOperatorBinding{ Name: "example", @@ -151,8 +206,9 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { koK8sop: &ngrokv1alpha1.KubernetesOperator{ Spec: ngrokv1alpha1.KubernetesOperatorSpec{ Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress}, }, @@ -164,8 +220,9 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { ngrokK8sop: &ngrok.KubernetesOperator{ EnabledFeatures: []string{"Ingress"}, // API returns title cased features Deployment: ngrok.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, Binding: &ngrok.KubernetesOperatorBinding{ Name: "example", @@ -174,8 +231,9 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { koK8sop: &ngrokv1alpha1.KubernetesOperator{ Spec: ngrokv1alpha1.KubernetesOperatorSpec{ Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress, ngrokv1alpha1.KubernetesOperatorFeatureBindings}, Binding: &ngrokv1alpha1.KubernetesOperatorBinding{ @@ -194,8 +252,9 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { ngrokK8sop: &ngrok.KubernetesOperator{ EnabledFeatures: []string{"Ingress"}, // API returns title cased features Deployment: ngrok.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, Binding: &ngrok.KubernetesOperatorBinding{ Name: "example", @@ -204,8 +263,9 @@ func Test_ngrokK8sopMatchesKubernetesOperator(t *testing.T) { koK8sop: &ngrokv1alpha1.KubernetesOperator{ Spec: ngrokv1alpha1.KubernetesOperatorSpec{ Deployment: &ngrokv1alpha1.KubernetesOperatorDeployment{ - Name: "example", - Namespace: "ngrok-operator", + ClusterName: "my-cluster", + Name: "example", + Namespace: "ngrok-operator", }, EnabledFeatures: []string{ngrokv1alpha1.KubernetesOperatorFeatureIngress, ngrokv1alpha1.KubernetesOperatorFeatureBindings}, Binding: &ngrokv1alpha1.KubernetesOperatorBinding{ From 40dd029b4c94eab700de04da85e8db642689437d Mon Sep 17 00:00:00 2001 From: hjkatz Date: Thu, 19 Dec 2024 19:57:01 +0000 Subject: [PATCH 11/11] Update README.md and values.schema.json with readme-generator-for-helm --- helm/ngrok-operator/README.md | 2 +- helm/ngrok-operator/values.schema.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/helm/ngrok-operator/README.md b/helm/ngrok-operator/README.md index 4e6aa4ab..4553d7d7 100644 --- a/helm/ngrok-operator/README.md +++ b/helm/ngrok-operator/README.md @@ -39,7 +39,7 @@ To uninstall the chart: | ------------------- | ------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------- | | `nameOverride` | String to partially override generated resource names | `""` | | `fullnameOverride` | String to fully override generated resource names | `""` | -| `clusterId` | String to identify the kubernetes cluster in the ngrok dashboard (must be unique per Helm release per cluster) | `""` | +| `clusterName` | String to identify the kubernetes cluster in the ngrok dashboard (must be unique per Helm release per cluster) | `""` | | `description` | ngrok-operator description that will appear in the ngrok dashboard | `The official ngrok Kubernetes Operator.` | | `commonLabels` | Labels to add to all deployed objects | `{}` | | `commonAnnotations` | Annotations to add to all deployed objects | `{}` | diff --git a/helm/ngrok-operator/values.schema.json b/helm/ngrok-operator/values.schema.json index 5e51ffa9..5e40ece6 100644 --- a/helm/ngrok-operator/values.schema.json +++ b/helm/ngrok-operator/values.schema.json @@ -12,7 +12,7 @@ "description": "String to fully override generated resource names", "default": "" }, - "clusterId": { + "clusterName": { "type": "string", "description": "String to identify the kubernetes cluster in the ngrok dashboard (must be unique per Helm release per cluster)", "default": ""