Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load cluster domain as a config option. Fall back to default if not set #339

Merged
merged 11 commits into from
Aug 23, 2024
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -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

Expand Down
5 changes: 4 additions & 1 deletion cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ type managerOpts struct {
managerName string
useExperimentalGatewayAPI bool
zapOpts *zap.Options
clusterDomain string

// env vars
namespace string
Expand Down Expand Up @@ -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")
jonstacks marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down Expand Up @@ -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, ",")
Expand Down
1 change: 1 addition & 0 deletions helm/ingress-controller/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | `{}` |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions helm/ingress-controller/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: ""

Expand Down
39 changes: 33 additions & 6 deletions internal/store/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -56,6 +56,7 @@ type Driver struct {
ingressMetadata string
gatewayMetadata string
managerName types.NamespacedName
clusterDomain string

syncMu sync.Mutex
syncRunning bool
Expand All @@ -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
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions internal/store/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Loading