Skip to content

Add Service Discovery permissions and terminator for ecs_service#330

Open
jonpspri wants to merge 2 commits intomattclay:mainfrom
jonpspri:ecs_service
Open

Add Service Discovery permissions and terminator for ecs_service#330
jonpspri wants to merge 2 commits intomattclay:mainfrom
jonpspri:ecs_service

Conversation

@jonpspri
Copy link

@jonpspri jonpspri commented Feb 12, 2026

Summary

  • Expand servicediscovery IAM permissions to support all namespace types (HTTP, Public DNS, Private DNS) along with GetNamespace, ListServices, and TagResource
  • Add ELB service-linked role creation permission to the paas policy
  • ServiceDiscoveryNamespace terminator to clean up all namespace types, including deleting child services before namespace removal

Context

These changes support the ecs_service and cloudmap integration tests in community.aws:

Test plan

  • Verify terminator discovers and cleans up all namespace types (HTTP, DNS_PUBLIC, DNS_PRIVATE) in the test account
  • Verify child services are deleted before namespace removal
  • Verify IAM policies deploy without errors

Copilot AI review requested due to automatic review settings February 12, 2026 13:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds AWS Cloud Map (Service Discovery) support needed for ecs_service Service Connect integration tests by expanding IAM policies and adding a cleanup terminator for stale HTTP namespaces.

Changes:

  • Extend networking IAM policy with servicediscovery permissions required for Cloud Map HTTP namespace operations.
  • Extend paas IAM policy to allow creation of the ELB service-linked role.
  • Add a new terminator to discover and delete stale Cloud Map HTTP namespaces.
  • Update aws/config.yml account IDs and region (appears unrelated to the stated PR scope).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
aws/terminator/networking.py Adds ServiceDiscoveryHttpNamespace terminator to list/delete stale HTTP namespaces.
aws/policy/paas.yaml Grants iam:CreateServiceLinkedRole for ELB service-linked role creation.
aws/policy/networking.yaml Adds servicediscovery permissions for Cloud Map HTTP namespace support.
aws/config.yml Changes lambda/access/test account IDs and AWS region (not mentioned in PR description).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Add IAM permissions and terminator class to support ecs_service
Service Connect integration tests:

- Add servicediscovery permissions (CreateHttpNamespace, DeleteNamespace,
  GetOperation, ListNamespaces) to networking policy
- Add ELB service-linked role creation permission to paas policy
- Add ServiceDiscoveryHttpNamespace terminator class for cleanup of
  HTTP namespaces created during integration tests

Expand Service Discovery support for all namespace types

Generalize the terminator from HTTP-only to all namespace types
(HTTP, DNS_PUBLIC, DNS_PRIVATE) and delete child services before
deleting namespaces. Add corresponding IAM permissions.

Extract ECS task definition cleanup into its own terminator class

Move task definition lifecycle management out of Ecs.terminate() into a
new EcsTaskDefinition(DbTerminator) class. This fixes the blast radius
issue where cleaning up any stale cluster would deregister all task
definitions in the account. Task definitions are now independently
tracked and aged via DynamoDB.

Also adds ecs:DeleteTaskDefinitions to the paas policy and fully deletes
(not just deregisters) stale task definitions. Subsumes PR mattclay#331.

Split ECS policy into separate paas-ecs policy file
@jonpspri jonpspri force-pushed the ecs_service branch 2 times, most recently from 0f514d0 to 9c6ecb5 Compare February 17, 2026 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments