From ea75cac1a8175ded647905e4364237af45157ac9 Mon Sep 17 00:00:00 2001 From: Repinoid Date: Wed, 30 Sep 2026 20:35:09 +0300 Subject: [PATCH] =?UTF-8?q?fix(core):=20idempotency=20pre-check=20=D1=81?= =?UTF-8?q?=D1=80=D0=B0=D0=B2=D0=BD=D0=B8=D0=B2=D0=B0=D0=B5=D1=82=20=D1=81?= =?UTF-8?q?=20live,=20=D0=B0=20=D0=BD=D0=B5=20=D1=81=20paramValue=20=D1=84?= =?UTF-8?q?=D0=BE=D1=80=D0=BC=D1=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Проблема: modifierDesiredEqualsCurrent сравнивает desired с paramValue из cfsParams (дефолт ФОРМЫ операции), а не с состоянием инстанса. Пропуск modify на такой основе может быть ложным (HAR/edge_.har: needEnableAVI paramValue=false при live=true). - core/modifier_compare.go: добавлен modifierDesiredEqualsLive (источник — instanceLiveParams/state.params; неопределённость => не пропускаем) и хелперы modifierValuesEqual / modifierCodeMap; modifierDesiredEqualsCurrent переведён на них. - core/operation_run_bycode.go: idempotent-путь использует live-pre-check; при ошибке чтения live modify НЕ пропускается. - resources_core/nsxt_snat_resource.go: setSnat -> RunInstanceOperationUniversalByIdempotent (лишний modify на повторном apply больше не отправляется). - modifier_compare_test.go: TestModifierDesiredEqualsLive (совпало/отличается/live недоступен). Проверено: go build ./... OK; go test ./internal/core/... -short -> PASS. --- provider/internal/core/modifier_compare.go | 96 +++++++++++++------ .../internal/core/modifier_compare_test.go | 57 ++++++++++- .../internal/core/operation_run_bycode.go | 15 ++- .../resources_core/nsxt_snat_resource.go | 7 +- 4 files changed, 139 insertions(+), 36 deletions(-) diff --git a/provider/internal/core/modifier_compare.go b/provider/internal/core/modifier_compare.go index ea87492..415fec0 100644 --- a/provider/internal/core/modifier_compare.go +++ b/provider/internal/core/modifier_compare.go @@ -1,6 +1,7 @@ package core import ( + "context" "strings" "terraform-provider-nubes/internal/core/jsonutil" @@ -19,15 +20,7 @@ func (c *UniversalClient) modifierDesiredEqualsCurrent(desired map[string]string return false } - codeToParam := make(map[string]universalCfsParam) - for _, p := range cfsParams { - if key := strings.ToLower(strings.TrimSpace(p.Code)); key != "" { - codeToParam[key] = p - } - if key := strings.ToLower(strings.TrimSpace(p.SvcOperationCfsParam)); key != "" { - codeToParam[key] = p - } - } + codeToParam := modifierCodeMap(cfsParams) for code, wanted := range desired { param, ok := codeToParam[strings.ToLower(strings.TrimSpace(code))] @@ -41,28 +34,75 @@ func (c *UniversalClient) modifierDesiredEqualsCurrent(desired map[string]string current = *param.ParamValue } - dataType := strings.ToLower(strings.TrimSpace(param.DataType)) - if strings.HasPrefix(dataType, "array") { - // array-map-fixed: сравнивать сырые значения как JSON. - if !jsonutil.JSONStringsEquivalent(wanted, current) { - return false - } - continue - } - if strings.HasPrefix(dataType, "map") || strings.Contains(dataType, "json") { - if !jsonutil.JSONStringsEquivalent(wanted, current) { - return false - } - continue - } - - // Скаляры: нормализуем обе стороны. - nw := normalizeUniversalValueV6(wanted, param) - nc := normalizeUniversalValueV6(current, param) - if nw != nc { + if !modifierValuesEqual(wanted, current, param) { return false } } return true } + +// modifierValuesEqual сравнивает одно значение с учётом типа параметра: +// - array*/map*/json — смысловое JSON-сравнение (порядок ключей не значим); +// - скаляры — нормализация через normalizeUniversalValueV6 и сравнение строками. +func modifierValuesEqual(wanted, current string, param universalCfsParam) bool { + dataType := strings.ToLower(strings.TrimSpace(param.DataType)) + if strings.HasPrefix(dataType, "array") || + strings.HasPrefix(dataType, "map") || + strings.Contains(dataType, "json") { + return jsonutil.JSONStringsEquivalent(wanted, current) + } + return normalizeUniversalValueV6(wanted, param) == normalizeUniversalValueV6(current, param) +} + +// modifierDesiredEqualsLive — idempotency-pre-check по ЖИВЫМ параметрам инстанса +// (state.params), а не по paramValue формы операции. +// +// Зачем: paramValue из cfsParams — это дефолт ФОРМЫ операции, а не состояние +// инстанса (см. instanceLiveParams, HAR/edge_.har). Пропуск modify на основе +// формы может быть ложным. Здесь источник — live. +// +// (true, nil) возвращается только если ВСЕ desired-поля найдены в live и совпали. +// Любая неопределённость (пусто, поле не найдено, ошибка) → (false, …): modify +// лучше выполнить лишний раз, чем пропустить нужный. +func (c *UniversalClient) modifierDesiredEqualsLive(ctx context.Context, instanceUid string, desired map[string]string, cfsParams []universalCfsParam) (bool, error) { + if len(desired) == 0 { + return false, nil + } + live, err := c.instanceLiveParams(ctx, instanceUid) + if err != nil { + return false, err + } + if len(live) == 0 { + return false, nil + } + codeToParam := modifierCodeMap(cfsParams) + for code, wanted := range desired { + param, ok := codeToParam[strings.ToLower(strings.TrimSpace(code))] + if !ok { + return false, nil + } + current, ok := lookupLiveParam(live, param) + if !ok { + return false, nil + } + if !modifierValuesEqual(wanted, current, param) { + return false, nil + } + } + return true, nil +} + +// modifierCodeMap индексирует cfsParams по нижнему регистру Code и SvcOperationCfsParam. +func modifierCodeMap(cfsParams []universalCfsParam) map[string]universalCfsParam { + m := make(map[string]universalCfsParam, len(cfsParams)*2) + for _, p := range cfsParams { + if key := strings.ToLower(strings.TrimSpace(p.Code)); key != "" { + m[key] = p + } + if key := strings.ToLower(strings.TrimSpace(p.SvcOperationCfsParam)); key != "" { + m[key] = p + } + } + return m +} diff --git a/provider/internal/core/modifier_compare_test.go b/provider/internal/core/modifier_compare_test.go index 0ec293b..72b52fc 100644 --- a/provider/internal/core/modifier_compare_test.go +++ b/provider/internal/core/modifier_compare_test.go @@ -1,6 +1,10 @@ package core -import "testing" +import ( + "context" + "net/http" + "testing" +) func TestModifierDesiredEqualsCurrent_Scalars(t *testing.T) { c := &UniversalClient{} @@ -61,3 +65,54 @@ func TestModifierDesiredEqualsCurrent_ArrayPreservesOrder(t *testing.T) { t.Errorf("array-map-fixed с другим порядком не должен быть равен") } } + +func TestModifierDesiredEqualsLive(t *testing.T) { + cfsParams := []universalCfsParam{ + {SvcOperationCfsParamId: 372, Code: "ipSpaceName", DataType: "string"}, + } + + newClient := func(liveParams string) (*UniversalClient, func()) { + return makeTestClient(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"instance":{"state":{"params":` + liveParams + `}}}`)) + }) + } + + t.Run("совпало с live", func(t *testing.T) { + c, cleanup := newClient(`{"ipSpaceName":"internet-ipv4-v1"}`) + defer cleanup() + equal, err := c.modifierDesiredEqualsLive(context.Background(), "inst-1", map[string]string{"ipSpaceName": "internet-ipv4-v1"}, cfsParams) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !equal { + t.Errorf("expected equal when live matches desired") + } + }) + + t.Run("отличается от live", func(t *testing.T) { + c, cleanup := newClient(`{"ipSpaceName":"no-needed"}`) + defer cleanup() + equal, err := c.modifierDesiredEqualsLive(context.Background(), "inst-1", map[string]string{"ipSpaceName": "internet-ipv4-v1"}, cfsParams) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if equal { + t.Errorf("expected not equal when live differs from desired") + } + }) + + t.Run("live недоступен — ошибка, не пропускаем", func(t *testing.T) { + c, cleanup := makeTestClient(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }) + defer cleanup() + equal, err := c.modifierDesiredEqualsLive(context.Background(), "inst-1", map[string]string{"ipSpaceName": "internet-ipv4-v1"}, cfsParams) + if err == nil { + t.Errorf("expected error when live is unavailable") + } + if equal { + t.Errorf("must not report equal when live is unavailable") + } + }) +} diff --git a/provider/internal/core/operation_run_bycode.go b/provider/internal/core/operation_run_bycode.go index 964df75..d66e5c8 100644 --- a/provider/internal/core/operation_run_bycode.go +++ b/provider/internal/core/operation_run_bycode.go @@ -60,10 +60,17 @@ func (c *UniversalClient) runInstanceOperationByCode(ctx context.Context, instan return err } - // Idempotency pre-check: если все desired уже равны live-значениям — пропускаем run. - // desired = явно заданные пользователем коды (params, keyed by code), БЕЗ досылки. - if idempotent && c.modifierDesiredEqualsCurrent(params, cfsParams) { - return nil + // Idempotency pre-check: если все desired уже равны LIVE-значениям инстанса — + // пропускаем run. Источник сравнения — state.params (live), НЕ paramValue формы + // операции: форма может не совпадать с состоянием (см. instanceLiveParams). + // желаемые = явно заданные пользователем коды (params, keyed by code), БЕЗ досылки. + if idempotent { + equal, liveErr := c.modifierDesiredEqualsLive(ctx, instanceUid, params, cfsParams) + if liveErr == nil && equal { + return nil + } + // Ошибка/несовпадение live — не пропускаем: выполняем modify. Если live + // действительно недоступен, следующий шаг вернёт ошибку явно (не молча). } codeToParam := make(map[string]universalCfsParam) diff --git a/provider/internal/resources_core/nsxt_snat_resource.go b/provider/internal/resources_core/nsxt_snat_resource.go index d545348..497b630 100644 --- a/provider/internal/resources_core/nsxt_snat_resource.go +++ b/provider/internal/resources_core/nsxt_snat_resource.go @@ -250,9 +250,10 @@ func (r *NsxtSnatResource) setSnat(ctx context.Context, nsxtUID types.String, ip unlock := r.client.LockInstance(uid) defer unlock() - // ByCode, а не ByIdempotent: idempotency-сравнение идёт с paramValue ФОРМЫ операции, - // а не с live-состоянием инстанса — можно ложно пропустить modify. - return r.client.RunInstanceOperationUniversalByCode(ctx, uid, "modify", map[string]string{ + // Idempotent: core.RunInstanceOperationUniversalByIdempotent выполняет pre-check + // по LIVE-значениям инстанса (state.params) и пропускает modify, если + // ipSpaceName уже совпадает — повторный apply не дёргает платформу зря. + return r.client.RunInstanceOperationUniversalByIdempotent(ctx, uid, "modify", map[string]string{ "ipSpaceName": value, }) }