feat: add support for certificate fetching via SDS ref secret#8745
feat: add support for certificate fetching via SDS ref secret#8745zirain merged 7 commits intoenvoyproxy:mainfrom
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ac7d7798e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8745 +/- ##
==========================================
- Coverage 73.64% 73.63% -0.02%
==========================================
Files 245 246 +1
Lines 48864 49076 +212
==========================================
+ Hits 35985 36135 +150
- Misses 10874 10919 +45
- Partials 2005 2022 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I am looking forward to this. I like the use of no new K8s types. I guess going to have to use some extra tooling so that the secrets can be in all namespaces that BackendTLSPolicy are defined due not being able to use cross reference, which is not a big issue |
BackendTLSPolicy use LocalObjectReference, which make it unable to cross namespace reference. |
| const ( | ||
| // SDSSecretType is the type for secrets that reference SDS configuration | ||
| SDSSecretType = "gateway.envoyproxy.io/sds-ref" | ||
|
|
There was a problem hiding this comment.
can we make this opt in at EnvoyGateway or EnvoyProxy level ?
cc @guydc
There was a problem hiding this comment.
add EnableSDSSecretRef in EnvoyGateway.
| type: gateway.envoyproxy.io/sds-ref | ||
| data: | ||
| url: L3Zhci9ydW4vc2VjcmV0cy93b3JrbG9hZC1zcGlmZmUtdWRzL3NvY2tldA== # /var/run/secrets/workload-spiffe-uds/socket | ||
| secretName: Uk9PVENB # ROOTCA |
There was a problem hiding this comment.
I'd to reserve the name for sds provider name in the future, secretName sounds good to me for now.
There was a problem hiding this comment.
Using Name for the provider feels a bit ambiguous, and the naming patterns between the two are not consistent.
name: `foo provider`
secretName: `foo secret`
url: xxxxx
How about
provider: `foo provider`
secret: `foo secret`
url: xxxxx
Or
providerName: `foo provider`
secretName: `foo secret`
url: xxxxx
?
I prefer the former one for simplicity.
| // If set to true, the Lua EnvoyExtensionPolicy feature will be disabled. | ||
| DisableLua bool `json:"disableLua"` | ||
| // EnableSDSSecretRef enables read SDS(Secret Discovery Service) settings from a secret(with type gateway.envoyproxy.io/sds-ref). | ||
| EnableSDSSecretRef bool `json:"enableSDSSecretRef"` |
There was a problem hiding this comment.
I think we are gating the broader "fetching certificates from third-party SDS" feature, not just the specific "SDS secret type"?
It might makes sense to use a more general name like EnableThirdPartySDS here.
cc @arkodg
…ecret Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
…roxy#8745) * feat: add support for certificate fetching and rotation via SDS ref secret Signed-off-by: zirain <zirain2009@gmail.com> * fix Signed-off-by: zirain <zirain2009@gmail.com> * fix review comments Signed-off-by: zirain <zirain2009@gmail.com> * fix lint Signed-off-by: zirain <zirain2009@gmail.com> * add EnableSDSSecretRef in EnvoyGateway Signed-off-by: zirain <zirain2009@gmail.com> * fix gen Signed-off-by: zirain <zirain2009@gmail.com> * rename Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Jake Oliver <jake@truelayer.com>
suppress #8537
Original idea from @guydc