fix(core,gen): UseStateForUnknown для Optional+Computed + live-dосылка без тихого fallback (R1+R3+A/B)

2026-09-22 22:35:45 +03:00
parent 0473f80f69
commit b9fa18164c
10 changed files with 149 additions and 8 deletions
@@ -4,7 +4,7 @@ TOKEN_FILE="secrets/dev.token"

 # Release versions
 # Version
-VERSION="2.0.14"
+VERSION="2.0.15"

 NAMESPACE="nubes-dev"
 PROVIDER_NAME="nubes"
@@ -105,6 +105,24 @@ func ShouldBeOptionalComputed(p types.Param) bool {
 	return !p.Required && p.RefSvcId == 0 && p.Default == ""
 }

+// ShouldUseStateForUnknown решает, нужен ли UseStateForUnknown() plan-modifier
+// для скалярного параметра.
+//
+// Зачем: Optional+Computed параметр БЕЗ Default при незаданном значении в плане
+// = unknown. Если Update не перезаписывает его (в т.ч. ветка no-op с ранним
+// return, см. instance.go), unknown протекает в state и Terraform падает с
+// "Provider produced invalid result object after apply: ... unknown".
+// UseStateForUnknown схлопывает unknown в предыдущее known-значение из state,
+// не требуя сетевого вызова.
+//
+// НЕ применяем к JSON (у них свой JsonNormalize) и к полям с Default/Required.
+func ShouldUseStateForUnknown(p types.Param) bool {
+	if p.IsJson {
+		return false
+	}
+	return ShouldBeOptionalComputed(p)
+}
+
 // ParamFormat возвращает вызов Format* для форматирования значения параметра.
 func ParamFormat(p types.Param, varName string) string {
 	switch strings.ToLower(p.Type) {
@@ -132,6 +132,7 @@ func LoadSpecs(dir string) ([]types.GenResource, []types.GenSubresource, []types
 			HasDomainParam:           hasDomainParam,
 		}
 		params.Analyze(&gr.UsesBool, &gr.UsesInt64, &gr.UsesString, &gr.HasDefaults, &gr.NeedsBoolDefault, &gr.NeedsInt64Default, &gr.NeedsStringDefault, gr.SchemaParams)
+		gr.NeedsBoolUseStateForUnknown, gr.NeedsInt64UseStateForUnknown = analyzeUseStateForUnknown(gr.SchemaParams)
 		// Анализируем nested sub-params для default-импортов
 		if params.AnalyzeNestedDefaults(&gr.NeedsBoolDefault, &gr.NeedsInt64Default, &gr.NeedsStringDefault, gr.SchemaParams) {
 			gr.NeedsFmtImport = true
@@ -452,3 +453,21 @@ func validateModifierOperation(i int, op *lib.OperationSpec) error {
 	}
 	return nil
 }
+
+// analyzeUseStateForUnknown определяет, нужны ли импорты boolplanmodifier/int64planmodifier:
+// есть ли среди SchemaParams скалярный Optional+Computed (без Default) параметр
+// соответствующего типа, для которого генерируется UseStateForUnknown().
+func analyzeUseStateForUnknown(schemaParams []types.Param) (needsBool, needsInt64 bool) {
+	for _, p := range schemaParams {
+		if p.IsJson || p.Required || p.RefSvcId != 0 || p.Default != "" {
+			continue
+		}
+		switch strings.ToLower(p.Type) {
+		case "bool":
+			needsBool = true
+		case "int", "int64", "number":
+			needsInt64 = true
+		}
+	}
+	return needsBool, needsInt64
+}
@@ -25,6 +25,12 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
 	{{- end }}
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
+	{{- if .NeedsBoolUseStateForUnknown }}
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier"
+	{{- end }}
+	{{- if .NeedsInt64UseStateForUnknown }}
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier"
+	{{- end }}
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
 	"github.com/hashicorp/terraform-plugin-framework/types"
 )
@@ -109,7 +115,7 @@ func (r *{{ToCamel .Name}}Resource) Schema(ctx context.Context, req resource.Sch
 			{{- if ne (ParamDefaultExpr .) "" }}Computed: true, Default: {{ParamDefaultExpr .}},{{- else if or .IsJson (ShouldBeOptionalComputed .) }}Computed: true,{{- end }}
 			{{- if ne (ParamDescription .) "" }}MarkdownDescription: {{ParamDescription .}},{{end}}
 			{{- if .Sensitive }}Sensitive: true,{{end}}
-			{{- if .IsJson }}PlanModifiers: []planmodifier.String{resources_core.JsonNormalize()},{{- end }}
+			{{- if .IsJson }}PlanModifiers: []planmodifier.String{resources_core.JsonNormalize()},{{- else if ShouldUseStateForUnknown . }}PlanModifiers: []planmodifier.{{if eq (ParamType .) "types.Bool"}}Bool{boolplanmodifier.UseStateForUnknown()}{{else if eq (ParamType .) "types.Int64"}}Int64{int64planmodifier.UseStateForUnknown()}{{else}}String{stringplanmodifier.UseStateForUnknown()}{{end}},{{- end }}
 		},
 		{{- end }}
 		{{- end }}
@@ -83,6 +83,8 @@ type GenResource struct {
 	NeedsBoolDefault         bool
 	NeedsInt64Default        bool
 	NeedsStringDefault       bool
+	NeedsBoolUseStateForUnknown   bool
+	NeedsInt64UseStateForUnknown  bool
 	HasDomainParam           bool
 	DomainServiceIDs         []int
 	// HasRedeploy: сервис поддерживает redeploy (пересборка из git).
@@ -35,6 +35,7 @@ func WriteInstanceResource(outDir string, svc types.GenResource) error {
 		// читает обратно из state_params, обязан быть Optional+Computed, если он
 		// не Required и без Default. См. подробное объяснение в helpers.go.
 		"ShouldBeOptionalComputed": helpers.ShouldBeOptionalComputed,
+		"ShouldUseStateForUnknown": helpers.ShouldUseStateForUnknown,
 		"ParamFormat":              helpers.ParamFormat,
 		"ParamParse":               helpers.ParamParse,
 		"ParamDescription":         helpers.ParamDescription,
@@ -0,0 +1,79 @@
+# Ревью: модификаторы vc_nsxt / vc_org + досылка modify-params (Terraform Provider Nubes)
+
+Ты — ревьюер. Ничего не правь. Прочитай и выдай:
+1) подтверждение/опровержение каждого утверждения ниже;
+2) список багов/рисков, которые я НЕ заметил;
+3) список проблем, которые я заметил ошибочно (ложные тревоги);
+4) чёткую рекомендацию по каждому корректному фиксу (минимальную, без scope creep).
+
+## Контекст системы
+
+Go Terraform provider `terraform-provider-nubes` (plugin-framework), сервис Nubes Cloud.
+Генератор ресурсов: `TOOLS/resource-generator` (шаблон `instance.go`, `modifier.go`).
+Ядро: `provider/internal/core/` (HTTP + операции), `provider/internal/resources_core/` (обёртки).
+
+Сервисы, о которых речь:
+- `vc_nsxt` (serviceId 22). Операции: create (10), delete (25), **modify (111, kind: modifier, modifier: network)**, reconcile. У resource НЕТ instance-modify.
+- `vc_org` (serviceId 19). Модификатор `ip_space` (modify 207).
+
+Модификатор = отдельный TF resource (`nubes_vc_nsxt_network`, `nubes_vc_org_ip_space`), который вызывает операцию `modify` с параметрами.
+
+## Цель (FullPipe) — что должно работать
+
+1. Создать VDC (vc_vdc).
+2. Создать Edge (vc_nsxt) с ALB (`needEnableAVI=true`, `virtualServicesCount=3`).
+3. Выделить IP организации (vc_org → modifier ip_space, `vIPConfigure`).
+4. Применить SNAT для Edge (vc_nsxt → modifier network, `ipSpaceName` + `routedNetConfiguration`).
+
+## Что УЖЕ сделано (факты, проверь корректность)
+
+### Факт 1. `resource "nubes_vc_nsxt"` Update — no-op
+Шаблон `instance.go` генерирует `hasServiceParamChanges := false`, а цикл по `.ModifyParams` пуст (у vc_nsxt нет instance-modify). Поэтому `Update` всегда уходит в `if !hasServiceParamChanges { ...; return }` и НЕ вызывает `modify` (111). Изменение ALB/VS/qos через resource невозможно. Изменения Edge идут ТОЛЬКО через модификатор `nubes_vc_nsxt_network`.
+
+### Факт 2. Досылка незаданных modify-params (мой свежий фикс, коммиты a011358)
+Раньше незаданные params операции `modify` досылались значением `paramValue` из `GET /instanceOperations/{opUid}?fields=cfsParams`. Это ОШИБОЧНО: `paramValue` — дефолт ФОРМЫ операции, а не состояние инстанса. Для `needEnableAVI` там `"false"`, хотя live-значение инстанса `true` (подтверждается HAR/edge_.har и HAR/ipSpace0.har). Из-за этого каждый `modify` через модификатор сбрасывал ALB в false.
+
+Фикс: в `operation_cfs.go` добавлены `instanceLiveParams()` (читает live из `GET /instances/{uid}` → `state.params`) и `lookupLiveParam(live, cfsParam)`. В `runInstanceOperationByCode` и `RunInstanceOperationUniversalWithDefaults` приоритет теперь: **live state.params → paramValue → defaultValue**.
+
+### Факт 3. `ShouldRemoveFromState` (коммит 94c4c44)
+Раньше вызывал валидирующий `GetInstanceState`, который на статусе `deleted` кидал `instanceDeletedError` — и `Read` модификатора падал с "экземпляр … удалён" вместо тихого удаления из state. Переписан на `GetInstanceStateRaw` + различение 404/deleted (remove=true) vs сеть/5xx/403 (нужно `false, err`).
+
+## ОШИБКА, которую наблюдаю СЕЙЧАС (главное)
+
+`terraform apply` падает:
+
+```
+Error: Provider returned invalid result object after apply
+After the apply operation, the provider still indicated an unknown value for
+nubes_vc_nsxt.edge.qos_profile. All values must be known after apply...
+```
+
+`qos_profile` у resource `nubes_vc_nsxt` = `Optional+Computed` БЕЗ Default (`ShouldBeOptionalComputed` → true, потому что param qosProfile: not required, RefSvcId=0, Default=""). В конфиге не задаётся → в плане unknown. А `Update` (`hasServiceParamChanges=false` → ранний return) копирует только `State*`/`Vault*` outputs, но НЕ вызывает `RefreshResourceState`, поэтому `qos_profile` остаётся unknown.
+
+## МОИ ДИАГНОЗЫ (проверь каждый, а не только текущий)
+
+### Диагноз A (текущая ошибка)
+Ранний return в `Update` (шаблон instance.go) не схлопывает unknown→null read-back-computed поля. Нужно в ветке `!hasServiceParamChanges` вызывать тот же `RefreshResourceState`, а не копировать `State*`/`Vault*` вручную.
+
+### Диагноз B (вылезет после A)
+Тот же ранний return оставляет unknown для `need_enable_avi` и `virtual_services_count`, если их убрать из `edge.tf` (а их и должны убрать, раз ALB перенесён в модификатор). Один корень с A.
+
+### Диагноз C (дублирование конфига — НЕ починен)
+`edge.tf` ДО СИХ ПОР задаёт `need_enable_avi` и `virtual_services_count` (create), а `edge_network.tf` — те же значения (modifier). Это двойное задание одного и того же → возможен дрейф. Нужно определить: где канонически задавать ALB?
+
+### Диагноз D (ловушка destroy/delete)
+Модификатор имеет `delete_strategy: inverse`, override `needEnableAVI="false"`. При `terraform destroy` ALB выключится. Повторный `apply` через `resource "nubes_vc_nsxt"` (no-op Update) НЕ включит обратно, а включит только модификатор второй apply-волной. Нужно проверить порядок зависимостей.
+
+### Диагноз E
+`FetchInstanceOutputs` глотает любую API-ошибку (5xx/404) и возвращает пустые outputs без diagnostic → молчаливый дрейф. Нужен warning.
+
+## Конкретные вопросы
+
+1. Подтверди/опровергни Диагноз A как корень текущей ошибки.
+2. Есть ли проблема в моём фиксе досылки (Факт 2)? В частности:
+   - верно ли, что `state.params` — единственный достоверный источник live?
+   - не сломает ли `lookupLiveParam` (по Code/SvcOperationCfsParam/Name/Label) какие-то кейсы, где имя в state.params отличается регистром/форматом от этих ключей?
+   - не создаёт ли `instanceLiveParams` лишний сетевой вызов на каждый modify (перф)?
+3. Верна ли трактовка Факт 1 (resource Update — no-op)? Или правильнее ДОБАВИТЬ instance-modify в генератор?
+4. Какое каноническое место для `need_enable_avi`/`virtual_services_count`/`qos_profile`: create (edge.tf) или modifier (edge_network.tf)? Что делать с текущим дублированием?
+5. Что ещё я упустил в цепочке create→modify→read→destroy?
@@ -14,11 +14,15 @@ import (
 // дефолт ФОРМЫ операции, а НЕ состояние инстанса. Пример из HAR/edge_.har:
 // для needEnableAVI в cfsParams paramValue="false", тогда как live-значение инстанса
 // needEnableAVI=true. Досылка paramValue как live стирала ALB (reset-to-default).
-func (c *UniversalClient) instanceLiveParams(ctx context.Context, instanceUid string) map[string]string {
+//
+// При ошибке чтения live возвращает err: тихий возврат пустой map опасен — тогда
+// fallback уйдёт на paramValue (дефолт формы) и reset-баг вернётся без признаков
+// (см. R3 в ревью). Вызывающий код обязан не молчать.
+func (c *UniversalClient) instanceLiveParams(ctx context.Context, instanceUid string) (map[string]string, error) {
 	live := map[string]string{}
 	params, err := c.GetInstanceStateParams(ctx, instanceUid)
 	if err != nil {
-		return live
+		return live, err
 	}
 	for k, v := range params {
 		key := strings.ToLower(strings.TrimSpace(k))
@@ -27,7 +31,7 @@ func (c *UniversalClient) instanceLiveParams(ctx context.Context, instanceUid st
 		}
 		live[key] = v
 	}
-	return live
+	return live, nil
 }

 // lookupLiveParam ищет live-значение параметра инстанса по возможным ключам cfsParam.
@@ -135,7 +135,10 @@ func (c *UniversalClient) RunInstanceOperationUniversalWithDefaults(ctx context.
 		sent[paramId] = true
 	}

-	live := c.instanceLiveParams(ctx, instanceUid)
+	live, liveErr := c.instanceLiveParams(ctx, instanceUid)
+	if liveErr != nil {
+		return fmt.Errorf("не удалось прочитать live-значения инстанса для досылки modify: %w", liveErr)
+	}
 	for _, param := range cfsParams {
 		if sent[param.SvcOperationCfsParamId] {
 			continue
@@ -143,9 +146,15 @@ func (c *UniversalClient) RunInstanceOperationUniversalWithDefaults(ctx context.

 		// Приоритет: live state.params инстанса → paramValue операции → defaultValue.
 		// paramValue из cfsParams — дефолт формы, не состояние инстанса (см. operation_cfs.go).
+		// Симметрично runInstanceOperationByCode: если ни одного источника нет — пропускаем
+		// (иначе уйдёт синтетический "0"/"false"/"[]" и нарушит constraint).
 		val, hasLive := lookupLiveParam(live, param)
 		if !hasLive {
-			if param.ParamValue != nil {
+			if (param.ParamValue == nil || strings.TrimSpace(*param.ParamValue) == "") &&
+				(param.DefaultValue == nil || strings.TrimSpace(*param.DefaultValue) == "") {
+				continue
+			}
+			if param.ParamValue != nil && strings.TrimSpace(*param.ParamValue) != "" {
 				val = *param.ParamValue
 			} else if param.DefaultValue != nil {
 				val = *param.DefaultValue
@@ -105,7 +105,10 @@ func (c *UniversalClient) runInstanceOperationByCode(ctx context.Context, instan
 		sent[paramId] = true
 	}

-	live := c.instanceLiveParams(ctx, instanceUid)
+	live, liveErr := c.instanceLiveParams(ctx, instanceUid)
+	if liveErr != nil {
+		return fmt.Errorf("не удалось прочитать live-значения инстанса для досылки modify: %w", liveErr)
+	}
 	for _, param := range cfsParams {
 		if sent[param.SvcOperationCfsParamId] {
 			continue