Fix: AWS EC2 volume tag errors

[TomerHeber]

resolves #190

Does not add tags to volume_tags if root_block_device block is used. (Per documentation they conflict).
I'm now also tagging root_block_device. I hope it makes sense.

[TomerHeber]

if the block does not exist - tag volume_tags as usual...

[yaronya]

Can you please also add the volume_tags block here and make sure it's not altered?

[TomerHeber]

You can't... it will create a confilct.
volume_tags should have been added, but wasn't added because root_block_device exist.

[TomerHeber]

@yaronya - based on the documentation, I've made larger changes. (it's less straighforward than I initially assumed).
Please recheck.

See the tag guide:
https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/instance#tag-guide

[terenho-jobber]

I suspect we have a similar problem with ebs_block_device too

[TomerHeber]

0..1

[TomerHeber]

0..*

[TomerHeber]

for swapping back in line 66.

[TomerHeber]

@yaronya - please review the input vs expected files.
Need to make sure it's correct, and no missing use-cases.

(tofu plan is successful).

[chpl]

Hey @TomerHeber
we think that the logic should be as following

• if there's volume_tags (even if there's no root_block_device or ebs_block_device)
  • add tags to it
• else
  • if there's any ebs_block_device add tags to it
  • add tags to the root_block_device even if it's not exists

[chpl]

from this

resource "aws_instance" "test" {
  ami           = "ami-0f403e3180720dd7e"
  instance_type = "t3a.nano"
  volume_tags = {
    "c" = "d"
  }
  root_block_device {
    volume_type = "gp3"
  }
  ebs_block_device {
    device_name = "/dev/xvdb"
    volume_type = "gp3"
  }
}

to this

resource "aws_instance" "test" {
  ami           = "ami-0f403e3180720dd7e"
  instance_type = "t3a.nano"
  volume_tags = merge({
    "c" = "d"
  }, local.terratag_added_main)
  root_block_device {
    volume_type = "gp3"
  }
  ebs_block_device {
    device_name = "/dev/xvdb"
    volume_type = "gp3"
  }
}

[chpl]

from this

resource "aws_instance" "test" {
  ami           = "ami-0f403e3180720dd7e"
  instance_type = "t3a.nano"
  ebs_block_device {
    device_name = "/dev/xvdb"
    volume_type = "gp3"
  }
}

to this

resource "aws_instance" "test" {
  ami           = "ami-0f403e3180720dd7e"
  instance_type = "t3a.nano"
  root_block_device {
    tags = local.terratag_added_main
  }
  ebs_block_device {
    device_name = "/dev/xvdb"
    volume_type = "gp3"
    tags = local.terratag_added_main
  }
}

[TomerHeber]

that makes sense.
I'll update the code. Thanks!

[RLRabinowitz]

Tiny note - Looking at the note of volume_tags, there might be some inconsistent behaviour if the user is adding tags of his own to attached aws_ebs_volume.

Might be something worth considering. I don't think we can really tell if there's an attached aws_ebs_volume, so maybe adding volume_tags here is good enough

[TomerHeber]

Tiny note - Looking at the note of volume_tags, there might be some inconsistent behaviour if the user is adding tags of his own to attached aws_ebs_volume.

Might be something worth considering. I don't think we can really tell if there's an attached aws_ebs_volume, so maybe adding volume_tags here is good enough

Yeah - I saw it as well. I guess the user will have to fix it manually. (?)

[RLRabinowitz]

Hey @TomerHeber , we've discussed the implications of this fix. Please see @chpl 's comments here, here and here

This way:

• We will not add volume_tags when they are not intended (When they would conflict with the tags of a root_block_device or an ebs_block_device, or when there's an attached volume to the instance)
• We will add tags to the volumes whether or not volume_tags are defined (ebs_block_device and root_block_device will get the tags from this fix, other aws_ebs_volumes will get the tags from terratag itself)

[chpl]

@RLRabinowitz I commented here