search

cloudposse / terraform-aws-efs-backup

Terraform module designed to easily backup EFS filesystems to S3 using DataPipeline
https://cloudposse.com/accelerate
Apache License 2.0
43 stars 33 forks source link

Remove -resource-role tag from resource role #31

Closed josephchoe closed 6 years ago

josephchoe commented 6 years ago

what

why

references

osterman commented 6 years ago

@aknysh can you take a look at this on monday to make sure we're not breaking anything?

aknysh commented 6 years ago

thanks @josephchoe Looks good and I'll approve, but one comment:

When used from the terraform-aws-jenking module, the change will always work, since the jenkins module adds its own attributes efs-backup to the module: {namespace}-{environment}-{name}-{attributes}-efs-backup-resource-role

when used on its own (and when var.attributes are not provided), there is a possibility of naming collisions with other resources from the same project. For example, 

module "resource_role_label" {
  source     = "git::https://github.com/cloudposse/terraform-null-label.git?ref=tags/0.3.1"
  namespace  = "${var.namespace}"
  stage      = "${var.stage}"
  name       = "${var.name}"
  delimiter  = "${var.delimiter}"
  attributes = ["${compact(concat(var.attributes, list("resource-role")))}"]
  tags       = "${var.tags}"
}

resource "aws_iam_role" "resource_role" {
  name               = "${module.resource_role_label.id}"
  assume_role_policy = "${data.aws_iam_policy_document.resource_role.json}"
}

If we remove resource-role from the attributes, there is a high probability that we already have aws_iam_role with the same name in the AWS account ({namespace}-{environment}-{name})

The same applies for the instance profile

resource "aws_iam_instance_profile" "resource_role" {
  name = "${module.resource_role_label.id}"
  role = "${aws_iam_role.resource_role.name}"
}