Migrate IAM role implementation#13798
Conversation
Test Results - Preflight, Unit23 123 tests 21 252 ✅ 6m 19s ⏱️ Results for commit 6706e17. ♻️ This comment has been updated with latest results. |
LocalStack Community integration with Pro 2 files 2 suites 2m 39s ⏱️ For more details on these failures, see this check. Results for commit 6706e17. ♻️ This comment has been updated with latest results. |
| @markers.snapshot.skip_snapshot_verify( | ||
| paths=["$..Role.Tags"] | ||
| ) # Moto returns an empty list for no tags | ||
| def test_put_role_policy_encoding(self, snapshot, aws_client, create_role, region_name): |
| @dataclasses.dataclass | ||
| class RoleEntity: | ||
| """Wrapper for Role with inline policies and managed policy tracking.""" | ||
|
|
||
| role: Role # From localstack.aws.api.iam | ||
| inline_policies: dict[str, str] = field(default_factory=dict) # policy_name -> document | ||
| attached_policy_arns: list[str] = field( | ||
| default_factory=list | ||
| ) # ARNs of attached managed policies | ||
| linked_service: str | None = None # For service-linked roles |
| if permissions_boundary: | ||
| self._validate_permissions_boundary(context, permissions_boundary) | ||
|
|
||
| with self._role_lock: |
| def tag_role( | ||
| self, | ||
| context: RequestContext, | ||
| role_name: roleNameType, | ||
| tags: tagListType, | ||
| **kwargs, | ||
| ) -> None: | ||
| self._validate_tags(tags, case_sensitive=False) | ||
|
|
||
| store = self._get_store(context) | ||
| with self._role_lock: | ||
| role_entity = self._get_role_entity(store, role_name) | ||
|
|
||
| # Initialize tags if not present | ||
| if "Tags" not in role_entity.role or role_entity.role["Tags"] is None: | ||
| role_entity.role["Tags"] = [] | ||
|
|
||
| # Merge tags - update existing keys, add new ones, case-insensitive | ||
| existing_keys = { | ||
| tag["Key"].lower(): i for i, tag in enumerate(role_entity.role["Tags"]) | ||
| } | ||
| for tag in tags: | ||
| key = tag["Key"].lower() | ||
| if key in existing_keys: | ||
| role_entity.role["Tags"][existing_keys[key]] = tag | ||
| else: | ||
| role_entity.role["Tags"].append(tag) | ||
|
|
||
| def untag_role( | ||
| self, | ||
| context: RequestContext, | ||
| role_name: roleNameType, | ||
| tag_keys: tagKeyListType, | ||
| **kwargs, | ||
| ) -> None: | ||
| self._validate_tag_keys(tag_keys) | ||
|
|
||
| store = self._get_store(context) | ||
| with self._role_lock: | ||
| role_entity = self._get_role_entity(store, role_name) | ||
|
|
||
| if "Tags" in role_entity.role and role_entity.role["Tags"]: | ||
| # Remove tags with matching keys (case-sensitive) | ||
| tag_keys_set = {key.lower() for key in tag_keys} | ||
| role_entity.role["Tags"] = [ | ||
| tag | ||
| for tag in role_entity.role["Tags"] | ||
| if tag["Key"].lower() not in tag_keys_set | ||
| ] | ||
|
|
||
| def list_role_tags( | ||
| self, | ||
| context: RequestContext, | ||
| role_name: roleNameType, | ||
| marker: markerType = None, | ||
| max_items: maxItemsType = None, | ||
| **kwargs, | ||
| ) -> ListRoleTagsResponse: | ||
| store = self._get_store(context) | ||
| with self._role_lock: | ||
| role_entity = self._get_role_entity(store, role_name) | ||
| tags = list(role_entity.role.get("Tags") or []) | ||
|
|
||
| # Sort alphabetically by key, then by key length | ||
| tags.sort(key=lambda k: k["Key"]) | ||
| tags.sort(key=lambda k: len(k["Key"])) | ||
|
|
||
| paginated_list = PaginatedList(tags) | ||
|
|
||
| def _token_generator(tag: Tag) -> str: | ||
| return tag.get("Key") | ||
|
|
||
| # base64 encode/decode to avoid plaintext tag as marker | ||
| if marker: | ||
| marker = base64.b64decode(marker).decode("utf-8") | ||
|
|
||
| result, next_marker = paginated_list.get_page( | ||
| token_generator=_token_generator, next_token=marker, page_size=max_items or 100 | ||
| ) | ||
|
|
||
| if next_marker: | ||
| next_marker = base64.b64encode(next_marker.encode("utf-8")).decode("utf-8") | ||
| return ListRoleTagsResponse(Tags=result, IsTruncated=True, Marker=next_marker) | ||
| else: | ||
| return ListRoleTagsResponse(Tags=result, IsTruncated=False) |
There was a problem hiding this comment.
Since time is precious now we should simply migrate but let's remember later that these operations related to tags will change due to the resource group tagging project.
There was a problem hiding this comment.
Agreed! I reached out to Ben on the proper current way to do this, I am not sure since the service is in community, and I would like not to use the override appraoch. We can migrate it in a big bang at the end of the project.
| role: Role = { | ||
| "Path": path, | ||
| "RoleName": role_name, | ||
| "RoleId": role_id, | ||
| "Arn": role_arn, | ||
| "CreateDate": datetime.now(tz=UTC), | ||
| "AssumeRolePolicyDocument": quote(policy_doc), | ||
| "MaxSessionDuration": 3600, | ||
| "RoleLastUsed": RoleLastUsed(), | ||
| } |
There was a problem hiding this comment.
nit: IMO defining the role as Role(Path=path....) is more explicit about the usage of the api typed dict.
There was a problem hiding this comment.
Agreed, did not catch that one! I changed it in most usages (especially in later PRs)
| policy_entity = store.MANAGED_POLICIES[policy_arn] | ||
| policy_entity.policy["AttachmentCount"] += 1 |
There was a problem hiding this comment.
idea: wouldn't it be better to calculate AttachmentCount per request? this relies on us always remembering to update the value. Maybe it's not worth it since there is only 2 operations for now that affect that count.
There was a problem hiding this comment.
There will be a total of 6 operations for this (users, roles, groups). I am not super happy with this currently, I agree!
Let's refactor we implemented them all, we can check then what the best pattern would be!
81cb654 to
6241c44
Compare
7c790f5 to
6706e17
Compare
Test Results (amd64) - Acceptance7 tests 5 ✅ 3m 0s ⏱️ Results for commit 6706e17. |
Test Results (amd64) - Integration, Bootstrap 5 files ±0 5 suites ±0 11m 21s ⏱️ -2s For more details on these failures, see this check. Results for commit 6706e17. ± Comparison against base commit 6241c44. |
6706e17 to
5691483
Compare
Motivation
We are migrating IAM role handling from moto to LocalStack now - this includes attachement and detachement of policies.
Changes
1. Entity Structure
The
RoleEntitydataclass wraps the APIRoletype with additional relations:Rationale:
role: Rolestores all core role attributes (Path, RoleName, RoleId, Arn, CreateDate, AssumeRolePolicyDocument, Description, MaxSessionDuration, PermissionsBoundary, Tags, RoleLastUsed)inline_policiesis a dict mapping policy name to policy document JSON string, matching how inline policies workattached_policy_arnsis a list (not set) to preserve attachment order; theManagedPolicyEntitytracks its ownAttachmentCountlinked_serviceis needed for service-linked role identification2. Storage Key
Roles are keyed by role name in
IamStore.ROLES:Rationale:
CrossRegionAttributesince IAM is a global service3. Locking Strategy
A separate
_role_lockis used for role operations, independent of_policy_lock:Rationale:
4. Trust Policy Validation
Trust policies (AssumeRolePolicyDocument) are validated separately from regular IAM policies, as in moto:
Resourcefield (unlike regular policies)sts:AssumeRole,sts:AssumeRoleWithSAML,sts:AssumeRoleWithWebIdentity,sts:TagSession,sts:SetSourceIdentity, or*5. Permissions Boundary Handling
Permissions boundaries are validated to:
:policy/)Stored in
role["PermissionsBoundary"]asAttachedPermissionsBoundary:{ "PermissionsBoundaryType": "Policy", "PermissionsBoundaryArn": "<arn>" }6. Managed Policy Attachment
When attaching/detaching customer-managed policies:
attached_policy_arnsAttachmentCountonManagedPolicyEntity.policyis incremented/decremented with a policy lockarn:{partition}:iam::aws:policy/) are tracked inattached_policy_arnsbut don't update any count since they're not in our store7. Instance Profile Interaction
Instance profiles remain in moto for now. The
delete_roleoperation checks moto's instance profiles to ensure the role is not attached before deletion:Future work: When instance profiles are migrated, they should store role names and look up from
IamStore.ROLES.Migrated Operations
Core CRUD
create_role- Creates role in native store with validationget_role- Returns role from native storedelete_role- Deletes from native store with constraint checkslist_roles- Lists from native store with path filtering and paginationupdate_role- Updates description and/or max_session_durationupdate_role_description- Updates description onlyupdate_assume_role_policy- Updates trust policy with validationTagging
tag_role- Adds/updates tags with validationuntag_role- Removes tags by keylist_role_tags- Lists tags with paginationInline Policies
put_role_policy- Adds/updates inline policy with validationget_role_policy- Returns inline policy documentlist_role_policies- Lists inline policy namesdelete_role_policy- Removes inline policyManaged Policy Attachment
attach_role_policy- Attaches managed policy, updates AttachmentCountdetach_role_policy- Detaches managed policy, updates AttachmentCountlist_attached_role_policies- Lists attached policies with path filteringPermissions Boundary
put_role_permissions_boundary- Sets permissions boundarydelete_role_permissions_boundary- Removes permissions boundaryService-Linked Roles
create_service_linked_role- Creates in native store with linked_service setdelete_service_linked_role- Deletes from native store (allows attached policies)get_service_linked_role_deletion_status- Returns SUCCESS (unchanged)Known Limitations
Deviations from Moto Behavior
Tests
Tests are in
tests/aws/services/iam/test_iam_roles.pyand cover:Related
Closes UNC-229