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

Add --push-ignore-immutable-tag-errors boolean CLI option #2774

Merged

Conversation

matheuscscp
Copy link
Contributor

Description

This change adds the boolean CLI flag --push-ignore-immutable-tag-errors for ignoring known tag immutability errors after a push. When the flag is true, an error known to be due to a tag immutability feature, e.g. Google Artifact Registry's tag immutability, a push would simply ignore such error and return with success.

In my use case we have a Google Artifact Registry with immutable tags enabled, but our CD pipelines may try to build and push the same tag in parallel. Since we ensure on our side that the produced images would necessarily be identical, we would like to be able to ignore such errors and consider the push successful so our pipelines can just move on.

Submitter Checklist

  • Tests: I'd definitely add them, but it looks like a significant refactor would be required for mocking the error returned by this call.

Reviewer Notes

  • The code flow looks good.
  • Unit tests and or integration tests added.

Release Notes

This change adds the boolean CLI flag --push-ignore-immutable-tag-errors for ignoring known tag immutability errors after a push. When the flag is true, an error known to be due to a tag immutability feature, e.g. Google Artifact Registry's tag immutability, a push would simply ignore such error and return with success.

@aaron-prindle
Copy link
Collaborator

Thanks for the PR here @matheuscscp. If you can rebase your changes here and fix the lint error (snippet below) we should be able to merge this. To fix the below you need to the added PushIgnoreImmutableTagErrors bool field to be with the other bool fields:

error snippet

Error: pkg/config/options.go:34:22: struct of size 112 bytes could be of size 104 bytes:

struct{
RegistryMirrors github.com/GoogleContainerTools/kaniko/pkg/config.multiArg,
InsecureRegistries github.com/GoogleContainerTools/kaniko/pkg/config.multiArg,
SkipTLSVerifyRegistries github.com/GoogleContainerTools/kaniko/pkg/config.multiArg,
RegistriesCertificates github.com/GoogleContainerTools/kaniko/pkg/config.keyValueArg,
RegistriesClientCertificates github.com/GoogleContainerTools/kaniko/pkg/config.keyValueArg,
PushRetry int,
SkipDefaultRegistryFallback bool,
Insecure bool,
SkipTLSVerify bool,
InsecurePull bool,
SkipTLSVerifyPull bool,
PushIgnoreImmutableTagErrors bool,
}

type RegistryOptions struct {
                     ^
make: *** [Makefile:67: test] Error 1

@matheuscscp
Copy link
Contributor Author

Thanks for the PR here @matheuscscp. If you can rebase your changes here and fix the lint error (snippet below) we should be able to merge this. To fix the below you need to the added PushIgnoreImmutableTagErrors bool field to be with the other bool fields:

error snippet

Error: pkg/config/options.go:34:22: struct of size 112 bytes could be of size 104 bytes:

struct{ RegistryMirrors github.com/GoogleContainerTools/kaniko/pkg/config.multiArg, InsecureRegistries github.com/GoogleContainerTools/kaniko/pkg/config.multiArg, SkipTLSVerifyRegistries github.com/GoogleContainerTools/kaniko/pkg/config.multiArg, RegistriesCertificates github.com/GoogleContainerTools/kaniko/pkg/config.keyValueArg, RegistriesClientCertificates github.com/GoogleContainerTools/kaniko/pkg/config.keyValueArg, PushRetry int, SkipDefaultRegistryFallback bool, Insecure bool, SkipTLSVerify bool, InsecurePull bool, SkipTLSVerifyPull bool, PushIgnoreImmutableTagErrors bool, }

type RegistryOptions struct {
                     ^
make: *** [Makefile:67: test] Error 1

Thanks @aaron-prindle! I just rebased and fixed the lint error 👌

@aaron-prindle
Copy link
Collaborator

@matheuscscp thanks for the PR here, can you also add this flag to the README.md section which lists kaniko's CLI flag options:
https://github.com/GoogleContainerTools/kaniko?tab=readme-ov-file#additional-flags

Once that is done we should be able to merge

@matheuscscp
Copy link
Contributor Author

@matheuscscp thanks for the PR here, can you also add this flag to the README.md section which lists kaniko's CLI flag options: https://github.com/GoogleContainerTools/kaniko?tab=readme-ov-file#additional-flags

Once that is done we should be able to merge

@aaron-prindle Done! 👌

Copy link
Collaborator

@aaron-prindle aaron-prindle left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR here @matheuscscp

@aaron-prindle aaron-prindle merged commit 8bbd69d into GoogleContainerTools:main Feb 29, 2024
10 checks passed
@matheuscscp matheuscscp deleted the push-immutable-tag branch February 29, 2024 18:13
@matheuscscp
Copy link
Contributor Author

Thanks for merging this, @aaron-prindle! Can you please cut a release? Thanks 🙏 🙏 🙏

@aaron-prindle
Copy link
Collaborator

@matheuscscp Kaniko v1.21.0 is released now with this change included. Thanks for the PR!

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.

2 participants