From 93e26ab8313e349798759c70a5eb5d3ab67bce52 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Tue, 2 Nov 2021 00:19:39 -0700 Subject: [PATCH 1/9] template kube dns service IP using api request --- charts/consul/templates/client-daemonset.yaml | 8 +++++++ .../templates/client-dns-clusterrole.yaml | 24 +++++++++++++++++++ .../client-dns-clusterrolebinding.yaml | 22 +++++++++++++++++ charts/consul/templates/client-role.yaml | 9 ++++++- .../consul/templates/server-statefulset.yaml | 10 ++++++++ 5 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 charts/consul/templates/client-dns-clusterrole.yaml create mode 100644 charts/consul/templates/client-dns-clusterrolebinding.yaml diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml index 7689b3ff8a..d00011da74 100644 --- a/charts/consul/templates/client-daemonset.yaml +++ b/charts/consul/templates/client-daemonset.yaml @@ -201,6 +201,11 @@ spec: - "/bin/sh" - "-ec" - | + {{- if .Values.dns.enabled }} + export kube_dns_service_ip=$( curl -s --cacert /var/run/secrets/kubernetes.io/serviceaccount/ca.crt \ + https://${KUBERNETES_SERVICE_HOST}:${KUBERNETES_SERVICE_PORT}/api/v1/namespaces/kube-system/services/kube-dns \ + -H "Authorization: Bearer $( cat /var/run/secrets/kubernetes.io/serviceaccount/token )" | jq -r .spec.clusterIP ) + {{- end }} CONSUL_FULLNAME="{{template "consul.fullname" . }}" {{ template "consul.extraconfig" }} @@ -276,6 +281,9 @@ spec: {{- range $value := .Values.global.recursors }} -recursor={{ quote $value }} \ {{- end }} + {{- if .Values.dns.enabled }} + -recursor="$kube_dns_service_ip" \ + {{- end }} -config-file=/consul/extra-config/extra-from-values.json \ -domain={{ .Values.global.domain }} volumeMounts: diff --git a/charts/consul/templates/client-dns-clusterrole.yaml b/charts/consul/templates/client-dns-clusterrole.yaml new file mode 100644 index 0000000000..a5613d69ff --- /dev/null +++ b/charts/consul/templates/client-dns-clusterrole.yaml @@ -0,0 +1,24 @@ +{{- if (or (and (ne (.Values.client.enabled | toString) "-") .Values.client.enabled) (and (eq (.Values.client.enabled | toString) "-") .Values.global.enabled)) }} +{{- if .Values.dns.enabled }} +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: {{ template "consul.fullname" . }}-client-dns +# namespace: {{ .Release.Namespace }} + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} +rules: +{{- if .Values.dns.enabled }} + - apiGroups: [""] + resources: + - services + resourceNames: + - "kube-dns" + verbs: + - get +{{- end }} +{{- end }} +{{- end }} diff --git a/charts/consul/templates/client-dns-clusterrolebinding.yaml b/charts/consul/templates/client-dns-clusterrolebinding.yaml new file mode 100644 index 0000000000..4c01635210 --- /dev/null +++ b/charts/consul/templates/client-dns-clusterrolebinding.yaml @@ -0,0 +1,22 @@ +{{- if (or (and (ne (.Values.client.enabled | toString) "-") .Values.client.enabled) (and (eq (.Values.client.enabled | toString) "-") .Values.global.enabled)) }} +{{- if .Values.dns.enabled }} +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: {{ template "consul.fullname" . }}-client-dns + namespace: {{ .Release.Namespace }} + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: {{ template "consul.fullname" . }}-client-dns +subjects: + - kind: ServiceAccount + name: {{ template "consul.fullname" . }}-client + namespace: {{ .Release.Namespace }} # TODO:nitya try regular rolebinding with this line +{{- end }} +{{- end }} diff --git a/charts/consul/templates/client-role.yaml b/charts/consul/templates/client-role.yaml index 7f05b82e6b..bc402718f1 100644 --- a/charts/consul/templates/client-role.yaml +++ b/charts/consul/templates/client-role.yaml @@ -10,7 +10,7 @@ metadata: heritage: {{ .Release.Service }} release: {{ .Release.Name }} component: client -{{- if (or .Values.global.acls.manageSystemACLs .Values.global.enablePodSecurityPolicies .Values.global.openshift.enabled) }} +{{- if (or .Values.dns.enabled .Values.global.acls.manageSystemACLs .Values.global.enablePodSecurityPolicies .Values.global.openshift.enabled) }} rules: {{- if .Values.global.enablePodSecurityPolicies }} - apiGroups: ["policy"] @@ -37,6 +37,13 @@ rules: verbs: - use {{- end}} +{{- if .Values.dns.enabled }} + - apiGroups: [""] + resources: + - services + verbs: + - get +{{- end }} {{- else}} rules: [] {{- end }} diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml index 2e332cb7ae..05c0303973 100644 --- a/charts/consul/templates/server-statefulset.yaml +++ b/charts/consul/templates/server-statefulset.yaml @@ -192,6 +192,13 @@ spec: - | CONSUL_FULLNAME="{{template "consul.fullname" . }}" + {{- if .Values.dns.enabled }} + export kube_dns_service_ip=$( curl -s --cacert /var/run/secrets/kubernetes.io/serviceaccount/ca.crt \ + https://${KUBERNETES_SERVICE_HOST}:${KUBERNETES_SERVICE_PORT}/api/v1/namespaces/kube-system/services/kube-dns \ + -H "Authorization: Bearer $( cat /var/run/secrets/kubernetes.io/serviceaccount/token )" | jq -r .spec.clusterIP ) + {{- end }} + + {{ template "consul.extraconfig" }} exec /usr/local/bin/docker-entrypoint.sh consul agent \ @@ -254,6 +261,9 @@ spec: {{- range $value := .Values.global.recursors }} -recursor={{ quote $value }} \ {{- end }} + {{- if .Values.dns.enabled }} + -recursor="$kube_dns_service_ip" \ + {{- end }} -config-file=/consul/extra-config/extra-from-values.json \ -server volumeMounts: From 8962ecea4545802107c739a50d9a4f87dc3ad7fb Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Wed, 3 Nov 2021 00:23:06 -0700 Subject: [PATCH 2/9] when dns is enabled, dns queries are directed to consul. Tested with this branch and consul branch `dns-redirect-to-consul`. --- charts/consul/templates/client-daemonset.yaml | 11 +++++++---- .../consul/templates/connect-inject-deployment.yaml | 10 ++++++++++ charts/consul/templates/server-statefulset.yaml | 9 +++++---- control-plane/connect-inject/container_init.go | 7 +++++++ control-plane/connect-inject/handler.go | 5 +++++ control-plane/subcommand/inject-connect/command.go | 4 ++++ 6 files changed, 38 insertions(+), 8 deletions(-) diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml index d00011da74..eea5a6943b 100644 --- a/charts/consul/templates/client-daemonset.yaml +++ b/charts/consul/templates/client-daemonset.yaml @@ -201,12 +201,15 @@ spec: - "/bin/sh" - "-ec" - | + CONSUL_FULLNAME="{{template "consul.fullname" . }}" + {{- if .Values.dns.enabled }} - export kube_dns_service_ip=$( curl -s --cacert /var/run/secrets/kubernetes.io/serviceaccount/ca.crt \ - https://${KUBERNETES_SERVICE_HOST}:${KUBERNETES_SERVICE_PORT}/api/v1/namespaces/kube-system/services/kube-dns \ - -H "Authorization: Bearer $( cat /var/run/secrets/kubernetes.io/serviceaccount/token )" | jq -r .spec.clusterIP ) + export kube_dns_service_ip=$(cat /etc/resolv.conf | grep nameserver | cut -d' ' -f2) + echo $kube_dns_service_ip > /consul/kubednsip + if [[ -z $kube_dns_service_ip ]]; then + echo "Unable to get kube-dns Service IP" + fi {{- end }} - CONSUL_FULLNAME="{{template "consul.fullname" . }}" {{ template "consul.extraconfig" }} diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml index d89635d059..2656fb8dde 100644 --- a/charts/consul/templates/connect-inject-deployment.yaml +++ b/charts/consul/templates/connect-inject-deployment.yaml @@ -85,6 +85,13 @@ spec: - | CONSUL_FULLNAME="{{template "consul.fullname" . }}" + {{- if .Values.dns.enabled }} + consul_fullname_with_underscore=$(echo $CONSUL_FULLNAME | tr '-' '_') + dns_var_name=${consul_fullname_with_underscore}_dns_service_host + dns_var_name_in_caps=$(echo $dns_var_name | tr 'a-z' 'A-Z') + export dns_ip=$(eval echo \$${dns_var_name_in_caps}) + {{- end }} + consul-k8s-control-plane inject-connect \ -log-level={{ default .Values.global.logLevel .Values.connectInject.logLevel }} \ -log-json={{ .Values.global.logJSON }} \ @@ -108,6 +115,9 @@ spec: {{- else }} -transparent-proxy-default-overwrite-probes=false \ {{- end }} + {{- if .Values.dns.enabled }} + -consul-dns-ip=$dns_ip \ + {{- end }} {{- if .Values.global.openshift.enabled }} -enable-openshift \ {{- end }} diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml index 05c0303973..0386f63bba 100644 --- a/charts/consul/templates/server-statefulset.yaml +++ b/charts/consul/templates/server-statefulset.yaml @@ -193,12 +193,13 @@ spec: CONSUL_FULLNAME="{{template "consul.fullname" . }}" {{- if .Values.dns.enabled }} - export kube_dns_service_ip=$( curl -s --cacert /var/run/secrets/kubernetes.io/serviceaccount/ca.crt \ - https://${KUBERNETES_SERVICE_HOST}:${KUBERNETES_SERVICE_PORT}/api/v1/namespaces/kube-system/services/kube-dns \ - -H "Authorization: Bearer $( cat /var/run/secrets/kubernetes.io/serviceaccount/token )" | jq -r .spec.clusterIP ) + export kube_dns_service_ip=$(cat /etc/resolv.conf | grep nameserver | cut -d' ' -f2) + echo $kube_dns_service_ip > /consul/kubednsip + if [[ -z $kube_dns_service_ip ]]; then + echo "Unable to get kube-dns Service IP" + fi {{- end }} - {{ template "consul.extraconfig" }} exec /usr/local/bin/docker-entrypoint.sh consul agent \ diff --git a/control-plane/connect-inject/container_init.go b/control-plane/connect-inject/container_init.go index e69a91fbb1..d1880b21ff 100644 --- a/control-plane/connect-inject/container_init.go +++ b/control-plane/connect-inject/container_init.go @@ -66,6 +66,9 @@ type initContainerCommandData struct { // TProxyExcludeUIDs is a list of additional user IDs to exclude from traffic redirection via // the consul connect redirect-traffic command. TProxyExcludeUIDs []string + + // ConsulDNSIP is the IP of the Consul DNS Service. + ConsulDNSIP string } // initCopyContainer returns the init container spec for the copy container which places @@ -118,6 +121,7 @@ func (h *Handler) containerInit(namespace corev1.Namespace, pod corev1.Pod) (cor TProxyExcludeOutboundPorts: splitCommaSeparatedItemsFromAnnotation(annotationTProxyExcludeOutboundPorts, pod), TProxyExcludeOutboundCIDRs: splitCommaSeparatedItemsFromAnnotation(annotationTProxyExcludeOutboundCIDRs, pod), TProxyExcludeUIDs: splitCommaSeparatedItemsFromAnnotation(annotationTProxyExcludeUIDs, pod), + ConsulDNSIP: h.ConsulDNSIP, EnvoyUID: envoyUserAndGroupID, } @@ -331,6 +335,9 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD {{- if .ConsulNamespace }} -namespace="{{ .ConsulNamespace }}" \ {{- end }} + {{- if .ConsulDNSIP }} + -consul-dns-ip="{{ .ConsulDNSIP }}" \ + {{- end }} {{- range .TProxyExcludeInboundPorts }} -exclude-inbound-port="{{ . }}" \ {{- end }} diff --git a/control-plane/connect-inject/handler.go b/control-plane/connect-inject/handler.go index aad0c4c53f..ec52918dad 100644 --- a/control-plane/connect-inject/handler.go +++ b/control-plane/connect-inject/handler.go @@ -134,6 +134,11 @@ type Handler struct { // to point them to the Envoy proxy. TProxyOverwriteProbes bool + // ConsulDNSIP is the IP of the Consul DNS service on Kubernetes. When this values is not empty, + // it should be passed to the redirect traffic command so DNS requests are directed to Consul from + // mesh services. + ConsulDNSIP string + // EnableOpenShift indicates that when tproxy is enabled, the security context for the Envoy and init // containers should not be added because OpenShift sets a random user for those and will not allow // those containers to be created otherwise. diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index e93e07a48e..b92fc41517 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -91,6 +91,7 @@ type Command struct { // Transparent proxy flags. flagDefaultEnableTransparentProxy bool flagTransparentProxyDefaultOverwriteProbes bool + flagConsulDNSIP string flagEnableOpenShift bool @@ -161,6 +162,8 @@ func (c *Command) init() { "Enable transparent proxy mode for all Consul service mesh applications by default.") c.flagSet.BoolVar(&c.flagTransparentProxyDefaultOverwriteProbes, "transparent-proxy-default-overwrite-probes", true, "Overwrite Kubernetes probes to point to Envoy by default when in Transparent Proxy mode.") + c.flagSet.StringVar(&c.flagConsulDNSIP, "consul-dns-ip", "", + "ClusterIP of the Consul DNS service.") c.flagSet.BoolVar(&c.flagEnableOpenShift, "enable-openshift", false, "Indicates that the command runs in an OpenShift cluster.") c.flagSet.StringVar(&c.flagLogLevel, "log-level", zapcore.InfoLevel.String(), @@ -471,6 +474,7 @@ func (c *Command) Run(args []string) int { CrossNamespaceACLPolicy: c.flagCrossNamespaceACLPolicy, EnableTransparentProxy: c.flagDefaultEnableTransparentProxy, TProxyOverwriteProbes: c.flagTransparentProxyDefaultOverwriteProbes, + ConsulDNSIP: c.flagConsulDNSIP, EnableOpenShift: c.flagEnableOpenShift, Log: ctrl.Log.WithName("handler").WithName("connect"), LogLevel: c.flagLogLevel, From 7b669286d3876e4dd67dab6922245003a5a0d4c2 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Wed, 3 Nov 2021 00:26:37 -0700 Subject: [PATCH 3/9] adding demo.yaml for testing. remove before merging --- demo.yaml | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 demo.yaml diff --git a/demo.yaml b/demo.yaml new file mode 100644 index 0000000000..338345012e --- /dev/null +++ b/demo.yaml @@ -0,0 +1,136 @@ +# Service to expose frontend +apiVersion: v1 +kind: Service +metadata: + name: frontend +spec: + selector: + app: frontend + ports: + - name: http + protocol: TCP + port: 9090 + targetPort: 9090 +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: frontend +--- +# frontend +apiVersion: apps/v1 +kind: Deployment +metadata: + name: frontend + labels: + app: frontend +spec: + replicas: 1 + selector: + matchLabels: + app: frontend + template: + metadata: + labels: + app: frontend + annotations: + "consul.hashicorp.com/connect-inject": "true" + "consul.hashicorp.com/transparent-proxy-exclude-outbound-cidrs": "151.101.2.133,151.101.66.133,151.101.130.133,151.101.194.133" + spec: + serviceAccountName: frontend + containers: + - name: frontend + image: nicholasjackson/fake-service:v0.22.4 + securityContext: + capabilities: + add: ["NET_ADMIN"] + ports: + - containerPort: 9090 + env: + - name: "LISTEN_ADDR" + value: "0.0.0.0:9090" + - name: "UPSTREAM_URIS" + value: "http://backend" + - name: "NAME" + value: "frontend" + - name: "MESSAGE" + value: "Hello World" + - name: "HTTP_CLIENT_KEEP_ALIVES" + value: "false" +--- +apiVersion: consul.hashicorp.com/v1alpha1 +kind: ServiceDefaults +metadata: + name: frontend +spec: + protocol: "http" +--- +# Service to expose backend +apiVersion: v1 +kind: Service +metadata: + name: backend +spec: + selector: + app: backend + ports: + - name: http + protocol: TCP + port: 80 + targetPort: 9090 +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: backend +--- +# deployment for backend +apiVersion: apps/v1 +kind: Deployment +metadata: + name: backend + labels: + app: backend +spec: + replicas: 1 + selector: + matchLabels: + app: backend + template: + metadata: + labels: + app: backend + annotations: + "consul.hashicorp.com/connect-inject": "true" + spec: + serviceAccountName: backend + containers: + - name: backend + image: nicholasjackson/fake-service:v0.22.4 + ports: + - containerPort: 9090 + env: + - name: "LISTEN_ADDR" + value: "0.0.0.0:9090" + - name: "NAME" + value: "backend" + - name: "MESSAGE" + value: "Response from backend" +--- +apiVersion: consul.hashicorp.com/v1alpha1 +kind: ServiceDefaults +metadata: + name: backend +spec: + protocol: "http" +--- +apiVersion: consul.hashicorp.com/v1alpha1 +kind: ServiceIntentions +metadata: + name: backend +spec: + destination: + name: backend + sources: + - name: frontend + action: allow From b372c9d99ec4071c42f4c3ab52e5ab1e87fd2d09 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Wed, 3 Nov 2021 00:45:37 -0700 Subject: [PATCH 4/9] remove client clusterrole/rolbinding since they're not currently used --- .../templates/client-dns-clusterrole.yaml | 24 ------------------- .../client-dns-clusterrolebinding.yaml | 22 ----------------- 2 files changed, 46 deletions(-) delete mode 100644 charts/consul/templates/client-dns-clusterrole.yaml delete mode 100644 charts/consul/templates/client-dns-clusterrolebinding.yaml diff --git a/charts/consul/templates/client-dns-clusterrole.yaml b/charts/consul/templates/client-dns-clusterrole.yaml deleted file mode 100644 index a5613d69ff..0000000000 --- a/charts/consul/templates/client-dns-clusterrole.yaml +++ /dev/null @@ -1,24 +0,0 @@ -{{- if (or (and (ne (.Values.client.enabled | toString) "-") .Values.client.enabled) (and (eq (.Values.client.enabled | toString) "-") .Values.global.enabled)) }} -{{- if .Values.dns.enabled }} -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: {{ template "consul.fullname" . }}-client-dns -# namespace: {{ .Release.Namespace }} - labels: - app: {{ template "consul.name" . }} - chart: {{ template "consul.chart" . }} - heritage: {{ .Release.Service }} - release: {{ .Release.Name }} -rules: -{{- if .Values.dns.enabled }} - - apiGroups: [""] - resources: - - services - resourceNames: - - "kube-dns" - verbs: - - get -{{- end }} -{{- end }} -{{- end }} diff --git a/charts/consul/templates/client-dns-clusterrolebinding.yaml b/charts/consul/templates/client-dns-clusterrolebinding.yaml deleted file mode 100644 index 4c01635210..0000000000 --- a/charts/consul/templates/client-dns-clusterrolebinding.yaml +++ /dev/null @@ -1,22 +0,0 @@ -{{- if (or (and (ne (.Values.client.enabled | toString) "-") .Values.client.enabled) (and (eq (.Values.client.enabled | toString) "-") .Values.global.enabled)) }} -{{- if .Values.dns.enabled }} -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: {{ template "consul.fullname" . }}-client-dns - namespace: {{ .Release.Namespace }} - labels: - app: {{ template "consul.name" . }} - chart: {{ template "consul.chart" . }} - heritage: {{ .Release.Service }} - release: {{ .Release.Name }} -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: {{ template "consul.fullname" . }}-client-dns -subjects: - - kind: ServiceAccount - name: {{ template "consul.fullname" . }}-client - namespace: {{ .Release.Namespace }} # TODO:nitya try regular rolebinding with this line -{{- end }} -{{- end }} From 13bf1205bb5c4cc957c8c6892db29b0dd62aeb33 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Mon, 8 Nov 2021 15:50:34 -0500 Subject: [PATCH 5/9] Add unit tests for the changes made. --- charts/consul/templates/client-daemonset.yaml | 5 +- .../templates/connect-inject-deployment.yaml | 11 +-- .../consul/templates/server-statefulset.yaml | 5 +- charts/consul/test/unit/client-daemonset.bats | 22 +++++ .../test/unit/connect-inject-deployment.bats | 24 ++++++ .../consul/test/unit/server-statefulset.bats | 22 +++++ charts/consul/values.yaml | 3 + control-plane/connect-inject/annotations.go | 6 ++ .../connect-inject/container_init.go | 35 +++++++- .../connect-inject/container_init_test.go | 83 +++++++++++++++++++ control-plane/connect-inject/handler.go | 7 +- .../subcommand/inject-connect/command.go | 10 ++- 12 files changed, 209 insertions(+), 24 deletions(-) diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml index eea5a6943b..5a1d191c5d 100644 --- a/charts/consul/templates/client-daemonset.yaml +++ b/charts/consul/templates/client-daemonset.yaml @@ -203,9 +203,8 @@ spec: - | CONSUL_FULLNAME="{{template "consul.fullname" . }}" - {{- if .Values.dns.enabled }} + {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} export kube_dns_service_ip=$(cat /etc/resolv.conf | grep nameserver | cut -d' ' -f2) - echo $kube_dns_service_ip > /consul/kubednsip if [[ -z $kube_dns_service_ip ]]; then echo "Unable to get kube-dns Service IP" fi @@ -284,7 +283,7 @@ spec: {{- range $value := .Values.global.recursors }} -recursor={{ quote $value }} \ {{- end }} - {{- if .Values.dns.enabled }} + {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} -recursor="$kube_dns_service_ip" \ {{- end }} -config-file=/consul/extra-config/extra-from-values.json \ diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml index 2656fb8dde..e571e85e13 100644 --- a/charts/consul/templates/connect-inject-deployment.yaml +++ b/charts/consul/templates/connect-inject-deployment.yaml @@ -85,13 +85,6 @@ spec: - | CONSUL_FULLNAME="{{template "consul.fullname" . }}" - {{- if .Values.dns.enabled }} - consul_fullname_with_underscore=$(echo $CONSUL_FULLNAME | tr '-' '_') - dns_var_name=${consul_fullname_with_underscore}_dns_service_host - dns_var_name_in_caps=$(echo $dns_var_name | tr 'a-z' 'A-Z') - export dns_ip=$(eval echo \$${dns_var_name_in_caps}) - {{- end }} - consul-k8s-control-plane inject-connect \ -log-level={{ default .Values.global.logLevel .Values.connectInject.logLevel }} \ -log-json={{ .Values.global.logJSON }} \ @@ -115,8 +108,8 @@ spec: {{- else }} -transparent-proxy-default-overwrite-probes=false \ {{- end }} - {{- if .Values.dns.enabled }} - -consul-dns-ip=$dns_ip \ + {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} + -enable-consul-dns=true \ {{- end }} {{- if .Values.global.openshift.enabled }} -enable-openshift \ diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml index 0386f63bba..0eb7353968 100644 --- a/charts/consul/templates/server-statefulset.yaml +++ b/charts/consul/templates/server-statefulset.yaml @@ -192,9 +192,8 @@ spec: - | CONSUL_FULLNAME="{{template "consul.fullname" . }}" - {{- if .Values.dns.enabled }} + {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} export kube_dns_service_ip=$(cat /etc/resolv.conf | grep nameserver | cut -d' ' -f2) - echo $kube_dns_service_ip > /consul/kubednsip if [[ -z $kube_dns_service_ip ]]; then echo "Unable to get kube-dns Service IP" fi @@ -262,7 +261,7 @@ spec: {{- range $value := .Values.global.recursors }} -recursor={{ quote $value }} \ {{- end }} - {{- if .Values.dns.enabled }} + {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} -recursor="$kube_dns_service_ip" \ {{- end }} -config-file=/consul/extra-config/extra-from-values.json \ diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats index e986c2984a..3065b1dbab 100755 --- a/charts/consul/test/unit/client-daemonset.bats +++ b/charts/consul/test/unit/client-daemonset.bats @@ -1170,6 +1170,28 @@ load _helpers [ "${actual}" = "true" ] } +#-------------------------------------------------------------------- +# DNS + +@test "client/DaemonSet: recursor IP is not set by default" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/client-daemonset.yaml \ + . | tee /dev/stderr | + yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-recursor=\"$kube_dns_service_ip\"")' | tee /dev/stderr) + [ "${actual}" = "false" ] +} + +@test "client/DaemonSet: recursor IP set to kubeDNS IP if dns.enableRedirection" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/client-daemonset.yaml \ + --set 'dns.enableRedirection=true' \ + . | tee /dev/stderr | + yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-recursor=\"$kube_dns_service_ip\"")' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + #-------------------------------------------------------------------- # hostNetwork diff --git a/charts/consul/test/unit/connect-inject-deployment.bats b/charts/consul/test/unit/connect-inject-deployment.bats index a8e23d0c53..969a338b38 100755 --- a/charts/consul/test/unit/connect-inject-deployment.bats +++ b/charts/consul/test/unit/connect-inject-deployment.bats @@ -519,6 +519,30 @@ EOF [ "${actual}" = "true" ] } +#-------------------------------------------------------------------- +# DNS + +@test "connectInject/Deployment: -enable-consul-dns unset default" { + 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" ] +} + +@test "connectInject/Deployment: -enable-consul-dns is false if dns.enabled=false is disabled" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/connect-inject-deployment.yaml \ + --set 'connectInject.enabled=true' \ + --set 'dns.enableRedirection=true' \ + . | tee /dev/stderr | + yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-enable-consul-dns=true")' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + #-------------------------------------------------------------------- # global.tls.enabled diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats index 647ad2994d..2ed882f4c1 100755 --- a/charts/consul/test/unit/server-statefulset.bats +++ b/charts/consul/test/unit/server-statefulset.bats @@ -580,6 +580,28 @@ load _helpers [ "${actualBaz}" = "qux" ] } +#-------------------------------------------------------------------- +# DNS + +@test "server/StatefulSet: recursor IP is unset by default" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/server-statefulset.yaml \ + . | tee /dev/stderr | + yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-recursor=\"$kube_dns_service_ip\"")' | tee /dev/stderr) + [ "${actual}" = "false" ] +} + +@test "server/StatefulSet: recursor IP set to kubeDNS IP if dns.enableRedirection is true" { + cd `chart_dir` + local actual=$(helm template \ + -s templates/server-statefulset.yaml \ + --set 'dns.enableRedirection=true' \ + . | tee /dev/stderr | + yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-recursor=\"$kube_dns_service_ip\"")' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + #-------------------------------------------------------------------- # annotations diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 68ffbb6630..7bf05d77b4 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -1122,6 +1122,9 @@ dns: # @type: boolean enabled: "-" + # @type: boolean + enableRedirection: false + # Used to control the type of service created. For # example, setting this to "LoadBalancer" will create an external load # balancer (for supported K8S installations) diff --git a/control-plane/connect-inject/annotations.go b/control-plane/connect-inject/annotations.go index 929c9e4c69..ccc9ab6341 100644 --- a/control-plane/connect-inject/annotations.go +++ b/control-plane/connect-inject/annotations.go @@ -90,6 +90,12 @@ const ( // annotationConsulNamespace is the Consul namespace the service is registered into. annotationConsulNamespace = "consul.hashicorp.com/consul-namespace" + // keyConsulDNS enables or disables Consul DNS for a given pod. It can also be set as a label + // on a namespace to define the default behaviour for connect-injected pods which do not otherwise override this setting + // with their own annotation. + // This annotation/label takes a boolean value (true/false). + keyConsulDNS = "consul.hashicorp.com/consul-dns" + // keyTransparentProxy enables or disables transparent proxy for a given pod. It can also be set as a label // on a namespace to define the default behaviour for connect-injected pods which do not otherwise override this setting // with their own annotation. diff --git a/control-plane/connect-inject/container_init.go b/control-plane/connect-inject/container_init.go index d1880b21ff..7f05aed621 100644 --- a/control-plane/connect-inject/container_init.go +++ b/control-plane/connect-inject/container_init.go @@ -2,6 +2,7 @@ package connectinject import ( "bytes" + "os" "strconv" "strings" "text/template" @@ -16,6 +17,7 @@ const ( envoyUserAndGroupID = 5995 copyContainerUserAndGroupID = 5996 netAdminCapability = "NET_ADMIN" + dnsServiceHostEnvSuffix = "DNS_SERVICE_HOST" ) type initContainerCommandData struct { @@ -110,6 +112,21 @@ func (h *Handler) containerInit(namespace corev1.Namespace, pod corev1.Pod) (cor return corev1.Container{}, err } + dnsEnabled, err := consulDNSEnabled(namespace, pod, h.EnableConsulDNS) + if err != nil { + return corev1.Container{}, err + } + + var consulDNSIP string + if dnsEnabled { + for _, e := range os.Environ() { + if strings.Contains(e, dnsServiceHostEnvSuffix) { + dnsServiceHostEnv := strings.SplitN(e, "=", 2) + consulDNSIP = dnsServiceHostEnv[1] + } + } + } + data := initContainerCommandData{ AuthMethod: h.AuthMethod, ConsulPartition: h.ConsulPartition, @@ -121,7 +138,7 @@ func (h *Handler) containerInit(namespace corev1.Namespace, pod corev1.Pod) (cor TProxyExcludeOutboundPorts: splitCommaSeparatedItemsFromAnnotation(annotationTProxyExcludeOutboundPorts, pod), TProxyExcludeOutboundCIDRs: splitCommaSeparatedItemsFromAnnotation(annotationTProxyExcludeOutboundCIDRs, pod), TProxyExcludeUIDs: splitCommaSeparatedItemsFromAnnotation(annotationTProxyExcludeUIDs, pod), - ConsulDNSIP: h.ConsulDNSIP, + ConsulDNSIP: consulDNSIP, EnvoyUID: envoyUserAndGroupID, } @@ -243,6 +260,22 @@ func transparentProxyEnabled(namespace corev1.Namespace, pod corev1.Pod, globalE return globalEnabled, nil } +// consulDNSEnabled returns true if Consul DNS should be enabled for this pod. +// It returns an error when the annotation value cannot be parsed by strconv.ParseBool or if we are unable +// to read the pod's namespace label when it exists. +func consulDNSEnabled(namespace corev1.Namespace, pod corev1.Pod, globalEnabled bool) (bool, error) { + // First check to see if the pod annotation exists to override the namespace or global settings. + if raw, ok := pod.Annotations[keyConsulDNS]; ok { + return strconv.ParseBool(raw) + } + // Next see if the namespace has been defaulted. + if raw, ok := namespace.Labels[keyConsulDNS]; ok { + return strconv.ParseBool(raw) + } + // Else fall back to the global default. + return globalEnabled, nil +} + // pointerToInt64 takes an int64 and returns a pointer to it. func pointerToInt64(i int64) *int64 { return &i diff --git a/control-plane/connect-inject/container_init_test.go b/control-plane/connect-inject/container_init_test.go index e131ba20f7..d33800433e 100644 --- a/control-plane/connect-inject/container_init_test.go +++ b/control-plane/connect-inject/container_init_test.go @@ -2,6 +2,7 @@ package connectinject import ( "fmt" + "os" "strings" "testing" @@ -310,6 +311,88 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) { } } +func TestHandlerContainerInit_consulDNS(t *testing.T) { + cases := map[string]struct { + globalEnabled bool + annotations map[string]string + expectedContainsCmd string + namespaceLabel map[string]string + }{ + "enabled globally, ns not set, annotation not provided": { + globalEnabled: true, + expectedContainsCmd: `/consul/connect-inject/consul connect redirect-traffic \ + -consul-dns-ip="10.0.34.16" \ + -proxy-id="$(cat /consul/connect-inject/proxyid)" \ + -proxy-uid=5995`, + }, + "enabled globally, ns not set, annotation is false": { + globalEnabled: true, + annotations: map[string]string{keyConsulDNS: "false"}, + expectedContainsCmd: `/consul/connect-inject/consul connect redirect-traffic \ + -proxy-id="$(cat /consul/connect-inject/proxyid)" \ + -proxy-uid=5995`, + }, + "enabled globally, ns not set, annotation is true": { + globalEnabled: true, + annotations: map[string]string{keyConsulDNS: "true"}, + expectedContainsCmd: `/consul/connect-inject/consul connect redirect-traffic \ + -consul-dns-ip="10.0.34.16" \ + -proxy-id="$(cat /consul/connect-inject/proxyid)" \ + -proxy-uid=5995`, + }, + "disabled globally, ns not set, annotation not provided": { + expectedContainsCmd: `/consul/connect-inject/consul connect redirect-traffic \ + -proxy-id="$(cat /consul/connect-inject/proxyid)" \ + -proxy-uid=5995`, + }, + "disabled globally, ns not set, annotation is false": { + annotations: map[string]string{keyConsulDNS: "false"}, + expectedContainsCmd: `/consul/connect-inject/consul connect redirect-traffic \ + -proxy-id="$(cat /consul/connect-inject/proxyid)" \ + -proxy-uid=5995`, + }, + "disabled globally, ns not set, annotation is true": { + annotations: map[string]string{keyConsulDNS: "true"}, + expectedContainsCmd: `/consul/connect-inject/consul connect redirect-traffic \ + -consul-dns-ip="10.0.34.16" \ + -proxy-id="$(cat /consul/connect-inject/proxyid)" \ + -proxy-uid=5995`, + }, + "disabled globally, ns enabled, annotation not set": { + expectedContainsCmd: `/consul/connect-inject/consul connect redirect-traffic \ + -consul-dns-ip="10.0.34.16" \ + -proxy-id="$(cat /consul/connect-inject/proxyid)" \ + -proxy-uid=5995`, + namespaceLabel: map[string]string{keyConsulDNS: "true"}, + }, + "enabled globally, ns disabled, annotation not set": { + globalEnabled: true, + expectedContainsCmd: `/consul/connect-inject/consul connect redirect-traffic \ + -proxy-id="$(cat /consul/connect-inject/proxyid)" \ + -proxy-uid=5995`, + namespaceLabel: map[string]string{keyConsulDNS: "false"}, + }, + } + for name, c := range cases { + t.Run(name, func(t *testing.T) { + h := Handler{EnableConsulDNS: c.globalEnabled, EnableTransparentProxy: true} + os.Setenv(dnsServiceHostEnvSuffix, "10.0.34.16") + defer os.Unsetenv(dnsServiceHostEnvSuffix) + + pod := minimal() + pod.Annotations = c.annotations + + ns := testNS + ns.Labels = c.namespaceLabel + container, err := h.containerInit(ns, *pod) + require.NoError(t, err) + actualCmd := strings.Join(container.Command, " ") + + require.Contains(t, actualCmd, c.expectedContainsCmd) + }) + } +} + func TestHandlerContainerInit_namespacesAndPartitionsEnabled(t *testing.T) { minimal := func() *corev1.Pod { return &corev1.Pod{ diff --git a/control-plane/connect-inject/handler.go b/control-plane/connect-inject/handler.go index ec52918dad..f943dfadb8 100644 --- a/control-plane/connect-inject/handler.go +++ b/control-plane/connect-inject/handler.go @@ -134,10 +134,9 @@ type Handler struct { // to point them to the Envoy proxy. TProxyOverwriteProbes bool - // ConsulDNSIP is the IP of the Consul DNS service on Kubernetes. When this values is not empty, - // it should be passed to the redirect traffic command so DNS requests are directed to Consul from - // mesh services. - ConsulDNSIP string + // EnableConsulDNS enables traffic redirection so that DNS requests are directed to Consul + // from mesh services. + EnableConsulDNS bool // EnableOpenShift indicates that when tproxy is enabled, the security context for the Envoy and init // containers should not be added because OpenShift sets a random user for those and will not allow diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index b92fc41517..9798a4e49e 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -91,7 +91,9 @@ type Command struct { // Transparent proxy flags. flagDefaultEnableTransparentProxy bool flagTransparentProxyDefaultOverwriteProbes bool - flagConsulDNSIP string + + // Consul DNS flags. + flagEnableConsulDNS bool flagEnableOpenShift bool @@ -162,8 +164,8 @@ func (c *Command) init() { "Enable transparent proxy mode for all Consul service mesh applications by default.") c.flagSet.BoolVar(&c.flagTransparentProxyDefaultOverwriteProbes, "transparent-proxy-default-overwrite-probes", true, "Overwrite Kubernetes probes to point to Envoy by default when in Transparent Proxy mode.") - c.flagSet.StringVar(&c.flagConsulDNSIP, "consul-dns-ip", "", - "ClusterIP of the Consul DNS service.") + c.flagSet.BoolVar(&c.flagEnableConsulDNS, "enable-consul-dns", false, + "Enables Consul DNS lookup for services in the mesh.") c.flagSet.BoolVar(&c.flagEnableOpenShift, "enable-openshift", false, "Indicates that the command runs in an OpenShift cluster.") c.flagSet.StringVar(&c.flagLogLevel, "log-level", zapcore.InfoLevel.String(), @@ -474,7 +476,7 @@ func (c *Command) Run(args []string) int { CrossNamespaceACLPolicy: c.flagCrossNamespaceACLPolicy, EnableTransparentProxy: c.flagDefaultEnableTransparentProxy, TProxyOverwriteProbes: c.flagTransparentProxyDefaultOverwriteProbes, - ConsulDNSIP: c.flagConsulDNSIP, + EnableConsulDNS: c.flagEnableConsulDNS, EnableOpenShift: c.flagEnableOpenShift, Log: ctrl.Log.WithName("handler").WithName("connect"), LogLevel: c.flagLogLevel, From 44abc9b4aa6013ea90978b5734b9eafa96c9e302 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Tue, 9 Nov 2021 15:47:42 -0500 Subject: [PATCH 6/9] Add feedback from Iryna's review --- charts/consul/templates/_helpers.tpl | 12 ++++++++++++ charts/consul/templates/client-daemonset.yaml | 7 ++----- charts/consul/templates/server-statefulset.yaml | 7 ++----- charts/consul/test/unit/client-daemonset.bats | 8 ++++---- .../consul/test/unit/connect-inject-deployment.bats | 2 +- charts/consul/test/unit/server-statefulset.bats | 8 ++++---- charts/consul/values.yaml | 3 +++ 7 files changed, 28 insertions(+), 19 deletions(-) diff --git a/charts/consul/templates/_helpers.tpl b/charts/consul/templates/_helpers.tpl index 49676dc329..27fa26a230 100644 --- a/charts/consul/templates/_helpers.tpl +++ b/charts/consul/templates/_helpers.tpl @@ -28,6 +28,18 @@ is passed to consul as a -config-file param on command line. [ -n "${HOSTNAME}" ] && sed -Ei "s|HOSTNAME|${HOSTNAME?}|g" /consul/extra-config/extra-from-values.json {{- end -}} +{{/* +Sets up a list of recusor flags for Consul agents by iterating over the IPs of every nameserver +in /etc/resolv.conf and concatenating them into a string of arguments that can be passed directly +to the consul agent command. +*/}} +{{- define "consul.recursors" -}} + recursor_flags="" + for ip in $(cat /etc/resolv.conf | grep nameserver | cut -d' ' -f2) + do + recursor_flags=$recursor_flags" -recursor=$ip" + done +{{- end -}} {{/* Create chart name and version as used by the chart label. diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml index 5a1d191c5d..e19f841f9c 100644 --- a/charts/consul/templates/client-daemonset.yaml +++ b/charts/consul/templates/client-daemonset.yaml @@ -204,10 +204,7 @@ spec: CONSUL_FULLNAME="{{template "consul.fullname" . }}" {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} - export kube_dns_service_ip=$(cat /etc/resolv.conf | grep nameserver | cut -d' ' -f2) - if [[ -z $kube_dns_service_ip ]]; then - echo "Unable to get kube-dns Service IP" - fi + {{ template "consul.recursors" }} {{- end }} {{ template "consul.extraconfig" }} @@ -284,7 +281,7 @@ spec: -recursor={{ quote $value }} \ {{- end }} {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} - -recursor="$kube_dns_service_ip" \ + $recursor_flags \ {{- end }} -config-file=/consul/extra-config/extra-from-values.json \ -domain={{ .Values.global.domain }} diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml index 0eb7353968..af49c66770 100644 --- a/charts/consul/templates/server-statefulset.yaml +++ b/charts/consul/templates/server-statefulset.yaml @@ -193,10 +193,7 @@ spec: CONSUL_FULLNAME="{{template "consul.fullname" . }}" {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} - export kube_dns_service_ip=$(cat /etc/resolv.conf | grep nameserver | cut -d' ' -f2) - if [[ -z $kube_dns_service_ip ]]; then - echo "Unable to get kube-dns Service IP" - fi + {{ template "consul.recursors" }} {{- end }} {{ template "consul.extraconfig" }} @@ -262,7 +259,7 @@ spec: -recursor={{ quote $value }} \ {{- end }} {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} - -recursor="$kube_dns_service_ip" \ + $recursor_flags \ {{- end }} -config-file=/consul/extra-config/extra-from-values.json \ -server diff --git a/charts/consul/test/unit/client-daemonset.bats b/charts/consul/test/unit/client-daemonset.bats index 3065b1dbab..a6f8a63b1a 100755 --- a/charts/consul/test/unit/client-daemonset.bats +++ b/charts/consul/test/unit/client-daemonset.bats @@ -1173,22 +1173,22 @@ load _helpers #-------------------------------------------------------------------- # DNS -@test "client/DaemonSet: recursor IP is not set by default" { +@test "client/DaemonSet: recursor flags is not set by default" { cd `chart_dir` local actual=$(helm template \ -s templates/client-daemonset.yaml \ . | tee /dev/stderr | - yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-recursor=\"$kube_dns_service_ip\"")' | tee /dev/stderr) + yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("$recursor_flags")' | tee /dev/stderr) [ "${actual}" = "false" ] } -@test "client/DaemonSet: recursor IP set to kubeDNS IP if dns.enableRedirection" { +@test "client/DaemonSet: add recursor flags if dns.enableRedirection is true" { cd `chart_dir` local actual=$(helm template \ -s templates/client-daemonset.yaml \ --set 'dns.enableRedirection=true' \ . | tee /dev/stderr | - yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-recursor=\"$kube_dns_service_ip\"")' | tee /dev/stderr) + yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("$recursor_flags")' | tee /dev/stderr) [ "${actual}" = "true" ] } diff --git a/charts/consul/test/unit/connect-inject-deployment.bats b/charts/consul/test/unit/connect-inject-deployment.bats index 969a338b38..6cdd6fd52b 100755 --- a/charts/consul/test/unit/connect-inject-deployment.bats +++ b/charts/consul/test/unit/connect-inject-deployment.bats @@ -532,7 +532,7 @@ EOF [ "${actual}" = "false" ] } -@test "connectInject/Deployment: -enable-consul-dns is false if dns.enabled=false is disabled" { +@test "connectInject/Deployment: -enable-consul-dns is true if dns.enabled=true and dns.enableRedirection=true" { cd `chart_dir` local actual=$(helm template \ -s templates/connect-inject-deployment.yaml \ diff --git a/charts/consul/test/unit/server-statefulset.bats b/charts/consul/test/unit/server-statefulset.bats index 2ed882f4c1..a7a31bb84d 100755 --- a/charts/consul/test/unit/server-statefulset.bats +++ b/charts/consul/test/unit/server-statefulset.bats @@ -583,22 +583,22 @@ load _helpers #-------------------------------------------------------------------- # DNS -@test "server/StatefulSet: recursor IP is unset by default" { +@test "server/StatefulSet: recursor flags unset by default" { cd `chart_dir` local actual=$(helm template \ -s templates/server-statefulset.yaml \ . | tee /dev/stderr | - yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-recursor=\"$kube_dns_service_ip\"")' | tee /dev/stderr) + yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("$recursor_flags")' | tee /dev/stderr) [ "${actual}" = "false" ] } -@test "server/StatefulSet: recursor IP set to kubeDNS IP if dns.enableRedirection is true" { +@test "server/StatefulSet: add recursor flags if dns.enableRedirection is true" { cd `chart_dir` local actual=$(helm template \ -s templates/server-statefulset.yaml \ --set 'dns.enableRedirection=true' \ . | tee /dev/stderr | - yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("-recursor=\"$kube_dns_service_ip\"")' | tee /dev/stderr) + yq -c -r '.spec.template.spec.containers[0].command | join(" ") | contains("$recursor_flags")' | tee /dev/stderr) [ "${actual}" = "true" ] } diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 7bf05d77b4..6723cf7121 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -1122,6 +1122,9 @@ dns: # @type: boolean enabled: "-" + # Used to determine if services using Consul Connect use Consul 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 From 75db8e97ffe80d63def61a7c30ff2a72618027be Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Tue, 9 Nov 2021 19:58:46 -0500 Subject: [PATCH 7/9] Add suggestions from Luke. --- charts/consul/templates/_helpers.tpl | 2 +- charts/consul/templates/client-daemonset.yaml | 4 +- .../templates/connect-inject-deployment.yaml | 5 +- .../consul/templates/server-statefulset.yaml | 4 +- .../test/unit/connect-inject-deployment.bats | 12 +- charts/consul/values.yaml | 2 +- .../connect-inject/container_init.go | 33 +++-- .../connect-inject/container_init_test.go | 33 ++++- control-plane/connect-inject/handler.go | 4 + .../subcommand/inject-connect/command.go | 4 + demo.yaml | 136 ------------------ 11 files changed, 79 insertions(+), 160 deletions(-) delete mode 100644 demo.yaml diff --git a/charts/consul/templates/_helpers.tpl b/charts/consul/templates/_helpers.tpl index 27fa26a230..480c4b8895 100644 --- a/charts/consul/templates/_helpers.tpl +++ b/charts/consul/templates/_helpers.tpl @@ -37,7 +37,7 @@ to the consul agent command. recursor_flags="" for ip in $(cat /etc/resolv.conf | grep nameserver | cut -d' ' -f2) do - recursor_flags=$recursor_flags" -recursor=$ip" + recursor_flags="$recursor_flags -recursor=$ip" done {{- end -}} diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml index e19f841f9c..28c9512ce6 100644 --- a/charts/consul/templates/client-daemonset.yaml +++ b/charts/consul/templates/client-daemonset.yaml @@ -203,7 +203,7 @@ spec: - | CONSUL_FULLNAME="{{template "consul.fullname" . }}" - {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} + {{- if (and .Values.dns.enabled .Values.dns.enableRedirection) }} {{ template "consul.recursors" }} {{- end }} @@ -280,7 +280,7 @@ spec: {{- range $value := .Values.global.recursors }} -recursor={{ quote $value }} \ {{- end }} - {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} + {{- if (and .Values.dns.enabled .Values.dns.enableRedirection) }} $recursor_flags \ {{- end }} -config-file=/consul/extra-config/extra-from-values.json \ diff --git a/charts/consul/templates/connect-inject-deployment.yaml b/charts/consul/templates/connect-inject-deployment.yaml index e571e85e13..11946b6235 100644 --- a/charts/consul/templates/connect-inject-deployment.yaml +++ b/charts/consul/templates/connect-inject-deployment.yaml @@ -83,8 +83,6 @@ spec: - "/bin/sh" - "-ec" - | - CONSUL_FULLNAME="{{template "consul.fullname" . }}" - consul-k8s-control-plane inject-connect \ -log-level={{ default .Values.global.logLevel .Values.connectInject.logLevel }} \ -log-json={{ .Values.global.logJSON }} \ @@ -108,7 +106,8 @@ spec: {{- else }} -transparent-proxy-default-overwrite-probes=false \ {{- end }} - {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} + -resource-prefix={{ template "consul.fullname" . }} \ + {{- if (and .Values.dns.enabled .Values.dns.enableRedirection) }} -enable-consul-dns=true \ {{- end }} {{- if .Values.global.openshift.enabled }} diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml index af49c66770..5e8cc89e76 100644 --- a/charts/consul/templates/server-statefulset.yaml +++ b/charts/consul/templates/server-statefulset.yaml @@ -192,7 +192,7 @@ spec: - | CONSUL_FULLNAME="{{template "consul.fullname" . }}" - {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} + {{- if (and .Values.dns.enabled .Values.dns.enableRedirection) }} {{ template "consul.recursors" }} {{- end }} @@ -258,7 +258,7 @@ spec: {{- range $value := .Values.global.recursors }} -recursor={{ quote $value }} \ {{- end }} - {{- if (and .Values.dns.enabled .Values.dns.enableRedirection)}} + {{- if (and .Values.dns.enabled .Values.dns.enableRedirection) }} $recursor_flags \ {{- end }} -config-file=/consul/extra-config/extra-from-values.json \ diff --git a/charts/consul/test/unit/connect-inject-deployment.bats b/charts/consul/test/unit/connect-inject-deployment.bats index 6cdd6fd52b..1fb0dad743 100755 --- a/charts/consul/test/unit/connect-inject-deployment.bats +++ b/charts/consul/test/unit/connect-inject-deployment.bats @@ -522,7 +522,7 @@ EOF #-------------------------------------------------------------------- # DNS -@test "connectInject/Deployment: -enable-consul-dns unset default" { +@test "connectInject/Deployment: -enable-consul-dns unset by default" { cd `chart_dir` local actual=$(helm template \ -s templates/connect-inject-deployment.yaml \ @@ -543,6 +543,16 @@ EOF [ "${actual}" = "true" ] } +@test "connectInject/Deployment: -resource-prefix always set" { + 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("-resource-prefix=RELEASE-NAME-consul")' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + #-------------------------------------------------------------------- # global.tls.enabled diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml index 6723cf7121..9d5af13ac8 100644 --- a/charts/consul/values.yaml +++ b/charts/consul/values.yaml @@ -1122,7 +1122,7 @@ dns: # @type: boolean enabled: "-" - # Used to determine if services using Consul Connect use Consul DNS + # If true, services using Consul Connect will use Consul 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 diff --git a/control-plane/connect-inject/container_init.go b/control-plane/connect-inject/container_init.go index 7f05aed621..46f91f3286 100644 --- a/control-plane/connect-inject/container_init.go +++ b/control-plane/connect-inject/container_init.go @@ -2,6 +2,7 @@ package connectinject import ( "bytes" + "errors" "os" "strconv" "strings" @@ -69,8 +70,8 @@ type initContainerCommandData struct { // the consul connect redirect-traffic command. TProxyExcludeUIDs []string - // ConsulDNSIP is the IP of the Consul DNS Service. - ConsulDNSIP string + // ConsulDNSClusterIP is the IP of the Consul DNS Service. + ConsulDNSClusterIP string } // initCopyContainer returns the init container spec for the copy container which places @@ -117,13 +118,14 @@ func (h *Handler) containerInit(namespace corev1.Namespace, pod corev1.Pod) (cor return corev1.Container{}, err } - var consulDNSIP string + var consulDNSClusterIP string if dnsEnabled { - for _, e := range os.Environ() { - if strings.Contains(e, dnsServiceHostEnvSuffix) { - dnsServiceHostEnv := strings.SplitN(e, "=", 2) - consulDNSIP = dnsServiceHostEnv[1] - } + // If Consul DNS is enabled, we find the environment variable that has the value + // of the ClusterIP of the Consul DNS Service. constructDNSServiceHostName returns + // the name of the env variable whose value is the ClusterIP of the Consul DNS Service. + consulDNSClusterIP = os.Getenv(h.constructDNSServiceHostName()) + if consulDNSClusterIP == "" { + return corev1.Container{}, errors.New("failed to find ClusterIP for Consul DNS Service Host") } } @@ -138,7 +140,7 @@ func (h *Handler) containerInit(namespace corev1.Namespace, pod corev1.Pod) (cor TProxyExcludeOutboundPorts: splitCommaSeparatedItemsFromAnnotation(annotationTProxyExcludeOutboundPorts, pod), TProxyExcludeOutboundCIDRs: splitCommaSeparatedItemsFromAnnotation(annotationTProxyExcludeOutboundCIDRs, pod), TProxyExcludeUIDs: splitCommaSeparatedItemsFromAnnotation(annotationTProxyExcludeUIDs, pod), - ConsulDNSIP: consulDNSIP, + ConsulDNSClusterIP: consulDNSClusterIP, EnvoyUID: envoyUserAndGroupID, } @@ -244,6 +246,15 @@ func (h *Handler) containerInit(namespace corev1.Namespace, pod corev1.Pod) (cor return container, nil } +// constructDNSServiceHostName use the resource prefix and the DNS Service hostname suffix to construct the +// key of the env variable whose value is the cluster IP of the Consul DNS Service. +// It translates "resource-prefix" into "RESOURCE_PREFIX_DNS_SERVICE_HOST". +func (h *Handler) constructDNSServiceHostName() string { + upcaseResourcePrefix := strings.ToUpper(h.ResourcePrefix) + upcaseResourcePrefixWithUnderscores := strings.ReplaceAll(upcaseResourcePrefix, "-", "_") + return strings.Join([]string{upcaseResourcePrefixWithUnderscores, dnsServiceHostEnvSuffix}, "_") +} + // transparentProxyEnabled returns true if transparent proxy should be enabled for this pod. // It returns an error when the annotation value cannot be parsed by strconv.ParseBool or if we are unable // to read the pod's namespace label when it exists. @@ -368,8 +379,8 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD {{- if .ConsulNamespace }} -namespace="{{ .ConsulNamespace }}" \ {{- end }} - {{- if .ConsulDNSIP }} - -consul-dns-ip="{{ .ConsulDNSIP }}" \ + {{- if .ConsulDNSClusterIP }} + -consul-dns-ip="{{ .ConsulDNSClusterIP }}" \ {{- end }} {{- range .TProxyExcludeInboundPorts }} -exclude-inbound-port="{{ . }}" \ diff --git a/control-plane/connect-inject/container_init_test.go b/control-plane/connect-inject/container_init_test.go index d33800433e..58cfe95d73 100644 --- a/control-plane/connect-inject/container_init_test.go +++ b/control-plane/connect-inject/container_init_test.go @@ -375,9 +375,9 @@ func TestHandlerContainerInit_consulDNS(t *testing.T) { } for name, c := range cases { t.Run(name, func(t *testing.T) { - h := Handler{EnableConsulDNS: c.globalEnabled, EnableTransparentProxy: true} - os.Setenv(dnsServiceHostEnvSuffix, "10.0.34.16") - defer os.Unsetenv(dnsServiceHostEnvSuffix) + h := Handler{EnableConsulDNS: c.globalEnabled, EnableTransparentProxy: true, ResourcePrefix: "consul-consul"} + os.Setenv("CONSUL_CONSUL_DNS_SERVICE_HOST", "10.0.34.16") + defer os.Unsetenv("CONSUL_CONSUL_DNS_SERVICE_HOST") pod := minimal() pod.Annotations = c.annotations @@ -393,6 +393,33 @@ func TestHandlerContainerInit_consulDNS(t *testing.T) { } } +func TestHandler_constructDNSServiceHostName(t *testing.T) { + cases := []struct { + prefix string + result string + }{ + { + prefix: "consul-consul", + result: "CONSUL_CONSUL_DNS_SERVICE_HOST", + }, + { + prefix: "release", + result: "RELEASE_DNS_SERVICE_HOST", + }, + { + prefix: "consul-dc1", + result: "CONSUL_DC1_DNS_SERVICE_HOST", + }, + } + + for _, c := range cases { + t.Run(c.prefix, func(t *testing.T) { + h := Handler{ResourcePrefix: c.prefix} + require.Equal(t, c.result, h.constructDNSServiceHostName()) + }) + } +} + func TestHandlerContainerInit_namespacesAndPartitionsEnabled(t *testing.T) { minimal := func() *corev1.Pod { return &corev1.Pod{ diff --git a/control-plane/connect-inject/handler.go b/control-plane/connect-inject/handler.go index f943dfadb8..da2b4f681d 100644 --- a/control-plane/connect-inject/handler.go +++ b/control-plane/connect-inject/handler.go @@ -138,6 +138,10 @@ type Handler struct { // from mesh services. EnableConsulDNS bool + // ResourcePrefix is the prefix used for the installation which is used to determine the Service + // name of the Consul DNS service. + ResourcePrefix string + // EnableOpenShift indicates that when tproxy is enabled, the security context for the Envoy and init // containers should not be added because OpenShift sets a random user for those and will not allow // those containers to be created otherwise. diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index 9798a4e49e..abebf69b81 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -94,6 +94,7 @@ type Command struct { // Consul DNS flags. flagEnableConsulDNS bool + flagResourcePrefix string flagEnableOpenShift bool @@ -166,6 +167,8 @@ func (c *Command) init() { "Overwrite Kubernetes probes to point to Envoy by default when in Transparent Proxy mode.") c.flagSet.BoolVar(&c.flagEnableConsulDNS, "enable-consul-dns", false, "Enables Consul DNS lookup for services in the mesh.") + c.flagSet.StringVar(&c.flagResourcePrefix, "resource-prefix", "", + "Release prefix of the Consul installation used to determine Consul DNS Service name.") c.flagSet.BoolVar(&c.flagEnableOpenShift, "enable-openshift", false, "Indicates that the command runs in an OpenShift cluster.") c.flagSet.StringVar(&c.flagLogLevel, "log-level", zapcore.InfoLevel.String(), @@ -477,6 +480,7 @@ func (c *Command) Run(args []string) int { EnableTransparentProxy: c.flagDefaultEnableTransparentProxy, TProxyOverwriteProbes: c.flagTransparentProxyDefaultOverwriteProbes, EnableConsulDNS: c.flagEnableConsulDNS, + ResourcePrefix: c.flagResourcePrefix, EnableOpenShift: c.flagEnableOpenShift, Log: ctrl.Log.WithName("handler").WithName("connect"), LogLevel: c.flagLogLevel, diff --git a/demo.yaml b/demo.yaml deleted file mode 100644 index 338345012e..0000000000 --- a/demo.yaml +++ /dev/null @@ -1,136 +0,0 @@ -# Service to expose frontend -apiVersion: v1 -kind: Service -metadata: - name: frontend -spec: - selector: - app: frontend - ports: - - name: http - protocol: TCP - port: 9090 - targetPort: 9090 ---- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: frontend ---- -# frontend -apiVersion: apps/v1 -kind: Deployment -metadata: - name: frontend - labels: - app: frontend -spec: - replicas: 1 - selector: - matchLabels: - app: frontend - template: - metadata: - labels: - app: frontend - annotations: - "consul.hashicorp.com/connect-inject": "true" - "consul.hashicorp.com/transparent-proxy-exclude-outbound-cidrs": "151.101.2.133,151.101.66.133,151.101.130.133,151.101.194.133" - spec: - serviceAccountName: frontend - containers: - - name: frontend - image: nicholasjackson/fake-service:v0.22.4 - securityContext: - capabilities: - add: ["NET_ADMIN"] - ports: - - containerPort: 9090 - env: - - name: "LISTEN_ADDR" - value: "0.0.0.0:9090" - - name: "UPSTREAM_URIS" - value: "http://backend" - - name: "NAME" - value: "frontend" - - name: "MESSAGE" - value: "Hello World" - - name: "HTTP_CLIENT_KEEP_ALIVES" - value: "false" ---- -apiVersion: consul.hashicorp.com/v1alpha1 -kind: ServiceDefaults -metadata: - name: frontend -spec: - protocol: "http" ---- -# Service to expose backend -apiVersion: v1 -kind: Service -metadata: - name: backend -spec: - selector: - app: backend - ports: - - name: http - protocol: TCP - port: 80 - targetPort: 9090 ---- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: backend ---- -# deployment for backend -apiVersion: apps/v1 -kind: Deployment -metadata: - name: backend - labels: - app: backend -spec: - replicas: 1 - selector: - matchLabels: - app: backend - template: - metadata: - labels: - app: backend - annotations: - "consul.hashicorp.com/connect-inject": "true" - spec: - serviceAccountName: backend - containers: - - name: backend - image: nicholasjackson/fake-service:v0.22.4 - ports: - - containerPort: 9090 - env: - - name: "LISTEN_ADDR" - value: "0.0.0.0:9090" - - name: "NAME" - value: "backend" - - name: "MESSAGE" - value: "Response from backend" ---- -apiVersion: consul.hashicorp.com/v1alpha1 -kind: ServiceDefaults -metadata: - name: backend -spec: - protocol: "http" ---- -apiVersion: consul.hashicorp.com/v1alpha1 -kind: ServiceIntentions -metadata: - name: backend -spec: - destination: - name: backend - sources: - - name: frontend - action: allow From 9d16c5a6823f61ab751569a84e7af7101ead348f Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Wed, 10 Nov 2021 13:40:15 -0500 Subject: [PATCH 8/9] Update control-plane/connect-inject/container_init.go Co-authored-by: Luke Kysow <1034429+lkysow@users.noreply.github.com> --- control-plane/connect-inject/container_init.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/control-plane/connect-inject/container_init.go b/control-plane/connect-inject/container_init.go index 46f91f3286..18831fa57b 100644 --- a/control-plane/connect-inject/container_init.go +++ b/control-plane/connect-inject/container_init.go @@ -2,7 +2,7 @@ package connectinject import ( "bytes" - "errors" + "fmt" "os" "strconv" "strings" @@ -125,7 +125,7 @@ func (h *Handler) containerInit(namespace corev1.Namespace, pod corev1.Pod) (cor // the name of the env variable whose value is the ClusterIP of the Consul DNS Service. consulDNSClusterIP = os.Getenv(h.constructDNSServiceHostName()) if consulDNSClusterIP == "" { - return corev1.Container{}, errors.New("failed to find ClusterIP for Consul DNS Service Host") + return corev1.Container{}, fmt.Errorf("environment variable %s is not found", h.constructDNSServiceHostName()) } } From 521fb7950afe728b555043b893fbd3d533abb810 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Wed, 10 Nov 2021 14:25:08 -0500 Subject: [PATCH 9/9] Add CHANGELOG --- CHANGELOG.md | 7 +++++++ charts/consul/templates/client-role.yaml | 9 +-------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f4d60e80a..cec3581f34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ ## UNRELEASED +FEATURES: +* Helm Chart + * Add support for Consul services to utilize Consul DNS for service discovery. Set `dns.enableRedirection` to allow services to + use Consul DNS via the Consul DNS Service. [[GH-833](https://github.com/hashicorp/consul-k8s/pull/833)] +* Control Plane + * Connect: Allow services using Connect to utilize Consul DNS to perform service discovery. [[GH-833](https://github.com/hashicorp/consul-k8s/pull/833)] + BREAKING CHANGES: * Previously [UI metrics](https://www.consul.io/docs/connect/observability/ui-visualization) would be enabled when `global.metrics=false` and `ui.metrics.enabled=-`. If you are no longer seeing UI metrics, diff --git a/charts/consul/templates/client-role.yaml b/charts/consul/templates/client-role.yaml index bc402718f1..7f05b82e6b 100644 --- a/charts/consul/templates/client-role.yaml +++ b/charts/consul/templates/client-role.yaml @@ -10,7 +10,7 @@ metadata: heritage: {{ .Release.Service }} release: {{ .Release.Name }} component: client -{{- if (or .Values.dns.enabled .Values.global.acls.manageSystemACLs .Values.global.enablePodSecurityPolicies .Values.global.openshift.enabled) }} +{{- if (or .Values.global.acls.manageSystemACLs .Values.global.enablePodSecurityPolicies .Values.global.openshift.enabled) }} rules: {{- if .Values.global.enablePodSecurityPolicies }} - apiGroups: ["policy"] @@ -37,13 +37,6 @@ rules: verbs: - use {{- end}} -{{- if .Values.dns.enabled }} - - apiGroups: [""] - resources: - - services - verbs: - - get -{{- end }} {{- else}} rules: [] {{- end }}