Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(kic): add upstream TLS verification guide #8201

Merged
merged 13 commits into from
Dec 18, 2024

Conversation

czeslavo
Copy link
Contributor

@czeslavo czeslavo commented Nov 29, 2024

Description

Adds a guide covering upstream TLS verification configuration features added in KIC 3.4.

Part of #8133 (this guide will have to be extended with Gateway API instructions once Kong/kubernetes-ingress-controller#6631 is implemented).

Testing instructions

KIC nightly image has to be used for testing:

helm upgrade --install kong kong/ingress -n kong --create-namespace --set controller.ingressController.image.repository=kong/nightly-ingress-controller --set controller.ingressController.image.tag=2024-11-28 --set controller.ingressController.image.effectiveSemver=3.4.0

Preview link: https://deploy-preview-8201--kongdocs.netlify.app/kubernetes-ingress-controller/unreleased/guides/security/verify-upstream-tls/

Checklist

@czeslavo czeslavo self-assigned this Nov 29, 2024
@czeslavo czeslavo added team-k8s Kubernetes task tracking (including KIC,KGO) area/docs labels Nov 29, 2024
@czeslavo czeslavo added this to the KIC v3.4.x milestone Nov 29, 2024
Copy link

netlify bot commented Nov 29, 2024

Deploy Preview for kongdocs ready!

Name Link
🔨 Latest commit 5d8a855
🔍 Latest deploy log https://app.netlify.com/sites/kongdocs/deploys/67617d8bc86acf0008f547df
😎 Deploy Preview https://deploy-preview-8201--kongdocs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
9 paths audited
Performance: 93 (🟢 up 3 from production)
Accessibility: 92 (no change from production)
Best Practices: 98 (🟢 up 8 from production)
SEO: 99 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@czeslavo czeslavo force-pushed the docs/upstream-tls-verification branch 4 times, most recently from c659dde to cd90ea5 Compare November 29, 2024 12:44
@czeslavo czeslavo marked this pull request as ready for review November 29, 2024 12:49
@czeslavo czeslavo requested a review from a team as a code owner November 29, 2024 12:49
@czeslavo czeslavo requested a review from a team November 29, 2024 12:49
@czeslavo czeslavo added the review:general Review for general accuracy and presentation. Does the doc work? Does it output correctly? label Nov 29, 2024
@czeslavo czeslavo force-pushed the docs/upstream-tls-verification branch 3 times, most recently from 3928706 to 9309404 Compare December 2, 2024 11:24
@lahabana
Copy link
Contributor

Probably more related to the CRD move than this but I was missing a bunch of RBAC to some CRDs (.e.g: kongupstreampolicies, konglicences, kongcustomentities, kongvaults, kongconsumergroups) once fixing clusterrole/kong-controller I got unblocked.

@lahabana
Copy link
Contributor

One thing I'm unsure here is how we articulate this guide for BackendTLS. Should we just add a last section to this guide: "Configure Kong Gateway to verify the upstream TLS certificate using GatewayAPI BackendTLS"?

@pmalek
Copy link
Member

pmalek commented Dec 12, 2024

Probably more related to the CRD move than this but I was missing a bunch of RBAC to some CRDs (.e.g: kongupstreampolicies, konglicences, kongcustomentities, kongvaults, kongconsumergroups) once fixing clusterrole/kong-controller I got unblocked.

Those should all be in place, weird 🤔 .

ℹ️ BackendTLSPolicy and ConfigMap (not yet used in this guide) rules were fixed in https://github.com/Kong/charts/releases/tag/ingress-0.15.2 and https://github.com/Kong/charts/releases/tag/kong-2.44.1.

@pmalek pmalek force-pushed the docs/upstream-tls-verification branch from a175f36 to 130c695 Compare December 12, 2024 18:27
Copy link
Contributor

@lahabana lahabana left a comment

Choose a reason for hiding this comment

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

This is clear and works well.

