Skip to content

feat!: convert to copier-managed skeleton - #11

Merged
oscar-hernandez-nttd merged 2 commits into
mainfrom
feat!/copier-conversion
Jul 22, 2026
Merged

feat!: convert to copier-managed skeleton#11
oscar-hernandez-nttd merged 2 commits into
mainfrom
feat!/copier-conversion

Conversation

@oscar-hernandez-nttd

Copy link
Copy Markdown
Contributor

Summary

Converts this repository to the copier-managed skeleton layout, bringing its CI/CD tooling, linting configuration, and agent guidance in line with the rest of the Launch Terraform module library. This is the first reference-architecture / collection module converted under this effort (prior conversions were primitives only) — a test run of the same flow one level up the stack.

Jira: EN-3436

This PR contains two commits:

  1. feat!: convert to copier-managed skeleton — the automated conversion via updater.sh, delivering updated Makefile, GitHub workflows, .agents/ reference docs, .tool-versions, and related tooling/config files. This is a breaking change for anyone relying on the previous hand-maintained CI setup. It intentionally does not touch anything under tests/ or the module's Terraform interface.

  2. fix!: bump lambda_function primitive constraint to ~> 2.0 and fix readonly test wiring — two required follow-ups surfaced by consuming the now-converted primitive and by the skeleton's own new tooling:

    • This module's main.tf pinned the lambda_function primitive with version = "~> 1.0". That primitive was itself converted and released as 2.0.0 in #48 — a pure feat! version bump with an empty variables.tf/outputs.tf/main.tf diff against 1.0.7. A pessimistic ~> constraint on 1.0 never resolves to 2.0.0, so without this bump the conversion here would silently keep consuming the old, unconverted primitive.
    • The conversion delivered .github/scripts/validate-readonly-test.sh, a new pre-commit hook that enforces a testing convention this repo's tests/ directory was already violating before this conversion — nobody was checking for it until now. tests/post_deploy_functional_readonly/main_test.go called lib.RunSetupTestTeardown (a full apply/destroy cycle, not read-only) and imported testimpl from the primitive module's package instead of this repo's own tests/testimpl. This commit rewires the entrypoint to lib.RunNonDestructiveTest and fixes the import to use this repo's own testimpl.TestComposableComplete, which is already non-mutating (read-only GetFunction assertions).

    It's a fix!: rather than folded into the feat!: commit because it corrects pre-existing defects the conversion's own new tooling and the primitive's release surfaced, not the conversion itself. It's marked breaking (!) because of the version constraint bump.

