feat: cert-manager integration #1261

10ko · 2019-10-10T21:39:28Z

What this PR does / why we need it:
Implement the cert-manager integration. This will allow installation and configuration of Tls certificates through Garden.

Special notes for your reviewer:
NOTE: please merge after #1317
For this feature to work properly, it needs better "readiness" check for Pods. I implemented and reverted a basic check in favour of @edvald implementation in the "artifact PR".

edvald · 2019-10-11T12:47:56Z

garden-service/src/plugins/kubernetes/config.ts

@@ -342,6 +345,14 @@ export const kubernetesConfigBase = providerConfigBaseSchema
    tlsCertificates: joiArray(tlsCertificateSchema)
      .unique("name")
      .description("One or more certificates to use for ingress."),
+    tlsManager: joi.string()
+      .optional()
+      .example("cert-manager")


Pro-tip: You can use .allow(...values) to specify which specific values are allowed

edvald · 2019-10-11T12:49:50Z

garden-service/src/plugins/kubernetes/init.ts

+  await helm({ ctx, namespace: systemNamespace, log, args: [...installArgs] })
+  console.log("Success?")
+  return ""
+}


Why not just use a helm module and additionally set that label on the system namespace? Is there something else that requires the custom code?

I thought of implementing the installation programmatically because I wanted to create other resources needed (like Issuers, etc) before starting to deploy the services.
I see your point and I might end up doing that since I might be able to delegate the creation of those resources later on but still before deploying.

Still working that part out because we need the certificate in order to deploy an ingress but cert-manager issues a certifcate annotating the ingress itself.
This part will undoubtely change soon.

10ko · 2019-10-31T15:32:07Z

Still working on the guide and on an example, code is here and has been tested against one of our dev clusters.
Will remove the "WIP:" part once the documentation is up.

edvald · 2019-10-31T16:58:30Z

garden-service/src/plugins/kubernetes/config.ts

+        name: joi.string()
+          .required()
+          .allow("cert-manager")
+          .description("The name of the certificate manager of choice. Currently supporting only cert-manager.")


Do we actually anticipate supporting multiple ones? I think I'd suggest calling the config key certManager and having a certManager.enable field. If we do end up supporting others, I expect it'll be after this functionality is split into separate plugins.

That is fair enough. I am still not sure how the plugin architecture would develop but if you think it would be possible to support something like this, that's ok with me. :)
I'll change it to be more specific.

I would though leave the field managedBy: cert-manager on the IngressTlsCertificate or change it to managedBy: "cert-manager" | "manual" with a default to "manual" of course.
I am not a big fan of managedByCertManager: boolean, certManagerCertificate or similar.

Sure, makes sense 👍

edvald · 2019-10-31T17:28:11Z

Just noticed this, probably worth documenting that the minimum supported K8s version is 1.11: https://docs.cert-manager.io/en/latest/getting-started/install/kubernetes.html

docs/reference/providers/kubernetes.md

edvald · 2019-11-06T15:08:45Z

docs/reference/providers/kubernetes.md

