From 545436203a027a260b657d1bab07d11dfa98b0fc Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Wed, 13 Jul 2022 10:34:54 -0400 Subject: [PATCH] Add ACL support for Cluster Peering --- CHANGELOG.md | 5 +- .../peering_connect_namespaces_test.go | 99 +++++++++---------- .../tests/peering/peering_connect_test.go | 68 +++++-------- .../consul/templates/server-acl-init-job.yaml | 3 + .../consul/templates/server-statefulset.yaml | 1 - .../consul/test/unit/server-acl-init-job.bats | 31 ++++++ .../subcommand/server-acl-init/command.go | 6 ++ .../subcommand/server-acl-init/rules.go | 5 + .../subcommand/server-acl-init/rules_test.go | 44 ++++++++- 9 files changed, 167 insertions(+), 95 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 275d77ed11..c12de4ce7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## UNRELEASED +FEATURES: +* [Experimental] Cluster Peering: + * Add support for ACLs and TLS. [[GH-1343](https://github.com/hashicorp/consul-k8s/pull/1343)] [[GH-1366](https://github.com/hashicorp/consul-k8s/pull/1366)] + ## 0.46.0 (July 20, 2022) IMPROVEMENTS: * Control Plane @@ -7,7 +11,6 @@ IMPROVEMENTS: * CLI * Update minimum go version for project to 1.18 [[GH-1292](https://github.com/hashicorp/consul-k8s/pull/1292)] - FEATURES: * [Experimental] Cluster Peering: * Add support for secret watchers on the Peering Acceptor and Peering Dialer controllers. [[GH-1284](https://github.com/hashicorp/consul-k8s/pull/1284)] diff --git a/acceptance/tests/peering/peering_connect_namespaces_test.go b/acceptance/tests/peering/peering_connect_namespaces_test.go index 66323ee487..da329a4a35 100644 --- a/acceptance/tests/peering/peering_connect_namespaces_test.go +++ b/acceptance/tests/peering/peering_connect_namespaces_test.go @@ -66,6 +66,24 @@ func TestPeering_ConnectNamespaces(t *testing.T) { true, false, }, + { + "default destination namespace", + defaultNamespace, + false, + true, + }, + { + "single destination namespace", + staticServerNamespace, + false, + true, + }, + { + "mirror k8s namespaces", + staticServerNamespace, + true, + true, + }, } for _, c := range cases { @@ -77,8 +95,8 @@ func TestPeering_ConnectNamespaces(t *testing.T) { "global.peering.enabled": "true", "global.enableConsulNamespaces": "true", - "global.image": "thisisnotashwin/consul@sha256:0733380a1a177d269c53fff62464e3a4840ea0c3ca24c6164282f8a822ec8825", - "global.imageK8S": "thisisnotashwin/consul-k8s@sha256:6fe1ec532876073813c824f27b2c972c03a41376e0729a502f6f3302dc352379", + "global.image": "thisisnotashwin/consul@sha256:a7db15820b3724c5a9f78a858ac06b5b44bc95e025d6325d55ac546308dd07ea", + "global.imageK8S": "thisisnotashwin/consul-k8s@sha256:78e58cc3dc750af60c59a7d9ea6399a2632a8626adf26e02b915513c3424b7a1", "global.tls.enabled": "true", "global.tls.httpsOnly": strconv.FormatBool(c.ACLsAndAutoEncryptEnabled), @@ -267,61 +285,40 @@ func TestPeering_ConnectNamespaces(t *testing.T) { }) } - logger.Log(t, "checking that connection is successful") - if cfg.EnableTransparentProxy { - k8s.CheckStaticServerConnectionSuccessful(t, staticClientOpts, staticClientName, fmt.Sprintf("http://static-server.virtual.%s.%s.consul", c.destinationNamespace, staticServerPeer)) - } else { - k8s.CheckStaticServerConnectionSuccessful(t, staticClientOpts, staticClientName, "http://localhost:1234") - } + if c.ACLsAndAutoEncryptEnabled { + logger.Log(t, "checking that the connection is not successful because there's no allow intention") + if cfg.EnableTransparentProxy { + k8s.CheckStaticServerConnectionMultipleFailureMessages(t, staticClientOpts, staticClientName, false, []string{"curl: (56) Recv failure: Connection reset by peer", "curl: (52) Empty reply from server", fmt.Sprintf("curl: (7) Failed to connect to static-server.%s port 80: Connection refused", c.destinationNamespace)}, "", fmt.Sprintf("http://static-server.virtual.%s.%s.consul", c.destinationNamespace, staticServerPeer)) + } else { + k8s.CheckStaticServerConnectionFailing(t, staticClientOpts, staticClientName, "http://localhost:1234") + } - denyAllIntention := &api.ServiceIntentionsConfigEntry{ - Name: "*", - Kind: api.ServiceIntentions, - Namespace: "*", - Sources: []*api.SourceIntention{ - { - Name: "*", - Namespace: "*", - Action: api.IntentionActionDeny, - Peer: staticClientPeer, + intention := &api.ServiceIntentionsConfigEntry{ + Name: staticServerName, + Kind: api.ServiceIntentions, + Namespace: staticServerNamespace, + Sources: []*api.SourceIntention{ + { + Name: staticClientName, + Namespace: staticClientNamespace, + Action: api.IntentionActionAllow, + Peer: staticClientPeer, + }, }, - }, - } - _, _, err = staticServerPeerClient.ConfigEntries().Set(denyAllIntention, &api.WriteOptions{}) - require.NoError(t, err) - - logger.Log(t, "checking that the connection is not successful because there's no allow intention") - if cfg.EnableTransparentProxy { - k8s.CheckStaticServerConnectionMultipleFailureMessages(t, staticClientOpts, staticClientName, false, []string{"curl: (56) Recv failure: Connection reset by peer", "curl: (52) Empty reply from server", "curl: (7) Failed to connect to static-server.ns1 port 80: Connection refused"}, "", fmt.Sprintf("http://static-server.virtual.%s.%s.consul", c.destinationNamespace, staticServerPeer)) - } else { - k8s.CheckStaticServerConnectionFailing(t, staticClientOpts, staticClientName, "http://localhost:1234") - } + } - intention := &api.ServiceIntentionsConfigEntry{ - Name: staticServerName, - Kind: api.ServiceIntentions, - Namespace: staticServerNamespace, - Sources: []*api.SourceIntention{ - { - Name: staticClientName, - Namespace: staticClientNamespace, - Action: api.IntentionActionAllow, - Peer: staticClientPeer, - }, - }, - } + // Set the destination namespace to be the same + // unless mirrorK8S is true. + if !c.mirrorK8S { + intention.Namespace = c.destinationNamespace + intention.Sources[0].Namespace = c.destinationNamespace + } - // Set the destination namespace to be the same - // unless mirrorK8S is true. - if !c.mirrorK8S { - intention.Namespace = c.destinationNamespace - intention.Sources[0].Namespace = c.destinationNamespace + logger.Log(t, "creating intentions in server peer") + _, _, err = staticServerPeerClient.ConfigEntries().Set(intention, &api.WriteOptions{}) + require.NoError(t, err) } - logger.Log(t, "creating intentions in server peer") - _, _, err = staticServerPeerClient.ConfigEntries().Set(intention, &api.WriteOptions{}) - require.NoError(t, err) - logger.Log(t, "checking that connection is successful") if cfg.EnableTransparentProxy { k8s.CheckStaticServerConnectionSuccessful(t, staticClientOpts, staticClientName, fmt.Sprintf("http://static-server.virtual.%s.%s.consul", c.destinationNamespace, staticServerPeer)) diff --git a/acceptance/tests/peering/peering_connect_test.go b/acceptance/tests/peering/peering_connect_test.go index 84fc7752b5..a869136f0b 100644 --- a/acceptance/tests/peering/peering_connect_test.go +++ b/acceptance/tests/peering/peering_connect_test.go @@ -40,6 +40,10 @@ func TestPeering_Connect(t *testing.T) { "default installation", false, }, + { + "default installation", + true, + }, } for _, c := range cases { @@ -50,8 +54,8 @@ func TestPeering_Connect(t *testing.T) { commonHelmValues := map[string]string{ "global.peering.enabled": "true", - "global.image": "thisisnotashwin/consul@sha256:0733380a1a177d269c53fff62464e3a4840ea0c3ca24c6164282f8a822ec8825", - "global.imageK8S": "thisisnotashwin/consul-k8s@sha256:6fe1ec532876073813c824f27b2c972c03a41376e0729a502f6f3302dc352379", + "global.image": "thisisnotashwin/consul@sha256:a7db15820b3724c5a9f78a858ac06b5b44bc95e025d6325d55ac546308dd07ea", + "global.imageK8S": "thisisnotashwin/consul-k8s@sha256:78e58cc3dc750af60c59a7d9ea6399a2632a8626adf26e02b915513c3424b7a1", "global.tls.enabled": "true", "global.tls.httpsOnly": strconv.FormatBool(c.ACLsAndAutoEncryptEnabled), @@ -211,50 +215,32 @@ func TestPeering_Connect(t *testing.T) { helpers.Cleanup(t, cfg.NoCleanupOnFailure, func() { k8s.KubectlDeleteK(t, staticServerPeerClusterContext.KubectlOptions(t), "../fixtures/cases/crd-peers/default") }) - logger.Log(t, "checking that connection is successful") - if cfg.EnableTransparentProxy { - k8s.CheckStaticServerConnectionSuccessful(t, staticClientOpts, staticClientName, fmt.Sprintf("http://static-server.virtual.%s.consul", staticServerPeer)) - } else { - k8s.CheckStaticServerConnectionSuccessful(t, staticClientOpts, staticClientName, "http://localhost:1234") - } - denyAllIntention := &api.ServiceIntentionsConfigEntry{ - Name: "*", - Kind: api.ServiceIntentions, - Sources: []*api.SourceIntention{ - { - Name: "*", - Action: api.IntentionActionDeny, - Peer: staticClientPeer, + if c.ACLsAndAutoEncryptEnabled { + logger.Log(t, "checking that the connection is not successful because there's no allow intention") + if cfg.EnableTransparentProxy { + k8s.CheckStaticServerConnectionMultipleFailureMessages(t, staticClientOpts, staticClientName, false, []string{"curl: (56) Recv failure: Connection reset by peer", "curl: (52) Empty reply from server", "curl: (7) Failed to connect to static-server port 80: Connection refused"}, "", fmt.Sprintf("http://static-server.virtual.%s.consul", staticServerPeer)) + } else { + k8s.CheckStaticServerConnectionFailing(t, staticClientOpts, staticClientName, "http://localhost:1234") + } + + intention := &api.ServiceIntentionsConfigEntry{ + Name: staticServerName, + Kind: api.ServiceIntentions, + Sources: []*api.SourceIntention{ + { + Name: staticClientName, + Action: api.IntentionActionAllow, + Peer: staticClientPeer, + }, }, - }, - } - _, _, err = staticServerPeerClient.ConfigEntries().Set(denyAllIntention, &api.WriteOptions{}) - require.NoError(t, err) + } - logger.Log(t, "checking that the connection is not successful because there's no allow intention") - if cfg.EnableTransparentProxy { - k8s.CheckStaticServerConnectionMultipleFailureMessages(t, staticClientOpts, staticClientName, false, []string{"curl: (56) Recv failure: Connection reset by peer", "curl: (52) Empty reply from server", "curl: (7) Failed to connect to static-server.ns1 port 80: Connection refused"}, "", fmt.Sprintf("http://static-server.virtual.%s.consul", staticServerPeer)) - } else { - k8s.CheckStaticServerConnectionFailing(t, staticClientOpts, staticClientName, "http://localhost:1234") + logger.Log(t, "creating intentions in server peer") + _, _, err = staticServerPeerClient.ConfigEntries().Set(intention, &api.WriteOptions{}) + require.NoError(t, err) } - intention := &api.ServiceIntentionsConfigEntry{ - Name: staticServerName, - Kind: api.ServiceIntentions, - Sources: []*api.SourceIntention{ - { - Name: staticClientName, - Action: api.IntentionActionAllow, - Peer: staticClientPeer, - }, - }, - } - - logger.Log(t, "creating intentions in server peer") - _, _, err = staticServerPeerClient.ConfigEntries().Set(intention, &api.WriteOptions{}) - require.NoError(t, err) - logger.Log(t, "checking that connection is successful") if cfg.EnableTransparentProxy { k8s.CheckStaticServerConnectionSuccessful(t, staticClientOpts, staticClientName, fmt.Sprintf("http://static-server.virtual.%s.consul", staticServerPeer)) diff --git a/charts/consul/templates/server-acl-init-job.yaml b/charts/consul/templates/server-acl-init-job.yaml index c736384a4e..8709b82af7 100644 --- a/charts/consul/templates/server-acl-init-job.yaml +++ b/charts/consul/templates/server-acl-init-job.yaml @@ -185,6 +185,9 @@ spec: -enable-partitions=true \ -partition={{ .Values.global.adminPartitions.name }} \ {{- end }} + {{- 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)) }} -allow-dns=true \ {{- end }} diff --git a/charts/consul/templates/server-statefulset.yaml b/charts/consul/templates/server-statefulset.yaml index 486b4a5ba5..42df4f2f58 100644 --- a/charts/consul/templates/server-statefulset.yaml +++ b/charts/consul/templates/server-statefulset.yaml @@ -296,7 +296,6 @@ spec: -config-dir=/consul/userconfig/{{ .name }} \ {{- end }} {{- end }} - -hcl='ports { grpc = 8503 }' \ -config-file=/consul/extra-config/extra-from-values.json volumeMounts: - name: data-{{ .Release.Namespace | trunc 58 | trimSuffix "-" }} diff --git a/charts/consul/test/unit/server-acl-init-job.bats b/charts/consul/test/unit/server-acl-init-job.bats index 8259ec1c36..7b442a2ef8 100644 --- a/charts/consul/test/unit/server-acl-init-job.bats +++ b/charts/consul/test/unit/server-acl-init-job.bats @@ -1386,6 +1386,37 @@ load _helpers [ "${actual}" = "true" ] } +#-------------------------------------------------------------------- +# cluster peering + +@test "serverACLInit/Job: cluster peering disabled by default" { + cd `chart_dir` + local object=$(helm template \ + -s templates/server-acl-init-job.yaml \ + --set 'global.acls.manageSystemACLs=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) + + local actual=$(echo $object | + yq 'any(contains("enable-peering"))' | tee /dev/stderr) + [ "${actual}" = "false" ] +} + +@test "serverACLInit/Job: cluster peering enabled when peering is enabled" { + cd `chart_dir` + local object=$(helm template \ + -s templates/server-acl-init-job.yaml \ + --set 'global.acls.manageSystemACLs=true' \ + --set 'global.peering.enabled=true' \ + --set 'connectInject.enabled=true' \ + . | tee /dev/stderr | + yq '.spec.template.spec.containers[0].command' | tee /dev/stderr) + + local actual=$(echo $object | + yq 'any(contains("enable-peering"))' | tee /dev/stderr) + [ "${actual}" = "true" ] +} + #-------------------------------------------------------------------- # admin partitions diff --git a/control-plane/subcommand/server-acl-init/command.go b/control-plane/subcommand/server-acl-init/command.go index 93a6fefc31..ddf9b32ba3 100644 --- a/control-plane/subcommand/server-acl-init/command.go +++ b/control-plane/subcommand/server-acl-init/command.go @@ -79,6 +79,9 @@ type Command struct { flagPartitionName string // name of the Admin Partition flagPartitionTokenFile string + // Flags to support peering. + flagEnablePeering bool // true if Cluster Peering is enabled + // Flags to support namespaces. flagEnableNamespaces bool // Use namespacing on all components flagConsulSyncDestinationNamespace string // Consul namespace to register all catalog sync services into if not mirroring @@ -181,6 +184,9 @@ func (c *Command) init() { c.flags.StringVar(&c.flagPartitionTokenFile, "partition-token-file", "", "[Enterprise Only] Path to file containing ACL token to be used in non-default partitions.") + c.flags.BoolVar(&c.flagEnablePeering, "enable-peering", false, + "Enables Cluster Peering.") + c.flags.BoolVar(&c.flagEnableNamespaces, "enable-namespaces", false, "[Enterprise Only] Enables namespaces, in either a single Consul namespace or mirrored [Enterprise only feature]") c.flags.StringVar(&c.flagConsulSyncDestinationNamespace, "consul-sync-destination-namespace", consulDefaultNamespace, diff --git a/control-plane/subcommand/server-acl-init/rules.go b/control-plane/subcommand/server-acl-init/rules.go index eb24b6138e..8b2dec7a14 100644 --- a/control-plane/subcommand/server-acl-init/rules.go +++ b/control-plane/subcommand/server-acl-init/rules.go @@ -8,6 +8,7 @@ import ( type rulesData struct { EnablePartitions bool + EnablePeering bool PartitionName string EnableNamespaces bool SyncConsulDestNS string @@ -315,6 +316,9 @@ partition "{{ .PartitionName }}" { {{- if .EnableNamespaces }} operator = "write" {{- end }} +{{- end }} +{{- if .EnablePeering }} + peering = "write" {{- end }} node_prefix "" { policy = "write" @@ -416,6 +420,7 @@ partition "{{ .PartitionName }}" { func (c *Command) rulesData() rulesData { return rulesData{ EnablePartitions: c.flagEnablePartitions, + EnablePeering: c.flagEnablePeering, PartitionName: c.flagPartitionName, EnableNamespaces: c.flagEnableNamespaces, SyncConsulDestNS: c.flagConsulSyncDestinationNamespace, diff --git a/control-plane/subcommand/server-acl-init/rules_test.go b/control-plane/subcommand/server-acl-init/rules_test.go index 723ab55f4e..1e736f9a95 100644 --- a/control-plane/subcommand/server-acl-init/rules_test.go +++ b/control-plane/subcommand/server-acl-init/rules_test.go @@ -848,12 +848,14 @@ func TestInjectRules(t *testing.T) { cases := []struct { EnableNamespaces bool EnablePartitions bool + EnablePeering bool PartitionName string Expected string }{ { EnableNamespaces: false, EnablePartitions: false, + EnablePeering: false, Expected: ` node_prefix "" { policy = "write" @@ -866,6 +868,7 @@ func TestInjectRules(t *testing.T) { { EnableNamespaces: true, EnablePartitions: false, + EnablePeering: false, Expected: ` operator = "write" node_prefix "" { @@ -877,13 +880,51 @@ func TestInjectRules(t *testing.T) { policy = "write" } }`, + }, + { + EnableNamespaces: true, + EnablePartitions: false, + EnablePeering: true, + Expected: ` + operator = "write" + peering = "write" + node_prefix "" { + policy = "write" + } + namespace_prefix "" { + acl = "write" + service_prefix "" { + policy = "write" + } + }`, + }, + { + EnableNamespaces: true, + EnablePartitions: true, + EnablePeering: false, + PartitionName: "part-1", + Expected: ` +partition "part-1" { + node_prefix "" { + policy = "write" + } + namespace_prefix "" { + policy = "write" + acl = "write" + service_prefix "" { + policy = "write" + } + } +}`, }, { EnableNamespaces: true, EnablePartitions: true, + EnablePeering: true, PartitionName: "part-1", Expected: ` partition "part-1" { + peering = "write" node_prefix "" { policy = "write" } @@ -899,13 +940,14 @@ partition "part-1" { } for _, tt := range cases { - caseName := fmt.Sprintf("ns=%t, partition=%t", tt.EnableNamespaces, tt.EnablePartitions) + caseName := fmt.Sprintf("ns=%t, partition=%t, peering=%t", tt.EnableNamespaces, tt.EnablePartitions, tt.EnablePeering) t.Run(caseName, func(t *testing.T) { cmd := Command{ flagEnablePartitions: tt.EnablePartitions, flagPartitionName: tt.PartitionName, flagEnableNamespaces: tt.EnableNamespaces, + flagEnablePeering: tt.EnablePeering, } injectorRules, err := cmd.injectRules()