You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Daniel Estévez <no...@github.com> on 2018/08/29 18:10:55 UTC
[jclouds/jclouds] JCLOUDS-1441: Enables support for ARM regions in
China (#1237)
- Only one blocker hardcoded was fixed in AzureComputeHttpApiModule.java
- Endpoints for GraphAPI and VaultAPI are also hardcoded but we could deal with this as separate PRs (i couldn't make this work with Guice)
- Also added two new China regions to keep the plugin up to date.
Just with this and using the right credentials and oauth endpoint for the China regions i could run all the livetests (see Jira)
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/1237
-- Commit Summary --
* JCLOUDS-1441: Enables support for ARM regions in China
-- File Changes --
M providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/config/AzureComputeHttpApiModule.java (4)
M providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/config/GraphRBAC.java (3)
M providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Region.java (2)
M providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/features/VaultApi.java (122)
M providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/config/ParseTenantIdTest.java (7)
M providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/internal/BaseAzureComputeApiLiveTest.java (3)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/1237.patch
https://github.com/jclouds/jclouds/pull/1237.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1237
Re: [jclouds/jclouds] JCLOUDS-1441: Enables support for ARM regions
in China (#1237)
Posted by Daniel Estévez <no...@github.com>.
Closed #1237.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1237#event-1830625159
Re: [jclouds/jclouds] JCLOUDS-1441: Enables support for ARM regions
in China (#1237)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> @@ -92,7 +97,14 @@ protected final String provideTenant(@Named("oauth.endpoint") final String oauth
if (!m.matches()) {
throw new IllegalArgumentException("Could not parse tenantId from: " + oauthEndpoint);
}
- return m.group(1);
+ return m.group(2);
+ }
+
+ @Provides
+ @Singleton
+ @Named(IS_CHINA_ENDPOINT)
+ protected final boolean isChinaEndpoint(@Named("oauth.endpoint") final String oauthEndpoint) {
+ return CHINA_OAUTH_ENDPOINT_PATTERN.matcher(oauthEndpoint).matches();
Add proper unit tests for this method
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1237#pullrequestreview-152162782
Re: [jclouds/jclouds] JCLOUDS-1441: Enables support for ARM regions
in China (#1237)
Posted by Daniel Estévez <no...@github.com>.
Yes, a simple `isChina` is easily implementable as we can parse `ContextBuilder.endpoint`
Thanks for the tip, i'll try that approach and update the PR since i think we can suppose this will be the only exception and Microsoft won't be adding other additional endpoints.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1237#issuecomment-418149912
Re: [jclouds/jclouds] JCLOUDS-1441: Enables support for ARM regions
in China (#1237)
Posted by Ignasi Barrera <no...@github.com>.
nacx approved this pull request.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1237#pullrequestreview-152388787
Re: [jclouds/jclouds] JCLOUDS-1441: Enables support for ARM regions
in China (#1237)
Posted by Daniel Estévez <no...@github.com>.
@danielestevez pushed 1 commit.
f1087b6 Adapted Endpoints for APIs GraphRBAC and Vault
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1237/files/eef75b8bf50c56a365cb81be9a61694a29ffbedf..f1087b632942508f7b8281f1ae37e51bb143f557
Re: [jclouds/jclouds] JCLOUDS-1441: Enables support for ARM regions
in China (#1237)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> @@ -54,7 +54,12 @@
public class AzureComputeHttpApiModule extends HttpApiModule<AzureComputeApi> {
private static final Pattern OAUTH_TENANT_PATTERN = Pattern
- .compile("https://login.microsoft(?:online)?.com/([^/]+)/oauth2/token");
+ .compile("https://login.(microsoft(?:online)?.com|chinacloudapi.cn)/([^/]+)/oauth2/token");
+
+ private static final Pattern CHINA_OAUTH_ENDPOINT_PATTERN = Pattern
+ .compile("https://login.(chinacloudapi.cn)/([^/]+)/oauth2/token");
nit. You don't need the first grouping
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1237#pullrequestreview-152388750
Re: [jclouds/jclouds] JCLOUDS-1441: Enables support for ARM regions
in China (#1237)
Posted by Ignasi Barrera <no...@github.com>.
Thanks! A couple hints to do this properly:
Is there any way we can determine if the `ContextBuilder` was created for China? (For example by looking at the `endpoint` that has been configured?
If we can determine that, then I'd suggest the following approach to make the whole thing configurable:
1. Create a `@Provides` method in the Http API Module, that gets the endpoint (or whatever that allows us to know if it is China), and returns a boolean saying if we're in China or not. We can also qualify that method with a name. The idea is to be able to inject that `isChina` boolean wherever we need it.
2. Modify the [GraphRBAC](https://github.com/jclouds/jclouds/blob/master/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/config/GraphRBAC.java) endpoint supplier to get the `isChina` boolean injected and produce the right URI in its `get()` method.
3. Modify the [OAuthResource](https://github.com/jclouds/jclouds/blob/master/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/config/OAuthResource.java) to accept a second value (`String china()` or similar) so we can use the annotation to configure the normal endpoint and the endpoint for China regions.
4. Modify the [AzureOAuthConfigFactory](https://github.com/jclouds/jclouds/blob/master/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/config/AzureOAuthConfigFactory.java) to get the `isChina` boolean injected and take it into account to read the right field of the `OAuthResource` annotation and return the right endpoint.
With this we should be good to get the right endpoints automatically configured.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1237#issuecomment-417249540
Re: [jclouds/jclouds] JCLOUDS-1441: Enables support for ARM regions
in China (#1237)
Posted by Daniel Estévez <no...@github.com>.
Squashed and merged to [2.1.x](http://git-wip-us.apache.org/repos/asf/jclouds/commit/951b6b44) and [master](http://git-wip-us.apache.org/repos/asf/jclouds/commit/a07ab5a9)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1237#issuecomment-419123254
Re: [jclouds/jclouds] JCLOUDS-1441: Enables support for ARM regions
in China (#1237)
Posted by Daniel Estévez <no...@github.com>.
@danielestevez pushed 1 commit.
4511cf1 Adds unit test for china oauth endpoint check
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1237/files/f1087b632942508f7b8281f1ae37e51bb143f557..4511cf13e810afa866290c8920f44f67b7302f67