Conversation
Since #978, token-vendor gives a key's ConfigMap an owner reference to the Robot CR derived from its device ID (robot-<name>), so the key is deleted along with the Robot. This doesn't work when the device ID doesn't match a Robot CR. For example, the device-manager publishes a bootstrap key as robot-node-<uuid> so that a new node can fetch its config, but the node can join a cluster with a different name (eg vmpool's vmp-...). No Robot called node-<uuid> ever exists, so the ConfigMap never gets an owner and leaks. Add an optional robot-name parameter to public-key.publish to name the owning Robot CR explicitly. It's stored in the cloudrobotics.com/robot-name label, and the ConfigMap informer indexes keys by their owner's name, so the owner reference is also set if the Robot CR is created after the key is published. Callers that don't pass robot-name keep the current behavior. The device-manager will start passing it once this is rolled out. TAG=agy CONV=4b9dee19-63da-4d08-af67-f0eebad7d73c Created using jj-spr
Contributor
Author
|
This PR is a standalone change. |
ensonic
reviewed
Sep 23, 2026
| return nil | ||
| } | ||
| // TODO(rodrigoq): The cache can be stale if the Robot CR was just deleted | ||
| // and re-created, eg when the device-manager replaces a cluster. Then the |
Contributor
There was a problem hiding this comment.
Can where phase this more genericly?
ensonic
reviewed
Sep 23, 2026
| } | ||
|
|
||
| // A key can be owned by a Robot CR that doesn't match its device ID, eg when a | ||
| // device registers as robot-node-<uuid> but joins a cluster with another name. |
Contributor
There was a problem hiding this comment.
tbh, I don't understand the "but joins a cluster with another name" part.
ensonic
reviewed
Sep 23, 2026
| deleted when the Robot CR is deleted. Defaults to the device-id without the | ||
| robot- prefix. Set this if the device-id doesn't match a Robot CR, eg for a | ||
| device that registers under its node name but joins a cluster with another | ||
| name. |
Contributor
There was a problem hiding this comment.
Same here - "joins a cluster with another name" sounds like multi-node cluster related, but in that case we would not register a new key for the additional nodes, right.
ensonic
approved these changes
Sep 23, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since #978, token-vendor gives a key's ConfigMap an owner reference to the
Robot CR derived from its device ID (robot-), so the key is deleted
along with the Robot. This doesn't work when the device ID doesn't match a
Robot CR. For example, you might publish a key for
robot- so that a new robot can fetch its config, but also publish
a key robot-. No Robot called
ever exists, so the ConfigMap never gets an owner and leaks.
Add an optional robot-name parameter to public-key.publish to name the
owning Robot CR explicitly. It's stored in the cloudrobotics.com/robot-name
label, and the ConfigMap informer indexes keys by their owner's name, so the
owner reference is also set if the Robot CR is created after the key is
published.
Callers that don't pass robot-name keep the current behavior.
Only tested via unit tests, I'm planning to test the client against the nightly
so might need to follow up to fix this.
Alternative considered: The client could also patch the created ConfigMap
after calling into the token-vendor, but that couples the client to the backend
representation.
TAG=agy
CONV=4b9dee19-63da-4d08-af67-f0eebad7d73c