Skip to content

Add option to use default auth with Crane image resolver#30

Merged
jberkhahn merged 1 commit into
operator-framework:mainfrom
jberkhahn:crane_auth
Feb 13, 2023
Merged

Add option to use default auth with Crane image resolver#30
jberkhahn merged 1 commit into
operator-framework:mainfrom
jberkhahn:crane_auth

Conversation

@jberkhahn

@jberkhahn jberkhahn commented Feb 9, 2023

Copy link
Copy Markdown
Contributor

Kinda weird that this forces no-auth as the default, but I've left that as the default option, and added the option to turn on Crane's default auth (which uses local auth, i.e. if you've already docker login'd in your terminal, it will use that token or whatever).

Fixes #6027 and #6095 in Operator SDK.

@everettraven

Copy link
Copy Markdown
Contributor

/rerun

@everettraven

Copy link
Copy Markdown
Contributor

/retest

@everettraven everettraven left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One question but other than that looks good to me.

/lgtm

if ok {
opts = append(opts, WithUserPassAuth(username, args["password"]))
} else {
usedefault := args["usedefault"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this default to false if the index doesn't exist?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no, this defaults to empty string

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Feb 10, 2023
@jberkhahn

Copy link
Copy Markdown
Contributor Author

hmm, trying to figure out why this error is happening, i'm able to reproduce it running the tests locally, although it seems like it might be because i'm on an m1 mac and the image isn't compatible?

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

Labels

lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't pin images if docker authentication is required.

2 participants