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

r/aws_elb: Add listener ssl_certificate_id ARN validation #2276

Merged
merged 1 commit into from
Nov 15, 2017

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Nov 14, 2017

Closes #2122

@radeksimko radeksimko added the enhancement Requests to existing resources that expand the functionality or scope. label Nov 15, 2017
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the enhancement, just FYI - this probably won't help in the mentioned example as we don't/can't run validation at plan time if the field contains references - it's not possible to guess what's going to be the interpolated string before we have chance to fully interpolate.

We discussed some approaches to that problem, e.g. adding more specific types to fields, so that we know that it's always going to be a valid ARN, even if it is computed by the API, but we haven't gotten beyond the stage of brainstorming.

That said this may help folks who have hardcoded value in ssl_certificate_id, so 🤷‍♂️ LGTM.

I'm ok with closing the issue, because such early validation is a feature request which belongs to core anyway - i.e. hashicorp/terraform.

@radeksimko radeksimko merged commit 5dc07a7 into hashicorp:master Nov 15, 2017
@bflad bflad deleted the elb_iam_cert_validatearn branch November 15, 2017 12:27
@bflad
Copy link
Contributor Author

bflad commented Nov 15, 2017

Makes sense -- I wonder if core during the plan could use the validateFunc of the computed attribute to ensure its "output" aligns with the input? If that doesn't work even a simple new "outputFormat" schema option that provides a necessary validation string/type? I agree it is definitely a core enhancement.

I'll poke around the core issues and create something if I can't find anything. 😄

@ghost
Copy link

ghost commented Apr 10, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope.
Projects
None yet
2 participants