-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
issue #4449 Fix the issue using Arn field instead of Name in IamInstanceProfileSpecification #4511
Conversation
Changing the parameter it uses will likely break existing configurations. 😄 Can you please introduce a separate Terraform attribute into the schema? e.g. |
@@ -191,6 +191,11 @@ func resourceAwsSpotFleetRequest() *schema.Resource { | |||
ForceNew: true, | |||
Optional: true, | |||
}, | |||
"iam_instance_profile_arn": { |
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.
Can you please
- Document the new argument in the argument reference near the existing
iam_instance_profile
argument - Add a simple acceptance test to cover this new argument? Copy pasting an existing test and test configuration that uses
iam_instance_profile
is fine 👍
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.
Documented the new argument under launch_specification
section and added the acceptance test.
@@ -27,13 +27,15 @@ resource "aws_spot_fleet_request" "cheap_compute" { | |||
ami = "ami-1234" | |||
spot_price = "2.793" | |||
placement_tenancy = "dedicated" | |||
iam_instance_profile_arn = "arn:aws:iam::12345678:instance-profile/webserver-role" |
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.
We should either fix this so its a properly formatted ARN (account ID is 12 digits 123456789012
) or just reference a "fake" resource ("${aws_iam_instance_profile.example.arn}"
). Also as a nitpick, if these are going to be added in here, please line up all the equals symbols similar to how terraform fmt
would e.g.
ami = "ami-1234"
spot_price = "2.793"
placement_tenancy = "dedicated"
iam_instance_profile_arn = "${aws_iam_instance_profile.example.arn}"
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.
Done 👍
$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSpotFleetRequest_iamInstanceProfileArn' |
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.
Looks good @saravanan30erd -- thanks so much for this contribution! 🚀
18 tests passed (all tests)
=== RUN TestAccAWSSpotFleetRequest_placementTenancy
--- PASS: TestAccAWSSpotFleetRequest_placementTenancy (52.80s)
=== RUN TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameSubnet
--- PASS: TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameSubnet (216.13s)
=== RUN TestAccAWSSpotFleetRequest_associatePublicIpAddress
--- PASS: TestAccAWSSpotFleetRequest_associatePublicIpAddress (258.66s)
=== RUN TestAccAWSSpotFleetRequest_lowestPriceSubnetInGivenList
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceSubnetInGivenList (259.78s)
=== RUN TestAccAWSSpotFleetRequest_overriddingSpotPrice
--- PASS: TestAccAWSSpotFleetRequest_overriddingSpotPrice (261.04s)
=== RUN TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameAz
--- PASS: TestAccAWSSpotFleetRequest_multipleInstanceTypesInSameAz (261.53s)
=== RUN TestAccAWSSpotFleetRequest_withEBSDisk
--- PASS: TestAccAWSSpotFleetRequest_withEBSDisk (263.18s)
=== RUN TestAccAWSSpotFleetRequest_lowestPriceAzOrSubnetInRegion
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceAzOrSubnetInRegion (264.08s)
=== RUN TestAccAWSSpotFleetRequest_withoutSpotPrice
--- PASS: TestAccAWSSpotFleetRequest_withoutSpotPrice (264.41s)
=== RUN TestAccAWSSpotFleetRequest_withWeightedCapacity
--- PASS: TestAccAWSSpotFleetRequest_withWeightedCapacity (264.45s)
=== RUN TestAccAWSSpotFleetRequest_withTags
--- PASS: TestAccAWSSpotFleetRequest_withTags (265.23s)
=== RUN TestAccAWSSpotFleetRequest_diversifiedAllocation
--- PASS: TestAccAWSSpotFleetRequest_diversifiedAllocation (266.23s)
=== RUN TestAccAWSSpotFleetRequest_WithELBs
--- PASS: TestAccAWSSpotFleetRequest_WithELBs (307.47s)
=== RUN TestAccAWSSpotFleetRequest_instanceInterruptionBehavior
--- PASS: TestAccAWSSpotFleetRequest_instanceInterruptionBehavior (321.92s)
=== RUN TestAccAWSSpotFleetRequest_iamInstanceProfileArn
--- PASS: TestAccAWSSpotFleetRequest_iamInstanceProfileArn (325.75s)
=== RUN TestAccAWSSpotFleetRequest_lowestPriceAzInGivenList
--- PASS: TestAccAWSSpotFleetRequest_lowestPriceAzInGivenList (335.53s)
=== RUN TestAccAWSSpotFleetRequest_WithTargetGroups
--- PASS: TestAccAWSSpotFleetRequest_WithTargetGroups (452.63s)
=== RUN TestAccAWSSpotFleetRequest_changePriceForcesNewRequest
--- PASS: TestAccAWSSpotFleetRequest_changePriceForcesNewRequest (706.64s)
This has been released in version 1.19.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
I recently upgraded to 1.19.0 without changing my existing Changing to the new _arn attribute fixed the problem, and using the ARN seems like a fine idea. Would it be better to simply remove the existing |
when using |
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! |
Used Arn field instead of Name in Type IamInstanceProfileSpecification.