Repository navigation
Conversation
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
This is a well-scoped, deliberately bounded experimental Terraform backend. The implementation hangs together:
- Strict
preflight()validation (src/core/project/backends/terraform/compiler.ts) rejects every unsupported field before any I/O, with good test coverage viatest.eachincompiler.test.ts. - Customer strings are consistently sent through
literal()to escape${...}and%{...}so Terraform never interpolates untrusted input, while resource references are preserved. - Resource addresses use
key()(name + stable 10-char sha256 suffix), so reordering resources inagentcore.jsondoes not force replacement — covered by the "reordering resources preserves addresses" test. - The custom
.agentcore-operation.lockcovers package → generate → init → plan → apply as one unit, independent of Terraform's own state lock; the error message tells the user how to recover from a stuck lock. - Credentials are resolved per operation, re-materialized for
apply, andAWS_PROFILE/AWS_WEB_IDENTITY_TOKEN_FILE/AWS_ROLE_ARN/ etc. are stripped from the env so Terraform cannot bypass the resolved credentials. Account mismatch is checked before Terraform runs. TerraformAssetPackagerproduces deterministic ZIPs (fixed mtime, sorted entries, excluded.env*/.venv/node_modules/terraform, symlink rejection, 250 MiB cap), and theaws-opentelemetry-distrodependency check prevents silently shipping an uninstrumented runtime.- Tests prefer real temp directories and only stub the process runner / credential resolver — no excessive mocking at non-I/O boundaries.
terraform.test.tsalso covers the credential-account mismatch, failed-step state preservation, teardown confirmation, and CDK-adoption refusal.
The README is explicit that live deployment, updates and teardown are unverified, and the backend hard-fails if a CDK deployment already exists, so there is no risk of this silently taking over real projects.
Non-blocking observation
No CLI telemetry was added for --managed-by Terraform, the Terraform deploy path, or the preflight rejections (see src/cli/telemetry/README.md). Given this is a bounded PoC that users opt into, that is defensible, but adding a counter for Terraform create/deploy/validate-reject outcomes would make it much easier to tell whether anyone exercises the PoC and which preflight rules bite them. Worth doing in a follow-up if this backend graduates.
|
Claude Security Review: no high-confidence findings. (run) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #2602 +/- ##
============================================
- Coverage 96.57% 96.56% -0.01%
============================================
Files 699 704 +5
Lines 57427 58595 +1168
============================================
+ Hits 55462 56585 +1123
- Misses 1965 2010 +45 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
- Pin the L3 Terraform module commit that aligns IAM, names, log group ownership and gateway discovery variables with the L3 construct. - Preflight accepts Python entrypoints with a handler suffix and treats AGENTCORE_GATEWAY_<NAME>_AUTH_TYPE, not _ID, as a reserved variable. - Scope the example Cedar policy to AgentCore::Gateway resources.
|
Claude Security Review: no high-confidence findings. (run) |
- Pin the L3 module commit that reproduces the CDK L2 gateway trust and waits for IAM propagation before creating AgentCore resources. - Record live results in the example and document the LOG_ONLY default-deny behavior, evaluation result location, and per-account policy name uniqueness.
|
Claude Security Review: no high-confidence findings. (run) |
Description
Add an experimental Terraform deployment backend for the refactor branch.
agentcore create --name MyProject --iac terraformimmediately scaffolds editable HCL root files underagentcore/terraform/, referencing the application module maintained in the L3 constructs repository.--iacaccepts CDK and Terraform case-insensitively; CDK remains the default.The root passes a
specobject to the module, initially usingjsondecode(file(...))to readagentcore.json. Customers can edit supported JSON settings and run Terraform directly, or pass inline HCL with references to their own resources. Named outputs expose runtime roles and resource identifiers for additional permissions and infrastructure. Build packages Python code and preserves root HCL edits. Every deployment target uses this same root; target inputs, plans, provider initialization data, and local state are isolated under.targets/<target>/. Customer edits apply to whichever target is planned next, without copying configuration. The module source is pinned to an immutable Git commit; upgrading it is an explicit root HCL change. During this draft PoC the pin points to the contributor fork for the companion L3 PR.The connected example covers memory, a Python CodeZip runtime, policy engine/policy, an IAM MCP gateway with Python Lambda targets, and builtin online evaluation. AWSCC 1.106.0 manages the AgentCore primitives except GatewayTarget, which that release does not expose. AWS 6.69.0 manages gateway targets and supporting IAM, S3 and Lambda. Like the L3 construct, the module leaves runtime, Lambda and evaluation log groups to the services. Terraform references establish resource dependencies; resources use logical-name keys to preserve addresses when lists are reordered.
The CLI supports separate local state per target for one operator. Target binding rejects account/region reassignment, legacy state layouts, missing current state with a populated backup, remote backends and custom state paths. A shared-root lock serializes CLI operations that update artifacts and the provider lockfile. The CLI selects the target backend and variables file explicitly and applies the saved plan. Backend checks also detect custom state configured by standalone Terraform, and legacy-layout checks preserve existing target roots while allowing customer modules. Removing all project resources requests teardown confirmation and preserves resources customers declared outside the application module.
This remains a draft PoC with bounded feature coverage. CDK migration, remote state, other runtime packaging/authentication/networking modes, other gateway target types and custom evaluators are outside scope. Gateway JSON Schemas are validated to an explicit eight-level limit. The typed object contract rejects known unsupported settings, but Terraform object conversion is not general typo detection for arbitrary attributes.
Live testing (manual, one test account, Terraform 1.16.6, via the packaged CLI):
statuslists every resource; repeat plan: no changespingand a model-backed Strands prompt returned 200/users/{actorId}/factstools/listreturned the tool;tools/callreturned the Lambda result through IAM SigV4permitallows the call;forbidfilterstools/listand denies the call naming the policy; a loneLOG_ONLYpolicy leaves default-deny in force; restoringpermitrestores accessBuiltin.Helpfulnessscore (3 of 3, 0.83); probe sessions without model spans are logged as evaluation errorsremove all+deploytore down the fresh project; no runtime, memory, gateway, policy engine, evaluation, Lambda, role or bucket remainedDefects found live and fixed: an over-broad shared trust condition, an unscoped sample Cedar policy, module-owned log groups and evaluation output configuration that the L3 does not create, an empty-tags perpetual diff, and first-apply failures from IAM propagation (memory trust, gateway policy, evaluation policy), now handled by per-resource
time_sleepwaits. The gateway trust policy now reproduces the CDK L2 role exactly, including its unconditioned statement. Policy names are unique per account and Region, as with CDK.These are manual live runs, not automated end-to-end tests; the repository e2e suite was not run. CloudWatch Transaction Search follows the existing CLI account-level setup and is outside Terraform state.
Related Issue
Related to #1919.
Companion module PR: aws/agentcore-l3-cdk-constructs#424.
Pinned module commit:
39b824c1cdbc4926bea745616610e516344537f4in the contributor fork. Replace the PoC source with an upstream release when available.Documentation PR
Included here: the connected example and standalone Terraform root in
examples/terraform-poc/, and regeneratedcommand.md. Module implementation, tests, and API documentation belong to the companion L3 PR.Type of Change
Testing
Full CLI unit suite: 4,658 passed, zero failures. Regression coverage includes shared root edits across targets, isolated state/inputs/plans, source locking, legacy layout preservation, standalone backend checks, and preflight handler-suffix and reserved-variable rules.
Companion L3 module: 36 mock-provider tests passed, including runtime/memory, gateway/policy and online-evaluation parity with the L3 construct.
Live: see the table above.
Typecheck, lint, formatting, bundle build and secret scanning passed.
I ran
bun testI ran the relevant end-to-end tests with
bun run test:e2e, or explained why they are not applicableI ran
bun run typecheckI ran
bun run lint:checkI ran
bun run format:checkI ran
bun run buildIf I modified
src/assets/, I updated affected snapshots withbun test <test-file> --update-snapshotsand committed themScaffolding is covered by command and backend tests; the companion L3 PR owns the Terraform mock-provider tests. Existing snapshots remain unchanged. Live behavior was verified manually as described above; automated e2e coverage for the Terraform backend is outstanding.
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.