From c56bc9028885e588204aec574a2e55c5ff38cb3f Mon Sep 17 00:00:00 2001 From: Callum Dempsey Leach Date: Mon, 7 Oct 2024 12:25:40 +0100 Subject: [PATCH] [Fix] Add Sufficient Network Privileges to the Databricks Default Cross Account Policy (#4027) ## Changes Currently, the Databricks-provided Cross Account Policy IAM Role does not include all the necessary permissions to set up a workspace. Attempting to set up a workspace using this policy results in the following error (see [Issue #4026](https://github.com/databricks/terraform-provider-databricks/issues/4026)): ``` MALFORMED_REQUEST: Failed credentials validation checks: Allocate Address ``` This makes it difficult for new engineers to onboard to Databricks without troubleshooting unexpected errors. This PR adds the missing network permissions to the Databricks Managed VPC policy types ("managed" and "customer"), ensuring that all required permissions are included for successful workspace deployment. These changes are not applied to the "restricted" policy type to avoid allowing Elastic IP allocations, which may not be desirable for some Databricks customers. See the bottom of the description for the full list. ## Tests This change has been tested locally and is running in our staging workspace using the same configuration. As this is a fix for 'managed' type Databricks deployment configurations, I have matched this with positive and negative unit tests to guard precise and expected roles. I have then added extra tests to confirm the expected policies across each branch, 'managed', 'customer', and 'restricted'. Feel free to remove these if overboard, as I recognise you _could_ make a similar weaker assertion using 'len'. - [x] `make test` run locally - [x] Relevant acceptance tests are passing - [ ] Relevant change in `docs/` folder (if necessary) - [x] Covered with integration tests in `internal/acceptance` - [ ] Using Go SDK (N/A) The full list of permissions which align with the Databricks documentation, now included in the "managed" policy type, are: ```json [ "ec2:AllocateAddress", "ec2:AssignPrivateIpAddresses", "ec2:AssociateDhcpOptions", "ec2:AssociateIamInstanceProfile", "ec2:AssociateRouteTable", "ec2:AttachInternetGateway", "ec2:AttachVolume", "ec2:AuthorizeSecurityGroupEgress", "ec2:AuthorizeSecurityGroupIngress", "ec2:CancelSpotInstanceRequests", "ec2:CreateDhcpOptions", "ec2:CreateFleet", "ec2:CreateInternetGateway", "ec2:CreateLaunchTemplate", "ec2:CreateLaunchTemplateVersion", "ec2:CreateNatGateway", "ec2:CreateRoute", "ec2:CreateRouteTable", "ec2:CreateSecurityGroup", "ec2:CreateSubnet", "ec2:CreateTags", "ec2:CreateVolume", "ec2:CreateVpc", "ec2:CreateVpcEndpoint", "ec2:DeleteDhcpOptions", "ec2:DeleteFleets", "ec2:DeleteInternetGateway", "ec2:DeleteLaunchTemplate", "ec2:DeleteLaunchTemplateVersions", "ec2:DeleteNatGateway", "ec2:DeleteRoute", "ec2:DeleteRouteTable", "ec2:DeleteSecurityGroup", "ec2:DeleteSubnet", "ec2:DeleteTags", "ec2:DeleteVolume", "ec2:DeleteVpc", "ec2:DeleteVpcEndpoints", "ec2:DescribeAvailabilityZones", "ec2:DescribeFleetHistory", "ec2:DescribeFleetInstances", "ec2:DescribeFleets", "ec2:DescribeIamInstanceProfileAssociations", "ec2:DescribeInstanceStatus", "ec2:DescribeInstances", "ec2:DescribeInternetGateways", "ec2:DescribeLaunchTemplates", "ec2:DescribeLaunchTemplateVersions", "ec2:DescribeNatGateways", "ec2:DescribeNetworkAcls", "ec2:DescribePrefixLists", "ec2:DescribeReservedInstancesOfferings", "ec2:DescribeRouteTables", "ec2:DescribeSecurityGroups", "ec2:DescribeSpotInstanceRequests", "ec2:DescribeSpotPriceHistory", "ec2:DescribeSubnets", "ec2:DescribeVolumes", "ec2:DescribeVpcAttribute", "ec2:DescribeVpcs", "ec2:DetachInternetGateway", "ec2:DisassociateIamInstanceProfile", "ec2:DisassociateRouteTable", "ec2:GetLaunchTemplateData", "ec2:GetSpotPlacementScores", "ec2:ModifyFleet", "ec2:ModifyLaunchTemplate", "ec2:ModifyVpcAttribute", "ec2:ReleaseAddress", "ec2:ReplaceIamInstanceProfileAssociation", "ec2:RequestSpotInstances", "ec2:RevokeSecurityGroupEgress", "ec2:RevokeSecurityGroupIngress", "ec2:RunInstances", "ec2:TerminateInstances" ] ``` Resolves #4026 --- aws/data_aws_crossaccount_policy.go | 4 + aws/data_aws_crossaccount_policy_test.go | 471 ++++++++++++++++++++++- 2 files changed, 473 insertions(+), 2 deletions(-) diff --git a/aws/data_aws_crossaccount_policy.go b/aws/data_aws_crossaccount_policy.go index 6737e9376a..a5da5d9365 100644 --- a/aws/data_aws_crossaccount_policy.go +++ b/aws/data_aws_crossaccount_policy.go @@ -103,6 +103,10 @@ func DataAwsCrossaccountPolicy() common.Resource { // additional permissions for Databricks-managed VPC policy if data.PolicyType == "managed" { actions = append(actions, []string{ + "ec2:AttachInternetGateway", + "ec2:AllocateAddress", + "ec2:AssociateDhcpOptions", + "ec2:AssociateRouteTable", "ec2:CreateDhcpOptions", "ec2:CreateInternetGateway", "ec2:CreateNatGateway", diff --git a/aws/data_aws_crossaccount_policy_test.go b/aws/data_aws_crossaccount_policy_test.go index 2bdf183993..177cb166e9 100644 --- a/aws/data_aws_crossaccount_policy_test.go +++ b/aws/data_aws_crossaccount_policy_test.go @@ -16,7 +16,7 @@ func TestDataAwsCrossAccountDatabricksManagedPolicy(t *testing.T) { }.Apply(t) assert.NoError(t, err) j := d.Get("json") - assert.Lenf(t, j, 3032, "Strange length for policy: %s", j) + assert.Lenf(t, j, 3171, "Strange length for policy: %s", j) } func TestDataAwsCrossAccountCustomerManagedPolicy(t *testing.T) { @@ -42,7 +42,474 @@ func TestDataAwsCrossAccountPolicy_WithPassRoles(t *testing.T) { }.Apply(t) assert.NoError(t, err) j := d.Get("json") - assert.Lenf(t, j, 3168, "Strange length for policy: %s", j) + assert.Lenf(t, j, 3307, "Strange length for policy: %s", j) +} + +func TestDataAwsCrossAccountManagedPolicyRoles(t *testing.T) { + expectedJSON := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "ec2:AssignPrivateIpAddresses", + "ec2:CancelSpotInstanceRequests", + "ec2:DescribeAvailabilityZones", + "ec2:DescribeIamInstanceProfileAssociations", + "ec2:DescribeInstanceStatus", + "ec2:DescribeInstances", + "ec2:DescribeInternetGateways", + "ec2:DescribeNatGateways", + "ec2:DescribeNetworkAcls", + "ec2:DescribePrefixLists", + "ec2:DescribeReservedInstancesOfferings", + "ec2:DescribeRouteTables", + "ec2:DescribeSecurityGroups", + "ec2:DescribeSpotInstanceRequests", + "ec2:DescribeSpotPriceHistory", + "ec2:DescribeSubnets", + "ec2:DescribeVolumes", + "ec2:DescribeVpcAttribute", + "ec2:DescribeVpcs", + "ec2:CreateTags", + "ec2:DeleteTags", + "ec2:GetSpotPlacementScores", + "ec2:RequestSpotInstances", + "ec2:DescribeFleetHistory", + "ec2:ModifyFleet", + "ec2:DeleteFleets", + "ec2:DescribeFleetInstances", + "ec2:DescribeFleets", + "ec2:CreateFleet", + "ec2:DeleteLaunchTemplate", + "ec2:GetLaunchTemplateData", + "ec2:CreateLaunchTemplate", + "ec2:DescribeLaunchTemplates", + "ec2:DescribeLaunchTemplateVersions", + "ec2:ModifyLaunchTemplate", + "ec2:DeleteLaunchTemplateVersions", + "ec2:CreateLaunchTemplateVersion", + "ec2:AssociateIamInstanceProfile", + "ec2:AttachVolume", + "ec2:AuthorizeSecurityGroupEgress", + "ec2:AuthorizeSecurityGroupIngress", + "ec2:CreateVolume", + "ec2:DeleteVolume", + "ec2:DetachVolume", + "ec2:DisassociateIamInstanceProfile", + "ec2:ReplaceIamInstanceProfileAssociation", + "ec2:RevokeSecurityGroupEgress", + "ec2:RevokeSecurityGroupIngress", + "ec2:RunInstances", + "ec2:TerminateInstances", + "ec2:AttachInternetGateway", + "ec2:AllocateAddress", + "ec2:AssociateDhcpOptions", + "ec2:AssociateRouteTable", + "ec2:CreateDhcpOptions", + "ec2:CreateInternetGateway", + "ec2:CreateNatGateway", + "ec2:CreateRoute", + "ec2:CreateRouteTable", + "ec2:CreateSecurityGroup", + "ec2:CreateSubnet", + "ec2:CreateVpc", + "ec2:CreateVpcEndpoint", + "ec2:DeleteDhcpOptions", + "ec2:DeleteInternetGateway", + "ec2:DeleteNatGateway", + "ec2:DeleteRoute", + "ec2:DeleteRouteTable", + "ec2:DeleteSecurityGroup", + "ec2:DeleteSubnet", + "ec2:DeleteVpc", + "ec2:DeleteVpcEndpoints", + "ec2:DetachInternetGateway", + "ec2:DisassociateRouteTable", + "ec2:ModifyVpcAttribute", + "ec2:ReleaseAddress" + ], + "Resource": "*" + }, + { + "Effect": "Allow", + "Action": [ + "iam:CreateServiceLinkedRole", + "iam:PutRolePolicy" + ], + "Resource": "arn:aws:iam::*:role/aws-service-role/spot.amazonaws.com/AWSServiceRoleForEC2Spot", + "Condition": { + "StringLike": { + "iam:AWSServiceName": "spot.amazonaws.com" + } + } + } + ] +}` + + d, err := qa.ResourceFixture{ + Read: true, + Resource: DataAwsCrossaccountPolicy(), + NonWritable: true, + HCL: `policy_type = "managed"`, + ID: ".", + }.Apply(t) + assert.NoError(t, err) + actualJSON := d.Get("json").(string) + assert.Equal(t, expectedJSON, actualJSON) + + // Negative test: ensure that customer policy is not equal to customer policy + managedD, err := qa.ResourceFixture{ + Read: true, + Resource: DataAwsCrossaccountPolicy(), + NonWritable: true, + HCL: `policy_type = "customer"`, + ID: ".", + }.Apply(t) + assert.NoError(t, err) + managedJSON := managedD.Get("json").(string) + assert.NotEqual(t, actualJSON, managedJSON) +} + +func TestDataAwsCrossAccountCustomerManagedPolicyRoles(t *testing.T) { + expectedJSON := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "ec2:AssignPrivateIpAddresses", + "ec2:CancelSpotInstanceRequests", + "ec2:DescribeAvailabilityZones", + "ec2:DescribeIamInstanceProfileAssociations", + "ec2:DescribeInstanceStatus", + "ec2:DescribeInstances", + "ec2:DescribeInternetGateways", + "ec2:DescribeNatGateways", + "ec2:DescribeNetworkAcls", + "ec2:DescribePrefixLists", + "ec2:DescribeReservedInstancesOfferings", + "ec2:DescribeRouteTables", + "ec2:DescribeSecurityGroups", + "ec2:DescribeSpotInstanceRequests", + "ec2:DescribeSpotPriceHistory", + "ec2:DescribeSubnets", + "ec2:DescribeVolumes", + "ec2:DescribeVpcAttribute", + "ec2:DescribeVpcs", + "ec2:CreateTags", + "ec2:DeleteTags", + "ec2:GetSpotPlacementScores", + "ec2:RequestSpotInstances", + "ec2:DescribeFleetHistory", + "ec2:ModifyFleet", + "ec2:DeleteFleets", + "ec2:DescribeFleetInstances", + "ec2:DescribeFleets", + "ec2:CreateFleet", + "ec2:DeleteLaunchTemplate", + "ec2:GetLaunchTemplateData", + "ec2:CreateLaunchTemplate", + "ec2:DescribeLaunchTemplates", + "ec2:DescribeLaunchTemplateVersions", + "ec2:ModifyLaunchTemplate", + "ec2:DeleteLaunchTemplateVersions", + "ec2:CreateLaunchTemplateVersion", + "ec2:AssociateIamInstanceProfile", + "ec2:AttachVolume", + "ec2:AuthorizeSecurityGroupEgress", + "ec2:AuthorizeSecurityGroupIngress", + "ec2:CreateVolume", + "ec2:DeleteVolume", + "ec2:DetachVolume", + "ec2:DisassociateIamInstanceProfile", + "ec2:ReplaceIamInstanceProfileAssociation", + "ec2:RevokeSecurityGroupEgress", + "ec2:RevokeSecurityGroupIngress", + "ec2:RunInstances", + "ec2:TerminateInstances" + ], + "Resource": "*" + }, + { + "Effect": "Allow", + "Action": [ + "iam:CreateServiceLinkedRole", + "iam:PutRolePolicy" + ], + "Resource": "arn:aws:iam::*:role/aws-service-role/spot.amazonaws.com/AWSServiceRoleForEC2Spot", + "Condition": { + "StringLike": { + "iam:AWSServiceName": "spot.amazonaws.com" + } + } + } + ] +}` + + d, err := qa.ResourceFixture{ + Read: true, + Resource: DataAwsCrossaccountPolicy(), + NonWritable: true, + HCL: `policy_type = "customer"`, + ID: ".", + }.Apply(t) + assert.NoError(t, err) + actualJSON := d.Get("json").(string) + assert.Equal(t, expectedJSON, actualJSON) + + // Negative test: ensure that customer policy is not equal to managed policy + managedD, err := qa.ResourceFixture{ + Read: true, + Resource: DataAwsCrossaccountPolicy(), + NonWritable: true, + HCL: `policy_type = "managed"`, + ID: ".", + }.Apply(t) + assert.NoError(t, err) + managedJSON := managedD.Get("json").(string) + assert.NotEqual(t, actualJSON, managedJSON) +} + +func TestDataAwsCrossAccountRestrictedPolicyRoles(t *testing.T) { + expectedJSON := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "ec2:AssignPrivateIpAddresses", + "ec2:CancelSpotInstanceRequests", + "ec2:DescribeAvailabilityZones", + "ec2:DescribeIamInstanceProfileAssociations", + "ec2:DescribeInstanceStatus", + "ec2:DescribeInstances", + "ec2:DescribeInternetGateways", + "ec2:DescribeNatGateways", + "ec2:DescribeNetworkAcls", + "ec2:DescribePrefixLists", + "ec2:DescribeReservedInstancesOfferings", + "ec2:DescribeRouteTables", + "ec2:DescribeSecurityGroups", + "ec2:DescribeSpotInstanceRequests", + "ec2:DescribeSpotPriceHistory", + "ec2:DescribeSubnets", + "ec2:DescribeVolumes", + "ec2:DescribeVpcAttribute", + "ec2:DescribeVpcs", + "ec2:CreateTags", + "ec2:DeleteTags", + "ec2:GetSpotPlacementScores", + "ec2:RequestSpotInstances", + "ec2:DescribeFleetHistory", + "ec2:ModifyFleet", + "ec2:DeleteFleets", + "ec2:DescribeFleetInstances", + "ec2:DescribeFleets", + "ec2:CreateFleet", + "ec2:DeleteLaunchTemplate", + "ec2:GetLaunchTemplateData", + "ec2:CreateLaunchTemplate", + "ec2:DescribeLaunchTemplates", + "ec2:DescribeLaunchTemplateVersions", + "ec2:ModifyLaunchTemplate", + "ec2:DeleteLaunchTemplateVersions", + "ec2:CreateLaunchTemplateVersion" + ], + "Resource": "*" + }, + { + "Effect": "Allow", + "Action": [ + "iam:CreateServiceLinkedRole", + "iam:PutRolePolicy" + ], + "Resource": "arn:aws:iam::*:role/aws-service-role/spot.amazonaws.com/AWSServiceRoleForEC2Spot", + "Condition": { + "StringLike": { + "iam:AWSServiceName": "spot.amazonaws.com" + } + } + }, + { + "Sid": "InstancePoolsSupport", + "Effect": "Allow", + "Action": [ + "ec2:AssociateIamInstanceProfile", + "ec2:DisassociateIamInstanceProfile", + "ec2:ReplaceIamInstanceProfileAssociation" + ], + "Resource": "arn:aws:ec2:us-west-2:123456789012:instance/*", + "Condition": { + "StringEquals": { + "ec2:ResourceTag/Vendor": "Databricks" + } + } + }, + { + "Sid": "AllowEc2RunInstancePerTag", + "Effect": "Allow", + "Action": "ec2:RunInstances", + "Resource": [ + "arn:aws:ec2:us-west-2:123456789012:volume/*", + "arn:aws:ec2:us-west-2:123456789012:instance/*" + ], + "Condition": { + "StringEquals": { + "aws:RequestTag/Vendor": "Databricks" + } + } + }, + { + "Sid": "AllowEc2RunInstanceImagePerTag", + "Effect": "Allow", + "Action": "ec2:RunInstances", + "Resource": "arn:aws:ec2:us-west-2:123456789012:image/*", + "Condition": { + "StringEquals": { + "aws:ResourceTag/Vendor": "Databricks" + } + } + }, + { + "Sid": "AllowEc2RunInstancePerVPCid", + "Effect": "Allow", + "Action": "ec2:RunInstances", + "Resource": [ + "arn:aws:ec2:us-west-2:123456789012:network-interface/*", + "arn:aws:ec2:us-west-2:123456789012:subnet/*", + "arn:aws:ec2:us-west-2:123456789012:security-group/*" + ], + "Condition": { + "StringEquals": { + "ec2:vpc": "arn:aws:ec2:us-west-2:123456789012:vpc/vpc-abcdefg12345" + } + } + }, + { + "Sid": "AllowEc2RunInstanceOtherResources", + "Effect": "Allow", + "Action": "ec2:RunInstances", + "NotResource": [ + "arn:aws:ec2:us-west-2:123456789012:image/*", + "arn:aws:ec2:us-west-2:123456789012:network-interface/*", + "arn:aws:ec2:us-west-2:123456789012:subnet/*", + "arn:aws:ec2:us-west-2:123456789012:security-group/*", + "arn:aws:ec2:us-west-2:123456789012:volume/*", + "arn:aws:ec2:us-west-2:123456789012:instance/*" + ] + }, + { + "Sid": "EC2TerminateInstancesTag", + "Effect": "Allow", + "Action": "ec2:TerminateInstances", + "Resource": "arn:aws:ec2:us-west-2:123456789012:instance/*", + "Condition": { + "StringEquals": { + "ec2:ResourceTag/Vendor": "Databricks" + } + } + }, + { + "Sid": "EC2AttachDetachVolumeTag", + "Effect": "Allow", + "Action": [ + "ec2:AttachVolume", + "ec2:DetachVolume" + ], + "Resource": [ + "arn:aws:ec2:us-west-2:123456789012:instance/*", + "arn:aws:ec2:us-west-2:123456789012:volume/*" + ], + "Condition": { + "StringEquals": { + "ec2:ResourceTag/Vendor": "Databricks" + } + } + }, + { + "Sid": "EC2CreateVolumeByTag", + "Effect": "Allow", + "Action": "ec2:CreateVolume", + "Resource": "arn:aws:ec2:us-west-2:123456789012:volume/*", + "Condition": { + "StringEquals": { + "aws:RequestTag/Vendor": "Databricks" + } + } + }, + { + "Sid": "EC2DeleteVolumeByTag", + "Effect": "Allow", + "Action": "ec2:DeleteVolume", + "Resource": [ + "arn:aws:ec2:us-west-2:123456789012:volume/*" + ], + "Condition": { + "StringEquals": { + "ec2:ResourceTag/Vendor": "Databricks" + } + } + }, + { + "Sid": "VpcNonresourceSpecificActions", + "Effect": "Allow", + "Action": [ + "ec2:AuthorizeSecurityGroupEgress", + "ec2:AuthorizeSecurityGroupIngress", + "ec2:RevokeSecurityGroupEgress", + "ec2:RevokeSecurityGroupIngress" + ], + "Resource": "arn:aws:ec2:us-west-2:123456789012:security-group/sg-12345678", + "Condition": { + "StringEquals": { + "ec2:vpc": "arn:aws:ec2:us-west-2:123456789012:vpc/vpc-abcdefg12345" + } + } + } + ] +}` + + d, err := qa.ResourceFixture{ + Read: true, + Resource: DataAwsCrossaccountPolicy(), + NonWritable: true, + HCL: ` +policy_type = "restricted" +aws_account_id = "123456789012" +vpc_id = "vpc-abcdefg12345" +region = "us-west-2" +security_group_id = "sg-12345678" +`, + ID: ".", + }.Apply(t) + assert.NoError(t, err) + actualJSON := d.Get("json").(string) + assert.Equal(t, expectedJSON, actualJSON) + + // Negative test: ensure that restricted policy is not equal to managed policy + managedD, err := qa.ResourceFixture{ + Read: true, + Resource: DataAwsCrossaccountPolicy(), + NonWritable: true, + HCL: `policy_type = "managed"`, + ID: ".", + }.Apply(t) + assert.NoError(t, err) + managedJSON := managedD.Get("json").(string) + assert.NotEqual(t, actualJSON, managedJSON) + + // Negative test: ensure that restricted policy is not equal to customer policy + customerD, err := qa.ResourceFixture{ + Read: true, + Resource: DataAwsCrossaccountPolicy(), + NonWritable: true, + HCL: `policy_type = "customer"`, + ID: ".", + }.Apply(t) + assert.NoError(t, err) + customerJSON := customerD.Get("json").(string) + assert.NotEqual(t, actualJSON, customerJSON) } func TestDataAwsCrossAccountRestrictedPolicy(t *testing.T) {