Add namespace label selector support for reflection#620
Open
davidswimbird wants to merge 6 commits intoemberstack:mainfrom
Open
Add namespace label selector support for reflection#620davidswimbird wants to merge 6 commits intoemberstack:mainfrom
davidswimbird wants to merge 6 commits intoemberstack:mainfrom
Conversation
Add two new annotations that allow selecting target namespaces by Kubernetes label selectors instead of only name regex patterns: - reflection-allowed-namespaces-selector - reflection-auto-namespaces-selector When both a name-pattern and a label selector annotation are set, a namespace matches if it satisfies either condition (OR logic). The label selector supports standard Kubernetes syntax: equality (=, ==, !=), existence, and set-based (in, notin) expressions. Closes emberstack#409
davidswimbird
force-pushed
the
davidswimbird/discussion-409
branch
from
a75fa1c to
a704f21
Compare
Contributor
|
@davidswimbird while I review this, please update the documentation as well (readme) |
There was a problem hiding this comment.
Pull request overview
Adds namespace label selector support to the reflector’s “allowed namespaces” and “auto namespaces” logic, enabling selection of target namespaces via standard Kubernetes label selector syntax in addition to existing name/regex matching.
Changes:
- Introduces two new reflection annotations for label selectors (allowed/auto namespaces selectors) and wires them into
MirroringProperties. - Implements label-selector matching (with OR logic vs name-pattern matching) and updates mirroring flows to use
V1Namespacewhen available. - Adds unit tests and test helpers for selector behavior; exposes internals to the test project.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ES.Kubernetes.Reflector.Tests/LabelSelectorMatchTests.cs | Adds unit tests for label selector parsing/matching and OR logic with name patterns. |
| tests/ES.Kubernetes.Reflector.Tests/Additions/ReflectorAnnotationsBuilder.cs | Adds builder helpers for the new selector annotations in tests. |
| src/ES.Kubernetes.Reflector/Mirroring/Core/ResourceMirror.cs | Caches namespaces for selector evaluation and uses V1Namespace overloads for matching. |
| src/ES.Kubernetes.Reflector/Mirroring/Core/MirroringPropertiesExtensions.cs | Parses new annotations, adds selector-aware matching, and implements label selector evaluation. |
| src/ES.Kubernetes.Reflector/Mirroring/Core/MirroringProperties.cs | Adds properties for allowed/auto namespace label selectors. |
| src/ES.Kubernetes.Reflector/Mirroring/Core/Annotations.cs | Adds constants for the new selector annotations. |
| src/ES.Kubernetes.Reflector/ES.Kubernetes.Reflector.csproj | Adds InternalsVisibleTo to allow unit tests to access LabelSelectorMatch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Document the new reflection-allowed-namespaces-selector and reflection-auto-namespaces-selector annotations from emberstack#620.
Invalid or degenerate selectors (e.g. ",", "!", "!=value") previously produced no valid requirements or empty keys, causing LabelSelectorMatch to return true and unintentionally allow reflection to all namespaces. Now returns false when no requirements are parsed or a key is empty.
The namespace cache was only populated on Added events, so label changes (Modified) left stale entries and deleted namespaces were never evicted. - Handle Modified: update cache and reconcile auto-reflections (create if now matching, delete if no longer matching) - Handle Deleted: evict cache entry and clean up auto-reflection tracking
Cover unbalanced parentheses, empty set-based expressions, and bare operators to document that the parser fails closed on all of these.
|
Automatically marked as stale due to no recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Author
|
Hey @winromulus. How's the review coming along? :) |
Contributor
|
@davidswimbird a couple of comments left as pending. Can you check those please? |
Author
|
@winromulus resolved all comments. They had already been adressed, just didn't want to resolve comments without you having a look :) |
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
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.
Add two new annotations that allow selecting target namespaces by
Kubernetes label selectors instead of only name regex patterns:
When both a name-pattern and a label selector annotation are set, a
namespace matches if it satisfies either condition (OR logic). The label
selector supports standard Kubernetes syntax: equality (=, ==, !=),
existence, and set-based (in, notin) expressions.
This is useful when you want to reflect a secret to a subset of non-deterministic namespaces, that can't be represented with a regex.
For my use case, it makes the manual overhead of managing reflector a lot smaller, as I can automate the label configuration for my namespaces ahead of time, to know that they will be receiving the secrets when the namespace is created with a specified label.
This is based on the suggestion in: Discussion #409