Skip to content

Commit

Permalink
Fix deletion of inline role policies (#4045)
Browse files Browse the repository at this point in the history
In #3587 we introduced
auto-naming for the inline role policies and added filtering of the
inputs within a transformation. This didn't take the special case of
removing all inline policies into consideration, which requires an empty
object as the delete marker.

Fixes #4031
  • Loading branch information
flostadler authored Jun 11, 2024
1 parent 37cbce8 commit f9d0b01
Show file tree
Hide file tree
Showing 7 changed files with 149 additions and 3 deletions.
6 changes: 5 additions & 1 deletion examples/examples_nodejs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,11 @@ func TestRoleInlinePolicyAutoName(t *testing.T) {

policyEmpty := res.Outputs["inlinePolicyEmpty"]

require.Equal(t, policyEmpty.Value, []interface{}{})
// Check that the delete marker is present
require.Len(t, policyEmpty.Value, 1)
deleteMarker := policyEmpty.Value.([]interface{})[0]
require.Empty(t, deleteMarker.(map[string]interface{})["policy"])

require.Regexp(t, regexp.MustCompile("testrole-*"), inlinePolicy.Name)
require.JSONEq(t, `{"Version": "2012-10-17", "Statement": [{"Effect": "Allow", "Action": "s3:GetObject", "Resource": "*" }]}`, inlinePolicy.Policy)
}
Expand Down
55 changes: 54 additions & 1 deletion examples/examples_py_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
package examples

import (
"context"
"os"
"path/filepath"
"testing"

"github.com/aws/aws-sdk-go-v2/config"
iamsdk "github.com/aws/aws-sdk-go-v2/service/iam"
"github.com/pulumi/pulumi/pkg/v3/testing/integration"
"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -70,13 +74,49 @@ func TestSecretManagerPy(t *testing.T) {
func TestRegress3905(t *testing.T) {
test := getPythonBaseOptions(t).
With(integration.ProgramTestOptions{
Dir: filepath.Join(getCwd(t), "regress-3905"),
Dir: filepath.Join(getCwd(t), "regress-3905"),
ExpectRefreshChanges: true, // JobDefinition.retry_strategy is suffering from a perma diff if the dict is empty. This is caused by the upstream provider ignoring empty object types
})

integration.ProgramTest(t, &test)
}

func TestRegress4031(t *testing.T) {
iamClient := configureIAMClient(t)
test := getPythonBaseOptions(t).
With(integration.ProgramTestOptions{
Dir: filepath.Join(getCwd(t), "regress-4031"),
ExtraRuntimeValidation: func(t *testing.T, stack integration.RuntimeValidationStackInfo) {
roleName := stack.Outputs["roleName"].(string)
rolePolicies, err := iamClient.ListRolePolicies(context.TODO(), &iamsdk.ListRolePoliciesInput{
RoleName: &roleName,
})

assert.NoError(t, err, "failed to get policies of role %s", roleName)
// Should have two inline policies called "bucket1" and "bucket2"
assert.Contains(t, rolePolicies.PolicyNames, "bucket1")
assert.Contains(t, rolePolicies.PolicyNames, "bucket2")
},
EditDirs: []integration.EditDir{
{
Dir: filepath.Join(getCwd(t), "regress-4031", "step1"),
Additive: true,
ExtraRuntimeValidation: func(t *testing.T, stack integration.RuntimeValidationStackInfo) {
roleName := stack.Outputs["roleName"].(string)
rolePolicies, err := iamClient.ListRolePolicies(context.TODO(), &iamsdk.ListRolePoliciesInput{
RoleName: &roleName,
})

assert.NoError(t, err, "failed to get policies of role %s", roleName)
assert.Empty(t, rolePolicies.PolicyNames)
},
},
},
})

integration.ProgramTest(t, &test)
}

func getPythonBaseOptions(t *testing.T) integration.ProgramTestOptions {
envRegion := getEnvRegion(t)
pythonBase := integration.ProgramTestOptions{
Expand All @@ -90,3 +130,16 @@ func getPythonBaseOptions(t *testing.T) integration.ProgramTestOptions {

return pythonBase
}

func configureIAMClient(t *testing.T) *iamsdk.Client {
loadOpts := []func(*config.LoadOptions) error{}
if p, ok := os.LookupEnv("AWS_PROFILE"); ok {
loadOpts = append(loadOpts, config.WithSharedConfigProfile(p))
}
if r, ok := os.LookupEnv("AWS_REGION"); ok {
loadOpts = append(loadOpts, config.WithRegion(r))
}
cfg, err := config.LoadDefaultConfig(context.TODO(), loadOpts...)
assert.NoErrorf(t, err, "failed to load AWS config")
return iamsdk.NewFromConfig(cfg)
}
10 changes: 10 additions & 0 deletions examples/regress-4031/Pulumi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
name: regress-4031
runtime:
name: python
options:
virtualenv: venv
description: A minimal AWS Python Pulumi program to reproduce regression 4031
config:
pulumi:tags:
value:
pulumi:template: ""
46 changes: 46 additions & 0 deletions examples/regress-4031/__main__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import pulumi
import pulumi_aws


role = pulumi_aws.iam.Role(
"test-role",
assume_role_policy=pulumi_aws.iam.get_policy_document(
statements=[
pulumi_aws.iam.GetPolicyDocumentStatementArgs(
actions=["sts:AssumeRole"],
principals=[
pulumi_aws.iam.GetPolicyDocumentStatementPrincipalArgs(
type="Service",
identifiers=["ecs-tasks.amazonaws.com"],
),
],
),
],
).json,
inline_policies=[
pulumi_aws.iam.RoleInlinePolicyArgs(
name="bucket1",
policy=pulumi_aws.iam.get_policy_document(
statements=[
pulumi_aws.iam.GetPolicyDocumentStatementArgs(
actions=["s3:GetObject"],
resources=["arn:aws:s3:::bucket1/*"],
),
],
).json,
),
pulumi_aws.iam.RoleInlinePolicyArgs(
name="bucket2",
policy=pulumi_aws.iam.get_policy_document(
statements=[
pulumi_aws.iam.GetPolicyDocumentStatementArgs(
actions=["s3:GetObject"],
resources=["arn:aws:s3:::bucket2/*"],
),
],
).json,
),
],
)

pulumi.export("roleName", role.name)
2 changes: 2 additions & 0 deletions examples/regress-4031/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pulumi>=3.0.0,<4.0.0
pulumi-aws>=6.0.0,<7.0.0
23 changes: 23 additions & 0 deletions examples/regress-4031/step1/__main__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import pulumi
import pulumi_aws


role = pulumi_aws.iam.Role(
"test-role",
assume_role_policy=pulumi_aws.iam.get_policy_document(
statements=[
pulumi_aws.iam.GetPolicyDocumentStatementArgs(
actions=["sts:AssumeRole"],
principals=[
pulumi_aws.iam.GetPolicyDocumentStatementPrincipalArgs(
type="Service",
identifiers=["ecs-tasks.amazonaws.com"],
),
],
),
],
).json,
inline_policies=[pulumi_aws.iam.RoleInlinePolicyArgs()]
)

pulumi.export("roleName", role.name)
10 changes: 9 additions & 1 deletion provider/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -2495,7 +2495,9 @@ func ProviderFromMeta(metaInfo *tfbridge.MetadataInfo) *tfbridge.ProviderInfo {
// inlinePolicies: []
// or a list with empty objects
// inlinePolicies: [{}]
// In both cases the role will be created _without_ any inline policies attached.
// If an empty list is provided for inline policies then the provider will not manage any inline
// policies in this resource.
// Providing a list with an empty object will cause the provider to remove all inline policies
// If a policy is provided, then both the `policy` and the `name` fields are required.
// If one is provided and the other is not, then no error will be thrown and no inline policy
// will be created.
Expand All @@ -2505,8 +2507,14 @@ func ProviderFromMeta(metaInfo *tfbridge.MetadataInfo) *tfbridge.ProviderInfo {
for _, value := range pv.ArrayValue() {
if value.IsObject() {
policy := value.ObjectValue()
// Filter out policies with no policy document. This can happens due to the auto-naming
// of the inline policy. It is not valid and will end up with no policy being created in AWS, but will show
// a perpetual diff which is confusing to users.
// An absent policy document is the one exception because it is the delete marker.
if policy.HasValue("policy") && policy["policy"].IsString() && policy["policy"].StringValue() != "" {
inlinePolicy = append(inlinePolicy, value)
} else if !policy.HasValue("policy") || policy["policy"].IsNull() {
inlinePolicy = append(inlinePolicy, value)
}
}
}
Expand Down

0 comments on commit f9d0b01

Please sign in to comment.