Skip to content

Commit 4a1f037

Browse files
test: handle some labels
Signed-off-by: Jintao Zhang <[email protected]>
1 parent e26ec00 commit 4a1f037

File tree

3 files changed

+100
-67
lines changed

3 files changed

+100
-67
lines changed

controller/gateway/controller.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -460,10 +460,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
460460
return ctrl.Result{}, fmt.Errorf("failed to update DataPlane with KonnectExtension to make it work as Hybrid: %w", err)
461461
}
462462
}
463-
} else {
463+
}
464+
465+
var controlplane *gwtypes.ControlPlane
466+
if !isGatewayHybrid(gatewayConfig) {
464467
// Provision controlplane creates a controlplane and adds the ControlPlaneReady condition to the Gateway status
465468
// if the controlplane is ready, the ControlPlaneReady status is set to true, otherwise false.
466-
controlplane := r.provisionControlPlane(ctx, logger, &gateway, gatewayConfig)
469+
controlplane = r.provisionControlPlane(ctx, logger, &gateway, gatewayConfig)
467470
// Set the ControlPlaneReady Condition to False. This happens only if:
468471
// * the new status is false and there was no ControlPlaneReady condition in the gateway
469472
// * the new status is false and the previous status was true
@@ -505,7 +508,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
505508
// If the code is run outside of k8s (like in envtest or integration test), do not create network policies.
506509
if k8sutils.RunningOnKubernetes() {
507510
log.Trace(logger, "ensuring DataPlane's NetworkPolicy exists")
508-
createdOrUpdated, err := r.ensureDataPlaneHasNetworkPolicy(ctx, &gateway, dataplane)
511+
createdOrUpdated, err := r.ensureDataPlaneHasNetworkPolicy(ctx, &gateway, dataplane, controlplane)
509512
if err != nil {
510513
return ctrl.Result{}, err
511514
}

controller/gateway/controller_reconciler_utils.go

Lines changed: 39 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,7 @@ func (r *Reconciler) ensureDataPlaneHasNetworkPolicy(
392392
ctx context.Context,
393393
gateway *gwtypes.Gateway,
394394
dataplane *operatorv1beta1.DataPlane,
395+
controlplane *gwtypes.ControlPlane,
395396
) (createdOrUpdate bool, err error) {
396397
networkPolicies, err := gatewayutils.ListNetworkPoliciesForGateway(ctx, r.Client, gateway)
397398
if err != nil {
@@ -406,8 +407,8 @@ func (r *Reconciler) ensureDataPlaneHasNetworkPolicy(
406407
return false, errors.New("number of networkPolicies reduced")
407408
}
408409

409-
// generate the network policy that allows the KO pod to access the admin APIs of dataplane pods.
410-
generatedPolicy, err := generateDataPlaneNetworkPolicy(r.Namespace, dataplane, r.PodLabels)
410+
// generate the network policy that allows the ControlPlane pod to access the admin APIs of dataplane pods.
411+
generatedPolicy, err := generateDataPlaneNetworkPolicy(dataplane, controlplane)
411412
if err != nil {
412413
return false, fmt.Errorf("failed generating network policy for DataPlane %s: %w", dataplane.Name, err)
413414
}
@@ -434,29 +435,20 @@ func (r *Reconciler) ensureDataPlaneHasNetworkPolicy(
434435
return true, r.Create(ctx, generatedPolicy)
435436
}
436437

437-
// generateDataPlaneNetworkPolicy generates the NetworkPolicy that allows the KO pod to access admin API of dataplane pods.
438-
// the params `namespace` and `podLabels` are namespace and labels of the KO pod itself, and `dataplane` is the target dataplane.
438+
// generateDataPlaneNetworkPolicy generates the NetworkPolicy that allows the ControlPlane pod to access admin API of dataplane pods.
439+
// The ControlPlane (KIC) is the component that actually communicates with the DataPlane admin API, not the operator directly.
440+
// In hybrid mode (Konnect), controlplane may be nil since there's no local ControlPlane - in that case,
441+
// the admin API access restriction is omitted.
439442
func generateDataPlaneNetworkPolicy(
440-
namespace string,
441443
dataplane *operatorv1beta1.DataPlane,
442-
podLabels map[string]string,
444+
controlplane *gwtypes.ControlPlane,
443445
) (*networkingv1.NetworkPolicy, error) {
444446
var (
445447
protocolTCP = corev1.ProtocolTCP
446448
adminAPISSLPort = intstr.FromInt(consts.DataPlaneAdminAPIPort)
447449
proxyPort = intstr.FromInt(consts.DataPlaneProxyPort)
448450
proxySSLPort = intstr.FromInt(consts.DataPlaneProxySSLPort)
449451
metricsPort = intstr.FromInt(consts.DataPlaneMetricsPort)
450-
// The label keys to match Kong operator pod.
451-
// To not create new NetworkPolicy on upgrade of , we just keep the keys marking the application
452-
// and remove the keys related to versions such as `version`,`pod-template-hash`,`helm.sh/chart`.
453-
podLabelSelectorKeys = []string{
454-
"app",
455-
"app.kubernetes.io/component",
456-
"app.kubernetes.io/instance",
457-
"app.kubernetes.io/name",
458-
"control-plane",
459-
}
460452
)
461453

462454
// Check if KONG_PROXY_LISTEN and/or KONG_ADMIN_LISTEN are set in
@@ -490,35 +482,31 @@ func generateDataPlaneNetworkPolicy(
490482
}
491483
}
492484

493-
// Construct the policy to allow the KO pod to access DataPlane admin APIs.
494-
policyPeerForControllerPod := networkingv1.NetworkPolicyPeer{
495-
NamespaceSelector: &metav1.LabelSelector{
496-
MatchLabels: map[string]string{
497-
"kubernetes.io/metadata.name": namespace,
485+
// Construct the policy to allow the ControlPlane pod to access DataPlane admin APIs.
486+
// For hybrid mode (Konnect), there's no local ControlPlane, so we don't add admin API restrictions.
487+
var limitAdminAPIIngress networkingv1.NetworkPolicyIngressRule
488+
if controlplane != nil {
489+
policyPeerForControlPlanePod := networkingv1.NetworkPolicyPeer{
490+
PodSelector: &metav1.LabelSelector{
491+
MatchLabels: map[string]string{
492+
"app": controlplane.Name,
493+
},
494+
},
495+
NamespaceSelector: &metav1.LabelSelector{
496+
MatchLabels: map[string]string{
497+
"kubernetes.io/metadata.name": dataplane.Namespace,
498+
},
498499
},
499-
},
500-
}
501-
502-
if len(podLabels) > 0 {
503-
matchPodLabels := map[string]string{}
504-
for _, key := range podLabelSelectorKeys {
505-
value, ok := podLabels[key]
506-
if ok {
507-
matchPodLabels[key] = value
508-
}
509500
}
510-
policyPeerForControllerPod.PodSelector = &metav1.LabelSelector{
511-
MatchLabels: matchPodLabels,
501+
limitAdminAPIIngress = networkingv1.NetworkPolicyIngressRule{
502+
Ports: []networkingv1.NetworkPolicyPort{
503+
{Protocol: &protocolTCP, Port: &adminAPISSLPort},
504+
},
505+
From: []networkingv1.NetworkPolicyPeer{
506+
policyPeerForControlPlanePod,
507+
},
512508
}
513509
}
514-
limitAdminAPIIngress := networkingv1.NetworkPolicyIngressRule{
515-
Ports: []networkingv1.NetworkPolicyPort{
516-
{Protocol: &protocolTCP, Port: &adminAPISSLPort},
517-
},
518-
From: []networkingv1.NetworkPolicyPeer{
519-
policyPeerForControllerPod,
520-
},
521-
}
522510

523511
allowProxyIngress := networkingv1.NetworkPolicyIngressRule{
524512
Ports: []networkingv1.NetworkPolicyPort{
@@ -533,6 +521,15 @@ func generateDataPlaneNetworkPolicy(
533521
},
534522
}
535523

524+
ingressRules := []networkingv1.NetworkPolicyIngressRule{
525+
allowProxyIngress,
526+
allowMetricsIngress,
527+
}
528+
// Only add admin API restriction when there's a local ControlPlane (not hybrid mode)
529+
if controlplane != nil {
530+
ingressRules = append([]networkingv1.NetworkPolicyIngressRule{limitAdminAPIIngress}, ingressRules...)
531+
}
532+
536533
return &networkingv1.NetworkPolicy{
537534
ObjectMeta: metav1.ObjectMeta{
538535
Namespace: dataplane.Namespace,
@@ -547,11 +544,7 @@ func generateDataPlaneNetworkPolicy(
547544
PolicyTypes: []networkingv1.PolicyType{
548545
networkingv1.PolicyTypeIngress,
549546
},
550-
Ingress: []networkingv1.NetworkPolicyIngressRule{
551-
limitAdminAPIIngress,
552-
allowProxyIngress,
553-
allowMetricsIngress,
554-
},
547+
Ingress: ingressRules,
555548
},
556549
}, nil
557550
}

pkg/utils/kubernetes/self_metadata.go

Lines changed: 55 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,47 +37,84 @@ func GetSelfPodLabels() (map[string]string, error) {
3737
locations []string
3838
)
3939

40+
// Prefer explicit override first
4041
if override := os.Getenv("KONG_OPERATOR_POD_LABELS_FILE"); override != "" {
4142
locations = append(locations, override)
4243
}
4344

44-
locations = append(locations, podLabelsFile)
45+
// Then try TELEPRESENCE_ROOT mounted path
4546
if root := os.Getenv("TELEPRESENCE_ROOT"); root != "" {
4647
relPath := strings.TrimPrefix(podLabelsFile, "/")
47-
locations = append([]string{filepath.Join(root, relPath)}, locations...)
48+
locations = append(locations, filepath.Join(root, relPath))
4849
}
4950

51+
// Finally fall back to standard pod labels file
52+
locations = append(locations, podLabelsFile)
53+
5054
for _, path := range locations {
5155
buf, err := os.ReadFile(path)
5256
if err != nil {
5357
lastErr = err
5458
continue
5559
}
5660

57-
labels := strings.SplitSeq(string(buf), "\n")
58-
ret := make(map[string]string)
59-
for label := range labels {
60-
labelKV := strings.SplitN(label, "=", 2)
61-
if len(labelKV) != 2 {
62-
return nil, fmt.Errorf("invalid label format, should be key=value")
63-
}
64-
key := labelKV[0]
65-
// The value in labels are escaped, e.g: "ko" => "\"ko\"". So we need to unquote it.
66-
value, err := strconv.Unquote(labelKV[1])
67-
if err != nil {
68-
continue
69-
}
70-
ret[key] = value
61+
ret := parsePodLabels(string(buf))
62+
if len(ret) > 0 {
63+
return ret, nil
7164
}
72-
return ret, nil
65+
lastErr = fmt.Errorf("no valid labels found in %s", path)
7366
}
7467

7568
if lastErr != nil {
76-
return nil, fmt.Errorf("cannot find pod labels from file %s: %w", locations[len(locations)-1], lastErr)
69+
return nil, fmt.Errorf("cannot find pod labels from %v: %w", locations, lastErr)
7770
}
7871
return nil, fmt.Errorf("cannot determine pod labels")
7972
}
8073

74+
// parsePodLabels parses pod labels from DownwardAPI format.
75+
// Supports both newline-separated and comma-separated formats.
76+
func parsePodLabels(content string) map[string]string {
77+
ret := make(map[string]string)
78+
content = strings.TrimSpace(content)
79+
if content == "" {
80+
return ret
81+
}
82+
83+
// Try newline-separated first
84+
lines := strings.Split(content, "\n")
85+
// If we only have one line and it contains commas, try comma-separated
86+
if len(lines) == 1 && strings.Contains(content, ",") {
87+
lines = strings.Split(content, ",")
88+
}
89+
90+
for _, label := range lines {
91+
label = strings.TrimSpace(label)
92+
if label == "" {
93+
continue
94+
}
95+
96+
labelKV := strings.SplitN(label, "=", 2)
97+
if len(labelKV) != 2 {
98+
continue
99+
}
100+
101+
key := strings.TrimSpace(labelKV[0])
102+
value := strings.TrimSpace(labelKV[1])
103+
if key == "" {
104+
continue
105+
}
106+
107+
// Try to unquote the value (DownwardAPI escapes values)
108+
if unquoted, err := strconv.Unquote(value); err == nil {
109+
value = unquoted
110+
}
111+
112+
ret[key] = value
113+
}
114+
115+
return ret
116+
}
117+
81118
// RunningOnKubernetes returns true if it is running in the kubernetes environment.
82119
// If the env KUBERNETES_SERVICE_HOST is configured to access the kubernetes API server,
83120
// it is considered to be running on k8s.

0 commit comments

Comments
 (0)