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
|
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
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughValidation for AWS ARN fields was changed: Go struct validation annotations for 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Code Review by Qodo
1. PrivateZoneIAMRole pattern undocumented
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
everettraven
left a comment
everettraven
left a comment
There was a problem hiding this comment.
Overall, this change seems reasonable.
Only question - is every component that relies on the input provided in these fields able to successfully handle the newly introduced partitions?
|
/hold
Ah sorry, I am the middle of checking these scenarios. So, I am holding this PR for now... Background: AWS Europe Sovereign Cloud (EUSC) requires AWS SDK v2 for out-of-the-box support (i.e. able to resolve the correct endpoint for EUSC regions). However, several cluster operators still use AWS SDK v1 (now EOL), and migrating them to SDK v2 is not feasible within the 4.22 timeline or near future. As a workaround, we're enabling EUSC support through user-provided service endpoints. I'm currently testing whether the new EUSC region honors these custom endpoints, or if only minor patches are needed to make it work. |
openshift-ci
bot
added
the
do-not-merge/hold
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
operator/v1/tests/clustercsidrivers.operator.openshift.io/AAA_ungated.yaml (1)
134-156: Consider adding positive test coverage for the remaining ISO-variant partitions.The validation regex includes
aws-iso-b,aws-iso-e, andaws-iso-f, but onlyaws-isohas a positive test case. Since this PR is already adding broad partition coverage, it would be a natural opportunity to close this gap.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@operator/v1/tests/clustercsidrivers.operator.openshift.io/AAA_ungated.yaml` around lines 134 - 156, Add positive test cases alongside the existing "Should be able to specify an AWS ISO KMS key ARN" case to cover the other ISO partitions: aws-iso-b, aws-iso-e, and aws-iso-f. For each new test, copy the existing initial/expected blocks (preserving apiVersion/kind/metadata/spec structure) and change the kmsKeyARN host partition portion to use arn:aws-iso-b:kms:..., arn:aws-iso-e:kms:..., and arn:aws-iso-f:kms:... respectively so the validation regex is exercised for each partition variant (match the format used in the original kmsKeyARN string).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@operator/v1/tests/clustercsidrivers.operator.openshift.io/AAA_ungated.yaml`:
- Around line 180-191: The negative test uses an ARN that matches the CRD regex,
so update the initial test case for "Should not be able to specify invalid AWS
KMS key ARN" by replacing spec.driverConfig.aws.kmsKeyARN with a deliberately
malformed ARN that fails the pattern—e.g. use an invalid partition name (not
matching arn:(aws|aws-cn|...)) or an account ID with incorrect digit count (not
12 digits); ensure you edit the initial block for the test case that sets
spec.driverConfig.aws.kmsKeyARN so the value no longer matches the regex and the
expectedError for spec.driverConfig.aws.kmsKeyARN triggers.
---
Nitpick comments:
In `@operator/v1/tests/clustercsidrivers.operator.openshift.io/AAA_ungated.yaml`:
- Around line 134-156: Add positive test cases alongside the existing "Should be
able to specify an AWS ISO KMS key ARN" case to cover the other ISO partitions:
aws-iso-b, aws-iso-e, and aws-iso-f. For each new test, copy the existing
initial/expected blocks (preserving apiVersion/kind/metadata/spec structure) and
change the kmsKeyARN host partition portion to use arn:aws-iso-b:kms:...,
arn:aws-iso-e:kms:..., and arn:aws-iso-f:kms:... respectively so the validation
regex is exercised for each partition variant (match the format used in the
original kmsKeyARN string).
operator/v1/tests/clustercsidrivers.operator.openshift.io/AAA_ungated.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
payload-manifests/crds/0000_10_config-operator_01_dnses.crd.yaml (1)
74-77: LGTM —aws-euscpartition and updated description are correct.The AWS European Sovereign Cloud is accessed using partition name
aws-eusc, so the regex addition is accurate. The documentation on lines 74–76 precisely maps to the updated pattern.One concern worth tracking (which the PR is already on hold for): the AWS European Sovereign Cloud uses a distinct domain namespace (
*.amazonaws.eu) separate from commercial AWS. Validating the ARN format here is a necessary first step, but ensure every downstream operator that consumesprivateZoneIAMRolecan resolve*.amazonaws.euendpoints — AWS SDK (Terraform AWS provider) 6.x and Terraform 1.14+ support ESC natively; older versions require manual endpoint configuration. Components still on AWS SDK v1 (EOL) will not resolve EUSC endpoints out of the box, which the PR discussion already flags.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@payload-manifests/crds/0000_10_config-operator_01_dnses.crd.yaml` around lines 74 - 77, CRD change adds the aws-eusc partition to the ARN regex for privateZoneIAMRole but downstream consumers may not resolve AWS European Sovereign Cloud endpoints; update the `privateZoneIAMRole` description (and any operator docs) to explicitly state the requirement that consumers support the aws-eusc (`*.amazonaws.eu`) endpoints and list the minimum SDK/provider versions (e.g., AWS SDK v2+/Terraform AWS provider 6.x, Terraform 1.14+) or add a validating webhook/annotation check to reject configurations when the operator runtime is on unsupported SDK/provider versions; reference `privateZoneIAMRole` and the updated ARN pattern in your edits so the note is colocated with the schema change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@payload-manifests/crds/0000_10_config-operator_01_dnses.crd.yaml`:
- Around line 74-77: CRD change adds the aws-eusc partition to the ARN regex for
privateZoneIAMRole but downstream consumers may not resolve AWS European
Sovereign Cloud endpoints; update the `privateZoneIAMRole` description (and any
operator docs) to explicitly state the requirement that consumers support the
aws-eusc (`*.amazonaws.eu`) endpoints and list the minimum SDK/provider versions
(e.g., AWS SDK v2+/Terraform AWS provider 6.x, Terraform 1.14+) or add a
validating webhook/annotation check to reject configurations when the operator
runtime is on unsupported SDK/provider versions; reference `privateZoneIAMRole`
and the updated ARN pattern in your edits so the note is colocated with the
schema change.
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@tthvo It might make sense for us to feature gate this change so that we can merge the validation change and add automated regression testing that helps us identify cases where this causes things to break. From there, we can update things accordingly under the same feature gate. |
Right... I am just a bit hesitant if we should (or have ever) create a feature gate for installing into a (new) region. I'll double check with @patrickdillon when he's back next week. My understand is that allowing
To answer your original question, after some testing, I'd say yes with a small adjustment to make sure the operators honour necessary custom service endpoints, which they really should. They may have missed it when adding these arn input fields previously - bug? The following API fields are updated to allow
|
|
@everettraven Thanks for having a look! I added my views and (local) test results above. Please let me know what you think 🙏 I'll check with Patrick about the feature gate when he's back. |
This commit adds CI infrastructure support for AWS European Sovereign Cloud (EUSC) testing using the eusc-de-east-1 region. Changes: - Add cluster-secrets-aws-eusc-qe to secret bootstrap config - Add aws-eusc-qe-quota-slice boskos resource pool with 5 quota slices in eusc-de-east-1 region - Generate updated _boskos.yaml configuration Region Details: - Region: eusc-de-east-1 (Brandenburg, Germany) - Availability zones: eusc-de-east-1a, eusc-de-east-1b (2 zones only) - Note: No edge zones (Local/Wavelength) available in this region Dependencies: - Installer support: openshift/installer#10303 - Ingress operator support: openshift/cluster-ingress-operator#1360 - API support (optional): openshift/api#2708
This commit adds CI infrastructure support for AWS European Sovereign Cloud (EUSC) testing using the eusc-de-east-1 region. Changes: - Add cluster-secrets-aws-eusc-qe to secret bootstrap config - Add aws-eusc-qe-quota-slice boskos resource pool with 5 quota slices in eusc-de-east-1 region - Generate updated _boskos.yaml configuration Region Details: - Region: eusc-de-east-1 (Brandenburg, Germany) - Availability zones: eusc-de-east-1a, eusc-de-east-1b (2 zones only) - Note: No edge zones (Local/Wavelength) available in this region Dependencies: - Installer support: openshift/installer#10303 - Ingress operator support: openshift/cluster-ingress-operator#1360 - API support (optional): openshift/api#2708
This commit adds CI infrastructure support for AWS European Sovereign Cloud (EUSC) testing using the eusc-de-east-1 region. Changes: - Add cluster-secrets-aws-eusc-qe to secret bootstrap config - Add aws-eusc-qe-quota-slice boskos resource pool with 5 quota slices in eusc-de-east-1 region - Generate updated _boskos.yaml configuration Region Details: - Region: eusc-de-east-1 (Brandenburg, Germany) - Availability zones: eusc-de-east-1a, eusc-de-east-1b (2 zones only) - Note: No edge zones (Local/Wavelength) available in this region Dependencies: - Installer support: openshift/installer#10303 - Ingress operator support: openshift/cluster-ingress-operator#1360 - API support (optional): openshift/api#2708
This commit adds CI infrastructure support for AWS European Sovereign Cloud (EUSC) testing using the eusc-de-east-1 region. Changes: - Add cluster-secrets-aws-eusc-qe to secret bootstrap config - Add aws-eusc-qe-quota-slice boskos resource pool with 5 quota slices in eusc-de-east-1 region - Generate updated _boskos.yaml configuration Region Details: - Region: eusc-de-east-1 (Brandenburg, Germany) - Availability zones: eusc-de-east-1a, eusc-de-east-1b (2 zones only) - Note: No edge zones (Local/Wavelength) available in this region Dependencies: - Installer support: openshift/installer#10303 - Ingress operator support: openshift/cluster-ingress-operator#1360 - API support (optional): openshift/api#2708
This commit adds CI infrastructure support for AWS European Sovereign Cloud (EUSC) testing using the eusc-de-east-1 region. Changes: - Add cluster-secrets-aws-eusc-qe to secret bootstrap config - Add aws-eusc-qe-quota-slice boskos resource pool with 5 quota slices in eusc-de-east-1 region - Generate updated _boskos.yaml configuration Region Details: - Region: eusc-de-east-1 (Brandenburg, Germany) - Availability zones: eusc-de-east-1a, eusc-de-east-1b (2 zones only) - Note: No edge zones (Local/Wavelength) available in this region Dependencies: - Installer support: openshift/installer#10303 - Ingress operator support: openshift/cluster-ingress-operator#1360 - API support (optional): openshift/api#2708
openshift-ci
bot
added
size/XXL
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
machineconfiguration/v1/tests/controllerconfigs.machineconfiguration.openshift.io/AWSEuropeanSovereignCloudInstall.yaml (1)
7-104: Add gate-specific aws-eusc ARN coverage in this fixture.This gate-scoped file currently validates only a minimal create path, so it does not actually assert
AWSEuropeanSovereignCloudInstallbehavior for ControllerConfig. Please add at least one explicitspec.dns.spec.platform.aws.privateZoneIAMRolecase usingarn:aws-eusc:...(and ideally one invalid-partition rejection case) to make this test meaningful.Suggested test addition (example)
tests: onCreate: @@ - name: Should be able to create a minimal ControllerConfig initial: | ... expected: | ... + - name: Should allow aws-eusc privateZoneIAMRole when feature gate is enabled + initial: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: ControllerConfig + spec: + ... + dns: + apiVersion: config.openshift.io/v1 + kind: DNS + spec: + platform: + type: AWS + aws: + privateZoneIAMRole: arn:aws-eusc:iam::123456789012:role/my-role + expected: | + apiVersion: machineconfiguration.openshift.io/v1 + kind: ControllerConfig + spec: + ... + dns: + apiVersion: config.openshift.io/v1 + kind: DNS + spec: + platform: + type: AWS + aws: + privateZoneIAMRole: arn:aws-eusc:iam::123456789012:role/my-roleAs per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@machineconfiguration/v1/tests/controllerconfigs.machineconfiguration.openshift.io/AWSEuropeanSovereignCloudInstall.yaml` around lines 7 - 104, Add gate-specific aws-eusc ARN coverage by updating the onCreate test cases for ControllerConfig: add at least one test (under the existing onCreate block or a new named test) that sets spec.dns.spec.platform.aws.privateZoneIAMRole to a valid aws-eusc ARN (e.g., "arn:aws-eusc:...") and include the same value in the expected output; also add a negative test that supplies an invalid partition ARN (e.g., wrong partition prefix) and asserts rejection. Locate and modify the initial/expected YAML for the "Should be able to create a minimal ControllerConfig" or add a new test entry so it references spec.dns.spec.platform.aws.privateZoneIAMRole and validates both the valid arn:aws-eusc:... case and an invalid-partition rejection case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/v1/types_dns.go`:
- Around line 137-138: The unconditional validation annotation that rejects
aws-eusc (the line with FeatureGateAwareXValidation:featureGate="") must be
removed/converted so it does not coexist with the gate-enabled validator;
instead ensure there are two mutually-exclusive validators for the same field:
one that applies only when AWSEuropeanSovereignCloudInstall is disabled (the
rule that excludes aws-eusc) and the other that applies only when
AWSEuropeanSovereignCloudInstall is enabled (the rule that includes aws-eusc).
Update the FeatureGateAwareXValidation annotations around the IAM role ARN (the
two rules shown) so the base “reject aws-eusc” rule is not unconditional but
tied to the inverse/disabled state and the gate-enabled rule
(featureGate=AWSEuropeanSovereignCloudInstall) replaces it when the gate is
enabled, preventing both validators from being appended simultaneously.
In
`@payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yaml`:
- Around line 129-133: The two x-kubernetes-validations entries are conjunctive
so the first rule rejects aws-eusc ARNs even though the second allows them;
update the validation to use a single rule that accepts both by merging the
patterns (include aws-eusc in the alternation) or remove the first redundant
rule so only the correct pattern remains—target the x-kubernetes-validations
block and adjust the rule string(s) where the KMS key ARN regex is defined to
include aws-eusc in the allowed provider list.
---
Nitpick comments:
In
`@machineconfiguration/v1/tests/controllerconfigs.machineconfiguration.openshift.io/AWSEuropeanSovereignCloudInstall.yaml`:
- Around line 7-104: Add gate-specific aws-eusc ARN coverage by updating the
onCreate test cases for ControllerConfig: add at least one test (under the
existing onCreate block or a new named test) that sets
spec.dns.spec.platform.aws.privateZoneIAMRole to a valid aws-eusc ARN (e.g.,
"arn:aws-eusc:...") and include the same value in the expected output; also add
a negative test that supplies an invalid partition ARN (e.g., wrong partition
prefix) and asserts rejection. Locate and modify the initial/expected YAML for
the "Should be able to create a minimal ControllerConfig" or add a new test
entry so it references spec.dns.spec.platform.aws.privateZoneIAMRole and
validates both the valid arn:aws-eusc:... case and an invalid-partition
rejection case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4d9914cc-47c7-4448-a1d0-053442dd5401
⛔ Files ignored due to path filters (35)
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_dnses-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*config/v1/zz_generated.featuregated-crd-manifests/dnses.config.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**config/v1/zz_generated.featuregated-crd-manifests/dnses.config.openshift.io/AWSEuropeanSovereignCloudInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_controllerconfigs-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AWSClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AWSDualStackInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AWSEuropeanSovereignCloudInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AzureClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/AzureDualStackInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/DualReplica.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/DyanmicServiceEndpointIBMCloud.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/GCPClusterHostedDNSInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/HighlyAvailableArbiter+DualReplica.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/HighlyAvailableArbiter.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/NutanixMultiSubnets.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/OnPremDNSRecords.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/VSphereHostVMGroupZonal.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/controllerconfigs.machineconfiguration.openshift.io/VSphereMultiNetworks.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**operator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/AWSEuropeanSovereignCloudInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**operator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/VSphereConfigurableMaxAllowedBlockVolumesPerNode.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**
📒 Files selected for processing (14)
config/v1/tests/dnses.config.openshift.io/AWSEuropeanSovereignCloudInstall.yamlconfig/v1/types_dns.goconfig/v1/zz_generated.featuregated-crd-manifests.yamlmachineconfiguration/v1/tests/controllerconfigs.machineconfiguration.openshift.io/AWSEuropeanSovereignCloudInstall.yamlmachineconfiguration/v1/zz_generated.featuregated-crd-manifests.yamloperator/v1/tests/clustercsidrivers.operator.openshift.io/AWSEuropeanSovereignCloudInstall.yamloperator/v1/types_csi_cluster_driver.gooperator/v1/zz_generated.featuregated-crd-manifests.yamlpayload-manifests/crds/0000_10_config-operator_01_dnses-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_dnses-Default.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_dnses-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_dnses-OKD.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_dnses-TechPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yaml
✅ Files skipped from review due to trivial changes (1)
- payload-manifests/crds/0000_10_config-operator_01_dnses-TechPreviewNoUpgrade.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- operator/v1/types_csi_cluster_driver.go
payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yaml
Outdated
Show resolved
Hide resolved
|
/jira refresh |
|
@liweinan: 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. |
This commit adds CI infrastructure support for AWS European Sovereign Cloud (EUSC) testing using the eusc-de-east-1 region. Changes: - Add cluster-secrets-aws-eusc-qe to secret bootstrap config - Add aws-eusc-qe-quota-slice boskos resource pool with 5 quota slices in eusc-de-east-1 region - Generate updated _boskos.yaml configuration Region Details: - Region: eusc-de-east-1 (Brandenburg, Germany) - Availability zones: eusc-de-east-1a, eusc-de-east-1b (2 zones only) - Note: No edge zones (Local/Wavelength) available in this region Dependencies: - Installer support: openshift/installer#10303 - Ingress operator support: openshift/cluster-ingress-operator#1360 - API support (optional): openshift/api#2708
|
/hold Figuring out why some manifests have conflicting xvalidation regex check 😞 |
openshift-ci
bot
added
the
do-not-merge/hold
|
You may need to update https://github.com/openshift/kubernetes-sigs-controller-tools/blob/master/pkg/crd/markers/patch_validation.go to support negation of feature gates for the XValidation marker to support this. I don't think we have had this use case before |
|
@JoelSpeed I think you should be able to do the negation semantics for XValidations. We did it in api/config/v1/types_authentication.go Lines 353 to 356 in 5c75e62 |
This commit adds CI infrastructure support for AWS European Sovereign Cloud (EUSC) testing using the eusc-de-east-1 region. Changes: - Add cluster-secrets-aws-eusc-qe to secret bootstrap config - Add aws-eusc-qe-quota-slice boskos resource pool with 5 quota slices in eusc-de-east-1 region - Generate updated _boskos.yaml configuration Region Details: - Region: eusc-de-east-1 (Brandenburg, Germany) - Availability zones: eusc-de-east-1a, eusc-de-east-1b (2 zones only) - Note: No edge zones (Local/Wavelength) available in this region Dependencies: - Installer support: openshift/installer#10303 - Ingress operator support: openshift/cluster-ingress-operator#1360 - API support (optional): openshift/api#2708
|
I did a bit more digging as to why the example I shared previously seems to work but the same thing here might not be working. It looks like because api/tools/codegen/pkg/manifestmerge/crd-schema.json Lines 719 to 736 in 54a3998 Because the keys for the map here are unique, and our manifest merge logic uses a different field manager based on the CRD path ( ), the merged rules is what the "correct" behavior is here.@JoelSpeed This seems like expected behavior based on our tooling, but I'm doing some additional exploration to see if we should modify the CRD schema semantics we pass through to our tooling to enable this case. |
|
@tthvo I've had a chat with @JoelSpeed and it seems reasonable for us to make some tooling changes to address this. I've got #2770 up as a draft that I'm going to get polished up and take it out of draft to unblock you. |
|
Ahha, thank you guys! That sounds good to me! Phew, I couldn't get it to work all day yesterday 😅 I'll regenerate the manifests asap after 🙏 |
tthvo
force-pushed
the
CORS-4337
branch
2 times, most recently
from
f3e6f83 to
808bc50
Compare
According to AWS docs, ARNs in AWS European Sovereign Cloud begin with arn:aws-eusc: Thus, to support EUS Cloud, we need to update the validation to allow this new format. This commits only focuses on kmsKeyARN and privateZoneIAMRole.
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@everettraven @JoelSpeed I think this is good for another review. PTAL 👍 |
|
@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. |
6 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.