You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Jim Spring <no...@github.com> on 2017/10/30 21:27:11 UTC
[jclouds/jclouds-labs] Add Azure KeyVault support -- vault management
and certificate/key/secret operations (#416)
This PR builds off the initial work done by @andreaturli. It implements the following functionality:
- KeyVault management
- Certificate operations
- Key operations
- Secret operations
Currently, in "Key operations", two functions are there, but not tested:
- [Merge Certificate](https://docs.microsoft.com/en-us/rest/api/keyvault/MergeCertificate/MergeCertificate)
- [Update Certificate with Version](https://docs.microsoft.com/en-us/rest/api/keyvault/UpdateCertificate/UpdateCertificate)
Merging certificates is meant for the case where the KeyVault is used to generate a Certificate Signing Request and you have the cert signed by an external agency. Thus then merging the certificate / chain back into the KeyVault where there is a pending certificate operation for that CSR/key.
Updating a specific version of a certificate responds with an error that the Certificate Policy can't be updated on a specific version of a certificate. However, specifying no Certificate Policy results in an error (required) as does specifying the exact same policy (can't update policy on a specific version).
I've got inquiries out on each of these.
One other place to put some eyes on is the KeyVaultFilter.java file. Given operations outside of KeyVault management operate on the KeyVault url directly, the KeyVaultFilter has been modified to work with the case where we are doing mock tests instead of direct to OAuth.
As mentioned on the email, there will be two separate PRs for the additional two components of the KeyVault:
1. Crytographic operations -- decrypt, encrypt, wrap key, unwrap key, sign, and verify.
2. Storage account operations -- these rely on having a storage account to operate on.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/416
-- Commit Summary --
* Add initial KeyVault support
* Add initial KeyVault support, based off of @andreaturi's work. Vault management, Certificates, Keys and Secrets supported.
-- File Changes --
M azurecompute-arm/README.md (27)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/AzureComputeApi.java (14)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/AzureComputeProviderMetadata.java (42)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/AzureComputeServiceAdapter.java (2)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/functions/VirtualMachineToStatus.java (2)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/strategy/CleanupResources.java (6)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/AvailabilitySet.java (4)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Certificate.java (595)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Key.java (152)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/OSProfile.java (2)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/SKU.java (27)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Secret.java (144)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/StorageProfile.java (2)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Vault.java (105)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/VaultProperties.java (131)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/features/DeploymentApi.java (4)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/features/VaultApi.java (636)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/filters/KeyVaultFilter.java (108)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/functions/TrueOn20X.java (37)
M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/PublicIPAddressApiLiveTest.java (4)
M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/PublicIPAddressApiMockTest.java (2)
A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/VaultApiLiveTest.java (1130)
A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/VaultApiMockTest.java (2331)
M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/VirtualMachineApiLiveTest.java (2)
M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/VirtualMachineApiMockTest.java (4)
M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/filters/ApiVersionFilterTest.java (4)
M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/internal/BaseAzureComputeApiLiveTest.java (4)
M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/internal/BaseAzureComputeApiMockTest.java (6)
A azurecompute-arm/src/test/resources/getvault.json (60)
A azurecompute-arm/src/test/resources/vaultbackupkey.json (3)
A azurecompute-arm/src/test/resources/vaultbackupsecret.json (3)
A azurecompute-arm/src/test/resources/vaultcreate.json (26)
A azurecompute-arm/src/test/resources/vaultcreatecertificate.json (11)
A azurecompute-arm/src/test/resources/vaultcreatekey.json (15)
A azurecompute-arm/src/test/resources/vaultdeletecertificate.json (58)
A azurecompute-arm/src/test/resources/vaultdeletecertificatecontacts.json (8)
A azurecompute-arm/src/test/resources/vaultdeletecertificateissuer.json (21)
A azurecompute-arm/src/test/resources/vaultdeletecertificateoperation.json (11)
A azurecompute-arm/src/test/resources/vaultdeletekey.json (15)
A azurecompute-arm/src/test/resources/vaultdeletesecret.json (10)
A azurecompute-arm/src/test/resources/vaultget.json (26)
A azurecompute-arm/src/test/resources/vaultgetcertificate.json (55)
A azurecompute-arm/src/test/resources/vaultgetcertificatecontacts.json (8)
A azurecompute-arm/src/test/resources/vaultgetcertificateissuer.json (21)
A azurecompute-arm/src/test/resources/vaultgetcertificateoperation.json (11)
A azurecompute-arm/src/test/resources/vaultgetcertificatepolicy.json (37)
A azurecompute-arm/src/test/resources/vaultgetdeleted.json (12)
A azurecompute-arm/src/test/resources/vaultgetdeletedcertificate.json (55)
A azurecompute-arm/src/test/resources/vaultgetdeletedkey.json (18)
A azurecompute-arm/src/test/resources/vaultgetdeletedsecret.json (13)
A azurecompute-arm/src/test/resources/vaultgetkey.json (15)
A azurecompute-arm/src/test/resources/vaultgetkeyversions.json (23)
A azurecompute-arm/src/test/resources/vaultgetsecret.json (11)
A azurecompute-arm/src/test/resources/vaultgetsecretversions.json (25)
A azurecompute-arm/src/test/resources/vaultimportcertificate.json (52)
A azurecompute-arm/src/test/resources/vaultlist.json (29)
A azurecompute-arm/src/test/resources/vaultlistcertificateissuers.json (7)
A azurecompute-arm/src/test/resources/vaultlistcertificates.json (27)
A azurecompute-arm/src/test/resources/vaultlistcertificateversions.json (17)
A azurecompute-arm/src/test/resources/vaultlistdeleted.json (15)
A azurecompute-arm/src/test/resources/vaultlistdeletedcertificates.json (18)
A azurecompute-arm/src/test/resources/vaultlistdeletedkeys.json (15)
A azurecompute-arm/src/test/resources/vaultlistdeletedsecrets.json (16)
A azurecompute-arm/src/test/resources/vaultlistkeys.json (42)
A azurecompute-arm/src/test/resources/vaultlistsecrets.json (40)
A azurecompute-arm/src/test/resources/vaultrecoverdeletedcertificate.json (52)
A azurecompute-arm/src/test/resources/vaultrecoverdeletedkey.json (15)
A azurecompute-arm/src/test/resources/vaultrecoverdeletedsecret.json (10)
A azurecompute-arm/src/test/resources/vaultrestorekey.json (15)
A azurecompute-arm/src/test/resources/vaultrestoresecret.json (10)
A azurecompute-arm/src/test/resources/vaultsetcertificatecontacts.json (8)
A azurecompute-arm/src/test/resources/vaultsetcertificateissuer.json (21)
A azurecompute-arm/src/test/resources/vaultsetsecret.json (11)
A azurecompute-arm/src/test/resources/vaultupdatecertificate.json (58)
A azurecompute-arm/src/test/resources/vaultupdatecertificateissuer.json (21)
A azurecompute-arm/src/test/resources/vaultupdatecertificateoperation.json (11)
A azurecompute-arm/src/test/resources/vaultupdatecertificatepolicy.json (37)
A azurecompute-arm/src/test/resources/vaultupdatekey.json (18)
A azurecompute-arm/src/test/resources/vaultupdatekeywithversion.json (18)
A azurecompute-arm/src/test/resources/vaultupdatesecret.json (13)
A azurecompute-arm/src/test/resources/vaultupdatesecretwithversion.json (13)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/416.patch
https://github.com/jclouds/jclouds-labs/pull/416.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-labs/pull/416
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
342965b updates based off comments from @andreaturli
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/ba42d3eb3a505308db72146524ea4fd49cbc8ce0..342965b754089cc6f40749c363568b9c6cd4e260
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> + @Fallback(NullOnNotFoundOr404.class)
+ @OAuthResource("https://vault.azure.net")
+ DeletedKeyBundle deleteKey(@EndpointParam URI vaultBaseUrl, @PathParam("keyName") String keyName);
+
+ @Named("key:get_versions")
+ @GET
+ @SelectJson("value")
+ @Path("/keys/{keyName}/versions")
+ @Fallback(EmptyListOnNotFoundOr404.class)
+ @OAuthResource("https://vault.azure.net")
+ List<Key> getKeyVersions(@EndpointParam URI vaultBaseUrl, @PathParam("keyName") String keyName);
+
+ @Named("key:update")
+ @PATCH
+ @MapBinder(BindToJsonPayload.class)
+ @Path("/keys/{keyName}{keyVersion}")
I've seen the same thing in the certificates methods, and then the tests building the version parameter as `"/" + version`. Is that intentional?
--
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/416#discussion_r156001516
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Andrea Turli <no...@github.com>.
andreaturli requested changes on this pull request.
> @@ -212,7 +213,7 @@ NetworkSecurityRuleApi getNetworkSecurityRuleApi(@PathParam("resourcegroup") Str
ImageApi getVirtualMachineImageApi(@PathParam("resourcegroup") String resourcegroup);
/**
- * The metrics API includes operations to get insights into entities within your
+ * The metrics API includes operations to getVault insights into entities within your
I think `get` was more approriate here
> @@ -221,11 +222,20 @@ NetworkSecurityRuleApi getNetworkSecurityRuleApi(@PathParam("resourcegroup") Str
MetricsApi getMetricsApi(@PathParam("resourceid") String resourceid);
/**
- * The metric definitions API includes operations to get insights available for entities within your
+ * The metric definitions API includes operations to getVault insights available for entities within your
I think `get` was more approriate here
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.API_VERSION_PREFIX;
+import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.DEFAULT_SUBNET_ADDRESS_PREFIX;
+import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.DEFAULT_VNET_ADDRESS_SPACE_PREFIX;
+import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.IMAGE_PUBLISHERS;
+import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.OPERATION_TIMEOUT;
+import static org.jclouds.compute.config.ComputeServiceProperties.IMAGE_AUTHENTICATE_SUDO;
+import static org.jclouds.compute.config.ComputeServiceProperties.IMAGE_LOGIN_USER;
+import static org.jclouds.compute.config.ComputeServiceProperties.POLL_INITIAL_PERIOD;
+import static org.jclouds.compute.config.ComputeServiceProperties.POLL_MAX_PERIOD;
+import static org.jclouds.compute.config.ComputeServiceProperties.RESOURCENAME_DELIMITER;
+import static org.jclouds.compute.config.ComputeServiceProperties.RESOURCENAME_PREFIX;
+import static org.jclouds.compute.config.ComputeServiceProperties.TEMPLATE;
+import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_TERMINATED;
+import static org.jclouds.oauth.v2.config.CredentialType.CLIENT_CREDENTIALS_SECRET;
+import static org.jclouds.oauth.v2.config.OAuthProperties.CREDENTIAL_TYPE;
+import static org.jclouds.oauth.v2.config.OAuthProperties.RESOURCE;
please avoid to re-order import statements when possible
> @@ -56,7 +56,7 @@ public static StatusAndBackendStatus create(NodeMetadata.Status status, String b
// goes through stages: Accepted -> Running -> Succeeded.
// Only when the deployment has SUCCEEDED is the resource deployed using the
// template actually ready.
- // To get details about the resource(s) deployed via template, one needs to
+ // To getVault details about the resource(s) deployed via template, one needs to
I think `get` was more approriate here
> @@ -91,7 +91,7 @@ public boolean cleanupNode(final String id) {
logger.debug(">> destroying %s ...", id);
boolean vmDeleted = deleteVirtualMachine(resourceGroupName, virtualMachine);
- // We don't delete the network here, as it is global to the resource
+ // We don't deleteVault the network here, as it is global to the resource
I think `delete` was more approriate here
> @@ -222,7 +222,7 @@ public IdReference apply(IpConfiguration input) {
private static boolean isOrphanedJcloudsAvailabilitySet(AvailabilitySet availabilitySet) {
// We check for the presence of the 'jclouds' tag to make sure we only
- // delete availability sets that were automatically created by jclouds
+ // deleteVault availability sets that were automatically created by jclouds
I think `delete` was more approriate here
> @@ -140,7 +140,7 @@ public boolean cleanupManagedDisks(VirtualMachine virtualMachine) {
Set<String> nonDeletedDisks = filterValues(deleteJobs, not(resourceDeleted)).keySet();
if (!nonDeletedDisks.isEmpty()) {
- logger.warn(">> could not delete disks: %s", Joiner.on(',').join(nonDeletedDisks));
+ logger.warn(">> could not deleteVault disks: %s", Joiner.on(',').join(nonDeletedDisks));
I think `delete` was more approriate here
> @@ -47,13 +47,13 @@
public abstract int platformFaultDomainCount();
/**
- * A list of virtual machines in the availability set
+ * A listVaults of virtual machines in the availability set
I think `list` was more approriate here
> */
@Nullable
public abstract List<IdReference> virtualMachines();
/**
- * A list of statuses in the availability set
+ * A listVaults of statuses in the availability set
I think `list` was more approriate here
> @@ -52,7 +52,7 @@ public static SSHPublicKey create(final String path, final String keyData) {
}
/**
- * The list of public keys and paths
+ * The listVaults of public keys and paths
I think `list` was more approriate here
> @@ -39,7 +39,7 @@
public abstract OSDisk osDisk();
/**
- * The list of the data disks of the storage profile
+ * The listVaults of the data disks of the storage profile
I think `list` was more approriate here
> @@ -46,8 +46,8 @@
/**
* - create deployment
- * - delete deployment
- * - get information about deployment
+ * - deleteVault deployment
I think `delete` and `get` were more approriate here
> @@ -94,7 +94,7 @@ public void getPublicIPAddress() {
return ipApi.get(name).properties().provisioningState().equals("Succeeded");
}
}, 10 * 1000).apply(publicIpAddressName);
- assertTrue(jobDone, "get operation did not complete in the configured timeout");
+ assertTrue(jobDone, "getVault operation did not complete in the configured timeout");
I think `get` was more approriate here
> @@ -127,7 +127,7 @@ public void createPublicIPAddress() throws InterruptedException {
assertEquals(ip.tags().get("testkey"), "testvalue");
assertNotNull(ip.properties());
assertEquals(ip.properties().provisioningState(), "Updating");
- assertNull(ip.properties().ipAddress()); // as we don't get IP address until Succeeded state
+ assertNull(ip.properties().ipAddress()); // as we don't getVault IP address until Succeeded state
I think `get` was more approriate here
> @@ -330,7 +330,7 @@ public boolean apply(String name) {
return !api().get(name).properties().provisioningState().equals(VirtualMachineProperties.ProvisioningState.CREATING);
}
}, 60 * 20 * 1000).apply(vmName);
- assertTrue(ready, "createOrUpdate operation did not complete in the configured timeout");
+ assertTrue(ready, "createOrUpdateVault operation did not complete in the configured timeout");
I think `createOrUpdate ` was more approriate here
> @@ -88,8 +88,8 @@ public void testGetInstanceDetails() throws Exception {
assertEquals(actual.statuses().get(0).code(), expected.statuses().get(0).code());
assertEquals(actual.statuses().get(0).displayStatus(), expected.statuses().get(0).displayStatus());
assertEquals(actual.statuses().get(0).level(), expected.statuses().get(0).level());
- // assertEquals(actual.statuses().get(0).time().toString(),
- // expected.statuses().get(0).time().toString());
+ // assertEquals(actual.statuses().getVault(0).time().toString(),
I think `get` was more approriate here
> @@ -76,7 +76,7 @@ public void testFailIfNoGeneratedHttpRequest() {
@Test
public void testOverrideMethodVersion() {
Properties props = new Properties();
- props.setProperty(API_VERSION_PREFIX + "named:get", "namedversion");
+ props.setProperty(API_VERSION_PREFIX + "named:getVault", "namedversion");
I think `get` was more approriate here
> @@ -63,7 +63,7 @@ public void setup() {
config = createMock(InvocationConfig.class);
expect(config.getCommandName(noName)).andReturn("VersionedApi.noName");
- expect(config.getCommandName(named)).andReturn("named:get");
+ expect(config.getCommandName(named)).andReturn("named:getVault");
I think `get` was more approriate here
--
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/416#pullrequestreview-73122599
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
1d48e0f add request body check to mock tests, improve stability of live test ordering
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/fd2c2e191d00d35ef3f7222497e8bf8807fd40f5..1d48e0fe42abbe2ed7b630c376d66806665921d5
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
3f94a99 add merge mock tests
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/d477519731ec9a9b20e68f405e42a9163313b8d3..3f94a9924e7c422f3b7afa0726afaaa4d6a4c8bb
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
578dd76 use integers for unix time stamp fields, immutable lists and maps, non-null collections
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/95ec18713f814816d97da9787e359cd96c8bd0dd..578dd76f7b80e32b03812d14f4461562aa62c607
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@nacx - all the requested changes should be in
--
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/416#issuecomment-352896792
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 2 commits.
59f8328 Fix build and checkstyle
ba42d3e Merge pull request #1 from nacx/fix-build
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/bb1682fadb1d7fe3906107fc2819bf65b03a0a5e..ba42d3eb3a505308db72146524ea4fd49cbc8ce0
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
Many thanks for this huge effort!
I've commented mostly style things. Excellent job!
> + }
+
+ @Provides
+ protected Predicate<Supplier<Provisionable>> provideResourceAvailablePredicate(final AzureComputeApi api,
+ @Named(OPERATION_TIMEOUT) Integer operationTimeout, PollPeriod pollPeriod) {
+ return retry(new ResourceInStatusPredicate("Succeeded"), operationTimeout, pollPeriod.pollInitialPeriod,
+ pollPeriod.pollMaxPeriod);
+ }
+
+ @Provides
+ @Named("STORAGE")
+ protected Predicate<URI> provideStorageAccountAvailablePredicate(final AzureComputeApi api,
+ @Named(OPERATION_TIMEOUT) Integer operationTimeout, PollPeriod pollPeriod) {
+ return retry(new ActionDonePredicate(api), operationTimeout, pollPeriod.pollInitialPeriod,
+ pollPeriod.pollMaxPeriod);
+ }
I'd say this predicate is no longer used so we can remove it (not mandatory for this PR, just as a reminder for us)
> + }
+
+ @Provides
+ @Named(VAULT_DELETE_STATUS)
+ protected VaultPredicates.DeletedVaultStatusPredicateFactory provideDeletedVaultStatusPredicateFactory(final AzureComputeApi api,
+ @Named(OPERATION_TIMEOUT) Integer operationTimeout,
+ final PollPeriod pollPeriod) {
+ return new VaultPredicates.DeletedVaultStatusPredicateFactory(api, operationTimeout.longValue(), pollPeriod.pollInitialPeriod, pollPeriod.pollMaxPeriod);
+ }
+
+ public static class VaultPredicates {
+ public static class DeletedVaultStatusPredicateFactory {
+ private final AzureComputeApi api;
+ private final long operationTimeout;
+ private final long initialPeriod;
+ private final long maxPeriod;
I know the existing VirtualMachine predicate uses this pattern, but I think it should be better to let the factory create predicates that don't retry, just simple predicates, and let the `@Provides` method decorate it with the `retry` logic. This way we have an injectable, blocking predicate that retries, but we also can instantiate a simple predicate just to check the status of the resource. Does it make sense to you?
The same change could be applied to the existing VM predicate, to align it with all others.
> +
+ DeletedVaultStatusPredicateFactory(final AzureComputeApi api, final long operationTimeout, final long initialPeriod, final long maxPeriod) {
+ this.api = checkNotNull(api, "api cannot be null");
+ this.operationTimeout = operationTimeout;
+ this.initialPeriod = initialPeriod;
+ this.maxPeriod = maxPeriod;
+ }
+
+ public Predicate<String> create(final String resourceGroup, final boolean shouldBePresent) {
+ checkNotNull(resourceGroup, "resourceGroup cannot be null");
+ return retry(new Predicate<String>() {
+ @Override
+ public boolean apply(final String name) {
+ checkNotNull(name, "name cannot be null");
+ boolean present = false;
+ checkNotNull(name, "name cannot be null");
Duplicate check.
> + DeletedVaultStatusPredicateFactory(final AzureComputeApi api, final long operationTimeout, final long initialPeriod, final long maxPeriod) {
+ this.api = checkNotNull(api, "api cannot be null");
+ this.operationTimeout = operationTimeout;
+ this.initialPeriod = initialPeriod;
+ this.maxPeriod = maxPeriod;
+ }
+
+ public Predicate<String> create(final String resourceGroup, final boolean shouldBePresent) {
+ checkNotNull(resourceGroup, "resourceGroup cannot be null");
+ return retry(new Predicate<String>() {
+ @Override
+ public boolean apply(final String name) {
+ checkNotNull(name, "name cannot be null");
+ boolean present = false;
+ checkNotNull(name, "name cannot be null");
+ List<Vault.DeletedVault> vaults = api.getVaultApi(resourceGroup).listDeletedVaults();
More idiomatic:
```java
return shouldBePresent == Iterables.any(vaults, new Predicate<Vault.DeletedVault>() {
@Override public boolean apply(Vault.DeletedVault input) {
return input.name().equals(name);
}
});
```
> + }
+
+ public Predicate<String> create(final String resourceGroup, final URI vaultUri, final boolean shouldBePresent) {
+ checkNotNull(resourceGroup, "resourceGroup cannot be null");
+ checkNotNull(vaultUri, "vaultUri cannot be null");
+ return retry(new Predicate<String>() {
+ @Override
+ public boolean apply(final String name) {
+ boolean present = false;
+ checkNotNull(name, "name cannot be null");
+ DeletedKeyBundle key = api.getVaultApi(resourceGroup).getDeletedKey(vaultUri, name);
+ if (shouldBePresent) {
+ return key != null;
+ } else {
+ return key == null;
+ }
Just?
```java
return shouldBePresent == (key != null);
```
> +
+ public Predicate<String> create(final String resourceGroup, final URI vaultUri, final boolean isRecovered) {
+ checkNotNull(resourceGroup, "resourceGroup cannot be null");
+ checkNotNull(vaultUri, "vaultUri cannot be null");
+ return retry(new Predicate<String>() {
+ @Override
+ public boolean apply(final String name) {
+ checkNotNull(name, "name cannot be null");
+ boolean result = false;
+ KeyBundle key = api.getVaultApi(resourceGroup).getKey(vaultUri, name);
+ if (key != null) {
+ if (isRecovered) {
+ result = true;
+ } else {
+ result = key.attributes().recoveryLevel().contains("Recoverable");
+ }
[minor style] Prefer inline conditionals.
> + }
+
+ public Predicate<String> create(final String resourceGroup, final URI vaultUri, final boolean shouldBePresent) {
+ checkNotNull(resourceGroup, "resourceGroup cannot be null");
+ checkNotNull(vaultUri, "vaultUri cannot be null");
+ return retry(new Predicate<String>() {
+ @Override
+ public boolean apply(final String name) {
+ boolean present = false;
+ checkNotNull(name, "name cannot be null");
+ DeletedSecretBundle secret = api.getVaultApi(resourceGroup).getDeletedSecret(vaultUri, name);
+ if (shouldBePresent) {
+ return secret != null;
+ } else {
+ return secret == null;
+ }
```java
return shouldBePresent == (secret != null);
```
> @@ -31,10 +31,10 @@
@AutoValue
public abstract static class Permissions {
- @Nullable public abstract List<String> certificates();
- @Nullable public abstract List<String> keys();
- @Nullable public abstract List<String> secrets();
- @Nullable public abstract List<String> storage();
+ public abstract List<String> certificates();
+ public abstract List<String> keys();
+ public abstract List<String> secrets();
+ public abstract List<String> storage();
[minor] leading space.
>
@RequestFilters({ OAuthFilter.class, ApiVersionFilter.class })
@Consumes(MediaType.APPLICATION_JSON)
public interface VaultApi {
+ static class PrependSlashOrEmptyString implements Function<Object, String> {
+ public String apply(Object from) {
+ if((from == null) || (from.toString().length() == 0)) {
[minor style] space after `if`.
> @@ -411,11 +426,12 @@ CertificateOperation createCertificate(@EndpointParam URI vaultBaseUrl,
@Named("certificate:get")
@GET
- @Path("/certificates/{certificateName}")
+ @Path("/certificates/{certificateName}{certificateVersion}{certificateVersion}")
Duplicate version variable.
> public void forceVaultRemoval() {
// see if the vault has been deleted or not
Vault vault = api().getVault(vaultName);
if (vault != null) {
if ((vault.properties().enableSoftDelete() != null) && vault.properties().enableSoftDelete()) {
api().deleteVault(vaultName);
- waitForOperation("vault", "delete", vaultName);
+ Object[] args = { resourceGroupName, vaultName, true };
Unused args?
> @@ -199,107 +119,13 @@ public void forceVaultRemoval() {
DeletedVault deletedVault = api().getDeletedVault(LOCATION, vaultName);
if (deletedVault != null) {
api().purgeVault(LOCATION, vaultName);
- waitForOperation("vault", "purge", vaultName);
+ Object[] args = { resourceGroupName, vaultName, true };
Unused args?
> @@ -394,8 +218,11 @@ public void testUpdateVaultToSoftDelete() {
@Test(dependsOnMethods = {"testPurgeDeletedKey", "testPurgeDeletedSecret"})
public void testDelete() {
- api().deleteVault(vaultName);
- assertTrue(waitForOperation("vault", "delete", vaultName));
+ boolean result = api().deleteVault(vaultName);
+ assertTrue(result);
+ Object[] args = { resourceGroupName, vaultName, true };
Unused args?
> @@ -414,7 +241,10 @@ public void testListDeleted() {
@Test(dependsOnMethods = {"testGetDeleted", "testListDeleted"})
public void testPurgeDeletedVault() {
api().purgeVault(LOCATION, vaultName);
- assertTrue(waitForOperation("vault", "purge", vaultName));
+ Object[] args = { resourceGroupName, vaultName, false };
Unused args?
--
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/416#pullrequestreview-84682266
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 2 commits.
77b0e00 first part of PR request feedback
3dcf25e PR feedback, part 2
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/b3b1164b284996fbe50914f5bfe4b8dd2e9edc10..3dcf25eb34f2965e1301de0ef5dbb54574f2ffd4
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
f43172e merge current head
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/8006a5a405cbadd82a217c0248c7e530d2a81ec4..f43172e01c2f1b30f342d2742bf4ce31415f3927
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Ignasi Barrera <no...@github.com>.
nacx approved this pull request.
Finally! Thanks for the huge effort @jmspring! LGTM. Could you squash the changes and rebase the branch for a clean merge?
--
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/416#pullrequestreview-85293057
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
d477519 make checkstyle happy
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/0d04b2be044b5111d641c015484942e2fabbf69e..d477519731ec9a9b20e68f405e42a9163313b8d3
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@nacx - added Merge tests. Let me know when I should update PR to include OAuth dynamic patch.
--
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/416#issuecomment-347728489
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Ignasi Barrera <no...@github.com>.
Closed #416.
--
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/416#event-1408793861
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
65d4099 support dynamic oauth
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/f43172e01c2f1b30f342d2742bf4ce31415f3927..65d40992ad85db95908653b339fb9aecdedbe7e9
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
74ecb1b modify how version handled for certs/keys/secrets
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/65d40992ad85db95908653b339fb9aecdedbe7e9..74ecb1b51eae01479f1614df38a2f03c21176823
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Ignasi Barrera <no...@github.com>.
Finally merged to [master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/fa63f6b1) and [2.0.x](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/50d71c8e). Many thanks for this huge effort @jmspring!
--
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/416#issuecomment-355170030
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@andreaturli - updates per your request. I hadn't caught those changes when doing the PR. Probably the side affect of a refactor / reformat via IntelliJ.
--
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/416#issuecomment-341266082
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
5fdc251 don't use wildcard import
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/fb858b5491b912f696c7ab61216f3fb73eb6919d..5fdc2519bda0002ab366890c49be869e70e9663f
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
fd2c2e1 use predicates for wait
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/3dcf25eb34f2965e1301de0ef5dbb54574f2ffd4..fd2c2e191d00d35ef3f7222497e8bf8807fd40f5
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
fd8c5fc refactor azure predicates
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/1d48e0fe42abbe2ed7b630c376d66806665921d5..fd8c5fc24a283dba3f1ed0884ad89234bb6a1cd8
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
c9f8713 changes based off of PR feedback
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/fd8c5fc24a283dba3f1ed0884ad89234bb6a1cd8..c9f871301c2d6b123d4c58f5ddde1354a10e5119
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
8f3744b syntax fixes
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/c9f871301c2d6b123d4c58f5ddde1354a10e5119..8f3744b33e4f474cca9037dff2a556e6ee410c82
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
Many thanks @jmspring!
This looks quite good! I've added some general comments. Apart from those, could you paste the summary of the live tests results?
> @@ -53,6 +53,33 @@ Verify service principal
az login -u <Application-id> -p <password> --service-principal --tenant <Tenant-id>
```
+## KeyVault Live Tests
+
+In order to run the KeyVault live tests, an additional parameter is needed. This is the `objectId` of
+the service principal being used for the tests. One can retrieve the service principal `objectId` by
+doing the following using the Azure 2.0 CLI:
Is this property actually needed? Or are now the tests using the existing service principal supplier to automatically get the `objectId`?
> @@ -252,6 +253,15 @@ NetworkSecurityRuleApi getNetworkSecurityRuleApi(@PathParam("resourcegroup") Str
@Delegate
GraphRBACApi getGraphRBACApi();
+ /**
+ * Managing your key vaults as well as the keys, secrets, and certificates within your key vaults can be
+ * accomplished through a REST interface.+
[minor] Remove the trailing `+`.
> +import org.jclouds.javax.annotation.Nullable;
+import org.jclouds.json.SerializedNames;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
+
+@AutoValue
+public abstract class VaultProperties {
+
+ @AutoValue
+ public abstract static class Permissions {
+
+ @Nullable public abstract List<String> certificates();
+ @Nullable public abstract List<String> keys();
+ @Nullable public abstract List<String> secrets();
+ @Nullable public abstract List<String> storage();
These lists are never null since you enforce immutability and presence. Let's remove the `@Nullable` annotation and apply the same pattern for all other collections where you enforce presence.
> + @Fallback(NullOnNotFoundOr404.class)
+ @OAuthResource("https://vault.azure.net")
+ DeletedKeyBundle deleteKey(@EndpointParam URI vaultBaseUrl, @PathParam("keyName") String keyName);
+
+ @Named("key:get_versions")
+ @GET
+ @SelectJson("value")
+ @Path("/keys/{keyName}/versions")
+ @Fallback(EmptyListOnNotFoundOr404.class)
+ @OAuthResource("https://vault.azure.net")
+ List<Key> getKeyVersions(@EndpointParam URI vaultBaseUrl, @PathParam("keyName") String keyName);
+
+ @Named("key:update")
+ @PATCH
+ @MapBinder(BindToJsonPayload.class)
+ @Path("/keys/{keyName}{keyVersion}")
Shouldn't this be `{keyName}/{keyVersion}`?
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * 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 static org.jclouds.http.HttpUtils.releasePayload;
+/**
+ * Parses an http response code from http responser
+ */
+@Singleton
+public class TrueOn20X implements Function<HttpResponse, Boolean> {
Is this class actually used?
> +import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Supplier;
+import com.google.common.collect.ImmutableList;
+
+import javax.inject.Inject;
+
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.assertEquals;
+import static org.testng.AssertJUnit.assertNull;
+
+@Test(groups = "live", testName = "VaultApiLiveTest")
+public class VaultApiLiveTest extends BaseAzureComputeApiLiveTest {
+ @Inject private Supplier<ServicePrincipal> svcPrincipal;
Injection does not work in tests. Remove this and get it from the injector in the base test class if needed.
> + private static String cryptoText = "R29sZCUyNTIxJTJCR29sZCUyNTIxJTJCR2" +
+ "9sZCUyQmZyb20lMkJ0aGUlMkJBbWVyaWNhbiUyQlJpdmVyJTI1MjE";
+ private static String cryptoAlgorithm = "RSA-OAEP";
+ private static String hashToSign = "FvabKT6qGwpml59iHUJ72DZ4XyJcJ8bgpgFA4_8JFmM";
+ private static String signatureAlgorithm = "RS256";
+ private static String contentEncryptionKey = "YxzoHR65aFwD2_IOiZ5rD08jMSALA1y7b_yYW0G3hyI";
+
+ @BeforeClass
+ @Override
+ public void setup() {
+ super.setup();
+ createTestResourceGroup();
+ vaultName = String.format("kv%s", this.getClass().getSimpleName().toLowerCase());
+ }
+
+ @AfterClass
`alwaysRun = true` to cleanup stuff even if tests fail.
> + return;
+ }
+ }
+
+ DeletedVault deletedVault = api().getDeletedVault(LOCATION, vaultName);
+ if (deletedVault != null) {
+ api().purgeVault(LOCATION, vaultName);
+ waitForOperation("vault", "purge", vaultName);
+ }
+ }
+
+ // Note: Operations on a KeyVault that supports soft delete can take time
+ // to settle. Thus for tests where appropriate, a wait operation with a
+ // one minute timeout will be used to make sure things are in the desired
+ // state before proceeding to the next test.
+ private boolean waitForOperation(String itemType, String operation, String entityName) {
jclouds already provides clean means to actively wait for things, and allows to configure the timeouts, etc, via properties. Please use the `Predicates2.retry` helper instead of manually doing this.
This logic might be useful for users too, so I'd strongly recommend to copy how [we configure active waits for Azure ARM](https://github.com/jclouds/jclouds-labs/blob/master/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/config/AzureComputeServiceContextModule.java#L145-L385) and apply the same pattern here:
1. Refactor each conditional in this method into its own `Predicate` class.
2. Put all those predicates in an `AzurePredicatesModule` (or similar), and consider moving the ones that already exist (linked above) to that class.
3. For each defined predicate class, add a `@Provides` method that exposes the predicate encapsulated with the `retry` helper. Consider using the standard jclouds timeout properties or just create Azure-specific properties to configure them if that makes sense, but follow the same pattern.
4. [Initialize the predicates in the base test class](https://github.com/jclouds/jclouds-labs/blob/master/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/internal/BaseAzureComputeApiLiveTest.java#L106-L112) so you can easily use them in the tests.
> + if (i == 0) {
+ break;
+ }
+ try {
+ Thread.sleep(6000);
+ } catch (java.lang.InterruptedException ie) {
+ break;
+ }
+ }
+ }
+
+ return done;
+ }
+
+ @Test
+ @Inject
Remove all `@Inject` annotations from tests.
> + ),
+ ImmutableList.<String>of()
+ ))))
+ .build(),
+ null);
+ vaultUri = vault.properties().vaultUri();
+ assertTrue(!vault.name().isEmpty());
+ }
+
+ @Test(dependsOnMethods = "testCreate")
+ public void testGet() {
+ Vault vaultFound = api().getVault(vaultName);
+ assertTrue(!vaultFound.name().isEmpty());
+ }
+
+ @Test(dependsOnMethods = "testGet")
Better depend on the `testCreate` to maximize the chances of this test being run.
> + "Backup",
+ "Restore",
+ "Purge"
+ ),
+ ImmutableList.<String>of()
+ ))))
+ .build(),
+ null);
+ vaultUri = vault.properties().vaultUri();
+ assertTrue(!vault.name().isEmpty());
+ }
+
+ @Test(dependsOnMethods = "testCreate")
+ public void testGet() {
+ Vault vaultFound = api().getVault(vaultName);
+ assertTrue(!vaultFound.name().isEmpty());
Better `assertNotNull(vaultFound)` to avoid an unexpected NPE?
> + vaultUri = vault.properties().vaultUri();
+ assertTrue(!vault.name().isEmpty());
+ }
+
+ @Test(dependsOnMethods = "testCreate")
+ public void testGet() {
+ Vault vaultFound = api().getVault(vaultName);
+ assertTrue(!vaultFound.name().isEmpty());
+ }
+
+ @Test(dependsOnMethods = "testGet")
+ public void testList() {
+ for (Vault vault : api().listVaults()) {
+ assertNotNull(vault.name());
+ VaultProperties props = vault.properties();
+ assertNotNull(props);
If properties are not nullable there is no need to check this. Deserialization will already fail.
> +
+ Map<String, String> tags = new HashMap<String, String>();
+ tags.put("purpose", "testing again");
+ KeyBundle updatedKey = api().updateKey(vaultUri, KEY_NAME, "/" + version, null, null, tags);
+ assertNotNull(updatedKey);
+
+ keys = api().getKeyVersions(vaultUri, KEY_NAME);
+ assertNotNull(keys);
+ boolean found = false;
+ for (Key k : keys) {
+ if (k.kid().contains(version)) {
+ key = k;
+ found = true;
+ break;
+ }
+ }
style minor:
```java
assertTrue(Iterables.anyMatch(keys, new Predicate<Key>() {
@Override boolean apply(Key input) {
return input.kid().contains(version);
}
}));
```
> + assertEquals(certBundle.tags().size(), 1);
+ }
+
+ @Test(dependsOnMethods = "testUpdateCertificate")
+ public void testUpdateCertificateVersion() {
+ // create a new version of the certificate
+ /*
+ * XXX -- update using version complains about needing policy (required input), yet
+ * passing in the same policy results in the error:
+ *
+ * Policy cannot be updated with a specific version of a certificate
+ *
+ * Will uncomment/fix once this issue is resolved.
+ *
+ */
+ assertTrue(true);
Better throw a SkipException to see the reason why this is not executed in the test output. this will also serve as a reminder for us that this needs to be implemented.
> + "A6RII-HF2BkBcVa9KcAX3_di4KQE70PXgHf-dlz_RgLOJILeG50wzFeBFCLsjEEPp3itmoai" +
+ "E6vfDidCRm5At8Vjka0G-N_afwkIijfQZLT0VaXvL39cIJE2QN3HJPZM8YPUlkFlYnY4GIRy" +
+ "RWSBpK_KYuVufzUGtDi6Sh8pUa67ppa7DHVZlixlmnVqI3Oeg6XUvMqbFFqVSrcNbRQDwVGL" +
+ "3cUtK-KB1PfKg";
+ private static String keySignedData = "uO0r4P1cB-fKsDZ8cj5ahiNw8Tdsudt5zLCeEKOt29" +
+ "LAlPDpeGx9Q1SOFNaR7JlRYVelxsohdzvydwX8ao6MLnqlpdEj0Xt5Aadp-kN84AXW238gab" +
+ "S1AUyiWILCmdsBFeRU4wTRSxz2qGS_0ztHkaNln32P_9GJC72ZRlgZoVA4C_fowZolUoCWGj" +
+ "4V7fAzcSoiNYipWP0HkFe3xmuz-cSQg3CCAs-MclHHfMeSagLJZZQ9bpl5LIr-Ik89bNtqEq" +
+ "yP7Jb_fCgHajAx2lUFcRZhSIKuCfrLPMl6wzejQ2rQXX-ixEkDa73dYaPIrVW4IL3iC0Ufxn" +
+ "fxYffHJ7QCRw";
+ private static String keyWrappedData = "1jcTlu3KJNDBYydhaH9POWOo0tAPGkpsZVizCkHpC" +
+ "3g_9Kg91Q3HKK-rfZynn5W5nVPM-SVFHA3JTankcXX8gx8GycwUh4pMoyil_DV35m2QjyuiT" +
+ "ln83OJXw-nMvRXyKdVfF7nyRcs256kW7gthAOsYUVBrfFS7DFFxsXqLNREsA8j85IqIXIm8p" +
+ "AB3C9uvl1I7SQhLvrwZZXXqjeCWMfseVJwWgsQFyyqH2P0f3-xnngV7cvik2k3Elrk3G_2Cu" +
+ "JCozIIrANg9zG9Z8DrwSNNm9YooxWkSu0ZeDLOJ0bMdhcPGGm5OvKz3oZqX-39yv5klNlCRb" +
+ "r0q7gqmI0x25w";
Consider moving all these huge strings and the ones in the live test to files in `src/test/resources` for better readability.
> + "4V7fAzcSoiNYipWP0HkFe3xmuz-cSQg3CCAs-MclHHfMeSagLJZZQ9bpl5LIr-Ik89bNtqEq" +
+ "yP7Jb_fCgHajAx2lUFcRZhSIKuCfrLPMl6wzejQ2rQXX-ixEkDa73dYaPIrVW4IL3iC0Ufxn" +
+ "fxYffHJ7QCRw";
+ private static String keyWrappedData = "1jcTlu3KJNDBYydhaH9POWOo0tAPGkpsZVizCkHpC" +
+ "3g_9Kg91Q3HKK-rfZynn5W5nVPM-SVFHA3JTankcXX8gx8GycwUh4pMoyil_DV35m2QjyuiT" +
+ "ln83OJXw-nMvRXyKdVfF7nyRcs256kW7gthAOsYUVBrfFS7DFFxsXqLNREsA8j85IqIXIm8p" +
+ "AB3C9uvl1I7SQhLvrwZZXXqjeCWMfseVJwWgsQFyyqH2P0f3-xnngV7cvik2k3Elrk3G_2Cu" +
+ "JCozIIrANg9zG9Z8DrwSNNm9YooxWkSu0ZeDLOJ0bMdhcPGGm5OvKz3oZqX-39yv5klNlCRb" +
+ "r0q7gqmI0x25w";
+
+ @BeforeMethod
+ public void start() throws IOException {
+ super.start();
+ try {
+ vaultUri = server.getUrl("").toURI();
+ } catch (URISyntaxException use) {
Better add the exception to the method signature instead of silently catching it? This way if it is thrown tests will be skipped instead of run with a `null` URI.
> + "Delete",
+ "Recover",
+ "Backup",
+ "Restore",
+ "Purge"
+ ),
+ ImmutableList.<String>of()
+ ))))
+ .build(),
+ null);
+
+ String path = String.format(
+ "/subscriptions/%s/resourcegroups/%s/providers/Microsoft.KeyVault/vaults/%s?%s",
+ subscriptionId, resourceGroup, vaultName, apiVersion
+ );
+ assertSent(server, "PUT", path);
I know this can be kind of a pain, but we need to assert the sent payload too for all requests that have a body. This *really* helps future maintenance of the different APIs.
> + ))))
+ .build(),
+ null);
+
+ String path = String.format(
+ "/subscriptions/%s/resourcegroups/%s/providers/Microsoft.KeyVault/vaults/%s?%s",
+ subscriptionId, resourceGroup, vaultName, apiVersion
+ );
+ assertSent(server, "PUT", path);
+
+ assertNotNull(vault);
+ assertNotNull(vault.properties().vaultUri());
+ assertTrue(!vault.name().isEmpty());
+ }
+
+ public void createVaultReturns404() throws InterruptedException {
There is no need to test the 404 behavior if there is no custom `@Fallback` defined, so you can remove all the 404 tests for those methods that do not have fallbacks. The default behavior (throwing the exception) is already tested in jclouds-core.
> + "/subscriptions/%s/providers/Microsoft.KeyVault/locations/%s/deletedVaults/%s?%s",
+ subscriptionId, location, vaultName, apiVersion
+ );
+ assertSent(server, "GET", path);
+
+ assertNull(vault);
+ }
+
+
+ // Key mock tests
+ public void listKeys() throws InterruptedException {
+ server.enqueue(jsonResponse("/vaultlistkeys.json").setResponseCode(200));
+ final VaultApi vaultApi = api.getVaultApi(resourceGroup);
+ List<Key> keys = vaultApi.listKeys(vaultUri);
+
+ String path = String.format("/keys?%s", apiVersion);
Same comment about skipping the test.
> @@ -99,6 +100,7 @@ public void setup() {
vaultResourceGroup = System.getProperty("test.azurecompute-arm.vault.resource.group");
vaultName = System.getProperty("test.azurecompute-arm.vault.name");
vaultCertificateUrl = System.getProperty("test.azurecompute-arm.vault.certificate.url");
+ tenantId = System.getProperty("test.azurecompute-arm.tenantId");
Change to: `tenantId = injector.getInstance(Key.get(String.class, Tenant.class));`
--
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/416#pullrequestreview-80448092
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@nacx See my note in Slack re: Predicate factories. Here are the test results:
```
-------------------------------------------------------
T E S T S
-------------------------------------------------------
Java HotSpot(TM) 64-Bit Server VM warning: ignoring option MaxPermSize=256m; support was removed in 8.0
Running org.jclouds.azurecompute.arm.features.VaultApiLiveTest
Configuring TestNG with: TestNG652Configurator
Starting test testCreate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testCreate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 4341ms
Test suite progress: tests succeeded: 1, failed: 0, skipped: 0.
Starting test testGet(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGet(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 273ms
Test suite progress: tests succeeded: 2, failed: 0, skipped: 0.
Starting test testList(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testList(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 310ms
Test suite progress: tests succeeded: 3, failed: 0, skipped: 0.
Starting test testCreateCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testCreateCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 2577ms
Test suite progress: tests succeeded: 4, failed: 0, skipped: 0.
Starting test testCreateKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testCreateKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 435ms
Test suite progress: tests succeeded: 5, failed: 0, skipped: 0.
Starting test testEncryptDecrypt(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testEncryptDecrypt(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 660ms
Test suite progress: tests succeeded: 6, failed: 0, skipped: 0.
Starting test testGetCertificateOperation(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetCertificateOperation(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 23164ms
Test suite progress: tests succeeded: 7, failed: 0, skipped: 0.
Starting test testGetCertificatePolicy(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetCertificatePolicy(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 309ms
Test suite progress: tests succeeded: 8, failed: 0, skipped: 0.
Starting test testImportCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testImportCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 723ms
Test suite progress: tests succeeded: 9, failed: 0, skipped: 0.
Starting test testImportKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testImportKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 275ms
Test suite progress: tests succeeded: 10, failed: 0, skipped: 0.
Starting test testListKeys(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testListKeys(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 281ms
Test suite progress: tests succeeded: 11, failed: 0, skipped: 0.
Starting test testSignVerify(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testSignVerify(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 562ms
Test suite progress: tests succeeded: 12, failed: 0, skipped: 0.
Starting test testWrapUnwrapKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testWrapUnwrapKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 614ms
Test suite progress: tests succeeded: 13, failed: 0, skipped: 0.
Starting test testGetCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 308ms
Test suite progress: tests succeeded: 14, failed: 0, skipped: 0.
Starting test testListCertificateVersions(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testListCertificateVersions(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 305ms
Test suite progress: tests succeeded: 15, failed: 0, skipped: 0.
Starting test testListCertificates(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testListCertificates(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 615ms
Test suite progress: tests succeeded: 16, failed: 0, skipped: 0.
Starting test testGetKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 310ms
Test suite progress: tests succeeded: 17, failed: 0, skipped: 0.
Starting test testMergeCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testMergeCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) skipped.
Test suite progress: tests succeeded: 17, failed: 0, skipped: 1.
Starting test testUpdateCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testUpdateCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 1535ms
Test suite progress: tests succeeded: 18, failed: 0, skipped: 1.
Starting test testSetCertificateIssuer(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testSetCertificateIssuer(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 308ms
Test suite progress: tests succeeded: 19, failed: 0, skipped: 1.
Starting test testUpdateCertificatePolicy(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testUpdateCertificatePolicy(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 608ms
Test suite progress: tests succeeded: 20, failed: 0, skipped: 1.
Starting test testUpdateCertificateVersion(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testUpdateCertificateVersion(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) skipped.
Test suite progress: tests succeeded: 20, failed: 0, skipped: 2.
Starting test testUpdateKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testUpdateKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 259ms
Test suite progress: tests succeeded: 21, failed: 0, skipped: 2.
Starting test testGetCertificateIssuer(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetCertificateIssuer(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 191ms
Test suite progress: tests succeeded: 22, failed: 0, skipped: 2.
Starting test testGetCertificateIssuers(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetCertificateIssuers(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 239ms
Test suite progress: tests succeeded: 23, failed: 0, skipped: 2.
Starting test testListKeyVersions(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testListKeyVersions(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 2015ms
Test suite progress: tests succeeded: 24, failed: 0, skipped: 2.
Starting test testUpdateCertificateIssuer(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testUpdateCertificateIssuer(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 360ms
Test suite progress: tests succeeded: 25, failed: 0, skipped: 2.
Starting test testUpdateKeyWithVersion(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testUpdateKeyWithVersion(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 921ms
Test suite progress: tests succeeded: 26, failed: 0, skipped: 2.
Starting test testDeleteCertificateIssuer(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testDeleteCertificateIssuer(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 615ms
Test suite progress: tests succeeded: 27, failed: 0, skipped: 2.
Starting test testBackupRestoreKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testBackupRestoreKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 1705ms
Test suite progress: tests succeeded: 28, failed: 0, skipped: 2.
Starting test testSetCertificateContacts(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testSetCertificateContacts(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 264ms
Test suite progress: tests succeeded: 29, failed: 0, skipped: 2.
Starting test testDeleteKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testDeleteKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 242ms
Test suite progress: tests succeeded: 30, failed: 0, skipped: 2.
Starting test testSetSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testSetSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 246ms
Test suite progress: tests succeeded: 31, failed: 0, skipped: 2.
Starting test testGetCertificateContacts(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetCertificateContacts(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 302ms
Test suite progress: tests succeeded: 32, failed: 0, skipped: 2.
Starting test testGetSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 308ms
Test suite progress: tests succeeded: 33, failed: 0, skipped: 2.
Starting test testGetSecrets(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetSecrets(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 307ms
Test suite progress: tests succeeded: 34, failed: 0, skipped: 2.
Starting test testDeleteCertificateContacts(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testDeleteCertificateContacts(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 617ms
Test suite progress: tests succeeded: 35, failed: 0, skipped: 2.
Starting test testUpdateSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testUpdateSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 754ms
Test suite progress: tests succeeded: 36, failed: 0, skipped: 2.
Starting test testUpdateCertificateOperation(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testUpdateCertificateOperation(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 9172ms
Test suite progress: tests succeeded: 37, failed: 0, skipped: 2.
Starting test testListSecretVersions(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testListSecretVersions(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 818ms
Test suite progress: tests succeeded: 38, failed: 0, skipped: 2.
Starting test testDeleteCertificateOperation(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testDeleteCertificateOperation(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 613ms
Test suite progress: tests succeeded: 39, failed: 0, skipped: 2.
Starting test testUpdateSecretWithVersion(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testUpdateSecretWithVersion(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 662ms
Test suite progress: tests succeeded: 40, failed: 0, skipped: 2.
Starting test testDeleteCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testDeleteCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 570ms
Test suite progress: tests succeeded: 41, failed: 0, skipped: 2.
Starting test testBackupRestoreSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testBackupRestoreSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 1844ms
Test suite progress: tests succeeded: 42, failed: 0, skipped: 2.
Starting test testDeleteSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testDeleteSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 308ms
Test suite progress: tests succeeded: 43, failed: 0, skipped: 2.
Starting test testUpdateVaultToSoftDelete(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testUpdateVaultToSoftDelete(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 1529ms
Test suite progress: tests succeeded: 44, failed: 0, skipped: 2.
Starting test testCreateRecoverableKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testCreateRecoverableKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 3603ms
Test suite progress: tests succeeded: 45, failed: 0, skipped: 2.
Starting test testCreateRecoverableSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testCreateRecoverableSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 693ms
Test suite progress: tests succeeded: 46, failed: 0, skipped: 2.
Starting test testImportRecoverableCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testImportRecoverableCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 1028ms
Test suite progress: tests succeeded: 47, failed: 0, skipped: 2.
Starting test testDeleteRecoverableKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testDeleteRecoverableKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 23549ms
Test suite progress: tests succeeded: 48, failed: 0, skipped: 2.
Starting test testDeleteRecoverableSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testDeleteRecoverableSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 15359ms
Test suite progress: tests succeeded: 49, failed: 0, skipped: 2.
Starting test testDeleteRecoverableCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testDeleteRecoverableCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 35022ms
Test suite progress: tests succeeded: 50, failed: 0, skipped: 2.
Starting test testListDeletedCertificates(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testListDeletedCertificates(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 308ms
Test suite progress: tests succeeded: 51, failed: 0, skipped: 2.
Starting test testListDeletedKeys(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testListDeletedKeys(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 305ms
Test suite progress: tests succeeded: 52, failed: 0, skipped: 2.
Starting test testListDeletedSecrets(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testListDeletedSecrets(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 305ms
Test suite progress: tests succeeded: 53, failed: 0, skipped: 2.
Starting test testGetDeletedCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetDeletedCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 311ms
Test suite progress: tests succeeded: 54, failed: 0, skipped: 2.
Starting test testGetDeletedKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetDeletedKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 170ms
Test suite progress: tests succeeded: 55, failed: 0, skipped: 2.
Starting test testGetDeletedSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetDeletedSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 439ms
Test suite progress: tests succeeded: 56, failed: 0, skipped: 2.
Starting test testRecoverDeletedCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testRecoverDeletedCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 23348ms
Test suite progress: tests succeeded: 57, failed: 0, skipped: 2.
Starting test testRecoverDeletedKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testRecoverDeletedKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 15050ms
Test suite progress: tests succeeded: 58, failed: 0, skipped: 2.
Starting test testRecoverDeletedSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testRecoverDeletedSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 15360ms
Test suite progress: tests succeeded: 59, failed: 0, skipped: 2.
Starting test testPurgeDeletedCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testPurgeDeletedCertificate(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 15975ms
Test suite progress: tests succeeded: 60, failed: 0, skipped: 2.
Starting test testPurgeDeletedKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testPurgeDeletedKey(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 35506ms
Test suite progress: tests succeeded: 61, failed: 0, skipped: 2.
Starting test testPurgeDeletedSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testPurgeDeletedSecret(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 24090ms
Test suite progress: tests succeeded: 62, failed: 0, skipped: 2.
Starting test testDelete(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testDelete(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 9529ms
Test suite progress: tests succeeded: 63, failed: 0, skipped: 2.
Starting test testGetDeleted(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testGetDeleted(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 299ms
Test suite progress: tests succeeded: 64, failed: 0, skipped: 2.
Starting test testListDeleted(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testListDeleted(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 614ms
Test suite progress: tests succeeded: 65, failed: 0, skipped: 2.
Starting test testPurgeDeletedVault(org.jclouds.azurecompute.arm.features.VaultApiLiveTest)
[TestNG] Test testPurgeDeletedVault(org.jclouds.azurecompute.arm.features.VaultApiLiveTest) succeeded: 1228ms
Test suite progress: tests succeeded: 66, failed: 0, skipped: 2.
Tests run: 68, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 355.984 sec - in org.jclouds.azurecompute.arm.features.VaultApiLiveTest
Results :
Tests run: 68, Failures: 0, Errors: 0, Skipped: 2
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 09:30 min
[INFO] Finished at: 2017-12-20T18:49:20-08:00
[INFO] Final Memory: 31M/513M
[INFO] ------------------------------------------------------------------------
```
--
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/416#issuecomment-353252019
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
8006a5a missing resource
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/3f94a9924e7c422f3b7afa0726afaaa4d6a4c8bb..8006a5a405cbadd82a217c0248c7e530d2a81ec4
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
95ec187 remove TrueIf2xx; Null on 404 Fallback for Patch/Post/Put
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/342965b754089cc6f40749c363568b9c6cd4e260..95ec18713f814816d97da9787e359cd96c8bd0dd
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 3 commits.
b994b6a crypto apis and live tests
bd9cbd7 add crypto mock tests
36e3c64 Merge pull request #2 from jmspring/feature/keyvault-crypto
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/74ecb1b51eae01479f1614df38a2f03c21176823..36e3c64ed29a3e9a2a15068a3eeff8451bf5ae78
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
b3b1164 fix lint issues
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/5fdc2519bda0002ab366890c49be869e70e9663f..b3b1164b284996fbe50914f5bfe4b8dd2e9edc10
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
fb858b5 integrate updated graphrbac api
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/be61d50fe8b6561a2dc638b753cffc2fe3a95102..fb858b5491b912f696c7ab61216f3fb73eb6919d
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
be61d50 Merge branch 'master' into feature/keyvault-crud
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/36e3c64ed29a3e9a2a15068a3eeff8451bf5ae78..be61d50fe8b6561a2dc638b753cffc2fe3a95102
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
Thanks @jmspring! Huge effort!
I'm adding some general comments after a first review, but I still have to go in deeper detail through the entire PR again:
* In the model classes, enforce immutable lists.
* Also try to use non-null collections and enforce presence where possible (`tags` are an example of field where null is OK, but in most cases I'd say it should be safe to enforce presence in collections).
* Use the `Date` type for fields that represent dates.
* In the API methods, remove all 404 fallbacks from PUT/POST/PATCH methods.
* Consider removing the TrueIf2xx class if jclouds already provides that behavior by default.
Regarding the KeyVaultFilter. It looks like a clone of the `ClientCredentialsSecretFlow` filter. Instead of overriding it to cover the MWS use case, couldn't we configure the tests to use "localhost" for the vaultURL so the default OAuthFilter can be used? What are the limitations that made you change this?
Thanks for all the effort!
> @@ -411,7 +411,7 @@ private OSProfile createOsProfile(String computerName, Template template) {
private List<NetworkInterfaceCard> createNetworkInterfaceCards(final String nodeName, final String location,
AzureTemplateOptions options) {
- // Prefer a sorted list of NICs with the ones with public IPs first, to
+ // Prefer a sorted listVaults of NICs with the ones with public IPs first, to
typo?
> + }
+ }
+
+ @AutoValue
+ public abstract static class SubjectAlternativeNames {
+ public abstract List<String> dnsNames();
+
+ public abstract List<String> emails();
+
+ public abstract List<String> upns();
+
+ @SerializedNames({"dns_names", "emails", "upns"})
+ public static SubjectAlternativeNames create(final List<String> dnsNames,
+ final List<String> emails,
+ final List<String> upns) {
+ return new AutoValue_Certificate_SubjectAlternativeNames(dnsNames, emails, upns);
Enforce immutability
> +
+ @Nullable
+ public abstract SubjectAlternativeNames subjectAltNames();
+
+ @Nullable
+ public abstract String subject();
+
+ public abstract int validityMonths();
+
+ @SerializedNames({"ekus", "key_usage", "sans", "subject", "validity_months"})
+ public static X509CertificateProperties create(final List<String> enhancedKeyUsage,
+ final List<String> keyUsage,
+ final SubjectAlternativeNames subjectAltNames,
+ final String subject,
+ final int validityMonths) {
+ return new AutoValue_Certificate_X509CertificateProperties(enhancedKeyUsage, keyUsage, subjectAltNames, subject, validityMonths);
Enforce immutability in all lists when present. Is there a reason to keep these lists null, or could we better default to empty lists, to avoid the null-collection anti-pattern?
> + @SerializedNames({"id", "provider"})
+ public static CertificateIssuer create(final String id,
+ final String provider) {
+ return new AutoValue_Certificate_CertificateIssuer(id, provider);
+ }
+ }
+
+ @AutoValue
+ public abstract static class IssuerAttributes {
+ @Nullable
+ public abstract String created();
+
+ public abstract boolean enabled();
+
+ @Nullable
+ public abstract String updated();
Can we use a `Date` type for the created and updated fields?
> + public abstract String id();
+
+ @SerializedNames({"contacts", "id"})
+ public static Contacts create(final List<Contact> contacts,
+ final String id) {
+ return new AutoValue_Certificate_Contacts(contacts, id);
+ }
+ }
+
+ @AutoValue
+ public abstract static class DeletedCertificateBundle {
+ @Nullable
+ public abstract CertificateAttributes attributes();
+
+ @Nullable
+ public abstract String bytes();
[minor] The method name suggests a return type of `byte[]`, but let's keep it as-is if a string really makes more sense given the content of the field.
> + public static Contacts create(final List<Contact> contacts,
+ final String id) {
+ return new AutoValue_Certificate_Contacts(contacts, id);
+ }
+ }
+
+ @AutoValue
+ public abstract static class DeletedCertificateBundle {
+ @Nullable
+ public abstract CertificateAttributes attributes();
+
+ @Nullable
+ public abstract String bytes();
+
+ @Nullable
+ public abstract String deletedDate();
Change type to `Date`?
> + thumbprint
+ );
+ }
+ }
+
+ @Nullable
+ public abstract CertificateAttributes attributes();
+
+ @Nullable
+ public abstract String id();
+
+ @Nullable
+ public abstract Map<String, String> tags();
+
+ @Nullable
+ public abstract String x5t();
Why not "thumbprint" as in other classes?
> + }
+
+ @AutoValue
+ public abstract static class KeyAttributes {
+ @Nullable
+ public abstract Boolean enabled();
+ @Nullable
+ public abstract String created();
+ @Nullable
+ public abstract String expires();
+ @Nullable
+ public abstract String notBefore();
+ @Nullable
+ public abstract String recoveryLevel();
+ @Nullable
+ public abstract String updated();
Use `Date` types where possible.
> +public interface VaultApi {
+ // Vault operations
+ @Named("vault:list")
+ @SelectJson("value")
+ @GET
+ @Path("/resourcegroups/{resourcegroup}/providers/Microsoft.KeyVault/vaults")
+ @Fallback(EmptyListOnNotFoundOr404.class)
+ List<Vault> listVaults();
+
+ @Named("vault:create_or_update")
+ @PUT
+ @MapBinder(BindToJsonPayload.class)
+ @Fallback(NullOnNotFoundOr404.class)
+ @Path("/resourcegroups/{resourcegroup}/providers/Microsoft.KeyVault/vaults/{vaultName}")
+ Vault createOrUpdateVault(@PathParam("vaultName") String vaultName, @PayloadParam("location") String location,
+ @PayloadParam("properties") VaultProperties properties);
Add a `tags` parameter after the "location" one, for consistency with other APIs.
> +
+@RequestFilters({ OAuthFilter.class, ApiVersionFilter.class })
+@Consumes(MediaType.APPLICATION_JSON)
+public interface VaultApi {
+ // Vault operations
+ @Named("vault:list")
+ @SelectJson("value")
+ @GET
+ @Path("/resourcegroups/{resourcegroup}/providers/Microsoft.KeyVault/vaults")
+ @Fallback(EmptyListOnNotFoundOr404.class)
+ List<Vault> listVaults();
+
+ @Named("vault:create_or_update")
+ @PUT
+ @MapBinder(BindToJsonPayload.class)
+ @Fallback(NullOnNotFoundOr404.class)
Do not use 404 fallbacks in write (PUT/POST) operations. An unexisting path should be very unexpected in this case and throw an exception (as opposed to get/delete, where you might expect the resource to not exist and null is fine as a return value).
> + @Path("/providers/Microsoft.KeyVault/deletedVaults")
+ @GET
+ @SelectJson("value")
+ @Fallback(EmptyListOnNotFoundOr404.class)
+ List<DeletedVault> listDeletedVaults();
+
+ @Named("vault:get_deleted")
+ @GET
+ @Path("/providers/Microsoft.KeyVault/locations/{location}/deletedVaults/{vaultName}")
+ @Fallback(NullOnNotFoundOr404.class)
+ DeletedVault getDeletedVault(@PathParam("location") String location, @PathParam("vaultName") String vaultName);
+
+ @Named("vault:purge")
+ @POST
+ @ResponseParser(TrueOn20X.class)
+ @Fallback(FalseOnNotFoundOr404.class)
Remove fallback
> +import com.google.common.base.Supplier;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.jclouds.Constants.PROPERTY_SESSION_INTERVAL;
+import static org.jclouds.oauth.v2.config.OAuthProperties.RESOURCE;
+
+/**
+ * Allow users to customize the api versions for each method call.
+ * <p>
+ * In Azure ARM, each method may have its own api version. This filter allows to
+ * configure the versions of each method, so there is no need to change the code
+ * when Azure deprecates old versions.
+ */
Wrong javadoc
> + @MapBinder(BindToJsonPayload.class)
+ @Fallback(NullOnNotFoundOr404.class)
+ @Path("/resourcegroups/{resourcegroup}/providers/Microsoft.KeyVault/vaults/{vaultName}")
+ Vault createOrUpdateVault(@PathParam("vaultName") String vaultName, @PayloadParam("location") String location,
+ @PayloadParam("properties") VaultProperties properties);
+
+ @Named("vault:get")
+ @Path("/resourcegroups/{resourcegroup}/providers/Microsoft.KeyVault/vaults/{vaultName}")
+ @GET
+ @Fallback(NullOnNotFoundOr404.class)
+ Vault getVault(@PathParam("vaultName") String vaultName);
+
+ @Named("vault:delete")
+ @Path("/resourcegroups/{resourcegroup}/providers/Microsoft.KeyVault/vaults/{vaultName}")
+ @DELETE
+ @ResponseParser(TrueOn20X.class)
Is this needed? Doesn't jclouds return true for 2xx by default when the return type is a boolean? If so, let's remove tis parser class.
--
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/416#pullrequestreview-74423778
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@nacx - Most of the changes you requested are done. Where "Date" is specified in the Azure KeyVault REST docs, I've used a Date type. In the case where it is a "unix timestamp", I've gone with Integer.
Re: KeyVaultFilter - the issue is Vault URIs for OAuth verification differ from the management (default) resource URI. In the case of Mock tests, management APIs use bearer token auth instead of client secret. I need to figure out the same for Vault manipulation methods.
Let me know about the deeper pass on things.
--
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/416#issuecomment-344794392
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault
management and certificate/key/secret operations (#416)
Posted by Jim Spring <no...@github.com>.
@jmspring pushed 1 commit.
0d04b2b fix typo
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/416/files/578dd76f7b80e32b03812d14f4461562aa62c607..0d04b2be044b5111d641c015484942e2fabbf69e