Test plan

  • make check passes locally (AWS_PROFILE=020127659860_AdministratorAccess AWS_REGION=us-east-2), including a real apply → assert → destroy cycle for the functional suite; plan shows 15 to add / 0 to change / 0 to destroy, confirming the version bump causes no resource replacement.
  • Pre-commit validate-readonly-test hook passes.
  • tests/post_deploy_functional_readonly builds and is statically confirmed non-destructive (RunNonDestructiveTest requires pre-existing deployed state and never calls apply/destroy).
  • No leftover terraform.tfstate under examples/*/ after the run.

…donly test wiring

The primitive module's own copier conversion (v2.0.0) is a pure major-version
bump with no interface changes, but the pessimistic ~> 1.0 constraint here
never resolves to it. Also corrects tests/post_deploy_functional_readonly,
which called lib.RunSetupTestTeardown (apply/destroy) instead of
lib.RunNonDestructiveTest, and imported testimpl from the primitive module
instead of this repo's own tests/testimpl package.

@bryce-lynn-nttd bryce-lynn-nttd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

APPROVE — First reference-module conversion; clean mechanical skeleton migration plus two well-justified follow-ups. Verified against head ba17fa6.

  • Scope is tight and correct. Conversion touches only tooling/CI/agent-config; the Terraform interface is untouched except the single load-bearing line in main.tf. .lcafenv removed, .copier-answers.yml pinned to skeleton 0.7.2, workflow SHAs single-sourced to launch-workflows@0.15.4.
  • Primitive constraint bump is safe. ~> 1.0~> 2.0 on module.lambda_function correctly consumes the now-converted primitive. Confirmed lambda_function 2.0.0 is a real release with an empty interface diff vs 1.0.7, so the bump is API-compatible — consistent with the reported 15-add / 0-change / 0-destroy plan. fix! framing is the conservative-correct call.
  • Readonly test wiring corrected. Entrypoint moved from RunSetupTestTeardown to RunNonDestructiveTest, and the import fixed from the primitive's package to this repo's own tests/testimpl. TestComposableComplete is genuinely non-mutating (only GetFunction reads with specific-value assert.Equal on ARN / name / override-name), so it's valid for both the readonly and functional suites. The new validate-readonly-test hook correctly caught the pre-existing defect.
  • CI fully green.

Nice work — the reference-tier flow holds up.

Generated with Cursor Agent (Opus 4.8)

@rakesh-gorige-nttd rakesh-gorige-nttd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR Review: feat!: convert to copier-managed skeleton (#11)

Repo: launchbynttdata/tf-aws-module_reference-bulk_lambda_function
Author: @oscar-hernandez-nttd
Branch: feat!/copier-conversionmain
Jira: EN-3436
Head: ba17fa6 (2 commits)


What This PR Does Well

  • Clean two-commit structure. Separating the mechanical skeleton conversion (feat!:) from the load-bearing follow-ups (fix!:) makes review and release notes much easier to follow. The PR description explains the rationale well.
  • First reference-architecture conversion. Good pilot for the batch — proves the copier flow works one level above primitives without touching the module's Terraform interface (except the version constraint).
  • Primitive constraint bump is correct and necessary. ~> 1.0~> 2.0 on module.lambda_function is required to consume the converted primitive. I verified lambda_function 2.0.0 has no diff on main.tf, variables.tf, or outputs.tf vs 1.0.7 — the bump is semver-only, not an API change.
  • Readonly test wiring is properly fixed. The pre-existing defects were worse than just the wrong runner:
    • RunSetupTestTeardownRunNonDestructiveTest (was doing full apply/destroy in a "readonly" suite)
    • Import fixed from the primitive's tests/testimpl to this repo's own tests/testimpl — the old import would have exercised the wrong module config and wrong assertions (single-function vs bulk map outputs)
    • Entrypoint renamed to TestBulkLambdaFunctionsModuleReadOnly for clarity
  • TestComposableComplete is genuinely non-destructive. It only calls GetFunction and uses specific-value assert.Equal on ARNs/names/overrides — valid for both functional and readonly suites.
  • Skeleton enrollment looks correct. .copier-answers.yml pinned to skeleton 0.7.2; workflow SHAs consistently point to launch-workflows@edddecd (tag 0.15.4); legacy workflows removed as expected.
  • CI fully green — pre-commit, Terraform lint, Terraform tests (including apply/assert/destroy), and legacy status checks all pass.

Fixes Required

Blocking

No blocking issues found.

Non-Blocking

# File & Line Issue
1 .github/release-drafter.yml autolabeler section removed (branch-name → major/minor/patch labeling). This is intentional per skeleton standardization — labeling now comes from the conventional-commit PR title workflow (breaking change, feature, etc.). No action needed unless the team wants to document this for reference-module consumers.
2 PR title Title is feat!: convert to copier-managed skeleton but the second commit is fix!: . CI accepted it, and the PR body covers both commits well. Consider noting in merge commit / release notes that this PR ships both a skeleton conversion and a primitive constraint bump.
3 tests/post_deploy_functional_readonly/main_test.go RunNonDestructiveTest requires pre-existing deployed state. That's correct by design, but worth confirming in team docs that the readonly suite is for post-deploy validation only (not CI apply/destroy).

Summary

This is a strong reference-tier copier conversion. The skeleton migration is mechanical and complete, and the follow-up commit addresses two real pre-existing problems that the new validate-readonly-test hook correctly surfaced. The primitive version bump is safe and necessary.

Recommendation: APPROVE


Test Plan Verification

Item Status
Pre-commit / validate-readonly-test CI pass
Terraform lint + validate + plan CI pass
Functional suite (apply → assert → destroy) CI pass
Readonly suite statically non-destructive Verified in diff
No module interface change beyond version constraint Verified

@oscar-hernandez-nttd
oscar-hernandez-nttd merged commit 53e5cd9 into main Jul 22, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants