Skip to content

refactor(cloudformation): move AWS::Logs::LogGroup into its own provisioner - #2756

Open
hectorvent wants to merge 1 commit into
refactor/cfn-s3-provisionerfrom
refactor/cfn-logs-provisioner
Open

refactor(cloudformation): move AWS::Logs::LogGroup into its own provisioner#2756
hectorvent wants to merge 1 commit into
refactor/cfn-s3-provisionerfrom
refactor/cfn-logs-provisioner

Conversation

@hectorvent

Copy link
Copy Markdown
Collaborator

Summary

Migrates AWS::Logs::LogGroup into LogsCfnProvisioner. Behaviour-preserving, but this is the
first slice carrying a real update path: the body decides between reconciling in place and
replacing, based on the prior physical id and the recorded FlociLogGroupNameMode, and infers that
mode from the physical id's shape for stacks persisted before the attribute existed.

Monolith 7,315 → 7,208 lines; legacy switch 70 → 69 types. Docs table unchanged, as expected.

Stacked on #2754#2744#2739. Review those first.

Three things this slice needed that earlier ones did not

UPDATE_ROLLBACK_RESTORED_ATTR moved to CfnRollback. It was package-private on the monolith,
so unreachable from the provisioners package. CfnRollback exists for exactly this (it was added
to solve the same problem for the IAM role rollback constants). The monolith's constant now
delegates there, so CloudFormationService's two references are untouched.

Uses ctx.priorPhysicalId() rather than reading it back off the StackResource. The two are
equal on entry, but the method assigns the new physical id before it finishes, so a
context-captured value cannot later be invalidated by an edit to the body. This is the first real
use of the accessor added in #2732.

The existing regression test stays pointed at the dispatcher, now with the provisioner
registered. That is load-bearing, not decoration, and I verified it by mutation: dropping the
registration makes both tests fail with

expected: <test-stack-LogGroup-abc123def456> but was: <LogGroup-8df62014>

which is the dispatcher's stub arm answering instead of the provisioner. Testing through the
dispatcher also covers the plumbing this migration changes, that existingPhysicalId and
existingAttributes reach ProvisionContext.

reconcileLogGroup moves with the slice; isGeneratedName and its prefix helper are copied,
because a not-yet-migrated type still calls them. logsService leaves the monolith entirely (its
constructor parameter stays, unused, until the endgame).

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

AWS Compatibility

N/A for the wire protocol. aws-logs-loggroup.json gives primaryIdentifier LogGroupName and
read-only Arn; both are asserted, Arn in the exact
arn:aws:logs:<region>:<account>:log-group:<name>:* form AWS uses.

The AWS behaviour this preserves is that LogGroupName is not updatable in place: a rename creates
the replacement before deleting the original, so a colliding name fails the update without
destroying the existing group.

Checklist

  • ./mvnw test passes locally
  • New or updated integration test added
  • Commit messages follow Conventional Commits

CloudFormation + Cloud Control: 854 tests, 0 failures, 0 errors. LogsCfnProvisionerTest adds
11 covering both update branches, retention set and cleared, tag removal on reconcile, an
unparseable RetentionInDays, the explicit-name-removed replacement, the generated-name-kept
no-op, and the failed-rename case that must leave the original intact and flag the rollback. The
#1965 legacy-name-mode regression test is preserved unchanged apart from the registration line.

…sioner

Behaviour-preserving move of the log group's provision and delete paths.
Unlike the earlier slices this one carries a real update path: the body
decides between reconciling in place and replacing, from the prior physical
id and the recorded FlociLogGroupNameMode, and it infers that mode from the
physical id's shape for stacks persisted before the attribute existed.

Uses ctx.priorPhysicalId() rather than reading it back off the StackResource.
The two are equal on entry, but the method assigns the new physical id before
it finishes, so a context-captured value cannot be invalidated by a later
edit to the body.

UPDATE_ROLLBACK_RESTORED_ATTR moves to CfnRollback, which exists for exactly
this: it was package-private on the monolith and so unreachable from the
provisioners package. The monolith's constant now delegates there, leaving
CloudFormationService's two references untouched.

reconcileLogGroup moves with the slice. isGeneratedName and its prefix helper
are copied, because a type that has not migrated yet still calls them, and
logsService leaves the monolith entirely.

CloudFormationLogGroupProvisionerTest keeps testing through the dispatcher,
now with the provisioner registered. That is load-bearing rather than
decoration: with an empty registry the type would reach the stub arm and the
test would assert against a synthetic id. Going through the dispatcher also
covers the plumbing this migration changed, that existingPhysicalId and
existingAttributes reach ProvisionContext.
@hectorvent
hectorvent requested a review from pgermosen as a code owner August 30, 2026 05:56
@greptile-apps

greptile-apps Bot commented Aug 30, 2026

Copy link
Copy Markdown

Greptile Summary

Migrates AWS::Logs::LogGroup from the legacy CloudFormation switch into a dedicated CDI-discovered provisioner while preserving create, update, replacement, deletion, rollback-marker, Ref, and Fn::GetAtt behavior.

  • Moves log-group reconciliation and deletion into LogsCfnProvisioner.
  • Relocates the shared update-rollback marker to CfnRollback.
  • Updates dispatcher regression tests, focused provisioner tests, and the resource inventory mapping.

Confidence Score: 5/5

The PR appears safe to merge because the extracted provisioner preserves the existing LogGroup lifecycle behavior and remains covered by registry and focused tests.

Registry-first provisioning and deletion reach the CDI-discovered LogGroup provisioner, while physical IDs, attributes, name-mode migration, reconciliation, and rollback-marker behavior remain equivalent to the prior implementation.

Important Files Changed

Filename Overview
src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/LogsCfnProvisioner.java Extracts LogGroup create, update, replacement, reconciliation, attribute generation, and delete behavior without an actionable semantic regression.
src/main/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationResourceProvisioner.java Removes the migrated switch and delete arms while retaining registry-first dispatch and the shared rollback constant alias.
src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/CfnRollback.java Exposes the unchanged update-rollback marker value to extracted provisioners.
src/test/java/io/github/hectorvent/floci/services/cloudformation/provisioners/LogsCfnProvisionerTest.java Adds focused coverage for creation, reconciliation, replacement, rollback marking, attributes, and deletion.
src/test/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationLogGroupProvisionerTest.java Preserves dispatcher-level legacy name-mode coverage with explicit provisioner registration.
src/test/resources/cloudformation/supported-resource-types.tsv Updates the inventory owner from the legacy switch to LogsCfnProvisioner.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[CloudFormation update] --> B[Resource registry]
    B --> C[LogsCfnProvisioner]
    C --> D{Same physical name exists?}
    D -->|Yes| E[Reconcile retention and tags]
    D -->|No| F[Create replacement]
    F --> G[Delete prior group when present]
    E --> H[Store physical ID, ARN, and name mode]
    G --> H
Loading

Reviews (1): Last reviewed commit: "refactor(cloudformation): move AWS::Logs..." | Re-trigger Greptile

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