-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add documentation for Serving Encryption #5804
Conversation
✅ Deploy Preview for knative ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ReToCode 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:
Approvers can indicate their approval by writing |
/cc @pierDipi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been away from this for a bit, so mostly just questions for my understanding, but it's cool to see this all coming together!
to automatically distribute the CA bundles. Please refer to their documentation for more information on how to do this. | ||
|
||
|
||
### Trust during rotation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question:
just want to verify that I understand this page.
- cluster-local clients must be implemented to consume the CA data (this isn't Knative's business).
- The Knative system components, though, have been implemented so that, provided the right configmap exists, it can pull in and populate the right components with the right CA data.
- With the trust bundle, there is one "place" to put CA data so it can be consumed by many.
- Even with that, when rotating, it is still up to the user to coordinate adding in the new CA data/removing the old CA data and giving enough time for things to switch over
Does that sound correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is exactly how it is 👍
docs/serving/encryption/install-and-configure-net-certmanager.md
Outdated
Show resolved
Hide resolved
## Trust | ||
|
||
!!! warning | ||
A quick note on trust, Knative will automatically trust the CA that signed the Certificates, if the cert-manager issuer allows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question:
I'm not sure I understand this line.
Is it saying that if your Secret has a ca.crt
field, then Knative will trust it?
Could you please help me understand what the implications of that are?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that's it. Cert-Manager has different issuers. Some of them add the CA to ca.crt
in the Secrets, other do not (like an external ACME CA will never populate the CA there). For convenience, Knative will also trust the ca.crt
to make a simple setup work out of the box (without an additional CA bundle in a ConfigMap).
/unhold |
Updated the documentation to reflect that net-certmanager is now integrated in Serving. @skonto PTAL. |
/unassign |
@skonto mind taking another look? We can use downstream enablement of this feature to test if the doc is complete. |
@@ -0,0 +1,50 @@ | |||
# Serving Encryption Overview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we call it native encryption or something. I think we need to distinguish between what Istio offers with a full mesh (which we support) and what it is documented here (ingress only + tls in Knative components.)
As a new Knative user it would be nice to know the options I have for encryption and security in general.
Maybe a table would help with key features/capabilities for each option.
Currently, all control-plane traffic (including Kubernetes PreStopHooks and metadata like metrics) is not encrypted. | ||
|
||
## The parts in detail | ||
The different parts are independent of each other and (can) use different Certificate Authorities to sign certificates. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a user perspective what is the value of allowing part 2, without part 3? What setup do we recommend here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is really. It just gives the user the option to do the transition in steps, e.g. maybe they already have 1, they could start with 3 and add 2 once they have the CA stuff figured out. There is not really a recommendation, this highly depends on the environment and the requirements of a company.
* Accessing it from a `Secret`/`ConfigMap` via K8s API | ||
|
||
If reloading certificates without downtime is important for your client, the workload must either watch changes on the K8s resource (Secret/ConfigMap) or watch the filesystem. | ||
If the workload is watching the filesystem, it is important to note that using `ionotify` to catch changing Secrets/ConfigMaps is not very reliable on K8s. Tests have shown that it is more reliable to regularly poll and check the certificate on the filesystem for changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need to have an example on how to do the rotation. I think a minimum example should include manual rotation based on workload restarting (as others have). That would be appreciated by operators I suppose to start with something.
I am ok merging this to get early feedback, also we will test it downstream anyway. I have some comments for follow up work. |
@skonto updated as discussed. |
/lgtm |
spec: | ||
ca: | ||
secretName: knative-selfsigned-ca | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this example, I would add some comments to the two ClusterIssuer
to explain what they are for and where they are used. Also, I probably would align the naming so that 'selfsigned' is on the same positions (so either selfsigned-cluster-issuer/selfsigned-knative-issuer or cluster-selfsigned-issuer/knative-selfsigned-issuer
Great documentation! Thanks a lot, @ReToCode ! |
Fixes #5805
Fixes #5800
Proposed Changes