-
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
r/aws_launch_configuration: support security groups and user data for import #2800
r/aws_launch_configuration: support security groups and user data for import #2800
Conversation
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 @loivis 😄
Can you please add acceptance testing to ensure the behavior is correct? It can be as simple as adding the below import TestStep
to TestAccAWSLaunchConfiguration_withVpcClassicLink
and removing user_data
from the ignore list in TestAccAWSLaunchConfiguration_importBasic
{
ResourceName: "aws_launch_configuration.foo",
ImportState: true,
ImportStateVerify: true,
},
d.Set("associate_public_ip_address", lc.AssociatePublicIpAddress) | ||
if len(lc.SecurityGroups) > 0 { |
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.
This can be simplified and error checked with:
if err := d.Set("security_groups", flattenStringList(lc.SecurityGroups)); err != nil {
return fmt.Errorf("error setting security_groups: %s", err)
}
} | ||
d.Set("security_groups", securityGroups) | ||
} | ||
if *lc.UserData != "" { |
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 safely dereference this pointer or it can cause a panic: if aws.StringValue(lc.UserData) !=""
|
||
d.Set("vpc_classic_link_id", lc.ClassicLinkVPCId) | ||
d.Set("vpc_classic_link_security_groups", lc.ClassicLinkVPCSecurityGroups) | ||
if len(lc.ClassicLinkVPCSecurityGroups) > 0 { |
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.
This can be simplified and error checked with:
if err := d.Set("vpc_classic_link_security_groups", flattenStringList(lc.ClassicLinkVPCSecurityGroups)); err != nil {
return fmt.Errorf("error setting vpc_classic_link_security_groups: %s", err)
}
9ba43de
to
f9eac46
Compare
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.
You are awesome, @loivis! LGTM 🚀
10 tests passed (all tests)
=== RUN TestAccAWSLaunchConfiguration_withSpotPrice
--- PASS: TestAccAWSLaunchConfiguration_withSpotPrice (6.62s)
=== RUN TestAccAWSLaunchConfiguration_withEncryption
--- PASS: TestAccAWSLaunchConfiguration_withEncryption (9.20s)
=== RUN TestAccAWSLaunchConfiguration_importBasic
--- PASS: TestAccAWSLaunchConfiguration_importBasic (9.55s)
=== RUN TestAccAWSLaunchConfiguration_withBlockDevices
--- PASS: TestAccAWSLaunchConfiguration_withBlockDevices (12.76s)
=== RUN TestAccAWSLaunchConfiguration_withVpcClassicLink
--- PASS: TestAccAWSLaunchConfiguration_withVpcClassicLink (18.54s)
=== RUN TestAccAWSLaunchConfiguration_basic
--- PASS: TestAccAWSLaunchConfiguration_basic (22.09s)
=== RUN TestAccAWSLaunchConfiguration_updateRootBlockDevice
--- PASS: TestAccAWSLaunchConfiguration_updateRootBlockDevice (22.58s)
=== RUN TestAccAWSLaunchConfiguration_ebs_noDevice
--- PASS: TestAccAWSLaunchConfiguration_ebs_noDevice (24.58s)
=== RUN TestAccAWSLaunchConfiguration_updateEbsBlockDevices
--- PASS: TestAccAWSLaunchConfiguration_updateEbsBlockDevices (24.84s)
=== RUN TestAccAWSLaunchConfiguration_withIAMProfile
--- PASS: TestAccAWSLaunchConfiguration_withIAMProfile (28.17s)
This has been released in version 1.14.1 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
The |
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! |
Fix #2648 .
Noticed that
ClassicLinkVPCSecurityGroups
is also returned as[]*string
likeSecurityGroups
. So the same change is applied on both.