Skip to content

Comments

update: renamed flag --plain-http to --insecure-registry#674

Merged
priteshbandi merged 9 commits intomainfrom
unknown repository
May 22, 2023
Merged

update: renamed flag --plain-http to --insecure-registry#674
priteshbandi merged 9 commits intomainfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented May 19, 2023 •
edited by ghost
Loading

This PR is based on discussions in issue #623 with following changes:

  1. Renamed flag --plain-http to --insecure-registry for notation sign, verify, list, inspect and login. And added the original flag name --plain-http as an alias of the new name.
  2. Updated the flag's description to use HTTP protocol while connecting to registries. Should be used only for testing. Related specs are updated as well.

Resolves #623.

renamed --plain-http to --insecure-registry; updated descriptions
e183721 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@codecov-commenter
Copy link

codecov-commenter commented May 19, 2023 •
edited by codecov bot
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.35%. Comparing base (c55aab0) to head (2b50e15).
Report is 205 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #674      +/-   ##
==========================================
- Coverage   61.37%   61.35%   -0.02%     
==========================================
  Files          42       42              
  Lines        2281     2280       -1     
==========================================
- Hits         1400     1399       -1     
  Misses        782      782              
  Partials       99       99

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

update based on discussions
a5dd35b 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@ghost ghost changed the title update: renamed --plain-http to --insecure-registry update: added --insecure-registry flag with alias --plain-http May 19, 2023
updated specs
d7d1b6d 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
shizhMSFT
shizhMSFT reviewed May 19, 2023
View reviewed changes
Copy link
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.

@yizha1 @Two-Hearts Should we just drop --plain-http in RC versions?

@yizha1
Copy link
Contributor

yizha1 commented May 19, 2023

@yizha1 @Two-Hearts Should we just drop --plain-http in RC versions?

RC-5 is the last RC release. The next is v1 release.

@shizhMSFT
Copy link
Contributor

shizhMSFT commented May 19, 2023 •
edited
Loading

@yizha1 @Two-Hearts Should we just drop --plain-http in RC versions?

RC-5 is the last RC release. The next is v1 release.

Then we should drop the --plain-http flag and it can be added if needed. After 1.0.0, we are not able to remove any non-experimental flag but we are able to add new flags.

sajayantony
sajayantony reviewed May 19, 2023
View reviewed changes
cmd/notation/common.go Outdated
opts.Password = os.Getenv(defaultPasswordEnv)
}

func aliasNormalizeFunc(f *pflag.FlagSet, name string) pflag.NormalizedName {
Copy link
Contributor

@sajayantony sajayantony May 19, 2023

Choose a reason for hiding this comment

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

Recommend removing aliasing to avoid breaking post RC to GA. Also this should be used only for test either way.

Copy link
Author

@ghost ghost May 22, 2023

Choose a reason for hiding this comment

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

Recommend removing aliasing to avoid breaking post RC to GA. Also this should be used only for test either way.

Got it. Updated the code to remove aliasing.

Patrick Zheng added 4 commits May 22, 2023 10:47
Merge branch 'notaryproject:main' into chore
a02ac0d
updated per code review
14dd144 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
updated per code review
f0bab7c 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
update
13f7038 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@ghost ghost changed the title update: added --insecure-registry flag with alias --plain-http update: renamed flag --plain-http to --insecure-registry May 22, 2023
@ghost ghost requested review from sajayantony and shizhMSFT May 22, 2023 03:23
@ghost
Copy link
Author

ghost commented May 22, 2023

@yizha1 @Two-Hearts Should we just drop --plain-http in RC versions?

RC-5 is the last RC release. The next is v1 release.

Then we should drop the --plain-http flag and it can be added if needed. After 1.0.0, we are not able to remove any non-experimental flag but we are able to add new flags.

Thanks @shizhMSFT, good point! I updated the code to reflect it.

@yizha1
Copy link
Contributor

yizha1 commented May 22, 2023

Agree to drop --plain-http flag in upcoming RC release

Patrick Zheng added 2 commits May 22, 2023 14:37
updated notation-go dependency
bc5e9db 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
update
2b50e15 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
shizhMSFT

This comment was marked as duplicate.

Sign in to view

shizhMSFT
shizhMSFT approved these changes May 22, 2023
View reviewed changes
Copy link
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

priteshbandi
priteshbandi approved these changes May 22, 2023
View reviewed changes
Copy link
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 13bac0e into notaryproject:main May 22, 2023
@ghost ghost deleted the chore branch May 22, 2023 23:50
@priteshbandi priteshbandi mentioned this pull request May 26, 2023
Closed
6 tasks
7h3-3mp7y-m4n pushed a commit to 7h3-3mp7y-m4n/notation that referenced this pull request Mar 29, 2025
@7h3-3mp7y-m4n
update: renamed flag --plain-http to --insecure-registry (notaryproje…
3a8e127 
…ct#674)

This PR is based on discussions in issue notaryproject#623 with following changes:
1. Renamed flag `--plain-http` to `--insecure-registry` for notation `sign`, `verify`, `list`, `inspect` and `login`. <del>And added the original flag name `--plain-http` as an alias of the new name.</del>
2. Updated the flag's description to `use HTTP protocol while connecting to registries. Should be used only for testing`. Related specs are updated as well.

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: renamed flag --plain-http to --insecure-registry (notaryproje…
a552615 
…ct#674)

This PR is based on discussions in issue notaryproject#623 with following changes:
1. Renamed flag `--plain-http` to `--insecure-registry` for notation `sign`, `verify`, `list`, `inspect` and `login`. <del>And added the original flag name `--plain-http` as an alias of the new name.</del>
2. Updated the flag's description to `use HTTP protocol while connecting to registries. Should be used only for testing`. Related specs are updated as well.

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

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

@JeyJeyGao JeyJeyGao Awaiting requested review from JeyJeyGao

@justincormack justincormack Awaiting requested review from justincormack

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

@patrickzheng200 patrickzheng200 Awaiting requested review from patrickzheng200

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

@SteveLasker SteveLasker Awaiting requested review from SteveLasker

@sajayantony sajayantony Awaiting requested review from sajayantony

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Flag --plain-http didn't explicitly remind users the insecure connection to registries

6 participants

@codecov-commenter @yizha1 @shizhMSFT @sajayantony @priteshbandi @FeynmanZhou