fix: strip :* suffix from log group ARN before listing tags

[james00012]

Fixes #1150

Problem

cloudwatch-loggroup silently deletes nothing. DescribeLogGroups returns the Arn field with a trailing :* suffix (e.g. arn:aws:logs:us-east-1:123:log-group:/aws/eks/x:*), but ListTagsForResource rejects that form with ValidationException: Invalid resourceArn. The error was caught and the log group skipped via continue, so no matching log group was ever returned for deletion. Regression introduced in #1103, which added the tag-listing call.

Fix

• Use the LogGroupArn field, which AWS returns without the :* suffix specifically for the tagging-API resourceArn, falling back to a trimmed Arn only when LogGroupArn is absent.
• Raise the tag-list failure log from Debug to Warn. The skip is intentional (a log group whose tags we cannot read may carry an exclude tag, so failing closed is safer for a destructive tool), but it should be visible rather than silent.

Testing

• TestListCloudWatchLogGroups_TagInclusionFilter covers both the LogGroupArn path and the Arn fallback. The mock now rejects any :*-suffixed ARN, so a regression to the unnormalized ARN fails the suite.
• TestListCloudWatchLogGroups_SkipsOnTagListError covers the skip-on-error path.
• Verified end-to-end against a live AWS account: the pre-fix binary reports Invalid resourceArn for every log group and finds none; the fixed binary finds and deletes the target group.

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Fixed CloudWatch Log Group tag retrieval by correctly handling trailing ARN suffixes returned during log-group discovery.
  • Improved resilience when tag listing fails by skipping the affected log group instead of failing the entire tag retrieval process.
  • Updated and added tests to cover ARN suffix handling and skip-on-tag-error behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7841751a-f809-446c-bbab-6ad5195edb5b

📥 Commits

Reviewing files that changed from the base of the PR and between 9d48d3b and 39bbfeb.

📒 Files selected for processing (2)

• aws/resources/cloudwatch_loggroup.go
• aws/resources/cloudwatch_loggroup_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• aws/resources/cloudwatch_loggroup.go

---

Walkthrough

listCloudWatchLogGroups now sanitizes ARNs before tag lookup by preferring logGroup.LogGroupArn or falling back to trimming the trailing :* from logGroup.Arn. The strings import supports suffix removal. Tests are updated to use realistic :* ARN suffixes, with mock validation rejecting invalid formats and a new test verifying graceful skipping when tag retrieval fails.

Changes

CloudWatch Log Group ARN suffix fix

Layer / File(s)	Summary
ARN sanitization in listCloudWatchLogGroups aws/resources/cloudwatch_loggroup.go	Imports strings and updates listCloudWatchLogGroups to construct a valid ResourceArn for ListTagsForResource by preferring logGroup.LogGroupArn, otherwise trimming the :* suffix from logGroup.Arn. Tag-list errors now log at warn level with explicit skip notation.
Test mock validation and error-skipping coverage aws/resources/cloudwatch_loggroup_test.go	Mock ListTagsForResource now rejects ARNs ending in :* and returns ServiceUnavailableException for tag-error ARNs. Existing test data updated to use realistic :* suffixes. New test TestListCloudWatchLogGroups_SkipsOnTagListError verifies log groups are skipped when tag retrieval fails.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

An ARN walked in with a :* on its tail,
DescribeLogGroups returned it unchanged.
But ListTagsForResource cried "That's invalid, pal!"
So we checked LogGroupArn or trimmed—rearranged.
Now cloud-nuke deletes what it's meant to, no fail. ☁️✂️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main fix: stripping the :* suffix from CloudWatch log group ARNs before calling ListTagsForResource.
Description check	✅ Passed	The description thoroughly covers the problem, the fix, testing approach, and end-to-end verification. However, it lacks some template sections like release notes and migration guide.
Linked Issues check	✅ Passed	The PR fully addresses issue #1150 by implementing the core fix (using LogGroupArn with trimmed Arn fallback) and raising logging visibility from Debug to Warn as intended.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the ARN validation issue: the main logic change, test coverage including regression detection, and improved logging.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/cloudwatch-loggroup-arn-tags

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}