Add Certificate refs for https listeners #1211
Conversation
| @@ -71,18 +71,33 @@ spec: | |||
| items: | |||
| type: string | |||
| type: array | |||
| certificates: | |||
There was a problem hiding this comment.
@EdgeJ this is a breaking change - can we use certificates here instead of certificateArn ?
There was a problem hiding this comment.
Absolutely. I thought about this after I threw up the Pr, but I wanted to gauge the reaction here before going back to fix it up. I don't see any reason the old API and the new selector can't live side by side. Will fix this up.
There was a problem hiding this comment.
@haarchri I updated the changes to insert the ARN refs in the certificates field instead of creating new fields to address backwards compatibility.
EdgeJ
force-pushed
the
edgej/elbv2-certificate-ref
branch
from
b2e2e03 to
561de19
Compare
|
Not sure why e2e tests are failing here. Setup problems? |
There was a problem hiding this comment.
@EdgeJ they are failing due to crossplane/crossplane#2944 -- fix is incoming 👍🏻
Signed-off-by: EdgeJ <[email protected]>
Signed-off-by: EdgeJ <[email protected]>
EdgeJ
force-pushed
the
edgej/elbv2-certificate-ref
branch
from
561de19 to
45ef7e0
Compare
Rebased on master and pushed. Seems like it's fixed. |
* Add Certificate refs for https listeners Signed-off-by: EdgeJ <[email protected]> Signed-off-by: Felipe Barbosa <[email protected]>
Signed-off-by: EdgeJ [email protected]
Description of your changes
This change adds references and selectors for ACM certificates to automate the creation of HTTPS listeners.
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
Created and deleted listeners using example YAML.