Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

Closed #250. --- 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-labs/pull/250#event-617393340

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

Merged to master as [a4334d46](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/a4334d46). Fantastic job @ritazh! I've been able to follow the steps in the README and all live tests passed. Some notes for the upcoming pull requests: * The TenantId is required, but in the README

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

Thanks for all the feedback @nacx! Made all the suggested updates. Ready for another round of review. --- 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-labs/pull/250#issuecomment-206534264

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + JobStatus status = api.getJobApi().jobStatus(URI.create(requestUrl)); > + > + assertEquals(status, JobStatus.DONE); > + > + assertSent(server, "GET", requestUrl); > + } > + > + public void testGetJobStatusInProgress() throws InterruptedException { > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + > + @AfterClass(alwaysRun = true) > + public void testDelete() throws Exception { > + URI uri = api().delete(resourcegroup); > + assertNotNull(uri); > + assertTrue(uri.toString().contains("api-version")); > + assertTrue(uri.toString().contains("operationresults")); > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> @@ -0,0 +1,15 @@ > +-BEGIN RSA PRIVATE KEY- > +MIICXgIBAAKBgQCwqwzakEPP+U9vx9JCuMHebFIVQZ4Sjaj2RU9dJ6YT2s3u7dC6 > +/0fGM5xm4fXmSHqyGC6PC8weQSkxnSpbU+R4jMWPM8ML4TIr5wP0avbg+wy3+WWI > +of0MN7YHkCfqpaaiKiMw7niK1y07YvxJN8LX1xLpE7aXgIpn6L/qtJdHnQIDAQAB >

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + > bind(HttpErrorHandler.class).annotatedWith(ServerError.class).to(AzureComputeErrorHandler.class); > + } > + > + @Override > + protected void installLocations() { > + super.installLocations(); > + bind(ImplicitLocationSupplier.class). > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + > bind(HttpErrorHandler.class).annotatedWith(ServerError.class).to(AzureComputeErrorHandler.class); > + } > + > + @Override > + protected void installLocations() { > + super.installLocations(); > + bind(ImplicitLocationSupplier.class). > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

Thanks for the hard work @ritazh! It's almost there. Apologies if I didn't myself clear in previous comments! --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> @@ -0,0 +1,15 @@ > +-BEGIN RSA PRIVATE KEY- > +MIICXgIBAAKBgQCwqwzakEPP+U9vx9JCuMHebFIVQZ4Sjaj2RU9dJ6YT2s3u7dC6 > +/0fGM5xm4fXmSHqyGC6PC8weQSkxnSpbU+R4jMWPM8ML4TIr5wP0avbg+wy3+WWI > +of0MN7YHkCfqpaaiKiMw7niK1y07YvxJN8LX1xLpE7aXgIpn6L/qtJdHnQIDAQAB >

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + > + @AfterClass(alwaysRun = true) > + public void testDelete() throws Exception { > + URI uri = api().delete(resourcegroup); > + assertNotNull(uri); > + assertTrue(uri.toString().contains("api-version")); > + assertTrue(uri.toString().contains("operationresults")); > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + } > + > + @Test(dependsOnMethods = "testCreate") > + public void testUpdateWithTag() { > + ImmutableMap tags = ImmutableMap. String>builder().put("test1", "value1").build(); > + > + final ResourceGroup resourceGroup = api().update("jcloudstest",

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + JobStatus status = api.getJobApi().jobStatus(URI.create(requestUrl)); > + > + assertEquals(status, JobStatus.DONE); > + > + assertSent(server, "GET", requestUrl); > + } > + > + public void testGetJobStatusInProgress() throws InterruptedException { > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + @MapBinder(BindToJsonPayload.class) > + ResourceGroup create(@PathParam("name") String name, > @PayloadParam("location") String location, @Nullable > @PayloadParam("tags")Map tags); > + > + @Named("resourcegroup:get") > + @GET > + @Path("/{name}") > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + > bind(HttpErrorHandler.class).annotatedWith(ServerError.class).to(AzureComputeErrorHandler.class); > + } > + > + @Override > + protected void installLocations() { > + super.installLocations(); > + bind(ImplicitLocationSupplier.class). > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

@nacx I have made all the updates from previous comments. --- 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-labs/pull/250#issuecomment-206076915

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + public AzureComputeProviderMetadata(final Builder builder) { > + super(builder); > + } > + > + public static class Builder extends BaseProviderMetadata.Builder { > + > + protected Builder() { > + super(); > + > + id("azurecompute-arm") > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> +/** > + * Parses job status from http response > + */ > +@Singleton > +public class URIParser implements Function { > + public URI apply(final HttpResponse from) { > + final String statusCode = Integer.toString(from.getStatusCode()); > + if (statusCode != null &&

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + } > + > + public static Properties defaultProperties() { > + final Properties properties = BaseHttpApiMetadata.defaultProperties(); > + return properties; > + } > + > + public static class Builder extends > BaseHttpApiMetadata.Builder { > + > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

Thanks @ritazh! I've added the final comments, most of them pretty straightforward. We have a pretty good foundation here! --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + public AzureComputeProviderMetadata(final Builder builder) { > + super(builder); > + } > + > + public static class Builder extends BaseProviderMetadata.Builder { > + > + protected Builder() { > + super(); > + > + id("azurecompute-arm") > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + } > + > + public static Properties defaultProperties() { > + final Properties properties = BaseHttpApiMetadata.defaultProperties(); > + return properties; > + } > + > + public static class Builder extends > BaseHttpApiMetadata.Builder { > + > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + assertEquals(status, JobStatus.DONE); > + > + assertTrue(uri.toString().contains("api-version")); > + assertTrue(uri.toString().contains("operationresults")); > + } > + > + public void testDeleteResourceGroupReturns404() throws > InterruptedException { > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + */ > +package org.jclouds.azurecompute.arm.internal; > + > +import java.util.Properties; > +import static org.jclouds.oauth.v2.config.OAuthProperties.AUDIENCE; > +import static org.jclouds.oauth.v2.config.OAuthProperties.CREDENTIAL_TYPE; > +import static >

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + public void testDelete() throws Exception { > + URI uri = api().delete(resourcegroup); > + assertNotNull(uri); > + assertTrue(uri.toString().contains("api-version")); > + assertTrue(uri.toString().contains("operationresults")); > + > + String[] params =

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + public AzureComputeProviderMetadata(final Builder builder) { > + super(builder); > + } > + > + public static class Builder extends BaseProviderMetadata.Builder { > + > + protected Builder() { > + super(); > + > + id("azurecompute-arm") > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + } > + > + @Test(dependsOnMethods = "testCreate") > + public void testUpdateWithTag() { > + ImmutableMap tags = ImmutableMap. String>builder().put("test1", "value1").build(); > + > + final ResourceGroup resourceGroup = api().update("jcloudstest",

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + * limitations under the License. > + */ > +package org.jclouds.azurecompute.arm.functions; > +import com.google.common.base.Function; > +import org.jclouds.http.HttpResponse; > + > +import javax.inject.Singleton; > + > +import java.net.URI; > +/** > + * Parses job status from http response > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + > +import static org.jclouds.http.HttpUtils.releasePayload; > +/** > + * Parses an http response code from http responser > + */ > +@Singleton > +public class StatusCodeParser implements Function { > + public String apply(final HttpResponse from) { > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + @Produces(MediaType.APPLICATION_JSON) > + @MapBinder(BindToJsonPayload.class) > + ResourceGroup create(@PathParam("name") String name, > @PayloadParam("location") String location, @Nullable > @PayloadParam("tags")Map tags); > + > + @Named("resourcegroup:get") > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + @Nullable > + ResourceGroup get(@PathParam("name") String name); > + > + @Named("resourcegroup:update") > + @PATCH > + @Produces(MediaType.APPLICATION_JSON) > + @Path("/{name}") > + @Nullable > + @MapBinder(BindToJsonPayload.class) > + ResourceGroup update(@PathParam("name")

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> +import org.jclouds.rest.annotations.RequestFilters; > +import org.jclouds.rest.annotations.ResponseParser; > +import org.jclouds.azurecompute.arm.functions.ParseJobStatus; > +import org.jclouds.azurecompute.arm.functions.ParseJobStatus.JobStatus; > + > +/** > + * The Azure Resource Manager API

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + > bind(HttpErrorHandler.class).annotatedWith(ServerError.class).to(AzureComputeErrorHandler.class); > + } > + > + @Override > + protected void installLocations() { > + super.installLocations(); > + bind(ImplicitLocationSupplier.class). > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + } > + > + public static Properties defaultProperties() { > + final Properties properties = BaseHttpApiMetadata.defaultProperties(); > + return properties; > + } > + > + public static class Builder extends > BaseHttpApiMetadata.Builder { > + > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + public AzureComputeProviderMetadata(final Builder builder) { > + super(builder); > + } > + > + public static class Builder extends BaseProviderMetadata.Builder { > + > + protected Builder() { > + super(); > + > + id("azurecompute-arm") > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + } > + > + public AzureComputeProviderMetadata() { > + super(builder()); > + } > + > + public static Properties defaultProperties() { > + final Properties properties = > AzureManagementApiMetadata.defaultProperties(); > + properties.setProperty(OPERATION_TIMEOUT,

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> +jclouds-labs > +2.0.0-SNAPSHOT > + > + azurecompute-arm > + jclouds Azure Compute ARM API > + jclouds components to access an implementation of Azure's ARM > Compute Service > + bundle > + > + > + > FIXME_subscription-id > + > FIXME_tenant-id > + >

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + @Fallback(NullOnNotFoundOr404.class) > + @Nullable > + ResourceGroup get(@PathParam("name") String name); > + > + @Named("resourcegroup:update") > + @PATCH > + @Produces(MediaType.APPLICATION_JSON) > + @Path("resourcegroups/{name}") > + @Fallback(NullOnNotFoundOr404.class) > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + @Fallback(NullOnNotFoundOr404.class) > + @Nullable > + ResourceGroup get(@PathParam("name") String name); > + > + @Named("resourcegroup:update") > + @PATCH > + @Produces(MediaType.APPLICATION_JSON) > + @Path("resourcegroups/{name}") > + @Fallback(NullOnNotFoundOr404.class) > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

@nacx @andreaturli The latest commit includes all the fixes from your comments. Here are some major updates: - using the latest org.jclouds.oauth.v2 with support for Azure AD authentication - id of this provider is `azurecompute-arm` - package name is `org.jclouds.azurecompute.arm` - update

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + * > + * @see href="https://msdn.microsoft.com/en-us/library/azure/dn790546.aspx;>docs > + */ > +@Path("/subscriptions/{subscriptionid}") > + > +@QueryParams(keys = "api-version", values = "2015-01-01") > +@RequestFilters(OAuthFilter.class) > +@Consumes(MediaType.APPLICATION_JSON) > +public

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + > + HashMap tags = new HashMap(); > + tags.put("tagname1", "tagvalue1"); > + > + ResourceGroup resourceGroup = > api.getResourceGroupApi(subscriptionid).create("jcloudstest", "West US", > tags); > + > + assertEquals(resourceGroup.name(),

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + @Fallback(NullOnNotFoundOr404.class) > + @Nullable > + ResourceGroup get(@PathParam("name") String name); > + > + @Named("resourcegroup:update") > + @PATCH > + @Produces(MediaType.APPLICATION_JSON) > + @Path("resourcegroups/{name}") > + @Fallback(NullOnNotFoundOr404.class) > +

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> +import org.jclouds.rest.annotations.MapBinder; > +import org.jclouds.rest.annotations.PayloadParam; > +import org.jclouds.rest.annotations.PATCH; > +import org.jclouds.rest.annotations.ResponseParser; > + > + > +import org.jclouds.rest.binders.BindToJsonPayload; > + > +/** > + * The Azure

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

@jmspring Thanks for opening the OAuth PR! I've commented there. --- 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-labs/pull/250#issuecomment-203153096

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> +import org.jclouds.rest.annotations.MapBinder; > +import org.jclouds.rest.annotations.PayloadParam; > +import org.jclouds.rest.annotations.PATCH; > +import org.jclouds.rest.annotations.ResponseParser; > + > + > +import org.jclouds.rest.binders.BindToJsonPayload; > + > +/** > + * The Azure

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

@nacx One other question around OAuth properties, specifically in regard to the live tests. Azure AD Oauth, as mentioned previously, is currently ID/Secret based. I will add support for ID/Certificate, but it also differs from the JWT implementation currently in the code base. This raises a

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + * Cache > + * This maintains a time-based Bearer Token cache. By default expires after > 59 minutes > + * (the maximum time a token is valid is 60 minutes). > + * This cache and expiry period is system-wide and does not attend to > per-instance expiry time > + * (e.g. "expires_in" from

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

Sounds good to me. But feel free to remove the duplicate bits if it is easier for you, even if it breaks this build temporary. As long as we can build both branches to test locally, I'm OK. Let's make things as easy as possible to make this move forward! :) --- You are receiving this because

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

> + * Cache > + * This maintains a time-based Bearer Token cache. By default expires after > 59 minutes > + * (the maximum time a token is valid is 60 minutes). > + * This cache and expiry period is system-wide and does not attend to > per-instance expiry time > + * (e.g. "expires_in" from

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm ResourceGroupApi (#250)

@nacx - Regarding Oauth, I am working on a separate PR for it into JClouds proper. The one question I do have, however, is the chicken and egg issue of doing a PR for OAuth into JClouds then this initial PR, since it will rely on those OAuth changes. Should it be multistage - 1. Get the