CNTRLPLANE-1752: Add PKI API to config.openshift.io/v1alpha1 #2645
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| --- | ||
| # ValidatingAdmissionPolicy that dynamically validates PKI certificate override names | ||
| # against registered PKICertificateDefinition resources | ||
| apiVersion: admissionregistration.k8s.io/v1 | ||
| kind: ValidatingAdmissionPolicy | ||
| metadata: | ||
| name: validate-pki-certificate-overrides.config.openshift.io | ||
| spec: | ||
| # Only validate PKI resources during CREATE/UPDATE | ||
| matchConstraints: | ||
| resourceRules: | ||
| - apiGroups: ["config.openshift.io"] | ||
| apiVersions: ["v1alpha1"] | ||
| operations: ["CREATE", "UPDATE"] | ||
| resources: ["pkis"] | ||
|
|
||
| # Use PKICertificateDefinition resources as the parameter source | ||
| paramKind: | ||
| apiVersion: config.openshift.io/v1alpha1 | ||
| kind: PKICertificateDefinition | ||
|
|
||
| # Validate each certificate override references a registered certificate name | ||
| validations: | ||
| # Skip validation if no overrides are present | ||
| - expression: "!has(object.spec.certificateManagement.custom.overrides) || size(object.spec.certificateManagement.custom.overrides) == 0" | ||
| reason: Skip | ||
|
Comment on lines
+24
to
+26
Contributor
There was a problem hiding this comment. Should be under |
||
|
|
||
| # Build list of all valid certificate names from all PKICertificateDefinition resources | ||
| - expression: | | ||
| has(params) && has(params.spec) && has(params.spec.certificates) ? | ||
| params.spec.certificates.map(cert, cert.name) : [] | ||
|
Contributor
There was a problem hiding this comment. What are you trying to achieve here? This appears to be trying to construct a list of names, did you want to create a variable? Validation expressions must map evaluate to a boolean I think you want a variable to use later? I'm not convinced I understand why you want to map here so I've left that out |
||
| messageExpression: "'No PKICertificateDefinition found for component ' + (has(params.spec) ? params.spec.component : 'unknown')" | ||
| reason: Invalid | ||
|
|
||
| # Validate each override.certificateName exists in a PKICertificateDefinition | ||
| - expression: | | ||
| !has(object.spec.certificateManagement.custom.overrides) || | ||
| object.spec.certificateManagement.custom.overrides.all(override, | ||
| params.exists(p, | ||
| has(p.spec) && has(p.spec.certificates) && | ||
| p.spec.certificates.exists(cert, cert.name == override.certificateName) | ||
| ) | ||
| ) | ||
| message: "certificateName in overrides must reference a certificate registered in a PKICertificateDefinition resource" | ||
| reason: Invalid | ||
|
Comment on lines
+36
to
+45
Contributor
There was a problem hiding this comment. Ok, so lets make the overrides a variable too And then this expression can be |
||
|
|
||
| # Validate that certificate names follow DNS subdomain rules | ||
| - expression: | | ||
| !has(object.spec.certificateManagement.custom.overrides) || | ||
| object.spec.certificateManagement.custom.overrides.all(override, | ||
| override.certificateName.matches('^[a-z0-9]([-a-z0-9]*[a-z0-9])?$') | ||
| ) | ||
| message: "certificateName must be a valid DNS subdomain (lowercase alphanumeric with hyphens)" | ||
| reason: Invalid | ||
|
Comment on lines
+47
to
+54
Contributor
There was a problem hiding this comment. This should be within the API itself, you don't need a VAP for this |
||
|
|
||
| --- | ||
| # ValidatingAdmissionPolicyBinding that applies the validation policy | ||
| apiVersion: admissionregistration.k8s.io/v1 | ||
| kind: ValidatingAdmissionPolicyBinding | ||
| metadata: | ||
| name: validate-pki-certificate-overrides.config.openshift.io | ||
| spec: | ||
| policyName: validate-pki-certificate-overrides.config.openshift.io | ||
| validationActions: ["Deny"] | ||
|
|
||
| # Bind to all PKICertificateDefinition resources in openshift-config namespace | ||
| paramRef: | ||
| name: "" # Empty means all resources of the paramKind | ||
| namespace: openshift-config | ||
| # If no PKICertificateDefinition resources exist, allow the PKI resource | ||
| # This prevents blocking PKI resource creation before any components have registered | ||
| parameterNotFoundAction: Allow | ||
|
|
||
| # Match all PKI resources | ||
| matchResources: | ||
| namespaceSelector: | ||
| matchExpressions: | ||
| - key: kubernetes.io/metadata.name | ||
| operator: In | ||
| values: [""] # Empty string matches cluster-scoped resources | ||
|
Comment on lines
+78
to
+80
Contributor
There was a problem hiding this comment. Are you sure it's metadata.name and not metadata.namespace? |
||
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
Oops, something went wrong.
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.
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.
Taking a closer look at the validations you are trying to enforce here, I don't believe that a VAP is actually suitable for your use case.
I could be wrong but as far as I understand it, when using parameter references that reference multiple parameter resources the policy is evaluated one parameter resource at a time. I think this makes the validation you are trying to achieve (rejecting PKI resources with
.spec.certificateManagement.custom.overridesthat do not have correspondingPKICertificateDefinitionresource entries) not feasible because you won't have a list of the dependent resources to build your validation from.If you truly want to pursue this approach of having validations that are dependent on the presence of values in another API you'll either need a validating admission webhook or to update our KAS admission plugins to enforce this validation.