-
Notifications
You must be signed in to change notification settings - Fork 584
CNTRLPLANE-1752: Add PKI API to config.openshift.io/v1alpha1 #2645
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
base: master
Are you sure you want to change the base?
Conversation
sanchezl
commented
Jan 12, 2026
- Add PKI API to config.openshift.io/v1alpha1
- make update
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Comment |
|
Skipping CI for Draft Pull Request. |
|
Hello @sanchezl! Some important instructions when contributing to openshift/api: |
|
/test required |
|
@sanchezl: The specified target(s) for The following commands are available to trigger optional jobs: Use DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/test unit |
|
@sanchezl: This pull request references CNTRLPLANE-1752 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest-required |
|
@sanchezl: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| // +kubebuilder:validation:XValidation:rule="self.type == 'Unmanaged' ? (!has(self.defaults) && !has(self.categories) && !has(self.overrides)) : true",message="defaults, categories, and overrides must not be set when type is Unmanaged" | ||
| // +kubebuilder:validation:XValidation:rule="self.type == 'Default' ? (!has(self.defaults) && !has(self.categories) && !has(self.overrides)) : true",message="defaults, categories, and overrides must not be set when type is Default" | ||
| // +union | ||
| type PKIManagementPolicy struct { |
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.
I want to note that I am not using a discriminated union pattern here, instead it is more of a "parameterized" union pattern (at at least a kind of parameterized union where its all fields or none).
This configuration models a "predefined behavior vs. customization" choice pattern, rather than mutually exclusive options with different configuration requirements (like platform-specific settings).
If we add more predefined behaviors (e.g., type: FIPS, type: CNSACompliant), they would be additional enum values with no configuration fields. They represent different default behaviors, not different configuration structures.
If we need to extend what's configurable (e.g., adding certificate lifetime configuration), we'd add fields to PKIProfile and/or CertificateConfig. These fields would apply regardless of which predefined behavior or custom configuration is in use.
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.
I'm not sure I'm following why this is a better approach than a discriminated union pattern.
Discriminated unions are a common pattern in OpenShift APIs (so our users will be used to them) and they allow for discriminators with no associated configuration fields.
If we add more predefined behaviors (e.g., type: FIPS, type: CNSACompliant), they would be additional enum values with no configuration fields. They represent different default behaviors, not different configuration structures.
If we need to extend what's configurable (e.g., adding certificate lifetime configuration), we'd add fields to PKIProfile and/or CertificateConfig. These fields would apply regardless of which predefined behavior or custom configuration is in use.
It sounds like if we added any additional pre-defined behaviors any new configuration options we add would be not configurable by end-users with these pre-defined behaviors.
To me, this reads that the only time we anticipate ever being able to configure something is when type is set to Custom - in which case an end-user must supply some kind of configuration.
Is this accurate?