feat(provider)!: track the Cozystack 1.6 API line - #27
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 84 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesProvider contracts and shared conversion
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: 🔵 Low · up to The provider upgrade adds new API surfaces and changes state/spec handling. Remaining bounded risks include misleading diagnostics for unresolved JSON values, accepting an empty custom configuration, and a few documentation wording inconsistencies; these are mergeable with explicit owner follow-up and no supplied evidence of high-impact production impact. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
internal/provider/schema_helpers_test.go (1)
15-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlso detect an exempt entry that matches no attribute.
The guard rejects an exemption that stops being needed. It does not reject an exemption whose path no longer exists, for example after a rename of
node_groups. Such an entry then silently exempts nothing and the comment above it becomes wrong.Track the exemptions the loop consumes, then fail on the leftovers.
♻️ Proposed guard for stale exemptions
func TestStorageClassRequiresReplace(t *testing.T) { t.Parallel() + used := map[string]bool{} + for typeName, found := range storageClassAttributes(t) { for path, attribute := range found { @@ if storageClassReplaceExempt[typeName+"."+path] { + used[typeName+"."+path] = true + if replaces { t.Errorf("%s: %s requires replacement now; drop it from the exempt list", typeName, path) } continue } @@ } } + + for key := range storageClassReplaceExempt { + if !used[key] { + t.Errorf("exempt entry %q matches no storage_class attribute", key) + } + } }Also applies to: 48-54
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/provider/schema_helpers_test.go` around lines 15 - 23, Update the storage-class exemption validation loop in the relevant schema helper test to track each entry consumed while matching resource attributes, then fail if any entries remain unmatched after the loop. Preserve the existing exemption behavior while ensuring renamed or removed attribute paths cannot remain silently in storageClassReplaceExempt.internal/provider/kubernetes_model.go (1)
814-840: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
control_planeandtalostreat an empty configured block differently.
expandControlPlanereturnsnilwhenapi_serveris unset, socontrol_plane = {}writes no spec key at all.expandTalosreturns an empty map fortalos = {}, so that block writestalos: {}. The two shapes are inconsistent for the same practitioner action.The behavior is not wrong today, because
keepConfiguredAttributesrebuilds the configured shape in state. State the rule in the comment, or return an empty map here for consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/provider/kubernetes_model.go` around lines 814 - 840, Update expandControlPlane so an empty configured control_plane block returns an empty map rather than nil when api_server is unset, matching expandTalos and preserving the configured block shape; alternatively, document in the function comment that nil is intentional because keepConfiguredAttributes reconstructs the state shape.internal/provider/kubernetes_schema.go (1)
237-266: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueA
custom_configblock with neither field set expands to an empty spec key.
configandsecret_refare bothOptional. A practitioner can writecustom_config = {}.expandOIDCCustomConfigthen returns an empty map, and the provider sendscustomConfig: {}. The chart has no configuration to mount in that case.Add a validator that requires at least one of the two fields, so the error appears at plan time.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/provider/kubernetes_schema.go` around lines 237 - 266, Update k8sOIDCCustomConfigResourceAttribute to add an object-level validator requiring at least one of config or secret_ref to be set, while preserving their existing mutual-exclusion validator. Ensure custom_config = {} is rejected during plan validation before expandOIDCCustomConfig can produce an empty map.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Line 17: Update the client-group guidance in the line describing
internal/client/groups.go to include gateway.cozystack.io alongside the existing
cozystack.io, backups.cozystack.io, and core.cozystack.io groups.
In `@docs/resources/postgres.md`:
- Around line 6-11: Update the source description for cozystack_postgres to use
“Cozystack-managed” in both generated description contexts, then run make docs
to regenerate the documentation. Do not edit docs/resources/postgres.md
directly, and verify regeneration leaves no additional diff.
Apply the same fix in `@docs/resources/clickhouse.md` at line 6: The same wording
correction is required in the ClickHouse generated description.
In `@internal/provider/kubernetes_schema.go`:
- Around line 493-509: Update the node_groups schema’s resources attribute in
the node-group data-source definition so its description states CPU and memory
are per worker node, matching the resource schema. Either define the resources
block inline with node-specific wording or extend resourcesDataSourceAttribute()
to accept and use a custom description, while preserving the existing
description for other callers.
In `@internal/provider/spec.go`:
- Around line 693-765: Update setOptionalJSONList to inspect each
jsontypes.Normalized element before unmarshalling and skip null or unknown
elements, preventing unresolved values from being reported as malformed JSON.
Preserve the existing invalid-JSON diagnostic for known elements whose
ValueString() cannot be unmarshalled.
In `@internal/provider/tenant_resource.go`:
- Around line 180-185: The tenant resource documentation must explicitly state
that the host apex is not covered by the ancestor’s certificate and that the
tenant must set gateway = true. Update the source schema or documentation
generator input containing this text, not generated docs, while preserving the
surrounding explanation.
In `@README.md`:
- Around line 61-71: The README’s generic statement that every added Kind is a
typed model is inaccurate because cozystack_tenant_gateway uses rawSpecNsSchema.
Update that statement to describe both typed models and raw-spec
implementations, while preserving the existing distinction between user-authored
typed resources and JSON-spec resources.
---
Nitpick comments:
In `@internal/provider/kubernetes_model.go`:
- Around line 814-840: Update expandControlPlane so an empty configured
control_plane block returns an empty map rather than nil when api_server is
unset, matching expandTalos and preserving the configured block shape;
alternatively, document in the function comment that nil is intentional because
keepConfiguredAttributes reconstructs the state shape.
In `@internal/provider/kubernetes_schema.go`:
- Around line 237-266: Update k8sOIDCCustomConfigResourceAttribute to add an
object-level validator requiring at least one of config or secret_ref to be set,
while preserving their existing mutual-exclusion validator. Ensure custom_config
= {} is rejected during plan validation before expandOIDCCustomConfig can
produce an empty map.
In `@internal/provider/schema_helpers_test.go`:
- Around line 15-23: Update the storage-class exemption validation loop in the
relevant schema helper test to track each entry consumed while matching resource
attributes, then fail if any entries remain unmatched after the loop. Preserve
the existing exemption behavior while ensuring renamed or removed attribute
paths cannot remain silently in storageClassReplaceExempt.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d52fc21-a878-41c6-9af8-6f1a6e93a9e3
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (96)
CHANGELOG.mdCLAUDE.mdREADME.mddocs/data-sources/clickhouse.mddocs/data-sources/kafka.mddocs/data-sources/kubernetes.mddocs/data-sources/kubernetes_nodes.mddocs/data-sources/nats.mddocs/data-sources/postgres.mddocs/data-sources/qdrant.mddocs/data-sources/tenant.mddocs/data-sources/tenant_gateway.mddocs/resources/clickhouse.mddocs/resources/foundationdb.mddocs/resources/harbor.mddocs/resources/httpcache.mddocs/resources/kafka.mddocs/resources/kubernetes.mddocs/resources/kubernetes_nodes.mddocs/resources/mariadb.mddocs/resources/marketplace_panel.mddocs/resources/mongodb.mddocs/resources/nats.mddocs/resources/openbao.mddocs/resources/opensearch.mddocs/resources/postgres.mddocs/resources/qdrant.mddocs/resources/rabbitmq.mddocs/resources/redis.mddocs/resources/tenant.mddocs/resources/tenant_gateway.mddocs/resources/vmdisk.mdexamples/data-sources/cozystack_kubernetes/data-source.tfexamples/data-sources/cozystack_kubernetes_nodes/data-source.tfexamples/data-sources/cozystack_marketplace_panel/data-source.tfexamples/data-sources/cozystack_tenant/data-source.tfexamples/data-sources/cozystack_tenant_gateway/data-source.tfexamples/resources/cozystack_clickhouse/resource.tfexamples/resources/cozystack_kafka/resource.tfexamples/resources/cozystack_kubernetes/resource.tfexamples/resources/cozystack_kubernetes_nodes/import.shexamples/resources/cozystack_kubernetes_nodes/resource.tfexamples/resources/cozystack_marketplace_panel/resource.tfexamples/resources/cozystack_nats/resource.tfexamples/resources/cozystack_postgres/resource.tfexamples/resources/cozystack_qdrant/resource.tfexamples/resources/cozystack_tenant/resource.tfexamples/resources/cozystack_tenant_gateway/resource.tfgo.modinternal/client/groups.gointernal/client/kubernetes.gointernal/provider/acc_test.gointernal/provider/clickhouse_model.gointernal/provider/clickhouse_model_test.gointernal/provider/clickhouse_schema.gointernal/provider/foundationdb_schema.gointernal/provider/generic_resource.gointernal/provider/harbor_schema.gointernal/provider/httpcache_schema.gointernal/provider/kafka_model.gointernal/provider/kafka_model_test.gointernal/provider/kafka_schema.gointernal/provider/kubernetes_model.gointernal/provider/kubernetes_model_test.gointernal/provider/kubernetes_nodes_model.gointernal/provider/kubernetes_nodes_model_test.gointernal/provider/kubernetes_nodes_schema.gointernal/provider/kubernetes_schema.gointernal/provider/kubernetes_schema_test.gointernal/provider/mariadb_schema.gointernal/provider/mongodb_schema.gointernal/provider/nats_model.gointernal/provider/nats_model_test.gointernal/provider/nats_schema.gointernal/provider/openbao_schema.gointernal/provider/opensearch_schema.gointernal/provider/postgresql_model.gointernal/provider/postgresql_model_test.gointernal/provider/postgresql_schema.gointernal/provider/provider.gointernal/provider/qdrant_data_source.gointernal/provider/qdrant_model.gointernal/provider/qdrant_model_test.gointernal/provider/qdrant_resource.gointernal/provider/rabbitmq_schema.gointernal/provider/rawspec_schema.gointernal/provider/redis_resource.gointernal/provider/schema_helpers.gointernal/provider/schema_helpers_test.gointernal/provider/spec.gointernal/provider/spec_test.gointernal/provider/tenant_data_source.gointernal/provider/tenant_model.gointernal/provider/tenant_model_test.gointernal/provider/tenant_resource.gointernal/provider/vmdisk_schema.go
💤 Files with no reviewable changes (3)
- examples/resources/cozystack_marketplace_panel/resource.tf
- examples/data-sources/cozystack_marketplace_panel/data-source.tf
- docs/resources/marketplace_panel.md
| The provider talks to the Cozystack aggregated Kubernetes API (`apps.cozystack.io/v1alpha1` and sibling groups) with `client-go`'s **dynamic client** — there is no typed clientset. "Typing" lives at the Terraform layer: typed schema attributes ↔ `map[string]any` spec ↔ `unstructured` over the wire. Most Kinds are an `Application` whose `spec` is an opaque JSON blob the server turns into a FluxCD `HelmRelease`. | ||
|
|
||
| - `internal/client/` — the wire layer. `application.go` holds the generalized `client.Resource{Resource,Kind,Group,Version,ClusterScoped,NoSpec}` (defaults to `apps.cozystack.io`/`v1alpha1`/namespaced) and CRUD over the dynamic client. `config.go` builds the `rest.Config` (kubeconfig/context/`host`+`token`/mTLS/`in_cluster`/`exec` credential plugin). `outputs.go` reads server-generated child Secrets/Services for connection details. `platform.go`/`groups.go` declare the `cozystack.io`, `backups.cozystack.io`, `core.cozystack.io`, and `dashboard.cozystack.io` Resources. | ||
| - `internal/client/` — the wire layer. `application.go` holds the generalized `client.Resource{Resource,Kind,Group,Version,ClusterScoped,NoSpec}` (defaults to `apps.cozystack.io`/`v1alpha1`/namespaced) and CRUD over the dynamic client. `config.go` builds the `rest.Config` (kubeconfig/context/`host`+`token`/mTLS/`in_cluster`/`exec` credential plugin). `outputs.go` reads server-generated child Secrets/Services for connection details. `platform.go`/`groups.go` declare the `cozystack.io`, `backups.cozystack.io`, and `core.cozystack.io` Resources. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add gateway.cozystack.io to the client-group guidance.
Line 17 lists the groups declared by internal/client/groups.go, but it omits gateway.cozystack.io. This PR adds cozystack_tenant_gateway in that group. Update the list so the architecture guidance covers the new wire-level group.
The v1.6.1 changelog identifies gateway.cozystack.io as the group for cozystack_tenant_gateway.
Proposed update
- `platform.go`/`groups.go` declare the `cozystack.io`, `backups.cozystack.io`, and `core.cozystack.io` Resources.
+ `platform.go`/`groups.go` declare the `cozystack.io`, `backups.cozystack.io`, `core.cozystack.io`, and `gateway.cozystack.io` Resources.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `internal/client/` — the wire layer. `application.go` holds the generalized `client.Resource{Resource,Kind,Group,Version,ClusterScoped,NoSpec}` (defaults to `apps.cozystack.io`/`v1alpha1`/namespaced) and CRUD over the dynamic client. `config.go` builds the `rest.Config` (kubeconfig/context/`host`+`token`/mTLS/`in_cluster`/`exec` credential plugin). `outputs.go` reads server-generated child Secrets/Services for connection details. `platform.go`/`groups.go` declare the `cozystack.io`, `backups.cozystack.io`, and `core.cozystack.io` Resources. | |
| - `platform.go`/`groups.go` declare the `cozystack.io`, `backups.cozystack.io`, `core.cozystack.io`, and `gateway.cozystack.io` Resources. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLAUDE.md` at line 17, Update the client-group guidance in the line
describing internal/client/groups.go to include gateway.cozystack.io alongside
the existing cozystack.io, backups.cozystack.io, and core.cozystack.io groups.
Comment on lines
+6
to
+11
| A Cozystack managed PostgreSQL instance, deployed inside a tenant namespace. The postgresql tuning, quorum, and bootstrap blocks use server defaults. | ||
| --- | ||
|
|
||
| # cozystack_postgres (Resource) | ||
|
|
||
| A Cozystack managed PostgreSQL instance, deployed inside a tenant namespace. The postgresql tuning, quorum, deprecated backup, and bootstrap blocks use server defaults. | ||
| A Cozystack managed PostgreSQL instance, deployed inside a tenant namespace. The postgresql tuning, quorum, and bootstrap blocks use server defaults. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align generated product descriptions with the source wording.
The PostgreSQL and ClickHouse descriptions use Cozystack managed instead of Cozystack-managed. Update both source descriptions and regenerate the generated documentation so the checked-in docs match; do not hand-edit generated files.
📍 Affects 2 files
docs/resources/postgres.md#L6-L11(this comment)docs/resources/clickhouse.md#L6-L6
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/resources/postgres.md` around lines 6 - 11, Update the source
description for cozystack_postgres to use “Cozystack-managed” in both generated
description contexts, then run make docs to regenerate the documentation. Do not
edit docs/resources/postgres.md directly, and verify regeneration leaves no
additional diff.
Apply the same fix in `@docs/resources/clickhouse.md` at line 6: The same wording
correction is required in the ClickHouse generated description.
Sources: Coding guidelines, Learnings, Linters/SAST tools
Comment on lines
+493
to
+509
| "disk_size": dsschema.StringAttribute{Computed: true, MarkdownDescription: "Persistent disk size."}, | ||
| "instance_type": dsschema.StringAttribute{Computed: true, MarkdownDescription: "Instance type."}, | ||
| "min_replicas": dsschema.Int64Attribute{Computed: true, MarkdownDescription: "Minimum replicas."}, | ||
| "max_replicas": dsschema.Int64Attribute{Computed: true, MarkdownDescription: "Maximum replicas."}, | ||
| "roles": dsschema.ListAttribute{Computed: true, ElementType: types.StringType, MarkdownDescription: "Node roles."}, | ||
| "storage_class": dsschema.StringAttribute{Computed: true, MarkdownDescription: "Worker node StorageClass."}, | ||
| "resources": resourcesDataSourceAttribute(), | ||
| "max_unhealthy": dsschema.StringAttribute{Computed: true, MarkdownDescription: "Per-group unhealthy-node tolerance."}, | ||
| "node_startup_timeout": dsschema.StringAttribute{Computed: true, MarkdownDescription: "Per-group machine startup timeout."}, | ||
| }, | ||
| }, | ||
| }, | ||
| attrTalos: k8sTalosDataSourceAttribute(), | ||
| attrNodeHealthCheck: k8sNodeHealthCheckDataSourceAttribute(), | ||
| attrOIDC: k8sOIDCDataSourceAttribute(), | ||
| attrControlPlane: k8sControlPlaneDataSourceAttribute(), | ||
| attrImages: k8sImagesDataSourceAttribute(), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The shared resources description does not match a node group.
resourcesDataSourceAttribute() describes CPU and memory "per replica". In node_groups the values are per worker node, which is how the resource schema describes them at Line 87. The generated data-source documentation then contradicts the resource documentation.
Declare the node-group resources block inline with node-oriented wording, or pass the description into the shared helper.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/provider/kubernetes_schema.go` around lines 493 - 509, Update the
node_groups schema’s resources attribute in the node-group data-source
definition so its description states CPU and memory are per worker node,
matching the resource schema. Either define the resources block inline with
node-specific wording or extend resourcesDataSourceAttribute() to accept and use
a custom description, while preserving the existing description for other
callers.
Comment on lines
+693
to
+765
| // setOptionalJSONList writes a list of JSON documents into spec under key. The | ||
| // upstream fields are free-form core/v1 objects, so they travel as normalized | ||
| // JSON strings rather than a hand-modelled Volume schema. | ||
| func setOptionalJSONList( | ||
| ctx context.Context, | ||
| spec map[string]any, | ||
| key string, | ||
| value types.List, | ||
| ) diag.Diagnostics { | ||
| var diags diag.Diagnostics | ||
|
|
||
| if value.IsNull() || value.IsUnknown() { | ||
| return diags | ||
| } | ||
|
|
||
| var items []jsontypes.Normalized | ||
|
|
||
| diags.Append(value.ElementsAs(ctx, &items, false)...) | ||
|
|
||
| if diags.HasError() { | ||
| return diags | ||
| } | ||
|
|
||
| out := make([]any, 0, len(items)) | ||
|
|
||
| for index, item := range items { | ||
| var document any | ||
|
|
||
| if err := json.Unmarshal([]byte(item.ValueString()), &document); err != nil { | ||
| diags.AddError( | ||
| "Invalid JSON document in "+key, | ||
| fmt.Sprintf("Element %d is not a JSON document: %s", index, err), | ||
| ) | ||
|
|
||
| return diags | ||
| } | ||
|
|
||
| out = append(out, document) | ||
| } | ||
|
|
||
| spec[key] = out | ||
|
|
||
| return diags | ||
| } | ||
|
|
||
| // specJSONListOrNull builds a list of JSON documents from a spec value. An | ||
| // absent key flattens to null; a present empty list stays an empty list. | ||
| func specJSONListOrNull(raw any) (types.List, diag.Diagnostics) { | ||
| var diags diag.Diagnostics | ||
|
|
||
| items, ok := raw.([]any) | ||
| if !ok { | ||
| return types.ListNull(jsontypes.NormalizedType{}), diags | ||
| } | ||
|
|
||
| elements := make([]attr.Value, 0, len(items)) | ||
|
|
||
| for _, item := range items { | ||
| encoded, err := json.Marshal(item) | ||
| if err != nil { | ||
| diags.AddError("Unable to encode a spec document", err.Error()) | ||
|
|
||
| return types.ListNull(jsontypes.NormalizedType{}), diags | ||
| } | ||
|
|
||
| elements = append(elements, jsontypes.NewNormalizedValue(string(encoded))) | ||
| } | ||
|
|
||
| value, listDiags := types.ListValue(jsontypes.NormalizedType{}, elements) | ||
| diags.Append(listDiags...) | ||
|
|
||
| return value, diags | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
setOptionalJSONList reports an unknown element as invalid JSON.
An unknown jsontypes.Normalized element returns "" from ValueString(), so json.Unmarshal fails and the diagnostic says the element "is not a JSON document". That message is misleading for a value that is unknown rather than malformed. The list itself is skipped when the whole list is unknown, but a known list can hold an unknown element, for example when an element interpolates an unresolved attribute of another resource.
Skip null and unknown elements, or report them with their own message.
🐛 Proposed handling for null and unknown elements
for index, item := range items {
var document any
+ if item.IsNull() || item.IsUnknown() {
+ diags.AddError(
+ "Missing JSON document in "+key,
+ fmt.Sprintf("Element %d has no value.", index),
+ )
+
+ return diags
+ }
+
if err := json.Unmarshal([]byte(item.ValueString()), &document); err != nil {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // setOptionalJSONList writes a list of JSON documents into spec under key. The | |
| // upstream fields are free-form core/v1 objects, so they travel as normalized | |
| // JSON strings rather than a hand-modelled Volume schema. | |
| func setOptionalJSONList( | |
| ctx context.Context, | |
| spec map[string]any, | |
| key string, | |
| value types.List, | |
| ) diag.Diagnostics { | |
| var diags diag.Diagnostics | |
| if value.IsNull() || value.IsUnknown() { | |
| return diags | |
| } | |
| var items []jsontypes.Normalized | |
| diags.Append(value.ElementsAs(ctx, &items, false)...) | |
| if diags.HasError() { | |
| return diags | |
| } | |
| out := make([]any, 0, len(items)) | |
| for index, item := range items { | |
| var document any | |
| if err := json.Unmarshal([]byte(item.ValueString()), &document); err != nil { | |
| diags.AddError( | |
| "Invalid JSON document in "+key, | |
| fmt.Sprintf("Element %d is not a JSON document: %s", index, err), | |
| ) | |
| return diags | |
| } | |
| out = append(out, document) | |
| } | |
| spec[key] = out | |
| return diags | |
| } | |
| // specJSONListOrNull builds a list of JSON documents from a spec value. An | |
| // absent key flattens to null; a present empty list stays an empty list. | |
| func specJSONListOrNull(raw any) (types.List, diag.Diagnostics) { | |
| var diags diag.Diagnostics | |
| items, ok := raw.([]any) | |
| if !ok { | |
| return types.ListNull(jsontypes.NormalizedType{}), diags | |
| } | |
| elements := make([]attr.Value, 0, len(items)) | |
| for _, item := range items { | |
| encoded, err := json.Marshal(item) | |
| if err != nil { | |
| diags.AddError("Unable to encode a spec document", err.Error()) | |
| return types.ListNull(jsontypes.NormalizedType{}), diags | |
| } | |
| elements = append(elements, jsontypes.NewNormalizedValue(string(encoded))) | |
| } | |
| value, listDiags := types.ListValue(jsontypes.NormalizedType{}, elements) | |
| diags.Append(listDiags...) | |
| return value, diags | |
| } | |
| // setOptionalJSONList writes a list of JSON documents into spec under key. The | |
| // upstream fields are free-form core/v1 objects, so they travel as normalized | |
| // JSON strings rather than a hand-modelled Volume schema. | |
| func setOptionalJSONList( | |
| ctx context.Context, | |
| spec map[string]any, | |
| key string, | |
| value types.List, | |
| ) diag.Diagnostics { | |
| var diags diag.Diagnostics | |
| if value.IsNull() || value.IsUnknown() { | |
| return diags | |
| } | |
| var items []jsontypes.Normalized | |
| diags.Append(value.ElementsAs(ctx, &items, false)...) | |
| if diags.HasError() { | |
| return diags | |
| } | |
| out := make([]any, 0, len(items)) | |
| for index, item := range items { | |
| var document any | |
| if item.IsNull() || item.IsUnknown() { | |
| diags.AddError( | |
| "Missing JSON document in "+key, | |
| fmt.Sprintf("Element %d has no value.", index), | |
| ) | |
| return diags | |
| } | |
| if err := json.Unmarshal([]byte(item.ValueString()), &document); err != nil { | |
| diags.AddError( | |
| "Invalid JSON document in "+key, | |
| fmt.Sprintf("Element %d is not a JSON document: %s", index, err), | |
| ) | |
| return diags | |
| } | |
| out = append(out, document) | |
| } | |
| spec[key] = out | |
| return diags | |
| } | |
| // specJSONListOrNull builds a list of JSON documents from a spec value. An | |
| // absent key flattens to null; a present empty list stays an empty list. | |
| func specJSONListOrNull(raw any) (types.List, diag.Diagnostics) { | |
| var diags diag.Diagnostics | |
| items, ok := raw.([]any) | |
| if !ok { | |
| return types.ListNull(jsontypes.NormalizedType{}), diags | |
| } | |
| elements := make([]attr.Value, 0, len(items)) | |
| for _, item := range items { | |
| encoded, err := json.Marshal(item) | |
| if err != nil { | |
| diags.AddError("Unable to encode a spec document", err.Error()) | |
| return types.ListNull(jsontypes.NormalizedType{}), diags | |
| } | |
| elements = append(elements, jsontypes.NewNormalizedValue(string(encoded))) | |
| } | |
| value, listDiags := types.ListValue(jsontypes.NormalizedType{}, elements) | |
| diags.Append(listDiags...) | |
| return value, diags | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/provider/spec.go` around lines 693 - 765, Update setOptionalJSONList
to inspect each jsontypes.Normalized element before unmarshalling and skip null
or unknown elements, preventing unresolved values from being reported as
malformed JSON. Preserve the existing invalid-JSON diagnostic for known elements
whose ValueString() cannot be unmarshalled.
Comment on lines
+180
to
+185
| "Without it the tenant publishes through the nearest ancestor that owns one — routing is " + | ||
| "not skipped, only ownership — which is why a tenant whose `host` is an apex the ancestor's " + | ||
| "certificate does not cover has to ask for `true`. In the pinned release `false` and leaving " + | ||
| "the attribute out resolve to the same thing; the attribute is still omitted from the spec " + | ||
| "when unset, because the chart distinguishes the states by the key's presence and rejects a " + | ||
| "null.", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clarify the apex certificate condition.
The phrase whose \host` is an apex the ancestor's certificate does not coveris incomplete. State that the apex is not covered by the ancestor's certificate and that the tenant must setgateway = true`.
Proposed wording
- "not skipped, only ownership — which is why a tenant whose `host` is an apex the ancestor's " +
- "certificate does not cover has to ask for `true`. In the pinned release `false` and leaving " +
+ "not skipped, only ownership — which is why a tenant whose `host` is an apex that the ancestor's " +
+ "certificate does not cover must set `gateway = true`. In the pinned release `false` and leaving " +As per coding guidelines, docs/**: Never hand-edit docs/; regenerate the generated documentation from this schema.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "Without it the tenant publishes through the nearest ancestor that owns one — routing is " + | |
| "not skipped, only ownership — which is why a tenant whose `host` is an apex the ancestor's " + | |
| "certificate does not cover has to ask for `true`. In the pinned release `false` and leaving " + | |
| "the attribute out resolve to the same thing; the attribute is still omitted from the spec " + | |
| "when unset, because the chart distinguishes the states by the key's presence and rejects a " + | |
| "null.", | |
| "Without it the tenant publishes through the nearest ancestor that owns one — routing is " + | |
| "not skipped, only ownership — which is why a tenant whose `host` is an apex that the ancestor's " + | |
| "certificate does not cover must set `gateway = true`. In the pinned release `false` and leaving " + | |
| "the attribute out resolve to the same thing; the attribute is still omitted from the spec " + | |
| "when unset, because the chart distinguishes the states by the key's presence and rejects a " + | |
| "null.", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/provider/tenant_resource.go` around lines 180 - 185, The tenant
resource documentation must explicitly state that the host apex is not covered
by the ancestor’s certificate and that the tenant must set gateway = true.
Update the source schema or documentation generator input containing this text,
not generated docs, while preserving the surrounding explanation.
Source: Coding guidelines
Comment on lines
61
to
+71
|
|
||
| `backup_plan` (schedule a backup) and `restore_job` (restore a backup) are user-authored, so they are fully typed (`application_ref`, `backup_class_name`, `schedule` / `backup_name`, `target_application_ref`, `options`). The remaining backups kinds are records or driver config and stay JSON-spec. | ||
|
|
||
| ### Gateway (`gateway.cozystack.io`) | ||
|
|
||
| | Kind | Resource | Data source | | ||
| | --- | --- | --- | | ||
| | TenantGateway — per-tenant Gateway API / Cilium Gateway | [`cozystack_tenant_gateway`](docs/resources/tenant_gateway.md) | [`cozystack_tenant_gateway`](docs/data-sources/tenant_gateway.md) | | ||
|
|
||
| Namespaced, JSON-spec (`apex`, `certMode`, `issuerName`, `dns01`, `wildcardSecretRef`, `attachedNamespaces`, `tlsPassthroughServices`, `gatewayClassName`) — the cozystack-controller reconciles the actual Gateway and per-listener Certificate resources from it. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the generic “typed model” statement.
cozystack_tenant_gateway uses rawSpecNsSchema in internal/provider/rawspec_schema.go. The earlier README statement at Line 7 that adding a Kind is a typed model is no longer accurate. Update it to cover typed and raw-spec implementations.
Proposed wording
-Adding a kind is a typed model, a schema, and a one-line resource descriptor — the create/read/update/delete/import/wait logic is shared.
+Adding a kind uses either a typed model or a raw-spec schema, plus a one-line resource descriptor — the create/read/update/delete/import/wait logic is shared.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 61 - 71, The README’s generic statement that every
added Kind is a typed model is inaccurate because cozystack_tenant_gateway uses
rawSpecNsSchema. Update that statement to describe both typed models and
raw-spec implementations, while preserving the existing distinction between
user-authored typed resources and JSON-spec resources.
Optional spec keys mostly treat absent and empty alike, and the existing expand/flatten helpers collapse the two accordingly. A few upstream keys give the empty form its own meaning: the chart substitutes a value of its own when the key is absent, and an explicitly empty value opts out of that substitution. Collapsing the states breaks both directions for those keys. Expanding a null attribute into an empty value writes an opt-out the operator never asked for, and expanding an empty one into an absent key reinstates the substitution they did ask to skip. Flattening a stored empty value back to null then reports drift on every apply. Add string, string-list, and nested-object-list helpers that write a key only when the attribute is set and read it back as null only when it is absent, leaving the collapsing helpers in place for the keys they suit. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
The existing coverage guard reflects one level of json tags, so it checks the top-level cluster spec and stops at nodeGroups. Every field inside a node group is therefore unguarded: upstream can add one and the provider keeps passing while silently not modelling it. Reflect the node-group spec the same way, against the entry expand emits, omitting the two fields the model deliberately leaves to server defaults. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Track the Cozystack 1.6 API line. The spec-coverage guards now flag every field the new tag added (kubernetes talos/nodeHealthCheck/oidc, tls on kafka/nats/qdrant/postgresql, tenant gateway, node-group health checks); follow-up commits on this branch model them. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
…#25) * feat(kubernetes): model the Talos worker image block Cozystack 1.6 moved tenant worker nodes from Ubuntu container disks to Talos, and the image coordinates became part of the Kubernetes spec. Left unmodelled, an operator has no way to point workers at a self-hosted image factory or a custom schematic without dropping out of Terraform. None of the four fields carries a provider-side default. Upstream moves the Talos release and the tested schematic ID with every platform release, so a materialised default would pin a cluster to whatever was current when the provider was built and silently detach it from the platform's rolling value. Unset fields are omitted from the emitted spec instead; the aggregated apiserver materialises its own defaults on every read, so state still reports the effective value. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * feat(kubernetes): model MachineHealthCheck tuning The 1.6 chart remediates worker nodes through a MachineHealthCheck whose tolerance and startup timeout are now part of the Kubernetes spec. Slow first boots — a Talos image pulled from the image factory onto a busy StorageClass — otherwise trip remediation and put the node group into a reboot loop with no way to raise the timeout from Terraform. Both fields are omitted from the spec while unset, so the platform's own tuning stays in force and moves with it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * feat(kubernetes): model tenant apiserver OIDC Without this block the only way into a tenant cluster is the static admin kubeconfig, which cannot be scoped per person and cannot be revoked individually. Modelling it lets an operator turn on platform identity and declare the per-user bindings in the same place the cluster is declared. mode and the user roles are validated against the upstream enums, so a typo fails at plan time rather than as a chart render error. custom_config takes either an inline AuthenticationConfiguration or a Secret reference and the two conflict, matching the chart, which reads only one. users keeps its presence distinction: an explicitly empty list binds nobody, an unset attribute leaves the platform default in place. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * feat(kubernetes): model per-node-group health overrides A node group that boots slower than the rest — a GPU pool pulling a large Talos image, say — needs a longer startup timeout than the cluster-wide one, and 1.6 added the per-group knobs for exactly that. Both are undefaulted upstream, where an absent key means "inherit the cluster-wide value", so they are written only when set rather than as empty strings. The node group's resources block now binds cpu and memory together. Upstream sizes the node by instanceType unless both are set, because KubeVirt cannot override an instance type's CPU and memory, and rejects a half-filled block at render time; the schema turns that into a plan-time error instead of a failed apply. The shared resources helper keeps its own any-field-wins semantics for the kinds that want it. The new model round-trip test binds the whole fixture through the resource schema, which is what catches a tfsdk tag that names no attribute — a failure mode the expand and flatten tests never reach. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * fix(kubernetes): drop v1.30 from the version validator Cozystack 1.6 removed v1.30 from the Kubernetes version enum. The provider still advertised it, so a config naming v1.30 planned cleanly and then failed against the server — the worst shape of validation error, since it surfaces after the practitioner has already committed to the apply. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * feat(kubernetes): model the control-plane apiServer passthrough controlPlane was unmanaged on purpose while it held nothing but component sizing. 1.6 added the apiServer passthrough — extra flags, extra volumes, extra volume mounts — which is the only supported way to hand the tenant kube-apiserver a feature gate or an AuthenticationConfiguration file. That is worth managing; the sizing, replica count, konnectivity and scheduler blocks stay with the server and are recorded as deliberate omissions in the new nested coverage guard. The two volume lists are free-form core/v1 objects upstream, so they travel as normalized JSON strings rather than a hand-modelled Volume schema that would need chasing every core/v1 addition. The attribute description carries the upstream warning about hand-rolled --oidc-* flags, which make the apiserver refuse to start when the chart also injects --authentication-config. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * feat(kubernetes): model image overrides An air-gapped or rate-limited environment needs the tenant-side images pulled from a mirror, and 1.6 exposes all three of them — the bootstrap kubectl Job, the talos-csr-signer sidecar, the wait-for-kubeconfig init container. Until now the whole block was unmanaged, which meant no mirror without editing the release by hand. No field carries a provider-side default. Upstream treats an empty value as "use the tag this chart shipped with", and that tag moves every release, so writing one from the provider would hold a cluster on an image the platform has already replaced. With controlPlane and images now modelled, the top-level coverage guard omits only addons. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * feat(tenant): pin the gateway three-state contract gateway landed in the pinned API, so the coverage guard now demands it and the attribute stops being a speculative passthrough. Its encoding is the subtle part and had no test: the chart distinguishes "unset" from "explicitly off" by whether the key is present, not by its value. An unset attribute therefore has to leave the key out — writing `gateway: null` fails the schema generated from the field's own documentation, and writing `false` silently takes every derived-apex tenant off the auto-enable path. The behaviour was already right; this makes it a contract. The table test covers all four states, a second test covers the read direction, and the attribute description now explains what the platform decides on the operator's behalf instead of describing the field as version-gated. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * feat(kubernetes): require replacement when storage_class changes Upstream marks the cluster storageClass immutable with a CEL rule, but the aggregated apiserver does not evaluate CEL rules embedded in an application schema, so the write is accepted. That is worse than a rejection: apply reports success, state records the new class, and every existing volume stays on the old one, because a PersistentVolumeClaim's class is fixed at creation and editing a StatefulSet's volumeClaimTemplates never migrates data. The divergence is permanent and invisible. Planning a replacement makes the cost of the change visible before it happens, which is the only place a practitioner can still decide against it. The per-node-group storageClass is deliberately left mutable, matching upstream, which does not mark it immutable precisely because a strict rule would block setting an optional field for the first time. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * docs(kubernetes): document the 1.6 blocks in examples and generated docs The examples now show what each new block is for rather than listing every attribute: a node group sized by explicit cpu/memory, a slow GPU pool overriding only its own remediation timeout, a self-hosted image factory with the Talos release still left to the platform, platform identity with user bindings, a bring-your-own issuer with the apiserver passthrough, and mirrored images. The tenant example covers the case that motivates the gateway attribute at all — a custom apex that wants a Gateway anyway, which the platform would not enable on its own. Both example files were checked against the real schema with `tofu validate`, and the resource description no longer claims the control-plane and image blocks are unmanaged. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * test(kubernetes): assert platform defaults reach state The 1.6 blocks deliberately carry no provider-side defaults, which only works because the aggregated apiserver materialises the schema defaults on every read. That assumption is invisible to unit tests — they never talk to a server — so the acceptance test now pins it: a cluster that pins nothing must still report a Talos release, a health-check tolerance, and an OIDC mode. The negative checks matter as much. A node group's health overrides are undefaulted upstream and must stay absent, and an unset tenant gateway must stay absent so the platform keeps deciding. Either one turning into an empty string or a false would be the encoding quietly collapsing. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * refactor(kubernetes): name the shared block constants by convention The package splits its name constants two ways: attr* when the Terraform attribute and the spec key are the same word, spec* when the camelCase form differs. Three of the new blocks fell on the attr* side but were named spec*, which reads as though the schema were keyed by a spec name. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * fix(kubernetes): expand the 1.6 blocks from the configuration The unset-block contract held only until the first update. Those blocks are Optional+Computed so the platform's effective values can land in state, and Terraform copies prior state into the plan wherever the configuration is silent — so expanding the plan alone sent the platform's own defaults back as explicit spec keys. From that write on, the release carried talos.version and the schematic ID, the chart's defaults no longer applied, and the cluster sat frozen on whatever was current the day something unrelated changed. Nothing in the plan showed it. Models can now take the configuration as the authority for the attributes they nominate, through an optional hook the shared Create/Update path calls before expand. Only cozystack_kubernetes uses it, and only for the five nested blocks; every other attribute still expands from the plan, where its materialised default belongs. The engine refuses to run a model that wants the configuration through a path that cannot supply it, rather than silently expanding the plan. Two smaller cases of the same "emit only what was asked for" rule: an oidc secretRef with no name, and a controlPlane with no apiServer, no longer travel as empty objects. The unit tests pin the round trip that was missing — a plan that only echoes a server response must still expand to no keys — and the acceptance test now runs a second apply, the one that used to do the pinning, and reads the release back to prove the keys stayed out. The JSON-document list helpers move to spec.go, where the other spec helpers live; they are not kubernetes-specific. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * fix(kubernetes): replace only on a configured storage_class change An unconditional RequiresReplace reads the plan, and the plan is not only what the practitioner wrote. Import a cluster that runs on a non-default class with a configuration that never mentions storage_class, and the schema default pulls the plan back to "replicated" — which the modifier then turns into a destroy and recreate of a cluster, from a configuration that says nothing about storage at all. Keying the replacement to a configured value keeps the protection where it belongs: a practitioner asking for a different class still gets the replacement, because the alternative is an apply that reports success while every existing volume stays on the old class. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * docs(kubernetes): regenerate for the configuration-driven blocks Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * fix(kubernetes): track only configured fields in the 1.6 blocks Making the blocks Optional+Computed so the platform's effective values could be read off the resource was the wrong trade. Terraform copies prior state into the plan wherever the configuration is silent, so those values came back as the plan and the next update wrote them into the release as explicit keys — the cluster froze on the Talos release and schematic that were current that day, with nothing in the plan to show it. Reading the request from the configuration instead fixed the freeze but broke the opposite motion: deleting a pinned line left the plan asserting the old value while the apply sent nothing, and Terraform failed the apply with an inconsistent-result error telling the operator to report a provider bug. Both symptoms come from asking one attribute to be two things. The resource now tracks only what the configuration sets: unset fields stay out of the request and out of state, so the platform's default applies and keeps moving with the platform, and a configured field still refreshes, so drift against it is still planned away. The data source is where the effective values — including the ones nobody configured — are read; it keeps reporting the whole surface, because that is what a data source is for. This drops the plan/config divergence entirely, so the hook added to the shared Create/Update path goes away with it and the engine is back to expanding the plan. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * test(spec): cover the JSON-document list helpers The presence-preserving section of spec.go exists to pin the difference between an absent key and an empty value, and every helper in it has a test saying so — except the two JSON-document ones, whose only exercise was the populated path through a kubernetes test. The invalid-document branch is reachable from configuration, not just defensive: `extra_volumes = [null]` decodes to a null value whose string form is empty, and the unmarshal fails. The diagnostic named the spec key but not the element, so a practitioner with six volumes got "unexpected end of JSON input" and nowhere to look; it now names the index. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * fix(kubernetes): stop writing a storage class nobody chose The attribute defaulted to "replicated" in the provider, which meant any cluster running on another class with a configuration that stays silent about storage got its plan pulled back to "replicated". The server accepts that write — it does not evaluate the upstream immutability rule — and the PersistentVolumeClaims stay where they are, so state and reality part ways for good. Dropping the default closes the path: an unset attribute leaves the key out and the platform supplies "replicated" itself, and the plan keeps whatever the cluster already runs on. Two more cases of the same rule, that a key nobody wrote should not be sent. A node group with `roles = []` now reaches the server as an empty list instead of an omitted key; collapsing the two made the read return null against a plan holding an empty list, which fails the apply on a perfectly valid configuration. And the control-plane block now says plainly that configuring it rewrites the whole control-plane section, so the sizing and replica count it does not model go back to platform defaults — those cannot be named from the configuration, so the usual "name it to keep it" escape does not exist there. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * refactor(kubernetes): tighten the block trimming and its comments Three small corrections and one test. The storage_class comment claimed the key stays out for the life of the resource; it stays out on create, and on later applies the plan holds what the last read reported and writes it back — a no-op, but the comment said something else. keepConfiguredAttributes returned a configured value verbatim when the server did not report the block; on a 1.6 cluster that cannot happen, but against an older one it would put a possibly-unknown value into state, so an unreported attribute now trims to null like any other. And oidc.custom_config.secret_ref exists to name a Secret, so `name` is required inside it rather than silently expanding to nothing. The new round trip sends the full model through expand and back through flatten. Expand's keys are guarded against the upstream json tags, but flatten's are hand-written in both the code and the tests, so a matched typo on both sides would have passed everything else here. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * fix(kubernetes): let a cluster declare no node groups `node_groups = {}` is the configuration upstream defaults to and documents: the chart renders a single md0 that provisions nothing until an unschedulable Pod brings the autoscaler in. The provider could not express it. The attribute is required, so the plan holds an empty map, while the shared map flatten collapses an empty map to null — the apply failed with an inconsistent-result error on a configuration the platform supports, and the provider's own example used it. An empty map now reads back as an empty map, while an absent key still reads as null, since that is a server that did not report the field rather than a cluster with no groups. While trimming the platform blocks, the null built for an attribute the server left out now comes from the attribute's own type instead of a switch over the shapes those blocks happen to use today; a bool or a number added to one of them would otherwise have produced a null string and panicked. The acceptance test grows a step for the empty-list contract. Whether an explicitly empty list survives the round trip is a question only a real server can answer — the unit tests echo the provider's own request back. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * docs(tenant): describe what the gateway key actually does The description promised behaviour the chart does not implement: that an unset key auto-enables a Gateway for a tenant whose apex derives from its parent, and disables it for a custom apex. The chart's helper resolves a missing key to false, unconditionally, and never reads `host` — the tenant simply gets no Gateway of its own and inherits its nearest ancestor's, falling back to Ingress if no ancestor owns one. The custom-apex tenant is the one that must ask for `true`, because the ancestor's certificate does not cover its apex. The wrong story came from upstream's own field comment, which the provider had copied verbatim. The encoding is unchanged and still correct: absent and false deploy the same thing today, but the chart branches on the key being missing rather than on a null, and the absence is what records that nobody chose. Two more places where the words claimed more than the code does. The control-plane block said out-of-band values are lost "once this block is in play"; the provider replaces the whole spec on every update, so they are lost either way. And an acceptance step described coverage for an empty node-group map that it did not have — it has it now, as its own step, because the map is exactly the case the unit tests cannot decide. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * docs(tenant): stop claiming an unset gateway equals false Upstream carries three different stories about this key. The field comment in the pinned api module says an unset key auto-enables the Gateway for a derived apex; the chart's own test suite says false is an opt-out that parks the tenant off its ancestor's Gateway while unset inherits; and the templates in that same release resolve both to the identical render, since the helper collapses absent and false before namespace.yaml ever reads it. The last statement is the one with a mechanism behind it, but it is a statement about today's templates, not about the contract — and betting a practitioner's tenant on it is what the previous wording did. The description now says what the provider does and what upstream means by each state, and steers toward leaving the key absent rather than writing false, which is the choice that cannot be wrong under either reading. Three smaller corrections in the same pass. The kubernetes API-server passthrough reserves `authentication-config` too, whenever OIDC is on, and the chart fails the render on it. The tenant data source called the inherited Gateway a per-tenant controller. And the flagship kubernetes example pinned storage_class to the value the schema had just stopped defaulting — pinning it is now the one thing that makes a later change a cluster replacement, which is not what a first example should teach. The empty-list acceptance step also covers oidc.users now, the other list whose empty form the provider claims to preserve. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * docs(tenant): describe gateway from the chart helper, not its prose Upstream says three different things about this key. The generated field comment says an unset key auto-enables the Gateway for a derived apex; a chart test comment says false parks the tenant off its ancestor's Gateway; the helper that actually decides says neither. Two rewrites of this paragraph have now tracked two of those stories, so this one follows the helper and nothing else. tenant.gatewayEffective returns true only for an explicit true, and its own header states what false means: the tenant does not skip Gateway routing, it attaches its published Routes to the nearest ancestor that owns a Gateway, exactly as ingress already inherits. So ownership is what the flag buys, and in this release false and an absent key buy the same nothing. The description now says that, and says which one this provider sends. The rest of the pass: the schema said out-of-band values are dropped after an import, when the provider replaces the whole spec on every update and drops them whichever way they were set; the resource flatten paired its blocks across two parallel slices, where a sixth block added to one and not the other would trim against its neighbour; and the data-source binding — the surface behind the documented talos.version output — had no test. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * docs(kubernetes): claim presence-sensitivity only where the chart has it The oidc.users description said an empty list "is not the same as leaving the attribute unset", and two comments said the same of a node group's roles. On the cluster they are the same: the chart ranges over both lists, and the platform defaults users to empty, so neither can tell an empty list from a missing key. What the empty list actually buys is on the Terraform side — the plan holds an empty list, so the read has to return one. The group's genuinely presence-sensitive keys are maxUnhealthy and nodeStartupTimeout, which the chart reads with hasKey and falls back to the cluster-wide values for. That is where the argument holds, and where it now lives. Also spelled out, next to the snapshot it depends on, that the resource flatten captures the configured blocks before the embedded flatten overwrites them — the ordering is the mechanism, and a refactor that moved one line past the other would invert it silently. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> --------- Signed-off-by: Aleksei Sviridkin <f@lex.la>
…ass replacement (#22) * feat(provider): model the tls block on kafka, nats, qdrant and postgresql The v1.6 API adds a tls block to these four kinds, holding a tri-state enabled flag: while the key is absent the chart follows the external flag, and an explicit true or false overrides it. Modelling it as a plain optional attribute would collapse that third state, so the block writes its spec key only when the flag is set and reads an empty block back as null. Kafka's toggle covers the external listener alone (the internal one is always TLS), and postgresql's only decides whether the external hostname joins the operator-managed server certificate, so both carry their own description. Without the block the spec-coverage guard fails for all four kinds against the pinned API module. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * feat(provider): manage the backup system-bucket opt-in on postgresql and clickhouse Cozystack v1.6 moves backups to a platform-managed bucket: setting useSystemBucket makes the release take bucket coordinates and credentials from the platform instead of per-release S3 settings, which are now deprecated upstream in favour of the default backup class. Only that flag is promoted out of the otherwise unmanaged backup block. An omitted block writes no backup key at all, so the chart defaults stay in force rather than the provider pinning a partial object built from one modelled field. Enabling the flag on an existing release does not start archiving until the first backup job runs, so the schema says to trigger one. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * feat(provider): replace instead of updating a configured storage class A volume's storage class is fixed when the volume is created, and nothing migrates it afterwards. The upstream schema marks the field immutable, but the aggregated apiserver does not evaluate that rule: the write is accepted, the stored spec carries the new class, and the data stays where it was. State and reality then disagree with no signal. Route every storage_class attribute through the shared helper, which now takes the per-kind default and plans a replacement when a configured value changes. The modifier is the configured-only variant: defaults are applied to the planned value whenever the configuration is null, before plan modifiers run, so the unconditional one would destroy a database on import or on deleting the attribute from a configuration. Two guards cover the pair — every storage class plans a replacement on a real change, and none of them does so while unconfigured — so a new kind cannot quietly regress either half. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> --------- Signed-off-by: Aleksei Sviridkin <f@lex.la>
* feat(client): add the KubernetesNodes resource descriptor Cozystack 1.6 splits worker node pools out of the Kubernetes CR into a standalone KubernetesNodes kind, served by the aggregated API under the plural kubernetesnodeses. Declaring the descriptor lets the provider address the kind through the same dynamic-client CRUD every other app kind uses. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * feat(provider): model the KubernetesNodes worker pool spec A pool carries the sizing fields a Kubernetes nodeGroup already carried plus kubelet reservations, a Talos image block, and image overrides. The aggregated API resolves the chart's schema defaults into the spec on every read, so a block the pool never configured comes back populated. Taking those values into state would write them back as explicit spec keys on the next update, freezing the pool on the Talos release and images that were current the day it was created — while the parent cluster, which does not model them at all, keeps following the chart. The talos, kubelet, and images blocks are therefore optional and not computed, and a field the configuration left unset stays out of state. The values that are semantically stable keep an explicit default, matching how the Kubernetes kind models the same fields. The read after an import is the exception and records everything the server reports. It is the only chance to capture what a pool already has, and hiding an existing override there would let the next update, which replaces the spec whole, delete it with nothing in the plan. The chart derives the pool name by stripping the parent cluster from the release name and fails the render when that prefix is missing, which otherwise lands as a HelmRelease that can never install. Name and cluster both force replacement, so a mistyped rename plans as destroy plus create and a check that waited for Create would fire with the pool already torn down; the generic resource therefore grows an optional hook that lets a kind reject a configuration while the plan is built. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * feat(provider): register cozystack_kubernetes_nodes KubernetesNodes was the one upstream app kind the provider did not serve. Registering the resource and its data source alongside the Kubernetes kind makes worker pools manageable on their own, which is the point of the split. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * docs(kubernetes-nodes): add pool examples and generated pages The example pairs a cluster with a standalone GPU pool so the naming rule is visible in the code rather than only in prose: the pool object is <cluster>-<pool>, and the pool part cannot reuse a group the parent cluster still manages. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * docs(kubernetes-nodes): tell importers to reconcile captured overrides An update replaces the whole spec, so an override captured on import and left out of configuration is removed by the first apply. The plan shows the removal, but only the configuration can prevent it. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> * feat(kubernetes-nodes): replace the pool when a configured storage class changes The aggregated apiserver accepts a storage class change without migrating any volume, so an in-place update records a class the disks do not live on. Matches the behaviour every other kind adopted in this release. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> --------- Signed-off-by: Aleksei Sviridkin <f@lex.la>
Cozystack 1.6 deleted the dashboard.cozystack.io API group upstream: the controller is gone and the platform migration runs kubectl delete crd on marketplacepanels.dashboard.cozystack.io during the upgrade. On a 1.6 cluster every cozystack_marketplace_panel call now fails with no-matches-for-kind, so the resource and data source are removed outright rather than deprecated. BREAKING CHANGE: cozystack_marketplace_panel resource and data source are gone. Practitioners must run terraform state rm on any existing cozystack_marketplace_panel entries before upgrading the provider. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
…ough (#23) Cozystack 1.6 adds a new gateway.cozystack.io/v1alpha1 group with a namespaced TenantGateway kind, declaring a tenant's per-namespace Gateway API / Cilium Gateway. It is a controller-facing CRD rather than a packages/apps application, so it follows the same raw-spec passthrough pattern already used for other platform kinds (spec = jsonencode(...)) instead of a fully typed model. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Changelog release sections repeat headings such as "Breaking changes" by design; the default rule flags any repeat anywhere in the file. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin (lexfrei)
force-pushed
the
feat/cozystack-1.6
branch
from
August 13, 2026 00:42
da40d45 to
7515db1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Moves the provider to the Cozystack 1.6 API line: the
api/apps/v1alpha1pin goes from v1.4.3 to v1.6.1 and every surface the new tag added is modelled. Assembled from five reviewed PRs: #25 (kubernetes and tenant surface), #22 (tls, backup opt-in, storage-class replacement), #24 (KubernetesNodes), #21 (marketplace_panel removal), #23 (TenantGateway).Breaking changes: the
cozystack_marketplace_panelresource and data source are gone (upstream deleted thedashboard.cozystack.iogroup; runterraform state rmbefore upgrading), a configuredstorage_classchange now replaces the object on every data-storing kind,cozystack_kuberneteslost itsreplicatedstorage-class default, andversion = "v1.30"fails at plan time. Details in the CHANGELOG.New resources:
cozystack_kubernetes_nodes(standalone worker pools) andcozystack_tenant_gateway(raw-spec passthrough).Changes
talos,oidc,node_health_check,control_plane.api_server,images, node-group health overrides,tenant.gateway,tlson four kinds,backup.use_system_bucketKubernetesNodesandTenantGatewaykinds; removemarketplace_panelstorage_classchanges, with a provider-wide guard testTesting
make test)make lint)make docsproduces no diff)make testacc), if applicable: planned against a 1.6 cluster before the release is announcedDocumentation
examples/make docs)Checklist
type(scope): description)Additional Notes
Each constituent PR carried an approving review; #24's requested changes (DCO sign-off, import docs) were addressed before its merge into this branch.
Summary by CodeRabbit
New Features
Documentation
Breaking Changes