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

provider/aws: read instance source_dest_check and save to state #3152

Merged
merged 1 commit into from
Sep 16, 2015
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions builtin/providers/aws/resource_aws_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,9 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error {
d.Set("subnet_id", instance.SubnetId)
}
d.Set("ebs_optimized", instance.EbsOptimized)
if instance.SubnetId != nil && *instance.SubnetId != "" {
d.Set("source_dest_check", instance.SourceDestCheck)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we forcefully set it to true if there isn't a subnet, rather than just letting it stick on whatever it's already set to? Doesn't really matter I guess, since the Update path won't do anything with it, but it could avoid leaving a misleading value in the state.

Alternatively the path in Update could return an error if source_dest_check is set to false when subnet isn't set, to make it more obvious that this combination is invalid.

Copy link
Contributor

Choose a reason for hiding this comment

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

Pulling this in as is, but is there a reason not to just read whatever it is without the guard?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@phinze Since the attribute only applies in VPCs, I figured it was better to only set it for those instances and keep it out of the state file for non-VPC instances.

}

if instance.Monitoring != nil && instance.Monitoring.State != nil {
monitoringState := *instance.Monitoring.State
Expand Down