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

[ISSUE] MalformedPolicyDocument trying to use databricks_aws_crossaccount_policy #688

Closed
iandelahorne opened this issue Jun 14, 2021 · 7 comments · Fixed by #756
Closed
Labels
aws Occurring on AWS cloud
Milestone

Comments

@iandelahorne
Copy link

Terraform Version

Terraform v0.14.11 and Terraform v0.13.7
AWS provider 3.45

Affected Resource(s)

  • databricks_aws_crossaccount_policy

Terraform Configuration Files

 data "databricks_aws_crossaccount_policy" "databricks" {
   pass_roles = [aws_iam_role.databricks_compute.arn]
 }

 resource "aws_iam_policy" "databricks_management" {
   name   = var.databricks_management_policy_name
   policy = data.databricks_aws_crossaccount_policy.databricks.json
}

Debug Output

Plan output:

  # aws_iam_policy.databricks_management will be created
  + resource "aws_iam_policy" "databricks_management" {
      + arn       = (known after apply)
      + id        = (known after apply)
      + name      = "databricks"
      + path      = "/"
      + policy    = jsonencode(
            {
              + Statement = [
                  + {
                      + Action   = [
                          + "ec2:AssociateDhcpOptions",
                          + "ec2:AssociateIamInstanceProfile",
                          + "ec2:AssociateRouteTable",
                          + "ec2:AttachInternetGateway",
                          + "ec2:AttachVolume",
                          + "ec2:AuthorizeSecurityGroupEgress",
                          + "ec2:AuthorizeSecurityGroupIngress",
                          + "ec2:CancelSpotInstanceRequests",
                          + "ec2:CreateDhcpOptions",
                          + "ec2:CreateInternetGateway",
                          + "ec2:CreateKeyPair",
                          + "ec2:CreateRoute",
                          + "ec2:CreateSecurityGroup",
                          + "ec2:CreateSubnet",
                          + "ec2:CreateTags",
                          + "ec2:CreateVolume",
                          + "ec2:CreateVpc",
                          + "ec2:DeleteInternetGateway",
                          + "ec2:DeleteKeyPair",
                          + "ec2:DeleteRoute",
                          + "ec2:DeleteRouteTable",
                          + "ec2:DeleteSecurityGroup",
                          + "ec2:DeleteSubnet",
                          + "ec2:DeleteTags",
                          + "ec2:DeleteVolume",
                          + "ec2:DeleteVpc",
                          + "ec2:DescribeAvailabilityZones",
                          + "ec2:DescribeNetworkAcls",
                          + "ec2:DescribeInternetGateways",
                          + "ec2:DescribeVpcAttribute",
                          + "ec2:DescribeIamInstanceProfileAssociations",
                          + "ec2:DescribeInstanceStatus",
                          + "ec2:DescribeInstances",
                          + "ec2:DescribePrefixLists",
                          + "ec2:DescribeReservedInstancesOfferings",
                          + "ec2:DescribeRouteTables",
                          + "ec2:DescribeSecurityGroups",
                          + "ec2:DescribeSpotInstanceRequests",
                          + "ec2:DescribeSpotPriceHistory",
                          + "ec2:DescribeSubnets",
                          + "ec2:DescribeVolumes",
                          + "ec2:DescribeVpcs",
                          + "ec2:DetachInternetGateway",
                          + "ec2:DisassociateIamInstanceProfile",
                          + "ec2:ModifyVpcAttribute",
                          + "ec2:ReplaceIamInstanceProfileAssociation",
                          + "ec2:RequestSpotInstances",
                          + "ec2:RevokeSecurityGroupEgress",
                          + "ec2:RevokeSecurityGroupIngress",
                          + "ec2:RunInstances",
                          + "ec2:TerminateInstances",
                          + "ec2:CreatePlacementGroup",
                          + "ec2:DeletePlacementGroup",
                          + "ec2:DescribePlacementGroups",
                          + "ec2:AllocateAddress",
                          + "ec2:CreateNatGateway",
                          + "ec2:CreateRouteTable",
                          + "ec2:CreateVpcEndpoint",
                          + "ec2:DeleteDhcpOptions",
                          + "ec2:DeleteNatGateway",
                          + "ec2:DeleteVpcEndpoints",
                          + "ec2:DescribeNatGateways",
                          + "ec2:DisassociateRouteTable",
                          + "ec2:ReleaseAddress",
                          + "ec2:DetachVolume",
                        ]
                      + Effect   = "Allow"
                      + Resource = "*"
                    },
                  + {
                      + Action    = [
                          + "iam:CreateServiceLinkedRole",
                          + "iam:PutRolePolicy",
                        ]
                      + Condition = {
                          + StringLike = {
                              + iam:AWSServiceName = "spot.amazonaws.com"
                            }
                        }
                      + Effect    = "Allow"
                      + Resource  = "arn:aws:iam::*:role/aws-service-role/spot.amazonaws.com/AWSServiceRoleForEC2Spot"
                    },
                  + {
                      + Action   = "iam:PassRole"
                      + Effect   = "Allow"
                      + Resource = [
                          + "arn:aws:iam::REDACTED:role/databricks_compute",
                        ]
                    },
                ]
              + Version   = "2008-10-17"
            }
        )
      + policy_id = (known after apply)
      + tags_all  = (known after apply)
    }

Expected Behavior

An IAM policy to be created

Actual Behavior

The provider errored with:

Error: error creating IAM policy databricks: MalformedPolicyDocument: Policy document must be version 2012-10-17 or greater.

This is due to the IAM policy document version being set to 2008-10-17 in data_aws_policies.go

Steps to Reproduce

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

  1. Create a minimal terraform configuration creating an AWS policy
  2. Attempt to terraform apply
@nfx
Copy link
Contributor

nfx commented Jun 16, 2021

Hi! @iandelahorne did you try creating an inline policy?

data "databricks_aws_assume_role_policy" "this" {
external_id = var.databricks_account_id
}

resource "aws_iam_role" "cross_account_role" {
name = "${local.prefix}-crossaccount"
assume_role_policy = data.databricks_aws_assume_role_policy.this.json
tags = var.tags
}

data "databricks_aws_crossaccount_policy" "this" {
}

resource "aws_iam_role_policy" "this" {
name = "${local.prefix}-policy"
role = aws_iam_role.cross_account_role.id
policy = data.databricks_aws_crossaccount_policy.this.json
}

@iandelahorne
Copy link
Author

Hi! @iandelahorne did you try creating an inline policy?

@nfx No, I didn't since our security policies / terraform best practices don't allow this.

@nfx
Copy link
Contributor

nfx commented Jun 17, 2021

@iandelahorne Where in TF best practices I can read that aws_iam_role_policy is not allowed?.. Would this example work for you? In the meantime, please use policy templates from official docs.

resource "aws_iam_role_policy" "this" {
name = "${local.prefix}-policy"
role = aws_iam_role.cross_account_role.id
policy = data.databricks_aws_crossaccount_policy.this.json
}

@iandelahorne
Copy link
Author

@nfx Sorry I was confusing this with a different check which is "no inline policies on users" which Checkov/Bridgecrew will definitely barf on.

That said: Our internal best practices at Patreon are to not use inline policies on roles with aws_iam_role_policy, but instead (for reusability) create an aws_iam_policy and attach it with aws_iam_role_policy_attachment.

Now, does inlining a policy work? Yes, it does with the current version. Here's my current example file I just tested with in terraform 0.13.7:

terraform {
  required_providers {
    databricks = {
      source  = "databrickslabs/databricks"
      version = "0.3.5"
    }
  }
}

locals {
  prefix = "databricks"
}

variable "databricks_account_id" {
  default = "databricks-account-id"
}

data "databricks_aws_assume_role_policy" "this" {
  external_id = var.databricks_account_id
}

resource "aws_iam_role" "cross_account_role" {
  name               = "${local.prefix}-crossaccount"
  assume_role_policy = data.databricks_aws_assume_role_policy.this.json
}

data "databricks_aws_crossaccount_policy" "this" {
}

resource "aws_iam_role_policy" "this" {
  name   = "${local.prefix}-policy"
  role   = aws_iam_role.cross_account_role.id
  policy = data.databricks_aws_crossaccount_policy.this.json
}

resource "aws_iam_policy" "databricks" {
  name   = "${local.prefix}-policy-standalone"
  policy = data.databricks_aws_crossaccount_policy.this.json
}

The aws_iam_role_policy works fine. aws_iam_policy with the same document does not - it fails with MalformedPolicyDocument.

Is there any reason why this can't just be bumped to 2012-10-07? I went in with the expectation that I could use this policy document in a policy resource - it certainly isn't documented that it only works with inline resources.

@nfx
Copy link
Contributor

nfx commented Jun 22, 2021

Hm... bumping the version might happen, but only in 0.4.0 - imagine the surprise of folks seeing their cross-account roles updated :)

@nfx
Copy link
Contributor

nfx commented Jul 8, 2021

@psg2 willing to help with this one?

@psg2
Copy link
Contributor

psg2 commented Jul 9, 2021

@psg2 willing to help with this one?

Yes, I can pick it by next week probably, this seems rather simple to fix. 😄

@nfx nfx added the aws Occurring on AWS cloud label Jul 19, 2021
@nfx nfx linked a pull request Aug 2, 2021 that will close this issue
@nfx nfx added this to the v0.3.7 milestone Aug 2, 2021
@nfx nfx closed this as completed in #756 Aug 2, 2021
michael-berk pushed a commit to michael-berk/terraform-provider-databricks that referenced this issue Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws Occurring on AWS cloud
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants