From 83d66284a9890c533f295e6e78d1ddfb300a60f4 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Wed, 13 Jul 2022 10:34:54 -0400 Subject: [PATCH] wip --- .../peering_connect_namespaces_test.go | 149 ++++++++++++------ .../tests/peering/peering_connect_test.go | 109 ++++++++----- .../consul/templates/server-acl-init-job.yaml | 3 + .../consul/test/unit/server-acl-init-job.bats | 30 ++++ .../subcommand/server-acl-init/command.go | 6 + .../subcommand/server-acl-init/rules.go | 5 + .../subcommand/server-acl-init/rules_test.go | 44 +++++- 7 files changed, 256 insertions(+), 90 deletions(-) diff --git a/acceptance/tests/peering/peering_connect_namespaces_test.go b/acceptance/tests/peering/peering_connect_namespaces_test.go index eb53f3226e..d484210342 100644 --- a/acceptance/tests/peering/peering_connect_namespaces_test.go +++ b/acceptance/tests/peering/peering_connect_namespaces_test.go @@ -65,6 +65,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 { @@ -76,9 +94,8 @@ func TestPeering_ConnectNamespaces(t *testing.T) { "global.peering.enabled": "true", "global.enableConsulNamespaces": "true", - // "global.image": "hashicorp/consul-enterprise:1.13.0-alpha2-ent", - "global.image": "thisisnotashwin/consul@sha256:0733380a1a177d269c53fff62464e3a4840ea0c3ca24c6164282f8a822ec8825", - "global.imageK8S": "thisisnotashwin/consul-k8s@sha256:6fe1ec532876073813c824f27b2c972c03a41376e0729a502f6f3302dc352379", + "global.image": "thisisnotashwin/consul@sha256:1cb055ffd00ef1652dd425d14cf50ef2b6ac3f455efcf338f032d53812ae1c6d", + "global.imageK8S": "thisisnotashwin/consul-k8s@sha256:23b4e35c46147851c799930f46a6840b83f8d6013d6cbcf0ef23a16b8c34f299", "global.tls.enabled": "false", "global.tls.httpsOnly": strconv.FormatBool(c.ACLsAndAutoEncryptEnabled), @@ -260,60 +277,94 @@ 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", "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") + } - 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)) + // 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) } else { - k8s.CheckStaticServerConnectionFailing(t, staticClientOpts, staticClientName, "http://localhost:1234") - } + 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") + } - intention := &api.ServiceIntentionsConfigEntry{ - Name: staticServerName, - Kind: api.ServiceIntentions, - Namespace: staticServerNamespace, - Sources: []*api.SourceIntention{ - { - Name: staticClientName, - Namespace: staticClientNamespace, - Action: api.IntentionActionAllow, - Peer: staticClientPeer, + denyAllIntention := &api.ServiceIntentionsConfigEntry{ + Name: "*", + Kind: api.ServiceIntentions, + Namespace: "*", + Sources: []*api.SourceIntention{ + { + Name: "*", + Namespace: "*", + Action: api.IntentionActionDeny, + Peer: staticClientPeer, + }, }, - }, - } + } + _, _, err = staticServerPeerClient.ConfigEntries().Set(denyAllIntention, &api.WriteOptions{}) + require.NoError(t, err) - // 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, "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") + } - 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, + 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 + } + + 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 { diff --git a/acceptance/tests/peering/peering_connect_test.go b/acceptance/tests/peering/peering_connect_test.go index 1f05ef5498..4304128074 100644 --- a/acceptance/tests/peering/peering_connect_test.go +++ b/acceptance/tests/peering/peering_connect_test.go @@ -39,6 +39,10 @@ func TestPeering_Connect(t *testing.T) { "default installation", false, }, + { + "default installation", + true, + }, } for _, c := range cases { @@ -49,9 +53,8 @@ func TestPeering_Connect(t *testing.T) { commonHelmValues := map[string]string{ "global.peering.enabled": "true", - // "global.image": "hashicorp/consul:1.13.0-alpha2", - "global.image": "thisisnotashwin/consul@sha256:0733380a1a177d269c53fff62464e3a4840ea0c3ca24c6164282f8a822ec8825", - "global.imageK8S": "thisisnotashwin/consul-k8s@sha256:6fe1ec532876073813c824f27b2c972c03a41376e0729a502f6f3302dc352379", + "global.image": "thisisnotashwin/consul@sha256:1cb055ffd00ef1652dd425d14cf50ef2b6ac3f455efcf338f032d53812ae1c6d", + "global.imageK8S": "thisisnotashwin/consul-k8s@sha256:23b4e35c46147851c799930f46a6840b83f8d6013d6cbcf0ef23a16b8c34f299", "global.tls.enabled": "false", "global.tls.httpsOnly": strconv.FormatBool(c.ACLsAndAutoEncryptEnabled), @@ -204,49 +207,75 @@ 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.ns1 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)) + logger.Log(t, "creating intentions in server peer") + _, _, err = staticServerPeerClient.ConfigEntries().Set(intention, &api.WriteOptions{}) + require.NoError(t, err) } 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, + 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, + }, }, - }, - } + } + _, _, 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") + } + + 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, "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 { 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/test/unit/server-acl-init-job.bats b/charts/consul/test/unit/server-acl-init-job.bats index 8259ec1c36..6d7ea4cdbe 100644 --- a/charts/consul/test/unit/server-acl-init-job.bats +++ b/charts/consul/test/unit/server-acl-init-job.bats @@ -1386,6 +1386,36 @@ 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' \ + . | 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 988f9bb16e..e2d1b3b1ff 100644 --- a/control-plane/subcommand/server-acl-init/command.go +++ b/control-plane/subcommand/server-acl-init/command.go @@ -77,6 +77,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 @@ -179,6 +182,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()