Added PR — https://github.com/jclouds/jclouds-labs/pull/416
Currently failing with no violations listed — https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/1850/org.apache.jclouds.labs$azurecompute-arm/violations/ I’ve added notes regarding the status in the PR and a couple of things to look into. -jim On October 30, 2017 at 1:32:21 AM, Ignasi Barrera (n...@apache.org) wrote: Thanks Jim. The plan LGTM. For the tests in (3), we could do it in two steps. A first impl that relies on an existing storage account, and then we can see if we really need to add the functionality to create the storage accounts dynamically. (Is there any planned ETA for the storage account APIs to the new ARM version?). Don't worry about huge PRs :) Happy to review! On 29 October 2017 at 04:01, Jim Spring <jmspr...@gmail.com> wrote: > An update — > > I’ll probably finish mock tests tomorrow and issue the PR outlined in (1). > That said, there are two API calls that will be stubbed out in tests: > > - Update Certificate Policy based off of a certificate version > - Azure is giving an error that modifying the policy of a versioned > certificate is not allowed (even in the case where the policy doesn’t > change). I need to talk with the product team on this. > - Merge Certificate > - I don’t think there can be a live test for this as it’s specific to the > case where you have a CSR generated with Azure KeyVault and manually have > the certificate signed externally. I’ll also check with the product team > to see if there is some hack around this at least to add a test. > > Regarding (3), the operations rely upon an external storage account being > created (this is confirmed by the product team), in essence the KeyVault > takes over management of keys and shared access signatures for the storage > account. The Azure storage account API relies on the old XML based API > which predates the Azure Resource Manager JSON apis. > > My proposal still stands at the three prior steps, with the caveat of tests > above. > > For (3), I will see if I can either leverage existing JClouds functionality > or I will (in Test) implement a minimal set of functionality needed to set > up the storage account in order to complete the KeyVault APIs. > > So, I should have the PR for (1) tomorrow, I’ll make sure to integrate > Andrea’s earlier input re: camel case. And it will be directly from the > branch I am developing on into JClouds proper. > > The PR, sadly, won’t be a small one. But, I’ve stuck to inner classes, > etc. to not cause class file bloat. > > -j > > > On October 26, 2017 at 3:10:05 AM, Ignasi Barrera (n...@apache.org) wrote: > > Sounds like a good plan. > > Regarding the tests, yes, mock tests are important. The implement "the > contract" between jclouds and the "provider API docs". They are very > useful to early detect incorrect json mappings, serialization issues, > verify pagination, redirections, authentication, and other stuff, so > please, include them. > > WRT (3), agree. Let's first clarify the usage patterns and expected > workflow when using the API and then let's discuss how can we > translate that into jclouds APIs. > > > Thanks for taking care of this! > > I. > > On 26 October 2017 at 05:10, Jim Spring <jmspr...@gmail.com> wrote: >> @Ignasi, @Andrea - >> >> Here is my proposal - the PR should be done in sections. I would propose >> three: >> >> 1. A PR implementing Vault, Key, Secret, and Certificate operations. >> 2. A PR implementing decrypt/encrypt/wrap/unwrap/sign/verify operations. >> 3. A PR implementing Storage Account and SAS operations >> >> The reasoning is as follows: >> >> (1) implements the management of the vault itself and cryptographic >> primitives. Applications will want to leverage the vault for the storage, >> retrival, and versioning of these resources. >> >> (2) adds the ability to use keys from (1) to perform operations. With >> non-HSM keys, these could probably be done externally. But this builds / >> is additive off of (1) and, in my opinion, will be used by fewer people >> than those wanting the support of (1). >> >> (3) To keep it clean, SA and SAS operations directly tie the KeyVault to >> needing to also implement Azure Storage Service functionality. Or have an >> existing storage account. If I were to think about the live tests, one >> would need to create a storage account in order to leverage this >> functionality. Storage accounts use the older ASM/XML based API. Before >> implementing this, I would prefer to talk with the internal KeyVault team >> to understand more about this area. >> >> In terms of timing, I will have (1) done (minus feedback) either later >> tonight or tomorrow including live tests. Are mock tests still required? >> >> I could have (2) done towards the end of this week/early next. >> >> (3) will depend on meetings with the KeyVault team to understand a bit > more >> about documentation gaps and usage patters. >> >> Thoughts? >> >> Thanks >> -jim >> >> >> On October 23, 2017 at 1:50:38 AM, Andrea Turli (andrea.tu...@gmail.com) >> wrote: >> >> FYI Jim has already started fixing some issue at >> https://github.com/andreaturli/jclouds-labs/pull/9/files >> >> I'd suggest to get my implementation in my andreaturli:feature/vault-api >> branch and open a PR against jclouds/jclouds-labs. >> >> Andrea >> >> On Mon, Oct 23, 2017 at 10:18 AM, Ignasi Barrera <n...@apache.org> wrote: >> >>> > How much of the KeyVault api should we implement? I’ve started > creating >>> actual tests for the Key operations and will be prioritizing the list in >>> order that I’ll be adding them. >>> >>> I don't know how complex the API is, but it is quite straightforward >>> to implement APIs in jclouds once you have a couple methods right, so >>> I'd say we'd try to implement all methods, if possible. I'll be happy >>> to help here. >>> >>> > >>> > I’ll try and do a PR per set (if you don’t mind Andrea). >>> >>> PRs always welcome! >>> >>> > >>> > I’ll try and catch you guys on IRC tomorrow morning pacific time. >>> >>> I've been quite offline recently but I hope to start having more time, >>> so feel free to ping me whenever you find me there. >>> >>> > >>> > -j >>> > >>> > Sent from my iThingy >>> > >>> >> On Oct 17, 2017, at 15:39, Jim Spring <jmspr...@gmail.com> wrote: >>> >> >>> >> Andrea - >>> >> >>> >> Take a look at the PR I did against your feature/vault-api branch. > I’m >>> extending the tests now. >>> >> >>> >> I’ll do a PR once I get all the key tests implemented. >>> >> >>> >> Would it make sense to find some time to chat? Or are you ok with me >>> moving forward on extending things? >>> >> >>> >> -jim >>> >> >>> >> >>> >>> On October 17, 2017 at 1:52:20 AM, Andrea Turli ( >>> andrea.tu...@gmail.com) wrote: >>> >>> >>> >>> Thanks Jim! >>> >>> >>> >>> An additional property makes sense for liveTests as well. >>> >>> Direct use of Azure AD GraphApi looks a bit hard to me, so let's > make >>> >>> jclouds accept an extra parameter that will allow at least to > specify >>> an >>> >>> already existing objectID. >>> >>> wdyt? >>> >>> >>> >>> On Tue, Oct 17, 2017 at 10:33 AM, Ignasi Barrera <n...@apache.org> >>> wrote: >>> >>> >>> >>> > Thanks Jim! >>> >>> > >>> >>> > I think it makes sense for the live tests to get the value >> configured >>> >>> > as a property. >>> >>> > >>> >>> > If someone wants to directly use the KeyVault API, I think it > makes >>> >>> > sense to have the objectId as a required parameter (as it is now). >> I >>> >>> > don't see an immediate need to auto-discover it or to have it >>> >>> > configured per-context in advance, so I'd say a property just for >> the >>> >>> > tests is fine. >>> >>> > >>> >>> > >>> >>> > I. >>> >>> > >>> >>> > On 16 October 2017 at 20:44, Jim Spring <jmspr...@gmail.com> > wrote: >>> >>> > > Andrea - >>> >>> > > >>> >>> > > A PR is present plumbing in the ObjectID. I’ll be looking at >>> fleshing >>> >>> > out >>> >>> > > the tests more today. >>> >>> > > >>> >>> > > -jim >>> >>> > > >>> >>> > > >>> >>> > > On October 16, 2017 at 9:48:39 AM, Jim Spring ( jmspr...@gmail.com) > >> >>> >>> > wrote: >>> >>> > > >>> >>> > > Andrea, Ignasi - >>> >>> > > >>> >>> > > I’ve managed to figure out the authentication issue Andrea was >>> running >>> >>> > into >>> >>> > > with his KeyVault implementation keeping the live tests from >>> working. I >>> >>> > > don’t think a PR is needed, but I am cleaning up the code to > just >>> double >>> >>> > > check. >>> >>> > > >>> >>> > > Basically, the issue is as follows: >>> >>> > > >>> >>> > > 1. KeyVault relies upon Azure AD for access control (this is the >>> Object >>> >>> > ID >>> >>> > > passed in) >>> >>> > > 2. A service principal (or other Azure AD object) typically has >>> two IDs >>> >>> > > associated with it: >>> >>> > > - The name or AppID >>> >>> > > - An ObjectID >>> >>> > > >>> >>> > > For Azure tests, currently, one must specify the Service >> Principal >>> >>> > Name/App >>> >>> > > ID as well as the secret. For the creation of the KeyVault in > the >>> test, >>> >>> > > one needs the “ObjectID” of the Service Principal used to login. >>> >>> > > >>> >>> > > There are two ways to fix this: >>> >>> > > >>> >>> > > 1. Implement the Azure AD Graph API in order to look the >>> information up >>> >>> > > (note, the SP may not have access to do so) >>> >>> > > 2. Add another parameter to be specified for Vault Live tests, >> say >>> >>> > > something like 'test.azurecompute-arm.identity.objectid’ >>> >>> > > >>> >>> > > Thoughts? >>> >>> > > >>> >>> > > I’m going through my code changes now and will reach out for >>> anything >>> >>> > > required there to Andrea. >>> >>> > > >>> >>> > > -jim >>> >>> > >>>
