Update the Auto TLS instruction #2083

ZhiminXiang · 2020-01-04T22:28:37Z

Fixes #issue-number
#1949

Proposed Changes

Add instruction about using the feature of HTTP01 challenge and per-namespace Certificate
Update the doc to use cert-manager 0.12.0 and the corresponding configs

…lenge feature

ZhiminXiang · 2020-01-04T22:29:08Z

/cc @mattmoor

ZhiminXiang · 2020-01-04T22:31:30Z

/hold

This PR is ready for review, but should be held until the related PR (knative/serving#6408) in serving is checked in.

ZhiminXiang · 2020-01-13T08:10:38Z

/hold cancel
The related code is checked into knative serving repository. So this PR is ready for review.

abrennan89

Some initial comments added, this could probably do with some changes to the structure to make it easier to follow, but that's just my 2 cents.
Otherwise the information looks good.

docs/serving/using-auto-tls.md

abrennan89 · 2020-01-13T17:34:14Z

docs/serving/using-auto-tls.md


+## Automatic TLS provision mode
+
+In Knative, we support the following modes of Auto TLS:


"Knative supports the following Auto TLS modes:"

abrennan89 · 2020-01-13T17:35:19Z

docs/serving/using-auto-tls.md

+
+In Knative, we support the following modes of Auto TLS:
+
+1.  Using DNS01 challenge


Why is this DNS01 rather than just DNS?

https://letsencrypt.org/docs/challenge-types/
Actually we should use DNS-01 challenge and HTTP-01 challenge. I updated the doc to use them.

abrennan89 · 2020-01-13T17:41:37Z

docs/serving/using-auto-tls.md

+1.  Using DNS01 challenge
+
+    In this mode, your cluster needs to be able to talk to your DNS server to verify the ownership of your domain.
+    Specifically, when using DNS challenge, we support:


Replace "Specifically....per namespace" with something like "Provision Certificate per namespace is supported when using DNS challenge mode."

"In this mode, a single Certificate will be provisioned per namespace and is reused across the Knative Services within the same namespace."
Remove 'if possible' and instead explain any caveats where this might not be possible.

Remove "If you want to have a fast certificate provision process, this way is recommended." and instead have a similar comment at the beginning of the explanation, e.g.
"Provision Certificate per namespace is supported when using DNS challenge mode. This is the recommended mode for faster certificate provision."

abrennan89 · 2020-01-13T17:42:04Z

docs/serving/using-auto-tls.md

+      - If you want to have a fast certificate provision process, this way is 
+      recommended.
+
+    - **Provision Certificate per Knative Service:**


Restructure similar to comments above

abrennan89 · 2020-01-13T17:49:24Z

docs/serving/using-auto-tls.md

-        [`ClusterIssuer` example](https://docs.cert-manager.io/en/latest/tasks/issuers/setup-acme.html#creating-a-basic-acme-issuer)
-      - Also see the
-        [`DNS-01` example](https://docs.cert-manager.io/en/latest/tasks/acme/configuring-dns01/index.html)
+    Use the cert-manager reference to determine how to configure your


Should these be numbered steps instead?

+1. Let's try to be consistent with numbering all steps.

This part is an expansion of section #### ClusterIssuer for DNS-01 challenge. There are no other steps after it.

abrennan89 · 2020-01-13T17:50:15Z

docs/serving/using-auto-tls.md

-   1. Add your `ClusterIssuer` configuration to your Knative cluster by running
-      the following commands, where `<filename>` is the name of the file that
-      you created:
+1.  ClusterIssuer for HTTP01 challenge


Again, this seems confusing structured this way - make this a heading instead with numbered steps on how to create the ClusterIssuer?

Changed this to use heading as this part is in parallel with ClusterIssuer for DNS-01 challenge

abrennan89 · 2020-01-13T17:50:35Z

docs/serving/using-auto-tls.md

+1.  ClusterIssuer for HTTP01 challenge

-      1. Add the configuration file to Knative:
+    Run the following command to apply the ClusterIssuer for HTT01 challenge:


Should be numbered step also, same for all of these steps?

abrennan89 · 2020-01-13T17:52:13Z

docs/serving/using-auto-tls.md

+    kubectl get configmap config-network --namespace knative-serving --output yaml
+    ```

 Congratulations! Knative is now configured to obtain and renew TLS certificates.


Remove "Congratulations!..." - this doesn't really add anything. Instead consider adding the steps to confirm this works as a 'verification' section or similar.

Added a section as "verification"

abrennan89 · 2020-01-13T17:52:46Z

docs/serving/using-auto-tls.md

+    ```

 Congratulations! Knative is now configured to obtain and renew TLS certificates.
 When your TLS certificate is active on your cluster, your Knative services will


Move this to earlier in the doc? Part of explanation of why someone would want to use it?

ZhiminXiang · 2020-01-13T23:05:32Z

/cc @rmoe

knative-prow-robot · 2020-01-13T23:05:33Z

@ZhiminXiang: GitHub didn't allow me to request PR reviews from the following users: rmoe.

Note that only knative members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @rmoe

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

samodell

Apologies @ZhiminXiang ; it looks like I made a few comments but never sent them to you.

samodell · 2020-01-14T02:52:45Z

docs/serving/using-auto-tls.md


-1. Determine if `networking-certmanager` is already installed by running the 
-    following command:
+### Step 1: Create cert-manager ClusterIssuer


Let's avoid putting "Step #" in headings. Instead, just leave the descriptions -- "Create cert-manager ClusterIssuer," for this one, but this should be applied throughout the page.

samodell · 2020-01-14T02:53:14Z

docs/serving/using-auto-tls.md

-        [`ClusterIssuer` example](https://docs.cert-manager.io/en/latest/tasks/issuers/setup-acme.html#creating-a-basic-acme-issuer)
-      - Also see the
-        [`DNS-01` example](https://docs.cert-manager.io/en/latest/tasks/acme/configuring-dns01/index.html)
+    Use the cert-manager reference to determine how to configure your


+1. Let's try to be consistent with numbering all steps.

samodell · 2020-01-22T22:26:59Z

/lgtm
/approve

Thanks for this!!

knative-prow-robot · 2020-01-22T22:27:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: samodell, ZhiminXiang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [samodell]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* initial version of auto tls instruction * Update auto-tls doc to mention namespace cert feature and http01 challenge feature * address comments * fix format * address comments

* Kourier Installation guide for Knative Serving (#2006) * Adds a new guide describing how to install knative with kourier * Links the knative with kourier installation guide in the main README file * Use kourier-system instead of knative-serving when getting kourier IP * Install Kourier using the included yaml in third_party * Update the Auto TLS instruction (#2083) * initial version of auto tls instruction * Update auto-tls doc to mention namespace cert feature and http01 challenge feature * address comments * fix format * address comments * Format cmd for easy reading&copy (#2120) * Updating to LATEST for Kafka Channel/Source (#2121) * Updating to LATEST for Kafka Channel/Source * Switching the values to 1 replica, but 3 partitions Co-authored-by: Joaquim Moreno Prusi <jmprusi@keepalive.io> Co-authored-by: Zhimin Xiang <zhiminx@google.com> Co-authored-by: 赤月 <974226358@qq.com> Co-authored-by: Matthias Wessendorf <mwessend@redhat.com>

Zhimin Xiang added 2 commits January 4, 2020 00:54

initial version of auto tls instruction

417ee95

Update auto-tls doc to mention namespace cert feature and http01 chal…

5dd1cce

…lenge feature

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jan 4, 2020

knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 4, 2020

knative-prow-robot requested review from abrennan89 and samodell January 4, 2020 22:28

knative-prow-robot requested a review from mattmoor January 4, 2020 22:29

knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 4, 2020

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 13, 2020

abrennan89 suggested changes Jan 13, 2020

View reviewed changes

Zhimin Xiang added 2 commits January 13, 2020 23:55

address comments

3e72852

fix format

526d2e4

samodell reviewed Jan 21, 2020

View reviewed changes

Zhimin Xiang added 2 commits January 22, 2020 21:35

address comments

87a2dda

Merge branch 'master' into auto_tls_instruction

a60bab6

knative-prow-robot assigned samodell Jan 22, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2020

knative-prow-robot added the approved label Jan 22, 2020

knative-prow-robot merged commit 27382ee into knative:master Jan 22, 2020

samodell added the cherrypick-0.12 label Jan 22, 2020

samodell removed the cherrypick-0.12 label Jan 23, 2020

abrennan89 mentioned this pull request Jun 11, 2020

Add abrennan89 to doc-approvers #2579

Merged

3 tasks


		## Automatic TLS provision mode

		In Knative, we support the following modes of Auto TLS:


		In Knative, we support the following modes of Auto TLS:

		1. Using DNS01 challenge

Update the Auto TLS instruction #2083

Update the Auto TLS instruction #2083

Uh oh!

Conversation

ZhiminXiang commented Jan 4, 2020

Proposed Changes

Uh oh!

ZhiminXiang commented Jan 4, 2020

Uh oh!

ZhiminXiang commented Jan 4, 2020

Uh oh!

ZhiminXiang commented Jan 13, 2020

Uh oh!

abrennan89 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZhiminXiang commented Jan 13, 2020

Uh oh!

knative-prow-robot commented Jan 13, 2020

Uh oh!

samodell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samodell commented Jan 22, 2020

Uh oh!

knative-prow-robot commented Jan 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants