Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

helm-op: Add support for connecting to tiller using tls #1200

Merged
merged 3 commits into from
Jul 5, 2018

Conversation

thojkooi
Copy link
Contributor

@thojkooi thojkooi commented Jul 4, 2018

  • Add support for connecting to tiller using TLS authentication.
    It is based on the way the helm client itself configures the TLS authentication and behaves the same way. The default is TLS disabled.
  • Updates the helm chart to support the TLS options. Expects the helm client certificates to be stored in a k8s secret in the same namespace as the helm-operator is deployed. The secretName can be passed along using the helm-deployment. When configuring TLS Verify, a CA certificate has to be passed and this will be created as a ConfigMap during the deployment.

Closes #1198

- Add support for connecting to tiller using TLS authentication.
   It is based on the way the helm client itself configures the TLS authentication. The default is tls disabled.
- Updates the helm chart to support the TLS options. Expects the helm client certificates to be stored in a k8s secret in the same namespace as the helm-operator is deployed. The secretName can be passed along using the helm-deployment. When configuring TLS Verify, a CA certificate has to be passed and this will be created as a ConfigMap during the deployment.

Closes fluxcd#1198
@stefanprodan stefanprodan changed the title Add support for connecting to tiller using tls helm-op: Add support for connecting to tiller using tls Jul 4, 2018
@stefanprodan
Copy link
Member

Thanks @thojkooi can you please provide an Docker Hub or Quay image for helm-op with your changes so I can test it.

@thojkooi
Copy link
Contributor Author

thojkooi commented Jul 4, 2018

@stefanprodan , sure. I just pushed one at thojkooi/helm-operator:tiller-tls

https://hub.docker.com/r/thojkooi/helm-operator/tags/

@stefanprodan
Copy link
Member

I can confirm that this change doesn't break setups without TLS and works ok with Tiller TLS.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

- --tiller-namespace={{ .Values.helmOperator.tillerNamespace }}
{{- if .Values.helmOperator.tls.enable }}
- --tiller-tls-enable={{ .Values.helmOperator.tls.enable }}
- --tiller-tls-key-path={{ .Values.helmOperator.tls.keyPath }}

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

…d of the full path

- Includes a fix for the CA Certificate ConfigMap. The ca.crt file content was not properly templated.
- Instead of the full path, a user only has the option to configure the key and certificate file name, as the path is hardcoded within the template.
- Add new helmOperator properties to the chart's README file
@thojkooi
Copy link
Contributor Author

thojkooi commented Jul 4, 2018

I've made some changes to the templating of the TLS cert and key paths with the suggestions from @squaremo. I've also fixed an issue I encountered with passing the CA Certificate.

I've written up some steps on how to configure flux and tiller with TLS here. Is this something you would like added to the documentation from Flux? I'm unsure what a good location for this would be.

@stefanprodan
Copy link
Member

@thojkooi please add the configuration to the Helm chart readme. Make a new section at the bottom and use code blocks for the yaml and script file, @dholbach will move it afterwords in the official docs.

Thank you very much for this.

…th TLS

Expans the flux chart README file with a section on how to configure the Helm Operator and install Tiller with TLS authentication enabled.
@thojkooi
Copy link
Contributor Author

thojkooi commented Jul 4, 2018

@stefanprodan done.

@squaremo
Copy link
Member

squaremo commented Jul 4, 2018

@thojkooi Thank you very much!

@stefanprodan I'll leave it to you to push the big green button

@stefanprodan stefanprodan merged commit 783f23e into fluxcd:master Jul 5, 2018
@thojkooi thojkooi deleted the tiller-tls branch July 5, 2018 10:46
@thojkooi
Copy link
Contributor Author

thojkooi commented Jul 5, 2018

Thanks for merging! 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants