Skip to content

Commit 4008908

Browse files
committed
fix: Remove labels that aren't referenced by explicit fields.
1 parent 99a1d06 commit 4008908

3 files changed

Lines changed: 272 additions & 0 deletions

File tree

‎pkg/admin/local.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ func (l *adminService) CreateBridge(ctx context.Context, req *bridgev1.CreateBri
112112
resources.SetNamespace(targetNS),
113113
resources.PruneAllMetadata(),
114114
resources.StripOrphanedVolumes(),
115+
resources.StripUnreferencedLabels(),
115116
resources.InjectProxyImage(proxyImage),
116117
resources.ClearClusterIPs(),
117118
resources.SuffixNames(suffix),

‎pkg/k8s/resources/manifests_test.go‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ func testCreateFromManifests(t *testing.T, client *k8stesting.Clientset, sourceM
5858

5959
transforms := []Transformer{
6060
PruneAllMetadata(),
61+
StripUnreferencedLabels(),
6162
InjectProxyImage(testProxyImage),
6263
SuffixNames(testSuffix),
6364
InjectLabels(),
@@ -487,6 +488,7 @@ data:
487488

488489
transforms := []Transformer{
489490
PruneAllMetadata(),
491+
StripUnreferencedLabels(),
490492
InjectProxyImage(testProxyImage),
491493
SuffixNames(testSuffix),
492494
InjectLabels(),
@@ -580,6 +582,197 @@ data:
580582
assert.Equal(t, "bridge-value", bridgeCM.Data["DB_HOST"])
581583
}
582584

585+
func TestStripUnreferencedLabels_RemovesSourceLabels(t *testing.T) {
586+
manifests := packTestManifests(t, map[string]string{
587+
"deploy.yaml": `apiVersion: apps/v1
588+
kind: Deployment
589+
metadata:
590+
name: my-app
591+
labels:
592+
app: my-app
593+
version: v1
594+
spec:
595+
selector:
596+
matchLabels:
597+
app: my-app
598+
template:
599+
metadata:
600+
labels:
601+
app: my-app
602+
version: v1
603+
spec:
604+
containers:
605+
- name: app
606+
image: my-app:latest
607+
ports:
608+
- containerPort: 3000
609+
`,
610+
})
611+
612+
client := k8stesting.NewSimpleClientset()
613+
deployName, _, err := testCreateFromManifests(t, client, manifests, "default")
614+
require.NoError(t, err)
615+
616+
deploy, err := client.AppsV1().Deployments("default").Get(context.Background(), deployName, metav1.GetOptions{})
617+
require.NoError(t, err)
618+
619+
// Source labels (app, version) should be stripped.
620+
assert.Empty(t, deploy.Labels["app"], "source label 'app' should be stripped from deployment metadata")
621+
assert.Empty(t, deploy.Labels["version"], "source label 'version' should be stripped from deployment metadata")
622+
623+
// Bridge labels should still be present (injected after strip).
624+
assert.Equal(t, meta.BridgeTypeProxy, deploy.Labels[meta.LabelBridgeType])
625+
}
626+
627+
func TestStripUnreferencedLabels_PreservesFieldRefLabels(t *testing.T) {
628+
manifests := packTestManifests(t, map[string]string{
629+
"deploy.yaml": `apiVersion: apps/v1
630+
kind: Deployment
631+
metadata:
632+
name: my-app
633+
labels:
634+
app: my-app
635+
version: v1
636+
region: us-east-1
637+
spec:
638+
selector:
639+
matchLabels:
640+
app: my-app
641+
template:
642+
metadata:
643+
labels:
644+
app: my-app
645+
version: v1
646+
region: us-east-1
647+
spec:
648+
containers:
649+
- name: app
650+
image: my-app:latest
651+
ports:
652+
- containerPort: 3000
653+
env:
654+
- name: APP_VERSION
655+
valueFrom:
656+
fieldRef:
657+
fieldPath: metadata.labels['version']
658+
- name: REGION
659+
valueFrom:
660+
fieldRef:
661+
fieldPath: metadata.labels['region']
662+
`,
663+
})
664+
665+
client := k8stesting.NewSimpleClientset()
666+
deployName, _, err := testCreateFromManifests(t, client, manifests, "default")
667+
require.NoError(t, err)
668+
669+
deploy, err := client.AppsV1().Deployments("default").Get(context.Background(), deployName, metav1.GetOptions{})
670+
require.NoError(t, err)
671+
672+
// Labels referenced by fieldRef should be preserved.
673+
assert.Equal(t, "v1", deploy.Labels["version"], "fieldRef-referenced label 'version' should be preserved")
674+
assert.Equal(t, "us-east-1", deploy.Labels["region"], "fieldRef-referenced label 'region' should be preserved")
675+
676+
// Non-referenced source labels should be stripped.
677+
assert.Empty(t, deploy.Labels["app"], "non-referenced label 'app' should be stripped")
678+
679+
// Pod template labels should also preserve fieldRef-referenced labels.
680+
podLabels := deploy.Spec.Template.Labels
681+
assert.Equal(t, "v1", podLabels["version"])
682+
assert.Equal(t, "us-east-1", podLabels["region"])
683+
}
684+
685+
func TestStripUnreferencedLabels_InitContainerFieldRef(t *testing.T) {
686+
manifests := packTestManifests(t, map[string]string{
687+
"deploy.yaml": `apiVersion: apps/v1
688+
kind: Deployment
689+
metadata:
690+
name: my-app
691+
labels:
692+
app: my-app
693+
tier: backend
694+
spec:
695+
selector:
696+
matchLabels:
697+
app: my-app
698+
template:
699+
metadata:
700+
labels:
701+
app: my-app
702+
tier: backend
703+
spec:
704+
initContainers:
705+
- name: init
706+
image: init:latest
707+
env:
708+
- name: TIER
709+
valueFrom:
710+
fieldRef:
711+
fieldPath: metadata.labels['tier']
712+
containers:
713+
- name: app
714+
image: my-app:latest
715+
ports:
716+
- containerPort: 3000
717+
`,
718+
})
719+
720+
client := k8stesting.NewSimpleClientset()
721+
deployName, _, err := testCreateFromManifests(t, client, manifests, "default")
722+
require.NoError(t, err)
723+
724+
deploy, err := client.AppsV1().Deployments("default").Get(context.Background(), deployName, metav1.GetOptions{})
725+
require.NoError(t, err)
726+
727+
// Label referenced by initContainer fieldRef should be preserved.
728+
assert.Equal(t, "backend", deploy.Labels["tier"], "fieldRef label from initContainer should be preserved")
729+
730+
// Non-referenced label should be stripped.
731+
assert.Empty(t, deploy.Labels["app"])
732+
}
733+
734+
func TestStripUnreferencedLabels_NoFieldRefs(t *testing.T) {
735+
manifests := packTestManifests(t, map[string]string{
736+
"deploy.yaml": `apiVersion: apps/v1
737+
kind: Deployment
738+
metadata:
739+
name: my-app
740+
labels:
741+
app: my-app
742+
team: platform
743+
spec:
744+
selector:
745+
matchLabels:
746+
app: my-app
747+
template:
748+
metadata:
749+
labels:
750+
app: my-app
751+
team: platform
752+
spec:
753+
containers:
754+
- name: app
755+
image: my-app:latest
756+
ports:
757+
- containerPort: 3000
758+
`,
759+
})
760+
761+
client := k8stesting.NewSimpleClientset()
762+
deployName, _, err := testCreateFromManifests(t, client, manifests, "default")
763+
require.NoError(t, err)
764+
765+
deploy, err := client.AppsV1().Deployments("default").Get(context.Background(), deployName, metav1.GetOptions{})
766+
require.NoError(t, err)
767+
768+
// All source labels should be stripped when there are no fieldRefs.
769+
assert.Empty(t, deploy.Labels["app"])
770+
assert.Empty(t, deploy.Labels["team"])
771+
772+
// Bridge labels should still be present.
773+
assert.Equal(t, meta.BridgeTypeProxy, deploy.Labels[meta.LabelBridgeType])
774+
}
775+
583776
// packTestManifests creates a tar.gz archive from a map of filename to YAML content.
584777
func packTestManifests(t *testing.T, files map[string]string) []byte {
585778
t.Helper()

‎pkg/k8s/resources/transforms.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package resources
22

33
import (
4+
"strings"
5+
46
"github.com/vercel/bridge/pkg/k8s/meta"
57

68
appsv1 "k8s.io/api/apps/v1"
@@ -81,6 +83,82 @@ func filterMounts(mounts []corev1.VolumeMount, validVolumes map[string]bool) []c
8183
return kept
8284
}
8385

86+
// StripUnreferencedLabels returns a Transformer that removes all labels from
87+
// Deployments (both object metadata and pod template metadata) except those
88+
// referenced by fieldRef expressions in the pod spec (e.g. downward API env
89+
// vars like metadata.labels['key']). This prevents existing Services from
90+
// selecting bridge pods via inherited source labels while preserving labels
91+
// required by the pod's runtime configuration.
92+
// Runs before InjectLabels — operates on original source labels.
93+
func StripUnreferencedLabels() Transformer {
94+
return TransformFunc(func(_ *TransformContext, b *Bundle) error {
95+
for _, r := range b.Resources {
96+
deploy, ok := r.Object.(*appsv1.Deployment)
97+
if !ok {
98+
continue
99+
}
100+
referenced := collectFieldRefLabels(&deploy.Spec.Template.Spec)
101+
deploy.Labels = keepOnly(deploy.Labels, referenced)
102+
deploy.Spec.Template.Labels = keepOnly(deploy.Spec.Template.Labels, referenced)
103+
}
104+
return nil
105+
})
106+
}
107+
108+
// collectFieldRefLabels scans all containers in a pod spec for env vars that
109+
// use fieldRef with metadata.labels['...'] and returns the set of label keys.
110+
func collectFieldRefLabels(podSpec *corev1.PodSpec) map[string]bool {
111+
keys := make(map[string]bool)
112+
for _, c := range podSpec.Containers {
113+
addFieldRefLabels(&c, keys)
114+
}
115+
for _, c := range podSpec.InitContainers {
116+
addFieldRefLabels(&c, keys)
117+
}
118+
return keys
119+
}
120+
121+
func addFieldRefLabels(c *corev1.Container, keys map[string]bool) {
122+
for _, env := range c.Env {
123+
if env.ValueFrom == nil || env.ValueFrom.FieldRef == nil {
124+
continue
125+
}
126+
if key, ok := parseLabelFieldPath(env.ValueFrom.FieldRef.FieldPath); ok {
127+
keys[key] = true
128+
}
129+
}
130+
}
131+
132+
// parseLabelFieldPath extracts the label key from a fieldPath like
133+
// metadata.labels['app'] or metadata.labels['app.kubernetes.io/name'].
134+
func parseLabelFieldPath(fieldPath string) (string, bool) {
135+
const prefix = "metadata.labels['"
136+
const suffix = "']"
137+
if strings.HasPrefix(fieldPath, prefix) && strings.HasSuffix(fieldPath, suffix) {
138+
key := fieldPath[len(prefix) : len(fieldPath)-len(suffix)]
139+
return key, true
140+
}
141+
return "", false
142+
}
143+
144+
// keepOnly returns a new map containing only the keys present in the keep set.
145+
// Returns nil if no labels remain or labels is nil.
146+
func keepOnly(labels map[string]string, keep map[string]bool) map[string]string {
147+
if len(labels) == 0 {
148+
return nil
149+
}
150+
filtered := make(map[string]string)
151+
for k, v := range labels {
152+
if keep[k] {
153+
filtered[k] = v
154+
}
155+
}
156+
if len(filtered) == 0 {
157+
return nil
158+
}
159+
return filtered
160+
}
161+
84162
// InjectProxyImage returns a Transformer that finds the target Deployment and
85163
// swaps its first container for the bridge proxy. It also sets replicas to 1.
86164
// Runs before SuffixNames — finds the deployment by tc.SourceName.

0 commit comments

Comments
 (0)