openshift
API-1844: Update KMSConfig to allow manualy managed kms plugins
KMSConfig is gated behind KMSEncryptionProvider feature gate which is in tech preview. This PR tries to add the changes proposed in https://github.com/openshift/enhancements/pull/1900 EP.
This PR;
- deprecates
AWSKMSConfigfield andAWSKMSProviderType, since they are not used by anywhere already (we'll tombstone them in next release) - adds new
ManualKMSProviderType andNamefield for manually managed KMS plugins
Note: We'll have to backport these changes into 4.21.
Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.
For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.
This repository is configured in: LGTM mode
@ardaguclu: This pull request references API-1844 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 task to target the "4.21.0" version, but no target version was set.
In response to this:
KMSConfigis gated behindKMSEncryptionProviderfeature gate which is currently in tech preview. However, currently there isn't any controller listeningKMSConfigandKMSEncryptionProvider. So that gives us an opportunity to change the API fields in a breaking way, according to the newest updates in this feature.
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.
Hello @ardaguclu! Some important instructions when contributing to openshift/api: API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.
![openshift-ci[bot] avatar](https://avatars.githubusercontent.com/in/91280?v=4 "Published by a pub.dev verified publisher"){kind=small}
📝 Walkthrough
Walkthrough
Removes KMS-related API surface: deletes KMSConfig and AWSKMSConfig types, their deepcopy helpers, Swagger/OpenAPI definitions, and the KMS field from APIServerEncryption. Updates generated, feature-gated CRD manifests to replace AWS KMS config with an External endpoint-based model (adds endpoint and managementModel, restricts kms.type to External). Payload CRD manifests remove the entire kms subtree. Adjusts tests and feature-gate YAML formatting for KMSEncryptionProvider.
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description check | ✅ Passed | The description directly addresses the changeset by explaining the deprecation of AWSKMSConfig and AWS KMSProviderType, and the addition of Manual KMSProviderType and Name field for manually managed KMS plugins. |
| Title check | ✅ Passed | The title refers to updating KMSConfig to allow manually managed KMS plugins, which aligns with the main objective of the PR: deprecating AWS KMS and adding support for manually managed KMS plugins via a new Manual KMSProviderType and Name field. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- [ ] 📝 Generate docstrings
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot] avatar](https://avatars.githubusercontent.com/in/347564?v=4 "Published by a pub.dev verified publisher"){kind=small}
@ardaguclu: This pull request references API-1844 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 task to target the "4.21.0" version, but no target version was set.
In response to this:
KMSConfigis gated behindKMSEncryptionProviderfeature gate which is in tech preview. However, currently there isn't any controller listeningKMSConfigandKMSEncryptionProvider. So that gives us an opportunity to change the API fields in a breaking way, according to the newest updates in this feature.
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.
@ardaguclu: This pull request references API-1844 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 task to target the "4.21.0" version, but no target version was set.
In response to this:
KMSConfigis gated behindKMSEncryptionProviderfeature gate which is in tech preview. However, currently there isn't any controller listeningKMSConfigandKMSEncryptionProvider. So that gives us an opportunity to change the API fields in a breaking way, according to the newest updates in this feature (AWS related kms configurations are obsolete, since we won't support it)
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.
@ardaguclu: This pull request references API-1844 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 task to target the "4.21.0" version, but no target version was set.
In response to this:
KMSConfigis gated behindKMSEncryptionProviderfeature gate which is in tech preview. However, currently there isn't any controller listeningKMSConfigandKMSEncryptionProvider. So that gives us an opportunity to change the API fields in a breaking way, according to the newest updates in this feature (AWS related kms configurations are obsolete, since we won't support it internally)
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.
@ardaguclu: This pull request references API-1844 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 task to target the "4.21.0" version, but no target version was set.
In response to this:
KMSConfigis gated behindKMSEncryptionProviderfeature gate which is in tech preview. However, currently there isn't any controller listeningKMSConfigandKMSEncryptionProvider. So that gives us an opportunity to change the API fields in a breaking way, according to the newest updates in this feature (AWS related kms configurations are obsolete, since we won't support it internally)Note: We'll have to backport these changes into 4.21.
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.
verify-crd-schema and verify-crdify failures are valid. Because we are removing aws related fields.
/cc @flavianmissi I'd like to hear your opinions. This aligns with the API definitions in your EP (External, Internal). Also we won't need to generate unix domain socket anymore. This will be our unique identifier.
I ran Claude api-review command on my local and it didn't find any issues.
Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn
/test e2e-aws-ovn-hypershift
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-techpreview
/test e2e-aws-serial-1of2
/test e2e-aws-serial-2of2
/test e2e-aws-serial-techpreview-1of2
/test e2e-aws-serial-techpreview-2of2
/test e2e-azure
/test e2e-gcp
/test e2e-upgrade
/test e2e-upgrade-out-of-change
@flavianmissi I've excluded abstract Linux paths from the validations (as we agreed upon). Could you PTAL?.
Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn
/test e2e-aws-ovn-hypershift
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-techpreview
/test e2e-aws-serial-1of2
/test e2e-aws-serial-2of2
/test e2e-aws-serial-techpreview-1of2
/test e2e-aws-serial-techpreview-2of2
/test e2e-azure
/test e2e-gcp
/test e2e-upgrade
/test e2e-upgrade-out-of-change
/lgtm cancel /hold
@ardaguclu @flavianmissi While I appreciate the team review and LGTM, please don't /lgtm on this repo until an API approver has had a pass. We are trying to avoid needless CI cycles so our E2Es don't run until the lgtm label is added, odds are that an API reviewer will ask for some changes so these E2E cycles are not going to help for the moment.
Will get you a reviewer assigned
/lgtm cancel /hold
@ardaguclu @flavianmissi While I appreciate the team review and LGTM, please don't
/lgtmon this repo until an API approver has had a pass. We are trying to avoid needless CI cycles so our E2Es don't run until thelgtmlabel is added, odds are that an API reviewer will ask for some changes so these E2E cycles are not going to help for the moment.Will get you a reviewer assigned
Thank you for the comment. This doesn't look like the flow in other OCP repositories but the motivation makes sense to me. As prerequisite we need to merge our EP https://github.com/openshift/enhancements/pull/1900 first (@JoelSpeed I've added you as api-approver, but we need to complete our reviews within the team first).
No worries @ardaguclu, it's a different process that we are trialing and I expect more repos to take on in the future. I'm hoping we can make it more obvious later
@yuqi-zhang is one of our API reviewers who's training up, he will take a first review pass here and on the EP, and once he's happy I'll take a look
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: flavianmissi Once this PR has been reviewed and has the lgtm label, please ask for approval from joelspeed. For more information see the Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@ardaguclu: This pull request references API-1844 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 story to target the "4.22.0" version, but no target version was set.
In response to this:
KMSConfigis gated behindKMSEncryptionProviderfeature gate which is in tech preview. This PR tries to add the changes proposed in https://github.com/openshift/enhancements/pull/1900 EP.Note: We'll have to backport these changes into 4.21.
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.
@ardaguclu: This pull request references API-1844 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 story to target the "4.22.0" version, but no target version was set.
In response to this:
KMSConfigis gated behindKMSEncryptionProviderfeature gate which is in tech preview. This PR tries to add the changes proposed in https://github.com/openshift/enhancements/pull/1900 EP.This PR;
- deprecates
AWSKMSConfigfield and AWSKMSProviderType, since they are not used by anywhere already (we'll tombstone them in next release)- adds new Manual
KMSProviderTypeandNamefield for manually managed KMS pluginsNote: We'll have to backport these changes into 4.21.
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.
@ardaguclu: This pull request references API-1844 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 story to target the "4.22.0" version, but no target version was set.
In response to this:
KMSConfigis gated behindKMSEncryptionProviderfeature gate which is in tech preview. This PR tries to add the changes proposed in https://github.com/openshift/enhancements/pull/1900 EP.This PR;
- deprecates
AWSKMSConfigfield andAWSKMSProviderType, since they are not used by anywhere already (we'll tombstone them in next release)- adds new
ManualKMSProviderType andNamefield for manually managed KMS pluginsNote: We'll have to backport these changes into 4.21.
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.
@ardaguclu: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| ci/prow/e2e-aws-serial-techpreview-1of2 | ab46a2aec16cfc361e32ece4b4e0afa0f89a4005 | link | true | /test e2e-aws-serial-techpreview-1of2 |
| ci/prow/e2e-aws-serial-techpreview-2of2 | ab46a2aec16cfc361e32ece4b4e0afa0f89a4005 | link | true | /test e2e-aws-serial-techpreview-2of2 |
| ci/prow/verify-crdify | a63a57ff2ca725597faf49301d5d33cdb86a2736 | link | true | /test verify-crdify |
| ci/prow/verify-crd-schema | a63a57ff2ca725597faf49301d5d33cdb86a2736 | link | true | /test verify-crd-schema |
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.
@ardaguclu: This pull request references CNTRLPLANE-2241 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 story to target the "4.22.0" version, but no target version was set.
In response to this:
KMSConfigis gated behindKMSEncryptionProviderfeature gate which is in tech preview. This PR tries to add the changes proposed in https://github.com/openshift/enhancements/pull/1900 EP.This PR;
- deprecates
AWSKMSConfigfield andAWSKMSProviderType, since they are not used by anywhere already (we'll tombstone them in next release)- adds new
ManualKMSProviderType andNamefield for manually managed KMS pluginsNote: We'll have to backport these changes into 4.21.
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.
As we agreed upon, we don't make any API changes in tech preview v1 to save some PRs for the backports. We'll simply ignore the KMSConfig. /close
@ardaguclu: Closed this PR.
In response to this:
As we agreed upon, we don't make any API changes in tech preview v1 to save some PRs for the backports. We'll simply ignore the KMSConfig. /close
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.