You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "JingsongLi (via GitHub)" <gi...@apache.org> on 2026/09/05 15:54:05 UTC
[PR] Align Paimon API contracts and harden provider recovery [terraform-provider-paimon]
JingsongLi opened a new pull request, #17:
URL: https://github.com/apache/terraform-provider-paimon/pull/17
Imported primary-key tables could fail option-only updates when omitted keys or nullability were treated as empty/default values. Java-normalized policy JSON could also be mistaken for a failed mutation or a content change requiring drop-and-create. This change preserves inherited schema state, configures primary keys through Paimon's Java option, and reconciles equivalent policies without detaching them.
## API and lifecycle changes
- Configure primary keys with `options["primary-key"]`; expose normalized `primary_keys` as a computed output. Preserve equivalent option spelling, permit same-value adoption after import, and require explicit `allow_replacement` for immutable/key changes.
- Preserve omitted field nullability and keys across updates, including deferred field/key configuration. Rename `catalog_id` to `server_id` and `allow_destructive_changes` to `allow_replacement`, with version-0 resource state migration.
- Compare known Java policy AST normalization across create, read, plan and update, including name-based field references, single-child predicates and atomic cast aliases. Keep non-atomic content replacement disabled by default, and avoid reading uncreated destinations during identity replacement.
- Treat field defaults as constant strings, reject mixed bearer/DLF configuration, and retain management of options whose removal has not been confirmed.
- Bound requests and mutation reconciliation; require and refresh ECS IMDS session tokens; preserve HTTP status against body overrides and suppress credential echoes in diagnostics. Definitively rejected policy creates do not adopt existing policies.
## Compatibility and validation
The HCL changes above require configuration updates; resource state metadata is upgraded automatically. `docs/api-compatibility.md` documents the migration and exact Paimon/Iceberg source references. Permission and policy resources remain experimental.
Passed locally:
- `make check` (vet, race tests, build, formatting and six example validations)
- `make test-acceptance` (Terraform 1.13.5 fixture lifecycles, imports, replacement guards, deferred schemas and policy normalization)
- `golangci-lint run`, `make check-license`, documentation link checks and `git diff --check`
- Two rounds of independent focused review and regression verification
This also adds OpenTofu protocol CI, an opt-in real Catalog workflow, and a public Registry installation verifier. Real REST/DLF integration was not run without an isolated endpoint; signed Registry installation was not run without a published release. Local OpenTofu execution was terminated by macOS, so OpenTofu compatibility remains unverified locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Align Paimon API contracts and harden provider recovery [terraform-provider-paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #17:
URL: https://github.com/apache/terraform-provider-paimon/pull/17#discussion_r3943504008
##########
internal/provider/resource_table.go:
##########
@@ -85,44 +87,97 @@ func (r *tableResource) Configure(_ context.Context, req resource.ConfigureReque
}
func (r *tableResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
- if req.State.Raw.IsNull() || req.Plan.Raw.IsNull() {
+ if req.Plan.Raw.IsNull() {
return
}
var config, state, plan tableResourceModel
resp.Diagnostics.Append(req.Config.Get(ctx, &config)...)
- resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
+ if !req.State.Raw.IsNull() {
+ resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
+ }
resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)
if resp.Diagnostics.HasError() {
return
}
- var configuredFields, stateFields, plannedFields []tableFieldModel
- resp.Diagnostics.Append(config.Fields.ElementsAs(ctx, &configuredFields, false)...)
- resp.Diagnostics.Append(state.Fields.ElementsAs(ctx, &stateFields, false)...)
- resp.Diagnostics.Append(plan.Fields.ElementsAs(ctx, &plannedFields, false)...)
- if resp.Diagnostics.HasError() {
+ stabilizeTableKeys(ctx, config, state, &plan, &resp.Diagnostics)
+ replacementPaths := make([]path.Path, 0)
+ if tableFieldsInspectable(config.Fields) && tableFieldsInspectable(plan.Fields) {
+ var configuredFields, stateFields, plannedFields []tableFieldModel
+ resp.Diagnostics.Append(config.Fields.ElementsAs(ctx, &configuredFields, false)...)
+ if !req.State.Raw.IsNull() {
+ resp.Diagnostics.Append(state.Fields.ElementsAs(ctx, &stateFields, false)...)
+ }
+ resp.Diagnostics.Append(plan.Fields.ElementsAs(ctx, &plannedFields, false)...)
+ if resp.Diagnostics.HasError() {
+ return
+ }
+ if len(configuredFields) != len(plannedFields) {
+ resp.Diagnostics.AddError("Unable to stabilize Paimon field identities", "The configured and planned field lists have different lengths. Please report this issue to the provider developers.")
+
+ return
+ }
+
+ stabilizePlannedFieldIdentities(configuredFields, stateFields, plannedFields)
+ if !req.State.Raw.IsNull() {
+ validateAddedFieldIDs(stateFields, configuredFields, &resp.Diagnostics)
+ }
+ stabilizeFieldNullability(ctx, configuredFields, stateFields, plannedFields, plan, state, &resp.Diagnostics)
+ plan.Fields = fieldsValueFromModels(ctx, plannedFields, &resp.Diagnostics)
+ if resp.Diagnostics.HasError() {
+ return
+ }
+ if !req.State.Raw.IsNull() {
+ keyFields := knownTableKeyNames(state.PartitionKeys, state.PrimaryKeys, plan.PartitionKeys, plan.PrimaryKeys)
+ if compositeFieldTypesRequireReplace(stateFields, plannedFields) ||
+ keyFieldTypesRequireReplace(stateFields, plannedFields, keyFields) ||
+ newNonNullableFieldsRequireReplace(stateFields, plannedFields) {
+ replacementPaths = append(replacementPaths, path.Root("fields"))
+ }
+ }
+
+ }
+ resp.Diagnostics.Append(resp.Plan.Set(ctx, &plan)...)
+ if req.State.Raw.IsNull() || resp.Diagnostics.HasError() {
return
}
- if len(configuredFields) != len(plannedFields) {
- resp.Diagnostics.AddError("Unable to stabilize Paimon field identities", "The configured and planned field lists have different lengths. Please report this issue to the provider developers.")
+ if !plan.PrimaryKeys.IsUnknown() && !state.PrimaryKeys.Equal(plan.PrimaryKeys) {
+ replacementPaths = append(replacementPaths, path.Root("options").AtMapKey("primary-key"))
+ }
+ if !plan.PartitionKeys.IsUnknown() && !state.PartitionKeys.Equal(plan.PartitionKeys) {
Review Comment:
**[P1] Enforce replacement opt-in for wholly unknown partition keys**
When the entire `partition_keys` value is unknown, this condition skips the opt-in check, but the attribute's `listplanmodifier.RequiresReplaceIfConfigured()` has already requested replacement. Terraform can consequently destroy/recreate the table with `allow_replacement = false`; replanning the replacement as a create does not rescue the check because that path has null prior state.
For example, apply this configuration against an existing `analytics` database, then change only `terraform_data.partition.input` from `["tenant"]` to `["region"]`:
```hcl
resource "terraform_data" "partition" {
input = ["tenant"]
}
resource "paimon_table" "events" {
database = "analytics"
name = "events"
fields = [
{ name = "tenant", type = "STRING" },
{ name = "region", type = "STRING" }
]
partition_keys = terraform_data.partition.output
allow_replacement = false
}
```
I reproduced this at `df3f709` using `acceptanceCatalog` and Terraform 1.13.5. The second apply succeeds instead of returning `Destructive table change is disabled`; the fixture records **two table creates**, confirming that the existing table was replaced without opt-in. The newly added test only covers a partially unknown list, for which `IsUnknown()` is false, so it misses this case.
Please make the opt-in cover attribute-level replacement decisions as well, including wholly unknown values, and add a protocol acceptance test that verifies the second apply is rejected without replacing the original table. The same interaction should also be checked for the `name`/`database` replacement modifiers.
##########
internal/provider/resource_table.go:
##########
@@ -85,44 +87,97 @@ func (r *tableResource) Configure(_ context.Context, req resource.ConfigureReque
}
func (r *tableResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
- if req.State.Raw.IsNull() || req.Plan.Raw.IsNull() {
+ if req.Plan.Raw.IsNull() {
return
}
var config, state, plan tableResourceModel
resp.Diagnostics.Append(req.Config.Get(ctx, &config)...)
- resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
+ if !req.State.Raw.IsNull() {
+ resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
+ }
resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)
if resp.Diagnostics.HasError() {
return
}
- var configuredFields, stateFields, plannedFields []tableFieldModel
- resp.Diagnostics.Append(config.Fields.ElementsAs(ctx, &configuredFields, false)...)
- resp.Diagnostics.Append(state.Fields.ElementsAs(ctx, &stateFields, false)...)
- resp.Diagnostics.Append(plan.Fields.ElementsAs(ctx, &plannedFields, false)...)
- if resp.Diagnostics.HasError() {
+ stabilizeTableKeys(ctx, config, state, &plan, &resp.Diagnostics)
+ replacementPaths := make([]path.Path, 0)
+ if tableFieldsInspectable(config.Fields) && tableFieldsInspectable(plan.Fields) {
+ var configuredFields, stateFields, plannedFields []tableFieldModel
+ resp.Diagnostics.Append(config.Fields.ElementsAs(ctx, &configuredFields, false)...)
+ if !req.State.Raw.IsNull() {
+ resp.Diagnostics.Append(state.Fields.ElementsAs(ctx, &stateFields, false)...)
+ }
+ resp.Diagnostics.Append(plan.Fields.ElementsAs(ctx, &plannedFields, false)...)
+ if resp.Diagnostics.HasError() {
+ return
+ }
+ if len(configuredFields) != len(plannedFields) {
+ resp.Diagnostics.AddError("Unable to stabilize Paimon field identities", "The configured and planned field lists have different lengths. Please report this issue to the provider developers.")
+
+ return
+ }
+
+ stabilizePlannedFieldIdentities(configuredFields, stateFields, plannedFields)
+ if !req.State.Raw.IsNull() {
+ validateAddedFieldIDs(stateFields, configuredFields, &resp.Diagnostics)
+ }
+ stabilizeFieldNullability(ctx, configuredFields, stateFields, plannedFields, plan, state, &resp.Diagnostics)
+ keyFields := append(stringListFromValue(ctx, state.PartitionKeys, &resp.Diagnostics), stringListFromValue(ctx, state.PrimaryKeys, &resp.Diagnostics)...)
+ keyFields = append(keyFields, stringListFromValue(ctx, plan.PartitionKeys, &resp.Diagnostics)...)
Review Comment:
Verified fixed at df3f709: my original protocol acceptance reproducer with known fields and `partition_keys = [terraform_data.partition.output]` now passes. The new null-element validation and mixed known/unknown replacement coverage also pass. I found a separate whole-list-unknown replacement opt-in bypass during follow-up testing and reported it in the new review; this original creation-conversion finding itself is addressed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Align Paimon API contracts and harden provider recovery [terraform-provider-paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #17:
URL: https://github.com/apache/terraform-provider-paimon/pull/17
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Align Paimon API contracts and harden provider recovery [terraform-provider-paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #17:
URL: https://github.com/apache/terraform-provider-paimon/pull/17#discussion_r3943438228
##########
internal/provider/resource_table.go:
##########
@@ -85,44 +87,97 @@ func (r *tableResource) Configure(_ context.Context, req resource.ConfigureReque
}
func (r *tableResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
- if req.State.Raw.IsNull() || req.Plan.Raw.IsNull() {
+ if req.Plan.Raw.IsNull() {
return
}
var config, state, plan tableResourceModel
resp.Diagnostics.Append(req.Config.Get(ctx, &config)...)
- resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
+ if !req.State.Raw.IsNull() {
+ resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
+ }
resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)
if resp.Diagnostics.HasError() {
return
}
- var configuredFields, stateFields, plannedFields []tableFieldModel
- resp.Diagnostics.Append(config.Fields.ElementsAs(ctx, &configuredFields, false)...)
- resp.Diagnostics.Append(state.Fields.ElementsAs(ctx, &stateFields, false)...)
- resp.Diagnostics.Append(plan.Fields.ElementsAs(ctx, &plannedFields, false)...)
- if resp.Diagnostics.HasError() {
+ stabilizeTableKeys(ctx, config, state, &plan, &resp.Diagnostics)
+ replacementPaths := make([]path.Path, 0)
+ if tableFieldsInspectable(config.Fields) && tableFieldsInspectable(plan.Fields) {
+ var configuredFields, stateFields, plannedFields []tableFieldModel
+ resp.Diagnostics.Append(config.Fields.ElementsAs(ctx, &configuredFields, false)...)
+ if !req.State.Raw.IsNull() {
+ resp.Diagnostics.Append(state.Fields.ElementsAs(ctx, &stateFields, false)...)
+ }
+ resp.Diagnostics.Append(plan.Fields.ElementsAs(ctx, &plannedFields, false)...)
+ if resp.Diagnostics.HasError() {
+ return
+ }
+ if len(configuredFields) != len(plannedFields) {
+ resp.Diagnostics.AddError("Unable to stabilize Paimon field identities", "The configured and planned field lists have different lengths. Please report this issue to the provider developers.")
+
+ return
+ }
+
+ stabilizePlannedFieldIdentities(configuredFields, stateFields, plannedFields)
+ if !req.State.Raw.IsNull() {
+ validateAddedFieldIDs(stateFields, configuredFields, &resp.Diagnostics)
+ }
+ stabilizeFieldNullability(ctx, configuredFields, stateFields, plannedFields, plan, state, &resp.Diagnostics)
+ keyFields := append(stringListFromValue(ctx, state.PartitionKeys, &resp.Diagnostics), stringListFromValue(ctx, state.PrimaryKeys, &resp.Diagnostics)...)
+ keyFields = append(keyFields, stringListFromValue(ctx, plan.PartitionKeys, &resp.Diagnostics)...)
Review Comment:
**[P2] Defer conversion of partially unknown partition-key lists**
`ModifyPlan` now runs for creates too, but this call converts `plan.PartitionKeys` to `[]string` even when the list contains an unknown element. For example, this valid configuration fails its initial plan:
```hcl
resource "terraform_data" "partition" {
input = "tenant"
}
resource "paimon_table" "events" {
database = "analytics"
name = "events"
fields = [{ name = "tenant", type = "STRING" }]
partition_keys = [terraform_data.partition.output]
}
```
With an existing `analytics` database and the usual provider configuration, Terraform reports `Value Conversion Error: Received unknown value, however the target type cannot handle unknown values` (`Path: [0]`, `Target Type: string`), before it can apply the dependency. I reproduced this using the repository's `acceptanceCatalog` and Terraform 1.13.5: the same acceptance test passes at base `0646712` and fails at head `8cf932f`.
Please defer key-dependent checks until the individual key elements are known, and avoid building replacement-only `keyFields` during creation. The new deferred-schema test does not cover this case because its entire `fields` value is unknown, so it skips this block. Add an acceptance case with known fields and a partially unknown `partition_keys` list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Align Paimon API contracts and harden provider recovery [terraform-provider-paimon]
Posted by "leaves12138 (via GitHub)" <gi...@apache.org>.
leaves12138 commented on code in PR #17:
URL: https://github.com/apache/terraform-provider-paimon/pull/17#discussion_r3943852916
##########
internal/provider/resource_table.go:
##########
@@ -85,44 +87,97 @@ func (r *tableResource) Configure(_ context.Context, req resource.ConfigureReque
}
func (r *tableResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
- if req.State.Raw.IsNull() || req.Plan.Raw.IsNull() {
+ if req.Plan.Raw.IsNull() {
return
}
var config, state, plan tableResourceModel
resp.Diagnostics.Append(req.Config.Get(ctx, &config)...)
- resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
+ if !req.State.Raw.IsNull() {
+ resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
+ }
resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)
if resp.Diagnostics.HasError() {
return
}
- var configuredFields, stateFields, plannedFields []tableFieldModel
- resp.Diagnostics.Append(config.Fields.ElementsAs(ctx, &configuredFields, false)...)
- resp.Diagnostics.Append(state.Fields.ElementsAs(ctx, &stateFields, false)...)
- resp.Diagnostics.Append(plan.Fields.ElementsAs(ctx, &plannedFields, false)...)
- if resp.Diagnostics.HasError() {
+ stabilizeTableKeys(ctx, config, state, &plan, &resp.Diagnostics)
+ replacementPaths := make([]path.Path, 0)
+ if tableFieldsInspectable(config.Fields) && tableFieldsInspectable(plan.Fields) {
+ var configuredFields, stateFields, plannedFields []tableFieldModel
+ resp.Diagnostics.Append(config.Fields.ElementsAs(ctx, &configuredFields, false)...)
+ if !req.State.Raw.IsNull() {
+ resp.Diagnostics.Append(state.Fields.ElementsAs(ctx, &stateFields, false)...)
+ }
+ resp.Diagnostics.Append(plan.Fields.ElementsAs(ctx, &plannedFields, false)...)
+ if resp.Diagnostics.HasError() {
+ return
+ }
+ if len(configuredFields) != len(plannedFields) {
+ resp.Diagnostics.AddError("Unable to stabilize Paimon field identities", "The configured and planned field lists have different lengths. Please report this issue to the provider developers.")
+
+ return
+ }
+
+ stabilizePlannedFieldIdentities(configuredFields, stateFields, plannedFields)
+ if !req.State.Raw.IsNull() {
+ validateAddedFieldIDs(stateFields, configuredFields, &resp.Diagnostics)
+ }
+ stabilizeFieldNullability(ctx, configuredFields, stateFields, plannedFields, plan, state, &resp.Diagnostics)
+ plan.Fields = fieldsValueFromModels(ctx, plannedFields, &resp.Diagnostics)
+ if resp.Diagnostics.HasError() {
+ return
+ }
+ if !req.State.Raw.IsNull() {
+ keyFields := knownTableKeyNames(state.PartitionKeys, state.PrimaryKeys, plan.PartitionKeys, plan.PrimaryKeys)
+ if compositeFieldTypesRequireReplace(stateFields, plannedFields) ||
+ keyFieldTypesRequireReplace(stateFields, plannedFields, keyFields) ||
+ newNonNullableFieldsRequireReplace(stateFields, plannedFields) {
+ replacementPaths = append(replacementPaths, path.Root("fields"))
+ }
+ }
+
+ }
+ resp.Diagnostics.Append(resp.Plan.Set(ctx, &plan)...)
+ if req.State.Raw.IsNull() || resp.Diagnostics.HasError() {
return
}
- if len(configuredFields) != len(plannedFields) {
- resp.Diagnostics.AddError("Unable to stabilize Paimon field identities", "The configured and planned field lists have different lengths. Please report this issue to the provider developers.")
+ if !plan.PrimaryKeys.IsUnknown() && !state.PrimaryKeys.Equal(plan.PrimaryKeys) {
+ replacementPaths = append(replacementPaths, path.Root("options").AtMapKey("primary-key"))
+ }
+ if !plan.PartitionKeys.IsUnknown() && !state.PartitionKeys.Equal(plan.PartitionKeys) {
Review Comment:
Verified fixed at 0ceec99. My original whole-list-unknown acceptance reproducer now receives the expected destructive-change diagnostic with allow_replacement = false, and the fixture records only the original table creation. The new name/database/partition-key guard tests also pass. I additionally exercised a real fixture apply with allow_replacement explicitly changed to true: the intended replacement succeeds and the following plan is clean. Both previous findings are addressed; I have approved the updated head.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org