Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Add ability to manual specify a server-cert #1046

Merged
merged 8 commits into from
Jul 26, 2021

Conversation

barrymars
Copy link
Contributor

@barrymars barrymars commented Jul 20, 2021

Changes proposed in this PR:

  • Adds the ability to manually specify a secret containing a pre-generated server-cert.

-This coupled with the autoEncrypt feature, removes the need for the tls-init job and so can be run in a gitops way without having to rely on helm hooks etc.

How I've tested this PR:
Used to deploy consul into both local 'kind' cluster and AWS EKS cluster.

Checklist:

  • Bats tests added
  • CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

@hashicorp-cla
Copy link

hashicorp-cla commented Jul 20, 2021

CLA assistant check
All committers have signed the CLA.

@barrymars barrymars marked this pull request as draft July 20, 2021 12:44
@barrymars barrymars changed the title WIP: Add ability to manual specify a server-cert Add ability to manual specify a server-cert Jul 20, 2021
@david-yu
Copy link
Contributor

Hi @barrymars out of curiosity are you potentially using Vault for minting your server certs? How do you envision rotating them? We are looking into further integrations with Vault to support retrieving server certs from Vault for TLS.

@barrymars
Copy link
Contributor Author

we're actually moving away from vault in favour of AWS secrets manager.

hadn't got as far as working out certificate rotation, still at the proof of concept stage

@barrymars barrymars marked this pull request as ready for review July 21, 2021 10:54
Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

Looking good, thanks for making this change!

With server cert provided, I think we also need to disable tls-init and tls-init-cleanup jobs.

@barrymars
Copy link
Contributor Author

good call @ishustava

I am using the helm chart via Tanka (which doesn't support hooks), so hadn't thought of that

I've also added a sanity check so that if setting serverCert you have to have caCert set too

templates/server-statefulset.yaml Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
templates/tls-init-cleanup-job.yaml Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
barrymars and others added 4 commits July 23, 2021 07:49
Co-authored-by: Iryna Shustava <ishustava@users.noreply.github.com>
Co-authored-by: Iryna Shustava <ishustava@users.noreply.github.com>
Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks so much for this contribution!

Copy link
Contributor

@ndhanushkodi ndhanushkodi left a comment

Choose a reason for hiding this comment

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

Looks good to me and @sadjamz!

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

Successfully merging this pull request may close these issues.

None yet

5 participants