Skip to content

fix: use --role-arn service role for companion stack create/update #9267

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.

Sign up for GitHub

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

Open
Sanjays2402 wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

fix: use --role-arn service role for companion stack create/update #9267

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
64dfdf0
fix: use service role for companion stack create/update
Sanjays2402 Sep 9, 2026
4bd8f41
Address review: forward role_arn in guided deploy flow
Sanjays2402 Sep 9, 2026
5fdfcc7
Address bot review: assume role_arn for direct ECR calls in companion…
Sanjays2402 Sep 9, 2026
75f5f7c
fix: lazy non-fatal assume-role for ECR calls; RoleARN on delete_stack
Sanjays2402 Sep 9, 2026
a3353b2
Address review feedback: catch BotoCoreError in _ecr_client_for_role …
Sanjays2402 Sep 10, 2026
c04273d
Address review: complete ECR fallback contract for assumed service role
Sanjays2402 Sep 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion samcli/commands/deploy/command.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ def do_cli(
# after we figure out how to enable resolve-images-repos in package
if resolve_image_repos:
image_repositories = sync_ecr_stack(
template_file, stack_name, region, s3_bucket, s3_prefix, image_repositories
template_file, stack_name, region, s3_bucket, s3_prefix, image_repositories, role_arn=role_arn

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[BUG] The fix only covers the non-guided branch, but --role-arn applies to guided deploys too — it is passed to DeployContext unconditionally at command.py:416, and it is also persisted into samconfig.toml. That means sam deploy --guided --role-arn <role> --resolve-image-repos still reproduces the exact failure from #5051: the main stack deploys under the service role, while the companion stack is created with the caller's credentials and fails on ecr:CreateRepository.

Both companion-stack entry points in the guided flow are affected:

  • samcli/commands/deploy/guided_context.py:191 — sync_ecr_stack(...) when --resolve-image-repos is set
  • samcli/commands/deploy/guided_context.py:362 — CompanionStackManager(stack_name, region, s3_bucket, s3_prefix) in prompt_image_repository, which calls manager.sync_repos() at line 390 and therefore reaches update_companion_stack() / create_stack

Threading role_arn through GuidedContext would close the gap, e.g. accept it in GuidedContext.__init__, pass role_arn=role_arn from the guided branch of do_cli, and forward it at both call sites:

# guided_context.py, in guided_prompts
image_repositories = (
   sync_ecr_stack(
       self.template_file,
       stack_name,
       region,
       managed_s3_bucket,
       self.s3_prefix,
       self.image_repositories,
       role_arn=self.role_arn,
   )
   if self.resolve_image_repositories
   else self.prompt_image_repository(...)
)

If leaving guided out is deliberate, it is worth stating why in the PR description, since the reported symptom is identical in that path.

@Sanjays2402 Sanjays2402 Sep 9, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — you're right, the guided path had the same bug. GuidedContext now accepts role_arn, do_cli passes --role-arn through on the guided branch, and it's forwarded at both companion-stack call sites (sync_ecr_stack in guided_prompts and the CompanionStackManager in prompt_image_repository). Added guided tests asserting the role reaches both. Verified: 24/24 guided-context tests and all companion-stack manager tests pass.

)
with osutils.tempfile_platform_independent() as output_template_file:
if guided:
Expand Down
36 changes: 26 additions & 10 deletions samcli/lib/bootstrap/companion_stack/companion_stack_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"""

import logging
from typing import Dict, List, Optional
from typing import Any, Dict, List, Optional

import boto3
from botocore.config import Config
Expand Down Expand Up @@ -41,14 +41,15 @@ class CompanionStackManager:
_cfn_client: CloudFormationClient
_s3_client: S3Client

def __init__(self, stack_name, region, s3_bucket, s3_prefix):
def __init__(self, stack_name, region, s3_bucket, s3_prefix, role_arn: Optional[str] = None):
self._companion_stack = CompanionStack(stack_name)
self._builder = CompanionStackBuilder(self._companion_stack)
self._boto_config = Config(region_name=region if region else None)
self._update_stack_waiter_config = {"Delay": 10, "MaxAttempts": 120}
self._delete_stack_waiter_config = {"Delay": 10, "MaxAttempts": 120}
self._s3_bucket = s3_bucket
self._s3_prefix = s3_prefix
self._role_arn = role_arn
try:
self._cfn_client = boto3.client("cloudformation", config=self._boto_config)
self._ecr_client = boto3.client("ecr", config=self._boto_config)
Expand Down Expand Up @@ -117,16 +118,21 @@ def update_companion_stack(self) -> None:
template_url = s3_uploader.to_path_style_s3_url(parts["Key"], parts.get("Version", None))

exists = self.does_companion_stack_exist()
# Use the service role passed via --role-arn (if any) so the companion stack is created/updated
# with the same permissions as the main stack.
stack_kwargs: Dict[str, Any] = {
"StackName": stack_name,
"TemplateURL": template_url,
"Capabilities": ["CAPABILITY_AUTO_EXPAND"],
}
if self._role_arn:
stack_kwargs["RoleARN"] = self._role_arn

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[BUG] The service role is now wired into create_stack/update_stack, but sync_repos() also reaches ECR directly, and that path still runs under the caller's credentials — so the failure mode from #5051 remains reachable.

def sync_repos(self) -> None:
   has_repo = bool(self.get_repository_mapping())
   if self.does_companion_stack_exist():
       self.delete_unreferenced_repos()   # <-- direct ECR call, caller creds
       ...
def delete_unreferenced_repos(self) -> None:
   repos = self.get_unreferenced_repos()
   for repo in repos:
       try:
           self._ecr_client.delete_repository(repositoryName=repo.physical_id, force=True)
       except self._ecr_client.exceptions.RepositoryNotFoundException:
           LOG.debug(...)

self._ecr_client is built from ambient credentials in __init__; a CloudFormation RoleARN only affects calls CloudFormation makes on your behalf, not SDK calls SAM CLI makes itself. For the user profile described in the issue (caller has no ECR permissions, the --role-arn role does), removing an image function from the template and re-deploying with --resolve-image-repos --role-arn will raise an AccessDenied ClientError on ecr:DeleteRepository. That error is not caught here (only RepositoryNotFoundException is), so the deploy aborts with a raw botocore error before the companion stack update runs.

Two options, either is fine:

  • Assume role_arn via STS and build self._ecr_client from those credentials when role_arn is set, so all companion-stack side effects use one identity.
  • If keeping caller credentials for ECR is intentional, catch ClientError with AccessDeniedException here and raise an actionable message explaining that ecr:DeleteRepository is required on the calling identity even when --role-arn is supplied.

Worth covering with a unit test for the unreferenced-repo path, since the current tests exercise only create/update.

@Sanjays2402 Sanjays2402 Sep 9, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — when role_arn is set, the manager now assumes it via STS and builds self._ecr_client from the assumed-role credentials, so delete_unreferenced_repos and all other direct ECR calls run under one identity. An assume-role failure raises an actionable AWSServiceClientError. Added tests covering the assumed-role delete path and the failure case.

@aws-sam-tooling-bot aws-sam-tooling-bot Bot Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[BUG] The service role is applied to create_stack/update_stack but not to _delete_companion_stack, which is reachable from sync_repos() whenever the last image function is removed:

if self.does_companion_stack_exist():
   self.delete_unreferenced_repos()
   if has_repo:
       self.update_companion_stack()
   else:
       self._delete_companion_stack()   # no RoleARN

DeleteStack accepts RoleARN. When omitted, CloudFormation reuses the role previously associated with the stack — but any companion stack created before this fix (or created without --role-arn) has no associated role, so CloudFormation falls back to a session derived from the caller's credentials and the stack delete can fail with the same AccessDenied class of error as #5051, this time on repository deletion. Passing the role keeps the whole companion-stack lifecycle on one identity:

def deletecompanion_stack(self) -> None:
   stack_name = self._companion_stack.stack_name
   waiter = self._cfn_client.get_waiter("stack_delete_complete")
   delete_kwargs: Dict[str, Any] = {"StackName": stack_name}
   if self._role_arn:
       delete_kwargs["RoleARN"] = self._role_arn
   self._cfn_client.delete_stack(**delete_kwargs)
   waiter.wait(StackName=stack_name, WaiterConfig=self._delete_stack_waiter_config)

@Sanjays2402 Sanjays2402 Sep 9, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — _delete_companion_stack now passes RoleARN on delete_stack when the role is set, so the whole companion-stack lifecycle (create/update/delete) runs under one identity. Also covered by a new test asserting delete_stack gets the RoleARN kwarg; the no-role case still deletes with just StackName.

if exists:
self._cfn_client.update_stack(
StackName=stack_name, TemplateURL=template_url, Capabilities=["CAPABILITY_AUTO_EXPAND"]
)
self._cfn_client.update_stack(**stack_kwargs)
update_waiter = self._cfn_client.get_waiter("stack_update_complete")
update_waiter.wait(StackName=stack_name, WaiterConfig=self._update_stack_waiter_config)
else:
self._cfn_client.create_stack(
StackName=stack_name, TemplateURL=template_url, Capabilities=["CAPABILITY_AUTO_EXPAND"]
)
self._cfn_client.create_stack(**stack_kwargs)
create_waiter = self._cfn_client.get_waiter("stack_create_complete")
create_waiter.wait(StackName=stack_name, WaiterConfig=self._update_stack_waiter_config)

Expand Down Expand Up @@ -279,7 +285,13 @@ def is_repo_uri(self, repo_uri: Optional[str], function_logical_id: str) -> bool


def sync_ecr_stack(
template_file: str, stack_name: str, region: str, s3_bucket: str, s3_prefix: str, image_repositories: Dict[str, str]
template_file: str,
stack_name: str,
region: str,
s3_bucket: str,
s3_prefix: str,
image_repositories: Dict[str, str],
role_arn: Optional[str] = None,
) -> Dict[str, str]:
"""Blocking call to sync local functions with ECR Companion Stack

Expand All @@ -297,6 +309,10 @@ def sync_ecr_stack(
S3 prefix for the bucket
image_repositories : Dict[str, str]
Mapping between function logical ID and ECR URI
role_arn : Optional[str]
Optional service role ARN used when creating or updating the companion stack.
When provided, the companion stack is created/updated with the same service
role as the main stack.

Returns
-------
Expand All @@ -305,7 +321,7 @@ def sync_ecr_stack(
for Functions without a repo specified.
"""
image_repositories = image_repositories.copy() if image_repositories else {}
manager = CompanionStackManager(stack_name, region, s3_bucket, s3_prefix)
manager = CompanionStackManager(stack_name, region, s3_bucket, s3_prefix, role_arn=role_arn)

stacks = SamLocalStackProvider.get_stacks(template_file, language_extensions_enabled=False)[0]
function_provider = SamFunctionProvider(stacks, ignore_code_extraction_warnings=True)
Expand Down
61 changes: 60 additions & 1 deletion tests/unit/lib/bootstrap/companion_stack/test_companion_stack_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,45 @@ def test_update_companion_stack(
self.cfn_client.get_waiter.assert_called_once_with("stack_update_complete")
cfn_waiter.wait.assert_called_once_with(StackName=self.companion_stack_name, WaiterConfig=ANY)

def test_create_companion_stack_with_role_arn(
self,
):
self._test_companion_stack_with_role_arn(exists=False)

def test_update_companion_stack_with_role_arn(
self,
):
self._test_companion_stack_with_role_arn(exists=True)

@patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.mktempfile")
@patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.S3Uploader")
@patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.parse_s3_url")
def _test_companion_stack_with_role_arn(
self,
parse_s3_url_mock,
s3_uploader_mock,
mktempfile_mock,
exists,
):
role_arn = "arn:aws:iam::123456789012:role/CloudFormationServiceRole"
self.boto3_client_mock.side_effect = [self.cfn_client, self.ecr_client, self.s3_client, self.sts_client]
manager = CompanionStackManager(self.stack_name, "region", "s3_bucket", "s3_prefix", role_arn=role_arn)

cfn_waiter = Mock()
self.cfn_client.get_waiter.return_value = cfn_waiter

manager.does_companion_stack_exist = lambda: exists
manager.update_companion_stack()

stack_call = self.cfn_client.update_stack if exists else self.cfn_client.create_stack
stack_call.assert_called_once_with(
StackName=self.companion_stack_name, TemplateURL=ANY, Capabilities=ANY, RoleARN=role_arn
)
self.cfn_client.get_waiter.assert_called_once_with(
"stack_update_complete" if exists else "stack_create_complete"
)
cfn_waiter.wait.assert_called_once_with(StackName=self.companion_stack_name, WaiterConfig=ANY)

def test_delete_companion_stack(self):
cfn_waiter = Mock()
self.cfn_client.get_waiter.return_value = cfn_waiter
Expand Down Expand Up @@ -276,7 +315,27 @@ def test_sync_ecr_stack(self, function_provider_mock, stack_provider_mock, manag

result = sync_ecr_stack("template.yaml", "stack-name", "region", "s3-bucket", "s3-prefix", image_repositories)

manager_mock.assert_called_once_with("stack-name", "region", "s3-bucket", "s3-prefix")
manager_mock.assert_called_once_with("stack-name", "region", "s3-bucket", "s3-prefix", role_arn=None)
function_provider_mock.assert_called_once_with(stacks, ignore_code_extraction_warnings=True)
manager_mock.return_value.sync_repos.assert_called_once_with()

self.assertEqual(result, {"Function1": "uri1", "Function2": "uri2"})

@patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.CompanionStackManager")
@patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.SamLocalStackProvider")
@patch("samcli.lib.bootstrap.companion_stack.companion_stack_manager.SamFunctionProvider")
def test_sync_ecr_stack_with_role_arn(self, function_provider_mock, stack_provider_mock, manager_mock):
image_repositories = {"Function1": "uri1"}
stacks = MagicMock()
stack_provider_mock.get_stacks.return_value = (stacks, None)
manager_mock.return_value.get_repository_mapping.return_value = {"Function2": "uri2"}
role_arn = "arn:aws:iam::123456789012:role/CloudFormationServiceRole"

result = sync_ecr_stack(
"template.yaml", "stack-name", "region", "s3-bucket", "s3-prefix", image_repositories, role_arn=role_arn
)

manager_mock.assert_called_once_with("stack-name", "region", "s3-bucket", "s3-prefix", role_arn=role_arn)
function_provider_mock.assert_called_once_with(stacks, ignore_code_extraction_warnings=True)
manager_mock.return_value.sync_repos.assert_called_once_with()

Expand Down