+
+[providers](#providers) > [tlsCertificates](#providerstlscertificates) > managedBy
+
+A reference to the Tls certificates manager used to generate the certificate.


edvald · 2019-11-06T15:09:09Z

docs/reference/providers/kubernetes.md

+
+[providers](#providers) > [certManager](#providerscertmanager) > enable
+
+Enable the cert-manager integration.


Would be good to link the guide here.

docs/reference/providers/kubernetes.md

edvald · 2019-11-06T15:11:25Z

garden-service/src/plugins/kubernetes/config.ts

@@ -19,10 +19,21 @@ export interface ProviderSecretRef {
  namespace: string
 }

+export type TlsManager = "cert-manager" | "manual"
+export type LEServerType = "staging" | "prod"


I'd prefer a more verbose name, LetsEncryptServerType

edvald · 2019-11-06T15:19:14Z

garden-service/src/plugins/kubernetes/config.ts

+export interface CertManagerConfig {
+  enable: boolean
+  email?: string
+  installOnly?: boolean


I'd suggest removing installOnly and replacing enable with install. That way people can choose to use their own cert-manager installation but still use the managedBy: cert-manager feature.

If we remove the enable, what would the config file look like in the case I want to use my own installation? Because all other options are not mandatory.

You would still need to specify the email if you want Garden to take care of it.
Nevermind. 🤦‍♂

For semantic validation, e.g. "this is required if that other field is set, but otherwise not", we can add explicit checks and error messages in the configureProvider handler.

Implemented that on the kubernetes.configureProvider

edvald · 2019-11-06T15:36:04Z

garden-service/src/plugins/kubernetes/config.ts

+            The email which will be used for creating Let's Encrypt certificates:
+            if your certificates are being created by Garden this field is required.`)
+          .example("[email protected]"),
+        installOnly: joi.boolean()


I think it might be good, just for clarity, to add a few fields that don't actually do anything for now, but set the config structure and serve to document our coverage a bit better. My suggestion:

Add an acmeChallengeType field with the only option we support (e.g. .allow("HTTP-01").

Add an issuer field with just "acme" supported for now.

Add an acmeServer field with just "letsencrypt-staging" and "letsencrypt-prod" supported for now (as shorthands for the appropriate server URLs, and later we could support custom URLs).

edvald · 2019-11-08T11:14:55Z

docs/guides/cert-manager-integration.md

+
+The goal of this integration is to give you a head start when setting the TLS certificates for your project with cert-manager, providing an easy way for installation and some sensible defaults while allowing full control of the underlying configuration.
+
+The goal of this integrations is NOT to be a wrapper around cert-manager, nor to allow a garden user to fully-customize certificates generation through Garden.


I think this is a little hard to digest. I suppose the point is that we don't aim to fully support all the features of cert-manager, but rather accommodate the most common use case, something to that effect?

Yes, what I wanted to express is that cert-manager is there, we just make your life easier at the beginning but at the same time you won't get a full wrapper around. Your sentece express it better.

edvald · 2019-11-08T11:16:16Z

docs/guides/cert-manager-integration.md

+
+### Requirements
+
+We assume you have been taking care of setting up your domain names and your networking configuration is in place and functioning correctly.


This could use more clarification. Specifically, we require you to have configured your DNS and routing so that the domains you will configure below are pointed to your ingress controller.

edvald · 2019-11-08T11:27:16Z

docs/guides/cert-manager-integration.md

+
+### Challenge Type
+
+The challenge type currently supported is Let's Encrypt [HTTP-01 challenge](https://letsencrypt.org/docs/challenge-types/).


On a second read, the flow in this section feels a little odd. I'd prefer to start the "Issuing your first certificate" section with what you're supposed to do + an example. Then we can elaborate with what's happening behind the scenes + the note on the limitations.

edvald · 2019-11-11T19:25:00Z

I've added a commit with some iteration on the docs. @eysi09 could you perhaps lend another pair of eyes on the docs+reference, and see if it all holds up?

eysi09

Had some comments on the docs. Some of these are my own opinions so feel free to to pipe through your own style and preferences.

eysi09 · 2019-11-12T00:00:08Z

docs/guides/cert-manager-integration.md

+
+### Scope
+
+This guide aims at outlining configuration and best practices when dealing with TLS certificates, cert-manager and Garden.


aims at outlining sounds a little weird. Why not just outlines?

eysi09 · 2019-11-12T00:02:51Z

docs/guides/cert-manager-integration.md

+
+This guide aims at outlining configuration and best practices when dealing with TLS certificates, cert-manager and Garden.
+
+When starting a new project or when maintaining your existing ones, dealing with the creation and renewal of certificates can easily become a very complex task. Many projects appeared in the last few years to help managing this complexity and one that stood out is [cert-manager](https://github.com/jetstack/cert-manager).


Many projects have appeared... (add have)

eysi09 · 2019-11-12T00:03:58Z

docs/guides/cert-manager-integration.md

+
+When starting a new project or when maintaining your existing ones, dealing with the creation and renewal of certificates can easily become a very complex task. Many projects appeared in the last few years to help managing this complexity and one that stood out is [cert-manager](https://github.com/jetstack/cert-manager).
+
+The goal of this integration is to give you a head start when setting the TLS certificates for your project with cert-manager, providing an easy way for installation and some sensible defaults while allowing full control of the underlying configuration.


The motivation behind this integration...

eysi09 · 2019-11-12T00:06:03Z

docs/guides/cert-manager-integration.md

+
+When starting a new project or when maintaining your existing ones, dealing with the creation and renewal of certificates can easily become a very complex task. Many projects appeared in the last few years to help managing this complexity and one that stood out is [cert-manager](https://github.com/jetstack/cert-manager).
+
+The goal of this integration is to give you a head start when setting the TLS certificates for your project with cert-manager, providing an easy way for installation and some sensible defaults while allowing full control of the underlying configuration.


full control over the underlying (over instead of of)

eysi09 · 2019-11-12T00:08:03Z

docs/guides/cert-manager-integration.md

+The goal of this integration is to give you a head start when setting the TLS certificates for your project with cert-manager, providing an easy way for installation and some sensible defaults while allowing full control of the underlying configuration.
+We don't aim to fully support all the features of cert-manager, but rather accommodate the most common use case while still allowing full control of the underlying setup.
+
+Please read the defaults settings and configurations in each of the following sections.


Please read about the default settings (s/read the/read about the/, s/defaults/default).

Or maybe just:

You can read about the default settings and configurations...

eysi09 · 2019-11-12T00:24:31Z

docs/guides/cert-manager-integration.md

+1) cert-manager will create a ClusterIssuer in your cluster which will generate your certificate.
+2) It will then create a Certificate resource to request the TLS certificate.
+3) Cert-manager will then automatically spin up an nginx ingress to solve the HTTP-01 acmeChallenge.
+4) Once the challenge is solved the TLS certificate will be stored as a secret using the name/namespace specified above (eg. `cert-manager-example/tls-secret-for-certificate`)


s/solved/solved,/

eysi09 · 2019-11-12T00:24:50Z

docs/guides/cert-manager-integration.md

+
+### ClusterIssuer vs Issuer
+
+cert-manager have two different Certificate issuers: namespaced and cluster one. Garden will only create ClusterIssuers.


s/have/has/

eysi09 · 2019-11-12T00:25:41Z

docs/guides/cert-manager-integration.md

+
+### The certificate creation timeouts and garden terminates
+
+> Please make sure your domain name is pointing at the right ip address.


eysi09 · 2019-11-12T00:26:14Z

docs/guides/cert-manager-integration.md

+Please find more info in the ["Issuing an ACME certificate using HTTP validation"](https://docs.cert-manager.io/en/release-0.11/tutorials/acme/http-validation.html#issuing-an-acme-certificate-using-http-validation) guide in the official cert-manager documentation.
+
+---
+If have any issue, found a bug or something is not clear in the documentation, please don't hesitate opening a new [Github issue](https://github.com/garden-io/garden/issues/new?template=BUG_REPORT.md) or ask us any question in our [Slack channel](https://chat.garden.io/).


s/opening/to open/

eysi09 · 2019-11-12T00:26:45Z

docs/reference/providers/kubernetes.md

+
+[providers](#providers) > [certManager](#providerscertmanager) > issuer
+
+the type of issuer for the certificate. Currently only supporting ACME Let's Encrypt issuers.


10ko changed the title ~~WIP: feat: implement installation of cert-manager~~ WIP: feat: cert-manager integration Oct 10, 2019

edvald reviewed Oct 11, 2019

View reviewed changes

10ko force-pushed the cert-manager branch 7 times, most recently from 83f7d5b to 2ec44f7 Compare October 31, 2019 13:55

10ko marked this pull request as ready for review October 31, 2019 15:30

edvald reviewed Oct 31, 2019

View reviewed changes

eysi09 assigned edvald Nov 6, 2019

10ko force-pushed the cert-manager branch from 998acf2 to cd78f66 Compare November 6, 2019 14:51

edvald reviewed Nov 6, 2019

View reviewed changes

10ko force-pushed the cert-manager branch 4 times, most recently from 079a2b5 to eabf8eb Compare November 7, 2019 14:54

10ko changed the title ~~WIP: feat: cert-manager integration~~ feat: cert-manager integration Nov 7, 2019

edvald reviewed Nov 8, 2019

View reviewed changes

10ko force-pushed the cert-manager branch 2 times, most recently from 32dec73 to 788c752 Compare November 8, 2019 16:59

edvald force-pushed the cert-manager branch 2 times, most recently from bcbf756 to d64a761 Compare November 11, 2019 19:36

eysi09 requested changes Nov 12, 2019

View reviewed changes

feat: implement cert-manager integration

fd0563e

edvald force-pushed the cert-manager branch from d64a761 to 61e03f3 Compare November 12, 2019 01:04

eysi09 approved these changes Nov 12, 2019

View reviewed changes

docs: overhaul documentation (TBS)

fe3e289

edvald force-pushed the cert-manager branch from 61e03f3 to fe3e289 Compare November 12, 2019 01:21

edvald merged commit 21f2775 into master Nov 12, 2019

edvald deleted the cert-manager branch November 12, 2019 01:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: cert-manager integration #1261

feat: cert-manager integration #1261

10ko commented Oct 10, 2019 •

edited

Loading

edvald Oct 11, 2019

edvald Oct 11, 2019

10ko Oct 11, 2019 •

edited

Loading

10ko commented Oct 31, 2019

edvald Oct 31, 2019

10ko Nov 1, 2019

10ko Nov 1, 2019 •

edited

Loading

edvald Nov 1, 2019

edvald commented Oct 31, 2019

edvald Nov 6, 2019

edvald Nov 6, 2019

edvald Nov 6, 2019

edvald Nov 6, 2019

10ko Nov 7, 2019 •

edited

Loading

10ko Nov 7, 2019

edvald Nov 7, 2019

10ko Nov 7, 2019

edvald Nov 6, 2019

edvald Nov 8, 2019

10ko Nov 8, 2019

edvald Nov 8, 2019

edvald Nov 8, 2019

edvald commented Nov 11, 2019

eysi09 left a comment

eysi09 Nov 12, 2019

eysi09 Nov 12, 2019

eysi09 Nov 12, 2019

eysi09 Nov 12, 2019

eysi09 Nov 12, 2019

eysi09 Nov 12, 2019

eysi09 Nov 12, 2019

eysi09 Nov 12, 2019

eysi09 Nov 12, 2019

eysi09 Nov 12, 2019


		[providers](#providers) > [tlsCertificates](#providerstlscertificates) > managedBy

		A reference to the Tls certificates manager used to generate the certificate.


		[providers](#providers) > [certManager](#providerscertmanager) > enable

		Enable the cert-manager integration.


		The goal of this integration is to give you a head start when setting the TLS certificates for your project with cert-manager, providing an easy way for installation and some sensible defaults while allowing full control of the underlying configuration.

		The goal of this integrations is NOT to be a wrapper around cert-manager, nor to allow a garden user to fully-customize certificates generation through Garden.


		### Requirements

		We assume you have been taking care of setting up your domain names and your networking configuration is in place and functioning correctly.


		### Challenge Type

		The challenge type currently supported is Let's Encrypt [HTTP-01 challenge](https://letsencrypt.org/docs/challenge-types/).


		### Scope

		This guide aims at outlining configuration and best practices when dealing with TLS certificates, cert-manager and Garden.


		This guide aims at outlining configuration and best practices when dealing with TLS certificates, cert-manager and Garden.

		When starting a new project or when maintaining your existing ones, dealing with the creation and renewal of certificates can easily become a very complex task. Many projects appeared in the last few years to help managing this complexity and one that stood out is [cert-manager](https://github.com/jetstack/cert-manager).


		### ClusterIssuer vs Issuer

		cert-manager have two different Certificate issuers: namespaced and cluster one. Garden will only create ClusterIssuers.


		### The certificate creation timeouts and garden terminates

		> Please make sure your domain name is pointing at the right ip address.


		[providers](#providers) > [certManager](#providerscertmanager) > issuer

		the type of issuer for the certificate. Currently only supporting ACME Let's Encrypt issuers.

feat: cert-manager integration #1261

feat: cert-manager integration #1261

Conversation

10ko commented Oct 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

10ko Oct 11, 2019 • edited Loading

Choose a reason for hiding this comment

10ko commented Oct 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

10ko Nov 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edvald commented Oct 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

10ko Nov 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edvald commented Nov 11, 2019

eysi09 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

10ko commented Oct 10, 2019 •

edited

Loading

10ko Oct 11, 2019 •

edited

Loading

10ko Nov 1, 2019 •

edited

Loading

10ko Nov 7, 2019 •

edited

Loading