hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.61k stars 9k forks source link

`aws_iam_role_policy` should notify the user to use `id`s instead of `arn`s for the `role` parameter #3087

Open carlosonunez opened 6 years ago

carlosonunez commented 6 years ago

Terraform Version

Run terraform -v to show the version. If you are not running the latest version of Terraform, please upgrade because your issue may have already been fixed.

Terraform v0.11.1

Affected Resource(s)

Terraform Configuration Files

resource "aws_iam_role_policy" "execution_role_policy" {
  name = "ecs_execution_role_policy"
  role = "${aws_iam_role.execution_role.id}"
  policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": [
        "logs:*",
        "cloudwatch:*"
      ],
      "Effect": "Allow",
      "Resource": "*"
    }
  ]
}
EOF
}

Expected Behavior

Error: Error applying plan:

1 error(s) occurred:

* aws_ecs_task_definition.task: 1 error(s) occurred:

* aws_iam_role_policy.execution_role_policy: Error putting IAM role policy EcsExecutionRolePolicy: ValidationError:

You've provided an ARN. Please provide an ID instead.

Actual Behavior

Error: Error applying plan:

1 error(s) occurred:

* aws_ecs_task_definition.task: 1 error(s) occurred:

* aws_iam_role_policy.execution_role_policy: Error putting IAM role policy EcsExecutionRolePolicy: ValidationError:
 The specified value for roleName is invalid. It must contain only alphanumeric characters and/or the following: +=
,.@_-
        status code: 400, request id: 30e2d314-ff1f-11e7-81b8-31fe685c36f6

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. Create a tf from the code above.
  2. terraform apply
bflad commented 6 years ago

@carlosonunez currently the aws_iam_role_policy resource does not perform any plan-time validation for the role attribute and unfortunately AWS only allows role names. It would be fairly trivial to add it though, based on the validation already performed by aws_iam_role's name attribute.

Basically, we could move this to its own function: https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_iam_role.go#L44-L56

Then add ValidateFunc to the role attribute schema here: https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_iam_role_policy.go#L49-L53

For most of these validation cases we don't specifically try to determine if the implementor was trying to specify a certain type of value (e.g. ARN) since its harder to maintain, but rather supply the valid character/length restrictions.

eschwartz commented 5 years ago

Can we update the docs for the aws_iam_role_policy resource, to clarify what the role property should be?

Currently all it says is:

role - (Required) The IAM role to attach to the policy.

It would be more clear if it said:

role - (Required) The id of the IAM role to attach to the policy.

Though I'm also confused as to whether this is supposed to be role.id or role.name.

It would be great if it could support role.arn as well.

carlosonunez commented 5 years ago

@bflad That's an interesting approach that could work. I'll try having a go at it.

fr34k8 commented 5 years ago

@eschwartz @carlosonunez did you consider moving this into 1.33.0 ? if so this, would be awesome! 👏

carlosonunez commented 5 years ago

@fr34k8 unfortunately i have to put this on the back burner, as other work has been prioritized above this. Apologies for the inconvenience. I'll leave this issue open for others who feel like this is something that should be addressed but am fine with the mods closing this as wontfix, too.

davehowell commented 5 years ago

It could be a basic layer of protection to check, for example in the resource aws_iam_role_policy_attachment and anywhere else where required, that the role attribute is the role.name and not a role.arn. You could literally just enforce that the argument is {something}.name It seems like an inconsistency in Amazon AWS API that most of the time it needs arn but sometimes it needs to be name.

Actually validating the format of the role name or the arn is not necessary because that error is already returned from the API, the problem is confusion in the terraform resource. It's just called role so not obvious that this error message really means you've stuck an arn where a name should go.

The specified value for roleName is invalid. It must contain only alphanumeric characters and/or the following: +=,.@_-

genert commented 5 years ago

This information needs to be added to the documentation, until this this issue is resolved.

mohsen0 commented 4 years ago

if arn is available, this is a good workaround

resource "aws_iam_role_policy_attachment" "additional_permissions" {
  role = regex("[\\w|-]+$", module.lambda.role_arn)
  policy_arn = aws_iam_policy.additional_permissions_lambda.arn
}
holyjak commented 4 years ago

I find the error message really confusing, I believed it meant something wrong was with the name attribute of the aws_iam_role_policy resource. A more accurate error message would certainly be most helpful.

mickeypash commented 3 years ago

Hey is there a corresponding PR for this? You've said you've looked at it @carlosonunez I'm happy to help get it over the line.

kiddom-kq commented 2 years ago

@holyjak Same! I lost the better part of a full day trying to figure out why this error kept coming back:

Error putting IAM role policy cloudwatch-kinesis: ValidationError: The specified value for roleName is invalid. It must contain only alphanumeric characters and/or the following: +=,.@_-

I didn't understand as cloudwatch-kinesis is absolutely alphanumeric with the exception of - which is in the 'safe' character set: +=,.@_-