-
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
Do not validate items that conflict #24202
Do not validate items that conflict #24202
Conversation
The `failure_tolerance_count` and `failure_tolerance_percentage` conflict with each other so one should be validated only if the other has failed. The same applies for `max_concurrent_count` and `max_concurrent_percentage`
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.
Welcome @MarkCBell 👋
It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.
Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.
Thanks again, and welcome to the community! 😃
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.
LGTM 🚀.
Prior to fix:
% make testacc TESTS=TestAccCloudFormationStackSet_operationPreferences PKG=cloudformation
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudformation/... -v -count 1 -parallel 20 -run='TestAccCloudFormationStackSet_operationPreferences' -timeout 180m
=== RUN TestAccCloudFormationStackSet_operationPreferences
=== PAUSE TestAccCloudFormationStackSet_operationPreferences
=== CONT TestAccCloudFormationStackSet_operationPreferences
stack_set_test.go:268: Step 4/5 error: Error running apply: exit status 1
Error: error updating CloudFormation StackSet (tf-acc-test-1440391482554017189): InvalidParameter: 1 validation error(s) found.
- minimum field value of 1, UpdateStackSetInput.OperationPreferences.MaxConcurrentCount.
with aws_cloudformation_stack_set.test,
on terraform_plugin_test.tf line 25, in resource "aws_cloudformation_stack_set" "test":
25: resource "aws_cloudformation_stack_set" "test" {
--- FAIL: TestAccCloudFormationStackSet_operationPreferences (47.90s)
FAIL
FAIL github.com/hashicorp/terraform-provider-aws/internal/service/cloudformation 51.741s
FAIL
make: *** [testacc] Error 1
After fix:
% make testacc TESTS=TestAccCloudFormationStackSet_operationPreferences PKG=cloudformation
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudformation/... -v -count 1 -parallel 20 -run='TestAccCloudFormationStackSet_operationPreferences' -timeout 180m
=== RUN TestAccCloudFormationStackSet_operationPreferences
=== PAUSE TestAccCloudFormationStackSet_operationPreferences
=== CONT TestAccCloudFormationStackSet_operationPreferences
--- PASS: TestAccCloudFormationStackSet_operationPreferences (81.30s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/cloudformation 85.855s
@MarkCBell Thanks for the contribution 🎉 👏. |
This functionality has been released in v4.10.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! |
@ewbankkit Thank you for working on this. However I believe that there is a bug in the logic that you have added to
will trigger neither case since You could address this by modifying the logic to:
I will open a new issue / PR proposing this change. Note that this does not apply to |
@ewbankkit I have proposed this change in PR #24250 |
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. |
Community Note
The
failure_tolerance_count
andfailure_tolerance_percentage
conflict with each other, so one should be validated only if the other has failed. The same applies formax_concurrent_count
andmax_concurrent_percentage
.Closes #24161