Closed danopia closed 1 month ago
Adding some extra information as I think this is also related but not 100% sure.
When using a blank inline policy (which enforces that any added inline policy is removed). See example: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role#example-of-removing-inline-policies
As of version 3.70 the inline block will always show a change (even if there is no inline policy to remove):
+ inline_policy {}
3.69.0 works as expect.
As of version 3.70 the inline block will always show a change (even if there is no inline policy to remove):
+ inline_policy {}
3.69.0 works as expect.
I also have this issue, but I don't think it's the same cause, so it should probably be split out into a different ticket. I believe the inline policy update issue is not a regression.
This makes it difficult to see what the actual changes to the policy are.
I think part of the problem here is that the schema of inline_policy
is TypeSet
, but it really should be a dictionary, since any change in a Set marks shows up as the whole object being different. However, changing that is probably backwards incompatible. Maybe if terraform core added support for specifying a "key" for a set, so that it does internal diffs for items within the set based on the key?
Alternatively, maybe it would be possible to create a custom diff for inline_policy
, and change the update to detect if a single policy is changed and do an update (with PutRolePolicy) instead of a delete and add?
This is actually a bit of a theme in a lot of issues with this provider: TypeSet
is used for complex objects in the schema, and then if there is a change in one of those objects, it is done as a destroy and create, instead of an in-place update, even when AWS has an API to do an in-place update.
@tmccombs Hit the same issue. I'll take a look at updating the custom diff for the policies to see if that could be updated to resolve this as that would break backwards compatibility.
I guess another alternative would be to have some sort of flag for not using inline_policy
directly on the iam role and use aws_iam_role_policy
instead. I believe s3 has moved to do this with removing it. But having a flag on the terraform resource that is optional to not have any inline policies managed by state directly in aws_iam_role
resource would avoid the issue inline policies on the resource breaking things. Alternatively removing inline_policy
all together (like s3 did) would resolve this issue. I'm fine with either approach, I know the flag would be a little hacky but temporarily resolve issues for customers now before a new major and/or minor release to remove completely.
@justinretzolk Would it be possible to have one of the maintainers give input in here before I start development? More than willing to help contribute this, just need to know which direction I should go before I start development.
One advantage of inline_policy over aws_iam_role_policy, is that it is better for detecting drift, since it will show a diff if there is a policy that isn't managed as part of the role. There is a similar issue with security group rules.
@tmccombs Ahhh good point. Let me make an upstream issue on terraform then because as you said, this seems like a common issue with TypeSet
and handling diffs for those types of resources. Hopefully maintainers there can give more insight on how this type of situation should be handled.
So from this thread from discuss Hashicorp about TypeSet
, it looks like TypeSet
is expected to act this way. However, the provider can control how things are updated in the backend and it could possibly be moved to something like 'TypeList`. I think first I'll try to update the backend to do a update then delete instead of the delete then add it's doing now so users have a better experience regarding this. Then make follow up PRs with playing around with other types for this.
Are there any plans to migrate this provider to the terraform-plugin-framework?
I'm going to attempt to upgrade iam_role
resource to the terraform-plugin-framework unless anyone objections or is already working on.
I'm going to try to break this updates into parts as much as possible as it will be a pretty big changeset. This first PR is just add arn valid functionality to internal terraform validators for the plugin framework
@justinretzolk Not sure the right channel to ask this but wanted to check before working more on my PR to upgrade the IAM resource to the plugin framework from sdk v2. I was planning on taking the inline_policy
attribute and turn it into inline_policies
as a plugin map attribute where the name of the iam role would be the key and the value would be the policies json. This way plans would be much easier to generate as each policy name has to be unique and users wouldn't run into issues of weird plans. However as this is a pretty big change, I wanted to clarify before continuing the upgrade work I was doing with maintainers of the project. If there's a better way to resolve this above issue, please let me know and I'm more than happy to work on it. Also if there's a better channel for me to communicate this please let me know.
@teddylear were you able to get any direction on this? cc @justinretzolk
@atheiman I haven't so far. I was waiting on a response, but for now I'll continue to work on this with using the plugin map attribute
as I listed above as I believe that most accurately represents how the inline policies work. Then if folks want it changed, it can be updated in the PR.
Was able to make above open PR that should resolve issue. inline_policy
was moved to inline_policies
as a map so it could make plans like below from current updated terraform.
resource "aws_iam_role" "main" {
assume_role_policy = data.aws_iam_policy_document.trust.json
name = "terraform-issue-reproduction"
inline_policies = {
"InlinePolicy" = data.aws_iam_policy_document.inline.json
}
}
# Inline policy document that we'll be changing
data "aws_iam_policy_document" "inline" {
statement {
sid = "S3"
actions = ["s3:ListBucket"]
resources = [
"arn:aws:s3:::some-bucket-a",
# After the first apply, uncomment this next line
# "arn:aws:s3:::some-bucket-b",
]
}
}
# Minimal, arbitrary trust statement to make the role valid
data "aws_iam_policy_document" "trust" {
statement {
actions = ["sts:AssumeRole"]
principals {
type = "Service"
identifiers = ["apigateway.amazonaws.com"]
}
}
}
And the plan to add second bucket in inline policy produces this plan which is close to original one in ticket:
# aws_iam_role.main will be updated in-place
~ resource "aws_iam_role" "main" {
id = "terraform-issue-reproduction"
~ inline_policies = {
~ "InlinePolicy" = jsonencode(
~ {
~ Statement = [
~ {
~ Resource = "arn:aws:s3:::some-bucket-a" -> [
+ "arn:aws:s3:::some-bucket-b",
+ "arn:aws:s3:::some-bucket-a",
]
# (3 unchanged attributes hidden)
},
]
# (1 unchanged attribute hidden)
}
)
}
name = "terraform-issue-reproduction"
# (8 unchanged attributes hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
Adding a third bucket produces this plan which is even more user friendly:
# aws_iam_role.main will be updated in-place
~ resource "aws_iam_role" "main" {
id = "terraform-issue-reproduction"
~ inline_policies = {
~ "InlinePolicy" = jsonencode(
~ {
~ Statement = [
~ {
~ Resource = [
+ "arn:aws:s3:::some-bucket-c",
"arn:aws:s3:::some-bucket-b",
# (1 unchanged element hidden)
]
# (3 unchanged attributes hidden)
},
]
# (1 unchanged attribute hidden)
}
)
}
name = "terraform-issue-reproduction"
# (8 unchanged attributes hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
If there is anything I should change to update about this please let me know.
Hello everyone - thanks for your interest and participation in this issue! I wanted to provide an update after some internal investigation from maintainers.
First, we revisited #35634, which has migrated the aws_iam_role
resource to Terraform Plugin Framework and replaced the inline_policy
block with a new inline_policies
argument. An initial review was completed back in April, with requested changes. The largest blocker is that removal of the inline_policy
block represents a breaking change and cannot be incorporated into a minor release. Additionally, given the ubiquity of this resource, a change like this would likely involve keeping both arguments for a period of time (maybe even across major versions) to avoid an abrupt removal of a long-standing pattern for defining inline policies.
Given this existing pull request included changes beyond a single argument addition, we next investigated adding a new inline_policies
argument to the existing Terraform Plugin SDKV2 based resource, which would be a non-breaking change. Similar to the PR above, this argument would be a map[string]string
, where the key is the inline policy name and value is the policy content. While resolving the issues described in the initial bug report, this introduces additional complexity and technical debt. There would now be 3 distinct options for defining inline policies:
inline_policy
blockinline_policies
argumentaws_iam_role_policy
standalone resourceConcerns around complexity led us to explore another pattern, now the subject of an open proposal, to implement "exclusive relationship management" resources. The inability to "exlusively manage" inline policy assignments using the standalone resource has often been cited as a barrier to using this currently available aoption. aws_iam_role_policy
, the standalone resource for managing inline policies, is the first use case we've prototyped that would benefit from this pattern. If implemented, an aws_iam_role_policies_exclusive
resource would allow aws_iam_role_policy
resources to be included in a list of inline policies exclusively managed by Terraform, replicating the behavior of the current inline_policy
argument.
While this does not resolve the issue with the inline_policy
argument, it does provide a clear migration path to aws_iam_role_policy
(now paired with aws_iam_role_policies_exclusive
when exclusive ownership is desired), where changes to policy content are more appropriately displayed and can be modified without re-creation.
The configuration and output below displays how this would function in the case where a standalone inline policy is modified:
% make plan
TF_CLI_CONFIG_FILE=dev.tfrc terraform plan
â•·
│ Warning: Provider development overrides are in effect
│
│ The following provider development overrides are set in the CLI configuration:
│ - hashicorp/aws in /Users/jaredbaker/go/bin
│
│ The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with published releases.
╵
data.aws_iam_policy_document.inline: Reading...
data.aws_iam_policy_document.trust: Reading...
data.aws_iam_policy_document.inline: Read complete after 0s [id=748695472]
data.aws_iam_policy_document.trust: Read complete after 0s [id=2851119427]
aws_iam_role.test: Refreshing state... [id=jb-test-role-policies-lock]
aws_iam_role_policy.test: Refreshing state... [id=jb-test-role-policies-lock:test-inline]
aws_iam_role_policies_exclusive.test: Refreshing state...
Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# aws_iam_role_policy.test will be updated in-place
~ resource "aws_iam_role_policy" "test" {
id = "jb-test-role-policies-lock:test-inline"
name = "test-inline"
~ policy = jsonencode(
~ {
~ Statement = [
~ {
~ Resource = "arn:aws:s3:::some-bucket-a" -> [
+ "arn:aws:s3:::some-bucket-b",
+ "arn:aws:s3:::some-bucket-a",
]
# (2 unchanged attributes hidden)
},
]
# (1 unchanged attribute hidden)
}
)
# (2 unchanged attributes hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
At this time our proposal is to proceed with the addition of a new aws_iam_role_policies_exclusive
resource, and recommend users who are looking for an improved inline policy update experience migrate to the standalone aws_iam_role_policy
resource. We welcome any feedback on the linked proposal, exclusive resource management PR, or this comment, and are planning to allow time for community feedback before proceeding. Thanks!
I like that approach. It would also be useful in other places, like security group rules.
I am not interested in using aws_iam_role_policies_exclusive
. I use inline_policy
attribute to shorten my terraform config (one less resource to read and relationship to track). Otherwise id use aws_iam_role_policy
. Terraform is pretty verbose and that can be intimidating to new users / make configuration difficult to follow.
Personally id rather a new inline_policies
attribute as previously proposed. I don’t see how aws_iam_role_policies_exclusive
helps with this issue as you stated:
While this does not resolve the issue with the inline_policy argument…
If aws_iam_role_policies_exclusive
is implemented and neither inline_policy
nor inline_policies
is fixed, I’ll probably just use aws_iam_role_policy
rather than have to explain to newer tf users what aws_iam_role_policies_exclusive
is (im not even really clear on it).
Edit: oh i see, aws_iam_role_policies_exclusive
would be used alongside aws_iam_role_policy
… to me that’s even worse. I would just use aws_iam_policy
and managed_policy_arns
attr or aws_iam_role_policy_attachment
resources if necessary, but i have never run into the issue as described to the point where i would be concerned enough to want yet another iam role policy resource/pattern. I really think the complexity of yet another iam role policy resource outweighs the benefits in my experience with aws + terraform (~ 6y).
Edit 2: I should say i really do appreciate the thoughtful reply and review of the issue. Thanks.
I like that approach. It would also be useful in other places, like security group rules.
Agreed! Security group rules to security groups is one of the relationships covered in the Potential Resources
section of the proposal. We're very interested in adding something here which hopefully allows us to deprecate and eventually remove the legacy pattern of defining rules inline on the security group resource.
@atheiman - Thanks for your feedback! We understand the hesitation to introduce another resource into configuration.
As maintainers we're attempting to balance the potential confusion of introducing "yet another way" to manage inline policies (should I use inline_policy
or inline_policies
? why do both exist on the same resource? etc.) against introducing a new resource. Either approach requires changing an existing configuration that uses the inline_policy
block, and critically the new aws_iam_role_policies_exclusive
resource covers a gap which has long prevented users from migrating to aws_iam_role_policy
, our preferred pattern. If this works as we expect, it opens the possibility of deprecating the inline_policies
block and eventually having only a single way to manage inline policies.
The full proposal has more detailed research and justification, so I won't add more to an already long thread here. I mostly want to describe that we weighed both options equally (including a working POC and RFC on the inline_policies
approach) and currently feel that the proposed approach is best for the long term maintainability of the provider.
Hello everyone - I wanted to share an update. The proposal for “exclusive relationship management” resources has been approved, and we’re proceeding with implementing a standalone resource to allow Terraform to exclusively manage inline policy assignments (#39203).
A consequence of this decision is that we are not intending to add a new argument to the aws_iam_role
resource to address the concerns with the existing inline_policy
argument. For users looking for in-place updates of inline policies and improved rendering of planned changes, we encourage adoption of the standalone aws_iam_role_policy
resource. If you’d like Terraform to continue exclusively managing inline policy assignments for the role (the current behavior of the inline_policy
argument), include the new aws_iam_role_policies_exclusive
resource as well.
Lastly, since the inline_policy
argument will be deprecated as part of #39203, we are intending to close this issue as won't fix
. Before doing so we’ll leave this open an additional two weeks (until 2024-10-01) for feedback. Please let us know if there are any technical issues preventing adoption of the recommended pattern above.
While removing the deprecated arguments from my config I noticed the warning doesn't mention which argument is deprecated:
â•·
│ Warning: Argument is deprecated
│
│ with module.main.aws_iam_role.backup_default_service_role,
│ on ../main_module/main.tf line 8290, in resource "aws_iam_role" "backup_default_service_role":
│ 8290: resource "aws_iam_role" "backup_default_service_role" {
│
│ Use the aws_iam_role_policy resource instead. If Terraform should exclusively manage all inline policy associations (the current
│ behavior of this argument), use the aws_iam_role_policies_exclusive resource as well.
╵
Shouldn't this warning mention that it's the inline_policy
argument that needs attention?
Thanks for the suggestion @BroMattMiller. #39430 has updated the deprecation message and added some detail to the registry documentation which was missed in the previous PR.
For those looking to migrate from the deprecated inline_policy
argument, this comment outlines how to convert to standalone resources.
Sometimes I don't want to create standalone policy because it is exclusive only for given role. With this new resource I'm forced to create standalone policy and then I can link this managed role to mimic inline role but I still have to maintain this managed "blueprint".
Is my understanding correct?
Also, is it only deprecation or active plan for removal of inline_policy
? I have plenty of code that uses inline_policy
approach, I accepted notable drawbacks of it and it works for me. Now, in any scenario I won't get enough time in my project to migrate all code to this new approach hence killing this feature will effectively kill me as well.
Also, 2-weeks is a little short notice to gather real feedback. Especially when announcement is done via deprecation warning (this is how I come to this issue).
Hi @kkurczewski - thanks for your feedback! I'll respond to each of these separately.
With this new resource I'm forced to create standalone policy and then I can link this managed role to mimic inline role but I still have to maintain this managed "blueprint".
The standalone aws_iam_role_policy
resource does not mimic an inline policy with a customer managed policy - it creates one exactly as the inline_policy
argument does. Specifically, it uses the PutRolePolicy
and DeleteRolePolicy
API's, so the policy defined in this resource could never be associated with another role in the way a policy created via aws_iam_policy
could.
Please let me know if I misunderstood what you were looking for clarification on with this question.
Also, is it only deprecation or active plan for removal of inline_policy?
Here is a relevant section of the underlying proposal.
Due to the popularity of the resources in this section, argument deprecations are likely to be "soft" deprecations where removal will not happen for several major releases, or until tooling is available to limit the amount of manual changes required to migrate to the preferred pattern. Despite this long removal window, a soft deprecation is still helpful for maintainers to reference when making best practice recommendations to the community.
TLDR; we have no plans to remove this argument in the next major version. The deprecation is primarily to allow maintainers and community members to have explicit direction on the preferred pattern for defining inline policies going forward.
Also, 2-weeks is a little short notice to gather real feedback.
The two week window is specific to closing this issue, and since discussion on this topic has spanned several years now we felt it best to set a date for resolution. The only viable option to resolving the original issue with re-creation of policies defined via the inline_policy
argument is to either make a breaking change to the type or introduce yet another argument into the aws_iam_role
resource. Because we've deemed neither of these viable options (described in this comment), we felt it best to close the issue as we don't intend to fix it. If we receive feedback within the 2 weeks which indicates migration to the standalone resource is not viable due to technical limitations we will of course leave this open beyond the 2 week period.
Hope this helps to clarify our thinking here, and thanks again for your feedback.
Here is a relevant section of the underlying proposal.
Due to the popularity of the resources in this section, argument deprecations are likely to be "soft" deprecations where removal will not happen for several major releases, or until tooling is available to limit the amount of manual changes required to migrate to the preferred pattern. Despite this long removal window, a soft deprecation is still helpful for maintainers to reference when making best practice recommendations to the community.
TLDR; we have no plans to remove this argument in the next major version. The deprecation is primarily to allow maintainers and community members to have explicit direction on the preferred pattern for defining inline policies going forward.
Okay, that's a big relief for me :+1:
The two week window is specific to closing this issue, and since discussion on this topic has spanned several years now we felt it best to set a date for resolution. The only viable option to resolving the original issue with re-creation of policies defined via the
inline_policy
argument is to either make a breaking change to the type or introduce yet another argument into theaws_iam_role
resource. Because we've deemed neither of these viable options (described in this comment), we felt it best to close the issue as we don't intend to fix it. If we receive feedback within the 2 weeks which indicates migration to the standalone resource is not viable due to technical limitations we will of course leave this open beyond the 2 week period.
I see, fair enough.
Regarding my concerns about usage of new resource, thanks for additional clarification. I need to sort it in my head and perhaps try it in practice with actual Terraform code, once then I will back to you.
@jar-b tl;dr; yeah, you are right. I was just confused.
With this new resource I'm forced to create standalone policy and then I can link this managed role to mimic inline role but I still have to maintain this managed "blueprint".
The standalone
aws_iam_role_policy
resource does not mimic an inline policy with a customer managed policy - it creates one exactly as theinline_policy
argument does. Specifically, it uses thePutRolePolicy
andDeleteRolePolicy
API's, so the policy defined in this resource could never be associated with another role in the way a policy created viaaws_iam_policy
could.Please let me know if I misunderstood what you were looking for clarification on with this question.
Okay, I checked with Terraform and now I see you were right and I just confused aws_iam_policy
and aws_iam_role_policy
.
As aws_iam_policy
and aws_iam_role_policy
have short and similar names and both blocks have a lot of warnings about usage (not a complaint) and a lot of examples (also not a complaint) I somewhat didn't realized actual difference. One has required argument role
(hence it is inlined) and second doesn't (hence it is managed).
Fun fact, when writing this post I also did similar mistake and confused aws_iam_policy
with aws_iam_role
.
Also, for some strange reason I had mental model where data
block doesn't create anything physical and resource
always create something physical, but as you said this is not always true. Various *_association
and *_attachment
blocks are notable "exceptions", perhaps a little bit more obvious.
On occasion I will try to play with this new resource and report additional feedback, if any.
Completely understand - the naming of these resources, while in line with the AWS API's, does make distinguishing inline versus customer managed policies difficult. I often find myself in the registry docs just to be sure :).
Appreciate your openness to trying out the new resource. Any feedback would be great!
The feedback window has elapsed and we have not received justification to keep the original request open. For related feature requests please open a new issue, optionally linking to this one.
[!WARNING] This issue has been closed, meaning that any additional comments are hard for our team to see. Please assume that the maintainers will not see them.
Ongoing conversations amongst community members are welcome, however, the issue will be locked after 30 days. Moving conversations to another venue, such as the AWS Provider forum, is recommended. If you have additional concerns, please open a new issue, referencing this one where needed.
[!WARNING] This issue has been closed, meaning that any additional comments are hard for our team to see. Please assume that the maintainers will not see them.
Ongoing conversations amongst community members are welcome, however, the issue will be locked after 30 days. Moving conversations to another venue, such as the AWS Provider forum, is recommended. If you have additional concerns, please open a new issue, referencing this one where needed.
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.
Community Note
Terraform CLI and Terraform AWS Provider Version
Affected Resource(s)
resource.aws_iam_role
data.aws_iam_policy_document
in the reproductionTerraform Configuration Files
Debug Output
Redacted debug-level log: https://gist.github.com/danopia/0941f0d5f487432770c670603b221ea1
Expected Behavior
Because the
inline_policy
blocks have the samename
value, Terraform should view this change as an update to the existinginline_policy
:I'd expect the diff to look like this:
Actual Behavior
Terraform prints a diff showing an
inline_policy
removal and then also aninline_policy
creation:Also, from the attached debug log, we can see that Terraform is actually doing a delete-then-create sequence:
Steps to Reproduce
terraform apply
some-bucket-b
terraform apply
References
I've found this issue mentioned within other
inline_policy
issues: