Skip to content

update: added log to notation login and logout commands#484

Merged
priteshbandi merged 7 commits intonotaryproject:mainfrom
patrickzheng200:loginNout
Dec 20, 2022
Merged

update: added log to notation login and logout commands#484
priteshbandi merged 7 commits intonotaryproject:mainfrom
patrickzheng200:loginNout

Conversation

@patrickzheng200
Copy link
Copy Markdown
Contributor

@patrickzheng200 patrickzheng200 commented Dec 14, 2022 •
edited
Loading

This PR adds logs to the login and logout commands.

This PR is a part of resolving notaryproject/roadmap#71.

Added log to notation login and logout commands
9eafa12 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 14, 2022 •
edited
Loading

Codecov Report

Merging #484 (aafaaa7) into main (7a947d5) will decrease coverage by 0.01%.
The diff coverage is 31.25%.

@@            Coverage Diff             @@
##             main     #484      +/-   ##
==========================================
- Coverage   32.14%   32.12%   -0.02%     
==========================================
  Files          25       25              
  Lines        1341     1357      +16     
==========================================
+ Hits          431      436       +5     
- Misses        898      909      +11     
  Partials       12       12
Impacted Files Coverage Δ
cmd/notation/registry.go 0.00% <0.00%> (ø)
cmd/notation/logout.go 44.11% <20.00%> (-10.05%) ⬇️
cmd/notation/login.go 41.86% <22.22%> (-0.83%) ⬇️
pkg/auth/native_store.go 91.11% <100.00%> (+0.41%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

updated corresponding CLI specs
0dd99c3 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@patrickzheng200 patrickzheng200 self-assigned this Dec 14, 2022
@patrickzheng200 patrickzheng200 added this to the RC-2 milestone Dec 14, 2022
JeyJeyGao
JeyJeyGao reviewed Dec 14, 2022
View reviewed changes
update per code review
ffa8e13 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@patrickzheng200 patrickzheng200 changed the title update: Added log to notation login and logout commands update: added log to notation login and logout commands Dec 14, 2022
@patrickzheng200 patrickzheng200 added the UX User experience changes label Dec 14, 2022
priteshbandi
priteshbandi reviewed Dec 15, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@priteshbandi priteshbandi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we are not adding much information in debug logs, do we really want debug log for login logout commands?

cmd/notation/login.go
},
}
opts.LoggingFlagOpts.ApplyFlags(command.Flags())
opts.SecureFlagOpts.ApplyFlags(command.Flags())
Copy link
Copy Markdown
Contributor

@priteshbandi priteshbandi Dec 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need SecureFlagOpts for login/logout?

Copy link
Copy Markdown
Contributor Author

@patrickzheng200 patrickzheng200 Dec 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need SecureFlagOpts for login/logout?

This flag only appears in login. It contains username and password fields which are required in the login process.

@patrickzheng200
Copy link
Copy Markdown
Contributor Author

patrickzheng200 commented Dec 19, 2022 •
edited
Loading

Looks like we are not adding much information in debug logs, do we really want debug log for login logout commands?

Oh yeah, so I think one good reason to add debug logs to login is that there are several https roundtrips among notation, registry, and authorization service -> spec. Printing out those traces could be useful during debug.
As for logout, the logger prints out the name of the credential helper program for debug purpose, as we are using Docker's credential helpers whose config file is out of Notation's scope.

update per code review
aafaaa7 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
shizhMSFT
shizhMSFT approved these changes Dec 19, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with suggestions.

Patrick Zheng added 3 commits December 19, 2022 18:32
priteshbandi
priteshbandi reviewed Dec 20, 2022
View reviewed changes
specs/commandline/logout.md
Comment on lines -26 to -31

### Log out from all logged in registries

```shell
notation logout --all
```
Copy link
Copy Markdown
Contributor

@priteshbandi priteshbandi Dec 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened issue to track this #489

@priteshbandi priteshbandi mentioned this pull request Dec 20, 2022
Closed
priteshbandi
priteshbandi approved these changes Dec 20, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@priteshbandi priteshbandi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@priteshbandi priteshbandi merged commit 9e4e47e into notaryproject:main Dec 20, 2022
@patrickzheng200 patrickzheng200 deleted the loginNout branch December 20, 2022 06:30
@patrickzheng200 patrickzheng200 restored the loginNout branch December 20, 2022 06:30
vaninrao10 pushed a commit to vaninrao10/notation that referenced this pull request Dec 29, 2022
@vaninrao10
update: added log to notation login and logout commands (notaryprojec…
3a20a17 
…t#484)

This PR adds logs to the login and logout commands.

This PR is a part of resolving
notaryproject/roadmap#71.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
vaninrao10 pushed a commit to vaninrao10/notation that referenced this pull request Dec 29, 2022
@vaninrao10
update: added log to notation login and logout commands (notaryprojec…
b4317b9 
…t#484)

This PR adds logs to the login and logout commands.

This PR is a part of resolving
notaryproject/roadmap#71.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
vaninrao10 pushed a commit to vaninrao10/notation that referenced this pull request Dec 29, 2022
@vaninrao10
update: added log to notation login and logout commands (notaryprojec…
106d17f 
…t#484)

This PR adds logs to the login and logout commands.

This PR is a part of resolving
notaryproject/roadmap#71.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
vaninrao10 pushed a commit to vaninrao10/notation that referenced this pull request Jan 3, 2023
@vaninrao10
update: added log to notation login and logout commands (notaryprojec…
0ecfa06 
…t#484)

This PR adds logs to the login and logout commands.

This PR is a part of resolving
notaryproject/roadmap#71.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
vaninrao10 pushed a commit to vaninrao10/notation that referenced this pull request Jan 3, 2023
@vaninrao10
update: added log to notation login and logout commands (notaryprojec…
36ecebf 
…t#484)

This PR adds logs to the login and logout commands.

This PR is a part of resolving
notaryproject/roadmap#71.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
vaninrao10 pushed a commit to vaninrao10/notation that referenced this pull request Jan 3, 2023
@vaninrao10
update: added log to notation login and logout commands (notaryprojec…
b2f8c4c 
…t#484)

This PR adds logs to the login and logout commands.

This PR is a part of resolving
notaryproject/roadmap#71.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
vaninrao10 pushed a commit to vaninrao10/notation that referenced this pull request Jan 5, 2023
@vaninrao10
update: added log to notation login and logout commands (notaryprojec…
5bb429e 
…t#484)

This PR adds logs to the login and logout commands.

This PR is a part of resolving
notaryproject/roadmap#71.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: vaninrao10 <111005862+vaninrao10@users.noreply.github.com>
7h3-3mp7y-m4n pushed a commit to 7h3-3mp7y-m4n/notation that referenced this pull request Mar 29, 2025
@7h3-3mp7y-m4n
update: added log to notation login and logout commands (notaryprojec…
639b9fb 
…t#484)

This PR adds logs to the login and logout commands.

This PR is a part of resolving
notaryproject/roadmap#71.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
FeynmanZhou pushed a commit to FeynmanZhou/notation that referenced this pull request May 15, 2025
@FeynmanZhou
update: added log to notation login and logout commands (notaryprojec…
a59adad 
…t#484)

This PR adds logs to the login and logout commands.

This PR is a part of resolving
notaryproject/roadmap#71.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@priteshbandi priteshbandi priteshbandi approved these changes

@shizhMSFT shizhMSFT shizhMSFT approved these changes

@rgnote rgnote Awaiting requested review from rgnote rgnote is a code owner

@JeyJeyGao JeyJeyGao Awaiting requested review from JeyJeyGao

Assignees

@patrickzheng200 patrickzheng200

Labels

UX User experience changes

Projects

None yet

Milestone

RC-2

Development

Successfully merging this pull request may close these issues.

5 participants

@patrickzheng200 @codecov-commenter @priteshbandi @shizhMSFT @JeyJeyGao