-
Notifications
You must be signed in to change notification settings - Fork 51
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
feat: restrict UIPlugin CRD names to allow a single instance per type #481
base: main
Are you sure you want to change the base?
Conversation
/lgtm Nice! We'll have to chat about OCP specific integration tests soon. |
AFAIU you can still create conflicting objects if you mess up the names and the types, right? For instance:
I don't think that it's possible to validate this case with OpenAPI but we might want the operator to stop reconciliation when it detects the issue? |
Ahh I think I might have been (or still am) confused what needs to be done. So there can always only be a single In that case can we not tie the name to the type? Something like
|
@jan--f this would be neat if it works indeed. |
Will do some tests based on @jan--f suggestion and update the PR |
Just a nitpick, but I think I would get rid of the |
3d749fb
to
7cbdc07
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jgbernalp The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold @csibbitt please check if changes are needed for STF |
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.
/lgtm
/hold
👍
STF does not use the UIPlugin, but Telemetry Operator does. I'll take care of this ASAP on OSPRH-7600 |
OU-402
COO-164
This PR adds a pattern restriction for the UIPlugin name so only a single instance of a specific plugin is allowed