From 83444dca24ba10581552bbdf68a2cd127d6e4826 Mon Sep 17 00:00:00 2001 From: "Deavon M. McCaffery" Date: Fri, 4 Nov 2022 22:31:34 +0000 Subject: [PATCH] fix(flux2): add variable to customise cluster domain (#139) * add new `clusterDomain` variable which defaults to `cluster.local` * use `clusterDomain` for source-controller `storage-adv-addr` argument * use `clusterDomain` for `events-addr` argument NOTES: The source-controller manifest included a hard-coded advertising address with `cluster.local`. In some environments, the cluster domain is modified, which broke the kustomize controller from being able to resolve the source controller to acquire artifacts. BREAKING CHANGE: The `eventsaddr` variable has been removed as it is no longer necessary. The new `clusterDomain` variable is now used to create a fully-qualified addresses throughout the deployment manifiests. Signed-off-by: Deavon M. McCaffery --- charts/flux2/Chart.yaml | 6 +++-- charts/flux2/README.md | 4 +-- charts/flux2/templates/helm-controller.yaml | 2 +- .../image-automation-controller.yaml | 2 +- .../templates/image-reflector-controller.yaml | 2 +- .../flux2/templates/kustomize-controller.yaml | 2 +- charts/flux2/templates/source-controller.yaml | 4 +-- .../helm-controller_test.yaml.snap | 4 +-- ...image-automation-controller_test.yaml.snap | 4 +-- .../image-reflector-controller_test.yaml.snap | 4 +-- ...kustomize-controller-secret_test.yaml.snap | 2 +- .../kustomize-controller_test.yaml.snap | 4 +-- .../notification-controller_test.yaml.snap | 2 +- .../source-controller_test.yaml.snap | 4 +-- charts/flux2/tests/helm-controller_test.yaml | 25 +++++++++++++------ .../image-automation-controller_test.yaml | 22 ++++++++++++---- .../image-reflector-controller_test.yaml | 22 ++++++++++++---- .../tests/kustomize-controller_test.yaml | 25 +++++++++++++------ .../flux2/tests/source-controller_test.yaml | 19 +++++++++++--- charts/flux2/values.yaml | 12 +-------- 20 files changed, 111 insertions(+), 60 deletions(-) diff --git a/charts/flux2/Chart.yaml b/charts/flux2/Chart.yaml index 7cb4735..d9e4afb 100644 --- a/charts/flux2/Chart.yaml +++ b/charts/flux2/Chart.yaml @@ -2,10 +2,12 @@ apiVersion: v2 name: flux2 description: A Helm chart for flux2 type: application -version: 1.7.0 +version: 2.0.0 appVersion: 0.36.0 sources: - https://github.com/fluxcd-community/helm-charts annotations: artifacthub.io/changes: | - - "[Chore]: Update App Version to upstream 0.36.0" + - "[added]: `clusterDomain` to correctly apply fqdns for event and storage addresses" + - "[removed]: `eventsaddr` has been removed in favour of `clusterDomain`" + - "[fixed]: storage address in source controller assumed cluster domain was always `cluster.local`" diff --git a/charts/flux2/README.md b/charts/flux2/README.md index e3ec215..45f671a 100644 --- a/charts/flux2/README.md +++ b/charts/flux2/README.md @@ -1,6 +1,6 @@ # flux2 -![Version: 1.7.0](https://img.shields.io/badge/Version-1.7.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 0.36.0](https://img.shields.io/badge/AppVersion-0.36.0-informational?style=flat-square) +![Version: 2.0.0](https://img.shields.io/badge/Version-2.0.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 0.36.0](https://img.shields.io/badge/AppVersion-0.36.0-informational?style=flat-square) A Helm chart for flux2 @@ -19,7 +19,7 @@ This helm chart is maintain and released by the fluxcd-community on a best effor | cli.nodeSelector | object | `{}` | | | cli.tag | string | `"v0.36.0"` | | | cli.tolerations | list | `[]` | | -| eventsaddr | string | `"http://notification-controller/"` | Maybe you need to use full domain name here, if you deploy flux in environments that use http proxy. In such environments they normally add `.cluster.local` and `.local` suffixes to `no_proxy` variable in order to prevent cluster-local traffic from going through http proxy. Without fully specified domain they need to mention `notifications-controller` explicitly in `no_proxy` variable after debugging http proxy logs eg: http://notification-controller.[NAMESPACE].svc.[CLUSTERDOMAIN] if notification controller is disabled this is not set | +| clusterDomain | string | `"cluster.local"` | | | extraObjects | list | `[]` | Array of extra K8s manifests to deploy | | helmcontroller.affinity | object | `{}` | | | helmcontroller.annotations."prometheus.io/port" | string | `"8080"` | | diff --git a/charts/flux2/templates/helm-controller.yaml b/charts/flux2/templates/helm-controller.yaml index 8e73cf2..66a7e01 100644 --- a/charts/flux2/templates/helm-controller.yaml +++ b/charts/flux2/templates/helm-controller.yaml @@ -39,7 +39,7 @@ spec: - --default-service-account={{ .Values.multitenancy.defaultServiceAccount | default "default" }} {{- end}} {{- if .Values.notificationcontroller.create }} - - --events-addr={{ .Values.eventsaddr }} + - --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.{{ .Values.clusterDomain | default "cluster.local" }}. {{- end}} - --watch-all-namespaces={{ .Values.watchallnamespaces }} - --log-level={{ .Values.loglevel | default "info" }} diff --git a/charts/flux2/templates/image-automation-controller.yaml b/charts/flux2/templates/image-automation-controller.yaml index 28c9d85..f7c23ae 100644 --- a/charts/flux2/templates/image-automation-controller.yaml +++ b/charts/flux2/templates/image-automation-controller.yaml @@ -38,7 +38,7 @@ spec: - --no-cross-namespace-refs=true {{- end}} {{- if .Values.notificationcontroller.create }} - - --events-addr={{ .Values.eventsaddr }} + - --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.{{ .Values.clusterDomain | default "cluster.local" }}. {{- end}} - --watch-all-namespaces={{ .Values.watchallnamespaces }} - --log-level={{ .Values.loglevel | default "info" }} diff --git a/charts/flux2/templates/image-reflector-controller.yaml b/charts/flux2/templates/image-reflector-controller.yaml index 0872f03..5f539a6 100644 --- a/charts/flux2/templates/image-reflector-controller.yaml +++ b/charts/flux2/templates/image-reflector-controller.yaml @@ -38,7 +38,7 @@ spec: - --no-cross-namespace-refs=true {{- end}} {{- if .Values.notificationcontroller.create }} - - --events-addr={{ .Values.eventsaddr }} + - --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.{{ .Values.clusterDomain | default "cluster.local" }}. {{- end}} - --watch-all-namespaces={{ .Values.watchallnamespaces }} - --log-level={{ .Values.loglevel | default "info" }} diff --git a/charts/flux2/templates/kustomize-controller.yaml b/charts/flux2/templates/kustomize-controller.yaml index 7ffa436..a268ceb 100644 --- a/charts/flux2/templates/kustomize-controller.yaml +++ b/charts/flux2/templates/kustomize-controller.yaml @@ -39,7 +39,7 @@ spec: - --default-service-account={{ .Values.multitenancy.defaultServiceAccount | default "default" }} {{- end}} {{- if .Values.notificationcontroller.create }} - - --events-addr={{ .Values.eventsaddr }} + - --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.{{ .Values.clusterDomain | default "cluster.local" }}. {{- end}} - --watch-all-namespaces={{ .Values.watchallnamespaces }} - --log-level={{ .Values.loglevel | default "info" }} diff --git a/charts/flux2/templates/source-controller.yaml b/charts/flux2/templates/source-controller.yaml index ec9768a..f6a87f1 100644 --- a/charts/flux2/templates/source-controller.yaml +++ b/charts/flux2/templates/source-controller.yaml @@ -37,14 +37,14 @@ spec: containers: - args: {{- if .Values.notificationcontroller.create }} - - --events-addr={{ .Values.eventsaddr }} + - --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.{{ .Values.clusterDomain | default "cluster.local" }}. {{- end}} - --watch-all-namespaces={{ .Values.watchallnamespaces }} - --log-level={{ .Values.loglevel | default "info" }} - --log-encoding=json - --enable-leader-election - --storage-path=/data - - --storage-adv-addr=source-controller.$(RUNTIME_NAMESPACE).svc.cluster.local. + - --storage-adv-addr=source-controller.$(RUNTIME_NAMESPACE).svc.{{ .Values.clusterDomain | default "cluster.local" }}. {{- range .Values.sourcecontroller.container.additionalargs }} - {{ . }} {{- end}} diff --git a/charts/flux2/tests/__snapshot__/helm-controller_test.yaml.snap b/charts/flux2/tests/__snapshot__/helm-controller_test.yaml.snap index 777fffd..8eb1014 100644 --- a/charts/flux2/tests/__snapshot__/helm-controller_test.yaml.snap +++ b/charts/flux2/tests/__snapshot__/helm-controller_test.yaml.snap @@ -9,7 +9,7 @@ should match snapshot of default values: app.kubernetes.io/part-of: flux app.kubernetes.io/version: 0.36.0 control-plane: controller - helm.sh/chart: flux2-1.7.0 + helm.sh/chart: flux2-2.0.0 name: helm-controller spec: replicas: 1 @@ -28,7 +28,7 @@ should match snapshot of default values: spec: containers: - args: - - --events-addr=http://notification-controller/ + - --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.cluster.local. - --watch-all-namespaces=true - --log-level=info - --log-encoding=json diff --git a/charts/flux2/tests/__snapshot__/image-automation-controller_test.yaml.snap b/charts/flux2/tests/__snapshot__/image-automation-controller_test.yaml.snap index f1b99e5..d46315d 100644 --- a/charts/flux2/tests/__snapshot__/image-automation-controller_test.yaml.snap +++ b/charts/flux2/tests/__snapshot__/image-automation-controller_test.yaml.snap @@ -9,7 +9,7 @@ should match snapshot of default values: app.kubernetes.io/part-of: flux app.kubernetes.io/version: 0.36.0 control-plane: controller - helm.sh/chart: flux2-1.7.0 + helm.sh/chart: flux2-2.0.0 name: image-automation-controller spec: replicas: 1 @@ -26,7 +26,7 @@ should match snapshot of default values: spec: containers: - args: - - --events-addr=http://notification-controller/ + - --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.cluster.local. - --watch-all-namespaces=true - --log-level=info - --log-encoding=json diff --git a/charts/flux2/tests/__snapshot__/image-reflector-controller_test.yaml.snap b/charts/flux2/tests/__snapshot__/image-reflector-controller_test.yaml.snap index d4d5447..87fb528 100644 --- a/charts/flux2/tests/__snapshot__/image-reflector-controller_test.yaml.snap +++ b/charts/flux2/tests/__snapshot__/image-reflector-controller_test.yaml.snap @@ -9,7 +9,7 @@ should match snapshot of default values: app.kubernetes.io/part-of: flux app.kubernetes.io/version: 0.36.0 control-plane: controller - helm.sh/chart: flux2-1.7.0 + helm.sh/chart: flux2-2.0.0 name: image-reflector-controller spec: replicas: 1 @@ -26,7 +26,7 @@ should match snapshot of default values: spec: containers: - args: - - --events-addr=http://notification-controller/ + - --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.cluster.local. - --watch-all-namespaces=true - --log-level=info - --log-encoding=json diff --git a/charts/flux2/tests/__snapshot__/kustomize-controller-secret_test.yaml.snap b/charts/flux2/tests/__snapshot__/kustomize-controller-secret_test.yaml.snap index 02d68d9..eb234cb 100644 --- a/charts/flux2/tests/__snapshot__/kustomize-controller-secret_test.yaml.snap +++ b/charts/flux2/tests/__snapshot__/kustomize-controller-secret_test.yaml.snap @@ -10,7 +10,7 @@ should match snapshot of default values: app.kubernetes.io/managed-by: Helm app.kubernetes.io/part-of: flux app.kubernetes.io/version: 0.36.0 - helm.sh/chart: flux2-1.7.0 + helm.sh/chart: flux2-2.0.0 name: test1 namespace: NAMESPACE type: Opaque diff --git a/charts/flux2/tests/__snapshot__/kustomize-controller_test.yaml.snap b/charts/flux2/tests/__snapshot__/kustomize-controller_test.yaml.snap index b72c2bc..c877887 100644 --- a/charts/flux2/tests/__snapshot__/kustomize-controller_test.yaml.snap +++ b/charts/flux2/tests/__snapshot__/kustomize-controller_test.yaml.snap @@ -9,7 +9,7 @@ should match snapshot of default values: app.kubernetes.io/part-of: flux app.kubernetes.io/version: 0.36.0 control-plane: controller - helm.sh/chart: flux2-1.7.0 + helm.sh/chart: flux2-2.0.0 name: kustomize-controller spec: replicas: 1 @@ -26,7 +26,7 @@ should match snapshot of default values: spec: containers: - args: - - --events-addr=http://notification-controller/ + - --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.cluster.local. - --watch-all-namespaces=true - --log-level=info - --log-encoding=json diff --git a/charts/flux2/tests/__snapshot__/notification-controller_test.yaml.snap b/charts/flux2/tests/__snapshot__/notification-controller_test.yaml.snap index be164ab..d7593e8 100644 --- a/charts/flux2/tests/__snapshot__/notification-controller_test.yaml.snap +++ b/charts/flux2/tests/__snapshot__/notification-controller_test.yaml.snap @@ -9,7 +9,7 @@ should match snapshot of default values: app.kubernetes.io/part-of: flux app.kubernetes.io/version: 0.36.0 control-plane: controller - helm.sh/chart: flux2-1.7.0 + helm.sh/chart: flux2-2.0.0 name: notification-controller spec: replicas: 1 diff --git a/charts/flux2/tests/__snapshot__/source-controller_test.yaml.snap b/charts/flux2/tests/__snapshot__/source-controller_test.yaml.snap index f0c27c4..c17b6e0 100644 --- a/charts/flux2/tests/__snapshot__/source-controller_test.yaml.snap +++ b/charts/flux2/tests/__snapshot__/source-controller_test.yaml.snap @@ -9,7 +9,7 @@ should match snapshot of default values: app.kubernetes.io/part-of: flux app.kubernetes.io/version: 0.36.0 control-plane: controller - helm.sh/chart: flux2-1.7.0 + helm.sh/chart: flux2-2.0.0 name: source-controller spec: replicas: 1 @@ -28,7 +28,7 @@ should match snapshot of default values: spec: containers: - args: - - --events-addr=http://notification-controller/ + - --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.cluster.local. - --watch-all-namespaces=true - --log-level=info - --log-encoding=json diff --git a/charts/flux2/tests/helm-controller_test.yaml b/charts/flux2/tests/helm-controller_test.yaml index 1e3b20f..595530c 100644 --- a/charts/flux2/tests/helm-controller_test.yaml +++ b/charts/flux2/tests/helm-controller_test.yaml @@ -30,11 +30,10 @@ tests: asserts: - contains: path: spec.template.spec.containers[0].args - content: - --testlabel1=testvalue1 + content: --testlabel1=testvalue1 - it: should match snapshot of default values asserts: - - matchSnapshot: { } + - matchSnapshot: {} set: helmcontroller.labels: labeltestkey: labeltestvalue @@ -56,12 +55,10 @@ tests: of: apps/v1 - contains: path: spec.template.spec.containers[0].args - content: - --no-cross-namespace-refs=true + content: --no-cross-namespace-refs=true - contains: path: spec.template.spec.containers[0].args - content: - --default-service-account=test1 + content: --default-service-account=test1 - it: should set imagePullPolicy to Always set: helmcontroller.imagePullPolicy: Always @@ -74,3 +71,17 @@ tests: - equal: path: spec.template.spec.containers[0].imagePullPolicy value: IfNotPresent + - it: should use default cluster domain when null + set: + clusterDomain: null + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.cluster.local. + - it: should use custom cluster domain + set: + clusterDomain: custom.domain + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.custom.domain. diff --git a/charts/flux2/tests/image-automation-controller_test.yaml b/charts/flux2/tests/image-automation-controller_test.yaml index 58919e7..9a0829c 100644 --- a/charts/flux2/tests/image-automation-controller_test.yaml +++ b/charts/flux2/tests/image-automation-controller_test.yaml @@ -26,11 +26,10 @@ tests: asserts: - contains: path: spec.template.spec.containers[0].args - content: - --testlabel1=testvalue1 + content: --testlabel1=testvalue1 - it: should match snapshot of default values asserts: - - matchSnapshot: { } + - matchSnapshot: {} - it: should have args for Multi-tenancy lockdown capabilities: majorVersion: 1 @@ -47,8 +46,7 @@ tests: of: apps/v1 - contains: path: spec.template.spec.containers[0].args - content: - --no-cross-namespace-refs=true + content: --no-cross-namespace-refs=true - it: should set imagePullPolicy to Always set: imageautomationcontroller.imagePullPolicy: Always @@ -61,3 +59,17 @@ tests: - equal: path: spec.template.spec.containers[0].imagePullPolicy value: IfNotPresent + - it: should use default cluster domain when null + set: + clusterDomain: null + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.cluster.local. + - it: should use custom cluster domain + set: + clusterDomain: custom.domain + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.custom.domain. diff --git a/charts/flux2/tests/image-reflector-controller_test.yaml b/charts/flux2/tests/image-reflector-controller_test.yaml index 86a5614..364f7ac 100644 --- a/charts/flux2/tests/image-reflector-controller_test.yaml +++ b/charts/flux2/tests/image-reflector-controller_test.yaml @@ -27,11 +27,10 @@ tests: asserts: - contains: path: spec.template.spec.containers[0].args - content: - --testlabel1=testvalue1 + content: --testlabel1=testvalue1 - it: should match snapshot of default values asserts: - - matchSnapshot: { } + - matchSnapshot: {} - it: should have args for Multi-tenancy lockdown capabilities: majorVersion: 1 @@ -48,8 +47,7 @@ tests: of: apps/v1 - contains: path: spec.template.spec.containers[0].args - content: - --no-cross-namespace-refs=true + content: --no-cross-namespace-refs=true - it: should set imagePullPolicy to Always set: imagereflectorcontroller.imagePullPolicy: Always @@ -62,3 +60,17 @@ tests: - equal: path: spec.template.spec.containers[0].imagePullPolicy value: IfNotPresent + - it: should use default cluster domain when null + set: + clusterDomain: null + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.cluster.local. + - it: should use custom cluster domain + set: + clusterDomain: custom.domain + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.custom.domain. diff --git a/charts/flux2/tests/kustomize-controller_test.yaml b/charts/flux2/tests/kustomize-controller_test.yaml index d26120e..c86de3c 100644 --- a/charts/flux2/tests/kustomize-controller_test.yaml +++ b/charts/flux2/tests/kustomize-controller_test.yaml @@ -26,11 +26,10 @@ tests: asserts: - contains: path: spec.template.spec.containers[0].args - content: - --testlabel1=testvalue1 + content: --testlabel1=testvalue1 - it: should match snapshot of default values asserts: - - matchSnapshot: { } + - matchSnapshot: {} - it: should have args for Multi-tenancy lockdown capabilities: majorVersion: 1 @@ -48,12 +47,10 @@ tests: of: apps/v1 - contains: path: spec.template.spec.containers[0].args - content: - --no-cross-namespace-refs=true + content: --no-cross-namespace-refs=true - contains: path: spec.template.spec.containers[0].args - content: - --default-service-account=test1 + content: --default-service-account=test1 - it: should set imagePullPolicy to Always set: kustomizecontroller.imagePullPolicy: Always @@ -66,3 +63,17 @@ tests: - equal: path: spec.template.spec.containers[0].imagePullPolicy value: IfNotPresent + - it: should use default cluster domain when null + set: + clusterDomain: null + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.cluster.local. + - it: should use custom cluster domain + set: + clusterDomain: custom.domain + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --events-addr=http://notification-controller.$(RUNTIME_NAMESPACE).svc.custom.domain. diff --git a/charts/flux2/tests/source-controller_test.yaml b/charts/flux2/tests/source-controller_test.yaml index cf3bf37..7e70136 100644 --- a/charts/flux2/tests/source-controller_test.yaml +++ b/charts/flux2/tests/source-controller_test.yaml @@ -26,11 +26,10 @@ tests: asserts: - contains: path: spec.template.spec.containers[0].args - content: - --testlabel1=testvalue1 + content: --testlabel1=testvalue1 - it: should match snapshot of default values asserts: - - matchSnapshot: { } + - matchSnapshot: {} - it: should set imagePullPolicy to Always set: sourcecontroller.imagePullPolicy: Always @@ -43,3 +42,17 @@ tests: - equal: path: spec.template.spec.containers[0].imagePullPolicy value: IfNotPresent + - it: should use default cluster domain when null + set: + clusterDomain: null + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --storage-adv-addr=source-controller.$(RUNTIME_NAMESPACE).svc.cluster.local. + - it: should use custom cluster domain + set: + clusterDomain: custom.domain + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: --storage-adv-addr=source-controller.$(RUNTIME_NAMESPACE).svc.custom.domain. diff --git a/charts/flux2/values.yaml b/charts/flux2/values.yaml index 46edcd6..c11ad3b 100644 --- a/charts/flux2/values.yaml +++ b/charts/flux2/values.yaml @@ -16,17 +16,7 @@ multitenancy: # minimum set of permissions. privileged: true -# -- Maybe you need to use full domain name here, if you deploy flux -# in environments that use http proxy. -# -# In such environments they normally add `.cluster.local` and `.local` -# suffixes to `no_proxy` variable in order to prevent cluster-local -# traffic from going through http proxy. Without fully specified -# domain they need to mention `notifications-controller` explicitly in -# `no_proxy` variable after debugging http proxy logs -# eg: http://notification-controller.[NAMESPACE].svc.[CLUSTERDOMAIN] -# if notification controller is disabled this is not set -eventsaddr: http://notification-controller/ +clusterDomain: cluster.local cli: image: ghcr.io/fluxcd/flux-cli