Skip to content

feat: Allow setting --insecure registries#311

Merged
jedevc merged 3 commits intostagingfrom
jedevc/insecure-registry
May 8, 2026
Merged

feat: Allow setting --insecure registries#311
jedevc merged 3 commits intostagingfrom
jedevc/insecure-registry

Conversation

@jedevc
Copy link
Copy Markdown
Member

@jedevc jedevc commented May 7, 2026

Replaces #246, by rewriting to use --insecure, allowing it on build, and all the relevant image subcmds (get, delete, inspect).

@jedevc jedevc requested review from Copilot and nderjung May 7, 2026 16:29
@jedevc jedevc changed the title fix(build): Allow returning credentials with no metros feat: Allow setting --insecure registries May 7, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the image registry resolver/accessor plumbing so CLI commands can opt into insecure registry access via a new --insecure flag (for build and images subcommands), and adjusts credential selection to work even when the current profile has no metros configured.

Changes:

  • Extend the images resolver/accessor to accept “insecure registry” options (per-host or all).
  • Add --insecure to build and images subcommands (get/inspect, delete, copy) and plumb the options through.
  • Update golden help output fixtures to reflect new flags and corrected grammar.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/images/resolver.go Adds insecure-host handling in resolver options and changes credential selection behavior when profile.Metros is empty.
internal/images/images.go Adds AccessorOpt functional options and a context-based mechanism to pass insecure options into Accessor.
internal/cmd/images.go Introduces custom get/delete command wrappers and adds --insecure to get/delete/copy to influence image access.
internal/cmd/build.go Adds --insecure to build and passes corresponding accessor options into images.Accessor.
cmd/unikraft/testdata/TestGolden/images/help Updates CLI help golden output (grammar fix + new --insecure flags).
cmd/unikraft/testdata/TestGolden/build/help Updates build help golden output to include --insecure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/images/resolver.go Outdated
Comment on lines +154 to +158
if len(profile.Metros) == 0 {
username := profile.Organization
if username == "" {
// organization may not be set on old or manually created
// profiles - so fall back to decoding the username from the
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

EEEK. Entirely correct. For this kind of scenario, credentials must be provided via something like docker login.

Comment thread internal/cmd/build.go
jedevc added 2 commits May 7, 2026 18:09
Signed-off-by: Justin Chadwell <justin@unikraft.com>
Signed-off-by: Justin Chadwell <justin@unikraft.com>
@jedevc jedevc force-pushed the jedevc/insecure-registry branch from 2e3f08f to cecb5e5 Compare May 7, 2026 17:09
This is a breaking change because now, `--watch 5s` is not valid, instead
we need to use `--watch=5s`. However, we now also allow `-w5s`.

Signed-off-by: Justin Chadwell <justin@unikraft.com>
@jedevc jedevc force-pushed the jedevc/insecure-registry branch from 59a456c to e7ee91e Compare May 8, 2026 10:05
Copy link
Copy Markdown
Member

@craciunoiuc craciunoiuc left a comment

Choose a reason for hiding this comment

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

All good here. Thanks!

Reviewed-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.com>
Approved-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.com>

@jedevc jedevc merged commit 259e544 into staging May 8, 2026
9 checks passed
@jedevc jedevc deleted the jedevc/insecure-registry branch May 8, 2026 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants