Skip to content

identity: add per-node identities#22208

Closed
oblazek wants to merge 3 commits intocilium:masterfrom
oblazek:ob-node-identities
Closed

identity: add per-node identities#22208
oblazek wants to merge 3 commits intocilium:masterfrom
oblazek:ob-node-identities

Conversation

@oblazek
Copy link
Copy Markdown
Contributor

@oblazek oblazek commented Nov 16, 2022 •
edited
Loading

With this one can specify this CNP and allow access only from/to specific nodes:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "from-nodes"
spec:
  endpointSelector:
    matchLabels:
      {}
  ingress:
  - fromNodes:
    - matchLabels:
        cilium.io/ci-node: k8s1
  egress:
  - toNodes:
    - matchLabels:
        cilium.io/ci-node: k8s1

instead of

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "from-nodes"
spec:
  endpointSelector:
    matchLabels:
      {}
  ingress:
  - fromEntities:
    - remote-node:

where all remote-nodes (as well as nodes from all remote clusters) are allowed.

This feature reuses existing global way to allocate identity just like how endpoint identities work, i.e. each cilium-agent waits for global identity allocator to be initialized and then allocates inside nodeDiscovery identity for each remote-node and saves it to a local ipcache.

When packets are sent out they still have ID 6 (remote-node), but corresponding cilium-agent knows better and checks ipcache (here for tunneling mode) where it finds the right identity. This way continuous upgrade is possible -- old nodes will find an ID == 6 and upgraded nodes might find something like 211584.

During upgrade user might have to specify both:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: "from-nodes"
spec:
  endpointSelector:
    matchLabels:
      {}
  ingress:
  - fromEntities:
    - remote-node
  - fromNodes:
    - matchLabels:
        cilium.io/ci-node: k8s1

so that old nodes as well as upgraded nodes correctly allow/block packets.

Fixes: #21615

Signed-off-by: Ondrej Blazek ondrej.blazek@firma.seznam.cz

@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 Nov 16, 2022
@oblazek oblazek force-pushed the ob-node-identities branch 2 times, most recently from 48a2d05 to abaf0cb Compare November 16, 2022 12:49
@oblazek
identity: add per-node identities
5d445c0 
This commit adds a new flag `--enable-per-node-identity` which enables cilium
to allocate a custom identity to each remote node object instead of `remote-node` ID.

Previously during node discovery agents learned about all remote nodes and upserted
their IPs with `remote-node` ID into their ipcaches. During policy
enforcement the ipcache was then checked for the correct entry in both direct-routing
and tunneling mode.
With this change each node will assign or get global ID of each remote-node
from identity backend and insert that into its own ipcache independently so that
each node can do its own decision and upgrade of cluster nodes can be done one by one.

Signed-off-by: Ondrej Blazek <ondrej.blazek@firma.seznam.cz>
@oblazek
docs: add documentation related to per-node IDs
32da0da 
Signed-off-by: Ondrej Blazek <ondrej.blazek@firma.seznam.cz>
@oblazek
policy: add fromNodes/toNodes section in C(C)NPs
d0dc2f0 
When per-node identities are enabled this means that entity 'remote-node'
does not work anymore as each node has custom identity based on its labels.
In this case user has to allow/block each node using fromNodes/toNodes
section in CNPs/CCNPs.

Signed-off-by: Ondrej Blazek <ondrej.blazek@firma.seznam.cz>
@aanm
Copy link
Copy Markdown
Member

aanm commented Nov 23, 2022

/test

@aanm aanm added the kind/community-contribution This was a contribution made by a community member. label Dec 1, 2022
@aanm
Copy link
Copy Markdown
Member

aanm commented Dec 5, 2022

@oblazek 👋 are you still working on the PR?

@oblazek
Copy link
Copy Markdown
Contributor Author

oblazek commented Dec 5, 2022 •
edited
Loading

@oblazek wave are you still working on the PR?

yeah kind of, I am trying to figure out how to convince Paul that it is unnecessary to change anything in BPF code related to identity_is_remote_node :) (the discussion here https://cilium.slack.com/archives/C2B917YHE/p1668583071436069)

Also I will try to run the net-next e2e test locally, as something went wrong :/

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 5, 2023

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jan 5, 2023
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 19, 2023

This pull request has not seen any activity since it was marked stale.
Closing.

@github-actions github-actions bot closed this Jan 19, 2023
@oblazek oblazek mentioned this pull request Jul 19, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. kind/community-contribution This was a contribution made by a community member. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@oblazek @aanm