New Citadel API proto #8152
New Citadel API proto #8152
Conversation
security/proto/istioca.proto
Outdated
| repeated string cert_chain = 1; | ||
| } | ||
|
|
||
| // Service for managing certificates issued by the Istio CA. |
There was a problem hiding this comment.
"Istio CA" is no longer a valid term here. Should rephrase to avoid mentioning.
There was a problem hiding this comment.
Good call. This should be general CA.
security/proto/istioca.proto
Outdated
|
|
||
| package istio.v1.auth; | ||
|
|
||
| import "gogoproto/gogo.proto"; |
There was a problem hiding this comment.
just curious - these lines are required ? seems not in https://github.com/istio/istio/blob/collab-gcp-identity/security/proto/ca/v1alpha1/istioca.proto
There was a problem hiding this comment.
They are not required as far as I see for now. I removed them.
Codecov Report
@@ Coverage Diff @@
## master #8152 +/- ##
=======================================
+ Coverage 71% 71% +1%
=======================================
Files 374 374
Lines 32673 32956 +283
=======================================
+ Hits 22906 23235 +329
+ Misses 8760 8712 -48
- Partials 1007 1009 +2
Continue to review full report at Codecov.
|
| // Using provided CSR, returns a signed certificate. | ||
| rpc CreateCertificate(IstioCertificateRequest) | ||
| returns (IstioCertificateResponse) { | ||
| } |
There was a problem hiding this comment.
Just so I understand the model, node agents ie daemon sets will request csrs from citadel and then the node agent will sign pod certificates and serve pod certificates to sidecar Envoy over SDS?
Won’t this put the CA in the critical path for all pod creation events
There was a problem hiding this comment.
Actually in OSS (k8s), the Envoy requests cert from node agent via SDS. Node agent creates the key pair and sends the CSR to Citadel with pod JWT. Citadel signs the cert and returns it to node agent, and node agent sends the cert to Envoy through SDS.
There was a problem hiding this comment.
So, if the CA is unavailable, the pod won't start or won't be reachable until the CA is available right? Also, if we are rotating the certs, the CA has to be available for the cert rotation to work online.
There was a problem hiding this comment.
Compared with current situation where Citadel watch service account and generate secrets, in an async way along the pod creation. For mTLS enabled workloads, Citadel is still on the critical path to get pods functioning and refresh the key cert (otherwise after new connection can't be established after certs expired).
If we implement Pilot such that SDS config is only distributed for mTLS enabled service/workloads, then that does not make much difference?
There was a problem hiding this comment.
Probably. If come with pilot, mixer requires mTLS, then with or without mTLS for service all pods will end up doing SDs
There was a problem hiding this comment.
Also please have a go implementation of the SDs client to talk to citadel. All istio components can use that library if they want to.
There was a problem hiding this comment.
Sure.
We're aware of the plan to remove envoy sidecar for control plane components. And their mTLS/security requirement definitely will be covered before we turn this on.
There was a problem hiding this comment.
|
/test all |
|
@myidpt: The following test failed, say
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. I understand the commands that are listed here. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: incfly, quanjielin 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 |
Introduce the new Citadel API, related issue: #8151.
Also updated other .proto_descriptor files in the same dir by rerunning mixer_codegen.sh.