Skip to content

Improve customizability of SSABasedGenericKubernetesResourceMacher #2757

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

murillio4
Copy link
Contributor

This PR addresses #2509 and makes it easier to customize the matching logic in SSABasedGenericKubernetesResourceMatcher without having to clone the entire class just to override its behavior.

The change is based on the solution suggested by @Donnerbart here, but it avoids passing the primary resource directly in order to keep the existing matches method signature unchanged. If the primary resource is needed, it can instead be made available through the context.

The implementation introduces a new protected matches method that handles the actual comparison. The public matches method delegates to this new method, making it easy to override in subclasses.

@openshift-ci openshift-ci bot requested review from csviri and xstefank April 7, 2025 22:50
@murillio4 murillio4 force-pushed the customizeable-ssabased-matcher-2553 branch from 6d39813 to 35dc05a Compare April 7, 2025 22:52
@murillio4 murillio4 force-pushed the customizeable-ssabased-matcher-2553 branch from 35dc05a to 5c4fdbd Compare April 7, 2025 22:59
Copy link
Collaborator

@metacosm metacosm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@metacosm
Copy link
Collaborator

metacosm commented Apr 8, 2025

We should probably make it easier to use an SSA matcher without having to override the KubernetesDependentResource.match method.

@metacosm metacosm merged commit f6f0183 into operator-framework:main Apr 9, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants