Support config to deploy internal certificates automatically #13005

nak3 · 2022-06-08T11:04:55Z

Currently users have to deploy certificates manually with several options such as
activator-san, activator-ca, queue-proxy-ca etc.

Such deployment and management of the certificates is a big burden for users.

Hence, this patch supports internal-encryption config to deploy
internal certificates automatically.

All certificates are managed and deployed by control-protocol/pkg/certificates.

codecov · 2022-06-08T11:09:48Z

Codecov Report

Merging #13005 (7a929b0) into main (752c336) will decrease coverage by 0.25%.
The diff coverage is 15.00%.

@@            Coverage Diff             @@
##             main   #13005      +/-   ##
==========================================
- Coverage   87.05%   86.79%   -0.26%     
==========================================
  Files         197      197              
  Lines       14443    14485      +42     
==========================================
- Hits        12573    12572       -1     
- Misses       1576     1619      +43     
  Partials      294      294

Impacted Files	Coverage Δ
cmd/activator/main.go	`0.00% <0.00%> (ø)`
cmd/controller/main.go	`0.00% <ø> (ø)`
cmd/queue/main.go	`0.64% <ø> (ø)`
pkg/reconciler/revision/cruds.go	`62.50% <0.00%> (-14.43%)`	⬇️
pkg/reconciler/revision/reconcile_resources.go	`66.88% <0.00%> (-13.92%)`	⬇️
pkg/reconciler/revision/resources/deploy.go	`90.10% <0.00%> (ø)`
pkg/reconciler/revision/revision.go	`92.13% <0.00%> (-4.34%)`	⬇️
pkg/reconciler/autoscaling/kpa/kpa.go	`95.26% <100.00%> (ø)`
pkg/reconciler/route/resources/ingress.go	`96.18% <100.00%> (ø)`
pkg/autoscaler/statserver/server.go	`77.77% <0.00%> (-1.49%)`	⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 752c336...7a929b0. Read the comment docs.

skonto · 2022-06-14T10:43:06Z

@nak3 pls rebase. Is there a test that uses control-protocol/pkg/certificates? Any pointer for tests? I think we need to document for the end-user how this works since he might want to have some sort of control about the certificate generation or at least have an understanding.
The cert-manager support should not be delayed imho as it is something people use a lot.

nak3 · 2022-06-14T11:10:23Z

@skonto Could you please review knative/networking#680 and merge it?
I can rebase this PR but this PR needs networking/pull/680 and I have to keep rebasing until I get it.

For test, the kourier-tls in actions use it and run all e2e&conformance test with the feature.
For documentation, yes I will add the docs once the PRs were mergeed.

nak3 · 2022-06-15T11:12:45Z

/cc @skonto

This needs knative-extensions/net-kourier#855 but ready for review.

third_party/kourier-latest/kourier.yaml

cmd/queue/main.go

pkg/reconciler/autoscaling/kpa/kpa_test.go

pkg/reconciler/revision/reconcile_resources.go

cmd/controller/main.go

skonto · 2022-06-16T08:34:00Z

pkg/reconciler/revision/cruds.go

@@ -46,6 +48,20 @@ func (c *Reconciler) createDeployment(ctx context.Context, rev *v1.Revision) (*a
 	return c.kubeclient.AppsV1().Deployments(deployment.Namespace).Create(ctx, deployment, metav1.CreateOptions{})
 }

+func (c *Reconciler) createSecret(ctx context.Context, ns *corev1.Namespace) (*corev1.Secret, error) {
+	secret := &corev1.Secret{


I guess data are filled in by the control-plane reconciler. Was this part of the original design to use the control-plane? It would be nice to have a high a level diagram of secrets created and encrypted paths (maybe there is one).
There might be one just I may lack context.

There is a doc, but I'm not sure it specifies this level of detail.

I'm slightly confused about where the secret comes from here, and what the lifecycle is if internal encryption is disabled. (It may be that it's fine to drop this resource, but I don't quite understand the lifecycle.)

The lifecycle of the secret in user's ns (queue-proxy's server cert) is:

The secret is created when users deploy the first Kservice in the namespace.

The secret is shared by all Kservice in the namespace.

The secret is dropped when the namespace is deleted. (the secret is neither dropped when all Kservice is deleted in the namespace nor the internal encryption is disabled.).

There are many ways to manage the secret in user's ns and so I chose the simplest way for now.

cmd/queue/main.go

nak3 · 2022-06-16T11:30:30Z

@skonto Thank you. Updated.

skonto · 2022-06-16T11:55:44Z

LGTM /cc @evankanderson for more comments.

linux-foundation-easycla · 2022-06-16T12:51:56Z

The committers listed above are authorized under a signed CLA.

✅ login: nak3 / name: Kenjiro Nakayama (70d58d7, 3af4f43, fb30cfc, 4d19c6a, 8140587, d313bd5, 56d0063, 6e5ba2a, 0389bee, dba0c95, de48142, d823ca3)
✅ login: knative-automation (200a1f3)

bumping knative.dev/net-kourier d758682...b9b1e8b: > b9b1e8b Use `internal-encryption` to deploy internal certificates automatically (# 855) > 427434c bump kind and k8s versions in kind-e2e tests (# 859) Signed-off-by: Knative Automation <automation@knative.team>

psschwei · 2022-06-28T20:25:47Z

Wanted to check on the status here... we're about two weeks away from cutting release 1.6, so not sure how much runway we'd want to leave between when this merges and release day.

nak3 · 2022-06-28T23:01:41Z

This is ready for another round of review. @evankanderson @psschwei Could you please take a look?
I would like to add this into releases 1.6.

evankanderson · 2022-06-29T07:21:40Z

Added some more comments. I'd prefer to get the per-namespace certs in and leave the activator in-path for now, with SNI in the activator as the plan for supporting mixed activator / queue-proxy pools

nak3 · 2022-06-29T08:46:18Z

Thank you! Updated.

evankanderson

/lgtm
/approve

knative-prow · 2022-06-30T16:38:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: evankanderson, nak3

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~~ [evankanderson,nak3]

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

…#13005) * Add certificate reconciler for internal certs * Fix cert path * Temporary use local networking repo * Support internal-encryption configuration * Use const for cert name * Fix lint * rm blank line * Drop unused variable * Use one line style * Use one line code * Update net-kourier nightly bumping knative.dev/net-kourier d758682...b9b1e8b: > b9b1e8b Use `internal-encryption` to deploy internal certificates automatically (# 855) > 427434c bump kind and k8s versions in kind-e2e tests (# 859) Signed-off-by: Knative Automation <automation@knative.team> * Verify SecretPKKey as well * Do not drop activator always in the path * Comment about ctrl-ca suffix Co-authored-by: Knative Automation <automation@knative.team>

…#13005) (#1183) * Add certificate reconciler for internal certs * Fix cert path * Temporary use local networking repo * Support internal-encryption configuration * Use const for cert name * Fix lint * rm blank line * Drop unused variable * Use one line style * Use one line code * Update net-kourier nightly bumping knative.dev/net-kourier d758682...b9b1e8b: > b9b1e8b Use `internal-encryption` to deploy internal certificates automatically (# 855) > 427434c bump kind and k8s versions in kind-e2e tests (# 859) Signed-off-by: Knative Automation <automation@knative.team> * Verify SecretPKKey as well * Do not drop activator always in the path * Comment about ctrl-ca suffix Co-authored-by: Knative Automation <automation@knative.team> Co-authored-by: Knative Automation <automation@knative.team>

…#13005) * Add certificate reconciler for internal certs * Fix cert path * Temporary use local networking repo * Support internal-encryption configuration * Use const for cert name * Fix lint * rm blank line * Drop unused variable * Use one line style * Use one line code * Update net-kourier nightly bumping knative.dev/net-kourier d758682...b9b1e8b: > b9b1e8b Use `internal-encryption` to deploy internal certificates automatically (# 855) > 427434c bump kind and k8s versions in kind-e2e tests (# 859) Signed-off-by: Knative Automation <automation@knative.team> * Verify SecretPKKey as well * Do not drop activator always in the path * Comment about ctrl-ca suffix Co-authored-by: Knative Automation <automation@knative.team>

…ically (#1236) * [RELEASE-v1.5] Add manifest patch for internal-tls to `openshift/release/artifacts` (#1202) * Add secret to 1.5 CI yaml * auto generated * Support config to deploy internal certificates automatically (knative#13005) * Add certificate reconciler for internal certs * Fix cert path * Temporary use local networking repo * Support internal-encryption configuration * Use const for cert name * Fix lint * rm blank line * Drop unused variable * Use one line style * Use one line code * Update net-kourier nightly bumping knative.dev/net-kourier d758682...b9b1e8b: > b9b1e8b Use `internal-encryption` to deploy internal certificates automatically (# 855) > 427434c bump kind and k8s versions in kind-e2e tests (# 859) Signed-off-by: Knative Automation <automation@knative.team> * Verify SecretPKKey as well * Do not drop activator always in the path * Comment about ctrl-ca suffix Co-authored-by: Knative Automation <automation@knative.team> * Update deps * Enable internal-tls on ocp-tls (#1203) * Enable internal-tls on OCP 4.8 * Use tls to match JOB name * Add a target to enable internal-tls in Makefile (#1224) * Add a target to enable internal-tls in Makefile * Update CI template for internal-tls enabled * Tests for encryption with Kourier local gateway (knative#13263) * Generate Secrets * Commit generated cert-secret.yaml * httpproxy enables tls client * httpproxy uses https when CA_CERT specified * Pass CA_CERT and SERVER_NAME env variables properly to tests * Avoid using cluster-local certificates for external services * Enable tls tests for cluster-local Kourier gateway * Need to create test resources including the test namespace first before installing Knative so that applying test/config/tls/cert-secret.yaml succeeds * TMP: Enable tls in the standard e2e make target - test purposes * Use knative-serving-ingress ns for deploying server-certs * Deploy certificates at test phase * Separate test and install of installing certs * Wait for knative-serving-ingress to exist * Revert "TMP: Enable tls in the standard e2e make target - test purposes" This reverts commit 5bb3549. Co-authored-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com> Co-authored-by: Knative Automation <automation@knative.team>

nak3 mentioned this pull request Jun 8, 2022

[WIP] Use cert-manager to deploy internal certificates #12998

Closed

2 tasks

This was referenced Jun 8, 2022

Use internal-encryption to deploy internal certificates automatically knative-extensions/net-kourier#855

Merged

Support internal-encryption to deploy internal certificates automatically knative/networking#680

Merged

nak3 added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 8, 2022

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 11, 2022

nak3 force-pushed the add-cert-controller-config branch from f3e00dc to fcfb58c Compare June 15, 2022 01:53

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 15, 2022

nak3 mentioned this pull request Jun 15, 2022

[main] Upgrade to latest dependencies #13037

Merged

knative-prow bot requested a review from skonto June 15, 2022 11:12