Update with Iryna's review

hashicorp · Oct 15, 2021 · dcb2d87 · dcb2d87
1 parent 11fd13f
commit dcb2d87
Show file tree

Hide file tree

Showing 10 changed files with 126 additions and 128 deletions.
diff --git a/charts/consul/templates/partition-init-job.yaml b/charts/consul/templates/partition-init-job.yaml
@@ -42,7 +42,7 @@ spec:
                 path: tls.crt
       {{- end }}
       containers:
-        - name: partition-create-job
+        - name: partition-init-job
           image: {{ .Values.global.imageK8S }}
           env:
             - name: NAMESPACE

diff --git a/charts/consul/test/acceptance/framework/consul/consul_cluster.go b/charts/consul/test/acceptance/framework/consul/consul_cluster.go
@@ -262,13 +262,10 @@ func (h *HelmCluster) SetupConsulClient(t *testing.T, secure bool) *api.Client {
 		}
 	}
 
-	clientsList, err := h.kubernetesClient.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{LabelSelector: "component=client"})
-	require.NoError(t, err)
-
 	tunnel := terratestk8s.NewTunnelWithLogger(
 		h.helmOptions.KubectlOptions,
 		terratestk8s.ResourceTypePod,
-		clientsList.Items[0].Name,
+		fmt.Sprintf("%s-consul-server-0", h.releaseName),
 		localPort,
 		remotePort,
 		h.logger)

diff --git a/charts/consul/test/acceptance/go.mod b/charts/consul/test/acceptance/go.mod
@@ -4,7 +4,7 @@ go 1.14
 
 require (
 	github.com/gruntwork-io/terratest v0.31.2
-	github.com/hashicorp/consul/api v1.9.0
+	github.com/hashicorp/consul/api v1.10.1-0.20210915232521-e0a7900f52bf
 	github.com/hashicorp/consul/sdk v0.8.0
 	github.com/stretchr/testify v1.5.1
 	gopkg.in/yaml.v2 v2.2.8

diff --git a/charts/consul/test/acceptance/go.sum b/charts/consul/test/acceptance/go.sum
@@ -225,8 +225,9 @@ github.com/gruntwork-io/gruntwork-cli v0.7.0 h1:YgSAmfCj9c61H+zuvHwKfYUwlMhu5arn
 github.com/gruntwork-io/gruntwork-cli v0.7.0/go.mod h1:jp6Z7NcLF2avpY8v71fBx6hds9eOFPELSuD/VPv7w00=
 github.com/gruntwork-io/terratest v0.31.2 h1:xvYHA80MUq5kx670dM18HInewOrrQrAN+XbVVtytUHg=
 github.com/gruntwork-io/terratest v0.31.2/go.mod h1:EEgJie28gX/4AD71IFqgMj6e99KP5mi81hEtzmDjxTo=
-github.com/hashicorp/consul/api v1.9.0 h1:T6dKIWcaihG2c21YUi0BMAHbJanVXiYuz+mPgqxY3N4=
-github.com/hashicorp/consul/api v1.9.0/go.mod h1:XjsvQN+RJGWI2TWy1/kqaE16HrR2J/FWgkYjdZQsX9M=
+github.com/hashicorp/consul/api v1.10.1-0.20210915232521-e0a7900f52bf h1:fouyN8SkrE4py09XaOru4PCM9zunem39CjOrMJMrKsc=
+github.com/hashicorp/consul/api v1.10.1-0.20210915232521-e0a7900f52bf/go.mod h1:sDjTOq0yUyv5G4h+BqSea7Fn6BU+XbolEz1952UB+mk=
+github.com/hashicorp/consul/sdk v0.7.0/go.mod h1:fY08Y9z5SvJqevyZNy6WWPXiG3KwBPAvlcdx16zZ0fM=
 github.com/hashicorp/consul/sdk v0.8.0 h1:OJtKBtEjboEZvG6AOUdh4Z1Zbyu0WcxQ0qatRrZHTVU=
 github.com/hashicorp/consul/sdk v0.8.0/go.mod h1:GBvyrGALthsZObzUGsfgHZQDXjg4lOjagTIwIR1vPms=
 github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=

diff --git a/charts/consul/test/acceptance/tests/partitions/partitions_test.go b/charts/consul/test/acceptance/tests/partitions/partitions_test.go
diff --git a/charts/consul/test/unit/partition-init-job.bats b/charts/consul/test/unit/partition-init-job.bats
@@ -98,7 +98,7 @@ load _helpers
 #--------------------------------------------------------------------
 # global.acls.bootstrapToken
 
-@test "partitionInit/Job: HTTP_TOKEN when global.acls.bootstrapToken is provided" {
+@test "partitionInit/Job: HTTP_TOKEN is set when global.acls.bootstrapToken is provided" {
   cd `chart_dir`
   local actual=$(helm template \
       -s templates/partition-init-job.yaml  \

diff --git a/control-plane/subcommand/server-acl-init/command.go b/control-plane/subcommand/server-acl-init/command.go
@@ -56,22 +56,22 @@ type Command struct {
 	flagIngressGatewayNames     []string
 	flagTerminatingGatewayNames []string
 
-	// Flags to configure Consul connection
+	// Flags to configure Consul connection.
 	flagServerAddresses     []string
 	flagServerPort          uint
 	flagConsulCACert        string
 	flagConsulTLSServerName string
 	flagUseHTTPS            bool
 
-	// Flags for ACL replication
+	// Flags for ACL replication.
 	flagCreateACLReplicationToken bool
 	flagACLReplicationTokenFile   string
 
-	// Flags to support partitions
+	// Flags to support partitions.
 	flagEnablePartitions bool   // true if Admin Partitions are enabled
 	flagPartitionName    string // name of the Admin Partition
 
-	// Flags to support namespaces
+	// 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
 	flagEnableSyncK8SNSMirroring         bool   // Enables mirroring of k8s namespaces into Consul for catalog sync
@@ -80,7 +80,7 @@ type Command struct {
 	flagEnableInjectK8SNSMirroring       bool   // Enables mirroring of k8s namespaces into Consul for Connect inject
 	flagInjectK8SNSMirroringPrefix       string // Prefix added to Consul namespaces created when mirroring injected services
 
-	// Flag to support a custom bootstrap token
+	// Flag to support a custom bootstrap token.
 	flagBootstrapTokenFile string
 
 	flagLogLevel string
@@ -396,7 +396,7 @@ func (c *Command) Run(args []string) int {
 		}
 	}
 
-	if c.flagEnablePartitions && c.flagPartitionName == "default" && isPrimary {
+	if c.flagEnablePartitions && c.flagPartitionName == consulDefaultPartition && isPrimary {
 		// Partition token must be local because only the Primary datacenter can have Admin Partitions.
 		err := c.createLocalACL("partitions", partitionRules, consulDC, isPrimary, consulClient)
 		if err != nil {
@@ -870,6 +870,7 @@ func (c *Command) validateFlags() error {
 }
 
 const consulDefaultNamespace = "default"
+const consulDefaultPartition = "default"
 const synopsis = "Initialize ACLs on Consul servers and other components."
 const help = `
 Usage: consul-k8s-control-plane server-acl-init [options]

diff --git a/control-plane/subcommand/server-acl-init/command_ent_test.go b/control-plane/subcommand/server-acl-init/command_ent_test.go
@@ -354,12 +354,13 @@ func TestRun_ACLPolicyUpdates(t *testing.T) {
 				case "connect-inject-token":
 					// The connect inject token doesn't have namespace config,
 					// but does change to operator:write from an empty string.
-					require.Contains(actRules, "acl = \"write\"")
+					require.Contains(actRules, "policy = \"write\"")
 				case "client-snapshot-agent-token", "enterprise-license-token":
 					// The snapshot agent and enterprise license tokens shouldn't change.
 					require.NotContains(actRules, "namespace")
+					require.Contains(actRules, "acl = \"write\"")
 				case "partitions-token":
-					require.Contains(actRules, "acl = \"write\"\noperator = \"write\"")
+					require.Contains(actRules, "operator = \"write\"")
 				default:
 					// Assert that the policies have the word namespace in them. This
 					// tests that they were updated. The actual contents are tested

diff --git a/control-plane/subcommand/server-acl-init/rules.go b/control-plane/subcommand/server-acl-init/rules.go
@@ -39,14 +39,12 @@ service "consul-snapshot" {
 const entLicenseRules = `operator = "write"`
 const entPartitionLicenseRules = `acl = "write"`
 
-const partitionRules = `acl = "write"
-operator = "write"
+const partitionRules = `operator = "write"
 agent_prefix "" {
   policy = "read"
 }
 partition_prefix "" {
   acl = "write"
-  mesh = "write"
 }`
 
 func (c *Command) crossNamespaceRule() (string, error) {
@@ -259,7 +257,6 @@ func (c *Command) injectRules() (string, error) {
 	injectRulesTpl := `
 {{- if .EnablePartitions }}
 partition "{{ .PartitionName }}" {
-  acl = "write"
 {{- else }}
 {{- if .EnableNamespaces }}
   operator = "write"

diff --git a/control-plane/subcommand/server-acl-init/rules_test.go b/control-plane/subcommand/server-acl-init/rules_test.go
@@ -644,7 +644,6 @@ func TestInjectRules(t *testing.T) {
 			PartitionName:    "part-1",
 			Expected: `
 partition "part-1" {
-  acl = "write"
   node_prefix "" {
     policy = "write"
   }