-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OCPBUGS-2895: Azure: Fix DiskEncryptionSet regex validation #6513
OCPBUGS-2895: Azure: Fix DiskEncryptionSet regex validation #6513
Conversation
@@ -47,7 +45,7 @@ func TestValidateDiskEncryption(t *testing.T) { | |||
name: "invalid disk encryption set (platform is stack cloud)", | |||
pool: validDiskEncryptionMachinePool(), | |||
cloudName: azure.StackCloud, | |||
expected: fmt.Sprintf(`diskEncryptionSet.diskEncryptionSet: Invalid value: azure.DiskEncryptionSet{SubscriptionID:"%s", ResourceGroup:"%s", Name:"%s"}: disk encryption sets are not supported on this platform`, subscriptionID, resourceGroup, diskEncryptionSetName), | |||
expected: fmt.Sprintf(`diskEncryptionSet.diskEncryptionSet: Invalid value: azure.DiskEncryptionSet{SubscriptionID:"%s", ResourceGroup:".+", Name:"%s"}: disk encryption sets are not supported on this platform`, subscriptionID, diskEncryptionSetName), |
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.
Unfortunately, this string is used as a regex itself, and since RGs allow characters like .
, (
, and )
, we can't use that and still get a match.
/retitle OCPBUGS-2895: Azure: Fix DiskEncryptionSet regex validation |
@SudoBrendan: This pull request references Jira Issue OCPBUGS-2895, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In 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 kubernetes/test-infra repository. |
/jira refresh |
@r4f4: This pull request references Jira Issue OCPBUGS-2895, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: In 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 kubernetes/test-infra repository. |
pre-merge build: registry.build05.ci.openshift.org/ci-ln-frpjg32/release:latest |
So the keyvault name is too long?
Where is that name coming from? |
/retest |
with the cluster_name_prefix=maxu-e2e-V1234 |
You cannot use uppercase character in |
Just for a bit of clarity: the new functionality we are testing should be to have a DiskEncryptionSet that includes any upper-case letters in its name. In addition, we should test a DiskEncrptionSet that resides within a ResourceGroup with upper-case letters. All other fields in the installer-config should be conforming to their standard validations. I did also modify the code to accept upper-case letters in the subscription ID, so it might be worth adding those as well. |
how to test the bug , now the subdomain domain name can not contains uppercase, blocked use uppercase in the cluster name? |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: patrickdillon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/cherry-pick release-4.11 |
@cblecker: once the present PR merges, I will cherry-pick it on top of release-4.11 in a new PR and assign it to you. In 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 kubernetes/test-infra repository. |
/skip |
@openshift-bot: This pull request references Jira Issue OCPBUGS-2895, which is invalid:
Comment In 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 kubernetes/test-infra repository. |
/jira refresh |
@r4f4: This pull request references Jira Issue OCPBUGS-2895, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In 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 kubernetes/test-infra repository. |
/cherry-pick release-4.12 |
@patrickdillon: once the present PR merges, I will cherry-pick it on top of release-4.12 in a new PR and assign it to you. In 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 kubernetes/test-infra repository. |
/label backport-risk-assessed |
/retest |
@SudoBrendan: The following tests failed, say
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/test-infra repository. I understand the commands that are listed here. |
This is a known error in the IBM cloud provider. We'll have to wait until Monday for it to be resolved. |
checked with
and
|
/label qe-approved |
/retest-required |
/override ci/prow/e2e-aws-ovn |
@sdodson: Overrode contexts on behalf of sdodson: ci/prow/e2e-aws-ovn, ci/prow/e2e-azure-ovn, ci/prow/images In 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 kubernetes/test-infra repository. |
ART is aware and putting in a temporary fix in openshift-eng/ocp-build-data#2129 |
@SudoBrendan: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-2895 has been moved to the MODIFIED state. In 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 kubernetes/test-infra repository. |
@cblecker: new pull request created: #6537 In 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 kubernetes/test-infra repository. |
@patrickdillon: new pull request created: #6538 In 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 kubernetes/test-infra repository. |
Current validation excludes upper-case letters. I expanded the test suite to cover some of the ugliest (legitimate) names possible.