Load cluster domain as a config option. Fall back to default if not s…

…et (#339) * Load cluster domain from environment variable. Fall back to default svc.cluster.local if not set * organize imports in driver store * Specify clusterDomain config option via cli arg * Fix default clusterd domain to svc.cluster.local. Should not start with a dot * fix IMG target. Fix helm readme metadata * Fixes helm test * remove dot from @param for helm readme generator helper. Breaks checker * Update README.md with readme-generator-for-helm * Remove changes from changelog --------- Co-authored-by: Tomas Boros <[email protected]> Co-authored-by: fr6nco <[email protected]> Co-authored-by: Jonathan Stacks <[email protected]>
ngrok · Aug 23, 2024 · b391e08 · b391e08
1 parent bdd3627
commit b391e08
Show file tree

Hide file tree

Showing 9 changed files with 58 additions and 9 deletions.
diff --git a/Makefile b/Makefile
@@ -1,6 +1,7 @@
 
 # Image URL to use all building/pushing image targets
 IMG ?= kubernetes-ingress-controller
+
 # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
 ENVTEST_K8S_VERSION = 1.29.0
 

diff --git a/cmd/main.go b/cmd/main.go
@@ -92,6 +92,7 @@ type managerOpts struct {
 	managerName               string
 	useExperimentalGatewayAPI bool
 	zapOpts                   *zap.Options
+	clusterDomain             string
 
 	// env vars
 	namespace   string
@@ -122,6 +123,7 @@ func cmd() *cobra.Command {
 	c.Flags().StringVar(&opts.watchNamespace, "watch-namespace", "", "Namespace to watch for Kubernetes resources. Defaults to all namespaces.")
 	c.Flags().StringVar(&opts.managerName, "manager-name", "ngrok-ingress-controller-manager", "Manager name to identify unique ngrok ingress controller instances")
 	c.Flags().BoolVar(&opts.useExperimentalGatewayAPI, "use-experimental-gateway-api", false, "sets up experemental gatewayAPI")
+	c.Flags().StringVar(&opts.clusterDomain, "cluster-domain", "svc.cluster.local", "Cluster domain used in the cluster")
 	c.Flags().StringVar(&opts.rootCAs, "root-cas", "trusted", "trusted (default) or host: use the trusted ngrok agent CA or the host CA")
 	opts.zapOpts = &zap.Options{}
 	goFlagSet := flag.NewFlagSet("manager", flag.ContinueOnError)
@@ -385,7 +387,8 @@ func getDriver(ctx context.Context, mgr manager.Manager, options managerOpts) (*
 			Namespace: options.namespace,
 			Name:      options.managerName,
 		},
-		options.useExperimentalGatewayAPI,
+		store.WithGatewayEnabled(options.useExperimentalGatewayAPI),
+		store.WithClusterDomain(options.clusterDomain),
 	)
 	if options.metaData != "" {
 		metaData := strings.TrimSuffix(options.metaData, ",")

diff --git a/helm/ingress-controller/README.md b/helm/ingress-controller/README.md
@@ -66,6 +66,7 @@ To uninstall the chart:
 | `region`                             | ngrok region to create tunnels in. Defaults to connect to the closest geographical region.                            | `""`                                  |
 | `rootCAs`                            | Set to "trusted" for the ngrok agent CA or "host" to trust the host's CA. Defaults to "trusted".                      | `""`                                  |
 | `serverAddr`                         | This is the address of the ngrok server to connect to. You should set this if you are using a custom ingress address. | `""`                                  |
+| `clusterDomain`                      | Injects the cluster domain name for service discovery.                                                                | `svc.cluster.local`                   |
 | `apiURL`                             | This is the URL of the ngrok API. You should set this if you are using a custom API URL.                              | `""`                                  |
 | `metaData`                           | This is a map of key/value pairs that will be added as meta data to all ngrok api resources created                   | `{}`                                  |
 | `affinity`                           | Affinity for the controller pod assignment                                                                            | `{}`                                  |

diff --git a/helm/ingress-controller/templates/controller-deployment.yaml b/helm/ingress-controller/templates/controller-deployment.yaml
@@ -94,6 +94,9 @@ spec:
         - --metrics-bind-address=:8080
         - --election-id={{ include "kubernetes-ingress-controller.fullname" . }}-leader
         - --manager-name={{ include "kubernetes-ingress-controller.fullname" . }}-manager
+        {{- if .Values.clusterDomain }}
+        - --cluster-domain={{ .Values.clusterDomain }}
+        {{- end }}
         securityContext:
           allowPrivilegeEscalation: false
         env:

diff --git a/helm/ingress-controller/tests/__snapshot__/controller-deployment_test.yaml.snap b/helm/ingress-controller/tests/__snapshot__/controller-deployment_test.yaml.snap
diff --git a/helm/ingress-controller/tests/controller-deployment_test.yaml b/helm/ingress-controller/tests/controller-deployment_test.yaml
@@ -30,6 +30,15 @@ tests:
       mountPath: /test-volume
   asserts:
   - matchSnapshot: {}
+- it: Should use the specified cluster domain name
+  set:
+    clusterDomain: svc.example.com
+  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-domain=svc.example.com
 - it: Should use the specified secret name for the credentials secret
   set:
     credentials.secret.name: test-secret-name

diff --git a/helm/ingress-controller/values.yaml b/helm/ingress-controller/values.yaml
@@ -73,6 +73,9 @@ rootCAs: ""
 ## @param serverAddr  This is the address of the ngrok server to connect to. You should set this if you are using a custom ingress address.
 serverAddr: ""
 
+## @param clusterDomain Injects the cluster domain name for service discovery.
+clusterDomain: svc.cluster.local
+
 ## @param apiURL  This is the URL of the ngrok API. You should set this if you are using a custom API URL.
 apiURL: ""
 

diff --git a/internal/store/driver.go b/internal/store/driver.go
@@ -29,7 +29,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const clusterDomain = "svc.cluster.local" // TODO: We can technically figure this out by looking at things like our resolv.conf or we can just take this as a helm option
+const defaultClusterDomain = "svc.cluster.local"
 
 var domainNameForResourceNameReplacer = strings.NewReplacer(
 	".", "-", // replace dots with dashes
@@ -56,6 +56,7 @@ type Driver struct {
 	ingressMetadata string
 	gatewayMetadata string
 	managerName     types.NamespacedName
+	clusterDomain   string
 
 	syncMu              sync.Mutex
 	syncRunning         bool
@@ -66,18 +67,44 @@ type Driver struct {
 	gatewayEnabled bool
 }
 
+type DriverOpt func(*Driver)
+
+func WithGatewayEnabled(enabled bool) DriverOpt {
+	return func(d *Driver) {
+		d.gatewayEnabled = enabled
+	}
+}
+
+func WithSyncAllowConcurrent(allowed bool) DriverOpt {
+	return func(d *Driver) {
+		d.syncAllowConcurrent = allowed
+	}
+}
+
+func WithClusterDomain(domain string) DriverOpt {
+	return func(d *Driver) {
+		d.clusterDomain = domain
+	}
+}
+
 // NewDriver creates a new driver with a basic logger and cache store setup
-func NewDriver(logger logr.Logger, scheme *runtime.Scheme, controllerName string, managerName types.NamespacedName, gatewayEnabled bool) *Driver {
+func NewDriver(logger logr.Logger, scheme *runtime.Scheme, controllerName string, managerName types.NamespacedName, opts ...DriverOpt) *Driver {
 	cacheStores := NewCacheStores(logger)
 	s := New(cacheStores, controllerName, logger)
-	return &Driver{
+	d := &Driver{
 		store:          s,
 		cacheStores:    cacheStores,
 		log:            logger,
 		scheme:         scheme,
 		managerName:    managerName,
-		gatewayEnabled: gatewayEnabled,
+		gatewayEnabled: false,
+		clusterDomain:  defaultClusterDomain,
 	}
+
+	for _, opt := range opts {
+		opt(d)
+	}
+	return d
 }
 
 // WithMetaData allows you to pass in custom metadata to be added to all resources created by the controller
@@ -1544,7 +1571,7 @@ func (d *Driver) calculateTunnelsFromIngress(tunnels map[tunnelKey]ingressv1alph
 				key := tunnelKey{ingress.Namespace, serviceName, strconv.Itoa(int(servicePort))}
 				tunnel, found := tunnels[key]
 				if !found {
-					targetAddr := fmt.Sprintf("%s.%s.%s:%d", serviceName, key.namespace, clusterDomain, servicePort)
+					targetAddr := fmt.Sprintf("%s.%s.%s:%d", serviceName, key.namespace, d.clusterDomain, servicePort)
 					tunnel = ingressv1alpha1.Tunnel{
 						ObjectMeta: metav1.ObjectMeta{
 							GenerateName:    fmt.Sprintf("%s-%d-", serviceName, servicePort),
@@ -1610,7 +1637,7 @@ func (d *Driver) calculateTunnelsFromGateway(tunnels map[tunnelKey]ingressv1alph
 				key := tunnelKey{httproute.Namespace, serviceName, strconv.Itoa(int(servicePort))}
 				tunnel, found := tunnels[key]
 				if !found {
-					targetAddr := fmt.Sprintf("%s.%s.%s:%d", serviceName, key.namespace, clusterDomain, servicePort)
+					targetAddr := fmt.Sprintf("%s.%s.%s:%d", serviceName, key.namespace, d.clusterDomain, servicePort)
 					tunnel = ingressv1alpha1.Tunnel{
 						ObjectMeta: metav1.ObjectMeta{
 							GenerateName:    fmt.Sprintf("%s-%d-", serviceName, servicePort),

diff --git a/internal/store/driver_test.go b/internal/store/driver_test.go
@@ -39,9 +39,9 @@ var _ = Describe("Driver", func() {
 			scheme,
 			defaultControllerName,
 			types.NamespacedName{Name: defaultManagerName},
-			false,
+			WithGatewayEnabled(false),
+			WithSyncAllowConcurrent(true),
 		)
-		driver.syncAllowConcurrent = true
 	})
 
 	Describe("Seed", func() {