6.4 KiB
6.4 KiB
Opus-код-ревью: модификаторы (kind: modifier) — 2026-09-22
Источник: ревью по prompt_for_opus_modifiers_review.md.
Объекты: nubes_vc_org_ip_space (vc_org.ip_space, modify 207) и nubes_vc_nsxt_network (vc_nsxt.network, modify 111).
Файлы: шаблон TOOLS/resource-generator/internal/templates/modifier.go, loader, generated/dev/go/19_vc_org_ip_space_modifier.go, 22_vc_nsxt_network_modifier.go, registry.go, provider/internal/resources_core/crud.go (RunOperationByCodeWithTimeout), provider/internal/core/client.go (RunInstanceOperationUniversalByCode, RefreshResourceState, ShouldRemoveFromState).
1. Жизненный цикл Create/Update/Read/Delete
- Create ≡ Update: оба тела идентичны — гонят
modifyс текущими params. Любое изменение любого атрибута = повторный запускmodifyцеликом, не дельта. - Идемпотентность на платформе, не в провайдере. Нет сравнения «до/после», нет проверки, что операция применила именно эти значения (только
validate-cfs+run). Если API-modifyаккумулирует, а не перезаписывает (особенноvIPConfigure) — повтор = дубли/лишний расход квоты. - Refresh (Read) частичный и потенциально вредный.
RefreshResourceStateтянетstate_paramsи перезаписывает input-поля:- если платформа не эхоит код в
state_params(типично для операционных modify-параметров) — дрейф не детектируется, Read почти no-op → управление «вслепую»; - если эхоит, но нормализованно (bool как
1/0, порядок ключей вroutedNetConfiguration/map-fixed) — вечный diff.JsonNormalize()стоит только как plan-modifier на Required-строке; веткаmap-fixedв refresh json-нормализацию не гарантирует.
- если платформа не эхоит код в
2. Delete = no-op — ожидаемо? Подводные камни
No-op ожидаем (обратного payload нет). Но:
- destroy убирает ресурс из state, оставляя эффект на платформе (выделенные IP, включённый AVI/LB). Инфраструктура и state расходятся молча.
- Самый опасный сценарий — taint/replace или destroy→apply: Create снова гонит
modify→ повторное выделение внешних IP (nubes_vc_org_ip_space). Прямой риск двойного выделения и расхода. BuildActionID(instanceUID, "modify", "ip_space")— детерминированный константный ID, не привязан к реальному opUid. State не отражает, какая операция и с какими значениями отработала; два модификатора одного типа на одном инстансе получили бы одинаковый ID.
3. Риски передачи по коду (code → id) в RunInstanceOperationUniversalByCode
- Резолв code→id полностью зависит от
GET /instanceOperations/{opUid}?fields=cfsParams— того запроса, что даёт 500 на проблемных инстансах. Без fallback модификатор неработоспособен целиком (без словаряcodeToParamпараметры не отправить). - Коды захардкожены в сгенерированном коде (
vIPConfigure,needEnableAVI…). Переименование на платформе ломается в рантайме («код параметра X не найден»), а не на компиляции — молчаливая деградация. - Частичный payload = скрытые сайд-эффекты.
CompactParamsвыкидывает пустые Optional. Дляmodifyпропуск параметра платформа может трактовать как «сбросить в дефолт» (не задалneedEnableAVI→ LB может выключиться). Семантика PATCH vs PUT не контролируется провайдером. - opId ищется среди
AvailableOperations: не то состояние инстанса → жёсткий отказ «операция недоступна».LockInstanceсериализует операции по инстансу — конкурентность закрыта корректно.
ТОП-3 критичных
- Двойное выделение при replace/destroy→apply (особенно
ip_space): no-op Delete + повторныйmodifyна Create + отсутствие проверки идемпотентности = риск задвоить внешние IP/квоту. Нужен guard передmodify(проверка поstate_params/наличию ресурса), либо явно документировать запрет replace. - Refresh либо слепой, либо вечный diff. Для операционных modify-параметров
state_paramsобычно их не возвращает → Read ничего не сверяет; там где возвращает — нормализация (bool/JSONmap-fixed) ломает план. Решить: честный drift-refresh с нормализацией, либо явно пометить поля как не-refreshable. - Жёсткая зависимость от падающего
?fields=cfsParams. code→id держится на запросе, который 500-тит на проблемных инстансах — модификатор ложится целиком. Fallback на/instanceOperations/default/{opId}— условие работоспособности, а не «приятная опция».
Мелочи
- Константный
BuildActionID— ID не привязан к реальной операции. - Захардкоженные коды ломаются в рантайме, а не на сборке.