Don't enable TLS if enable_tls is false

[moleskin-smile]

Terraform Helm provider 0.9.0 introduces a regression. In 0.8.0 if there
is no client_key, ca_certificate, or client_certificate defined at
the provider "Helm" level, TLS configuration is effectively disabled.
The 0.9.0 version checks if there are key.pem, cert.pem and ca.pem
present in Helm's home directory and if they are, TLS is enabled. This
behaviour can break some configurations where these certificate files
are present for other cluster(s) but current cluster doesn't use TLS.

This change makes the Helm provider behave similar to Helm CLI. Helm
CLI uses --tls option to explicitely enable TLS. This change makes
enable_tls option to work as advertised (quoting documentation:
enable_tls - (Optional) Enables TLS communications with the Tiller.
Defaults to false).

Fixes #244

[rporres]

I've been playing a bit with this and if you set enable_tls to true but there are no certs it does not complain. It just silently moves on. Can you take a look, @moleskin-smile?

[moleskin-smile]

@rporres: that's right. But this patch is for the case where TLS should be disabled, but certs are present in ~/.helm (for example for different, TLS-enabled cluster). Please see #244 for more detailed explanation.

[rporres]

Hi @moleskin-smile. I see it is a patch to fix #244, but we cannot introduce another issue when resolving it. Unfortunately this cannot be merged in the current state.

[moleskin-smile]

Hi @rporres.

Actually this patch fixes a regression introduced in 0.9.0.

It doesn't introduce another issue for enable_tls == true – look at the code – it changes current behavior only for the enable_tls == false case:

if !d.Get("enable_tls").(bool) {
        return nil
}

When enable_tls is true, everything works as before.

But, to be sure, I've tested it with enable_tls == true and without certificates, using patched and unpatched helm provider 0.9.0 and in both cases terraform plan fails with rpc error: code = Unavailable desc = transport is closing. Could you please share more details so I could reproduce the regression?

[rporres]

I understand your point. Your basically fixing the issue where enable_tls is false whereas I was thinking on the whole enable_tls behaviour that's kind of broken at this moment. I will take a further look to make sure we can merge this as it is and then create another issue to fix the issue when enable_tls is set to true but there are no certs there.

[moleskin-smile]

I agree about general weirdness of TLS-related code in this provider. More work is required to handle it better. This PR is just a simple fix that prevents my organization from adding workarounds to helm providers everywhere where TLS is disabled.

Thanks for looking at this.

[moleskin-smile]

Hey, any update on this? This PR is already approved, there are no conflicts with the base branch and no other solution was developed in the last 4 months.

[rporres]

@moleskin-smile Can you rebase from master to get the latest changes apply into your branch and see if there's anything breaking new tests? I think we're good to merge as there's no good fix that's either quite complicated or breaking.

[moleskin-smile]

@rporres: Sure. Rebased and force-pushed.