Skip to content
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

Update KeyVault to enable live testing in sovereign clouds for multiple services #25760

Merged
merged 10 commits into from
Dec 21, 2021
Merged

Update KeyVault to enable live testing in sovereign clouds for multiple services #25760

merged 10 commits into from
Dec 21, 2021

Conversation

v-hongli1
Copy link
Contributor

@v-hongli1 v-hongli1 commented Dec 1, 2021

These changes enable KeyVault to run live tests against Public, UsGov and China.

  1. Update file: test-resources.json
    i) Match with the ArmTemplateParameter setting, so change the parameter name of the endpoint suffix to keyVaultEndpointSuffix.
    ii) Modified permission of access policies for fix The user, group or application 'appid=***;oid=***;iss=https://sts.windows.net/***/' does not have keys getrotationpolicy permission on key vault 't1e479d51bbe44e7d;location=***'. For help resolving this issue, please see https://go.microsoft.com/fwlink/?linkid=2125287 excception.
    iii) Fix the execution exception of AccessTokenUtilTest.testGetAuthorizationToken in UsGov and China Cloud, add output parameter KEY_VAULT_ENDPOINT_SUFFIX

  2. Update file: /keys/KeyClientTestBase.java
    According to the repair suggestions in issue#18142, add the cloud test platform judgment method isPublicCloud .

  3. Update file: /keys/KeyClientTest.java, /keys/KeyAsyncClientTest.java
    According to the repair suggestions in issue#18142, skip Key Rotation related test cases in UsGov and China cloud.

  4. Update file: /jca/AccessTokenUtilTest.java
    Fix the execution exception of AccessTokenUtilTest.testGetAuthorizationToken in UsGov and China Cloud, and modify the parameter setting of AccessTokenUtil.getAccessToken.

  5. Update file: /certificates/CertificateClientTest.java
    i) Fixed the org.opentest4j.AssertionFailedError: expected: <false> but was: <true> exception in UsGov and China cloud.
    ii) Adjust the process flow to avoid waitForCompletion() to interfere with result data interference

  6. In the pipeline test, system parameters such as AZURE_KEYVAULT_CERTIFICATE_NAME are stored in Configuration, and cannot get them by System.getenv(name), so replaced with Configuration.getGlobalConfiguration().get(name, default Value), and the defaultValue is obtained through System.getenv(name) system parameters or defined constants.
    e.g.
    Update certificateName = System.getenv("AZURE_KEYVAULT_CERTIFICATE_NAME")
    to certificateName = Configuration.getGlobalConfiguration().get("AZURE_KEYVAULT_CERTIFICATE_NAME", System.getenv("AZURE_KEYVAULT_CERTIFICATE_NAME"))

Pipeline results:
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=1227309&view=results

@benbp, @scottaddie, @joshfree, @AlexGhiondea and @vcolin7 for notification.

@ghost ghost added KeyVault azure-spring All azure-spring related issues labels Dec 1, 2021
@vcolin7 vcolin7 assigned vcolin7 and unassigned vcolin7 Dec 9, 2021
Copy link
Member

@vcolin7 vcolin7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me, but I would wait for @chenrujun to review as well just to be sure, as he knows more about the JCA module than I do.

Copy link

@chenrujun chenrujun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Azure Azure deleted a comment from azure-pipelines bot Dec 13, 2021
@check-enforcer
Copy link

This pull request is protected by Check Enforcer.

What is Check Enforcer?

Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass.

Why am I getting this message?

You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged.

What should I do now?

If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows:
/check-enforcer evaluate
Typically evaulation only takes a few seconds. If you know that your pull request is not covered by a pipeline and this is expected you can override Check Enforcer using the following command:
/check-enforcer override
Note that using the override command triggers alerts so that follow-up investigations can occur (PRs still need to be approved as normal).

What if I am onboarding a new service?

Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment:
/azp run prepare-pipelines
This will run a pipeline that analyzes the source tree and creates the pipelines necessary to build and validate your pull request. Once the pipeline has been created you can trigger the pipeline using the following comment:
/azp run java - [service] - ci

sdk/keyvault/tests.yml Outdated Show resolved Hide resolved
@v-hongli1
Copy link
Contributor Author

@vcolin7 I rerun a new weekly pipeline, but the KeyVaultErrorException occurred when the test cases of KeyVaultBackupAsyncClientTest and KeyVaultBackupClientTest were executed on OS ubuntu2004_TestFromSource_HSM_nettty_111 of Public cloud.
For more details please check here.

@vcolin7 vcolin7 merged commit 42fb546 into Azure:main Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants