wip

acceptance/tests/mesh_v2/mesh_inject_test.go

@@ -25,7 +25,7 @@ const multiport = "multiport"
 // two ports. This tests inbound connections to each port of the multiport app, and outbound connections from the
 // multiport app to static-server.
 func TestMeshInject_MultiportService(t *testing.T) {
-	for _, secure := range []bool{false} {
+	for _, secure := range []bool{false, true} {
 		name := fmt.Sprintf("secure: %t", secure)
 
 		t.Run(name, func(t *testing.T) {
@@ -34,6 +34,11 @@ func TestMeshInject_MultiportService(t *testing.T) {
 			ctx := suite.Environment().DefaultContext(t)
 
 			helmValues := map[string]string{
+				// TODO(dans): remove me
+				"global.image":                "hashicorppreview/consul:1.17-dev-679b0f650f115816a77ea6d7dad9a91ca297c8b1", // This is an older commit with V1 API still enabled
+				"global.imageConsulDataplane": "hashicorppreview/consul-dataplane:1.3.0-dev",
+				"global.imageK8S":             "dstoughhashicorp/consul-k8s-control-plane:v2-acl-02",
+
 				"global.experiments[0]": "resource-apis",
 				// The UI is not supported for v2 in 1.17, so for now it must be disabled.
 				"ui.enabled":            "false",
@@ -74,9 +79,11 @@ func TestMeshInject_MultiportService(t *testing.T) {
 			require.Len(t, podList.Items, 1)
 			require.Len(t, podList.Items[0].Spec.Containers, 3)
 
-			{
-				// TODO: once ACLs are implemented, only run this block when secure=true and delete the below line and fixture since the default is deny when secure is true.
+			if !secure {
 				k8s.KubectlApplyK(t, ctx.KubectlOptions(t), "../../tests/fixtures/cases/trafficpermissions-deny")
+			}
+
+			{
 				// Now test that traffic is denied between the source and the destination.
 				if cfg.EnableTransparentProxy {
 					k8s.CheckStaticServerConnectionMultipleFailureMessages(t, ctx.KubectlOptions(t), connhelper.StaticClientName, false, []string{"curl: (56) Recv failure: Connection reset by peer", "curl: (52) Empty reply from server"}, "", "http://multiport:8080")
@@ -91,6 +98,8 @@ func TestMeshInject_MultiportService(t *testing.T) {
 				})
 			}
 
+			// TODO: consider adding a trafficpermission to a particular port and validate
+
 			// Check connection from static-client to multiport.
 			if cfg.EnableTransparentProxy {
 				k8s.CheckStaticServerConnectionSuccessful(t, ctx.KubectlOptions(t), connhelper.StaticClientName, "http://multiport:8080")

charts/consul/templates/server-acl-init-job.yaml

@@ -192,6 +192,10 @@ spec:
             {{- else }}
             -secrets-backend=kubernetes \
             {{- end }}
+          
+            {{- if (mustHas "resource-apis" .Values.global.experiments) }}
+            -enable-resource-apis=true \
+            {{- end }}
 
             {{- if .Values.global.acls.bootstrapToken.secretName }}
             -bootstrap-token-secret-name={{ .Values.global.acls.bootstrapToken.secretName }} \

charts/consul/test/unit/server-acl-init-job.bats

@@ -2411,3 +2411,36 @@ load _helpers
      yq -r '.spec.template.metadata.annotations["argocd.argoproj.io/hook-delete-policy"]' | tee /dev/stderr)
   [ "${actual}" = null ]
 }
+
+#--------------------------------------------------------------------
+# resource-apis
+
+@test "serverACLInit/Job: resource-apis is not set 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-resource-apis"))' | tee /dev/stderr)
+  [ "${actual}" = "false" ]
+}
+
+@test "serverACLInit/Job: -enable-resource-apis=true is set when global.experiments contains [\"resource-apis\"] " {
+  cd `chart_dir`
+  local object=$(helm template \
+      -s templates/server-acl-init-job.yaml  \
+      --set 'global.acls.manageSystemACLs=true' \
+      --set 'global.tls.enabled=true' \
+      --set 'connectInject.enabled=true' \
+      --set 'global.experiments[0]=resource-apis' \
+      --set 'ui.enabled=false' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.containers[0].command' | tee /dev/stderr)
+
+  local actual=$(echo $object |
+    yq 'any(contains("-enable-resource-apis=true"))' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}

control-plane/connect-inject/controllers/pod/pod_controller.go

@@ -10,6 +10,7 @@ import (
 	"strconv"
 
 	"github.com/go-logr/logr"
+	"github.com/hashicorp/consul/api"
 	pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1"
 	pbmesh "github.com/hashicorp/consul/proto-public/pbmesh/v2beta1"
 	"github.com/hashicorp/consul/proto-public/pbresource"
@@ -97,6 +98,12 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	}
 	r.ResourceClient = rc
 
+	apiClient, err := consul.NewClientFromConnMgr(r.ConsulClientConfig, r.ConsulServerConnMgr)
+	if err != nil {
+		r.Log.Error(err, "failed to create Consul API client", "name", req.Name)
+		return ctrl.Result{}, err
+	}
+
 	err = r.Client.Get(ctx, req.NamespacedName, &pod)
 
 	// If the pod object has been deleted (and we get an IsNotFound error),
@@ -108,8 +115,6 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 			errs = multierror.Append(errs, err)
 		}
 
-		// TODO: clean up ACL Tokens
-
 		// Delete destinations, if any exist
 		if err := r.deleteDestinations(ctx, req.NamespacedName); err != nil {
 			errs = multierror.Append(errs, err)
@@ -119,6 +124,15 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 			errs = multierror.Append(errs, err)
 		}
 
+		if r.AuthMethod != "" {
+			r.Log.Info("deleting ACL tokens for pod", "name", req.Name, "ns", req.Namespace)
+			err := r.deleteACLTokensForPod(apiClient, req.NamespacedName)
+			if err != nil {
+				r.Log.Error(err, "failed to delete ACL tokens for pod", "name", req.Name, "ns", req.Namespace)
+				errs = multierror.Append(errs, err)
+			}
+		}
+
 		return ctrl.Result{}, errs
 	} else if err != nil {
 		r.Log.Error(err, "failed to get Pod", "name", req.Name, "ns", req.Namespace)
@@ -205,6 +219,39 @@ func (r *Controller) deleteProxyConfiguration(ctx context.Context, pod types.Nam
 	return err
 }
 
+// deleteACLTokensForPod finds the ACL tokens that belongs to the pod and delete them from Consul.
+// It will only check for ACL tokens that have been created with the auth method this controller
+// has been configured with and will only delete tokens for the provided pod Name.
+func (r *Controller) deleteACLTokensForPod(apiClient *api.Client, pod types.NamespacedName) error {
+	// Skip if name is empty.
+	if pod.Name == "" {
+		return nil
+	}
+
+	// This filter matches the metadata we add as part of the dataplane injection for the v2 webhook.
+	filter := fmt.Sprintf("Meta[%q] == %q ", "pod", pod.String())
+
+	tokens, _, err := apiClient.ACL().TokenListFiltered(
+		api.ACLTokenFilterOptions{
+			AuthMethod: r.AuthMethod,
+		},
+		&api.QueryOptions{
+			Filter:    filter,
+			Namespace: r.getConsulNamespace(pod.Namespace),
+		})
+	if err != nil {
+		return fmt.Errorf("failed to get a list of tokens from Consul: %s", err)
+	}
+
+	for _, token := range tokens {
+		r.Log.Info("deleting ACL token", "name", pod.Name, "namespace", pod.Namespace, "ID", token.AccessorID)
+		if _, err := apiClient.ACL().TokenDelete(token.AccessorID, &api.WriteOptions{Namespace: r.getConsulNamespace(pod.Namespace)}); err != nil {
+			return fmt.Errorf("failed to delete token from Consul: %s", err)
+		}
+	}
+	return nil
+}
+
 func (r *Controller) writeWorkload(ctx context.Context, pod corev1.Pod) error {
 
 	// TODO: we should add some validation on the required fields here

control-plane/connect-inject/controllers/serviceaccount/serviceaccount_controller.go

@@ -23,6 +23,10 @@ import (
 	"github.com/hashicorp/consul-k8s/control-plane/namespaces"
 )
 
+const (
+	defaultServiceAccountName = "default"
+)
+
 type Controller struct {
 	client.Client
 	// ConsulServerConnMgr is the watcher for the Consul server addresses used to create Consul API v2 clients.
@@ -65,6 +69,12 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, err
 	}
 
+	// We don't allow the default service account synced to prevent unintended TrafficPermissions
+	if req.Name == defaultServiceAccountName {
+		r.Log.Info("Not syncing default Kubernetes service account", "namespace", req.Namespace)
+		return ctrl.Result{}, nil
+	}
+
 	// If the ServiceAccount object has been deleted (and we get an IsNotFound error),
 	// we need to deregister that WorkloadIdentity from Consul.
 	err = r.Client.Get(ctx, req.NamespacedName, &serviceAccount)

control-plane/connect-inject/controllers/serviceaccount/serviceaccount_controller_test.go

@@ -53,26 +53,11 @@ func TestReconcile_CreateWorkloadIdentity(t *testing.T) {
 	t.Parallel()
 	cases := []reconcileCase{
 		{
-			name:           "Default ServiceAccount",
+			name:           "Default ServiceAccount not synced",
 			svcAccountName: "default",
 			k8sObjects: func() []runtime.Object {
 				return []runtime.Object{createServiceAccount("default", "default")}
 			},
-			expectedResource: &pbresource.Resource{
-				Id: &pbresource.ID{
-					Name: "default",
-					Type: pbauth.WorkloadIdentityType,
-					Tenancy: &pbresource.Tenancy{
-						Namespace: constants.DefaultConsulNS,
-						Partition: constants.DefaultConsulPartition,
-					},
-				},
-				Data: getWorkloadIdentityData(),
-				Metadata: map[string]string{
-					constants.MetaKeyKubeNS:    constants.DefaultConsulNS,
-					constants.MetaKeyManagedBy: constants.ManagedByServiceAccountValue,
-				},
-			},
 		},
 		{
 			name:           "Custom ServiceAccount",

control-plane/connect-inject/webhookv2/consul_dataplane_sidecar.go

@@ -112,12 +112,6 @@ func (w *MeshWebhook) consulDataplaneSidecar(namespace corev1.Namespace, pod cor
 				Name:  "DP_CREDENTIAL_LOGIN_META",
 				Value: "pod=$(POD_NAMESPACE)/$(POD_NAME)",
 			},
-			// This entry exists to support certain versions of consul dataplane, where environment variable entries
-			// utilize this numbered notation to indicate individual KV pairs in a map.
-			{
-				Name:  "DP_CREDENTIAL_LOGIN_META1",
-				Value: "pod=$(POD_NAMESPACE)/$(POD_NAME)",
-			},
 		},
 		VolumeMounts: []corev1.VolumeMount{
 			{

control-plane/go.mod

@@ -18,7 +18,7 @@ require (
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20230825213844-4ea04860c5ed
 	github.com/hashicorp/consul-server-connection-manager v0.1.4
-	github.com/hashicorp/consul/api v1.10.1-0.20230914174054-e5808d85f751
+	github.com/hashicorp/consul/api v1.10.1-0.20231011171434-ca1a755f0c5a
 	github.com/hashicorp/consul/proto-public v0.4.1
 	github.com/hashicorp/consul/sdk v0.14.1
 	github.com/hashicorp/go-bexpr v0.1.11

control-plane/go.sum

@@ -263,8 +263,8 @@ github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20230825213844-4ea04860
 github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20230825213844-4ea04860c5ed/go.mod h1:mwODEC+VTCA1LY/m2RUG4S2c5lNRvBcsvqaMJtMLLos=
 github.com/hashicorp/consul-server-connection-manager v0.1.4 h1:wrcSRV6WGXFBNpNbN6XsdoGgBOyso7ZbN5VaWPEX1jY=
 github.com/hashicorp/consul-server-connection-manager v0.1.4/go.mod h1:LMqHkALoLP0HUQKOG21xXYr0YPUayIQIHNTlmxG100E=
-github.com/hashicorp/consul/api v1.10.1-0.20230914174054-e5808d85f751 h1:LnzgDq4e7ZfM1+XS6S21B9taQrbfdydXenL1xHyG1PQ=
-github.com/hashicorp/consul/api v1.10.1-0.20230914174054-e5808d85f751/go.mod h1:/Fz5sgOC0a5XY0BmPGj7aDSZRNgySLm02lV4xkU4DS4=
+github.com/hashicorp/consul/api v1.10.1-0.20231011171434-ca1a755f0c5a h1:+cjavDME42W6nzw+xyCQ+eczqTFA+Qk4SLl7BHZ2dxI=
+github.com/hashicorp/consul/api v1.10.1-0.20231011171434-ca1a755f0c5a/go.mod h1:+pNEP6hQgkrBLjQlYLI13/tyyb1GK3MGVw1PC/IHk9M=
 github.com/hashicorp/consul/proto-public v0.1.2-0.20230929231147-632fd65c091c h1:d1ULTfDs6Hha01yfITC55MPGIsQpv0VqQfS45WZHJiY=
 github.com/hashicorp/consul/proto-public v0.1.2-0.20230929231147-632fd65c091c/go.mod h1:KAOxsaELPpA7JX10kMeygAskAqsQnu3SPgeruMhYZMU=
 github.com/hashicorp/consul/sdk v0.4.1-0.20231011203909-c26d5cf62cb9 h1:j0Rvt1KiKIKlMc2UnA0ilHfIGo66SzrBztGZaCou3+w=

control-plane/subcommand/inject-connect/command.go

@@ -232,7 +232,7 @@ func (c *Command) init() {
 	c.flagSet.BoolVar(&c.flagLogJSON, "log-json", false,
 		"Enable or disable JSON output format for logging.")
 	c.flagSet.BoolVar(&c.flagResourceAPIs, "enable-resource-apis", false,
-		"Enable of disable Consul V2 Resource APIs.")
+		"Enable or disable Consul V2 Resource APIs.")
 
 	// Proxy sidecar resource setting flags.
 	c.flagSet.StringVar(&c.flagDefaultSidecarProxyCPURequest, "default-sidecar-proxy-cpu-request", "", "Default sidecar proxy CPU request.")

control-plane/subcommand/server-acl-init/command.go

@@ -44,6 +44,8 @@ type Command struct {
 	flagResourcePrefix string
 	flagK8sNamespace   string
 
+	flagResourceAPIs bool // Use V2 APIs
+
 	flagAllowDNS bool
 
 	flagSetServerTokens bool
@@ -131,6 +133,9 @@ func (c *Command) init() {
 
 	c.flags.BoolVar(&c.flagSetServerTokens, "set-server-tokens", true, "Toggle for setting agent tokens for the servers.")
 
+	c.flags.BoolVar(&c.flagResourceAPIs, "enable-resource-apis", false,
+		"Enable or disable Consul V2 Resource APIs. This will affect the binding rule used for Kubernetes auth (Service vs. WorkloadIdentity)")
+
 	c.flags.BoolVar(&c.flagAllowDNS, "allow-dns", false,
 		"Toggle for updating the anonymous token to allow DNS queries to work")
 	c.flags.BoolVar(&c.flagClient, "client", true,