-
Notifications
You must be signed in to change notification settings - Fork 156
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
Pulumi import or refresh adding managedPolicyArns to aws.iam.Role #2246
Comments
Hi @Fydon, thanks for the issue. I've raised this with the team so it can be prioritized appropriately and added to our work stack. |
Hello @roothorp, is there some news regarding this issue? |
Unfortunately this is still an issue with the latest: import * as pulumi from "@pulumi/pulumi";
import * as aws from "@pulumi/aws";
const lambdaRolePolicy = {
"Version": "2012-10-17",
"Statement": [
{
"Action": "sts:AssumeRole",
"Principal": {
"Service": "lambda.amazonaws.com",
},
"Effect": "Allow",
"Sid": "",
},
],
};
const role = new aws.iam.Role("my-role", {
assumeRolePolicy: JSON.stringify(lambdaRolePolicy),
});
const attachment = new aws.iam.RolePolicyAttachment("my-rpa", {
role: role,
policyArn: aws.iam.ManagedPolicy.LambdaFullAccess,
});
After calling pulumi up, pulumi refresh shows a non-empty diff and wants to add managedPolicyArns to the outputs of aws.iam.Role. If I let it proceed then subsequent
|
Noting that this issue also seems to affect Node overlays such as bucket.onObjectCreated, and most of the examples in the pulumi-aws repo, such as this one: https://github.com/pulumi/pulumi-aws/blob/master/examples/bucket/index.ts#L36 |
Also seeing this but if you run the CLI up or refresh commands enough, the Role Policy Attachment resource will get deleted and then recreated again. So with the following code: import * as aws from "@pulumi/aws";
const ec2Role = new aws.iam.Role("piers-role", {
assumeRolePolicy: aws.iam.assumeRolePolicyForPrincipal(aws.iam.Principals.Ec2Principal),
managedPolicyArns: [
"arn:aws:iam::aws:policy/AmazonEKS_CNI_Policy",
]
});
const rpa = new aws.iam.RolePolicyAttachment("piers-rpa", {
policyArn: "arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryReadOnly",
role: ec2Role.name
}) I get the following results in the output:
Click to view CLI output
Click to view CLI output
accepting causes this: Click to view CLI output
Click to view CLI output
Click to view CLI output
Click to view CLI output
and we've gone full circle. There were no code changes between the first |
I believe the Pulumi provider inherits this behavior from Terraform in https://github.com/hashicorp/terraform-provider-aws/blob/master/internal/service/iam/role.go#L640 This is somewhat a tough problem. I understand that the TF provider introduced these "sidecar" resources as a recommended alternative to the resource options like managed_policy_arns. In your use cases, do you always use sidecar resources or you use a mix? Ideally we would fix it at the provider level, but the provider has only visibility per resource in the current lifecycle, so that the Role resource cannot accurately guess if RolePolicyAttachment managing the stack is present or not. However we could perhaps make a provider-level configuration option that would force Role and other resources ignore properties managed by sidecar resources. Users that prefer those in their stacks could then opt into the desired behavior. |
I believe that the example @pierskarsenbarg provides is expected behaviour called out in the documentation:
The issue I'm raising is that |
@Fydon good catch on that documentation |
I believe I understand your issue @Fydon - the problem is that this behavior is also how Terraform works and we are by default inheriting this behavior. resource "aws_iam_role" "test_role" {
name = "test_role"
assume_role_policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
Action = "sts:AssumeRole"
Effect = "Allow"
Sid = ""
Principal = {
Service = "lambda.amazonaws.com"
}
},
]
})
}
data "aws_iam_policy_document" "policy" {
statement {
effect = "Allow"
actions = ["ec2:Describe*"]
resources = ["*"]
}
}
resource "aws_iam_policy" "policy" {
name = "test-policy-anton"
description = "A test policy"
policy = data.aws_iam_policy_document.policy.json
}
resource "aws_iam_role_policy_attachment" "test-attach" {
role = aws_iam_role.test_role.name
policy_arn = aws_iam_policy.policy.arn
} Then after
However if you do a
|
We're actually considering fixing this, but either fixing upstream or in the bridged provider is not straightforward since aws_iam_role cannot easily access the information on whether aws_iam_role_policy_attachment exists for this role or not. Since this is difficult I'm looking for feedback on whether:
Changing the default to not import managing_policy_arns might break users that are explicitly using this instead of the sidecar resource. |
Venelin suggested ignoreChanges workaround, let me try it here to see what happens. |
ignoreChanges unfortunately is not sufficient to work around here, but pulumi/pulumi#16015 should be able to. |
Looking at this in some more detail to see if we could fix anything here provider side. For the variation of the program provided by @pierskarsenbarg : import * as aws from "@pulumi/aws";
const ec2Role = new aws.iam.Role("piers-role", {
assumeRolePolicy: aws.iam.assumeRolePolicyForPrincipal(aws.iam.Principals.Ec2Principal),
});
const rpa = new aws.iam.RolePolicyAttachment("piers-rpa", {
policyArn: "arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryReadOnly",
role: ec2Role.name
}, {
ignoreChanges: ["managedPolicyArns"],
}) We do a pulumi up followed by pulumi refresh. gRPC log of the refresh has these two Read calls that it appears are not guaranteed to be ordered:
If they were ordered we could maybe observe during Role read that it's not using managedPolicyArns and then exclude that from the Read result in RolePolicyAttachment. |
Thank you for your work on this. My only concern with ignoreChanges and ignoreRefreshChanges is that it would be harder to track if someone had added policies to the role in the AWS Console, but this is probably the best we can get for now. For my use case it is unlikely to happen. |
Closing as won't fix. The recommended workaround is pulumi/pulumi#16015 once it becomes available. In the meanwhile using ignoreChanges is also helpful here even if it does not apply to Refresh changes, as it prevents subsequent Role updates. We are also looking at extending AWS Guard aka pulumi-policy-aws to add a rule detecting incompatible use of managedPolicyArns and RolePolicyAttachment in pulumi/pulumi-policy-aws#107 - it cannot fully solve the problem as there are example programs that pass the check yet still have a refresh problem, but it can be a great help to save time for programs where it does detect a violation. Ultimately with #3806 we hope to one day have a recommended way to work with these resources (either inline attributes or sidecar resources), and deprecate the other usage, which will eventually allow us to fix this problem. |
What happened?
When I import
aws.iam.Role
or runpulumi up --refresh pulumi
, Pulumi will populatemanagedPolicyArns
even if you useaws.iam.PolicyAttachment
,aws.iam.RolePolicyAttachment
oraws.iam.RolePolicy
to attach the same set of policies. Pulumi warns that you should not use both.It is possible to use a resource transform or setting
managedPolicyArns: []
to removemanagedPolicyArns
but then runningpulumi up --refresh
results in policies being detached and then only maybe being reattached.I asked about this problem on Slack, but the only responses I received were from someone who says that they are only a user and so I still feel that this is a bug. They say that the notes about
managedPolicyArns
are about a user setting it and that conflict does not exist if Pulumi sets it. I have never encountered any other application where the value of a property being set in a way that would cause a problem is not a problem if the user did not explicitly set the property to that value themselves.Steps to reproduce
aws.iam.Role
settingmanagedPolicyArns: []
and useaws.iam.PolicyAttachment
,aws.iam.RolePolicyAttachment
oraws.iam.RolePolicy
to attach the same set of policiespulumi up --refresh pulumi
Expected Behavior
After performing the steps, Pulumi state to continues to have managedPolicyArns is set to the empty set.
Actual Behavior
After performing the steps, Pulumi state has managedPolicyArns is set to contain any policies already in
aws.iam.PolicyAttachment
,aws.iam.RolePolicyAttachment
oraws.iam.RolePolicy
.Output of
pulumi about
Additional context
No response
Contributing
Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).
The text was updated successfully, but these errors were encountered: