notation login CLI by SteveLasker · Pull Request #223 · notaryproject/notation

SteveLasker · 2022-07-06T00:06:16Z

Updates for #119
Signed-off-by: Steve Lasker stevenlasker@hotmail.com

Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>

shizhMSFT · 2022-07-06T01:00:24Z

/cc @binbin-li for comments

shizhMSFT

LGTM with suggestions.

shizhMSFT · 2022-07-06T07:45:22Z

specs/notation-cli.md

+   --username value, -u value    Username for registry operations
+   --password value, -p value    Password for registry operations


How about environment variables associated with -u and -p like NOTATION_USERNAME and NOTATION_PASSWORD?

Suggested change

--username value, -u value Username for registry operations

--password value, -p value Password for registry operations

--username value, -u value Username for registry authentication

--password value, -p value Password for registry authentication

Should we also support --password-stdin like docker cli does?

For operations vs. authentication, I was thinking saying authentication was too obvious, and used the more generic, the -u/-p was used for multiple registry operations. If others feel different, I can change to either way.

shizhMSFT · 2022-07-06T07:46:25Z

specs/notation-cli.md

+  <server>                       The registry URL for authentication
+
+GLOBAL ARGUMENTS
+   --plain-http                  Registry access via plain HTTP (default: false)


It seems we only support basic configurations. Will we support more parameters like --insecure and --ca-file as oras?

@shizhMSFT, what is --insecure used for, and how common is the usage for --ca-file (I assume provides custom TLS trust store).

--insecure is to skip the TLS cert check. Since notation is security oriented, we should not provide this functionality.

--ca-file is to specify the root CA cert to registry access. See oras-project/oras#217

@shizhMSFT, I was thinking this would be done with the insecure registries option in config.
Does that still exist?
Seems calling --insecure is too tedious. For the scenarios/instances where https isn't supported, such as IOT environments, I just assumed the config would solve the problem and not something that's done ad-hoc requiring the cli to support it.

gokarnm

LGTM with suggestions.

gokarnm · 2022-07-07T05:50:23Z

specs/notation-cli.md

+```console
+notation login --help
+NAME:
+   notation login - Provides credentials for authenticated registry operations


Suggested change

notation login - Provides credentials for authenticated registry operations

notation login - Login to a registry with credentials for authenticated registry operations

gokarnm · 2022-07-07T05:50:36Z

specs/notation-cli.md

   certificate, cert  Manage certificates used for verification
   key                Manage keys used for signing
   list, ls           List signatures from remote
+   login              Provide credentials for authenticated registry operations   


Suggested change

login Provide credentials for authenticated registry operations

login Login to a registry with credentials for authenticated registry operations

Saying login, where the parameter says login feels redundant. I was just trying to find a synonym verb.

gokarnm · 2022-07-07T05:53:47Z

specs/notation-cli.md

+   --username value, -u value    Username for registry operations
+   --password value, -p value    Password for registry operations


Suggested change

--username value, -u value Username for registry operations

--password value, -p value Password for registry operations

--username value, -u value Username for registry authentication

--password value, -p value Password for registry authentication

gokarnm · 2022-07-07T05:57:06Z

specs/notation-cli.md

+   --username value, -u value    Username for registry operations
+   --password value, -p value    Password for registry operations


Should we also support --password-stdin like docker cli does?

gokarnm · 2022-07-07T05:58:14Z

specs/notation-cli.md

+  <server>                       The registry URL for authentication
+
+GLOBAL ARGUMENTS
+   --plain-http                  Registry access via plain HTTP (default: false)


@shizhMSFT, what is --insecure used for, and how common is the usage for --ca-file (I assume provides custom TLS trust store).

…mands Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>

SteveLasker · 2022-07-11T17:53:35Z

I've updated the PR to:

add $NOTATION_USERNAME, $NOTATION_PASSWORD on notation login
Removed -u -p from other commands, streamlinging so users can benefit from existing cred stores, not having to provide -u -p on every command. This is similar to docker push|pull where the user either does docker login, or benefits from a previously configured store.

* notation login CLI * Update to use environment variables and remove -u/-p on all other commands Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>

notation login CLI

0444dcf

Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>

SteveLasker added this to the RC-1 milestone Jul 6, 2022

SteveLasker requested a review from a team July 6, 2022 00:06

SteveLasker added 3 commits July 5, 2022 17:12

resolve merge conflicts

5ccecd2

Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>

add accidentlal removal of list back

0c16151

Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>

add accidentlal removal of list back

3a888a3

Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>

SteveLasker mentioned this pull request Jul 6, 2022

Document standard CLI help format #224

Closed

4 tasks

shizhMSFT approved these changes Jul 6, 2022

View reviewed changes

gokarnm approved these changes Jul 7, 2022

View reviewed changes

Update to use environment variables and remove -u/-p on all other com…

317dc30

…mands Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>

binbin-li mentioned this pull request Jul 12, 2022

[Feature] support notation login #218

Merged

9 tasks

SteveLasker merged commit 2212d98 into main Jul 12, 2022

SteveLasker deleted the issue-119 branch July 19, 2022 15:27

chloeyin mentioned this pull request Jul 25, 2022

notation sign doesn't work on main branch #259

Closed

		--username value, -u value Username for registry operations
		--password value, -p value Password for registry operations

	notation login - Provides credentials for authenticated registry operations
	notation login - Login to a registry with credentials for authenticated registry operations

	login Provide credentials for authenticated registry operations
	login Login to a registry with credentials for authenticated registry operations

Comments

Conversation

SteveLasker commented Jul 6, 2022

Uh oh!

shizhMSFT commented Jul 6, 2022

Uh oh!

shizhMSFT left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gokarnm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveLasker commented Jul 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants