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

WIP: Support multiple certificates on annotations. #498

Conversation

stafot
Copy link

@stafot stafot commented Jul 28, 2018

No description provided.

bigkraig and others added 30 commits May 1, 2018 09:34
…t-build-var-at-build-time

Fix build info injection [Small]
…e-docs-to-show-necessary-kube-ingress-annotation

Add missing kube ingress.class annotation to examples [Small] kubernetes-sigs#369
When not empty the Host header check won't be enabled as part of the ALB listener routing rules.

This commit closes kubernetes-sigs#371.
…nd-cidrs-annotation

Add support for the inbound-cidrs annotation
…eTimeout since time of 0 is invalid which is what is parsed from annotation
…-timeout

Fix so if no connection-idle-timeout is set
…er-refactor

Refactored alb-controller.go to enable more unit test coverage
…er-refactor

Refactored annotation/validation into interface
fix DescribeTargetGroupTargetsForArn with empty target list
…p-to-kubernetes-sigs

Transfer ownership to kubernetes sigs
Add usage and values for the attributes annotation
Credits to @masterzen who had added this to his PR but it had already been merged without this doc
Add license scan report and status
The ingress controller pod was getting in a crash loop specifically asking for waf-regional:GetWebACLForResource access. I searched the code and found references to the other permissions - specifically found them here: https://github.com/coreos/alb-ingress-controller/blob/master/pkg/aws/waf/waf.go
Update iam-policy.json - Provide WebACL permissions
bigkraig and others added 4 commits July 26, 2018 15:09
…so fixing a bug which disassociated the web acl.
Fix a bug which disassociated Web ACLs right after associating them
…services were using the same port they would conflict. This wasn't a problem when services were nodeports only.
TargetGroup names are not unique enough in the current state
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 28, 2018
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 28, 2018
@coveralls
Copy link

coveralls commented Jul 28, 2018

Coverage Status

Coverage decreased (-1.01%) to 22.378% when pulling dc25e2f on stafot:feature/support_multiple_certificates_annotations into a6565ea on kubernetes-sigs:master.

@stafot stafot force-pushed the feature/support_multiple_certificates_annotations branch 2 times, most recently from 7bd5339 to 1ab3b83 Compare July 28, 2018 23:52
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 28, 2018
@stafot stafot force-pushed the feature/support_multiple_certificates_annotations branch 2 times, most recently from 543ab03 to 11c2b2b Compare July 29, 2018 00:14
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 29, 2018
@stafot stafot force-pushed the feature/support_multiple_certificates_annotations branch 2 times, most recently from e1b099b to dc3e777 Compare July 29, 2018 00:21
@stafot stafot changed the title Support multiple certificates on annotations. WIP: Support multiple certificates on annotations. Jul 29, 2018
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 29, 2018
@stafot stafot force-pushed the feature/support_multiple_certificates_annotations branch from dc3e777 to 2fed6e7 Compare July 29, 2018 05:58
@stafot stafot force-pushed the feature/support_multiple_certificates_annotations branch from 2fed6e7 to dc25e2f Compare July 29, 2018 11:50
@argeualcantara
Copy link

@stafot Please don't forget to update the ingress-resources.md docs with the new usage of the annotation

@cv
Copy link

cv commented Sep 13, 2018

Hi @stafot! Could you have a look at #600 as well?

It solves this problem in a slightly different way, by allowing you to specify multiple host names in either spec.rules.host or spec.tls.hosts entries in the ingress object. Wildcard domains should work, too. Cheers!

@stafot
Copy link
Author

stafot commented Sep 13, 2018

@cv Thanks for your work. It looks that resolves the issue I tried to handle with this PR, so It looks that my WIP PR can be closed in favour of yours. Hope your PR will be merged soon so can test in my environment and provide you some more feedback.

@cv
Copy link

cv commented Sep 13, 2018 via email

@stafot
Copy link
Author

stafot commented Sep 13, 2018

@cv Thanks

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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet