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

Look up Certificate ARNs that match ingress' #600

Closed
wants to merge 1,189 commits into from
Closed

Conversation

cv
Copy link

@cv cv commented Sep 11, 2018

Hi! 👋

I'm attempting a setup in which users of my k8s cluster get some subdomains to play with, like <app>.<user>.dev.example.com. I'd like to ensure all of their public-facing pods have an HTTP redirect to HTTPS, and that the certificate for their domain (say, *.cv.dev.example.com) is stored and managed by ACM (which is done elsewhere, in a Terraform script).

In order to minimize confusion, I'd like to use the smallest possible number of annotations, so that most apps will just work without a lot of yaml and helm template tweaking.

To do that, I'd like to have aws-alb-ingress-controller look that certificate up for me, so that I don't have to leak the abstraction by asking my users to know their certificate ARN. That way, if there is a certificate that matches a spec.tls.hosts[] entry, I don't need to add the certificate-arn annotation to the ingresses.

In other words, if I have successfully issued a cert arn:aws:acm:xxx:yyy:certificate/zzz for the *.cv.dev.example.com domain, the following ingress...

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    alb.ingress.kubernetes.io/scheme: internet-facing
    alb.ingress.kubernetes.io/target-type: ip
    alb.ingress.kubernetes.io/listen-ports: '[{"HTTPS": 443}]'
    kubernetes.io/ingress.class: alb
  name: echoserver
  namespace: echoserver
spec:
  tls:
  - hosts:
    - echoserver.cv.dev.example.com
  rules:
  - host: echoserver.cv.dev.example.com
    http:
      paths:
      - backend:
          serviceName: echoserver
          servicePort: 80
        path: /

should:

  • Set up a HTTP->HTTPS redirect (out of scope for this PR, believe I can do that with actions already)
  • Set up a HTTPS listener with the arn:aws:acm:xxx:yyy:certificate/zzz cert

The implementation thus far is quite naive and almost pseudo-code, but should serve as a starting point to gather more feedback.

WDYT?

argeualcantara and others added 30 commits July 24, 2018 16:25
…ps-values

Support KOPS values for private/public subnet tags
Add `elasticloadbalancing:DescribeSSLPolicies` to work with the `alb.ingress.kubernetes.io/ssl-policy` annotation
Add DescribeSSLPolicies to iam-policy.json
…tting refreshing times for ~180 ALBs down 50% to around 3m40s
…gress references a service that doesn't exist.
@cv
Copy link
Author

cv commented Sep 24, 2018

Just saw this in the logs:

Warning  ERROR  1m               aws-alb-ingress-controller  Error creating 443 listener: TooManyCertificates: Up to '1' certificate ARNs can be specified, but '2' were specified
           status code: 400, request id: 8d7b8fc7-c003-11e8-af7c-0dc2f94b60d2

I should've read more of the code :) api.go states:

type CreateListenerInput struct {
	_ struct{} `type:"structure"`

	// [HTTPS listeners] The default SSL server certificate. You must provide exactly
	// one default certificate. To create a certificate list, use AddListenerCertificates.
	Certificates []*Certificate `type:"list"`
...

Added a call to AddListenerCertificates when more than a single cert is found. Doing some manual testing to make sure AWS likes that change now.

@hatemosphere
Copy link

@bigkraig could you please explain why this PR was closed?

@bigkraig
Copy link

bigkraig commented Oct 9, 2018

@hatemosphere we rebased the master branch which closed all of the existing PRs, you can see the impact it had if you look at the commits in here now. The best way to get this one back in order is to cherry-pick the commits into a branch that has been created off of the new master.

@khyew
Copy link

khyew commented Feb 12, 2019

@cv looks like your PR here got closed accidentally after a rebase of master. I'm still interested in your feature but the rebase also nuked GitHub's copy of your commits.

Would you mind cherry-picking them into a new branch and creating a new PR?

@cv
Copy link
Author

cv commented Feb 12, 2019

@khyew thanks for letting me know! I am no longer working on this, but will try to cherry-pick all the changes back in. Gimme a few minutes…

@cv
Copy link
Author

cv commented Feb 12, 2019

Well, that was overly enthusiastic of my part… I can't figure out how to cherry-pick the changes, or get a patch that applies at all. @khyew if you want to take a stab at it, all of the commits in the range from 54f7333 to 8ad75b3 (in the original tree, that is) need to come back.

@khyew
Copy link

khyew commented Feb 12, 2019

@cv just the commits that were authored by you in that range, correct?

@cv
Copy link
Author

cv commented Feb 12, 2019

@khyew yup! I'm pretty sure I was the only one to commit to the branch.

@khyew
Copy link

khyew commented Feb 13, 2019

How did you test your PR? Were you able to deploy your own branch of the controller to a cluster? I found this guide btw: https://kubernetes-sigs.github.io/aws-alb-ingress-controller/BUILDING/

@cv
Copy link
Author

cv commented Feb 13, 2019

Exactly, built a new Docker image, pushed it to ECR and ran it on my EKS cluster from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet