Skip to content
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

Closed

Conversation

odinnordico
Copy link
Contributor

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
image

image

Monterey
image

image

Additional information or special notes for your reviewer

@odinnordico odinnordico marked this pull request as ready for review May 31, 2022 20:29
@odinnordico odinnordico changed the title feat: upgrade carvel-imgpkg chore: upgrade carvel-imgpkg May 31, 2022
// 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"}
Copy link
Contributor

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

Copy link
Contributor

@scothis scothis left a 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?

pkg/commands/workload_test.go Outdated Show resolved Hide resolved
Co-authored-by: Wendy Maria Arango Chavarria <warango@vmware.com>
Signed-off-by: Diego Alfonso <dalfonso@vmware.com>
Copy link
Contributor

@scothis scothis left a 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 👍

pkg/source/imgpkg.go Outdated Show resolved Hide resolved
pkg/source/imgpkg.go Outdated Show resolved Hide resolved
pkg/source/imgpkg.go Outdated Show resolved Hide resolved
envVars := os.Environ()
_, present := os.LookupEnv("IMGPKG_ENABLE_IAAS_AUTH")
if !present {
envVars = append(envVars, "IMGPKG_ENABLE_IAAS_AUTH=false")
Copy link
Contributor

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.

Copy link
Contributor

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.

pkg/commands/workload_test.go Show resolved Hide resolved
pkg/commands/workload_test.go Outdated Show resolved Hide resolved
Co-authored-by: Wendy Maria Arango Chavarria <warango@vmware.com>
Signed-off-by: Diego Alfonso <dalfonso@vmware.com>
@odinnordico
Copy link
Contributor Author

New PR will be open with different approach based in carvel-imgpkg issue 397

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

workload create on Windows throws "x509: certificate signed by unknown authority"
5 participants