Add support for a Kubernetes secret in Image Policy #1715
Conversation
|
Skipping CI for Draft Pull Request. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
[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.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
enhancements/api-review/add-ClusterImagePolicy-and-ImagePolicy-for-Signature-Verification.md
Outdated
Show resolved
Hide resolved
enhancements/api-review/add-ClusterImagePolicy-and-ImagePolicy-for-Signature-Verification.md
Outdated
Show resolved
Hide resolved
| @@ -315,6 +338,69 @@ type ImagePolicyStatus struct { | |||
|
|
|||
| ### Implementation Details/Notes/Constraints [optional] | |||
|
|
|||
| #### Populating PolicyType using a Kuberentes Secret | |||
|
|
|||
| When the Machine Config Operator (MCO) detects a valid (non-nil) reference to a Kubernetes Secret, `ImagePolicySecretRef` in `PolicyRootOfTrust`, it will fetch the Secret and validate the keys within it according to the specified `PolicyType`. Based on this validation, the `MCO` will populate the corresponding `PolicyType` fields. For example, if the `PolicyType` is set to `PublicKey`, it will extract and validate the `public-key` and `rekor-key` from the Secret, populating the `PublicKey` struct appropriately. Similarly, for other types like `PKI` or `FulcioCAWithRekor`, the MCO will validate and populate fields from the Secret as needed. | |||
There was a problem hiding this comment.
Can we outline how the validation works? Which fields are optional and required.
There was a problem hiding this comment.
Existing API already has very extensive validation for every field. Any invalidate input would get caught right at the time of populating those structs, we just have to ensure the error is bubbled up correctly while we implement this feature in MCO.
| SecretPKIRootsDataKey = "ca-roots" // ca-roots | ||
| SecretPKIIntermediatesDataKey = "ca-intermediates" // ca-intermediates | ||
| SecretPKICertificateEmailKey = "certificate-email" // certificate-email | ||
| SecretPKICertificateHostnameKey = "certificate-hostname" // certificate-hostname |
There was a problem hiding this comment.
Do we have use cases where the data would be provided by different users? For example, ca-roots could be provided by secret A and the rest of the data by secret B?
There was a problem hiding this comment.
I don't see a reason for having multiple secrets per image policy. All we are doing here is, replacing someone manually setting those fields with a kubernetes secret owned by the same user. Also IMO, it is a much simpler design for users to follow. All they have to make sure is to have their external key provider (such as Vault) store the certs with the keys defined in this API. When an external secret operator (or Vault operator) will try to create the kubernetes secret it will automatically set the keys of the secret data to match the keys stored in their custody.
There was a problem hiding this comment.
It seems to me that this is just asking the same question in a different way: Does the external key provider always contain the complete set of keys for the policy?
- Is it plausible that the keys could be maintained by different teams (e.g. one department maintaining Rekor, and another maintaining the actual signing mechanism), i.e. different KMS accounts, and perhaps even different KMS servers?
- Are the KMS keys going to be maintained as a single unit in this sense? My first guess is that a KMS would contain, in one “managed item”, both the public+private key for Rekor; and in another “managed item”, both the public+private key for signing. Not that the KMS would store all policy keys for a single signing scope in one “managed item” — if nothing else, it is possible to use the same Rekor server for several scopes.
In both cases, the current design seems to assume that the controller syncing the KMS data into a Secret can extract data from KMS, and merge multiple KMS items, into one Secret. I don’t know what controller/operator is being used or planned to be used, and I’m perfectly happy to stay out of this, I’d just like to hear that yes, this has been tried once and such a syncing setup can be configured.
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
Signed-off-by: Harshal Patil <[email protected]>
|
@harche: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
| @@ -315,6 +337,69 @@ type ImagePolicyStatus struct { | |||
|
|
|||
| ### Implementation Details/Notes/Constraints [optional] | |||
|
|
|||
| #### Populating PolicyType using a Kuberentes Secret | |||
|
|
|||
| When the Machine Config Operator (MCO) detects a valid (non-nil) reference to a Kubernetes Secret, `ImagePolicySecret` in `PolicyRootOfTrust`, it will fetch the Secret and validate the keys within it according to the specified `PolicyType`. Based on this validation, the `MCO` will populate the corresponding `PolicyType` fields. For example, if the `PolicyType` is set to `PublicKey`, it will extract and validate the `public-key` and `rekor-key` from the Secret, populating the `PublicKey` struct appropriately. Similarly, for other types like `PKI` or `FulcioCAWithRekor`, the MCO will validate and populate fields from the Secret as needed. | |||
There was a problem hiding this comment.
Can the user still specify some values directly if a secret is used for other values?
If so, what happens if both the CR and the secret contain a value for an item?
| SecretPKICertificateEmailKey = "certificate-email" | ||
| SecretPKICertificateHostnameKey = "certificate-hostname" |
| SecretFulcioOidcIssuerKey = "oidc-issuer" | ||
| SecretFulcioSignedEmailKey = "signed-email" |
There was a problem hiding this comment.
(A) … here we are storing non-cryptographic strings in the KMS. Is that (possible at all) and necessary?
E.g. I could well imagine that there would be quite a few policies within a company, all referencing the same Rekor keys and Fulcio/PKI CAs, but with a different certificate subject for each registry namespace (e.g. one certificate subject per a project team, signing builds of that project). It makes sense to manage the cryptographic keys in a specialized software, but the registry repo / certificate subject mapping is something a bit different.
This rather depends on the “Can the user still specify some values directly” question — if the answer to that is “yes”, and this is only an option to store these parts of the policy in a Secret, I don’t see a reason to object. If the data had to be only stored in a Secret, that intuitively feels unnatural to me.
xref : https://issues.redhat.com/browse/OCPNODE-2674