docs: post-merge retro for PR #657 (issue #653 AWS IaC removal)#658
Merged
Conversation
Force-cutover of 6 AWS IaC modules to workflow-plugin-aws v0.2.0. Key divergence from #617: cloud_account_aws.go stays (AWSConfigProvider used by Phase 2 out-of-scope files). platform.dns module type stays; only Route53 backend is removed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes: infra.autoscaling_group missing from core (C-1), platform.dns validate-path guard gap (C-2), step count 15 not 16 (I-1), example/go.mod tidy + grep gate (I-2), platform.dns schema description stale (I-3), T1 file list ambiguity (m-1), T2 backend alternative justification (m-2), T3 DNS row keep/remove clarity (m-3). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add module/app_container.go as partial-edit target in T1: move ECSContainer struct in from platform_ecs.go, remove case *PlatformECS type switch branch, update default error message. - Record cycle 2 findings in adversarial review history. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ix refinement) After removing case *PlatformECS, all ECS-specific structs and methods in app_container.go become dead code. Design updated to remove them entirely rather than leave dead code in place. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix schema: infra.autoscaling_group auto-generated from infraTypes, no manual module_schema.go entry needed. - Add app_container.go comment lines to string-update list. - Record cycle 3 PASS in adversarial review history. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
6 tasks, single PR. Covers: - T1: delete 14 files + app_container.go C-3 fix + regression gate - T2: replace Route53 backend with migration error stub - T3: strip registration sites + add infra.autoscaling_group - T4: internal/legacyaws + migration errors in engine + wfctl - T5: modernize legacy-aws-types rule + migration doc - T6: go mod tidy + CI grep gate Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cycle 1) - T2: Init() succeeds for provider:aws (factory returns struct not error); migration error fires at Plan() time — fix test to call m.Plan() not m.Init(); fix mock import (module.NewMockApplication not mock.); remove deleted Route53 tests from file list - T4: replace nonexistent minimalConfigWithModule() with newIsolatedEngine(t) + config.WorkflowConfig pattern from engine_legacy_do_migration_test.go; fix package (workflow not workflow_test); fix Create() call to 4-arg signature; add PluginLoaded variant mirroring DO test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- T5 test used nonexistent findRule/check/fix helpers and wrong package (modernize_test); actual pattern is package modernize, direct legacyAWSRule() call, yaml.Unmarshal + rule.Check/Fix — mirrors legacy_do_rule_test.go exactly - Remove duplicate "Add to pipeline step registry tests" paragraph - Fix yaml.Marshal round-trip in Fix test to match actual pattern Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove platform.ecs, platform.networking, platform.apigateway, platform.autoscaling module types and their 15 step types (ecs/apigw/scaling/network) from all registration sites: plugins/platform, schema/schema.go, schema/module_schema.go, schema/step_schema_builtins.go, cmd/wfctl/type_registry.go. Add infra.autoscaling_group to plugins/infra and type_registry.go. Update DOCUMENTATION.md with AWS IaC removal notice + infra.autoscaling_group row. Fix multi_region.go error string to reference infra.container_service. Regenerate schema/testdata/editor-schemas.golden.json. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Create internal/legacyaws/types.go mirroring internal/legacydo/types.go. Maps 4 removed module types (platform.ecs/networking/apigateway/autoscaling) and 15 removed step types to their infra.*/step.iac_* successors. RemovedInVersion = v0.53.0. Wire into engine.go, cmd/wfctl/validate.go, cmd/wfctl/ci_validate.go: - extra schema module types list includes legacyaws.ModuleTypes - post-validate sweep checks legacyaws.IsModuleType / IsStepType - actionable FormatModuleError / FormatStepError returned on match Remove 15 legacy step entries from cmd/wfctl/type_registry.go KnownStepTypes(). Tests: engine_legacyaws_test.go (plugin not loaded + plugin loaded branches) + cmd/wfctl/legacy_aws_types_removed_test.go (registry absent + validate/ci_validate paths). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add modernize/legacy_aws_rule.go mirroring legacy_do_rule.go: - ID: legacy-aws-types, Severity: error - Auto-fixes: platform.ecs→infra.container_service, platform.apigateway→infra.api_gateway, platform.autoscaling→infra.autoscaling_group - Flags but does not auto-fix: platform.networking (1→2 split) + all 15 step types (config key shape mismatch: platform + state_store vs legacy service/gateway/scaling keys) - Uses walkTypeNodes helper from legacy_do_rule.go (same package) Register legacyAWSRule() in modernize/modernize.go AllRules() list. Add docs/migrations/v0.53.0-aws-iac-removal.md with full migration recipe (install plugin, modernize, add provider, manual rewrites for networking + steps). Tests: TestLegacyAWSRule_Rewrites (3 auto-fixable types) + TestLegacyAWSRule_GapTypesFlaggedNotRewritten (networking + 6 step types). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
go mod tidy drops 3 freed AWS SDK packages: - service/apigatewayv2 (was platform.apigateway) - service/applicationautoscaling (was platform.autoscaling) - service/route53 (was platform.dns Route53 backend) Also tidy example/go.mod. Add aws-sdk-banned CI job to .github/workflows/ci.yml mirroring the godo-banned job: grep-gates both *.go imports and go.mod entries for the three freed service paths, excluding aws_absent_test.go. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TestModernizeAllRulesRegistered: expected 9 rules → 10 (add legacy-aws-types). TestKnownStepTypesPopulated: remove 15 legacy AWS step entries from expected list (step.ecs_*/step.network_*/step.apigw_*/step.scaling_* all removed from KnownStepTypes). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a post-merge retrospective document for the AWS IaC removal effort (#653 / PR #657), and the diff also includes broad codebase changes that complete/lock in the AWS IaC cutover (removing legacy platform.* AWS IaC modules/steps, adding migration messaging, and enforcing SDK bans).
Changes:
- Remove legacy AWS IaC module/step types (
platform.ecs/networking/apigateway/autoscaling+ relatedstep.*) from core registration points and schemas. - Add migration enforcement/UX:
internal/legacyawsmappings, engine/wfctl validation errors, and awfctl modernizerule (legacy-aws-types). - Add CI/test regression gates preventing reintroduction of the removed AWS SDK service packages; update docs/migrations and schema golden output.
Reviewed changes
Copilot reviewed 49 out of 51 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| schema/testdata/editor-schemas.golden.json | Updates editor golden schemas to reflect removed legacy types and updated descriptions. |
| schema/step_schema_builtins.go | Removes built-in step schema registrations for deleted legacy AWS steps. |
| schema/schema.go | Removes legacy AWS module/step identifiers from the core type list. |
| schema/module_schema.go | Removes module schemas for deleted legacy AWS modules; updates platform.dns messaging; adjusts app.container descriptions. |
| plugins/platform/plugin.go | Removes legacy AWS module/step types from the platform plugin manifest and factories. |
| plugins/platform/plugin_test.go | Updates platform plugin tests to stop expecting removed step/module factories. |
| plugins/infra/plugin.go | Adds infra.autoscaling_group to the infra abstract type registry. |
| module/platform_networking_test.go | Deletes tests for removed legacy networking module/steps. |
| module/platform_ecs.go | Deletes the legacy ECS module implementation. |
| module/platform_ecs_test.go | Deletes tests for removed legacy ECS module/steps. |
| module/platform_dns.go | Switches provider: aws DNS backend to a migration-error backend. |
| module/platform_dns_test.go | Updates DNS tests to assert the new migration error behavior for provider: aws. |
| module/platform_dns_backends.go | Removes Route53 backend code; adds an AWS migration-error backend. |
| module/platform_aws_integration_test.go | Deletes integration tests for removed legacy AWS IaC surfaces. |
| module/platform_autoscaling.go | Deletes legacy autoscaling module implementation. |
| module/platform_autoscaling_test.go | Deletes tests for removed legacy autoscaling module/steps. |
| module/platform_apigateway.go | Deletes legacy API gateway module implementation. |
| module/platform_apigateway_test.go | Deletes tests for removed legacy API gateway module/steps. |
| module/pipeline_step_networking.go | Deletes legacy networking pipeline steps. |
| module/pipeline_step_ecs.go | Deletes legacy ECS pipeline steps. |
| module/pipeline_step_autoscaling.go | Deletes legacy autoscaling pipeline steps. |
| module/pipeline_step_apigateway.go | Deletes legacy API gateway pipeline steps. |
| module/multi_region.go | Updates error messaging to point to plugin-based successors (removes platform.ecs mention). |
| module/aws_api_gateway.go | Deletes AWS API Gateway helper module. |
| module/aws_absent_test.go | Adds a regression test to ensure removed AWS SDK service packages aren’t imported under module/. |
| module/app_container.go | Removes ECS app backend handling; improves error/help text around the removed ECS platform. |
| module/api_gateway_test.go | Removes tests that referenced the deleted AWS API gateway helper. |
| modernize/modernize.go | Registers the new legacy AWS modernize rule. |
| modernize/legacy_aws_rule.go | Adds legacy-aws-types rule to flag/auto-fix legacy AWS module types and flag steps. |
| modernize/legacy_aws_rule_test.go | Adds coverage for the legacy AWS modernize rule behavior. |
| internal/legacyaws/types.go | Introduces shared legacy AWS type mappings and migration error formatters. |
| go.mod | Drops direct dependencies on removed AWS SDK service packages. |
| go.sum | Removes checksums for dropped AWS SDK service packages. |
| example/go.mod | Removes indirect dependencies on the dropped AWS SDK service packages. |
| example/go.sum | Removes checksums for removed indirect AWS SDK packages. |
| engine.go | Adds legacy AWS module types to validation extras and emits actionable migration errors. |
| engine_legacyaws_test.go | Adds tests for engine legacy AWS migration error branches. |
| DOCUMENTATION.md | Updates docs to remove legacy AWS module/step references and document the new plugin-based path. |
| docs/retros/2026-05-13-issue-653-aws-iac-removal-retro.md | Adds the post-merge retrospective for #653/#657. |
| docs/plans/2026-05-13-issue-653-phase1-aws-cutover.md.scope-lock | Adds scope-lock hash artifact for the plan. |
| docs/migrations/v0.53.0-aws-iac-removal.md | Adds a migration guide for the v0.53.0 AWS IaC cutover. |
| cmd/wfctl/validate.go | Adds legacy AWS type allowance + actionable migration errors to wfctl validate. |
| cmd/wfctl/ci_validate.go | Adds legacy AWS type allowance + actionable migration errors to wfctl ci_validate. |
| cmd/wfctl/type_registry.go | Removes legacy AWS types from the wfctl registry; fixes platform.dns keys; adds infra.autoscaling_group. |
| cmd/wfctl/type_registry_test.go | Updates registry tests to no longer expect removed legacy AWS step types. |
| cmd/wfctl/modernize_test.go | Verifies the new legacy AWS modernize rule is registered. |
| cmd/wfctl/legacy_aws_types_removed_test.go | Adds wfctl tests locking absence of legacy AWS types and validating actionable migration errors. |
| .github/workflows/ci.yml | Adds a CI grep gate preventing reintroduction of removed AWS SDK service packages. |
Comment on lines
+33
to
+36
| f, _ := parser.ParseFile(fset, path, nil, parser.ImportsOnly) | ||
| if f == nil { | ||
| return nil // skip unparseable files | ||
| } |
Comment on lines
+109
to
+115
| func (b *awsRoute53ErrorBackend) err(m *PlatformDNS) error { | ||
| return fmt.Errorf( | ||
| "platform.dns %q: AWS Route53 backend removed from workflow core in v0.53.0 (issue #653).\n"+ | ||
| "Migrate to: infra.dns (provider: aws) with workflow-plugin-aws v0.2.0+.\n"+ | ||
| "Install: https://github.com/GoCodeAlone/workflow-plugin-aws\n"+ | ||
| "See docs/migrations/v0.53.0-aws-iac-removal.md", | ||
| m.name, |
Comment on lines
+1
to
+6
| # Retro: Issue #653 Phase 1 — Remove AWS IaC Modules from Workflow Core | ||
|
|
||
| **PR:** [#657](https://github.com/GoCodeAlone/workflow/pull/657) — feat(#653): Phase 1 — remove AWS IaC modules from workflow core | ||
| **Merged:** 2026-05-13 (sha `1389d024`) | ||
| **Branch:** `feat/issue-653-aws-iac-cutover-v2` | ||
| **Design:** `docs/plans/2026-05-13-issue-653-phase1-aws-cutover-design.md` |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Retro doc for PR #657 (feat(#653): remove AWS IaC modules from workflow core).
Covers adversarial-review findings scored, 2 gate misses (nilerr lint + ConfigKeys mismatch), skill activation audit, and plugin-level follow-ups building on the #617 precedent.
See: docs/retros/2026-05-13-issue-653-aws-iac-removal-retro.md