-
Notifications
You must be signed in to change notification settings - Fork 136
fix(registry): implement field selector filtering for label-based resources #1845
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ources Controller-runtime cache doesn't support field selectors, causing incorrect filtering when using kubectl with field selectors like --field-selector=metadata.namespace=tenant-kvaps or metadata.name=test. Changes: - Created pkg/registry/fields package with ParseFieldSelector utility - Refactored field selector parsing logic in application, tenantmodule, and tenantsecret registries to use common implementation - Implemented manual filtering for metadata.name and metadata.namespace in List() and Watch() methods - Removed Raw field usage and field selectors from client.ListOptions - Label selectors passed directly via LabelSelector field Field selectors now properly filter resources by name and namespace through manual post-processing after label-based filtering. See: kubernetes-sigs/controller-runtime#612 Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis pull request introduces custom field selector filtering for Kubernetes resources by replacing controller-runtime's field selector handling with client-side filtering logic. A new filter module parses field selectors to extract metadata.name and metadata.namespace constraints, which are then applied manually during List and Watch operations across Applications, TenantModule, and TenantSecret REST endpoints. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant REST as REST Handler
participant Filter as Field Filter
participant HelmRelease as HelmRelease Lister
participant Converter as Resource Converter
Client->>REST: List Request (field selector)
REST->>Filter: ParseFieldSelector()
Filter-->>REST: Filter{Name, Namespace}
REST->>REST: Check namespace match
alt Namespace mismatch
REST-->>Client: Empty List
else Namespace matches
REST->>HelmRelease: List with LabelSelector
HelmRelease-->>REST: HelmRelease items
loop For each HelmRelease
REST->>Filter: MatchesName() + MatchesNamespace()
Filter-->>REST: true/false
alt Matches
REST->>Converter: Convert to Resource
Converter-->>REST: Resource
REST->>REST: Add to results
end
end
REST-->>Client: Filtered Resource List
end
sequenceDiagram
participant Client
participant REST as REST Handler
participant Filter as Field Filter
participant Watcher as HelmRelease Watcher
participant CustomWatcher as Custom Event Filter
participant Converter as Resource Converter
Client->>REST: Watch Request (field selector)
REST->>Filter: ParseFieldSelector()
Filter-->>REST: Filter{Name, Namespace}
REST->>Watcher: Watch with LabelSelector
Watcher-->>CustomWatcher: HelmRelease events
loop For each event
CustomWatcher->>Filter: MatchesName() + MatchesNamespace()
Filter-->>CustomWatcher: true/false
alt Matches
CustomWatcher->>Converter: Convert to Resource event
Converter-->>CustomWatcher: Resource event
CustomWatcher-->>Client: Filtered event
else No match
CustomWatcher->>CustomWatcher: Filter out
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @kvaps, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new fieldfilter package to manually process Kubernetes field selectors for metadata.name and metadata.namespace. This change addresses a limitation where the controller-runtime cache does not support field selectors. The List and Watch methods in pkg/registry/apps/application/rest.go, pkg/registry/core/tenantmodule/rest.go, and pkg/registry/core/tenantsecret/rest.go have been updated to remove FieldSelector from client.ListOptions when querying the cache. Instead, they now parse the field selector using the new fieldfilter package and apply manual filtering for metadata.name and metadata.namespace on the fetched resources. Additionally, an early exit is implemented in List operations if the field selector's namespace does not match the context namespace.
…ources (#1845) ## What this PR does Fixes field selector filtering for registry resources (Applications, TenantModules, TenantSecrets) when using kubectl with field selectors like `--field-selector=metadata.namespace=tenant-kvaps` or `metadata.name=test`. Controller-runtime cache doesn't support field selectors natively, which caused incorrect filtering behavior. This PR implements manual filtering for `metadata.name` and `metadata.namespace` field selectors in List() and Watch() methods. Changes: - Created `pkg/registry/fields` package with `ParseFieldSelector` utility for common field selector parsing - Refactored field selector logic in application, tenantmodule, and tenantsecret registries to use the common implementation - Implemented manual post-processing filtering after label-based queries - Removed `Raw` field usage and field selectors from `client.ListOptions` ### Release note ```release-note [registry] Fix field selector filtering for kubectl queries with metadata.name and metadata.namespace selectors ```
What this PR does
Fixes field selector filtering for registry resources (Applications, TenantModules, TenantSecrets) when using kubectl with field selectors like
--field-selector=metadata.namespace=tenant-kvapsormetadata.name=test.Controller-runtime cache doesn't support field selectors natively, which caused incorrect filtering behavior. This PR implements manual filtering for
metadata.nameandmetadata.namespacefield selectors in List() and Watch() methods.Changes:
pkg/registry/fieldspackage withParseFieldSelectorutility for common field selector parsingRawfield usage and field selectors fromclient.ListOptionsRelease note
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.