Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Enable CRD controller to talk to Consul servers #1326

Merged
merged 1 commit into from
Jul 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 7 additions & 13 deletions acceptance/tests/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,13 @@ func TestController(t *testing.T) {
cfg := suite.Config()

cases := []struct {
secure bool
autoEncrypt bool
useVault bool
secure bool
useVault bool
}{
{false, false, false},
{true, false, false},
{true, true, false},
{true, true, true},
{false, false, true},
// Vault with TLS requires autoEncrypt set to true as well, so the below
// is not valid
// {true, false, true},
{false, false},
{true, false},
{false, true},
{true, true},
}

// The name of a service intention in consul is
Expand All @@ -50,15 +45,14 @@ func TestController(t *testing.T) {
const IntentionName = "svc1"

for _, c := range cases {
name := fmt.Sprintf("secure: %t; auto-encrypt: %t", c.secure, c.autoEncrypt)
name := fmt.Sprintf("secure: %t, vault: %t", c.secure, c.useVault)
t.Run(name, func(t *testing.T) {
ctx := suite.Environment().DefaultContext(t)

helmValues := map[string]string{
"controller.enabled": "true",
"connectInject.enabled": "true",
"global.tls.enabled": strconv.FormatBool(c.secure),
"global.tls.enableAutoEncrypt": strconv.FormatBool(c.autoEncrypt),
"global.acls.manageSystemACLs": strconv.FormatBool(c.secure),
}

Expand Down
7 changes: 2 additions & 5 deletions acceptance/tests/controller/main_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package controller

import (
"fmt"
"os"
"testing"

Expand All @@ -11,8 +10,6 @@ import (
var suite testSuite.Suite

func TestMain(m *testing.M) {
fmt.Println("Skipping controller tests because it's not supported with agentless yet")
os.Exit(0)
//suite = testSuite.NewSuite(m)
//os.Exit(suite.Run())
suite = testSuite.NewSuite(m)
os.Exit(suite.Run())
}
14 changes: 13 additions & 1 deletion charts/consul/templates/connect-inject-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ spec:
- mountPath: /consul/login
name: consul-data
readOnly: false
{{- if .Values.global.tls.enabled }}
{{- if and .Values.global.tls.enabled (not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots))}}
- name: consul-ca-cert
mountPath: /consul/tls/ca
readOnly: true
Expand All @@ -364,6 +364,18 @@ spec:
{{- if .Values.global.adminPartitions.enabled }}
-partition={{ .Values.global.adminPartitions.name }} \
{{- end }}
{{- if .Values.externalServers.enabled }}
{{- if .Values.global.tls.enabled }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[non-blocking] would would you think about putting this in a template since it seems to be repeated quite a bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah great question! I think I'll actually be able to get rid of this init container from all components altogether once agentless is done. My plan is to move login/logout logic into controllers to do at startup and shutdown. Other components that currently use this will switch to consul-dataplane instead doing the login for them.

-use-https \
{{- end }}
{{- range .Values.externalServers.hosts }}
-server-address={{ quote . }} \
{{- end }}
-server-port={{ .Values.externalServers.httpsPort }} \
{{- if .Values.externalServers.tlsServerName }}
-tls-server-name={{ .Values.externalServers.tlsServerName }} \
{{- end }}
{{- end }}
-consul-api-timeout={{ .Values.global.consulAPITimeout }} \
-log-level={{ default .Values.global.logLevel .Values.connectInject.logLevel }} \
-log-json={{ .Values.global.logJSON }}
Expand Down
93 changes: 52 additions & 41 deletions charts/consul/templates/controller-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,39 +65,33 @@ spec:
{{- end }}
{{- end }}
spec:
{{- if or .Values.global.acls.manageSystemACLs (and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt) }}
initContainers:
{{- if and .Values.global.tls.enabled .Values.global.tls.enableAutoEncrypt }}
{{- include "consul.getAutoEncryptClientCA" . | nindent 6 }}
{{- end }}
{{- if .Values.global.acls.manageSystemACLs }}
initContainers:
- name: controller-acl-init
env:
- name: HOST_IP
valueFrom:
fieldRef:
fieldPath: status.hostIP
{{- if .Values.global.tls.enabled }}
- name: CONSUL_CACERT
value: /consul/tls/ca/tls.crt
{{- end }}
{{- if not .Values.externalServers.enabled }}
- name: CONSUL_HTTP_ADDR
{{- if .Values.global.tls.enabled }}
value: https://$(HOST_IP):8501
value: https://{{ template "consul.fullname" . }}-server.{{ .Release.Namespace }}.svc:8501
{{- else }}
value: http://$(HOST_IP):8500
value: http://{{ template "consul.fullname" . }}-server.{{ .Release.Namespace }}.svc:8500
{{- end }}
{{- end }}
{{- if (and .Values.global.tls.enabled (not .Values.externalServers.useSystemRoots)) }}
- name: CONSUL_CACERT
{{- if .Values.global.secretsBackend.vault.enabled }}
value: "/vault/secrets/serverca.crt"
{{- else }}
value: "/consul/tls/ca/tls.crt"
{{- end }}
{{- end }}
image: {{ .Values.global.imageK8S }}
volumeMounts:
- mountPath: /consul/login
name: consul-data
readOnly: false
{{- if .Values.global.tls.enabled }}
{{- if .Values.global.tls.enableAutoEncrypt }}
- name: consul-auto-encrypt-ca-cert
{{- else }}
{{- if and .Values.global.tls.enabled (not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots))}}
- name: consul-ca-cert
{{- end }}
mountPath: /consul/tls/ca
readOnly: true
{{- end }}
Expand All @@ -116,6 +110,18 @@ spec:
{{- if .Values.global.adminPartitions.enabled }}
-partition={{ .Values.global.adminPartitions.name }} \
{{- end }}
{{- if .Values.externalServers.enabled }}
{{- if .Values.global.tls.enabled }}
-use-https \
{{- end }}
{{- range .Values.externalServers.hosts }}
-server-address={{ quote . }} \
{{- end }}
-server-port={{ .Values.externalServers.httpsPort }} \
{{- if .Values.externalServers.tlsServerName }}
-tls-server-name={{ .Values.externalServers.tlsServerName }} \
{{- end }}
{{- end }}
-consul-api-timeout={{ .Values.global.consulAPITimeout }} \
-log-level={{ default .Values.global.logLevel .Values.controller.logLevel }} \
-log-json={{ .Values.global.logJSON }}
Expand All @@ -127,7 +133,6 @@ spec:
memory: "25Mi"
cpu: "50m"
{{- end }}
{{- end }}
containers:
- command:
- "/bin/sh"
Expand All @@ -138,6 +143,19 @@ spec:
-log-level={{ default .Values.global.logLevel .Values.controller.logLevel }} \
-log-json={{ .Values.global.logJSON }} \
-resource-prefix={{ template "consul.fullname" . }} \
{{- if and .Values.externalServers.enabled (not .Values.externalServers.hosts) }}{{ fail "externalServers.hosts must be set if externalServers.enabled is true" }}{{ end -}}
{{- if .Values.externalServers.enabled }}
{{- if .Values.global.tls.enabled }}
-use-https \
{{- end }}
{{- range .Values.externalServers.hosts }}
-server-address={{ quote . }} \
{{- end }}
-server-port={{ .Values.externalServers.httpsPort }} \
{{- if .Values.externalServers.tlsServerName }}
-tls-server-name={{ .Values.externalServers.tlsServerName }} \
{{- end }}
{{- end }}
{{- if and .Values.global.secretsBackend.vault.enabled .Values.global.secretsBackend.vault.controller.tlsCert.secretName }}
-enable-webhook-ca-update \
-webhook-tls-cert-dir=/vault/secrets/controller-webhook/certs \
Expand Down Expand Up @@ -179,27 +197,29 @@ spec:
- name: CONSUL_HTTP_TOKEN_FILE
value: "/consul/login/acl-token"
{{- end }}
- name: HOST_IP
valueFrom:
fieldRef:
fieldPath: status.hostIP
{{- if (and .Values.controller.aclToken.secretName .Values.controller.aclToken.secretKey) }}
- name: CONSUL_HTTP_TOKEN
valueFrom:
secretKeyRef:
name: {{ .Values.controller.aclToken.secretName }}
key: {{ .Values.controller.aclToken.secretKey }}
{{- end }}
{{- if .Values.global.tls.enabled }}
- name: CONSUL_CACERT
value: /consul/tls/ca/tls.crt
{{- end }}
{{- if not .Values.externalServers.enabled }}
- name: CONSUL_HTTP_ADDR
{{- if .Values.global.tls.enabled }}
value: https://$(HOST_IP):8501
value: https://{{ template "consul.fullname" . }}-server.{{ .Release.Namespace }}.svc:8501
{{- else }}
value: http://$(HOST_IP):8500
value: http://{{ template "consul.fullname" . }}-server.{{ .Release.Namespace }}.svc:8500
{{- end }}
{{- end }}
{{- if (and .Values.global.tls.enabled (not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots))) }}
- name: CONSUL_CACERT
{{- if .Values.global.secretsBackend.vault.enabled }}
value: "/vault/secrets/serverca.crt"
{{- else }}
value: "/consul/tls/ca/tls.crt"
{{- end }}
{{- end }}
image: {{ .Values.global.imageK8S }}
name: controller
ports:
Expand All @@ -219,12 +239,8 @@ spec:
name: cert
readOnly: true
{{- end }}
{{- if .Values.global.tls.enabled }}
{{- if .Values.global.tls.enableAutoEncrypt }}
- name: consul-auto-encrypt-ca-cert
{{- else }}
{{- if and .Values.global.tls.enabled (not (and .Values.externalServers.enabled .Values.externalServers.useSystemRoots))}}
- name: consul-ca-cert
{{- end }}
mountPath: /consul/tls/ca
readOnly: true
{{- end }}
Expand All @@ -249,11 +265,6 @@ spec:
- key: {{ default "tls.crt" .Values.global.tls.caCert.secretKey }}
path: tls.crt
{{- end }}
{{- if .Values.global.tls.enableAutoEncrypt }}
- name: consul-auto-encrypt-ca-cert
emptyDir:
medium: "Memory"
{{- end }}
{{- end }}
- name: consul-data
emptyDir:
Expand Down
Loading