I'd like to be able to make calls like NewUpgrade(cfg) rather than
Upgrade{...}.Prepare, but I wouldn't be able to define a NewUpgrade
function while Config is in the helm package; there would be a circular
import when Plan tried to import run.
While writing docs in the previous commit, I noticed that we'd been
inconsistent in the naming scheme. Wikipedia's back-compat article
redirects from "backwards" to "backward", so I figure that's a
reasonable source of authority for which form to use.
The goal with these changes was to give users a clearer, more readable
interface, so we should present that interface up front and only
document the aliases as a backward-compatibility option.
I've renamed the envconfig tags to reflect the switch, but I left the
actual field names the same. I think they're sufficiently meaningful
inside the code, and leaving them unchanged avoids making a bunch of
churn in the rest of the code.
This includes a refactor to the way aliases are processed. I had been
thinking in terms of locking down the aliases names pretty tightly, in
order to provide an error if there are conflicts. After discussion with
@josmo, though, it seems like we can do it the same way we do for
"PLUGIN_"/non-prefixed variables, i.e. quietly override them.
I don't love the mismatch between the helm.Config field (CleanupOnFail)
and the setting name (cleanup_failed_upgrade). I do think the setting
name should contain "upgrade" since it's specific to the upgrade command,
but if I make the config field CleanupFailedUpgrade, it becomes the new
longest field name, and gofmt creates a bunch of churn. Is that a good
enough reason...?
Helm2's --timeout took a number of seconds, rather than the
ParseDuration-compatible string that helm3 uses. For backward-
compatibility, update a bare number into a duration string.
These comments were a reasonable attempt at ensuring the documentation
matched reality, but the checkbox in the pull request template is much
more likely to produce results.
It's a little tricky to find a balance between "brittle" and "thorough"
in this test--I'd like to verify that e.g. the certificate is in
clusters[0].cluster.certificate-authority-data, not at the root. On the
other hand, we can't actually show that it's a valid kubeconfig file
without actually *using* it, so there's a hard upper limit on the
strength of the assertions. I've settled on verifying that all the
settings make it into the file and the file is syntactically-valid yaml.
Redacting KubeToken may not be sufficient, since it's possible that
someone would put secrets in Values or StringValues. Unilaterally
redacting those seems unhelpful, though, since they may be the very
thing the user is trying to debug. I've settled on redacting the obvious
field without trying to promise that all sensitive data will be hidden.
