From b9fa18164c7ad7cd793aaef50dd093e62d265bcc Mon Sep 17 00:00:00 2001 From: Repinoid Date: Tue, 22 Sep 2026 22:35:45 +0300 Subject: [PATCH] =?UTF-8?q?fix(core,gen):=20UseStateForUnknown=20=D0=B4?= =?UTF-8?q?=D0=BB=D1=8F=20Optional+Computed=20+=20live-d=D0=BE=D1=81=D1=8B?= =?UTF-8?q?=D0=BB=D0=BA=D0=B0=20=D0=B1=D0=B5=D0=B7=20=D1=82=D0=B8=D1=85?= =?UTF-8?q?=D0=BE=D0=B3=D0=BE=20fallback=20(R1+R3+A/B)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- TOOLS/config/dev/profile.env | 2 +- .../internal/helpers/helpers.go | 18 +++++ .../internal/loader/loader.go | 19 +++++ .../internal/templates/instance.go | 8 +- .../internal/types/types.go | 2 + .../internal/writers/writers.go | 1 + prompt_for_opus_modifier_review_2.md | 79 +++++++++++++++++++ provider/internal/core/operation_cfs.go | 10 ++- provider/internal/core/operation_run.go | 13 ++- .../internal/core/operation_run_bycode.go | 5 +- 10 files changed, 149 insertions(+), 8 deletions(-) create mode 100644 prompt_for_opus_modifier_review_2.md diff --git a/TOOLS/config/dev/profile.env b/TOOLS/config/dev/profile.env index 6b1d377..73c44d0 100644 --- a/TOOLS/config/dev/profile.env +++ b/TOOLS/config/dev/profile.env @@ -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" diff --git a/TOOLS/resource-generator/internal/helpers/helpers.go b/TOOLS/resource-generator/internal/helpers/helpers.go index d75a513..dbe257e 100644 --- a/TOOLS/resource-generator/internal/helpers/helpers.go +++ b/TOOLS/resource-generator/internal/helpers/helpers.go @@ -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) { diff --git a/TOOLS/resource-generator/internal/loader/loader.go b/TOOLS/resource-generator/internal/loader/loader.go index 33f66c9..38082e8 100644 --- a/TOOLS/resource-generator/internal/loader/loader.go +++ b/TOOLS/resource-generator/internal/loader/loader.go @@ -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 +} diff --git a/TOOLS/resource-generator/internal/templates/instance.go b/TOOLS/resource-generator/internal/templates/instance.go index 5652da2..59a28e1 100644 --- a/TOOLS/resource-generator/internal/templates/instance.go +++ b/TOOLS/resource-generator/internal/templates/instance.go @@ -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 }} diff --git a/TOOLS/resource-generator/internal/types/types.go b/TOOLS/resource-generator/internal/types/types.go index dd6d8d0..f4ed71f 100644 --- a/TOOLS/resource-generator/internal/types/types.go +++ b/TOOLS/resource-generator/internal/types/types.go @@ -83,6 +83,8 @@ type GenResource struct { NeedsBoolDefault bool NeedsInt64Default bool NeedsStringDefault bool + NeedsBoolUseStateForUnknown bool + NeedsInt64UseStateForUnknown bool HasDomainParam bool DomainServiceIDs []int // HasRedeploy: сервис поддерживает redeploy (пересборка из git). diff --git a/TOOLS/resource-generator/internal/writers/writers.go b/TOOLS/resource-generator/internal/writers/writers.go index 68261ae..fff208c 100644 --- a/TOOLS/resource-generator/internal/writers/writers.go +++ b/TOOLS/resource-generator/internal/writers/writers.go @@ -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, diff --git a/prompt_for_opus_modifier_review_2.md b/prompt_for_opus_modifier_review_2.md new file mode 100644 index 0000000..038e9b8 --- /dev/null +++ b/prompt_for_opus_modifier_review_2.md @@ -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? diff --git a/provider/internal/core/operation_cfs.go b/provider/internal/core/operation_cfs.go index 53b422b..fccb41e 100644 --- a/provider/internal/core/operation_cfs.go +++ b/provider/internal/core/operation_cfs.go @@ -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. diff --git a/provider/internal/core/operation_run.go b/provider/internal/core/operation_run.go index ad350cf..60840cd 100644 --- a/provider/internal/core/operation_run.go +++ b/provider/internal/core/operation_run.go @@ -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 diff --git a/provider/internal/core/operation_run_bycode.go b/provider/internal/core/operation_run_bycode.go index 3fa04d2..964df75 100644 --- a/provider/internal/core/operation_run_bycode.go +++ b/provider/internal/core/operation_run_bycode.go @@ -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