From 86f5c37ecb1c90893b1acae500215239ca442515 Mon Sep 17 00:00:00 2001 From: Eric Date: Thu, 27 Apr 2023 09:37:08 -0400 Subject: [PATCH] set consul server locality from k8s node labels --- .changelog/2093.txt | 3 + charts/consul/templates/_helpers.tpl | 4 +- charts/consul/templates/client-daemonset.yaml | 1 + .../consul/templates/server-clusterrole.yaml | 16 ++ .../templates/server-clusterrolebinding.yaml | 18 ++ .../consul/templates/server-statefulset.yaml | 27 ++- control-plane/commands.go | 4 + .../subcommand/fetch-server-region/command.go | 158 ++++++++++++++++++ .../fetch-server-region/command_test.go | 108 ++++++++++++ 9 files changed, 332 insertions(+), 7 deletions(-) create mode 100644 .changelog/2093.txt create mode 100644 charts/consul/templates/server-clusterrole.yaml create mode 100644 charts/consul/templates/server-clusterrolebinding.yaml create mode 100644 control-plane/subcommand/fetch-server-region/command.go create mode 100644 control-plane/subcommand/fetch-server-region/command_test.go diff --git a/.changelog/2093.txt b/.changelog/2093.txt new file mode 100644 index 0000000000..20c657e566 --- /dev/null +++ b/.changelog/2093.txt @@ -0,0 +1,3 @@ +```release-note:improvement +control-plane: set agent localities on Consul servers to the server node's `topology.kubernetes.io/region` label. +``` diff --git a/charts/consul/templates/_helpers.tpl b/charts/consul/templates/_helpers.tpl index 3552c8c209..b1feb0dbd6 100644 --- a/charts/consul/templates/_helpers.tpl +++ b/charts/consul/templates/_helpers.tpl @@ -335,7 +335,7 @@ Consul server environment variables for consul-k8s commands. {{- end }} {{- if and .Values.externalServers.enabled .Values.externalServers.skipServerWatch }} - name: CONSUL_SKIP_SERVER_WATCH - value: "true" + value: "true" {{- end }} {{- end -}} @@ -366,7 +366,7 @@ Usage: {{ template "consul.validateCloudSecretKeys" . }} */}} {{- define "consul.validateCloudSecretKeys" -}} -{{- if and .Values.global.cloud.enabled }} +{{- if and .Values.global.cloud.enabled }} {{- if or (and .Values.global.cloud.resourceId.secretName (not .Values.global.cloud.resourceId.secretKey)) (and .Values.global.cloud.resourceId.secretKey (not .Values.global.cloud.resourceId.secretName)) }} {{fail "When either global.cloud.resourceId.secretName or global.cloud.resourceId.secretKey is defined, both must be set."}} {{- end }} diff --git a/charts/consul/templates/client-daemonset.yaml b/charts/consul/templates/client-daemonset.yaml index 09a70b394e..1125036fcb 100644 --- a/charts/consul/templates/client-daemonset.yaml +++ b/charts/consul/templates/client-daemonset.yaml @@ -387,6 +387,7 @@ spec: -recursor={{ quote $value }} \ {{- end }} -config-file=/consul/extra-config/extra-from-values.json \ + -config-file=/consul/extra-config/locality.json \ -domain={{ .Values.global.domain }} volumeMounts: - name: data diff --git a/charts/consul/templates/server-clusterrole.yaml b/charts/consul/templates/server-clusterrole.yaml new file mode 100644 index 0000000000..c22f562264 --- /dev/null +++ b/charts/consul/templates/server-clusterrole.yaml @@ -0,0 +1,16 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: {{ template "consul.fullname" . }}-server + namespace: {{ .Release.Namespace }} + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: server +rules: +- apiGroups: [""] + resources: ["nodes"] + verbs: + - get diff --git a/charts/consul/templates/server-clusterrolebinding.yaml b/charts/consul/templates/server-clusterrolebinding.yaml new file mode 100644 index 0000000000..854fda870e --- /dev/null +++ b/charts/consul/templates/server-clusterrolebinding.yaml @@ -0,0 +1,18 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: {{ template "consul.fullname" . }}-server + labels: + app: {{ template "consul.name" . }} + chart: {{ template "consul.chart" . }} + heritage: {{ .Release.Service }} + release: {{ .Release.Name }} + component: server +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: {{ template "consul.fullname" . }}-server +subjects: +- kind: ServiceAccount + name: {{ template "consul.fullname" . }}-server + namespace: {{ .Release.Namespace }} diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml index 8b73306fd7..aa9198f127 100644 --- a/charts/consul/templates/server-statefulset.yaml +++ b/charts/consul/templates/server-statefulset.yaml @@ -217,6 +217,22 @@ spec: {{- if .Values.server.priorityClassName }} priorityClassName: {{ .Values.server.priorityClassName | quote }} {{- end }} + initContainers: + - name: locality-init + image: {{ .Values.global.imageK8S }} + env: + - name: NODE_NAME + valueFrom: + fieldRef: + fieldPath: spec.nodeName + command: + - "/bin/sh" + - "-ec" + - | + consul-k8s-control-plane fetch-server-region -node-name "$NODE_NAME" -output-file /consul/extra-config/locality.json + volumeMounts: + - name: extra-config + mountPath: /consul/extra-config containers: - name: consul image: "{{ default .Values.global.image .Values.server.image }}" @@ -291,9 +307,9 @@ spec: {{- end }} {{- if .Values.global.cloud.enabled}} # These are mounted as secrets so that the consul server agent can use them. - # - the hcp-go-sdk in consul agent will already look for HCP_CLIENT_ID, HCP_CLIENT_SECRET, HCP_AUTH_URL, + # - the hcp-go-sdk in consul agent will already look for HCP_CLIENT_ID, HCP_CLIENT_SECRET, HCP_AUTH_URL, # HCP_SCADA_ADDRESS, and HCP_API_HOST. so nothing more needs to be done. - # - HCP_RESOURCE_ID is created for use in the + # - HCP_RESOURCE_ID is created for use in the # `-hcl="cloud { resource_id = \"${HCP_RESOURCE_ID}\" }"` logic in the command below. {{- if .Values.global.cloud.clientId.secretName }} - name: HCP_CLIENT_ID @@ -328,7 +344,7 @@ spec: valueFrom: secretKeyRef: name: {{ .Values.global.cloud.apiHost.secretName }} - key: {{ .Values.global.cloud.apiHost.secretKey }} + key: {{ .Values.global.cloud.apiHost.secretKey }} {{- end}} {{- if .Values.global.cloud.scadaAddress.secretName }} - name: HCP_SCADA_ADDRESS @@ -336,7 +352,7 @@ spec: secretKeyRef: name: {{ .Values.global.cloud.scadaAddress.secretName }} key: {{ .Values.global.cloud.scadaAddress.secretKey }} - {{- end}} + {{- end}} {{- end }} {{- include "consul.extraEnvironmentVars" .Values.server | nindent 12 }} command: @@ -375,7 +391,8 @@ spec: -config-dir=/consul/userconfig/{{ .name }} \ {{- end }} {{- end }} - -config-file=/consul/extra-config/extra-from-values.json + -config-file=/consul/extra-config/extra-from-values.json \ + -config-file=/consul/extra-config/locality.json {{- if and .Values.global.cloud.enabled .Values.global.cloud.resourceId.secretName }} -hcl="cloud { resource_id = \"${HCP_RESOURCE_ID}\" }" {{- end }} diff --git a/control-plane/commands.go b/control-plane/commands.go index 4b7cbed362..0da29c938b 100644 --- a/control-plane/commands.go +++ b/control-plane/commands.go @@ -11,6 +11,7 @@ import ( cmdConsulLogout "github.com/hashicorp/consul-k8s/control-plane/subcommand/consul-logout" cmdCreateFederationSecret "github.com/hashicorp/consul-k8s/control-plane/subcommand/create-federation-secret" cmdDeleteCompletedJob "github.com/hashicorp/consul-k8s/control-plane/subcommand/delete-completed-job" + cmdFetchServerRegion "github.com/hashicorp/consul-k8s/control-plane/subcommand/fetch-server-region" cmdGetConsulClientCA "github.com/hashicorp/consul-k8s/control-plane/subcommand/get-consul-client-ca" cmdGossipEncryptionAutogenerate "github.com/hashicorp/consul-k8s/control-plane/subcommand/gossip-encryption-autogenerate" cmdInjectConnect "github.com/hashicorp/consul-k8s/control-plane/subcommand/inject-connect" @@ -90,6 +91,9 @@ func init() { "install-cni": func() (cli.Command, error) { return &cmdInstallCNI.Command{UI: ui}, nil }, + "fetch-server-region": func() (cli.Command, error) { + return &cmdFetchServerRegion.Command{UI: ui}, nil + }, } } diff --git a/control-plane/subcommand/fetch-server-region/command.go b/control-plane/subcommand/fetch-server-region/command.go new file mode 100644 index 0000000000..248ce971e7 --- /dev/null +++ b/control-plane/subcommand/fetch-server-region/command.go @@ -0,0 +1,158 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package fetchserverregion + +import ( + "context" + "encoding/json" + "flag" + "fmt" + "os" + "sync" + + "github.com/hashicorp/consul-k8s/control-plane/subcommand/common" + "github.com/hashicorp/consul-k8s/control-plane/subcommand/flags" + "github.com/hashicorp/go-hclog" + "github.com/mitchellh/cli" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" +) + +// The consul-logout command issues a Consul logout API request to delete an ACL token. +type Command struct { + UI cli.Ui + + flagLogLevel string + flagLogJSON bool + flagNodeName string + flagOutputFile string + + flagSet *flag.FlagSet + k8s *flags.K8SFlags + + once sync.Once + help string + logger hclog.Logger + + // for testing + clientset kubernetes.Interface +} + +type Locality struct { + Region string `json:"region"` +} + +type Config struct { + Locality Locality `json:"locality"` +} + +func (c *Command) init() { + c.flagSet = flag.NewFlagSet("", flag.ContinueOnError) + c.flagSet.StringVar(&c.flagLogLevel, "log-level", "info", + "Log verbosity level. Supported values (in order of detail) are \"trace\", "+ + "\"debug\", \"info\", \"warn\", and \"error\".") + c.flagSet.BoolVar(&c.flagLogJSON, "log-json", false, + "Enable or disable JSON output format for logging.") + c.flagSet.StringVar(&c.flagNodeName, "node-name", "", + "Specifies the node name that will be used.") + c.flagSet.StringVar(&c.flagOutputFile, "output-file", "", + "The file path for writing the locality portion of a Consul agent configuration to.") + + c.k8s = &flags.K8SFlags{} + flags.Merge(c.flagSet, c.k8s.Flags()) + + c.help = flags.Usage(help, c.flagSet) + +} + +func (c *Command) Run(args []string) int { + var err error + c.once.Do(c.init) + + if err := c.flagSet.Parse(args); err != nil { + return 1 + } + + if c.logger == nil { + c.logger, err = common.Logger(c.flagLogLevel, c.flagLogJSON) + if err != nil { + c.UI.Error(err.Error()) + return 1 + } + } + + if c.flagNodeName == "" { + c.UI.Error("-node-name is required") + return 1 + } + + if c.flagOutputFile == "" { + c.UI.Error("-output-file is required") + return 1 + } + + if c.clientset == nil { + config, err := rest.InClusterConfig() + if err != nil { + // This just allows us to test it locally. + kubeconfig := clientcmd.RecommendedHomeFile + config, err = clientcmd.BuildConfigFromFlags("", kubeconfig) + if err != nil { + c.UI.Error(err.Error()) + return 1 + } + } + + c.clientset, err = kubernetes.NewForConfig(config) + if err != nil { + c.UI.Error(err.Error()) + return 1 + } + } + + config := c.fetchLocalityConfig() + + jsonData, err := json.Marshal(config) + if err != nil { + c.UI.Error(err.Error()) + return 1 + } + + err = os.WriteFile(c.flagOutputFile, jsonData, 0644) + if err != nil { + c.UI.Error(fmt.Sprintf("Error writing locality file: %s", err)) + return 1 + } + + return 0 +} + +func (c *Command) fetchLocalityConfig() Config { + var cfg Config + node, err := c.clientset.CoreV1().Nodes().Get(context.Background(), c.flagNodeName, metav1.GetOptions{}) + if err != nil { + return cfg + } + + cfg.Locality.Region = node.Labels[corev1.LabelTopologyRegion] + + return cfg +} + +func (c *Command) Synopsis() string { return synopsis } +func (c *Command) Help() string { + c.once.Do(c.init) + return c.help +} + +const synopsis = "Fetch the cloud region for a Consul server from the Kubernetes node's region label." +const help = ` +Usage: consul-k8s-control-plane fetch-server-region [options] + + Fetch the region for a Consul server. + Not intended for stand-alone use. +` diff --git a/control-plane/subcommand/fetch-server-region/command_test.go b/control-plane/subcommand/fetch-server-region/command_test.go new file mode 100644 index 0000000000..d9c6112b14 --- /dev/null +++ b/control-plane/subcommand/fetch-server-region/command_test.go @@ -0,0 +1,108 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package fetchserverregion + +import ( + "os" + "testing" + + "github.com/mitchellh/cli" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" +) + +func TestRun_FlagValidation(t *testing.T) { + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + } + + cases := map[string]struct { + args []string + err string + }{ + "missing node name": { + args: []string{}, + err: "-node-name is required", + }, + "missing output-file": { + args: []string{"-node-name", "n1"}, + err: "-output-file is required", + }, + } + + for n, c := range cases { + t.Run(n, func(t *testing.T) { + responseCode := cmd.Run(c.args) + require.Equal(t, 1, responseCode, ui.ErrorWriter.String()) + require.Contains(t, ui.ErrorWriter.String(), c.err) + }) + } +} + +func TestRun(t *testing.T) { + cases := map[string]struct { + region string + expected string + missingNode bool + }{ + "no region": { + expected: `{"locality":{"region":""}}`, + }, + "region": { + region: "us-east-1", + expected: `{"locality":{"region":"us-east-1"}}`, + }, + "missing node": { + region: "us-east-1", + missingNode: true, + expected: `{"locality":{"region":""}}`, + }, + } + + for n, c := range cases { + t.Run(n, func(t *testing.T) { + outputFile, err := os.CreateTemp("", "ca") + require.NoError(t, err) + t.Cleanup(func() { + os.RemoveAll(outputFile.Name()) + }) + + var objs []runtime.Object + if !c.missingNode { + objs = append(objs, &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-node", + Labels: map[string]string{ + corev1.LabelTopologyRegion: c.region, + }, + }, + }) + } + + k8s := fake.NewSimpleClientset(objs...) + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + clientset: k8s, + } + + responseCode := cmd.Run([]string{ + "-node-name", + "my-node", + "-output-file", + outputFile.Name(), + }) + require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) + require.NoError(t, err) + cfg, err := os.ReadFile(outputFile.Name()) + require.NoError(t, err) + require.Equal(t, c.expected, string(cfg)) + }) + } +}