Skip to content

Fixing issue with idempotent repo add#8777

Merged
mattfarina merged 1 commit into
helm:masterfrom
mattfarina:fix-8771
Sep 21, 2020
Merged

Fixing issue with idempotent repo add#8777
mattfarina merged 1 commit into
helm:masterfrom
mattfarina:fix-8771

Conversation

@mattfarina

Copy link
Copy Markdown
Collaborator

A security issue fixed in 3.3.2 caught repos with the same name
being added a second time and produced an error. This caused an
issue for tools, such as helmfile, that will add the same name
with the same configuration multiple times.

This fix checks that the configuration on the existing and new
repo are the same. If there is no change it notes it and exists
with a 0 exit code. If there is a change the existing error is
returned (for reverse compat). If --force-update is given the
user opts in to changing the config for the name.

Closes #8771

  • this PR contains documentation
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

A security issue fixed in 3.3.2 caught repos with the same name
being added a second time and produced an error. This caused an
issue for tools, such as helmfile, that will add the same name
with the same configuration multiple times.

This fix checks that the configuration on the existing and new
repo are the same. If there is no change it notes it and exists
with a 0 exit code. If there is a change the existing error is
returned (for reverse compat). If --force-update is given the
user opts in to changing the config for the name.

Closes helm#8771

Signed-off-by: Matt Farina <[email protected]>
@helm-bot helm-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 21, 2020

@bacongobbler bacongobbler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thank you for the extensive test cases and considering --force-update in the design. This seems much more extensive than #8778 (and it prevents a double lookup) so I closed it in favour of this one. LGTM!

@mattfarina mattfarina added this to the 3.3.4 milestone Sep 21, 2020
@mattfarina mattfarina merged commit 367277a into helm:master Sep 21, 2020
@mattfarina mattfarina deleted the fix-8771 branch September 21, 2020 15:38
@bacongobbler bacongobbler added needs-pick Indicates that a PR needs to be cherry-picked into the next release candidate. picked Indicates that a PR has been cherry-picked into the next release candidate. labels Sep 22, 2020
mumoshu pushed a commit to roboll/helmfile that referenced this pull request Oct 12, 2020
* Parse and process helm version using github.com/Masterminds/semver/v3.

* Add --force-update only when Helm version >= 3.3.2, < 3.3.4.

See: helm/helm#8777.

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

Labels

needs-pick Indicates that a PR needs to be cherry-picked into the next release candidate. picked Indicates that a PR has been cherry-picked into the next release candidate. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

helm repo add now fails in v3.3.2 and v3.3.3

3 participants