Skip to content

Conversation

@Tzvonimir
Copy link
Contributor

@Tzvonimir Tzvonimir commented Nov 7, 2025

Summary by CodeRabbit

  • New Features
    • Support for specifying KMS encryption keys (kms_key_id) and EBS snapshot IDs (snapshot_id) in AWS node block device mappings, preserved across configuration import/export.
  • Documentation
    • Updated node policy docs to describe the new kms_key_id and snapshot_id fields for AWS EBS volume configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

Walkthrough

Adds kms_key_id and snapshot_id string attributes to AWS EBS block device mappings in the node policy provider; updates schema, runtime object types, and toProto/fromProto conversion logic so those fields are preserved when converting between Terraform models and protobufs. (50 words)

Changes

Cohort / File(s) Summary
EBS Block Device Mapping (provider logic)
internal/provider/node_policy.go
Added kms_key_id and snapshot_id string attributes to the NodePolicyResource schema under aws.block_device_mappings.ebs. Extended EBS object type definitions and updated toProto / fromProto conversion paths to include these fields for round-trip preservation.
Documentation
docs/resources/node_policy.md
Documented the new kms_key_id and snapshot_id fields under aws.block_device_mappings.ebs with brief descriptions (encryption key and snapshot-based volume creation).

Sequence Diagram(s)

sequenceDiagram
  participant TF as Terraform model
  participant Prov as NodePolicy provider logic
  participant PB as Protobuf (AWSNodeClass)

  rect rgba(0,128,128,0.06)
  TF->>Prov: create/update NodePolicy (includes ebs{kms_key_id, snapshot_id})
  Prov->>PB: toProto (map EBS attrs -> pb.ebs.kms_key_id, pb.ebs.snapshot_id)
  end

  rect rgba(128,64,0,0.06)
  PB->>Prov: fromProto (pb.ebs.kms_key_id, pb.ebs.snapshot_id)
  Prov->>TF: populate Terraform state/object (ebs{kms_key_id, snapshot_id})
  end

  note over Prov: EBS fields threaded both directions for round-trip preservation
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify schema additions and types in internal/provider/node_policy.go.
  • Review toProto and fromProto mappings for consistent field names and nil/empty handling.
  • Check runtime object type construction (types.ObjectValueMust / attribute maps) for presence and ordering of new attributes.
  • Confirm docs reflect the exact schema names and descriptions.

Poem

🐰 I hopped into schemas, soft and spry,

Added keys and snapshots to volumes on high,
kms and snaps tucked snug in the trail,
Round-tripped neat without a fail,
A rabbit's patchwork—small but sly. 🥕🔐

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Add missing props' is vague and generic, providing no specific information about what props are being added or which component is affected. Use a more descriptive title that specifies the change, such as 'Add kms_key_id and snapshot_id to EBS block device mappings' or 'Add encryption-related fields to AWS EBS configuration'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch tzvonimir/missing-props

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
docs/resources/node_policy.md (1)

169-179: Consider enriching the example to demonstrate the new EBS fields.

The comprehensive AWS example shows basic block device configuration but doesn't demonstrate the newly added kms_key_id and snapshot_id fields. These are specialty use cases, but adding them to the example could help users understand the full range of options.

     block_device_mappings = [
       {
         device_name = "/dev/xvda"
         ebs = {
           volume_size           = "100Gi"
           volume_type           = "gp3"
           encrypted             = true
           delete_on_termination = true
+          kms_key_id            = "arn:aws:kms:region:account:key/key-id"
+          snapshot_id           = "snap-0123456789abcdef0"
         }
       }
     ]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0fb3942 and 1e4f119.

📒 Files selected for processing (1)
  • docs/resources/node_policy.md (1 hunks)
🔇 Additional comments (1)
docs/resources/node_policy.md (1)

399-400: Documentation additions are well-formatted and consistent.

The two new EBS fields (kms_key_id and snapshot_id) are correctly positioned in the nested schema section with clear, descriptive labels that align with AWS EBS terminology.

@Tzvonimir Tzvonimir merged commit cc46cbb into main Nov 7, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants