feat: add support for certificate fetching and rotation via SDS server#8537
feat: add support for certificate fetching and rotation via SDS server#8537zirain wants to merge 6 commits intoenvoyproxy:mainfrom
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| // such as the SPIRE or others. | ||
| // | ||
| // +optional | ||
| SDS *SDSProvider `json:"sds,omitempty"` |
There was a problem hiding this comment.
do we need to support multiple SDS here?
There was a problem hiding this comment.
In that case, we need to figure out how to reference to specific sds provider in clientCertificateRef:
caCertificateRefs:
- name: ROOTCA
kind: SDSwe probably need something like sds-provider-name://ROOTCA but which make thing more complex, becasue we use BackendObjectReference/BackendCluster to reference the backend, there's no provider name.
There was a problem hiding this comment.
we rely on parentRef like semantics
There was a problem hiding this comment.
AFAIT, CACertificateRefs uses gwapiv1.SecretObjectReference or gwapiv1.LocalObjectReference.
There was a problem hiding this comment.
Sorry, I missed the context. I’d like to better understand the reasoning behind this. Is the concern that introducing additional custom resources could impact reconciliation performance?
IMO, a CRD provides a better user experience, as it more closely aligns with Gateway API semantics. Using an implicit kind/name could be surprising to users, as it diverges from the behavior defined in the Gateway API specification.
There was a problem hiding this comment.
Regarding the secret-based option mentioned yesterday, I meant that we can still use a secret reference (just like today), but when the secret has a custom type like gateway.envoyproxy.io/sds-ref, we expect certain keys to exist in that secret that provide the necessary metadata, e.g. sds url and sds secret name.
apiVersion: v1
kind: Secret
metadata:
name: sds-secret-sample
type: gateway.envoyproxy.io/sds-ref
data:
url: /var/run/sds
secretName: my-tls-secret
This secret may contain additional things like authorization for sds server access.
There was a problem hiding this comment.
this sounds goos to me without introuduce any new API.
The only thing is that it's more like a hook instead of a new API.
There was a problem hiding this comment.
+1 to trying out @guy's suggestion, we can make this experimental
this avoids the need for an SDSProvider in the EnvoyProxy and instead requires an opt in flag like SupportSDSSecret in the EnvoyGateway API
There was a problem hiding this comment.
+1
This approach is more structured than a flat url while also avoids introducing an additional EG API.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8537 +/- ##
==========================================
+ Coverage 74.10% 74.19% +0.08%
==========================================
Files 242 243 +1
Lines 37638 37727 +89
==========================================
+ Hits 27893 27991 +98
+ Misses 7790 7782 -8
+ Partials 1955 1954 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
| // such as the SPIRE or others. | ||
| type SDSProvider struct { | ||
| // BackendObjectReference references a Kubernetes object that represents the backend. | ||
| gwapiv1.BackendObjectReference `json:",inline"` |
There was a problem hiding this comment.
shouldnt this be
gateway/api/v1alpha1/shared_types.go
Line 600 in ffceb4f
| // Name of the Secret object. | ||
| Name string `json:"name" yaml:"name"` | ||
| Name string `json:"name" yaml:"name"` | ||
| FromSDS *bool `json:"fromSDS,omitempty" yaml:"fromSDS,omitempty"` |
There was a problem hiding this comment.
does this need to be a named sds string ?
There was a problem hiding this comment.
I have no strong option about use a SDSName instead of the combination of FromSDS+Name.
| // such as the SPIRE or others. | ||
| // | ||
| // +optional | ||
| SDS *SDSProvider `json:"sds,omitempty"` |
There was a problem hiding this comment.
also needs smething like https://gateway-api.sigs.k8s.io/reference/1.5/spec/#allowedroutes to allow which namespaces and BTLSP can reference it
There was a problem hiding this comment.
Under what conditions, we need to limit a BTLSP reference to SDS provider.
There was a problem hiding this comment.
we dont want an unknown app to reference an sds secret unless allowed by the platform team
There was a problem hiding this comment.
this could be done in the next iterater or later.
Let's focus on #8537 (comment) first, do we really need mutiple SDS providers, and how will the API looks like.
| // such as the SPIRE or others. | ||
| type SDSProvider struct { | ||
| // BackendObjectReference references a Kubernetes object that represents the backend. | ||
| gwapiv1.BackendObjectReference `json:",inline"` |
There was a problem hiding this comment.
should we validate if it is a Service or Backend?
There was a problem hiding this comment.
we should, let's make an agreement about should this be multiple or not first.
fixes: #6366
The idea is introduce a new API
SDSinEnvoyProxyto define an external SDS server.ServiceorBackendkind: SDSincaCertificateRefsorclientCertificateRef,namewill be used to fetch certificate from (external) SDS server.