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

[Text Analytics]It will return error when use textAnalyticsClient with AAD authentication to run integration tests on UsGov cloud and China cloud #17563

Closed
zzhxiaofeng opened this issue Sep 10, 2021 · 17 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. Cognitive - Text Analytics test-sovereign-cloud

Comments

@zzhxiaofeng
Copy link
Contributor

zzhxiaofeng commented Sep 10, 2021

We are running live Tests against other clouds like US Gov and Azure China Cloud. The goal is to check whether new azure sdk package work with other clouds or not.

Error Description:
When we use textAnalyticsClient with azure active directory credential to run integration tests on UsGov cloud and China cloud, it will return an error. The error message is shown as following, for more details please check here:
image

Error Track:
The failed reason is that when we use azure active directory credential, the scope of credential is just supported on Public cloud. The scope of credential defines the set of permissions being requested by the application. In text analytics client, the value of scope is here.
image

Expected Behavior:
Add different scopes for different clouds in Text Analytics SDK.
The value of the scope in different clouds is follow:
Public cloud: https://cognitiveservices.azure.com/.default
UsGov cloud: https://cognitiveservices.azure.us/.default
China cloud: https://cognitiveservices.azure.cn/.default

@benbp, @deyaaeldeen, @ramya-rao-a, @meeraharidasa for notification.

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Sep 10, 2021
@ramya-rao-a ramya-rao-a added Client This issue points to a problem in the data-plane of the library. Cognitive - Text Analytics labels Sep 10, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Sep 10, 2021
@ramya-rao-a ramya-rao-a added this to the [2021] October milestone Sep 10, 2021
@v-xuto v-xuto added needs-team-triage Workflow: This issue needs the team to triage. test-sovereign-cloud labels Sep 10, 2021
@ramya-rao-a
Copy link
Contributor

@deyaaeldeen I would imagine other SDKs for cognitive services would have this issue as well, like Form Recognizer and Metrics Advisor

It is worth checking with the feature crews to see if our SDKs in other languages would have the same problem as well

@deyaaeldeen
Copy link
Member

/cc @witemple-msft @joheredi @KarishmaGhiya. I am digging deeper into this and will share my findings.

@ramya-rao-a
Copy link
Contributor

cc @witemple-msft and @KarishmaGhiya if they can share any history around this from the perspective of Form Recognizer and Metrics Advisor

@benbp
Copy link
Member

benbp commented Sep 10, 2021

@ramya-rao-a @deyaaeldeen FYI the cognitiveServicesEndpointSuffix arm template parameter will get auto-populated based on cloud in the live test pipeline.

You can see example usage from the NET tests:
https://github.com/Azure/azure-sdk-for-net/blob/4162f6fa2445b2127468b9cfd080f01c9da88eba/sdk/formrecognizer/test-resources.json#L23-L26

@ramya-rao-a
Copy link
Contributor

@benbp The ARM template will only help with the deployment. The problem here is that we are passing the wrong scope when trying to authenticate the client

@benbp
Copy link
Member

benbp commented Sep 10, 2021

@ramya-rao-a I see. If you set that scope dynamically as an ARM template output, then you can load it in from the environment when you construct it, something like:

var endpointSuffix = process.env.COGNITIVE_SERVICES_ENDPOINT_SUFFIX || ".cognitiveservices.azure.com";
const DEFAULT_COGNITIVE_SCOPE = "https://" + endpointSuffix + "/.default";

This is easier than having to do a switch on authority host, but also probably doesn't make it easier for customers, so perhaps it is better to do the switch.

EDIT: looks like something similar is done here: https://github.com/Azure/azure-sdk-for-js/pull/17443/files#diff-7a62be417c0ab079a338a0153787d9522a6246ec46b5e05bde866d0366fb7a2aR56

@ramya-rao-a
Copy link
Contributor

@sarangan12 fyi, you will likely face the same problem in @azure/search-documents due to the hard-coded scope

@deyaaeldeen
Copy link
Member

deyaaeldeen commented Sep 16, 2021

Quick update:

The scopes for TA are:

AzCloud: cognitiveservices.azure.com
UsGov: cognitiveservices.azure.us
China: cognitiveservices.azure.cn
Germany: TBD but most likely cognitiveservices.azure.de deprecated

I looked at using the authority host to infer the scope but found that you can actually use the authority host for Azure Cloud to authenticate into a sovereign cloud resource. I found that container registry exposes an audience option that can be used to determine the scope:

/**
* Gets or sets the audience to use for authentication with Azure Active Directory.
* The authentication scope will be set from this audience.
* See {@link KnownContainerRegistryAudience} for known audience values.
*/
audience?: string;

@benbp
Copy link
Member

benbp commented Sep 17, 2021

@deyaaeldeen Germany is a deprecated cloud that we don't support for testing.

@deyaaeldeen
Copy link
Member

@schaabs confirmed that it is not likely but it is valid for the authority host to be in one cloud while the resource being authenticated into is in another.

@deyaaeldeen deyaaeldeen modified the milestones: [2021] October, Backlog Oct 1, 2021
@zzhxiaofeng
Copy link
Contributor Author

zzhxiaofeng commented Dec 2, 2021

@benbp When we run text analytics integration tests in pipeline, some environment variables used in tests are not imported. For more details please check here. Please help to add these variables when you are free? These variables are shown as following:
https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/textanalytics/ai-text-analytics/tests.yml#L8
d5be52e6-25dd-4aa5-98de-23191bfcba51

@ramya-rao-a ramya-rao-a modified the milestones: Backlog, [2022] January Dec 3, 2021
@KarishmaGhiya
Copy link
Member

KarishmaGhiya commented Jan 28, 2022

@deyaaeldeen @ramya-rao-a We did not run the Metrics Advisor tests on Sovereign or US Gov, I don't think the service team expected any customers using their service on these clouds.

@ramya-rao-a ramya-rao-a removed the needs-team-triage Workflow: This issue needs the team to triage. label Feb 4, 2022
@v-xuto
Copy link
Member

v-xuto commented Feb 10, 2022

@deyaaeldeen @ramya-rao-a Is there any progress on this issue? When is it planned to be fixed?

@deyaaeldeen
Copy link
Member

.NET TA client library added an audience option to support other clouds in Azure/azure-sdk-for-net#26583. I am planning on doing the same and will try to finish it in this milestone.

@v-xuto
Copy link
Member

v-xuto commented Mar 8, 2023

@deyaaeldeen Is there any progress on this issue?

@deyaaeldeen
Copy link
Member

@v-xuto There is still in discussion, we need to design a way to support of sovereign clouds in the library.

Copy link

Hi @zzhxiaofeng, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 15, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 15, 2024
@xirzec xirzec removed this from the Backlog milestone May 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. Cognitive - Text Analytics test-sovereign-cloud
Projects
Development

No branches or pull requests

7 participants