My only point of confusion is around the likely connection reuse that would confuse anyone doing things quickly.
However, I'm not sure how we can fix this.

```text
deployment.apps/echo patched
service/echo patched
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: but I was blindly copy pasting and applying. So when I saw this I had to look back and check that I didn't miss a step because the last copy/paste only updates the service.

Might be worth doing either both patches in one block or have the output for each block (I prefer this second option).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, it will be cleaner to have the expected outputs separately: 5d8a855

@pmalek pmalek force-pushed the docs/upstream-tls-verification branch from ac0800f to fd72900 Compare December 13, 2024 13:08
@pmalek
Copy link
Member

pmalek commented Dec 13, 2024

ac0800f added the BackendTLSPolicy parts for this guide.

ℹ️ I've created Kong/kubernetes-ingress-controller#6834 to cover the implementation gap between using annotations and BackendTLSPolicy (the latter only allows CA certs in ConfigMaps, as per the Gateway API spec: https://github.com/kubernetes-sigs/gateway-api/blob/26051f587ccaf54823ccfd7eca5cbf2013aa8177/apis/v1alpha3/backendtlspolicy_types.go#L115-L119).

@pmalek pmalek linked an issue Dec 13, 2024 that may be closed by this pull request
2 tasks
@pmalek pmalek force-pushed the docs/upstream-tls-verification branch from fd72900 to 25efd42 Compare December 13, 2024 16:29
@czeslavo czeslavo force-pushed the docs/upstream-tls-verification branch from 26812a7 to 697f576 Compare December 16, 2024 11:56
@czeslavo
Copy link
Contributor Author

Thank you, @pmalek, for taking care of updating the PR! 🙇 I reviewed it and found a couple of nits, fixed them in 98035d0 and 697f576. PTAL

@pmalek
Copy link
Member

pmalek commented Dec 16, 2024

Thank you, @pmalek, for taking care of updating the PR! 🙇 I reviewed it and found a couple of nits, fixed them in 98035d0 and 697f576. PTAL

Looks good to me 👍

@czeslavo czeslavo force-pushed the docs/upstream-tls-verification branch from 643918a to 523dacf Compare December 16, 2024 13:07
@czeslavo
Copy link
Contributor Author

Changed annotations to use plural form (following Kong/kubernetes-ingress-controller#6845) in b8e6034 and added navtabs for CA certificates sourced from Secrets for BackendTLSPolicy (as we've decided it will be implemented before releasing KIC 3.4 in Kong/kubernetes-ingress-controller#6834) in 523dacf.

Copy link
Member

@mlavacca mlavacca left a comment

Choose a reason for hiding this comment

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

Thanks @czeslavo, the PR looks good to me apart from a detail: in the prerequisite section of the guide we are installing Gateway API from the standard channel, while BackendTLSPolicies are in the experimental one. We should update the Gateway API installation section with the proper channel.

@czeslavo
Copy link
Contributor Author

czeslavo commented Dec 17, 2024

in the prerequisite section of the guide we are installing Gateway API from the standard channel, while BackendTLSPolicies are in the experimental one. We should update the Gateway API installation section with the proper channel.

Good catch, fixed in 0d93270.

@czeslavo czeslavo requested a review from mlavacca December 17, 2024 12:17
@czeslavo czeslavo force-pushed the docs/upstream-tls-verification branch from 0d93270 to 9e49ba5 Compare December 17, 2024 12:17
@czeslavo czeslavo requested a review from mlavacca December 17, 2024 13:30
Copy link
Member

@mlavacca mlavacca left a comment

Choose a reason for hiding this comment

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

👍

@Guaris Guaris merged commit d99eadd into main Dec 18, 2024
15 checks passed
@Guaris Guaris deleted the docs/upstream-tls-verification branch December 18, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs review:general Review for general accuracy and presentation. Does the doc work? Does it output correctly? team-k8s Kubernetes task tracking (including KIC,KGO)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation for TLS verification support exists
5 participants