Tiller upgrades

[munnerz]

Hey, nice project! Glad to see some work around tiller & RBAC, and making the interactions here more transparent 😄

I've not had a chance to use it yet, but one thing that occurs to me is how this might behave when running helm init --upgrade (after having secured tiller with this plugin).

From looking at the source for the helm CLI, it appears to default to whatever service account name has been specified on the CLI (as you can see here: https://github.com/kubernetes/helm/blob/master/cmd/helm/installer/install.go#L64). This would cause the changes made by this plugin to be wiped out, even if a user has not specified a --service-account flag. Best case, this will cause tiller to not work for charts it previously worked with. Worst case, it could present a mechanism for privilege escalation.

I can see two paths here:

1. helm init --upgrade will use the existing serviceAccountName if a --service-account arg is not passed to it, meaning tiller will continue to use the service account set by this plugin after an upgrade.
2. This plugin prints out the name of the service account it has created, and users are then required to specify --service-account when performing upgrades in future.

As a side note, I also see that the helm cli only sets ServiceAccountName during an upgrade and not ServiceAccount (the old, deprecated field). This tool sets both. I'm not too sure what happens if both are specified and are different though to be honest! 😄

[michelleN]

oh these are all really good thoughts. Thanks for sharing.

The helm init --upgrade scenario is not one I had considered but a really great point. After looking at the helm code in the installer for upgrading Tiller, I was thinking the same thing as you. I tested this scenario out (adding a profile with this plugin and then running helm upgrade with no service account flag specified) and it didn't actually wipe out the service account name but this is a good link to consider. It seems like your option 1 is the current case. I'm thinking an e2e test on before each release of this plugin to make sure that is still the case. What do you think?

On that last point, I didn't realize the service account field is deprecated. I'll take that out. Thanks for the tip!