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

Fix SIGSEGV when storage driver returns nil error (#3625) #4627

Merged
merged 1 commit into from Sep 27, 2018

Conversation

nonspecialist
Copy link

@nonspecialist nonspecialist commented Sep 12, 2018

This addresses #3625 where tiller segfaults in uniqName -- the check for an existing name assumes that the storage driver's Get function always returns a valid error type, but for example the cfgmaps driver (default) often returns a nil

@nonspecialist
Copy link
Author

A note on testing ... this is a hard one to test for; the only existing test that might trigger this is

{"", "[a-z]+-[a-z]+", false, false},
and that would only trigger the issue if moniker chose the name happy-panda; with the current version of moniker having 243 animals and 390 descriptors, that's a 1 in 94,770 chance (0.001%) of triggering the bug during testing.

I could update this PR with a bigger rewrite that allows the use of something that satisfies the moniker interface in order to increase the test coverage, if that's considered worthwhile.

@nonspecialist
Copy link
Author

PR updated to add a test which, based on the original code causes a segfault and confirms that the fix is valid

@bacongobbler
Copy link
Member

hey! looks like the commit history is mangled. Mind taking a look into fixing that?

@nonspecialist
Copy link
Author

Yeah, whoops ... sorry. Fixing that now

helm#3625)

Signed-off-by: Colin Panisset <colin.panisset@cevo.com.au>
Copy link
Member

@bacongobbler bacongobbler 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!

@bacongobbler bacongobbler merged commit 12ace31 into helm:master Sep 27, 2018
@bacongobbler bacongobbler added this to the 2.11.1 - Bugfix milestone Sep 27, 2018
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
helm#3625) (helm#4627)

Signed-off-by: Colin Panisset <colin.panisset@cevo.com.au>
Signed-off-by: Sebastien Plisson <sebastien.plisson@gmail.com>
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
helm#3625) (helm#4627)

Signed-off-by: Colin Panisset <colin.panisset@cevo.com.au>
Signed-off-by: Sebastien Plisson <sebastien.plisson@gmail.com>
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
helm#3625) (helm#4627)

Signed-off-by: Colin Panisset <colin.panisset@cevo.com.au>
splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018
helm#3625) (helm#4627)

Signed-off-by: Colin Panisset <colin.panisset@cevo.com.au>
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.

None yet

2 participants