-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 RSA support to TLS libraries #3135
Conversation
Fixes #3131 Wrapped private keys into either `PrivateKeyEC` or `PrivateKeyRSA` to provide different certificate matching logic and marshaling depending on the block type. You can test having an RSA cert for the proxy injector by applying this patch: ```diff $ diff -u chart/templates/proxy_injector-rbac.yaml ~/tmp/proxy_injector-rbac.yaml --- chart/templates/proxy_injector-rbac.yaml 2019-07-24 14:34:43.570616936 -0500 +++ /home/alpeb/tmp/proxy_injector-rbac.yaml 2019-07-24 13:41:03.150285099 -0500 @@ -1,4 +1,5 @@ {{with .Values -}} +{{- $ca := genCA "linkerd-proxy-injector.linkerd.svc" 365 -}} --- ### ### Proxy Injector RBAC @@ -60,8 +61,8 @@ {{ .CreatedByAnnotation }}: {{ .CliVersion }} type: Opaque data: - crt.pem: {{ b64enc .ProxyInjector.CrtPEM }} - key.pem: {{ b64enc .ProxyInjector.KeyPEM }} + crt.pem: {{ b64enc $ca.Cert }} + key.pem: {{ b64enc $ca.Key }} --- apiVersion: admissionregistration.k8s.io/v1beta1 kind: MutatingWebhookConfiguration @@ -81,7 +82,7 @@ name: linkerd-proxy-injector namespace: {{ .Namespace }} path: "/" - caBundle: {{ b64enc .ProxyInjector.CrtPEM }} + caBundle: {{ b64enc $ca.Cert }} failurePolicy: {{ .WebhookFailurePolicy }} rules: - operations: [ "CREATE" ] ``` This will replace the logic to generate the cert with a call to Helm's `genCA`, which uses RSA. Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>
Integration test results for 3c8b794: fail 😕 |
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.
thanks for providing the test diff, confirmed works! a couple nits then 🚢
Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>
Integration test results for f8f3742: success 🎉 |
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.
➰ ⭐ LGTM
Fixes #3131
Wrapped private keys into either
PrivateKeyEC
orPrivateKeyRSA
toprovide different certificate matching logic and marshaling depending on
the block type.
You can test having an RSA cert for the proxy injector by applying this
patch:
This will replace the logic to generate the cert with a call to Helm's
genCA
, which uses RSA.