Config overrides: use apiVersion instead of group for easier UX#489
Closed
frobware wants to merge 5 commits intobpfman:mainfrom
Closed
Config overrides: use apiVersion instead of group for easier UX#489frobware wants to merge 5 commits intobpfman:mainfrom
frobware wants to merge 5 commits intobpfman:mainfrom
Conversation
This commit adds the ability to mark components as unmanaged in the bpfman-operator, preventing the operator from creating or updating specific objects. The implementation includes: - Added ComponentOverride struct to Config API with fields for kind, group, namespace, name, and unmanaged flag - Modified assureResource function to check for overrides and skip management of unmanaged components - Implemented isOverridden helper function to match objects against override specifications - Added tests to verify override functionality works correctly across all component types Signed-off-by: Andreas Karis <ak.karis@gmail.com>
Signed-off-by: Andreas Karis <ak.karis@gmail.com>
Signed-off-by: Andreas Karis <ak.karis@gmail.com>
Change the override selector from requiring a separate group field to using apiVersion, which matches the format users see in kubectl output. This makes overrides easier to configure by allowing direct copy/paste from manifests. The implementation now uses scheme-based GVK resolution instead of relying on the object's TypeMeta, which may not be populated for objects constructed in code. This ensures reliable matching regardless of how the object was created. Also fix typos and improve documentation to clarify that overrides are a debug/escape hatch feature for advanced users. Signed-off-by: Andrew McDermott <amcdermo@redhat.com>
Regenerated using make generate manifests bundle after APIVersion changes to ComponentOverride. Signed-off-by: Andrew McDermott <amcdermo@redhat.com>
88dfade to
7d6c080
Compare
Contributor
Author
|
This was done in #475. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #489 +/- ##
===========================
===========================
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
This is an experiment building on @andreaskaris's work in #475.
After reviewing the override functionality, I wanted to explore a couple of changes that I think make the feature easier to use and more robust.
What changed
1. Replace
groupwithapiVersionin the override specThe original implementation required users to specify:
This means users need to remember API group names (
apps,storage.k8s.io, empty string for core resources, etc.) and mentally splitapiVersioninto group/version.The new approach uses
apiVersiondirectly:This is the same format users see in
kubectl get -o yamloutput, so they can copy/paste directly. No need to remember that ConfigMaps have an empty group or that DaemonSets live inapps.2. Use scheme-based GVK resolution
The original implementation matched overrides using
resource.GetObjectKind().GroupVersionKind(). This is fragile because many objects constructed in code don't have their TypeMeta populated - it's only reliably set when objects are decoded from the API server.The PR had already worked around this by manually adding TypeMeta to the ConfigMap:
The new approach uses the controller's scheme to resolve GVK:
This works reliably for all objects regardless of how they were constructed, so we can remove the manual TypeMeta workaround.
Testing
I've tested this on a kind cluster:
The escape hatch semantics work as expected: complete hands-off while unmanaged, operator resumes control when the override is removed.
Summary
These changes are subjective improvements for UX and robustness. The core override functionality from #475 is unchanged - this just makes it easier to configure and removes a potential source of silent matching failures.
Happy to discuss if there are concerns or alternative approaches.