-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: upgrade carvel-imgpkg #147
Conversation
56dee5b
to
263a6aa
Compare
pkg/source/imgpkg.go
Outdated
// users can set this env var to true to let imgpkg find creds based on the IaaS provider | ||
// for CLI we are setting the default to false | ||
|
||
envVars := []string{"IMGPKG_ENABLE_IAAS_AUTH=false"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs doc update because this is introducing new feature, if its just deps update then remove this part
263a6aa
to
b651b06
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work with authenticated registries?
Co-authored-by: Wendy Maria Arango Chavarria <warango@vmware.com> Signed-off-by: Diego Alfonso <dalfonso@vmware.com>
b651b06
to
9114684
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even though this is currently a draft, 👎 this PR since there's already a 👍
envVars := os.Environ() | ||
_, present := os.LookupEnv("IMGPKG_ENABLE_IAAS_AUTH") | ||
if !present { | ||
envVars = append(envVars, "IMGPKG_ENABLE_IAAS_AUTH=false") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question Are you sure you want to change the imgpkg defaults? Many people expect the credential helpers to work by default. Minimally, this change should be called out on the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@odinnordico : I removed this env var section and built it locally. CLI commands for local path flow works. I am able to verify the images on the registry.
Co-authored-by: Wendy Maria Arango Chavarria <warango@vmware.com> Signed-off-by: Diego Alfonso <dalfonso@vmware.com>
2ade50b
to
8ccdcba
Compare
New PR will be open with different approach based in carvel-imgpkg issue 397 |
Pull request
What this PR does / why we need it
Upgrade imgpkg to latest version (v0.29.0)
Which issue(s) this PR fixes
Fixes #127
Describe testing done for PR
testing in local sandbox with MacOS Monterey and Windows 11.
Windows 11
Monterey
Additional information or special notes for your reviewer