Skip to content

chore(audit): close remaining stub gaps + ECS tag coverage#767

Merged
vieiralucas merged 2 commits intomainfrom
worktree-batch10-cleanup-audit
Apr 25, 2026
Merged

chore(audit): close remaining stub gaps + ECS tag coverage#767
vieiralucas merged 2 commits intomainfrom
worktree-batch10-cleanup-audit

Conversation

@vieiralucas
Copy link
Copy Markdown
Member

@vieiralucas vieiralucas commented Apr 25, 2026

Summary

  • Final batch of the 10-batch "fill all not-yet-implemented gaps" pass.
  • ECS: TagResource/UntagResource/ListTagsForResource now cover service, task, task-set, container-instance, and capacity-provider on top of cluster + task-definition.
  • SSM SendCommand/ListCommands return StatusDetails mirroring the command status (was literal "Details placeholder").
  • DynamoDB + SSM "Stubs" comment headers retitled to "Synthetic defaults" — the listed ops are a mix of canned synthetic responses and real-state-backed operations; "Stubs" was misleading after batches 7+ landed real impls.
  • ELBv2 docs already trimmed in batch 2; verified no stale "Not yet implemented" remains beyond the documented HTTP-routing data-plane non-goal.

Test plan

  • cargo test -p fakecloud-ssm --lib (199 pass)
  • cargo test -p fakecloud-ecs --lib (30 pass)
  • New E2E tag_untag_service_and_capacity_provider passes
  • Existing E2E tag_untag_cluster_and_task_definition still passes
  • cargo clippy --workspace --all-targets -- -D warnings clean
  • cargo fmt --check clean

Summary by cubic

Expands ECS tagging to services, tasks, task sets, container instances, and capacity providers, and fixes short-ARN lookups for services and container instances. Also aligns SSM command StatusDetails with Status and clarifies DynamoDB/SSM comments as “Synthetic defaults.”

  • New Features

    • ECS: TagResource/UntagResource/ListTagsForResource now support service, task, task-set, container-instance, and capacity-provider.
    • Added E2E test for service and capacity-provider tagging.
  • Bug Fixes

    • ECS: Tag/Untag/ListTags now resolve both short- and long-form ARNs for service and container-instance (no more false ResourceNotFound).
    • SSM: SendCommand and ListCommands now set StatusDetails to match Status (was a placeholder).

Written for commit 83963f9. Summary will update on new commits.

Closes the audit pass after the 9 prior implementation batches. Three
real fixes plus a doc-honesty pass:

- ECS: TagResource/UntagResource/ListTagsForResource now cover service,
  task, task-set, container-instance, and capacity-provider on top of
  the existing cluster + task-definition. The state already carried tag
  vectors on each of these structs; the dispatch was rejecting them
  with "not yet supported".
- SSM SendCommand/ListCommands no longer return literal "Details
  placeholder" for StatusDetails; mirrors the command Status as AWS does.
- DynamoDB and SSM "Stubs" comment headers retitled to "Synthetic
  defaults" — the listed ops are intentional canned defaults
  (DescribeEndpoints, DescribeLimits, GetConnectionStatus, GetCalendarState,
  DescribePatchGroupState, DescribeAvailablePatches) plus real-state ops
  (DescribePatchProperties, GetDefaultPatchBaseline, GetServiceSetting,
  ResetServiceSetting, UpdateServiceSetting). "Stubs" was misleading.
- New E2E `tag_untag_service_and_capacity_provider` exercises the
  expanded ECS tagging surface.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 25, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 101 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/fakecloud-ecs/src/service.rs 49.49% 100 Missing ⚠️
crates/fakecloud-ssm/src/service/mod.rs 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/fakecloud-ecs/src/service.rs">

<violation number="1" location="crates/fakecloud-ecs/src/service.rs:1220">
P2: Service tag operations use `tail` directly as the state key, which breaks for short service ARNs (`service/<name>`) because stored keys are `cluster/service`.</violation>

<violation number="2" location="crates/fakecloud-ecs/src/service.rs:1242">
P2: Container-instance tag operations lookup by raw ARN tail, which fails for short ARNs because state keys are stored as `cluster/id`.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread crates/fakecloud-ecs/src/service.rs
Comment thread crates/fakecloud-ecs/src/service.rs
Cubic flagged the new tag handlers using the raw ARN tail as the
storage-map key. Long-form ARNs already match (`cluster/name`), but
short-form ARNs (account-setting `serviceLongArnFormat=disabled` or
`containerInstanceLongArnFormat=disabled`) carry only the bare name,
so lookups returned ResourceNotFound.

- Add `resolve_service_key` and `resolve_container_instance_key`
  helpers that recognise both forms: long tail used directly when
  present, short tail resolved by scanning for a stored key ending
  with `/<tail>`.
- Wire all three tag operations (Tag/Untag/ListTags) through the
  resolver for service + container-instance.
- Unit tests cover the two-form lookup and unknown-name fallthrough.
@vieiralucas vieiralucas merged commit 7aa57dc into main Apr 25, 2026
38 checks passed
@vieiralucas vieiralucas deleted the worktree-batch10-cleanup-audit branch April 25, 2026 22:47
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.

1 participant