currently only creates a file in memory but can scaffold to a real t…#12
Conversation
- Set up preview branches
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds exported IR and Terraform model types and a Target interface, implements TerraformTarget to compile an IR into a TFFile, adds a writer that emits HCL to disk, unit/integration tests for compile and write, and invokes compilation in main to produce output.tf. Changes
Sequence Diagram(s)sequenceDiagram
participant Main as Application
participant TFTarget as TerraformTarget
participant Writer as WriteTerraformFile
participant FS as FileSystem
Main->>TFTarget: Compile(ir IR)
Note over TFTarget: Iterate ir.Nodes\nCreate TFBlock entries (Class, Labels, Attributes)
TFTarget-->>Main: TFFile
Main->>Writer: WriteTerraformFile(tf, path)
Note over Writer: Recursively render TFBlock -> HCL string
Writer->>FS: os.WriteFile(path, data)
FS-->>Writer: Success / Error
Writer-->>Main: return error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🚀 Preview deployed to: https://napkin-pr-12.fly.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
napkin-backend/compiler/target.go (1)
3-7:Attributes map[string]stringlimits expressible Terraform values.Real HCL resources use booleans, numbers, lists, and nested blocks — all coerced to strings here. Consider a
map[string]any(or a typed value union) when the IR needs to represent non-string attribute values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@napkin-backend/compiler/target.go` around lines 3 - 7, TFResource currently uses Attributes map[string]string which loses booleans, numbers, lists and nested blocks; change Attributes to map[string]any (or a small typed union/value struct) in the TFResource type and update any consumers/serializers (e.g. code that marshals to HCL/JSON or reads Attributes in functions referencing TFResource) to handle non-string types (booleans, numbers, []any, map[string]any) accordingly so attribute values are preserved and correctly emitted.napkin-backend/main.go (1)
44-44: Inconsistent logging — uselog.Printlninstead offmt.Println.All other output in
main()goes throughlog(lines 55, 62, 64). Usingfmt.Printlnhere omits the timestamp and skips log routing.♻️ Proposed fix
- fmt.Println("Terraform file written to output.tf") + log.Println("Terraform file written to output.tf")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@napkin-backend/main.go` at line 44, Replace the fmt.Println call in main() that prints "Terraform file written to output.tf" with log.Println so it uses the same logging pipeline and timestamps as the other log calls; locate the fmt.Println invocation in the main function and change it to use log.Println to maintain consistent logging behavior across main().napkin-backend/compiler/tf_target.go (1)
6-17: No validation ofnode.Typeornode.ID— empty values produce malformed HCL.An empty
TypeorIDsilently emitsresource "" "" { }, which is invalid Terraform. Returning an error here is cheap and prevents confusing downstream failures.♻️ Proposed fix
for _, node := range ir.Nodes { + if node.Type == "" || node.ID == "" { + return nil, fmt.Errorf("compiler: node %q has empty Type or ID", node.ID) + } resource := TFResource{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@napkin-backend/compiler/tf_target.go` around lines 6 - 17, The Compile method on TerraformTarget should validate node.Type and node.ID before emitting TFResource to avoid producing invalid HCL; in TerraformTarget.Compile (iterating ir.Nodes) check that node.Type and node.ID are non-empty and, if either is empty, return a descriptive error (including which field and the node reference) instead of appending a resource, so malformed resource "" "" is never produced; ensure the error is returned up to callers of Compile.napkin-backend/compiler/tf_writer.go (1)
14-16: Non-deterministic attribute ordering — idempotent generation is broken.Go map iteration order is randomised per run, so two calls with identical input can produce different files. This makes VCS diffs noisy and breaks any downstream tooling that relies on stable output (e.g.,
terraform fmtdiffing, plan caching).♻️ Proposed fix — sort keys before writing
+import ( + "os" + "sort" + "strings" +) + keys := make([]string, 0, len(v.Attributes)) + for k := range v.Attributes { + keys = append(keys, k) + } + sort.Strings(keys) + for _, key := range keys { + value := v.Attributes[key] builder.WriteString(" " + key + ` = "` + value + `"` + "\n") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@napkin-backend/compiler/tf_writer.go` around lines 14 - 16, The loop over v.Attributes is non-deterministic because Go map iteration is unordered; change the code in tf_writer.go (the block that iterates v.Attributes) to collect the map keys into a slice, sort that slice (e.g., with sort.Strings), and then iterate the sorted keys to write builder.WriteString lines so attribute output is stable and idempotent; update imports if needed to include "sort".napkin-backend/compiler/tf_target_test.go (1)
40-42: Test only validatesami; theinstance_typeattribute in the fixture is unverified.A regression that silently drops attributes beyond the first would pass undetected.
♻️ Proposed fix
if resource.Attributes["ami"] != "ami-123" { t.Errorf("expected ami attribute to be ami-123") } + + if resource.Attributes["instance_type"] != "t2.micro" { + t.Errorf("expected instance_type attribute to be t2.micro, got %s", resource.Attributes["instance_type"]) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@napkin-backend/compiler/tf_target_test.go` around lines 40 - 42, The test currently only asserts resource.Attributes["ami"] and misses verifying the fixture's instance_type, allowing regressions that drop attributes to pass; update the test in tf_target_test.go (the test that accesses resource.Attributes) to also assert that resource.Attributes["instance_type"] equals the expected fixture value and include a clear t.Errorf message on failure (and optionally assert the Attributes map contains both keys) so the test fails if instance_type is missing or incorrect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@napkin-backend/compiler/tf_target.go`:
- Around line 9-15: The TFResource being built in the loop copies
node.Attributes by reference, so changes to TFResource.Attributes will mutate
the original IR node; update the assignment in the loop that constructs
TFResource (where TFResource{..., Attributes: node.Attributes} is set) to
deep-copy node.Attributes into a new map before assigning (e.g., clone/map-copy
all key→value entries, recursively if values are maps/slices), or add a helper
like cloneAttributes and use it; ensure the copy logic is used for each node in
the ir.Nodes iteration so TFResource.Attributes is independent of the original
GraphNode.
In `@napkin-backend/compiler/tf_writer.go`:
- Around line 8-19: WriteTerraformFile currently only serialises tf.Resources
and drops TFFile.Modules and TFFile.Outputs; update WriteTerraformFile to also
iterate over tf.Modules and tf.Outputs and append them to the strings.Builder
(preferably after resources) using the same style as resources: open the block
with module "<name>" { and output "<name>" { respectively, emit each block's
attributes/arguments, close the block, and separate with newlines. Reference the
WriteTerraformFile function and the TFFile.Modules and TFFile.Outputs fields
when making the change so modules and outputs are no longer silently omitted.
- Around line 14-16: The loop writing attribute values writes them verbatim
(builder.WriteString in the v.Attributes iteration) which breaks HCL when values
contain " or \; change it to use strconv.Quote on the attribute value (e.g.,
replace the literal `"` + value + `"` construction with strconv.Quote(value)) so
the string is properly escaped, and add the strconv import if missing.
In `@napkin-backend/compiler/tr_writer_test.go`:
- Around line 22-26: The temp file created with os.CreateTemp (variable tmp) is
never closed; add a deferred close to avoid leaking the file descriptor and to
allow removal on Windows—e.g. immediately after the nil err check, add a defer
that closes tmp and then removes the file (use defer func(){ _ = tmp.Close(); _
= os.Remove(tmp.Name()) }()) so Close runs before Remove; reference tmp,
os.CreateTemp and tmp.Name() when making the change.
In `@napkin-backend/main.go`:
- Around line 39-44: The code currently calls
compiler.WriteTerraformFile(tfFile, "output.tf") which writes to a CWD-relative
path; change this to accept a configurable output path (via a command-line flag
like flag.String("out", "output.tf", ...) or an environment variable fallback)
and pass that variable to compiler.WriteTerraformFile instead of the hardcoded
"output.tf"; also update the fmt.Println to print the resolved output path.
Ensure the flag/env logic runs in main before calling
compiler.WriteTerraformFile and reference the existing
compiler.WriteTerraformFile and the printed message for the replacement.
---
Nitpick comments:
In `@napkin-backend/compiler/target.go`:
- Around line 3-7: TFResource currently uses Attributes map[string]string which
loses booleans, numbers, lists and nested blocks; change Attributes to
map[string]any (or a small typed union/value struct) in the TFResource type and
update any consumers/serializers (e.g. code that marshals to HCL/JSON or reads
Attributes in functions referencing TFResource) to handle non-string types
(booleans, numbers, []any, map[string]any) accordingly so attribute values are
preserved and correctly emitted.
In `@napkin-backend/compiler/tf_target_test.go`:
- Around line 40-42: The test currently only asserts resource.Attributes["ami"]
and misses verifying the fixture's instance_type, allowing regressions that drop
attributes to pass; update the test in tf_target_test.go (the test that accesses
resource.Attributes) to also assert that resource.Attributes["instance_type"]
equals the expected fixture value and include a clear t.Errorf message on
failure (and optionally assert the Attributes map contains both keys) so the
test fails if instance_type is missing or incorrect.
In `@napkin-backend/compiler/tf_target.go`:
- Around line 6-17: The Compile method on TerraformTarget should validate
node.Type and node.ID before emitting TFResource to avoid producing invalid HCL;
in TerraformTarget.Compile (iterating ir.Nodes) check that node.Type and node.ID
are non-empty and, if either is empty, return a descriptive error (including
which field and the node reference) instead of appending a resource, so
malformed resource "" "" is never produced; ensure the error is returned up to
callers of Compile.
In `@napkin-backend/compiler/tf_writer.go`:
- Around line 14-16: The loop over v.Attributes is non-deterministic because Go
map iteration is unordered; change the code in tf_writer.go (the block that
iterates v.Attributes) to collect the map keys into a slice, sort that slice
(e.g., with sort.Strings), and then iterate the sorted keys to write
builder.WriteString lines so attribute output is stable and idempotent; update
imports if needed to include "sort".
In `@napkin-backend/main.go`:
- Line 44: Replace the fmt.Println call in main() that prints "Terraform file
written to output.tf" with log.Println so it uses the same logging pipeline and
timestamps as the other log calls; locate the fmt.Println invocation in the main
function and change it to use log.Println to maintain consistent logging
behavior across main().
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
napkin-backend/compiler/target.gonapkin-backend/compiler/tf_target.gonapkin-backend/compiler/tf_target_test.gonapkin-backend/compiler/tf_writer.gonapkin-backend/compiler/tr_writer_test.gonapkin-backend/main.gonapkin-backend/napkin-backend
| func WriteTerraformFile(tf *TFFile, path string) error { | ||
| var builder strings.Builder | ||
|
|
||
| for _, v := range tf.Resources { | ||
| builder.WriteString(`resource "` + v.Type + `" "` + v.Name + `" {` + "\n") | ||
|
|
||
| for key, value := range v.Attributes { | ||
| builder.WriteString(" " + key + ` = "` + value + `"` + "\n") | ||
| } | ||
|
|
||
| builder.WriteString("}\n\n") | ||
| } |
There was a problem hiding this comment.
TFFile.Modules and TFFile.Outputs are silently dropped.
The writer only serialises tf.Resources; Modules and Outputs — both first-class fields of TFFile — are never emitted. Any caller that populates those fields will get a silently incomplete Terraform file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-backend/compiler/tf_writer.go` around lines 8 - 19, WriteTerraformFile
currently only serialises tf.Resources and drops TFFile.Modules and
TFFile.Outputs; update WriteTerraformFile to also iterate over tf.Modules and
tf.Outputs and append them to the strings.Builder (preferably after resources)
using the same style as resources: open the block with module "<name>" { and
output "<name>" { respectively, emit each block's attributes/arguments, close
the block, and separate with newlines. Reference the WriteTerraformFile function
and the TFFile.Modules and TFFile.Outputs fields when making the change so
modules and outputs are no longer silently omitted.
| err = compiler.WriteTerraformFile(tfFile, "output.tf") | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| fmt.Println("Terraform file written to output.tf") |
There was a problem hiding this comment.
"output.tf" is a relative path resolved against the process CWD.
This silently writes to different locations depending on how the binary is invoked (e.g., go run vs ./napkin-backend). Consider making the output path a command-line flag or env var.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-backend/main.go` around lines 39 - 44, The code currently calls
compiler.WriteTerraformFile(tfFile, "output.tf") which writes to a CWD-relative
path; change this to accept a configurable output path (via a command-line flag
like flag.String("out", "output.tf", ...) or an environment variable fallback)
and pass that variable to compiler.WriteTerraformFile instead of the hardcoded
"output.tf"; also update the fmt.Println to print the resolved output path.
Ensure the flag/env logic runs in main before calling
compiler.WriteTerraformFile and reference the existing
compiler.WriteTerraformFile and the printed message for the replacement.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@napkin-backend/compiler/tf_target.go`:
- Around line 10-16: The attributes map allocated in tf_target.go is never
populated, so copy all entries from the source node.Attributes into the newly
created attributes map before constructing TFResource; specifically iterate over
node.Attributes and assign each key/value into the local attributes map (the one
created with make) so TFResource.Attributes contains the original data when
returned by Compile (or the function that builds TFResource), preserving the
source GraphNode.Attributes rather than dropping them.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
napkin-backend/compiler/tf_target.gonapkin-backend/compiler/tf_writer.gonapkin-backend/compiler/tr_writer_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- napkin-backend/compiler/tf_writer.go
- napkin-backend/compiler/tr_writer_test.go
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
napkin-backend/main.go (1)
48-53:⚠️ Potential issue | 🟡 MinorMake Terraform output path configurable instead of CWD-relative.
Line 48 writes to
"output.tf"relative to process CWD, which is brittle acrossgo run, container entrypoints, and service managers.Suggested change
+import "flag" @@ func main() { + outPath := flag.String("out", "output.tf", "path to generated Terraform file") + flag.Parse() @@ - err = compiler.WriteTerraformFile(tfFile, "output.tf") + err = compiler.WriteTerraformFile(tfFile, *outPath) if err != nil { panic(err) } - fmt.Println("Terraform file written to output.tf") + fmt.Printf("Terraform file written to %s\n", *outPath)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@napkin-backend/main.go` around lines 48 - 53, The code currently writes Terraform to a hardcoded CWD-relative filename via compiler.WriteTerraformFile(tfFile, "output.tf"); make the output path configurable (e.g., add a CLI flag or env var) and pass that value into WriteTerraformFile instead of the literal "output.tf". Update main.go to read a new config (flag like tfOutputPath or env var such as NAPKIN_TF_OUTPUT), validate/normalize it (use filepath.Abs or filepath.Join if you need to resolve relative paths), and call compiler.WriteTerraformFile(tfFile, tfOutputPath); ensure any default remains "output.tf" for backward compatibility.
🧹 Nitpick comments (1)
napkin-backend/compiler/tf_target.go (1)
32-32: Remove compile-loop debug printing from stdout.Line 32 introduces noisy runtime output on every compile; prefer structured logging behind a debug flag (or remove entirely).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@napkin-backend/compiler/tf_target.go` at line 32, Remove the noisy fmt.Printf call that prints node.ID/node.Type/node.Class to stdout in tf_target.go: delete the fmt.Printf("ID=%s Type=%s Class=%s\n", node.ID, node.Type, node.Class) line and, if you need the info for debugging, replace it with a structured debug log behind the existing debug mechanism (e.g., use the project's logger.Debugf or guard the output with a debug flag) so normal compiles produce no stdout noise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@napkin-backend/compiler/target.go`:
- Around line 27-43: The IR currently uses map[string]string for attributes (in
TFBlock, TFFile, GraphNode.Attributes and TFResource) which cannot represent
Terraform typed expressions (strings, numbers, booleans, lists, maps/objects)
nor distinguish arguments from nested block constructs; change the model to
replace map[string]string with a typed value system (e.g., introduce a
TFValue/Value type and ValueKind enum for
String/Number/Bool/List/Map/Object/Expression) and make TFBlock explicitly
separate arguments (Attributes map[string]TFValue) from nested blocks (Blocks
[]TFBlock with block type/name/labels preserved) so meta-arguments like
depends_on can be lists and complex nested structures (e.g., metadata_options)
can be objects; also update GraphNode.Attributes to map[string]TFValue and
TFResource to use TFValue for its attributes so code that serializes to HCL can
emit proper typed values vs nested blocks.
In `@napkin-backend/compiler/tf_target.go`:
- Around line 39-51: Before assigning block.Labels, validate that required node
fields are present: for ClassResource and ClassData ensure node.Type and node.ID
are non-empty, for ClassProvider ensure node.Type is non-empty, and for
ClassModule ensure node.ID is non-empty. If a required field is missing, return
a clear error (or propagate an error) instead of creating the block; update the
code around the switch that sets block.Labels (the references to node.Class,
ClassResource, ClassData, ClassProvider, ClassModule, block.Labels, node.Type,
node.ID) to perform these checks and error handling prior to assigning labels.
In `@napkin-backend/compiler/tf_writer.go`:
- Around line 29-33: WriteTerraformFile currently dereferences tf without
checking for nil which will panic; add an early guard at the top of
WriteTerraformFile to return a descriptive error (e.g., using fmt.Errorf or
errors.New) when tf == nil, before creating the strings.Builder or calling
writeBlock, so callers can handle the error instead of crashing; reference
TFFile and writeBlock to locate the function to modify.
In `@napkin-backend/main.go`:
- Around line 29-37: The scaffolded aws_instance resource (ID "web", Class
compiler.ClassResource, Type "aws_instance") must include IMDSv2 and encrypted
root volume settings; update the resource creation in main.go where the
Attributes map for that aws_instance is built to add a metadata_options block
with http_endpoint = "enabled" and http_tokens = "required", and add a
root_block_device block with encrypted = true so the generated Terraform
includes IMDSv2 enforcement and an encrypted root disk.
---
Duplicate comments:
In `@napkin-backend/main.go`:
- Around line 48-53: The code currently writes Terraform to a hardcoded
CWD-relative filename via compiler.WriteTerraformFile(tfFile, "output.tf"); make
the output path configurable (e.g., add a CLI flag or env var) and pass that
value into WriteTerraformFile instead of the literal "output.tf". Update main.go
to read a new config (flag like tfOutputPath or env var such as
NAPKIN_TF_OUTPUT), validate/normalize it (use filepath.Abs or filepath.Join if
you need to resolve relative paths), and call
compiler.WriteTerraformFile(tfFile, tfOutputPath); ensure any default remains
"output.tf" for backward compatibility.
---
Nitpick comments:
In `@napkin-backend/compiler/tf_target.go`:
- Line 32: Remove the noisy fmt.Printf call that prints
node.ID/node.Type/node.Class to stdout in tf_target.go: delete the
fmt.Printf("ID=%s Type=%s Class=%s\n", node.ID, node.Type, node.Class) line and,
if you need the info for debugging, replace it with a structured debug log
behind the existing debug mechanism (e.g., use the project's logger.Debugf or
guard the output with a debug flag) so normal compiles produce no stdout noise.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
napkin-backend/compiler/target.gonapkin-backend/compiler/tf_run _test.gonapkin-backend/compiler/tf_target.gonapkin-backend/compiler/tf_target_test.gonapkin-backend/compiler/tf_writer.gonapkin-backend/compiler/tr_writer_test.gonapkin-backend/main.gonapkin-backend/napkin-backendnapkin-backend/output.tf
✅ Files skipped from review due to trivial changes (1)
- napkin-backend/compiler/tf_run _test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- napkin-backend/compiler/tf_target_test.go
- napkin-backend/compiler/tr_writer_test.go
| type TFBlock struct { | ||
| Class string | ||
| Labels []string | ||
| Attributes map[string]string | ||
| Blocks []TFBlock | ||
| } | ||
| type TFFile struct { | ||
| Block []TFBlock | ||
| } | ||
|
|
||
| type GraphNode struct { | ||
| ID string | ||
| Class NodeClass | ||
| Type string | ||
| Attributes map[string]string | ||
| Edges []string // list of node IDs this node depends on | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Terraform HCL argument value types nested blocks aws_instance
💡 Result:
In Terraform HCL, there are two different schema-driven “shapes” inside a block:
-
Arguments (aka attributes) use
name = <expression>and the value is an expression that evaluates to a Terraform type (string, number, bool, list, map, object, etc.). What types are allowed is defined by the block’s schema (e.g., the provider’s resource schema). [1] -
Nested blocks use
block_type { ... }and are not set with=. They represent structured sub-objects/collections defined by the schema, and can usually be repeated (list/set of objects) depending on how the provider defined it. [1] Provider schemas commonly model repeatable nested blocks as list/set “collections of structures”. [3]
How this applies to aws_instance
For resource "aws_instance" ..., things like root_block_device, ebs_block_device, and (historically) network_interface are nested blocks, not arguments—so this is invalid:
# INVALID (because it's a block, not an argument)
root_block_device = var.root_block_device…and Terraform will error with “An argument named ... is not expected here. Did you mean to define a block of type ...?” (common example shown for root_block_device / ebs_block_device). [4]
Instead, you must write them as blocks, optionally using a dynamic block if you want to drive them from variables:
variable "root_block_device" {
type = object({
volume_size = number
volume_type = string
})
default = null
}
resource "aws_instance" "this" {
ami = var.ami
instance_type = var.instance_type
dynamic "root_block_device" {
for_each = var.root_block_device == null ? [] : [var.root_block_device]
content {
volume_size = root_block_device.value.volume_size
volume_type = root_block_device.value.volume_type
}
}
}“Attributes as blocks” edge case
Terraform documents a rare compatibility behavior where some resources support both a nested block and an argument with the same name (and []=[] can mean “purge all”). This is uncommon and mainly relevant for JSON syntax or module abstraction quirks. [2]
Note on network_interface for aws_instance
Recent AWS provider releases have deprecated the network_interface block on aws_instance (guidance: use primary_network_interface and aws_network_interface_attachment instead). So whether you should use that nested block depends on your provider version. [5]
🏁 Script executed:
# First, let's examine the target.go file to understand the current implementation
cd napkin-backend && find . -name "target.go" -type fRepository: acmutsa/napkin
Length of output: 78
🏁 Script executed:
# Read the target.go file to see the current structure
cat -n napkin-backend/compiler/target.goRepository: acmutsa/napkin
Length of output: 1263
🏁 Script executed:
# Search for usages of TFBlock and TFFile to understand how they're being used
rg "TFBlock|TFFile" napkin-backend --type go -A 2 -B 2Repository: acmutsa/napkin
Length of output: 2880
IR attribute model cannot represent Terraform's type system and nested block semantics.
map[string]string attributes (lines 30, 41, and also in TFResource at line 15) cannot represent booleans, numbers, lists, maps, objects, or nested block constructs. In Terraform, arguments are expressions that evaluate to typed values (string, number, bool, list, map, object), and nested blocks (like root_block_device, ebs_block_device on aws_instance) are separate structural constructs using block_type { ... } syntax, not name = value assignments. The current model conflates these semantically distinct concepts. Additionally, meta-arguments like depends_on require list types, and complex nested structures like metadata_options require object/map representation.
The IR model will need to support typed values and distinguish between arguments and nested blocks to generate valid Terraform configuration for real resources.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-backend/compiler/target.go` around lines 27 - 43, The IR currently
uses map[string]string for attributes (in TFBlock, TFFile, GraphNode.Attributes
and TFResource) which cannot represent Terraform typed expressions (strings,
numbers, booleans, lists, maps/objects) nor distinguish arguments from nested
block constructs; change the model to replace map[string]string with a typed
value system (e.g., introduce a TFValue/Value type and ValueKind enum for
String/Number/Bool/List/Map/Object/Expression) and make TFBlock explicitly
separate arguments (Attributes map[string]TFValue) from nested blocks (Blocks
[]TFBlock with block type/name/labels preserved) so meta-arguments like
depends_on can be lists and complex nested structures (e.g., metadata_options)
can be objects; also update GraphNode.Attributes to map[string]TFValue and
TFResource to use TFValue for its attributes so code that serializes to HCL can
emit proper typed values vs nested blocks.
| switch node.Class { | ||
| case ClassResource: | ||
| block.Labels = []string{node.Type, node.ID} | ||
|
|
||
| case ClassData: | ||
| block.Labels = []string{node.Type, node.ID} | ||
|
|
||
| case ClassProvider: | ||
| block.Labels = []string{node.Type} | ||
|
|
||
| case ClassModule: | ||
| block.Labels = []string{node.ID} | ||
|
|
There was a problem hiding this comment.
Validate required node fields before assigning labels.
Line 39-Line 51 builds labels without checking required fields. Empty Type/ID will generate invalid Terraform blocks and defer failure to later stages.
Suggested change
switch node.Class {
case ClassResource:
+ if node.Type == "" || node.ID == "" {
+ return nil, fmt.Errorf("resource nodes require non-empty type and id (id=%q, type=%q)", node.ID, node.Type)
+ }
block.Labels = []string{node.Type, node.ID}
case ClassData:
+ if node.Type == "" || node.ID == "" {
+ return nil, fmt.Errorf("data nodes require non-empty type and id (id=%q, type=%q)", node.ID, node.Type)
+ }
block.Labels = []string{node.Type, node.ID}
case ClassProvider:
+ if node.Type == "" {
+ return nil, fmt.Errorf("provider nodes require non-empty type (id=%q)", node.ID)
+ }
block.Labels = []string{node.Type}
case ClassModule:
+ if node.ID == "" {
+ return nil, fmt.Errorf("module nodes require non-empty id")
+ }
block.Labels = []string{node.ID}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-backend/compiler/tf_target.go` around lines 39 - 51, Before assigning
block.Labels, validate that required node fields are present: for ClassResource
and ClassData ensure node.Type and node.ID are non-empty, for ClassProvider
ensure node.Type is non-empty, and for ClassModule ensure node.ID is non-empty.
If a required field is missing, return a clear error (or propagate an error)
instead of creating the block; update the code around the switch that sets
block.Labels (the references to node.Class, ClassResource, ClassData,
ClassProvider, ClassModule, block.Labels, node.Type, node.ID) to perform these
checks and error handling prior to assigning labels.
| func WriteTerraformFile(tf *TFFile, path string) error { | ||
| var builder strings.Builder | ||
|
|
||
| for _, block := range tf.Block { | ||
| writeBlock(&builder, block, "") |
There was a problem hiding this comment.
Guard WriteTerraformFile against nil input.
Line 32 will panic when tf == nil. Return an explicit error instead so callers can handle it safely.
Suggested change
import (
+ "errors"
"os"
"strconv"
"strings"
)
@@
func WriteTerraformFile(tf *TFFile, path string) error {
+ if tf == nil {
+ return errors.New("nil TFFile")
+ }
var builder strings.Builder🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-backend/compiler/tf_writer.go` around lines 29 - 33,
WriteTerraformFile currently dereferences tf without checking for nil which will
panic; add an early guard at the top of WriteTerraformFile to return a
descriptive error (e.g., using fmt.Errorf or errors.New) when tf == nil, before
creating the strings.Builder or calling writeBlock, so callers can handle the
error instead of crashing; reference TFFile and writeBlock to locate the
function to modify.
| { | ||
| ID: "web", | ||
| Class: compiler.ClassResource, | ||
| Type: "aws_instance", | ||
| Attributes: map[string]string{ | ||
| "ami": "ami-123", | ||
| "instance_type": "t2.micro", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Terraform aws_instance metadata_options http_tokens IMDSv2 requirement
💡 Result:
To make IMDSv2 required for a Terraform aws_instance, set the instance metadata option HttpTokens = required via metadata_options.http_tokens = "required".
resource "aws_instance" "example" {
# ...
metadata_options {
http_endpoint = "enabled" # keep IMDS reachable
http_tokens = "required" # IMDSv2-only (IMDSv1 disabled)
}
}Key points:
- In EC2,
HttpTokenscontrols whether IMDSv2 is optional (IMDSv1 still works) or required (IMDSv1 disabled). [1] - If you require IMDSv2 (
http_tokens = "required"), the IMDS endpoint must be enabled (http_endpoint = "enabled"), otherwise metadata is unreachable. [2] - Security tooling commonly flags Terraform
aws_instancebecause IMDS tokens are often left optional unless you set this explicitly. [3]
Sources: [1] [2] [3]
🌐 Web query:
Terraform aws_instance root_block_device encrypted default encryption configuration
💡 Result:
What controls “default encryption” for aws_instance root volumes?
AWS account/Region setting (recommended baseline): “EBS encryption by default”
If you enable it, all newly created EBS volumes in that Region are always encrypted, and you can’t opt out per-volume in that Region. It doesn’t affect existing volumes. [1]
In Terraform this is managed with aws_ebs_encryption_by_default (and deleting that Terraform resource disables default encryption). [2]
# Regional (per-provider-region or per "region" arg) setting
resource "aws_ebs_encryption_by_default" "this" {
enabled = true # defaults to true, but set explicitly for clarity
}Optional: set the default KMS key used when no key is specified
By default, AWS uses an AWS-managed key for EBS if you don’t specify a CMK. Terraform can set a customer-managed key via aws_ebs_default_kms_key (note: this does not itself enable encryption-by-default). [3]
resource "aws_ebs_default_kms_key" "this" {
key_arn = aws_kms_key.ebs.arn
}How does aws_instance.root_block_device.encrypted interact with that?
In the aws_instance resource, the root_block_device block has:
encrypted(defaults tofalsein the Terraform schema)kms_key_id(optional)- and Terraform notes these must be configured to perform drift detection, and changing
encrypted/kms_key_idforces instance replacement. [4]
resource "aws_instance" "example" {
ami = var.ami
instance_type = "t3.micro"
root_block_device {
encrypted = true
kms_key_id = aws_kms_key.ebs.arn # optional; omit to use the account/region default key
}
}Practical guidance
- If you want encryption everywhere with minimal per-resource config: enable EBS encryption by default (+ optionally set the default CMK). [1][2][3]
- If you need to enforce encryption (and/or a specific CMK) at the resource level and have Terraform track it: set
root_block_device.encrypted = true(andkms_key_idif needed), but be aware changes require replacement. [4]
Sources: AWS EBS encryption-by-default behavior and constraints [1]; Terraform AWS provider docs for aws_ebs_encryption_by_default [2], aws_ebs_default_kms_key [3], and aws_instance.root_block_device encryption semantics [4].
[1] (docs.aws.amazon.com)
[2] (raw.githubusercontent.com)
[3] (raw.githubusercontent.com)
[4] (raw.githubusercontent.com)
Citations:
- 1: https://docs.aws.amazon.com/ebs/latest/userguide/encryption-by-default.html?utm_source=openai
- 2: https://raw.githubusercontent.com/hashicorp/terraform-provider-aws/main/website/docs/r/ebs_encryption_by_default.html.markdown
- 3: https://raw.githubusercontent.com/hashicorp/terraform-provider-aws/main/website/docs/r/ebs_default_kms_key.html.markdown
- 4: https://raw.githubusercontent.com/hashicorp/terraform-provider-aws/main/website/docs/r/instance.html.markdown
🏁 Script executed:
# Locate and examine the napkin-backend/main.go file
fd "main.go" napkin-backend/Repository: acmutsa/napkin
Length of output: 80
🏁 Script executed:
# Check if the file exists and view the relevant lines around 29-37
if [ -f "napkin-backend/main.go" ]; then
wc -l napkin-backend/main.go
echo "---"
cat -n napkin-backend/main.go | sed -n '20,45p'
fiRepository: acmutsa/napkin
Length of output: 767
Add IMDSv2 enforcement and encrypted root block device to scaffolded aws_instance.
The aws_instance at lines 29–37 omits metadata_options (IMDSv2) and root_block_device (encryption), leaving the generated Terraform vulnerable to security scanning. Add:
metadata_options {
http_endpoint = "enabled"
http_tokens = "required"
}
root_block_device {
encrypted = true
}These are standard Terraform attributes, not account-level settings, and should be included in the IR/writer path for any production-facing EC2 scaffolding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-backend/main.go` around lines 29 - 37, The scaffolded aws_instance
resource (ID "web", Class compiler.ClassResource, Type "aws_instance") must
include IMDSv2 and encrypted root volume settings; update the resource creation
in main.go where the Attributes map for that aws_instance is built to add a
metadata_options block with http_endpoint = "enabled" and http_tokens =
"required", and add a root_block_device block with encrypted = true so the
generated Terraform includes IMDSv2 enforcement and an encrypted root disk.
* refactor backend for better file structure * Setup React Flow (#9) * basic graph structure (#11) * currently only creates a file in memory but can scaffold to a real t… (#12) * currently only creates a file in memory but can scaffold to a real tf file * slight refactor * forgot to commit this * created terraform and provider block * remove unnecessary import --------- Co-authored-by: Joshua Silva <72359611+joshuasilva414@users.noreply.github.com> Co-authored-by: joshuasilva414 <joshuasilva414@gmail.com> * initial analyzer design --------- Co-authored-by: joshuasilva414 <joshuasilva414@gmail.com> Co-authored-by: Eric Lee <155848001+Eric1305@users.noreply.github.com> Co-authored-by: Adan Santos <116550868+adanrsantos@users.noreply.github.com> Co-authored-by: Joshua Silva <72359611+joshuasilva414@users.noreply.github.com>
* Analyzer (#20) * refactor backend for better file structure * Setup React Flow (#9) * basic graph structure (#11) * currently only creates a file in memory but can scaffold to a real t… (#12) * currently only creates a file in memory but can scaffold to a real tf file * slight refactor * forgot to commit this * created terraform and provider block * remove unnecessary import --------- Co-authored-by: Joshua Silva <72359611+joshuasilva414@users.noreply.github.com> Co-authored-by: joshuasilva414 <joshuasilva414@gmail.com> * initial analyzer design --------- Co-authored-by: joshuasilva414 <joshuasilva414@gmail.com> Co-authored-by: Eric Lee <155848001+Eric1305@users.noreply.github.com> Co-authored-by: Adan Santos <116550868+adanrsantos@users.noreply.github.com> Co-authored-by: Joshua Silva <72359611+joshuasilva414@users.noreply.github.com> * Merge branch 'dev' into dev-kaydo * Implemented /compile Endpoint --------- Co-authored-by: Joshua Silva <72359611+joshuasilva414@users.noreply.github.com> Co-authored-by: diego <161160330+deeg454@users.noreply.github.com> Co-authored-by: joshuasilva414 <joshuasilva414@gmail.com> Co-authored-by: Eric Lee <155848001+Eric1305@users.noreply.github.com> Co-authored-by: Adan Santos <116550868+adanrsantos@users.noreply.github.com>
2 participants
What
Why
Files Added/ Modified
napkin-backend/compilercompiler/target.gocompiler/tf_writer.gocompiler/tf_target.gocompiler/tf_target_test.gocompiler/tf_writer_test.goResolves
Summary by CodeRabbit