Allow the user to specify individual credentials on the command line

[ijc]

e.g.

docker app install --credential name=somevalue bundle.json

Credentials added with --credential always come after those added with
--credential-set (irrespective of the order on the command line).

A credential specified with --credential cannot override any previous
credential, including those specified in a credential set.

The test bnudle used is based on
https://github.com/deislabs/example-bundles/blob/0e8af9a2f1270bd72045a515637a432e74743d5d/example-credentials/bundle.json
But with cnab/example-credentials:latest → a digested ref (with the digest I
pulled today)

Signed-off-by: Ian Campbell [email protected]

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

[ijc]

This is the basics, but I'm going to look at implementing support for --credential name=@file to read from file (like `curl has) and will add that here.

[ijc]

I added a commit to allow --credential name=@file.

I'm not 100% sure about this, especially the UX complexity needed to allow literal strings which start with @ (see the comment). It might be better to have a separate --credential-file name=file thing, but then we get into a complicated discussion about the precedence/ordering of --credential-set, --credential and --credential-file and how they interact.

[ijc]

Maybe the answer to "then we get into a complicated discussio" is a custom flag.Var compatible variable which can accumulate all of --credential-set, --credential and --credential-file into one place and preserve the order they appeared on the command line such that we can then do the more logical/expected thing and apply them in the order they are given.

[simonferquel]

I am not sure about the "@" syntax. Something that might be used a lot (I think) is exec instead of reading a file (e.g. for executing a gcp/aws/azure CLI tool for obtaining a short lived token). This is already supported in credential-sets, and I suppose also with doing things like --credential "name=$(awscli get-token ...)".

[ijc]

Exec instead of reading from a file is handled by the shell and not the tool I think (maybe it's different on windows?).

I think the right thing to do is just drop the @ syntax, you can always use --credential foo=<(/some/file) (or --credential foo=$(cat /some/file))

[ijc]

OK, dropped the final commit in favour of recommending the use of the shell syntax.

I need to look at why the linter in complaining.

[ijc]

Linter fixed.

[silvin-lubecki]

There's a conflict, please rebase 🦁

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@8aacca2). Click here to learn what that means.
The diff coverage is 80%.

@@            Coverage Diff            @@
##             master     #531   +/-   ##
=========================================
  Coverage          ?   70.06%           
=========================================
  Files             ?       55           
  Lines             ?     3053           
  Branches          ?        0           
=========================================
  Hits              ?     2139           
  Misses            ?      630           
  Partials          ?      284

Impacted Files	Coverage Δ
internal/commands/root.go	88.57% <100%> (ø)
internal/commands/cnab.go	72.85% <77.77%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8aacca2...9ece4d7. Read the comment docs.

[ijc]

@silvin-lubecki rebase, dropped that blank line and fixed a random bug I spotted while refactoring during the rebase (see the new first commit)

[silvin-lubecki]

LGTM 👍

[silvin-lubecki]

Tests are failing

=== RUN   TestCredentials/missing
    --- FAIL: TestCredentials/missing (0.42s)
        commands_test.go:465: assertion failed: 
            --- expected
            +++ actual
            @@ -1,2 +1,2 @@
            -bundle requires credential for secret2
            +bundle requires credential for secret3

[ijc]

Urk, it passed on the other CI. It must be somehow non-deterministic (not sure how I missed that!). I guess there is a map in there somewhere. Will see what I can do!

[ijc]

Hm, from running:

e2e$ DOCKERAPP_BINARY=../bin/docker-app DOCKERCLI_BINARY=../bin/docker-linux go test -test.run TestCredentials/missing -test.v -test.count=100 .

It seems to have a ~8% failure rate (I'd have guessed either 50% or 33%) I guess that's why I didn't see it.

It's easy enough to fix -- I'll just make sure only one credential is missing.

[ijc]

Fixed, it survived the -test.count=100 rune from above.

[jcsirot]

LGTM