diff --git a/CHANGELOG.md b/CHANGELOG.md index 3aca52b355..1556070952 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ BREAKING_CHANGES: * `client.snaphostAgent.replicas` * `client.snaphostAgent.serviceAccount` * Remove `global.secretsBackend.vault.consulSnapshotAgentRole` value. You should now use the `global.secretsBackend.vault.consulServerRole` for access to any Vault secrets. + * Change`dns.enabled` and `dns.enableRedirection` to default to the value of `connectInject.transparentProxy.defaultEnabled`. + Previously, `dns.enabled` defaulted to the value of `global.enabled` and `dns.enableRedirection` defaulted to the + value to `false`. [[GH-1688](https://github.com/hashicorp/consul-k8s/pull/1688)] * Peering: * Remove support for customizing the server addresses in peering token generation. Instead, mesh gateways should be used to establish peering connections if the server pods are not directly reachable. [[GH-1610](https://github.com/hashicorp/consul-k8s/pull/1610)] diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml index 3dee9c4101..e4a63095cf 100644 --- a/charts/consul/templates/connect-inject-deployment.yaml +++ b/charts/consul/templates/connect-inject-deployment.yaml @@ -5,10 +5,10 @@ {{- if and .Values.global.adminPartitions.enabled (not .Values.global.enableConsulNamespaces) }}{{ fail "global.enableConsulNamespaces must be true if global.adminPartitions.enabled=true" }}{{ end }} {{ template "consul.validateVaultWebhookCertConfiguration" . }} {{- template "consul.reservedNamesFailer" (list .Values.connectInject.consulNamespaces.consulDestinationNamespace "connectInject.consulNamespaces.consulDestinationNamespace") }} -{{- $serverEnabled := (or (and (ne (.Values.server.enabled | toString) "-") .Values.server.enabled) (and (eq (.Values.server.enabled | toString) "-") .Values.global.enabled)) -}} -{{- $serverExposeServiceEnabled := (or (and (ne (.Values.server.exposeService.enabled | toString) "-") .Values.server.exposeService.enabled) (and (eq (.Values.server.exposeService.enabled | toString) "-") .Values.global.adminPartitions.enabled)) -}} {{- if and .Values.externalServers.enabled (not .Values.externalServers.hosts) }}{{ fail "externalServers.hosts must be set if externalServers.enabled is true" }}{{ end -}} {{- if and .Values.externalServers.skipServerWatch (not .Values.externalServers.enabled) }}{{ fail "externalServers.enabled must be set if externalServers.skipServerWatch is true" }}{{ end -}} +{{- $dnsEnabled := (or (and (ne (.Values.dns.enabled | toString) "-") .Values.dns.enabled) (and (eq (.Values.dns.enabled | toString) "-") .Values.connectInject.transparentProxy.defaultEnabled)) -}} +{{- $dnsRedirectionEnabled := (or (and (ne (.Values.dns.enableRedirection | toString) "-") .Values.dns.enableRedirection) (and (eq (.Values.dns.enableRedirection | toString) "-") .Values.connectInject.transparentProxy.defaultEnabled)) -}} {{ template "consul.validateRequiredCloudSecretsExist" . }} {{ template "consul.validateCloudSecretKeys" . }} # The deployment for running the Connect sidecar injector @@ -153,7 +153,7 @@ spec: {{- else }} -transparent-proxy-default-overwrite-probes=false \ {{- end }} - {{- if (and .Values.dns.enabled .Values.dns.enableRedirection) }} + {{- if (and $dnsEnabled $dnsRedirectionEnabled) }} -enable-consul-dns=true \ {{- end }} {{- if .Values.global.openshift.enabled }} diff --git a/charts/consul/templates/dns-service.yaml b/charts/consul/templates/dns-service.yaml index 0fc66595e1..5bb446bc19 100644 --- a/charts/consul/templates/dns-service.yaml +++ b/charts/consul/templates/dns-service.yaml @@ -1,4 +1,4 @@ -{{- if (or (and (ne (.Values.dns.enabled | toString) "-") .Values.dns.enabled) (and (eq (.Values.dns.enabled | toString) "-") .Values.global.enabled)) }} +{{- if (or (and (ne (.Values.dns.enabled | toString) "-") .Values.dns.enabled) (and (eq (.Values.dns.enabled | toString) "-") .Values.connectInject.transparentProxy.defaultEnabled)) }} # Service for Consul DNS. apiVersion: v1 kind: Service diff --git a/charts/consul/templates/server-acl-init-job.yaml b/charts/consul/templates/server-acl-init-job.yaml index 3a57f096ec..197a199602 100644 --- a/charts/consul/templates/server-acl-init-job.yaml +++ b/charts/consul/templates/server-acl-init-job.yaml @@ -174,7 +174,7 @@ spec: {{- if .Values.global.peering.enabled }} -enable-peering=true \ {{- end }} - {{- if (or (and (ne (.Values.dns.enabled | toString) "-") .Values.dns.enabled) (and (eq (.Values.dns.enabled | toString) "-") .Values.global.enabled)) }} + {{- if (or (and (ne (.Values.dns.enabled | toString) "-") .Values.dns.enabled) (and (eq (.Values.dns.enabled | toString) "-") .Values.connectInject.transparentProxy.defaultEnabled)) }} -allow-dns=true \ {{- end }} diff --git a/charts/consul/test/unit/connect-inject-deployment.bats b/charts/consul/test/unit/connect-inject-deployment.bats index 12b0289979..2450a76b67 100755 --- a/charts/consul/test/unit/connect-inject-deployment.bats +++ b/charts/consul/test/unit/connect-inject-deployment.bats @@ -428,14 +428,14 @@ load _helpers #-------------------------------------------------------------------- # DNS -@test "connectInject/Deployment: -enable-consul-dns unset by default" { +@test "connectInject/Deployment: -enable-consul-dns is set by default due to inheriting from connectInject.transparentProxy.defaultEnabled" { cd `chart_dir` local actual=$(helm template \ -s templates/connect-inject-deployment.yaml \ --set 'connectInject.enabled=true' \ . | tee /dev/stderr | yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-enable-consul-dns=true")' | tee /dev/stderr) - [ "${actual}" = "false" ] + [ "${actual}" = "true" ] } @test "connectInject/Deployment: -enable-consul-dns is true if dns.enabled=true and dns.enableRedirection=true" { @@ -444,11 +444,42 @@ load _helpers -s templates/connect-inject-deployment.yaml \ --set 'connectInject.enabled=true' \ --set 'dns.enableRedirection=true' \ + --set 'dns.enabled=true' \ . | tee /dev/stderr | yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-enable-consul-dns=true")' | tee /dev/stderr) [ "${actual}" = "true" ] } +@test "connectInject/Deployment: -enable-consul-dns is not set when connectInject.transparentProxy.defaultEnabled is false" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + --set 'connectInject.transparentProxy.defaultEnabled=false' \ + . | tee /dev/stderr | + yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-enable-consul-dns=true")' | tee /dev/stderr) + [ "${actual}" = "false" ] +} + +@test "connectInject/Deployment: -enable-consul-dns is not set if dns.enabled is false or ens.enableRedirection is false" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + --set 'dns.enabled=false' \ + . | tee /dev/stderr | + yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-enable-consul-dns=true")' | tee /dev/stderr) + [ "${actual}" = "false" ] + + local actual=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + --set 'dns.enableRedirection=false' \ + . | tee /dev/stderr | + yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-enable-consul-dns=true")' | tee /dev/stderr) + [ "${actual}" = "false" ] +} + @test "connectInject/Deployment: -resource-prefix always set" { cd `chart_dir` local actual=$(helm template \ diff --git a/charts/consul/test/unit/dns-service.bats b/charts/consul/test/unit/dns-service.bats index d5fea64d04..bc5777ac53 100755 --- a/charts/consul/test/unit/dns-service.bats +++ b/charts/consul/test/unit/dns-service.bats @@ -2,7 +2,7 @@ load _helpers -@test "dns/Service: enabled by default" { +@test "dns/Service: enabled by default due to inheriting from connectInject.transparentProxy.defaultEnabled" { cd `chart_dir` local actual=$(helm template \ -s templates/dns-service.yaml \ @@ -11,11 +11,11 @@ load _helpers [ "${actual}" = "true" ] } -@test "dns/Service: enable with global.enabled false" { +@test "dns/Service: enable with connectInject.transparentProxy.defaultEnabled false" { cd `chart_dir` local actual=$(helm template \ -s templates/dns-service.yaml \ - --set 'global.enabled=false' \ + --set 'connectInject.transparentProxy.defaultEnabled=false' \ --set 'dns.enabled=true' \ . | tee /dev/stderr | yq 'length > 0' | tee /dev/stderr) @@ -30,11 +30,11 @@ load _helpers . } -@test "dns/Service: disable with global.enabled" { +@test "dns/Service: disable with connectInject.transparentProxy.defaultEnabled false" { cd `chart_dir` assert_empty helm template \ -s templates/dns-service.yaml \ - --set 'global.enabled=false' \ + --set 'connectInject.transparentProxy.defaultEnabled=false' \ . } diff --git a/charts/consul/test/unit/server-acl-init-job.bats b/charts/consul/test/unit/server-acl-init-job.bats index 483564c64b..5886d8f23d 100644 --- a/charts/consul/test/unit/server-acl-init-job.bats +++ b/charts/consul/test/unit/server-acl-init-job.bats @@ -114,7 +114,7 @@ load _helpers #-------------------------------------------------------------------- # dns -@test "serverACLInit/Job: dns acl option enabled with .dns.enabled=-" { +@test "serverACLInit/Job: dns acl option enabled with .dns.enabled=- due to inheriting from connectInject.transparentProxy.defaultEnabled" { cd `chart_dir` local actual=$(helm template \ -s templates/server-acl-init-job.yaml \ @@ -124,6 +124,29 @@ load _helpers [ "${actual}" = "true" ] } +@test "serverACLInit/Job: dns acl option disabled with connectInject.transparentProxy.defaultEnabled=false" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/server-acl-init-job.yaml \ + --set 'global.acls.manageSystemACLs=true' \ + --set 'connectInject.transparentProxy.defaultEnabled=false' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command | any(contains("allow-dns"))' | tee /dev/stderr) + [ "${actual}" = "false" ] +} + +@test "serverACLInit/Job: dns acl option enabled with .dns.enabled=true and connectInject.transparentProxy.defaultEnabled=false" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/server-acl-init-job.yaml \ + --set 'global.acls.manageSystemACLs=true' \ + --set 'connectInject.transparentProxy.defaultEnabled=false' \ + --set 'dns.enabled=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command | any(contains("allow-dns"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + @test "serverACLInit/Job: dns acl option enabled with .dns.enabled=true" { cd `chart_dir` local actual=$(helm template \ diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 6435b46ccb..4e80a38af3 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -1480,7 +1480,7 @@ dns: # for default DNS resolution. The DNS lookups fall back to the nameserver IPs # listed in /etc/resolv.conf if not found in Consul. # @type: boolean - enableRedirection: false + enableRedirection: "-" # Used to control the type of service created. For # example, setting this to "LoadBalancer" will create an external load