alzimmermsft
commented
3 years ago @weidongxu-microsoft could you triage this to the appropriate person?
Closed medvedzver closed 3 years ago
alzimmermsft
commented
3 years ago @weidongxu-microsoft could you triage this to the appropriate person?
weidongxu-microsoft
commented
3 years ago @medvedzver Would you send me some logs (https://github.com/Azure/azure-sdk-for-java/tree/master/sdk/resourcemanager#enabling-logging)?
I am wondering why "localhost:8080" would appear in error message.
weidongxu-microsoft
commented
3 years ago OK, I see the cause. LRO headers appears using a space in path "/subscriptions/c3b5358d-d824-450c-badc-734462b9bbf1/providers/Microsoft.HDInsight/locations/South Central US/azureasyncoperations/4db5b06f-5e48-4198-bfb6-fba450d19d57-0".
weidongxu-microsoft
commented
3 years ago Log
[ForkJoinPool-1-worker-3] INFO com.azure.resourcemanager.hdinsight.implementation.ClustersClientImpl$ClustersService.delete - --> DELETE https://management.azure.com/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/resourceGroups/rg71246/providers/Microsoft.HDInsight/clusters/cluster50610?api-version=2018-06-01-preview
Try count: 1
Date:Mon, 07 Jun 2021 05:23:00 GMT
Authorization:REDACTED
Content-Length:0
Content-Type:application/json
x-ms-client-request-id:cc9838a6-e435-4212-be4a-c20fc68f4a2a
Accept:application/json
User-Agent:azsdk-java-com.azure.resourcemanager.hdinsight/1.0.0-beta.3 (15.0.1; Windows 10; 10.0; auto-generated)
(empty body)
--> END DELETE
[reactor-http-nio-3] INFO com.azure.resourcemanager.hdinsight.implementation.ClustersClientImpl$ClustersService.delete - <-- 202 https://management.azure.com/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/resourceGroups/rg71246/providers/Microsoft.HDInsight/clusters/cluster50610?api-version=2018-06-01-preview (3403 ms, 0-byte body)
Cache-Control:no-cache
Pragma:no-cache
Expires:-1
Location:https://management.azure.com/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/providers/Microsoft.HDInsight/locations/East%20US%202/operationresults/54fa582f-4a51-4198-86bb-d0f656229c27-0?api-version=2018-06-01-preview
Retry-After:60
Azure-AsyncOperation:https://management.azure.com:443/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/providers/Microsoft.HDInsight/locations/East US 2/azureasyncoperations/54fa582f-4a51-4198-86bb-d0f656229c27-0?api-version=2018-06-01-preview
x-ms-request-id:4f42d2c3-1b3e-4a8f-b84e-4dab6ec07277
x-ms-hdi-served-by:global
x-ms-correlation-request-id:75dd062c-4c1e-45e0-9553-fb9a566b8c74
Strict-Transport-Security:max-age=31536000; includeSubDomains
x-ms-ratelimit-remaining-subscription-deletes:14999
x-ms-routing-request-id:SOUTHEASTASIA:20210607T052305Z:75dd062c-4c1e-45e0-9553-fb9a566b8c74
X-Content-Type-Options:nosniff
Date:Mon, 07 Jun 2021 05:23:04 GMT
content-length:0
(body content not logged)
<-- END HTTPAzure-AsyncOperation header returns an invalid URL with space in path.
ghost
commented
3 years ago Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @aim-for-better, @idear1203, @deshriva.
| Author: | medvedzver |
|---|---|
| Assignees: | weidongxu-microsoft |
| Labels: | `HDInsight`, `Mgmt`, `Service Attention`, `customer-reported`, `question` |
| Milestone: | - |
medvedzver
commented
3 years ago @weidongxu-microsoft Thanks for quick response. I guess you don't need logs anymore. Do you know of any fix I can do on my side? Except for try/catch.
What is LRO headers?
weidongxu-microsoft
commented
3 years ago @medvedzver
It is not easy to fix on released SDK.
LRO (long running operation) is the usual ARM solution for request that takes long time. First DELETE request will have Location header or Azure-AsyncOperation header that specifying the next GET request for polling result.
One can use manager.serviceClient().getClusters().beginDelete("resourcegroup", "cluster") to send the first DELETE. However you will need to have some mechanism to make sure it gets deleted eventually (as it does not do the GET polling).
weidongxu-microsoft
commented
3 years ago It is probably SDK issue. Spec is via location https://github.com/Azure/azure-rest-api-specs/blob/master/specification/hdinsight/resource-manager/Microsoft.HDInsight/stable/2018-06-01-preview/cluster.json#L216-L218
weidongxu-microsoft
commented
3 years ago @aim-for-better, @idear1203, @deshriva
Any idea why hdinsight service returns an invalid Azure-AsyncOperation:https://management.azure.com:443/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/providers/Microsoft.HDInsight/locations/East US 2/azureasyncoperations/54fa582f-4a51-4198-86bb-d0f656229c27-0?api-version=2018-06-01-preview header during DELETE?
On ARM spec, service can choose not to return Azure-AsyncOperation header, but if service returns it, it need to be valid.
https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/Addendum.md#delete-resource-asynchronously
aim-for-better
commented
3 years ago looking
aim-for-better
commented
3 years ago @medvedzver Would you send me some logs (https://github.com/Azure/azure-sdk-for-java/tree/master/sdk/resourcemanager#enabling-logging)?
I am wondering why "localhost:8080" would appear in error message.
I am also wondering this, could you please share your finding @weidongxu-microsoft ? Thanks~
weidongxu-microsoft
commented
3 years ago @aim-for-better
The localhost:8080 could be some artifact in SDK.
The problem is the invalid URI /locations/East US 2/azureasyncoperations/. The space in URI should be escaped to be %20.
And this problem seems occur on some region, but is OK on some other region. E.g., I get a good one from East US.
[reactor-http-nio-3] INFO com.azure.resourcemanager.hdinsight.implementation.ClustersClientImpl$ClustersService.delete - <-- 202 https://management.azure.com/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/resourceGroups/rg-weidxu4/providers/Microsoft.HDInsight/clusters/cl3weidxu?api-version=2018-06-01-preview (4715 ms, 0-byte body)
Cache-Control:no-cache
Pragma:no-cache
Expires:-1
Location:https://management.azure.com/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/providers/Microsoft.HDInsight/locations/East%20US/operationresults/7fdac345-5883-4356-a7c2-c148183de477-0-r?api-version=2018-06-01-preview
Retry-After:60
x-ms-hdi-matched-rule:ClusterResourcesAndSubResources
x-ms-hdi-routed-to:RegionalRp
Azure-AsyncOperation:https://management.azure.com/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/providers/Microsoft.HDInsight/locations/East%20US/azureasyncoperations/7fdac345-5883-4356-a7c2-c148183de477-0-r?api-version=2018-06-01-preview
x-ms-request-id:bf3b9b2e-3e58-44d9-a9c9-e76166b848a3
x-ms-hdi-served-by:eastus
x-ms-correlation-request-id:b58f6d8f-95bb-4519-99e4-da236c2f8eb9
Strict-Transport-Security:max-age=31536000; includeSubDomains
x-ms-ratelimit-remaining-subscription-deletes:14999
x-ms-routing-request-id:SOUTHEASTASIA:20210615T053421Z:b58f6d8f-95bb-4519-99e4-da236c2f8eb9
X-Content-Type-Options:nosniff
Date:Tue, 15 Jun 2021 05:34:20 GMT
content-length:0
(body content not logged)
<-- END HTTPAnd SDK works good with this region.
weidongxu-microsoft
commented
3 years ago @aim-for-better Let me know if this space in URI is some bug that can be fixed in service, or you really need SDK to have some protection on the URI (not very reasonable for SDK to handle incorrect URI from service, but I can do it if really necessary).
aim-for-better
commented
3 years ago Hi @weidongxu-microsoft , Thanks for your help.
Per my finding, in south central us, I delete a cluster via PowerShell cmdlet Remove-AzHDInsightCluster (powershell cmdlet is based c# sdk)the location header is valid url.
So now I am not sure whether it is Java SDK's issue.
aim-for-better
commented
3 years ago I don't know whether it is due to my subscription is special. Could you please help create a cluster and delete it via the PowerShell cmdlet Remove-AzHDInsightCluster in south central us and catch the traffic?
The cmdlet usage is https://docs.microsoft.com/en-us/powershell/module/az.hdinsight/remove-azhdinsightcluster?view=azps-6.0.0
The reason that I hope you can help create a cluster and test it is that I doubt that my subscription is special. Thanks
weidongxu-microsoft
commented
3 years ago My log on South Central US is still not correct.
Azure-AsyncOperation:https://management.azure.com:443/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/providers/Microsoft.HDInsight/locations/South Central US/azureasyncoperations/8d9775ea-f78c-46fe-b6b0-1fd14aa782d7-0?api-version=2018-06-01-preview
[reactor-http-nio-3] INFO com.azure.resourcemanager.hdinsight.implementation.ClustersClientImpl$ClustersService.delete - <-- 202 https://management.azure.com/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/resourceGroups/rg-weidxu4/providers/Microsoft.HDInsight/clusters/cl3weidxu?api-version=2018-06-01-preview (5510 ms, 0-byte body)
Cache-Control:no-cache
Pragma:no-cache
Expires:-1
Location:https://management.azure.com/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/providers/Microsoft.HDInsight/locations/South%20Central%20US/operationresults/8d9775ea-f78c-46fe-b6b0-1fd14aa782d7-0?api-version=2018-06-01-preview
Retry-After:60
Azure-AsyncOperation:https://management.azure.com:443/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/providers/Microsoft.HDInsight/locations/South Central US/azureasyncoperations/8d9775ea-f78c-46fe-b6b0-1fd14aa782d7-0?api-version=2018-06-01-preview
x-ms-request-id:0c0e00e3-71a6-4ccd-b2f3-45711fd49079
x-ms-hdi-served-by:global
x-ms-correlation-request-id:6fc5d6b2-72bc-4510-9d34-1748007d7ef9
Strict-Transport-Security:max-age=31536000; includeSubDomains
x-ms-ratelimit-remaining-subscription-deletes:14999
x-ms-routing-request-id:SOUTHEASTASIA:20210616T024359Z:6fc5d6b2-72bc-4510-9d34-1748007d7ef9
X-Content-Type-Options:nosniff
Date:Wed, 16 Jun 2021 02:43:58 GMT
content-length:0
(body content not logged)
<-- END HTTP@alzimmermsft
Asking for suggestion from core perspective. Currently core-management uses URL to verify the link. https://github.com/Azure/azure-sdk-for-java/blob/905d234bc59dc8c01a84376fad90d18bbdd3c121/sdk/core/azure-core-management/src/main/java/com/azure/core/management/implementation/polling/Util.java#L72-L87
But netty http client seems using URI to verify the link, from the stacktrace.
Caused by: java.lang.IllegalArgumentException: Illegal character in path at index 118: http://localhost:8080/subscriptions/c3b5358d-d824-450c-badc-734462b9bbf1/providers/Microsoft.HDInsight/locations/South Central US/azureasyncoperations/4db5b06f-5e48-4198-bfb6-fba450d19d57-0
at java.base/java.net.URI.create(URI.java:883)
at reactor.netty.http.HttpOperations.resolvePath(HttpOperations.java:389)And there is some minor different between URL and URI.
Would you suggest core-management might be better to use URI to verify the link?
aim-for-better
commented
3 years ago Service returns space in azure-asyncoperation header, and java sdk can't handle this. Hi @medvedzver is it a block issue for you?
alzimmermsft
commented
3 years ago @weidongxu-microsoft, @aim-for-better
I've read through the comments in this issue and I want to make sure I'm following correctly.
" " in the path instead of the expected %20 replacement." ", is then causing an exception to be thrown when it is being used.Is the above correct? Or is it a bit more nuanced as I see there are two headers, Azure-AsyncOperation and Location, where Azure-AsyncOperation is an endpoint that can be pinged for status updates on the operation and Location is an endpoint that will return the terminal status of the operation, where one is returning the properly URL-encoded endpoint and the other is not.
Is it known whether the Azure-AsyncOperation will need to be encoded? If this is something that can be known statically and safely the mentioned code path could be amended to take an additional flag which will cause it to encode the URL before using it.
weidongxu-microsoft
commented
3 years ago @alzimmermsft
Yes, your understanding is correct.
ARM mandate the Location header in LRO DELETE type, and Azure-AsyncOperation header is optional. Both of them can be used for polling, though the response would be in different format (other HTTP method could have different requirement, that is part of the complexity on ARM LRO).
https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/Addendum.md#delete-resource-asynchronously
Hence in this particular case, if we find Azure-AsyncOperation header is not valid, we should be able to fallback to the URI in Location header.
Maybe an alternative in Java SDK is try to fix the URI encoding before sending it out. This is preferred by @aim-for-better (and reasonable to some extent as other languages seems be doing this. I am not saying doing it in azure-core-http-netty, just in azure-core-management if it is safe.).
From ARM spec, both the Azure-AsyncOperation and Location should be a valid URI. Though it does not really say it had to be encoded or not.
https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/Addendum.md#asynchronous-operations
Statically it would be either encoded or does not even contains space in the first place, since 30+ major services works fine with Java SDK, as they got live tests run with each version upgrade.
And I don't think there is any other flag stating whether the URI is encoded or not.
PS:
Other SDK seems got their HTTP client to encode the URI for them (e.g. Python url3), if not already encoded. But it seems azure-core-http-netty does not. I haven't tried on azure-core-http-okhttp.
aim-for-better
commented
3 years ago PS: c# sdk encodes the URL explicitly before sending the polling request.
aim-for-better
commented
3 years ago we are investigating in more regions. It may take some time to fix this issue.
weidongxu-microsoft
commented
3 years ago @alzimmermsft
Any thought?
I probably going to enhance azure-core-management this week.
medvedzver
commented
3 years ago @aim-for-better It's not a blocking issue, we are catching the runtime exception for now.
aim-for-better
commented
3 years ago Hi @weidongxu-microsoft I have reported such issues to PM, and just want to confirm with you that whether Java SDK will fix such issue.
weidongxu-microsoft
commented
3 years ago @alzimmermsft
Any thought?
alzimmermsft
commented
3 years ago @weidongxu-microsoft I think adding the enhancement to handle a potential need for encoding is the best route
weidongxu-microsoft
commented
3 years ago @alzimmermsft
A risk is encoding twice in the case of some path segment is encoded and some is not. E.g. test.com/a b/c%20d encode to test.com/a%20b/c%2520d, but I would guess the expected result would be test.com/a%20b/c%20d.
weidongxu-microsoft
commented
3 years ago This seems to work (I guess no one uses URI fragment in LRO)
URL url = new URL("https://management.azure.com:443/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/providers/Microsoft.HDInsight/locations/South Central US/azureasyncoperations/8d9775ea-f78c-46fe-b6b0-1fd14aa782d7-0?api-version=2018-06-01-preview");
URI uri = new URI(url.getProtocol(), url.getUserInfo(), url.getHost(), url.getPort(), url.getPath(), url.getQuery(), "");However it have the "encode twice" problem (if there is a %20 or other already escaped segment in path, it will then get encoded again as %2520).
aim-for-better
commented
3 years ago This seems to work (I guess no one uses URI fragment in LRO)
URL url = new URL("https://management.azure.com:443/subscriptions/ec0aa5f7-9e78-40c9-85cd-535c6305b380/providers/Microsoft.HDInsight/locations/South Central US/azureasyncoperations/8d9775ea-f78c-46fe-b6b0-1fd14aa782d7-0?api-version=2018-06-01-preview"); URI uri = new URI(url.getProtocol(), url.getUserInfo(), url.getHost(), url.getPort(), url.getPath(), url.getQuery(), "");However it have the "encode twice" problem (if there is a
%20or other already escaped segment in path, it will then get encoded again as%2520).
Hi @weidongxu-microsoft , just sharing an implementation of C# hoping it will be helpful for avoiding "encode twice" issue. C# only replace space with "%20" explicitly once when sending the poll request.
weidongxu-microsoft
commented
3 years ago @aim-for-better Thanks for the comment. As the processing of the URL is in the azure-core-management SDK, which is a shared lib used by all mgmt SDKs, we would try to adopt a more general solution, than to "fix" one problem from one service (as URL path allows about 79 characters, all others is not valid, space is just one of hundreds invalid characters. What happens when another service try to be "creative" and uses another invalid character?).
So far PR https://github.com/Azure/azure-sdk-for-java/pull/22750 would treat an invalid URL as if that header does not exist (and make it possible to fallback to another header).
weidongxu-microsoft
commented
3 years ago https://github.com/Azure/azure-sdk-for-java/pull/22750 merged. Next version of azure-core-management would be 1.3.2 or 1.4.0.
After azure-core-management release, I will release azure-resourcemanager-hdinsight to let it depend on latest.
weidongxu-microsoft
commented
3 years ago The issue will be closed after either new azure-resourcemanager-hdinsight released, or service fix the invalid polling URI.
weidongxu-microsoft
commented
3 years ago PR for 1.0.0-beta.4 https://github.com/Azure/azure-sdk-for-java/pull/23522
weidongxu-microsoft
commented
3 years ago 1.0.0-beta.4 released https://search.maven.org/artifact/com.azure.resourcemanager/azure-resourcemanager-hdinsight/1.0.0-beta.4/jar
If this SDK is used together with other management SDKs, make sure the final application depends on azure-core-management 1.4.0 https://github.com/Azure/azure-sdk-for-java/tree/main/sdk/resourcemanager#dependency-management
ghost
commented
3 years ago Hi @medvedzver. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text “/unresolve” to remove the “issue-addressed” label and continue the conversation.
ghost
commented
3 years ago Hi @medvedzver, since you haven’t asked that we “/unresolve” the issue, we’ll close this out. If you believe further discussion is needed, please add a comment “/unresolve” to reopen the issue.
weidongxu-microsoft
commented
3 years ago Short survey for us to hear you: https://microsoft.qualtrics.com/jfe/form/SV_ehN0lIk2FKEBkwd?Q_CHL=ISSUES
Describe the bug Exception is thrown during cluster deletion. Cluster gets deleted from azure but function call throws exception.
Exception or Stack Trace
java.lang.RuntimeException: Max retries 3 times exceeded. Error Details: Illegal character in path at index 118: http://localhost:8080/subscriptions/c3b5358d-d824-450c-badc-734462b9bbf1/providers/Microsoft.HDInsight/locations/South Central US/azureasyncoperations/4db5b06f-5e48-4198-bfb6-fba450d19d57-0
Caused by: java.lang.IllegalArgumentException: Illegal character in path at index 118: http://localhost:8080/subscriptions/c3b5358d-d824-450c-badc-734462b9bbf1/providers/Microsoft.HDInsight/locations/South Central US/azureasyncoperations/4db5b06f-5e48-4198-bfb6-fba450d19d57-0 at java.base/java.net.URI.create(URI.java:883) at reactor.netty.http.HttpOperations.resolvePath(HttpOperations.java:389) at reactor.netty.http.client.HttpClientConnect$HttpClientHandler.requestWithBody(HttpClientConnect.java:492) at reactor.netty.http.client.HttpClientConnect$HttpIOHandlerObserver.lambda$onStateChange$0(HttpClientConnect.java:397) at reactor.core.publisher.MonoDefer.subscribe(MonoDefer.java:44) ... 10 more Caused by: java.net.URISyntaxException: Illegal character in path at index 118: http://localhost:8080/subscriptions/c3b5358d-d824-450c-badc-734462b9bbf1/providers/Microsoft.HDInsight/locations/South Central US/azureasyncoperations/4db5b06f-5e48-4198-bfb6-fba450d19d57-0 at java.base/java.net.URI$Parser.fail(URI.java:2913) at java.base/java.net.URI$Parser.checkChars(URI.java:3084) at java.base/java.net.URI$Parser.parseHierarchical(URI.java:3166) at java.base/java.net.URI$Parser.parse(URI.java:3114) at java.base/java.net.URI.(URI.java:600)
at java.base/java.net.URI.create(URI.java:881)
... 14 more
To Reproduce TokenCredential tokenCredential = new ClientSecretCredentialBuilder() .tenantId(TENANT_ID) .clientId(CLIENT_ID) .clientSecret(CLIENT_SECRET) .build();
manager.clusters().deleteById(clusterId);
Expected behavior No exception should be thrown.
Setup (please complete the following information): com.azure.resourcemanager:azure-resourcemanager-hdinsight:1.0.0-beta.2 com.azure.resourcemanager:azure-resourcemanager-network:2.4.0 com.azure:azure-identity:1.3.0
Information Checklist Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report