fix(codegen): emit z.record for freeform-JSON properties in input schemas (#457) #3
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/issue-457-json-type-input-schema"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Fixes swamp-club issue #457:
@swamp/aws/iam/role-policyPolicyDocument schema saysz.string()but AWS CloudControl expects JSONObject.codegen/shared/zodGenerator.tshad three fallback paths that emittedz.string()for freeform-JSON CloudFormation properties. CloudControl expects JSONObject on writes, so string input passed swamp validation but failed at the AWS API.z.record(z.string(), z.unknown()), making input schemas accept YAML objects — which is what CloudControl expects.8026920b6fixed the resource/state schema side but explicitly left inputs asz.string(). That decision was incorrect — CloudControl rejects strings on write.What changed
codegen/shared/zodGenerator.ts(3 one-line changes):generateObjectZodbare-object fallback (line 540) — PRIMARY fix for PolicyDocumentgenerateZodForPropertyjson case (line 196) — secondary fix fortype: ["string", "object"]propertiesgenerateSimplifiedZodForPropertybare-object fallback (line 390) — resource schema consistencycodegen/shared/zodGenerator_test.ts:z.recordfor json-type input schemastype: "object"with no propertiesRegenerated models (390 files):
Why this is correct
type: "object"(confirmed by fetchingcf-schema.jsonand inspectingAWS::IAM::RolePolicy)Stringfor these fields — onlyJSONObjectis acceptedexpected record, received string) before reaching AWSVerified
deno check main.tsanddeno testpass (15/15 tests)generate:aws iam) shows correct diffz.string()→z.record()Breaking change
Operators with string-valued freeform-JSON properties (e.g.,
PolicyDocument: '{"Version":...}') must switch to YAML objects. This is the correct behavior — string input never worked at CloudControl.Test plan
Adversarial Review
Critical / High
None found.
Medium
codegen/shared/zodGenerator.ts:196 -- json type now rejects string inputs across all providers
The case json in generateZodForProperty unconditionally emits z.record(z.string(), z.unknown()). This assumes every provider json-typed property expects a parsed object at the input boundary. If any provider normalizer produces type json for fields where users legitimately supply a JSON string (e.g. a stringified policy document), the generated input schema will reject it at runtime with a Zod validation error.
The resource schema already handles this ambiguity with z.union([z.string(), z.record(...)]) (line 324), but the input schema does not get the same treatment.
Breaking example: A user calls an extension with PolicyDocument as a JSON string instead of a parsed object. The old schema accepted it; the new schema rejects it.
Mitigation: If CloudControl definitively expects objects (not strings) for all json fields, this is correct and intentional -- just worth confirming that no existing callers rely on passing JSON strings. If there is any ambiguity, the input schema could use the same union pattern as the resource schema.
Low
codegen/shared/zodGenerator.ts:196 -- z.record(z.string(), z.unknown()) rejects JSON arrays
z.record only accepts plain objects, not arrays. If a json-typed CloudFormation property could legitimately be a JSON array at the top level, it would be rejected. For known CF schema fields like PolicyDocument, Configuration, etc., these are always objects, so this is unlikely in practice.
codegen/shared/zodGenerator_test.ts:425-428 -- test assertion is weaker than intended
The test asserts result.inputSchemaBody.includes("PolicyDocument: z.string()") is false to confirm no bare z.string(). However, this would not catch a regression where the output includes z.string().describe(...). The positive match on the expected expression (line 421-424) is the load-bearing assertion; the negative one is redundant.
Verdict
PASS -- This is a well-scoped bug fix. The three changed sites consistently replace an incorrect z.string() fallback with z.record(z.string(), z.unknown()) for freeform-JSON and bare-object properties. The logic is correct, the tests cover the changed paths, and the snapshot tests are unaffected. The medium finding is a semantic question about provider contract assumptions, not a code defect.
Code Review
Blocking Issues
None.
Suggestions
Dead branch in generateObjectBody indent logic (zodGenerator.ts:570): The indent ternary
depth > 0 ? 1 : 2has an unreachable 4-space branch. generateObjectBody is always invoked with depth + 1 from generateObjectZod, so depth is always >= 1 at that call site and the ternary always resolves to 1 (2 spaces). The 4-space case is dead code. Replace with a literal 2-space string for clarity.Double cast in test 14 (zodGenerator_test.ts:407): The cast
as unknown as CfPropertyis required because the "json" type is an internal normalized type not present in the CfProperty type union. A short inline comment noting that "json" comes from normalizeProperty rather than the CloudFormation spec would help future readers understand the cast.Indentation asymmetry between input and resource schemas: The input schema path in generateObjectBody uses 2-space indentation for nested object children, while the resource schema path in generateSimplifiedZodForProperty hardcodes 4 spaces (visible in the test-8 snapshot). Not a correctness issue, but the asymmetry means generated model files have mixed indentation style.