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

r/aws_instance: Fix associate_public_ip_address #1340

Merged
merged 3 commits into from
Aug 8, 2017

Conversation

grubernaut
Copy link
Contributor

Previously we had no way of determining if a boolean attribute was a
literal false or a zero-value type of false. With the core enhancement
added to Terraform via hashicorp/terraform#15723,
we can now reliably check for a zero-type value of a boolean attribute,
or a literal boolean false inside of schema.

This allows the associate_public_ip_address to work as expected,
when configuring an AWS Instance inside of a subnet that defaults to
public address assignment, with a literal false for the associate_public_ip_address attribute.

$ make testacc TEST=./aws TESTARGS="-run=TestAccAWSInstance_explicitAssociatePublicAddress"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstance_explicitAssociatePublicAddress -timeout 120m
=== RUN   TestAccAWSInstance_explicitAssociatePublicAddress
--- PASS: TestAccAWSInstance_explicitAssociatePublicAddress (113.04s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       113.045s

Fixes: #227

@grubernaut grubernaut added the bug Addresses a defect in current functionality. label Aug 3, 2017
@alejandroEsc
Copy link

Please verify this test matrix before accepting this PR:

Tests to perform on Launch Configurations and Instances:

  • Leave off the flag, start new instance in a public subnet and instance starts up as default (public)
  • Leave off the flag, start new instance in a public private subnet and instance starts as default (private)
  • add flag as public, start new instance in public subnet and instance starts up as public
  • add flag as public, start instance in a private subnet and instance starts as private
  • add flag as private, start new instance in public subnet and instance starts up as private
  • add flag as private, start new instance in private subnet and instance starts as private

please note that distinction between the three cases: default, public, private they are very different, Do the same tests for the both instances and launch configurations please.

@grubernaut
Copy link
Contributor Author

@alejandroEsc expanded tests to cover:

$ make testacc TEST=./aws TESTARGS="-run=TestAccAWSInstance_associatePublic_"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstance_associatePublic_ -timeout 120m
=== RUN   TestAccAWSInstance_associatePublic_defaultPrivate
--- PASS: TestAccAWSInstance_associatePublic_defaultPrivate (106.54s)
=== RUN   TestAccAWSInstance_associatePublic_defaultPublic
--- PASS: TestAccAWSInstance_associatePublic_defaultPublic (215.38s)
=== RUN   TestAccAWSInstance_associatePublic_explicitPublic
--- PASS: TestAccAWSInstance_associatePublic_explicitPublic (112.23s)
=== RUN   TestAccAWSInstance_associatePublic_explicitPrivate
--- PASS: TestAccAWSInstance_associatePublic_explicitPrivate (224.96s)
=== RUN   TestAccAWSInstance_associatePublic_overridePublic
--- PASS: TestAccAWSInstance_associatePublic_overridePublic (117.78s)
=== RUN   TestAccAWSInstance_associatePublic_overridePrivate
--- PASS: TestAccAWSInstance_associatePublic_overridePrivate (112.48s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	889.375s

Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @grubernaut ! I went ahead and ran all of the Instance tests:

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstance_ -timeout 120m
=== RUN   TestAccAWSInstance_importBasic
--- PASS: TestAccAWSInstance_importBasic (100.46s)
=== RUN   TestAccAWSInstance_basic
--- PASS: TestAccAWSInstance_basic (135.76s)
=== RUN   TestAccAWSInstance_GP2IopsDevice
--- PASS: TestAccAWSInstance_GP2IopsDevice (61.41s)
=== RUN   TestAccAWSInstance_blockDevices
--- PASS: TestAccAWSInstance_blockDevices (78.29s)
=== RUN   TestAccAWSInstance_rootInstanceStore
--- PASS: TestAccAWSInstance_rootInstanceStore (76.95s)
=== RUN   TestAccAWSInstance_sourceDestCheck
--- PASS: TestAccAWSInstance_sourceDestCheck (165.18s)
=== RUN   TestAccAWSInstance_disableApiTermination
--- PASS: TestAccAWSInstance_disableApiTermination (135.45s)
=== RUN   TestAccAWSInstance_vpc
--- PASS: TestAccAWSInstance_vpc (97.60s)
=== RUN   TestAccAWSInstance_ipv6_supportAddressCount
--- PASS: TestAccAWSInstance_ipv6_supportAddressCount (213.00s)
=== RUN   TestAccAWSInstance_ipv6AddressCountAndSingleAddressCausesError
--- PASS: TestAccAWSInstance_ipv6AddressCountAndSingleAddressCausesError (16.02s)
=== RUN   TestAccAWSInstance_ipv6_supportAddressCountWithIpv4
--- PASS: TestAccAWSInstance_ipv6_supportAddressCountWithIpv4 (211.82s)
=== RUN   TestAccAWSInstance_multipleRegions
--- PASS: TestAccAWSInstance_multipleRegions (227.12s)
=== RUN   TestAccAWSInstance_NetworkInstanceSecurityGroups
--- PASS: TestAccAWSInstance_NetworkInstanceSecurityGroups (103.82s)
=== RUN   TestAccAWSInstance_NetworkInstanceVPCSecurityGroupIDs
--- PASS: TestAccAWSInstance_NetworkInstanceVPCSecurityGroupIDs (119.91s)
=== RUN   TestAccAWSInstance_tags
--- PASS: TestAccAWSInstance_tags (118.69s)
=== RUN   TestAccAWSInstance_volumeTags
--- PASS: TestAccAWSInstance_volumeTags (111.58s)
=== RUN   TestAccAWSInstance_volumeTagsComputed
--- PASS: TestAccAWSInstance_volumeTagsComputed (116.40s)
=== RUN   TestAccAWSInstance_instanceProfileChange
--- PASS: TestAccAWSInstance_instanceProfileChange (110.94s)
=== RUN   TestAccAWSInstance_withIamInstanceProfile
--- PASS: TestAccAWSInstance_withIamInstanceProfile (131.25s)
=== RUN   TestAccAWSInstance_privateIP
--- PASS: TestAccAWSInstance_privateIP (206.05s)
=== RUN   TestAccAWSInstance_associatePublicIPAndPrivateIP
--- PASS: TestAccAWSInstance_associatePublicIPAndPrivateIP (93.22s)
=== RUN   TestAccAWSInstance_keyPairCheck
--- PASS: TestAccAWSInstance_keyPairCheck (176.10s)
=== RUN   TestAccAWSInstance_rootBlockDeviceMismatch
--- PASS: TestAccAWSInstance_rootBlockDeviceMismatch (99.94s)
=== RUN   TestAccAWSInstance_forceNewAndTagsDrift
--- PASS: TestAccAWSInstance_forceNewAndTagsDrift (264.61s)
=== RUN   TestAccAWSInstance_changeInstanceType
--- PASS: TestAccAWSInstance_changeInstanceType (116.70s)
=== RUN   TestAccAWSInstance_primaryNetworkInterface
--- PASS: TestAccAWSInstance_primaryNetworkInterface (226.93s)
=== RUN   TestAccAWSInstance_primaryNetworkInterfaceSourceDestCheck
--- PASS: TestAccAWSInstance_primaryNetworkInterfaceSourceDestCheck (144.18s)
=== RUN   TestAccAWSInstance_addSecondaryInterface
--- PASS: TestAccAWSInstance_addSecondaryInterface (147.57s)
=== RUN   TestAccAWSInstance_addSecurityGroupNetworkInterface
--- PASS: TestAccAWSInstance_addSecurityGroupNetworkInterface (284.84s)
=== RUN   TestAccAWSInstance_associatePublic_defaultPrivate
--- PASS: TestAccAWSInstance_associatePublic_defaultPrivate (99.39s)
=== RUN   TestAccAWSInstance_associatePublic_defaultPublic
--- PASS: TestAccAWSInstance_associatePublic_defaultPublic (196.88s)
=== RUN   TestAccAWSInstance_associatePublic_explicitPublic
--- PASS: TestAccAWSInstance_associatePublic_explicitPublic (223.46s)
=== RUN   TestAccAWSInstance_associatePublic_explicitPrivate
--- PASS: TestAccAWSInstance_associatePublic_explicitPrivate (131.60s)
=== RUN   TestAccAWSInstance_associatePublic_overridePublic
--- PASS: TestAccAWSInstance_associatePublic_overridePublic (223.39s)
=== RUN   TestAccAWSInstance_associatePublic_overridePrivate
--- PASS: TestAccAWSInstance_associatePublic_overridePrivate (207.08s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       5173.635s

@catsby
Copy link
Contributor

catsby commented Aug 7, 2017

I do not believe this covers launch configurations however, but this looks good to merge for instances.

Previously we had no way of determining if a boolean attribute was a
literal `false` or a zero-value type of `false`. With the core enhancement
added to Terraform via hashicorp/terraform#15723,
we can now reliably check for a zero-type value of a boolean attribute,
or a literal boolean `false` inside of schema.

This allows the `associate_public_ip_address` to work as expected,
when configuring an AWS Instance inside of a subnet that defaults to
public address assignment, with a literal `false` for the `associate_public_ip_address` attribute.

```
$ make testacc TEST=./aws TESTARGS="-run=TestAccAWSInstance_associatePublic_"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstance_associatePublic_ -timeout 120m
=== RUN   TestAccAWSInstance_associatePublic_defaultPrivate
--- PASS: TestAccAWSInstance_associatePublic_defaultPrivate (106.54s)
=== RUN   TestAccAWSInstance_associatePublic_defaultPublic
--- PASS: TestAccAWSInstance_associatePublic_defaultPublic (215.38s)
=== RUN   TestAccAWSInstance_associatePublic_explicitPublic
--- PASS: TestAccAWSInstance_associatePublic_explicitPublic (112.23s)
=== RUN   TestAccAWSInstance_associatePublic_explicitPrivate
--- PASS: TestAccAWSInstance_associatePublic_explicitPrivate (224.96s)
=== RUN   TestAccAWSInstance_associatePublic_overridePublic
--- PASS: TestAccAWSInstance_associatePublic_overridePublic (117.78s)
=== RUN   TestAccAWSInstance_associatePublic_overridePrivate
--- PASS: TestAccAWSInstance_associatePublic_overridePrivate (112.48s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	889.375s
```
Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

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

👍

@grubernaut grubernaut merged commit 1e91718 into master Aug 8, 2017
@grubernaut grubernaut deleted the b-fix-aws-instance-assign-ip branch August 8, 2017 22:19
nbaztec pushed a commit to nbaztec/terraform-provider-aws that referenced this pull request Sep 26, 2017
…instance-assign-ip

r/aws_instance: Fix associate_public_ip_address
@ghost
Copy link

ghost commented Apr 11, 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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_instance associate_public_ip_address
3 participants