Skip to content
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

Certificate Filter for Certificate Discovery supporting RSA 4096 #2719

Closed
conrad784 opened this issue Jul 7, 2022 · 11 comments
Closed

Certificate Filter for Certificate Discovery supporting RSA 4096 #2719

conrad784 opened this issue Jul 7, 2022 · 11 comments
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects

Comments

@conrad784
Copy link

conrad784 commented Jul 7, 2022

Is your feature request related to a problem?
For our frontends in our EKS we need certificates which are RSA_4096, which are currently not issued by AWS ACM. Therefore we have an external certificate provider and are importing the certificates via IaC to AWS.

When creating an ALB manually, I can select this certificate and everything is working fine. But the k8s load-balancer-controller only says

{"level":"error","ts":1657132899.6504095,"logger":"controller-runtime.manager.controller.ingress","msg":"Reconciler error","name":"example-app","namespace":"default","error":"ingress: default/example-app: no certificate found for host: example.com"}

Turns out, by default the AWS-cli also does not show the certificate when aws acm list-certificates only after adding --include keyTypes=RSA_2048,RSA_4096 the wanted certificate is shown (see keyTypes in https://docs.aws.amazon.com/cli/latest/reference/acm/list-certificates.html#options)
If I am providing the ingress annotations with the certificate ARN will result in an ALB with the right certificate provisioned but this can hardly be described as auto-configuration ;)

Describe the solution you'd like
It would be nice to be able to provide filters to the certificate discovery, I only tried hard-coding it (as I am lacking the required go-knowledge) for my case in c8bddae and it is working for me.
By default the keyTypes filter should be like defined in the aws-cli command reference ["RSA_2048,RSA_4096"], but this should be over-writable with a configuration of this controller.
Important to note: NLB and classic LB can't use RSA_4096 certificates from ACM (whereas it might be possible when loading it from IAM? https://docs.amazonaws.cn/en_us/elasticloadbalancing/latest/classic/elb-update-ssl-cert.html).

Describe alternatives you've considered
Chaining an NLB before the auto-configured ALB does not work, as it does not support the certificate.
Maybe creating the target group + ALB manually would also work, but this does not scale nicely.
There is some conflicting information from AWS, what you should/can do with those certificates within their service. One resource says you should upload them to IAM [1] but on an older entry it says ACM now supports those certificates [2] so I am not fully understanding why those server-certificates still exist in IAM. I can't test the IAM way as my current account is restricted not to use IAM. I also can not find if there is code to lookup certificates in IAM #60

[1] https://aws.amazon.com/premiumsupport/knowledge-center/import-ssl-certificate-to-iam/
[2] https://aws.amazon.com/about-aws/whats-new/2021/07/aws-certificate-manager-provides-expanded-usage-imported-ecdsa-rsa-certificates/

@conrad784 conrad784 changed the title Certificate Filter Certificate Filter for Certificate Discovery supporting RSA 4096 Jul 7, 2022
@M00nF1sh
Copy link
Collaborator

@conrad784
Thanks for providing these detailed information. I think the solution you described is a valid one, by provide some flags like certificate-discovery-key-types. But i'm wondering whether we should provide two flags for ALB and NLB separately(e.g. certificate-discovery-alb-key-types and certificate-discovery-nlb-key-types.

We'll also sync with ALB/NLB team to see the supported certificate types and see whether we can add these keyTypes by default without configuration.

@M00nF1sh M00nF1sh self-assigned this Jul 14, 2022
@M00nF1sh M00nF1sh added the kind/bug Categorizes issue or PR as related to a bug. label Jul 14, 2022
@kishorj kishorj added this to To do in v2.5.0 via automation Aug 24, 2022
@kishorj kishorj added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Sep 14, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 8, 2023
@conrad784
Copy link
Author

/remove-lifecycle stale

@kishorj kishorj removed this from To do in v2.5.0 Apr 11, 2023
@kishorj kishorj added this to To do in v2.6.0 via automation Apr 11, 2023
@yair-sedaka-dt
Copy link

yair-sedaka-dt commented Aug 1, 2023

Have the same issue self-discovery is not working for RSA 4096 certificates

@oliviassss
Copy link
Collaborator

@conrad784, @yair-sedaka, We have shipped the improvement with v2.6.0 release, I'm closing the issue as for now. Please feel free to reopen if you have any issue. Thanks

@nmofonseca
Copy link

@conrad784, @yair-sedaka, We have shipped the improvement with v2.6.0 release, I'm closing the issue as for now. Please feel free to reopen if you have any issue. Thanks

Hello @oliviassss ,

I can't find anything in the documentation, is this going to be added to the documentation? Meanwhile can you share how can one install the load balancer controller specyfing the list of certificates?

@oliviassss
Copy link
Collaborator

@nmofonseca, you can either use the ingress annotation alb.ingress.kubernetes.io/certificate-arn, or our controller can auto discover the matched certs from your ACM.
see our live doc for more details: https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.6/guide/ingress/cert_discovery/

@nmofonseca
Copy link

nmofonseca commented Jan 3, 2024

@nmofonseca, you can either use the ingress annotation alb.ingress.kubernetes.io/certificate-arn, or our controller can auto discover the matched certs from your ACM. see our live doc for more details: https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.6/guide/ingress/cert_discovery/

Hello @oliviassss ,

Thank you for your reply. I understand that I can either specify the certificate arn via ingress annotations or leverage the auto discovery

The problem stated in this issue was that auto discovery wouldn't work for e.g ECDSA certificates since the underlying aws acm api by default doesn't return them, unless we pass the keytype as includes.

So the proposed solution here initially was to allow to specify that filter which then the controller would use.

From what you said, in v2.6 improvements have been made but is not clear what that means.I did check the PR mentioned here #3314 but both my coding and knowledge of go are very limited.

So to double check from what you replied am I correct in assuming that the improvements in v2.6 are not an implementation as described here but just to allow th auto discovery to pick up any certificate type without the need to fiddle with filters?

If you could provide further clarifications it will be much appreciated

@oliviassss
Copy link
Collaborator

oliviassss commented Jan 3, 2024

@nmofonseca what's the controller version you are currently on? Maybe check by kubectl describe deployment -n kube-system aws-load-balancer-controller | grep -i Image
if you're on older version, you can upgrade to v2.6.0+ and retry. Either by editing the deployment, or helm upgrade.

@nmofonseca
Copy link

@nmofonseca what's the controller version you are currently on? Maybe check by kubectl describe deployment -n kube-system aws-load-balancer-controller | grep -i Image if you're on older version, you can upgrade to v2.6.0+ and retry. Either by editing the deployment, or helm upgrade.

I haven't tested yet with v2.6.x yet, was just try it to understand if the improvements mentioned require any additional steps or configuration, from what I am getting from you it doesn't seem to do.

Tomorrow will try to upgrade and test.

@nmofonseca
Copy link

hello @oliviassss ,

Apologies for the delay. I have now tested with the latest version 2.6.2 and it picked up my ECDSA certificate.

Thank you for the help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
No open projects
Development

No branches or pull requests

8 participants