Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of NET-4806: Fix ACL tokens for pods don't have pod name set into release/1.0.x #2821

Merged
merged 4 commits into from
Aug 22, 2023
Merged

Backport of NET-4806: Fix ACL tokens for pods don't have pod name set into release/1.0.x #2821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c77a005
backport of commit 950e96b724276a0b610c61b554b7144da26a9256
hashi-derek Aug 22, 2023
d0cc8ba
backport of commit 354b985a00c318b94bc5289b77b4707af0eca10a
hashi-derek Aug 22, 2023
f7d8fb2
backport of commit 2734b31ebb6c27f0989926aced7d9aa736874ac5
hashi-derek Aug 22, 2023
2422e38
backport of commit 58b159ab53f14131d3b70b75760130b8920ecc77
hashi-derek Aug 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/2808.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
control-plane: Fix issue where ACL tokens would have an empty pod name that prevented proper token cleanup.
```
12 changes: 12 additions & 0 deletions acceptance/tests/connect/connect_inject_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,18 @@ func TestConnectInject_CleanupKilledPods(t *testing.T) {
}
}
})
// Ensure the token is cleaned up
if secure {
retry.Run(t, func(r *retry.R) {
tokens, _, err := consulClient.ACL().TokenList(nil)
require.NoError(r, err)
for _, t := range tokens {
if strings.Contains(t.Description, podName) {
r.Errorf("Found a token that was supposed to be deleted for pod %v", podName)
}
}
})
}
})
}
}
Expand Down
25 changes: 24 additions & 1 deletion control-plane/connect-inject/webhook/consul_dataplane_sidecar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,29 @@ func (w *MeshWebhook) consulDataplaneSidecar(namespace corev1.Namespace, pod cor
Name: "DP_SERVICE_NODE_NAME",
Value: "$(NODE_NAME)-virtual",
},
// The pod name isn't known currently, so we must rely on the environment variable to fill it in rather than using args.
{
Name: "POD_NAME",
ValueFrom: &corev1.EnvVarSource{
FieldRef: &corev1.ObjectFieldSelector{FieldPath: "metadata.name"},
},
},
{
Name: "POD_NAMESPACE",
ValueFrom: &corev1.EnvVarSource{
FieldRef: &corev1.ObjectFieldSelector{FieldPath: "metadata.namespace"},
},
},
{
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{
{
Expand Down Expand Up @@ -205,7 +228,7 @@ func (w *MeshWebhook) getContainerSidecarArgs(namespace corev1.Namespace, mpi mu
"-credential-type=login",
"-login-auth-method="+w.AuthMethod,
"-login-bearer-token-path="+bearerTokenFile,
"-login-meta="+fmt.Sprintf("pod=%s/%s", namespace.Name, pod.Name),
// We don't know the pod name at this time, so we must use environment variables to populate the login-meta instead.
)
if w.EnableNamespaces {
if w.EnableK8SNSMirroring {
Expand Down
20 changes: 13 additions & 7 deletions control-plane/connect-inject/webhook/consul_dataplane_sidecar_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestHandlerConsulDataplaneSidecar(t *testing.T) {
w.AuthMethod = "test-auth-method"
},
additionalExpCmdArgs: " -credential-type=login -login-auth-method=test-auth-method -login-bearer-token-path=/var/run/secrets/kubernetes.io/serviceaccount/token " +
"-login-meta=pod=k8snamespace/test-pod -tls-disabled -graceful-port=20600 -telemetry-prom-scrape-path=/metrics",
"-tls-disabled -graceful-port=20600 -telemetry-prom-scrape-path=/metrics",
},
"with ACLs and namespace mirroring": {
webhookSetupFunc: func(w *MeshWebhook) {
Expand All @@ -48,7 +48,7 @@ func TestHandlerConsulDataplaneSidecar(t *testing.T) {
w.EnableK8SNSMirroring = true
},
additionalExpCmdArgs: " -credential-type=login -login-auth-method=test-auth-method -login-bearer-token-path=/var/run/secrets/kubernetes.io/serviceaccount/token " +
"-login-meta=pod=k8snamespace/test-pod -login-namespace=default -service-namespace=k8snamespace -tls-disabled -graceful-port=20600 -telemetry-prom-scrape-path=/metrics",
"-login-namespace=default -service-namespace=k8snamespace -tls-disabled -graceful-port=20600 -telemetry-prom-scrape-path=/metrics",
},
"with ACLs and single destination namespace": {
webhookSetupFunc: func(w *MeshWebhook) {
Expand All @@ -57,15 +57,15 @@ func TestHandlerConsulDataplaneSidecar(t *testing.T) {
w.ConsulDestinationNamespace = "test-ns"
},
additionalExpCmdArgs: " -credential-type=login -login-auth-method=test-auth-method -login-bearer-token-path=/var/run/secrets/kubernetes.io/serviceaccount/token " +
"-login-meta=pod=k8snamespace/test-pod -login-namespace=test-ns -service-namespace=test-ns -tls-disabled -graceful-port=20600 -telemetry-prom-scrape-path=/metrics",
"-login-namespace=test-ns -service-namespace=test-ns -tls-disabled -graceful-port=20600 -telemetry-prom-scrape-path=/metrics",
},
"with ACLs and partitions": {
webhookSetupFunc: func(w *MeshWebhook) {
w.AuthMethod = "test-auth-method"
w.ConsulPartition = "test-part"
},
additionalExpCmdArgs: " -credential-type=login -login-auth-method=test-auth-method -login-bearer-token-path=/var/run/secrets/kubernetes.io/serviceaccount/token " +
"-login-meta=pod=k8snamespace/test-pod -login-partition=test-part -service-partition=test-part -tls-disabled -graceful-port=20600 -telemetry-prom-scrape-path=/metrics",
"-login-partition=test-part -service-partition=test-part -tls-disabled -graceful-port=20600 -telemetry-prom-scrape-path=/metrics",
},
"with TLS and CA cert provided": {
webhookSetupFunc: func(w *MeshWebhook) {
Expand Down Expand Up @@ -216,11 +216,17 @@ func TestHandlerConsulDataplaneSidecar(t *testing.T) {
}
require.Equal(t, expectedProbe, container.ReadinessProbe)
require.Nil(t, container.StartupProbe)
require.Len(t, container.Env, 3)
require.Len(t, container.Env, 7)
require.Equal(t, container.Env[0].Name, "TMPDIR")
require.Equal(t, container.Env[0].Value, "/consul/connect-inject")
require.Equal(t, container.Env[2].Name, "DP_SERVICE_NODE_NAME")
require.Equal(t, container.Env[2].Value, "$(NODE_NAME)-virtual")
require.Equal(t, container.Env[3].Name, "POD_NAME")
require.Equal(t, container.Env[4].Name, "POD_NAMESPACE")
require.Equal(t, container.Env[5].Name, "DP_CREDENTIAL_LOGIN_META")
require.Equal(t, container.Env[5].Value, "pod=$(POD_NAMESPACE)/$(POD_NAME)")
require.Equal(t, container.Env[6].Name, "DP_CREDENTIAL_LOGIN_META1")
require.Equal(t, container.Env[6].Value, "pod=$(POD_NAMESPACE)/$(POD_NAME)")
})
}
}
Expand Down Expand Up @@ -628,10 +634,10 @@ func TestHandlerConsulDataplaneSidecar_Multiport(t *testing.T) {
expArgs = []string{
"-addresses 1.1.1.1 -grpc-port=8502 -proxy-service-id-path=/consul/connect-inject/proxyid-web " +
"-log-level=info -log-json=false -envoy-concurrency=0 -credential-type=login -login-auth-method=test-auth-method " +
"-login-bearer-token-path=/var/run/secrets/kubernetes.io/serviceaccount/token -login-meta=pod=k8snamespace/test-pod -tls-disabled -envoy-admin-bind-port=19000 -graceful-port=20600 -telemetry-prom-scrape-path=/metrics -- --base-id 0",
"-login-bearer-token-path=/var/run/secrets/kubernetes.io/serviceaccount/token -tls-disabled -envoy-admin-bind-port=19000 -graceful-port=20600 -telemetry-prom-scrape-path=/metrics -- --base-id 0",
"-addresses 1.1.1.1 -grpc-port=8502 -proxy-service-id-path=/consul/connect-inject/proxyid-web-admin " +
"-log-level=info -log-json=false -envoy-concurrency=0 -credential-type=login -login-auth-method=test-auth-method " +
"-login-bearer-token-path=/consul/serviceaccount-web-admin/token -login-meta=pod=k8snamespace/test-pod -tls-disabled -envoy-admin-bind-port=19001 -graceful-port=20601 -telemetry-prom-scrape-path=/metrics -- --base-id 1",
"-login-bearer-token-path=/consul/serviceaccount-web-admin/token -tls-disabled -envoy-admin-bind-port=19001 -graceful-port=20601 -telemetry-prom-scrape-path=/metrics -- --base-id 1",
}
}
expSAVolumeMounts := []corev1.VolumeMount{
Expand Down