-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
Update ECS cluster name validation #38993
Update ECS cluster name validation #38993
Conversation
Previous regex was not complete and allowed for apply time failures
Community NoteVoting for Prioritization
For Submitters
|
So I know there's a lot that goes on in this provider, and while this is pretty small, it like many other bugs can have consequences to deployments and availability of resources. I would think fixing bugs would be fairly high priority. That said I've read the contributing docs and am still curious what is a normal timeframe for getting PRs reviewed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this, @gregops312.
I've made a couple adjustments:
- I've removed the
.
that was in the regex, since that's not valid for ECS cluster names - We have an order that the character classes should appear in, since we cache compiled regexes. I've re-ordered the regexp.
@gdavison Thank you very much. I'll keep those things in mind should I come across anything else in the future. I also see you added a |
Yes, there is some documentation about adding a changelog entry (https://hashicorp.github.io/terraform-provider-aws/changelog-process/), but we have recently reorganized the contributor guide, so it may have been more buried when you created the PR originally. |
This functionality has been released in v5.69.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Previous regex was not complete and allowed for apply time failures
Description
Updating the ECS
validateClusterName
which contained and incomplete regex allowing for many cases with plans that wouldn't fail as expectedRelations
n/a
References
n/a
Output from Acceptance Testing
n/a