Address Code Review comments

charts/consul/test/unit/partition-init-job.bats

@@ -22,7 +22,7 @@ load _helpers
 
 @test "partitionInit/Job: disabled with global.adminPartitions.enabled=true and servers = true" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-job.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'server.enabled=true' \
@@ -31,7 +31,7 @@ load _helpers
 
 @test "partitionInit/Job: disabled with global.adminPartitions.enabled=true and global.enabled = true" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-job.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'global.enabled=true' \
@@ -40,7 +40,7 @@ load _helpers
 
 @test "partitionInit/Job: disabled with global.adminPartitions.enabled=false" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-job.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'server.enabled=true' \

charts/consul/test/unit/partition-init-podsecuritypolicy.bats

@@ -22,7 +22,7 @@ load _helpers
 
 @test "partitionInit/PodSecurityPolicy: disabled with global.adminPartitions.enabled=true and servers = true" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-podsecuritypolicy.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'server.enabled=true' \
@@ -31,7 +31,7 @@ load _helpers
 
 @test "partitionInit/PodSecurityPolicy: disabled with global.adminPartitions.enabled=true and global.enabled = true" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-podsecuritypolicy.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'global.enabled=true' \
@@ -40,7 +40,7 @@ load _helpers
 
 @test "partitionInit/PodSecurityPolicy: disabled with global.adminPartitions.enabled=false" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-podsecuritypolicy.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'server.enabled=true' \

charts/consul/test/unit/partition-init-role.bats

@@ -22,7 +22,7 @@ load _helpers
 
 @test "partitionInit/Role: disabled with global.adminPartitions.enabled=true and servers = true" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-role.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'server.enabled=true' \
@@ -31,7 +31,7 @@ load _helpers
 
 @test "partitionInit/Role: disabled with global.adminPartitions.enabled=true and global.enabled = true" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-role.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'global.enabled=true' \
@@ -40,7 +40,7 @@ load _helpers
 
 @test "partitionInit/Role: disabled with global.adminPartitions.enabled=false" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-role.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'server.enabled=true' \

charts/consul/test/unit/partition-init-rolebinding.bats

@@ -22,7 +22,7 @@ load _helpers
 
 @test "partitionInit/RoleBinding: disabled with global.adminPartitions.enabled=true and servers = true" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-rolebinding.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'server.enabled=true' \
@@ -31,7 +31,7 @@ load _helpers
 
 @test "partitionInit/RoleBinding: disabled with global.adminPartitions.enabled=true and global.enabled = true" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-rolebinding.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'global.enabled=true' \
@@ -40,7 +40,7 @@ load _helpers
 
 @test "partitionInit/RoleBinding: disabled with global.adminPartitions.enabled=false" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-rolebinding.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'server.enabled=true' \

charts/consul/test/unit/partition-init-serviceaccount.bats

@@ -22,7 +22,7 @@ load _helpers
 
 @test "partitionInit/ServiceAccount: disabled with global.adminPartitions.enabled=true and servers = true" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-serviceaccount.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'server.enabled=true' \
@@ -31,7 +31,7 @@ load _helpers
 
 @test "partitionInit/ServiceAccount: disabled with global.adminPartitions.enabled=true and global.enabled = true" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-serviceaccount.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'global.enabled=true' \
@@ -40,7 +40,7 @@ load _helpers
 
 @test "partitionInit/ServiceAccount: disabled with global.adminPartitions.enabled=false" {
   cd `chart_dir`
- assert_empty helm template \
+  assert_empty helm template \
       -s templates/partition-init-serviceaccount.yaml  \
       --set 'global.adminPartitions.enabled=true' \
       --set 'server.enabled=true' \

charts/consul/values.yaml

@@ -29,8 +29,16 @@ global:
   # Consul into Kubernetes will have, e.g. `service-name.service.consul`.
   domain: consul
 
+  # [Enterprise Only] Enabling `adminPartitions` allows creation of Admin Partitions in Kubernetes clusters.
+  # It additionally indicates that you are running Consul Enterprise v1.11+ with a valid Consul Enterprise
+  # license and would like to make use of configuration beyond registering everything into
+  # the `default` Consul partition.
   adminPartitions:
+    # If true, the Helm chart will enable Admin Partitions for the cluster. The clients in the server cluster
+    # must be installed in the default partition.
     enabled: false
+    # The name of the Admin Partition. Must be "default" in the server cluster ie the Kubernetes cluster that
+    # the Consul server pods are deployed onto.
     name: "default"
 
   # The name (and tag) of the Consul Docker image for clients and servers.

control-plane/subcommand/common/common.go

@@ -9,7 +9,10 @@ import (
 	"strings"
 
 	"github.com/go-logr/logr"
+	"github.com/hashicorp/consul-k8s/control-plane/consul"
+	godiscover "github.com/hashicorp/consul-k8s/control-plane/helper/go-discover"
 	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/go-discover"
 	"github.com/hashicorp/go-hclog"
 	"go.uber.org/zap/zapcore"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
@@ -115,3 +118,44 @@ func WriteFileWithPerms(outputFile, payload string, mode os.FileMode) error {
 	}
 	return os.Chmod(outputFile, mode)
 }
+
+// GetHTTPSScheme return the string value "https" if the bool https is true and "http" if it is false.
+func GetHTTPSScheme(https bool) string {
+	if https {
+		return "https"
+	} else {
+		return "http"
+	}
+}
+
+// GetResolvedServerAddresses resolves the Consul server address if it has been provided a provider else it returns the server addresses that were input to it.
+func GetResolvedServerAddresses(serverAddresses []string, providers map[string]discover.Provider, logger hclog.Logger) ([]string, error) {
+	var err error
+	if len(serverAddresses) == 1 && strings.Contains(serverAddresses[0], "provider=") {
+		serverAddresses, err = godiscover.ConsulServerAddresses(serverAddresses[0], providers, logger)
+		if err != nil {
+			logger.Error(fmt.Sprintf("Unable to discover any Consul addresses from %q: %s", serverAddresses[0], err))
+			return nil, err
+		}
+	}
+	return serverAddresses, nil
+}
+
+// GetConsulClientForServer returns a Consul client to communicate with a Consul server when provided the config required to create a client.
+func GetConsulClientForServer(serverAddresses []string, port uint, scheme, token, tlsServerName, consulCACert string, logger hclog.Logger) (*api.Client, error) {
+	serverAddr := fmt.Sprintf("%s:%d", serverAddresses[0], port)
+	consulClient, err := consul.NewClient(&api.Config{
+		Address: serverAddr,
+		Scheme:  scheme,
+		Token:   token,
+		TLSConfig: api.TLSConfig{
+			Address: tlsServerName,
+			CAFile:  consulCACert,
+		},
+	})
+	if err != nil {
+		logger.Error(fmt.Sprintf("Error creating Consul client for addr %q: %s", serverAddr, err))
+		return nil, err
+	}
+	return consulClient, nil
+}

control-plane/subcommand/common/common_test.go

@@ -11,7 +11,11 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/consul-k8s/control-plane/helper/go-discover/mocks"
 	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/go-discover"
+	"github.com/hashicorp/go-hclog"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
@@ -168,6 +172,57 @@ func TestWriteFileWithPerms(t *testing.T) {
 	require.Equal(t, payload, string(data))
 }
 
+func TestGetHTTPSScheme(t *testing.T) {
+	cases := []struct {
+		scheme  string
+		isHTTPS bool
+	}{
+		{
+			scheme:  "https",
+			isHTTPS: true,
+		},
+		{
+			scheme:  "http",
+			isHTTPS: false,
+		},
+	}
+	for _, testCase := range cases {
+		require.Equal(t, testCase.scheme, GetHTTPSScheme(testCase.isHTTPS))
+	}
+}
+
+func TestGetFilteredServerAddresses(t *testing.T) {
+	cases := map[string]struct {
+		inputServerAddresses    []string
+		providerMap             func() map[string]discover.Provider
+		expectedServerAddresses []string
+	}{
+		"without providers": {
+			inputServerAddresses: []string{"foo.bar", "hello.car"},
+			providerMap: func() map[string]discover.Provider {
+				return nil
+			},
+			expectedServerAddresses: []string{"foo.bar", "hello.car"},
+		},
+		"mock provider": {
+			inputServerAddresses: []string{"provider=mock"},
+			providerMap: func() map[string]discover.Provider {
+				provider := new(mocks.MockProvider)
+				provider.On("Addrs", mock.Anything, mock.Anything).Return([]string{"127.0.0.1", "foo.bar"}, nil)
+				providers := make(map[string]discover.Provider)
+				providers["mock"] = provider
+				return providers
+			},
+			expectedServerAddresses: []string{"127.0.0.1", "foo.bar"},
+		},
+	}
+	for _, testCase := range cases {
+		addresses, err := GetResolvedServerAddresses(testCase.inputServerAddresses, testCase.providerMap(), hclog.NewNullLogger())
+		require.NoError(t, err)
+		require.Equal(t, testCase.expectedServerAddresses, addresses)
+	}
+}
+
 // startMockServer starts an httptest server used to mock a Consul server's
 // /v1/acl/login endpoint. apiCallCounter will be incremented on each call to /v1/acl/login.
 // It returns a consul client pointing at the server.