CORS-4337: allow AWS Europe Sovereign Cloud partition#2708
CORS-4337: allow AWS Europe Sovereign Cloud partition#2708tthvo wants to merge 1 commit intoopenshift:masterfrom
Conversation
In AWS European Sovereign Cloud, ARNs begin with "arn:aws-eusc:". Thus, we need to update the validation to allow this new format. See https://docs.aws.eu/general/latest/gr/arns.html
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@tthvo: This pull request references CORS-4337 which is a valid jira issue. 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. |
|
Hello @tthvo! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
size/L
📝 WalkthroughWalkthroughThis change extends AWS partition support across the codebase to include aws-eusc (AWS Europe/EU-SC) in addition to existing partitions (aws, aws-cn, aws-us-gov). The modification updates AWS ARN validation patterns, test cases, and documentation to accommodate the new partition. Specifically, regex patterns in DNS and KMS configuration validation are updated, test cases are added to verify functionality across all supported partitions, and CRD schemas are revised to reflect the expanded ARN format with explicit partition notation. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoSupport AWS Europe Sovereign Cloud partition in ARN validation
WalkthroughsDescription• Add support for AWS Europe Sovereign Cloud partition (aws-eusc) • Update ARN validation patterns for DNS and KMS configurations • Add comprehensive test cases for all AWS partitions • Update documentation and generated manifests Diagramflowchart LR
A["AWS Partitions<br/>aws, aws-cn, aws-us-gov, aws-eusc"] -->|Update Validation| B["DNS IAM Role ARN"]
A -->|Update Validation| C["KMS Key ARN"]
B -->|Pattern Match| D["^arn:partition:iam::account:role/.*$"]
C -->|Pattern Match| E["^arn:partition:kms:region:account:key/id$"]
B -->|Test Cases| F["All Partitions Validated"]
C -->|Test Cases| F
File Changes1. config/v1/types_dns.go
|
|
[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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@config/v1/types_kmsencryption.go`:
- Around line 26-36: Update the XValidation message for keyARN to accurately
describe allowed characters: reference the keyARN field and its
+kubebuilder:validation:XValidation rule and change the message text to state
that the region may contain lowercase letters, digits and hyphens and that the
key ID must be lowercase hexadecimal characters and hyphens; ensure the new
message keeps the format example
`arn:<partition>:kms:<region>:<account_id>:key/<key_id>` and mentions the
account ID must be 12 digits and the region is lowercase letters/digits/hyphens
while the key ID is lowercase hex and hyphens.
| // keyARN specifies the Amazon Resource Name (ARN) of the AWS KMS key used for encryption. | ||
| // The value must adhere to the format `arn:aws:kms:<region>:<account_id>:key/<key_id>`, where: | ||
| // The value must adhere to the format `arn:<partition>:kms:<region>:<account_id>:key/<key_id>`, where: | ||
| // - `<partition>` is the AWS partition (aws, aws-cn, aws-us-gov, or aws-eusc). | ||
| // - `<region>` is the AWS region consisting of lowercase letters and hyphens followed by a number. | ||
| // - `<account_id>` is a 12-digit numeric identifier for the AWS account. | ||
| // - `<key_id>` is a unique identifier for the KMS key, consisting of lowercase hexadecimal characters and hyphens. | ||
| // | ||
| // +kubebuilder:validation:MaxLength=128 | ||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:XValidation:rule="self.matches('^arn:aws:kms:[a-z0-9-]+:[0-9]{12}:key/[a-f0-9-]+$')",message="keyARN must follow the format `arn:aws:kms:<region>:<account_id>:key/<key_id>`. The account ID must be a 12 digit number and the region and key ID should consist only of lowercase hexadecimal characters and hyphens (-)." | ||
| // +kubebuilder:validation:XValidation:rule="self.matches('^arn:(aws|aws-cn|aws-us-gov|aws-eusc):kms:[a-z0-9-]+:[0-9]{12}:key/[a-f0-9-]+$')",message="keyARN must follow the format `arn:<partition>:kms:<region>:<account_id>:key/<key_id>`. The account ID must be a 12 digit number and the region and key ID should consist only of lowercase hexadecimal characters and hyphens (-)." | ||
| // +required |
There was a problem hiding this comment.
Fix the keyARN validation message: region isn’t “hexadecimal.”
The regex allows lowercase letters/digits/hyphens for region, so the message should reflect that (and keep hex for key ID).
🛠️ Suggested message tweak
-// +kubebuilder:validation:XValidation:rule="self.matches('^arn:(aws|aws-cn|aws-us-gov|aws-eusc):kms:[a-z0-9-]+:[0-9]{12}:key/[a-f0-9-]+$')",message="keyARN must follow the format `arn:<partition>:kms:<region>:<account_id>:key/<key_id>`. The account ID must be a 12 digit number and the region and key ID should consist only of lowercase hexadecimal characters and hyphens (-)."
+// +kubebuilder:validation:XValidation:rule="self.matches('^arn:(aws|aws-cn|aws-us-gov|aws-eusc):kms:[a-z0-9-]+:[0-9]{12}:key/[a-f0-9-]+$')",message="keyARN must follow the format `arn:<partition>:kms:<region>:<account_id>:key/<key_id>`. The account ID must be a 12 digit number; the region must contain lowercase letters, digits, and hyphens (-); and the key ID must contain lowercase hexadecimal characters and hyphens (-)."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // keyARN specifies the Amazon Resource Name (ARN) of the AWS KMS key used for encryption. | |
| // The value must adhere to the format `arn:aws:kms:<region>:<account_id>:key/<key_id>`, where: | |
| // The value must adhere to the format `arn:<partition>:kms:<region>:<account_id>:key/<key_id>`, where: | |
| // - `<partition>` is the AWS partition (aws, aws-cn, aws-us-gov, or aws-eusc). | |
| // - `<region>` is the AWS region consisting of lowercase letters and hyphens followed by a number. | |
| // - `<account_id>` is a 12-digit numeric identifier for the AWS account. | |
| // - `<key_id>` is a unique identifier for the KMS key, consisting of lowercase hexadecimal characters and hyphens. | |
| // | |
| // +kubebuilder:validation:MaxLength=128 | |
| // +kubebuilder:validation:MinLength=1 | |
| // +kubebuilder:validation:XValidation:rule="self.matches('^arn:aws:kms:[a-z0-9-]+:[0-9]{12}:key/[a-f0-9-]+$')",message="keyARN must follow the format `arn:aws:kms:<region>:<account_id>:key/<key_id>`. The account ID must be a 12 digit number and the region and key ID should consist only of lowercase hexadecimal characters and hyphens (-)." | |
| // +kubebuilder:validation:XValidation:rule="self.matches('^arn:(aws|aws-cn|aws-us-gov|aws-eusc):kms:[a-z0-9-]+:[0-9]{12}:key/[a-f0-9-]+$')",message="keyARN must follow the format `arn:<partition>:kms:<region>:<account_id>:key/<key_id>`. The account ID must be a 12 digit number and the region and key ID should consist only of lowercase hexadecimal characters and hyphens (-)." | |
| // +required | |
| // keyARN specifies the Amazon Resource Name (ARN) of the AWS KMS key used for encryption. | |
| // The value must adhere to the format `arn:<partition>:kms:<region>:<account_id>:key/<key_id>`, where: | |
| // - `<partition>` is the AWS partition (aws, aws-cn, aws-us-gov, or aws-eusc). | |
| // - `<region>` is the AWS region consisting of lowercase letters and hyphens followed by a number. | |
| // - `<account_id>` is a 12-digit numeric identifier for the AWS account. | |
| // - `<key_id>` is a unique identifier for the KMS key, consisting of lowercase hexadecimal characters and hyphens. | |
| // | |
| // +kubebuilder:validation:MaxLength=128 | |
| // +kubebuilder:validation:MinLength=1 | |
| // +kubebuilder:validation:XValidation:rule="self.matches('^arn:(aws|aws-cn|aws-us-gov|aws-eusc):kms:[a-z0-9-]+:[0-9]{12}:key/[a-f0-9-]+$')",message="keyARN must follow the format `arn:<partition>:kms:<region>:<account_id>:key/<key_id>`. The account ID must be a 12 digit number; the region must contain lowercase letters, digits, and hyphens (-); and the key ID must contain lowercase hexadecimal characters and hyphens (-)." | |
| // +required |
🤖 Prompt for AI Agents
In `@config/v1/types_kmsencryption.go` around lines 26 - 36, Update the
XValidation message for keyARN to accurately describe allowed characters:
reference the keyARN field and its +kubebuilder:validation:XValidation rule and
change the message text to state that the region may contain lowercase letters,
digits and hyphens and that the key ID must be lowercase hexadecimal characters
and hyphens; ensure the new message keeps the format example
`arn:<partition>:kms:<region>:<account_id>:key/<key_id>` and mentions the
account ID must be 12 digits and the region is lowercase letters/digits/hyphens
while the key ID is lowercase hex and hyphens.
Code Review by Qodo
1. PrivateZoneIAMRole pattern undocumented
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| // operations on the cluster's private hosted zone specified in the cluster DNS config. | ||
| // When left empty, no role should be assumed. | ||
| // +kubebuilder:validation:Pattern:=`^arn:(aws|aws-cn|aws-us-gov):iam::[0-9]{12}:role\/.*$` | ||
| // +kubebuilder:validation:Pattern:=`^arn:(aws|aws-cn|aws-us-gov|aws-eusc):iam::[0-9]{12}:role\/.*$` |
There was a problem hiding this comment.
1. privatezoneiamrole pattern undocumented 📘 Rule violation ✓ Correctness
The PrivateZoneIAMRole field comment does not document the updated kubebuilder Pattern constraint (including the newly-allowed aws-eusc partition). This violates the requirement that validation markers and their constraints be described in field comments.
Agent Prompt
## Issue description
`PrivateZoneIAMRole` has a kubebuilder `Pattern` validation that was modified to include `aws-eusc`, but the field comment does not document the constraint as required.
## Issue Context
Compliance requires that all kubebuilder validation markers and their constraints be documented in the corresponding field comments.
## Fix Focus Areas
- config/v1/types_dns.go[134-140]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // +kubebuilder:validation:MaxLength=128 | ||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:XValidation:rule="self.matches('^arn:aws:kms:[a-z0-9-]+:[0-9]{12}:key/[a-f0-9-]+$')",message="keyARN must follow the format `arn:aws:kms:<region>:<account_id>:key/<key_id>`. The account ID must be a 12 digit number and the region and key ID should consist only of lowercase hexadecimal characters and hyphens (-)." | ||
| // +kubebuilder:validation:XValidation:rule="self.matches('^arn:(aws|aws-cn|aws-us-gov|aws-eusc):kms:[a-z0-9-]+:[0-9]{12}:key/[a-f0-9-]+$')",message="keyARN must follow the format `arn:<partition>:kms:<region>:<account_id>:key/<key_id>`. The account ID must be a 12 digit number and the region and key ID should consist only of lowercase hexadecimal characters and hyphens (-)." |
There was a problem hiding this comment.
2. keyarn message mismatches regex 📘 Rule violation ⛯ Reliability
The keyARN validation error message claims the region must be lowercase hexadecimal, but the validation regex allows broader values; this is misleading and reduces actionable context. Users will receive incorrect guidance when validation fails.
Agent Prompt
## Issue description
The `keyARN` validation message is misleading: it says the region must be lowercase hexadecimal characters, but the validation rule allows broader region formats.
## Issue Context
This message is user-facing (as seen in config schema validation tests) and must provide accurate, actionable guidance.
## Fix Focus Areas
- config/v1/types_kmsencryption.go[26-36]
- config/v1/tests/apiservers.config.openshift.io/KMSEncryptionProvider.yaml[173-189]
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-CustomNoUpgrade.crd.yaml[177-193]
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-DevPreviewNoUpgrade.crd.yaml[177-193]
- config/v1/zz_generated.featuregated-crd-manifests/apiservers.config.openshift.io/KMSEncryptionProvider.yaml[177-193]
- payload-manifests/crds/0000_10_config-operator_01_apiservers-CustomNoUpgrade.crd.yaml[177-193]
- payload-manifests/crds/0000_10_config-operator_01_apiservers-DevPreviewNoUpgrade.crd.yaml[177-193]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // The value must adhere to the format `arn:<partition>:kms:<region>:<account_id>:key/<key_id>`, where: | ||
| // - `<partition>` is the AWS partition (aws, aws-cn, aws-us-gov, or aws-eusc). | ||
| // - `<region>` is the AWS region consisting of lowercase letters and hyphens followed by a number. | ||
| // - `<account_id>` is a 12-digit numeric identifier for the AWS account. | ||
| // - `<key_id>` is a unique identifier for the KMS key, consisting of lowercase hexadecimal characters and hyphens. | ||
| // | ||
| // +kubebuilder:validation:MaxLength=128 | ||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:XValidation:rule="self.matches('^arn:aws:kms:[a-z0-9-]+:[0-9]{12}:key/[a-f0-9-]+$')",message="keyARN must follow the format `arn:aws:kms:<region>:<account_id>:key/<key_id>`. The account ID must be a 12 digit number and the region and key ID should consist only of lowercase hexadecimal characters and hyphens (-)." | ||
| // +kubebuilder:validation:XValidation:rule="self.matches('^arn:(aws|aws-cn|aws-us-gov|aws-eusc):kms:[a-z0-9-]+:[0-9]{12}:key/[a-f0-9-]+$')",message="keyARN must follow the format `arn:<partition>:kms:<region>:<account_id>:key/<key_id>`. The account ID must be a 12 digit number and the region and key ID should consist only of lowercase hexadecimal characters and hyphens (-)." |
There was a problem hiding this comment.
3. Partition support incomplete 🐞 Bug ✓ Correctness
This PR adds aws-eusc support for APIServer KMS keyARN and DNS role ARNs, but AWS CSI driver kmsKeyARN validation still rejects aws-eusc, creating inconsistent behavior for EUSC clusters. Users may successfully configure control-plane encryption but be blocked from configuring storage-class KMS encryption via the CSI driver config CRD validation.
Agent Prompt
### Issue description
aws-eusc is now allowed for APIServer KMS `keyARN`, but AWS CSI driver `kmsKeyARN` still rejects aws-eusc because its partition allowlist omits it. This creates inconsistent AWS partition support across APIs on EUSC clusters.
### Issue Context
EUSC ARNs start with `arn:aws-eusc:`. After this PR, APIServer KMS config will accept that partition, but `clustercsidrivers.operator.openshift.io` schema validation will still reject it for `spec.driverConfig.aws.kmsKeyARN`.
### Fix Focus Areas
- operator/v1/types_csi_cluster_driver.go[161-168]
- operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-Default.crd.yaml[120-130]
- payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-Default.crd.yaml[120-130]
- operator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/AAA_ungated.yaml[100-112]
### Notes
After updating the kubebuilder Pattern, re-run generators so all generated CRDs/manifests stay in sync.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
@tthvo: all tests passed! 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. |
2 participants
According to AWS docs, ARNs in AWS European Sovereign Cloud begin with
Thus, to support EUS Cloud, we need to update the validation to allow this new format.