Terraform: aws_instance requires ModifyInstanceAttribute due to changes in SourceDestCheck

[sethvargo]

Hi there,

Terraform Version

0.7.8

Affected Resource(s)

• aws_instance

Terraform Configuration Files

resource "aws_instance" "web" {
  subnet_id = "subnet-abcd1234"
}

Expected Behavior

Terraform should not require ModifyInstanceAttribute to change the SourceDestCheck field when a subnet is given.

Actual Behavior

Terraform requires SourceDestCheck.

Steps to Reproduce

1. terraform apply

Important Factoids

The root of the issue seems to be this line in the commit, which explicitly forces a sourcedestcheck update if we are a newresource.

References

• GH-8450
• GH-3550
• 338aab9

[stack72]

Hi @sethvargo

Sorry for the issue here - I have spent the morning looking into this and I cannot see how the change in https://github.com/hashicorp/terraform/commit/338aab9169fc1d62d9668f5eaaa910aa08fd864e has caused the issue.

Before the change:

Create of the AWS instance would call the Update func and we wouldn't call ModifyInstanceAttribute
Then subsequent changes to any value in the terraform config for AWS instance would trigger a call to the ModifyInstanceAttribute API

All the changes in 338aab9169fc1d62d9668f5eaaa910aa08fd864e do are to only call the ModifyInstanceAttribute the first time an instance has been created - to set True or False, then to only make the subsequent call when that specific param needed to change

Do you think we should skip the setting of source_dest_check by default?

Paul

[sethvargo]

Hey @stack72

Thank you for the reply. I briefly looked and tracked down the commit to 338aab9, but it's possible it could have been changed elsewhere.

What seems to be happening is that we force the source_dest_check to be set if the resource is new, but that forces a ModifyInstanceAttribute call, even if the value is already correct.

I think we shouldn't skip setting it, but we should skip setting it if its the default.

[stack72]

let me play around with this a bit and get back to you with some findings...

[stack72]

Hi @sethvargo

So I found the following when I removed the d.IsNewResource() from the update:

provider "aws" {
  region = "us-west-2"
}

resource "aws_vpc" "foo" {
    cidr_block = "10.70.0.0/16"
}

resource "aws_subnet" "foo" {
    cidr_block = "10.70.1.0/24"
    vpc_id = "${aws_vpc.foo.id}"
}

resource "aws_instance" "foo" {
    # us-west-2
    ami = "ami-4fccb37f"
    instance_type = "m1.small"
    subnet_id = "${aws_subnet.foo.id}"
    source_dest_check = false
}

This gave me the following output:

% terraform show
[WARN] /Users/stacko/Code/go/bin/terraform-provider-aws overrides an internal plugin for aws-provider.
  If you did not expect to see this message you will need to remove the old plugin.
  See https://www.terraform.io/docs/internals/internal-plugins.html
aws_instance.foo:
  id = i-30bac128
  ami = ami-4fccb37f
  associate_public_ip_address = false
  availability_zone = us-west-2b
  disable_api_termination = false
  ebs_block_device.# = 0
  ebs_optimized = false
  ephemeral_block_device.# = 0
  iam_instance_profile =
  instance_state = running
  instance_type = m1.small
  key_name =
  monitoring = false
  network_interface_id = eni-78e96006
  private_dns = ip-10-70-1-39.us-west-2.compute.internal
  private_ip = 10.70.1.39
  public_dns =
  public_ip =
  root_block_device.# = 0
  security_groups.# = 0
  source_dest_check = true
  subnet_id = subnet-74a0be10
  tags.% = 0
  tenancy = default
  vpc_security_group_ids.# = 1
  vpc_security_group_ids.2767469275 = sg-39235940

Notice source_dest_check = true even though I set it to source_dest_check = false in the config... this is because we are not passing the d.HasChange("source_dest_check") - this is why d.IsNewResource() was set into the check. We set that value by default.

this means we would need 2 terraform applies by default to roll out source_dest_check = false on a new instance created by terraform. This feels like it isn't the best thing to do here either

~ aws_instance.foo
    source_dest_check: "true" => "false"


Plan: 0 to add, 1 to change, 0 to destroy.

So rock || me || hardplace on what we should do here...

P.

[sethvargo]

Hmm. I'm a bit confused. Why does d.HasChange("source_dest_check") return false if you changed the value from true => false? Or is false the default value? Because if false is the default, the reason you're seeing this odd behavior is that tf's defaults are different from aws'.

[stack72]

@sethvargo

The Create func calls the Update func. So we create an aws_instance with source_dest_check = false - as we have specified source_dest_check in the config, we would usually need to make a ModifyCall to set this value. As we only have d.HasChange("source_dest_check"), there is no actual change detected in the schema as this hasn't been set to the schema

Therefore, we create the instance with the AWS default - which is true.

We would need another TF apply to set it to false

[sethvargo]

@stack72

Right, but if source_dest_check is _true_ (which is the default), you _still_ make that call to update the instance, at least that is the behavior I am seeing.

[hkjn]

FWIW, this causes sporadic 403s when building new aws_instance resources given that our IAM permissions disallows ec2:ModifyInstanceAttribute, which in turn causes TF to assume that the apply failed and causes it to not refresh .tfstate, despite the EC2 instance being built and next plan to show a clean diff.

We can workaround this by having some downstream resource which depends on the aws_instance, which introduces a diff during next plan and does cause the .tfstate to refresh correctly to pick up the private IP &c of the instance, but the workarounds due to this bug are causing some friction for us.

We might be able to contribute patches to help here, but it would be good for us to understand more about why this call is even necessary, since there's no actual diff when the call fails.

[david-resnick]

I'm still encountering this on 0.9.9. The describe response shows sourceDestCheck as true on a new instance, but terraform still tries setting it. This fails in my environment because ModifyInstanceAttribute is disallowed.

[grubernaut]

Sorry about the still lingering issue for you @david-resnick! I'll take a look later today and see if I can reproduce.