Skip to content

policy: Fix node selector handling for local host in KCNP#44446

Merged
giorio94 merged 2 commits intocilium:mainfrom
TheBeeZee:pr-fix-node-selector
Mar 26, 2026
Merged

policy: Fix node selector handling for local host in KCNP#44446
giorio94 merged 2 commits intocilium:mainfrom
TheBeeZee:pr-fix-node-selector

Conversation

@TheBeeZee
Copy link
Copy Markdown
Contributor

@TheBeeZee TheBeeZee commented Feb 19, 2026 •
edited
Loading

Node selector in k8s ClusterNetworkPolicy is supposed to match on all nodes. The existing code only matched on remote nodes. Add a node selector that also matches the local node (i.e. the same node that the subject pod is running on).

Fix node selector handling for k8s ClusterNetworkPolicy

@TheBeeZee TheBeeZee requested a review from a team as a code owner February 19, 2026 16:56
@TheBeeZee TheBeeZee requested a review from youngnick February 19, 2026 16:56
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 19, 2026
@TheBeeZee
Copy link
Copy Markdown
Contributor Author

TheBeeZee commented Feb 19, 2026

@jrajahalme @squeed For your attention

@TheBeeZee
Copy link
Copy Markdown
Contributor Author

TheBeeZee commented Feb 19, 2026

/test

@youngnick
Copy link
Copy Markdown
Contributor

youngnick commented Feb 20, 2026

Is this a behavior change? Will this cause policies that used to block not to?

@TheBeeZee
Copy link
Copy Markdown
Contributor Author

TheBeeZee commented Feb 20, 2026

Is this a behavior change? Will this cause policies that used to block not to?

It is a bug fix, the policy should have matched on the local node, too, but it didn't. This makes Cilium pass the k8s ClusterNetworkPolicy conformance tests.

Note that support for ClusterNetworkPolicy was just added last week and it is not enabled by default.

@TheBeeZee TheBeeZee force-pushed the pr-fix-node-selector branch from 070c283 to f4d422b Compare March 3, 2026 23:12
@TheBeeZee TheBeeZee requested a review from youngnick March 3, 2026 23:13
@TheBeeZee TheBeeZee marked this pull request as draft March 9, 2026 21:58
@TheBeeZee TheBeeZee marked this pull request as draft March 9, 2026 21:58
@TheBeeZee TheBeeZee force-pushed the pr-fix-node-selector branch from f4d422b to ad59498 Compare March 9, 2026 22:52
@TheBeeZee
Copy link
Copy Markdown
Contributor Author

TheBeeZee commented Mar 9, 2026

/test

@squeed squeed self-requested a review March 10, 2026 16:01
@TheBeeZee TheBeeZee force-pushed the pr-fix-node-selector branch 3 times, most recently from 746ad5b to 53511ec Compare March 10, 2026 20:52
@TheBeeZee TheBeeZee marked this pull request as ready for review March 10, 2026 20:54
@TheBeeZee TheBeeZee requested a review from a team as a code owner March 10, 2026 20:54
@TheBeeZee
Copy link
Copy Markdown
Contributor Author

TheBeeZee commented Mar 10, 2026

/test

@TheBeeZee TheBeeZee force-pushed the pr-fix-node-selector branch from 53511ec to ff86a48 Compare March 13, 2026 02:09
@squeed
Copy link
Copy Markdown
Contributor

squeed commented Mar 13, 2026

I think this is right.

I can't think of a reason to exclude the local node's labels. @oblazek is the original author of that, way back in #26924. Unless he objects, this LGTM. I'll give him a few days to comment.

@oblazek
Copy link
Copy Markdown
Contributor

oblazek commented Mar 19, 2026 •
edited
Loading

I am curious how much different is kCNP in comparison to CNPs/CCNPs, but for the latter I have no objection, nor for the nodemanager changes.

I can see only these changes:
(old)

$ k --context infra82.ko -n cilium exec -it cilium-vkv29 -- cilium-dbg bpf policy get 2026
Defaulted container "cilium-agent" out of: cilium-agent, config (init), mount-cgroup (init), apply-sysctl-overwrites (init), mount-bpf-fs (init), clean-cilium-state (init), install-cni-binaries (init)
POLICY   DIRECTION   LABELS (source:key[=value])                  PORT/PROTO   PROXY PORT   AUTH TYPE   BYTES   PACKETS   PREFIX
Allow    Ingress     reserved:host                                ANY          NONE         disabled    4746    61        0

(new)

…/Work/yamls ❯ k --context infra82.ko -n cilium exec -it cilium-tkpt4 -- cilium-dbg bpf policy get 2026
Defaulted container "cilium-agent" out of: cilium-agent, config (init), mount-cgroup (init), apply-sysctl-overwrites (init), mount-bpf-fs (init), clean-cilium-state (init), install-cni-binaries (init)
POLICY   DIRECTION   LABELS (source:key[=value])                    PORT/PROTO   PROXY PORT   AUTH TYPE   BYTES   PACKETS   PREFIX
Allow    Ingress     node:io.cilium.k8s.policy.cluster=infra82.ko   ANY          NONE         disabled    2558    33        0
                     reserved:host

(old)

$ k --context infra82.ko -n cilium exec -it cilium-vkv29 -- cilium-dbg identity get 1
Defaulted container "cilium-agent" out of: cilium-agent, config (init), mount-cgroup (init), apply-sysctl-overwrites (init), mount-bpf-fs (init), clean-cilium-state (init), install-cni-binaries (init)
ID   LABELS
1    reserved:host

(new)

$ k --context infra82.ko -n cilium exec -it cilium-tkpt4 -- cilium-dbg identity get 1
Defaulted container "cilium-agent" out of: cilium-agent, config (init), mount-cgroup (init), apply-sysctl-overwrites (init), mount-bpf-fs (init), clean-cilium-state (init), install-cni-binaries (init)
ID   LABELS
1    node:io.cilium.k8s.policy.cluster=infra82.ko
     reserved:host

which does not affect policies as localnode is always allowed to acces the corresponding pod.

@squeed squeed added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Mar 19, 2026
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 19, 2026
@squeed squeed enabled auto-merge March 19, 2026 13:58
@squeed
Copy link
Copy Markdown
Contributor

squeed commented Mar 19, 2026

/test

auto-merge was automatically disabled March 19, 2026 15:55

Head branch was pushed to by a user without write access

@TheBeeZee TheBeeZee force-pushed the pr-fix-node-selector branch from ff86a48 to d9a9e29 Compare March 19, 2026 15:55
@TheBeeZee
Copy link
Copy Markdown
Contributor Author

TheBeeZee commented Mar 19, 2026

I rebased the CL, that will hopefully get rid of the CI test failures.

@TheBeeZee
Copy link
Copy Markdown
Contributor Author

TheBeeZee commented Mar 19, 2026

/test

1 similar comment
@cilium-ariane
Copy link
Copy Markdown

cilium-ariane bot commented Mar 19, 2026

/test

@maintainer-s-little-helper
Copy link
Copy Markdown

maintainer-s-little-helper bot commented Mar 19, 2026

Commit f7b4e62 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Mar 19, 2026
@TheBeeZee TheBeeZee force-pushed the pr-fix-node-selector branch from f7b4e62 to 2d56a74 Compare March 19, 2026 17:09
@maintainer-s-little-helper
Copy link
Copy Markdown

maintainer-s-little-helper bot commented Mar 19, 2026

Commit f7b4e62 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@TheBeeZee
Copy link
Copy Markdown
Contributor Author

TheBeeZee commented Mar 19, 2026

/test

2 similar comments
@cilium-ariane
Copy link
Copy Markdown

cilium-ariane bot commented Mar 19, 2026

/test

@TheBeeZee
Copy link
Copy Markdown
Contributor Author

TheBeeZee commented Mar 23, 2026

/test

@TheBeeZee TheBeeZee force-pushed the pr-fix-node-selector branch from 2d56a74 to 169f736 Compare March 24, 2026 15:53
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Mar 24, 2026
@TheBeeZee
policy: Fix KCNP node selector handling for local host
a8db3d7 
Currently we only match labels on the remote node (reserved:remote-node),
this expands the search to the local node, too (reserved:host).

Signed-off-by: Blaz Zupan <blaz@google.com>
@TheBeeZee
node: Populate k8s labels on local node when per-node labels are enabled
a12dfd8 
Signed-off-by: Blaz Zupan <blaz@google.com>
@TheBeeZee TheBeeZee force-pushed the pr-fix-node-selector branch from 169f736 to a12dfd8 Compare March 25, 2026 22:50
@TheBeeZee
Copy link
Copy Markdown
Contributor Author

TheBeeZee commented Mar 25, 2026

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 26, 2026
@giorio94 giorio94 added this pull request to the merge queue Mar 26, 2026
Merged via the queue into cilium:main with commit 64c39f0 Mar 26, 2026
79 checks passed
@TheBeeZee TheBeeZee deleted the pr-fix-node-selector branch March 26, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@squeed squeed squeed approved these changes

@youngnick youngnick youngnick approved these changes

Assignees

No one assigned

Labels

ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@TheBeeZee @youngnick @squeed @oblazek @giorio94