fix(tfjson): Reset Computed flag for SchemaNestingModeSingle blocks with required children

[rwwiv]

Summary

Fixes an issue where SchemaNestingModeSingle blocks from Plugin Framework resources were incorrectly marked as Computed=true, causing them to be excluded from ForProvider/InitProvider parameters and only appear in Observation.

Problem

In tfJSONBlockTypeToV2Schema(), when MinItems==0 && MaxItems==0, the block was being marked as Computed=true:

if nb.MinItems == 0 && nb.MaxItems == 0 {
    v2sch.Computed = true
}

This heuristic works for SDK v2 collection types (List/Set/Map) where MinItems=0, MaxItems=0 typically indicates a computed-only collection. However, Plugin Framework resources using SchemaNestingModeSingle default to MinItems=0, MaxItems=0 even for user-configurable blocks.

This caused blocks like metadata and spec to be marked Computed=true, which makes IsObservation() return true, excluding them from the generated Parameters struct.

Solution

Move the Computed=true inference to only apply to collection nesting modes (Set/List/Map), not to SchemaNestingModeSingle. For single blocks, we now rely solely on hasRequiredChild() to determine Required/Optional, leaving Computed=false by default.

case tfjson.SchemaNestingModeSet:
    v2sch.Type = schemav2.TypeSet
    // For collection types, infer Computed when MinItems and MaxItems are both 0
    if nb.MinItems == 0 && nb.MaxItems == 0 {
        v2sch.Computed = true
    }
// ... same for List and Map ...

case tfjson.SchemaNestingModeSingle:
    // For SchemaNestingModeSingle (Plugin Framework), we do NOT infer
    // Computed=true from MinItems/MaxItems==0, because Plugin Framework
    // resources default to 0 for these values even for user-configurable blocks.
    v2sch.Type = schemav2.TypeList
    v2sch.Required = hasRequiredChild(nb)
    v2sch.Optional = !v2sch.Required
    // Computed remains false (the default)

Testing

Added comprehensive unit tests for tfJSONBlockTypeToV2Schema and hasRequiredChild:

• SchemaNestingModeSingle with required children → Computed=false, Required=true
• SchemaNestingModeSingle with only optional children → Computed=false, Optional=true
• SchemaNestingModeSingle with empty/nil block
• SchemaNestingModeSingle with nested blocks containing required children
• SchemaNestingModeList/Set/Map with MinItems=0, MaxItems=0 → Computed=true (preserves existing behavior)
• SchemaNestingModeList with MinItems=1 → Computed=false

Impact

This fix is particularly important for Terraform Plugin Framework resources. For example, the Grafana provider's App Platform resources (grafana_apps_rules_alertrule_v0alpha1, grafana_apps_rules_recordingrule_v0alpha1, etc.) have metadata and spec blocks that were incorrectly excluded from forProvider in the generated CRDs.

Before this fix:

spec:
  forProvider:
    options: {}  # Only options appeared

After this fix:

spec:
  forProvider:
    metadata:
      uid: "..."
      folderUid: "..."
    spec:
      title: "..."
      expressions: {}
      # ... all other fields
    options: {}

SDK v2-based resources are unaffected because they typically use MinItems=1, MaxItems=1 for required single blocks, so they never hit the MinItems == 0 && MaxItems == 0 path.

Summary by CodeRabbit

• Bug Fixes

  • Corrected inference of computed vs required/optional across nesting modes so nested configurations are treated as inputs when appropriate.
  • Single-instance (single/group) blocks now honor required/optional determination and enforce min/max item constraints instead of blanket computed behavior.

• Tests

  • Added comprehensive tests for schema conversions, required-child detection, deep nesting, and edge cases.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

tfJSONBlockTypeToV2Schema now differentiates collection nesting modes (Set/List/Map) — inferring Computed when MinItems and MaxItems are both 0 — from Single/Group blocks, which are treated as list-like (TypeList, MinItems=0, MaxItems=1) with Required/Optional derived from whether the block has required children.

Changes

Cohort / File(s)	Summary
Schema conversion logic pkg/types/conversion/tfjson/tfjson.go	Refactors tfJSONBlockTypeToV2Schema: for collection modes (Set/List/Map) infer Computed when MinItems==0 && MaxItems==0; for Single/Group set TypeList, MinItems=0, MaxItems=1, and determine Required/Optional via hasRequiredChild instead of inferring Computed from min/max. Adds clarifying comments.
Tests for conversion and required-child detection pkg/types/conversion/tfjson/tfjson_test.go	Adds comprehensive table-driven tests TestTfJSONBlockTypeToV2Schema and TestHasRequiredChild covering Single/List/Set/Map mappings, MinItems/MaxItems/Computed interactions, nested blocks with required attributes, nil/empty handling, and detection of required children across nested hierarchies.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hop through schema fields with glee,
Counting children under each tiny tree.
When zeros align, Computed waves hello,
Singles dress as lists, required ones show.
A carrot-coded change — tidy and spry!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main fix: addressing the Computed flag issue for SchemaNestingModeSingle blocks with required children, which directly corresponds to the primary changes in the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

🧪 Unit Test Generation v2 is now available!

We have significantly improved our unit test generation capabilities.

To enable: Add this to your .coderabbit.yaml configuration:

reviews:
  finishing_touches:
    unit_tests:
      enabled: true

Try it out by using the @coderabbitai generate unit tests command on your code files or under ✨ Finishing Touches on the walkthrough!

Have feedback? Share your thoughts on our Discord thread!

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/types/conversion/tfjson/tfjson.go (1)

170-179: Add regression test for required-child Single blocks.

The behavior introduced around Line 174 should be locked in with a unit test to prevent regressions (Computed must be false when a Single block has required children).

🧪 Suggested test (standard Go testing)

+// SPDX-FileCopyrightText: 2023 The Crossplane Authors <https://crossplane.io>
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package tfjson
+
+import (
+	"testing"
+
+	tfjsonsdk "github.com/hashicorp/terraform-json"
+	"github.com/zclconf/go-cty/cty"
+)
+
+func TestTFJSONBlockTypeToV2Schema_SingleRequiredChildResetsComputed(t *testing.T) {
+	nb := &tfjsonsdk.SchemaBlockType{
+		NestingMode: tfjsonsdk.SchemaNestingModeSingle,
+		MinItems:    0,
+		MaxItems:    0,
+		Block: &tfjsonsdk.SchemaBlock{
+			Attributes: map[string]*tfjsonsdk.SchemaAttribute{
+				"name": {Required: true, AttributeType: cty.String},
+			},
+		},
+	}
+
+	got := tfJSONBlockTypeToV2Schema(nb)
+
+	if got.Computed {
+		t.Fatalf("expected Computed=false for required-child single block")
+	}
+	if !got.Required || got.Optional {
+		t.Fatalf("expected Required=true and Optional=false")
+	}
+	if got.MinItems != 1 || got.MaxItems != 1 {
+		t.Fatalf("expected MinItems/MaxItems=1, got %d/%d", got.MinItems, got.MaxItems)
+	}
+}
+```
</details>
As per coding guidelines, "All Upjet code must be covered by tests; do not use Ginkgo or third-party testing libraries, use only standard Go testing".

</blockquote></details>

</blockquote></details>

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/types/conversion/tfjson/tfjson.go (1)

176-191: ⚠️ Potential issue | 🟠 Major

The heuristic Required = hasRequiredChild(nb) is incorrect for Plugin Framework semantics and will over-constrain the CRD.

In Terraform Plugin Framework, SingleNestedBlock and SingleNestedAttribute (Group) have no built-in Required flag on the block itself—blocks are optional by default. Required child attributes do not imply the block is required; they only apply when the block is present. To make a block required in Plugin Framework, you must use validators (e.g., objectvalidator.IsRequired()).

The current code marks a block as Required whenever it contains required children, which incorrectly forces optional blocks to be required in the generated schema. This breaks configurations where the block is legitimately optional.

Fix: Determine block Required/Optional status from the tfjson schema itself (if available), not from child attribute requirements. If tfjson does not encode block-level Required, consider whether the absence of a Required indicator in the Plugin Framework schema should default to Optional = true.