Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault management and certificate/key/secret operations (#416)
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)
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)
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)
@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)
@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
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault management and certificate/key/secret operations (#416)
@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)
nacx commented on this pull request. Many thanks for this huge effort! I've commented mostly style things. Excellent job! > +} + +@Provides +protected PredicateprovideResourceAvailablePredicate(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 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 create(final String resourceGroup, final boolean shouldBePresent) { +checkNotNull(resourceGroup, "resourceGroup cannot be null"); +return retry(new Predicate() { +@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 create(final String resourceGroup, final boolean shouldBePresent) { +checkNotNull(resourceGroup, "resourceGroup cannot be null"); +return retry(new Predicate() { +@Override +public boolean apply(final String name) { +checkNotNull(name, "name cannot be null"); +boolean present = false; +checkNotNull(name, "name cannot be null"); +List vaults = api.getVaultApi(resourceGroup).listDeletedVaults(); More idiomatic: ```java return shouldBePresent == Iterables.any(vaults, new Predicate() { @Override public boolean apply(Vault.DeletedVault input) { return input.name().equals(name); } }); ``` > +} + +public Predicate create(final String resourceGroup, final URI vaultUri, final boolean shouldBePresent) { +checkNotNull(resourceGroup, "resourceGroup cannot be null"); +
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault management and certificate/key/secret operations (#416)
@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)
@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)
@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)
@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)
@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)
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 -p --service-principal --tenant
```
+## 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 certificates();
+ @Nullable public abstract List keys();
+ @Nullable public abstract List secrets();
+ @Nullable public abstract List 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 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 {
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 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.
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault management and certificate/key/secret operations (#416)
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 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)
@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)
@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)
@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)
@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)
@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)
@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)
@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)
@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)
@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)
@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)
@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)
@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)
@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)
@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
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault management and certificate/key/secret operations (#416)
@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)
@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)
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 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 dnsNames();
+
+ public abstract List emails();
+
+ public abstract List upns();
+
+ @SerializedNames({"dns_names", "emails", "upns"})
+ public static SubjectAlternativeNames create(final List dnsNames,
+ final List emails,
+ final List 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
enhancedKeyUsage,
+ final List
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 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 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
Re: [jclouds/jclouds-labs] Add Azure KeyVault support -- vault management and certificate/key/secret operations (#416)
@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)
@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)
@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)
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 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 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
[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
