Repository: getmoto/moto
Pull request #6482 (2023-07-04 15:55:51)

Fix KeyError when attaching non-existent policy to role
## Motivation

The error handling for a non-existent policy was not implemented for `attach_role_policy`, (only for `attach_user_policy`).

Also, the error messages did not match the AWS error:
```
        "Error": {
          "Code": "NoSuchEntity",
          "Message": "Policy arn:aws:iam::111111111111:policy/policy-name does not exist or is not attachable.",
          "Type": "Sender"
        },
```

## Changes
* Add error handling for KeyError in `attach_role_policy`
* Change error message for `attach_user_policy`
* Add testing this error in `test_attach_detach_user_policy`
* Duplicate `test_attach_detach_user_policy` to usage with roles as `test_attach_detach_role_policy`

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/getmoto/moto/pull/6482?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto) Report
> Merging [#6482](https://app.codecov.io/gh/getmoto/moto/pull/6482?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto) (811c966) into [master](https://app.codecov.io/gh/getmoto/moto/commit/b66272717fa0aa46c3e1bf7894e9c719f8388ed3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto) (b662727) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##           master    #6482   +/-   ##
=======================================
  Coverage   96.18%   96.18%           
=======================================
  Files         800      800           
  Lines       78542    78549    +7     
=======================================
+ Hits        75542    75552   +10     
+ Misses       3000     2997    -3     
```

| Flag | Coverage Δ | |
|---|---|---|
| servertests | `36.87% <40.00%> (-0.01%)` | :arrow_down: |
| unittests | `96.12% <100.00%> (+<0.01%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://app.codecov.io/gh/getmoto/moto/pull/6482?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto) | Coverage Δ | |
|---|---|---|
| [moto/iam/models.py](https://app.codecov.io/gh/getmoto/moto/pull/6482?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto#diff-bW90by9pYW0vbW9kZWxzLnB5) | `96.01% <100.00%> (+0.12%)` | :arrow_up: |

... and [2 files with indirect coverage changes](https://app.codecov.io/gh/getmoto/moto/pull/6482/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto)



Patch:
diff --git a/moto/iam/models.py b/moto/iam/models.py
index a41c76214..a837fa797 100644
--- a/moto/iam/models.py
+++ b/moto/iam/models.py
@@ -1823,7 +1823,13 @@ class IAMBackend(BaseBackend):
 
     def attach_role_policy(self, policy_arn: str, role_name: str) -> None:
         arns = dict((p.arn, p) for p in self.managed_policies.values())
-        policy = arns[policy_arn]
+        try:
+            policy = arns[policy_arn]
+        except KeyError:
+            raise IAMNotFoundException(
+                f"Policy {policy_arn} does not exist or is not attachable."
+            )
+
         policy.attach_to(self.get_role(role_name))
 
     def update_role_description(self, role_name: str, role_description: str) -> Role:
@@ -1891,7 +1897,9 @@ class IAMBackend(BaseBackend):
         try:
             policy = arns[policy_arn]
         except KeyError:
-            raise IAMNotFoundException(f"Policy {policy_arn} was not found.")
+            raise IAMNotFoundException(
+                f"Policy {policy_arn} does not exist or is not attachable."
+            )
         policy.attach_to(self.get_user(user_name))
 
     def detach_user_policy(self, policy_arn: str, user_name: str) -> None:


Test patch:
diff --git a/tests/test_iam/test_iam.py b/tests/test_iam/test_iam.py
index a73c40064..3d98cf53b 100644
--- a/tests/test_iam/test_iam.py
+++ b/tests/test_iam/test_iam.py
@@ -2063,6 +2063,16 @@ def test_attach_detach_user_policy():
         Description="my user attached policy",
     )
 
+    # try a non-existent policy
+    non_existent_policy_arn = f"arn:aws:iam::{ACCOUNT_ID}:policy/not-existent"
+    with pytest.raises(ClientError) as exc:
+        client.attach_user_policy(UserName=user.name, PolicyArn=non_existent_policy_arn)
+    err = exc.value.response["Error"]
+    err["Code"].should.equal("NoSuchEntity")
+    err["Message"].should.equal(
+        f"Policy {non_existent_policy_arn} does not exist or is not attachable."
+    )
+
     client.attach_user_policy(UserName=user.name, PolicyArn=policy.arn)
 
     resp = client.list_attached_user_policies(UserName=user.name)
@@ -2077,6 +2087,45 @@ def test_attach_detach_user_policy():
     resp["AttachedPolicies"].should.have.length_of(0)
 
 
+@mock_iam()
+def test_attach_detach_role_policy():
+    iam = boto3.resource("iam", region_name="us-east-1")
+    client = boto3.client("iam", region_name="us-east-1")
+
+    role = iam.create_role(RoleName="test-role", AssumeRolePolicyDocument="{}")
+
+    policy_name = "RoleAttachedPolicy"
+    policy = iam.create_policy(
+        PolicyName=policy_name,
+        PolicyDocument=MOCK_POLICY,
+        Path="/mypolicy/",
+        Description="my role attached policy",
+    )
+
+    # try a non-existent policy
+    non_existent_policy_arn = f"arn:aws:iam::{ACCOUNT_ID}:policy/not-existent"
+    with pytest.raises(ClientError) as exc:
+        client.attach_role_policy(RoleName=role.name, PolicyArn=non_existent_policy_arn)
+    err = exc.value.response["Error"]
+    err["Code"].should.equal("NoSuchEntity")
+    err["Message"].should.equal(
+        f"Policy {non_existent_policy_arn} does not exist or is not attachable."
+    )
+
+    client.attach_role_policy(RoleName=role.name, PolicyArn=policy.arn)
+
+    resp = client.list_attached_role_policies(RoleName=role.name)
+    resp["AttachedPolicies"].should.have.length_of(1)
+    attached_policy = resp["AttachedPolicies"][0]
+    attached_policy["PolicyArn"].should.equal(policy.arn)
+    attached_policy["PolicyName"].should.equal(policy_name)
+
+    client.detach_role_policy(RoleName=role.name, PolicyArn=policy.arn)
+
+    resp = client.list_attached_role_policies(RoleName=role.name)
+    resp["AttachedPolicies"].should.have.length_of(0)
+
+
 @mock_iam()
 def test_only_detach_user_policy():
     iam = boto3.resource("iam", region_name="us-east-1")