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

Unify registry+repo behavior between plugins #31

Merged
merged 1 commit into from Jan 19, 2022
Merged

Conversation

kylelemons
Copy link
Contributor

@kylelemons kylelemons commented Oct 18, 2021

I noticed this when trying to do a bulk change of our repositories to use the upstreaam plugins/kaniko{,-ecr,-gcr} -- they don't behave the same when you pass

settings:
  registry: example.com
  repo: bar

Specifically:

  • kaniko will try index.docker.io/bar
  • kaniko-ecr and -gcr will try example.com/bar (which is the behavior I would expect)

So, with the current state, our non-ECR / non-GCR images have to repeat the registry in the settings block, and they are inconsistent with the internal one.

This PR allows use of either the "compatible" setting block like

settings:
  registry: example.com
  repo: bar

or the previously required

settings:
  registry: example.com
  repo: example.com/bar

with the same results of trying to push to example.com/bar

This change attempts to retain backward compatibility to avoid breaking anyone.

@kylelemons
Copy link
Contributor Author

Gentle ping -- I think this would simplify our configurations a lot and make the behavior consistent between plugins

@kylelemons
Copy link
Contributor Author

Another nudge; what do you think about this change?

@shubham149
Copy link
Contributor

shubham149 commented Jan 17, 2022

@kylelemons I didn't get a notification for it. Apologies for the delay.

@kylelemons
Copy link
Contributor Author

Awesome! No worries, I'm sure you're busy and we were getting toward the holiday season. Do you have an idea what the timeline for merge and release will be?

@shubham149 shubham149 merged commit 39f3398 into drone:main Jan 19, 2022
@shubham149
Copy link
Contributor

I have merged the change.

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