Skip to content

Commit

Permalink
fix(kuma-cp): fix issue with pod labels overriding existing dataplane…
Browse files Browse the repository at this point in the history
… labels (#12589)

Fixes: #12142

## Motivation

We should not override already present dataplane labels with pod labels

<!--
> Changelog: skip
-->
<!--
Uncomment the above section to explicitly set a [`> Changelog:` entry
here](https://github.com/kumahq/kuma/blob/master/CONTRIBUTING.md#submitting-a-patch)?
-->

---------

Signed-off-by: Marcin Skalski <skalskimarcin33@gmail.com>
  • Loading branch information
Automaat authored Jan 17, 2025
1 parent 3171f68 commit 8a00691
Show file tree
Hide file tree
Showing 30 changed files with 342 additions and 10 deletions.
2 changes: 2 additions & 0 deletions pkg/plugins/runtime/k8s/controllers/pod_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,7 @@ var _ = Describe("PodReconciler", func() {
mesh_proto.MeshTag: "poc",
mesh_proto.ResourceOriginLabel: "zone",
mesh_proto.EnvTag: mesh_proto.KubernetesEnvironment,
mesh_proto.ProxyTypeLabel: "sidecar",
},
OwnerReferences: []kube_meta.OwnerReference{
{
Expand Down Expand Up @@ -893,6 +894,7 @@ var _ = Describe("PodReconciler", func() {
labels:
app: sample
k8s.kuma.io/namespace: demo
kuma.io/display-name: pod-with-custom-admin-port
kuma.io/env: kubernetes
kuma.io/mesh: poc
kuma.io/origin: zone
Expand Down
20 changes: 17 additions & 3 deletions pkg/plugins/runtime/k8s/controllers/pod_converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package controllers
import (
"context"
"fmt"
"maps"
"reflect"
"regexp"

Expand Down Expand Up @@ -63,7 +64,7 @@ func (p *PodConverter) PodToDataplane(
labels, err := model.ComputeLabels(
core_mesh.DataplaneResourceTypeDescriptor,
currentSpec,
pod.Labels,
mergeLabels(dataplane.GetLabels(), pod.Labels),
model.NewNamespace(pod.Namespace, pod.Namespace == p.SystemNamespace),
dataplane.Mesh,
p.Mode,
Expand Down Expand Up @@ -103,7 +104,7 @@ func (p *PodConverter) PodToIngress(ctx context.Context, zoneIngress *mesh_k8s.Z
labels, err := model.ComputeLabels(
core_mesh.ZoneIngressResourceTypeDescriptor,
currentSpec,
pod.Labels,
mergeLabels(zoneIngress.GetLabels(), pod.Labels),
model.NewNamespace(pod.Namespace, pod.Namespace == p.SystemNamespace),
model.NoMesh,
p.Mode,
Expand All @@ -119,6 +120,7 @@ func (p *PodConverter) PodToIngress(ctx context.Context, zoneIngress *mesh_k8s.Z
return nil
}
zoneIngress.SetSpec(zoneIngressRes.Spec)
zoneIngress.SetLabels(labels)
return nil
}

Expand All @@ -142,7 +144,7 @@ func (p *PodConverter) PodToEgress(ctx context.Context, zoneEgress *mesh_k8s.Zon
labels, err := model.ComputeLabels(
core_mesh.ZoneEgressResourceTypeDescriptor,
currentSpec,
pod.Labels,
mergeLabels(zoneEgress.GetLabels(), pod.Labels),
model.NewNamespace(pod.Namespace, pod.Namespace == p.SystemNamespace),
model.NoMesh,
p.Mode,
Expand All @@ -158,6 +160,7 @@ func (p *PodConverter) PodToEgress(ctx context.Context, zoneEgress *mesh_k8s.Zon
}

zoneEgress.SetSpec(zoneEgressRes.Spec)
zoneEgress.SetLabels(labels)
return nil
}

Expand Down Expand Up @@ -411,6 +414,17 @@ func MetricsAggregateFor(pod *kube_core.Pod) ([]*mesh_proto.PrometheusAggregateM
return aggregateConfig, nil
}

func mergeLabels(existingLabels map[string]string, podLabels map[string]string) map[string]string {
mergedLabels := map[string]string{}
if existingLabels != nil {
mergedLabels = maps.Clone(existingLabels)
}
for k, v := range podLabels {
mergedLabels[k] = v
}
return mergedLabels
}

type ReachableBackendRefs struct {
Refs []*ReachableBackendRef `json:"refs,omitempty"`
}
Expand Down
49 changes: 45 additions & 4 deletions pkg/plugins/runtime/k8s/controllers/pod_converter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,15 @@ var _ = Describe("PodToDataplane(..)", func() {
Expect(err).ToNot(HaveOccurred())
}

// existing dataplane
existingDataplane := &mesh_k8s.Dataplane{}
if given.existingDataplane != "" {
bytes, err := os.ReadFile(filepath.Join("testdata", given.existingDataplane))
Expect(err).ToNot(HaveOccurred())
err = yaml.Unmarshal(bytes, existingDataplane)
Expect(err).ToNot(HaveOccurred())
}

converter := PodConverter{
ServiceGetter: serviceGetter,
InboundConverter: InboundConverter{
Expand All @@ -135,13 +144,12 @@ var _ = Describe("PodToDataplane(..)", func() {
}

// when
dataplane := &mesh_k8s.Dataplane{}
err = converter.PodToDataplane(context.Background(), dataplane, pod, &namespace, services, otherDataplanes)
err = converter.PodToDataplane(context.Background(), existingDataplane, pod, &namespace, services, otherDataplanes)

// then
Expect(err).ToNot(HaveOccurred())

actual, err := yaml.Marshal(dataplane)
actual, err := yaml.Marshal(existingDataplane)
Expect(err).ToNot(HaveOccurred())
Expect(actual).To(MatchGoldenYAML("testdata", given.dataplane))
},
Expand Down Expand Up @@ -312,6 +320,13 @@ var _ = Describe("PodToDataplane(..)", func() {
servicesForPod: "30.services-for-pod.yaml",
dataplane: "30.dataplane.yaml",
}),
Entry("Update existing dataplane with pod labels", testCase{
pod: "update-dataplane.pod.yaml",
servicesForPod: "update-dataplane.services-for-pod.yaml",
existingDataplane: "update-dataplane.existing-dataplane.yaml",
otherServices: "update-dataplane.other-services.yaml",
dataplane: "update-dataplane.dataplane.yaml",
}),
)

DescribeTable("should convert Ingress Pod into an Ingress Dataplane YAML version",
Expand Down Expand Up @@ -404,6 +419,18 @@ var _ = Describe("PodToDataplane(..)", func() {
existingDataplane: "ingress-exists.existing-dataplane.yaml",
dataplane: "ingress-exists.dataplane.yaml",
}),
Entry("Existing ZoneIngress with load balancer and ip should not be updated when no change", testCase{
pod: "ingress-exists.pod.yaml",
servicesForPod: "ingress-exists.services-for-pod.yaml",
existingDataplane: "ingress-exists.existing-dataplane.yaml",
dataplane: "ingress-exists.dataplane.yaml",
}),
Entry("Existing ZoneIngress should be updated when pod labels changes", testCase{
pod: "ingress-exists-labels.pod.yaml",
servicesForPod: "ingress-exists-labels.services-for-pod.yaml",
existingDataplane: "ingress-exists-labels.existing-dataplane.yaml",
dataplane: "ingress-exists-labels.dataplane.yaml",
}),
)

DescribeTable("should convert Egress Pod into an Egress Dataplane YAML version",
Expand Down Expand Up @@ -442,8 +469,15 @@ var _ = Describe("PodToDataplane(..)", func() {
},
}

// when
egress := &mesh_k8s.ZoneEgress{}
if given.existingDataplane != "" {
bytes, err = os.ReadFile(filepath.Join("testdata", "egress", given.existingDataplane))
Expect(err).ToNot(HaveOccurred())
err = yaml.Unmarshal(bytes, egress)
Expect(err).ToNot(HaveOccurred())
}

// when
err = converter.PodToEgress(ctx, egress, pod, services)

// then
Expand Down Expand Up @@ -480,6 +514,13 @@ var _ = Describe("PodToDataplane(..)", func() {
dataplane: "05.dataplane.yaml",
node: "05.node.yaml",
}),
Entry("Existing ZoneEgress should be updated when pod labels changes", testCase{ // KIND / Minikube use case
pod: "egress-exists-labels.pod.yaml",
servicesForPod: "egress-exists-labels.services-for-pod.yaml",
dataplane: "egress-exists-labels.dataplane.yaml",
node: "egress-exists-labels.node.yaml",
existingDataplane: "egress-exists-labels.existing-dataplane.yaml",
}),
)
})

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
metadata:
creationTimestamp: null
labels:
app: kuma-egress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneegress
spec:
networking:
address: 192.168.0.1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
metadata:
creationTimestamp: null
labels:
app: kuma-egress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneegress
spec:
networking:
address: 192.168.0.1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
metadata:
creationTimestamp: null
labels:
app: kuma-egress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneegress
spec:
networking:
address: 192.168.0.1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
metadata:
creationTimestamp: null
labels:
app: kuma-egress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneegress
spec:
networking:
address: 192.168.0.1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
metadata:
creationTimestamp: null
labels:
app: kuma-egress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneegress
spec:
networking:
address: 192.168.0.1
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
metadata:
creationTimestamp: null
labels:
app: kuma-egress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneegress
test: new
spec:
networking:
address: 192.168.0.1
port: 10002
zone: zone-1
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
metadata:
creationTimestamp: null
labels:
app: kuma-egress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneegress
test: old
spec:
networking:
address: 192.168.0.1
port: 10002
zone: zone-1
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: v1
kind: Node
metadata:
name: sample-node
status:
addresses:
- address: 10.128.15.193
type: InternalIP
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
metadata:
namespace: kuma-system
name: kuma-egress
labels:
app: kuma-egress
test: new
annotations:
kuma.io/egress: enabled
spec:
containers:
- ports:
- containerPort: 10002
status:
podIP: 192.168.0.1
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
metadata:
namespace: kuma-system
name: kuma-egress
spec:
type: NodePort
ports:
- port: 10002
targetPort: 10002
nodePort: 12345
selector:
app: kuma-egress
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
metadata:
creationTimestamp: null
labels:
app: kuma-ingress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneingress
spec:
networking:
address: 192.168.0.1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
metadata:
creationTimestamp: null
labels:
app: kuma-ingress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneingress
spec:
networking:
address: 192.168.0.1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
metadata:
creationTimestamp: null
labels:
app: kuma-ingress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneingress
spec:
networking:
address: 192.168.0.1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
metadata:
creationTimestamp: null
labels:
app: kuma-ingress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneingress
spec:
networking:
address: 192.168.0.1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
metadata:
creationTimestamp: null
labels:
app: kuma-ingress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneingress
spec:
networking:
address: 192.168.0.1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
metadata:
creationTimestamp: null
labels:
app: kuma-ingress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneingress
spec:
networking:
address: 192.168.0.1
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
metadata:
creationTimestamp: null
labels:
app: kuma-ingress
k8s.kuma.io/namespace: kuma-system
kuma.io/proxy-type: zoneingress
test: new
spec:
availableServices:
- instances: 3
mesh: mesh-1
tags:
kuma.io/protocol: http
kuma.io/service: service-1-zone-2
networking:
address: 192.168.0.1
advertisedAddress: 192.168.100.1
advertisedPort: 10001
port: 10001
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
metadata:
creationTimestamp: null
labels:
test: old
spec:
availableServices:
- instances: 3
mesh: mesh-1
tags:
kuma.io/protocol: http
kuma.io/service: service-1-zone-2
networking:
address: 192.168.0.1
advertisedAddress: 192.168.100.1
advertisedPort: 10001
port: 10001
Loading

0 comments on commit 8a00691

Please sign in to comment.