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

provider/aws: Add validation for aws_elb.name #2517

Merged
merged 1 commit into from
Jun 26, 2015

Conversation

radeksimko
Copy link
Member

@phinze
Copy link
Contributor

phinze commented Jun 26, 2015

must be unique within your AWS account

Someday maybe we can actually do an API query to verify this. Someday. Maybe. 😀

@phinze
Copy link
Contributor

phinze commented Jun 26, 2015

LGTM

@radeksimko
Copy link
Member Author

Someday maybe we can actually do an API query to verify this. Someday. Maybe.

I see your point, but the beauty of these validations is that it can all be done offline :) so to keep that benefit, we'd have to save all existing ELBs in the account somewhere otherwise I don't see the difference between asking "Create" API or "List"/"Search" api ... :-/

radeksimko added a commit that referenced this pull request Jun 26, 2015
provider/aws: Add validation for aws_elb.name
@radeksimko radeksimko merged commit 7217a37 into hashicorp:master Jun 26, 2015
@radeksimko radeksimko deleted the f-aws-elb-validation branch June 26, 2015 14:33
@phinze
Copy link
Contributor

phinze commented Jun 26, 2015

Good point. Something to think on.

My feeling is that it seems like we should do everything we can to catch as many potential errors in the Plan step instead of the Apply step - that would be the difference in doing a pre-Describe earlier. Of course there's always going to be a race condition. We can discuss more once we get to that layer of niceties. Someday. 👍 😀

@@ -25,6 +26,26 @@ func resourceAwsElb() *schema.Resource {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
value := v.(string)
if !regexp.MustCompile(`^[0-9a-z-]$`).MatchString(value) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@catsby I overlooked this one too... :/

Copy link
Contributor

Choose a reason for hiding this comment

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

no worries, they're getting sorted out 😄

@pmoust
Copy link
Contributor

pmoust commented Jun 30, 2015

woops, we 've got about 12 ELBs that are [0-9A-Za-z-].
@radeksimko: Any reason why you went for [0-9a-z-]?
I went in to test v0.6.0 and got this little surprise.

@ghost
Copy link

ghost commented May 1, 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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators May 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants