Files
tf_provider/docs/opus_review_answer.md
T

28 KiB
Raw Blame History

Ответ Opus 4.8: код-ревью и оценка архитектуры (3 задачи roadmap)

Дата: 2026-09-22 | Вопрос: docs/prompt_for_opus_review.md (коммит 7eab45e)

Ответ получен строго по контексту промпта (файлы не открывались).


Задание — код-ревью по контексту (файлы открывать запрещено). Отвечаю сжато по пунктам.

1. Покрывает ли модель 3 задачи

Задача Вердикт Причина
vcOrg modify (динамич. IP) 🟡 Покрыта, если аллокация = modify-параметр (кол-во/список). Проблема — тип массива (array-map-fixed = JSON-строка) и семантика «растёт по мере необходимости»
vcNsxt modify (SNAT + IP из пула vcOrg) 🔴 Текущая модель НЕ покрывает: refSvc ссылается на инстанс/uid, а нужен конкретный элемент списка IP. Нужна новая связка (computed-атрибут + ссылка)
k8sShturval create 🟢 Стандартный CRUD-create, покрыт. Риски — только long-create и типы параметров

2. Как моделировать пул IP vcOrg

  • (б) отдельный ресурс/подресурс на IP — ✗ противоречит архитектуре: «один сервис = один ресурс», подресурсы/data source не генерируются. Потребует ломки генератора.
  • (а) атрибут-массив на nubes_vc_org согласуется. Но НЕ через array-map-fixed (JSON-строка: нет семантического diff, ломается порядок, нельзя адресовать элемент).
  • Рекомендация: декларативное желаемое состояние — либо int64 desired_count (modify), либо реальный ListNestedAttribute; аллоцированные IP отдавать как типизированный computed (list объектов), а не плоскую JSON-мапу. Императивная семантика «добавь ещё один» несовместима с декларативным Terraform.

3. Как vcNsxt получает IP из vcOrg

Вариант Оценка
(а) refSvc-параметр 🔴 не может указать на конкретный элемент списка, только на uid инстанса
(б) computed-атрибут vcOrg + ссылка nubes_vc_org.x.<attr> 🟢 лучший фит. IP в nsxt — обычная string, пользователь берёт конкретный элемент
(в) data source ✗ не генерируется

Рекомендация — (б). Условие: computed-вывод vcOrg должен быть индексируемым типизированным (allocated_ips[0]), не спрятанным в JSON-строке state_out_flat — иначе граф зависимостей Terraform не построит ребро и придётся ручной depends_on.

4. Риски текущего кода для этих потоков

  • 🔴 modify с массивами: array-map-fixed = JSON-строка → нет семантического diff, смена порядка API даёт ложные modify, «дорастить пул» = полная замена списка (опасно, возможен ре-аллок).
  • 🟡 create-only guard: если параметры SNAT-IP / аллокации помечены не как is_modifiable → ModifyPlan заблокирует задачи 1 и 2. Проверить флаги в YAML.
  • 🟡 read-back: если API возвращает IP в нестабильном порядке → вечный drift на vcOrg.
  • 🟡 порядок плана: зависимость nsxt→vcOrg держится, только если ссылаемый IP — first-class computed (unknown на plan). Внутри JSON-строки Terraform зависимость не отследит → apply-time ошибки.

5. k8sShturval create — что критично

  • refSvc к vdc/org резолвится (display name ИЛИ uid).
  • 🔴 long-create: create-op должен ждать готовности (poll ready-state), а не только «принято»; следить за context timeout.
  • типы параметров: map-fixed (nested) vs array-map-fixed (JSON), корректность required/default.
  • read-back state_params после create для computed-полей.

6. Итог — рекомендации по приоритету

  1. 🔴 Добавить в vcOrg типизированный индексируемый computed для аллоцированных IP (list объектов, не JSON-мапа) — разблокирует задачу 2 через ссылку.
  2. 🔴 Моделировать аллокацию как декларативное желаемое состояние (int64 count или ListNested) с реконсиляцией в modify; параметры пометить is_modifiable.
  3. 🟡 Заменить array-map-fixed (JSON-строка) на ListNestedAttribute — ради адресуемости элементов, семантического diff и стабильного порядка.
  4. 🟡 Гарантировать граф зависимостей: ссылаемый IP — отдельный computed-атрибут, не поле JSON, чтобы не требовать depends_on.
  5. 🟢 k8sShturval: добавить ожидание готовности (poll) в long-create и проверить резолв refSvc.

Ключевой вывод: задачи 1 и 3 укладываются в текущую модель с доработками генератора; задача 2 требует новой абстракции — типизированного computed-вывода на vcOrg (вариант 3б), т.к. refSvc принципиально не адресует элемент списка.


Раунд 2 — уточняющие вопросы и пересмотр

После проверки кода выяснилось, что промпт раунда 1 описывал модель неполно. Opus получил уточнённый контекст и 7 доп. вопросов.

Уточнение контекста (передано Opus)

  • Генератор уже поддерживает 4 вида ресурсов: instance (nubes_<svc>), subresource, action (только redeploy), modifier (nubes_<svc>_<modifier>).
  • Modifier-ресурсы для задач 1 и 2 УЖЕ СГЕНЕРИРОВАНЫ:
    • nubes_vc_org_ip_space (vc_org, modify 207): vc_org_id (required), v_ip_configure (required, StringAttribute = JSON-строка). Computed-выводов НЕТ.
    • nubes_vc_nsxt_network (vc_nsxt, modify 111): vc_nsxt_id (required), need_enable_avi (bool), virtual_services_count (int64), ip_space_name (string optional), qos_profile (string), routed_net_configuration (required, JSON-строка). Computed-выводов НЕТ.
  • Маппинг: map-fixed → SingleNestedAttribute; array-map-fixed → StringAttribute (JSON-строка).
  • Завершение операции — поллинг до dtFinish. Delete modifier — no-op.
  • Связка NSXT↔Org идёт по имени ipSpace: vc_org.modify задаёт vIPConfigure[].name, vc_nsxt.modify принимает ipSpaceName = это имя. Конкретный IP нигде не адресуется.

Ответ Opus (пересмотр)

1. Верен ли прошлый вердикт про «новую абстракцию»

🟢 Прошлый вывод отменяется. Он строился на неполной модели. modifier-ресурсы уже существуют → задача 2 реализуема сегодня двумя модификаторами (nubes_vc_org_ip_spacenubes_vc_nsxt_network), связанными по имени ipSpace. Никакой новой абстракции и ломки генератора не требуется.

2. Минимальная доработка генератора для ссылки вместо строки

Добавить один computed-вывод на nubes_vc_org_ip_space — эхо v_ip_configure[].name (имя ipSpace). Тогда:

ip_space_name = nubes_vc_org_ip_space.x.ip_space_name

Это даёт ребро графа + защиту от опечатки. Адресовать IP не нужно — связка по имени. Опционально сверх минимума: валидация, что имя существует в org (refSvc-подобная проверка ip_space_name по vIPConfigure целевого org).

3. SNAT: свободная строка или computed обязателен

  • Функционально свободная строка + depends_on работает (связка по имени, IP не адресуется). 🟡 Риски: опечатка, нет автоматического ребра графа без ручного depends_on, нет drift-детекта.
  • Экспортировать имя (string), НЕ список IP. Список IP не нужен — привязка идёт по имени ipSpace, не по конкретному адресу.
  • Вердикт: не строго обязателен для функции, но настоятельно рекомендован для корректности графа. Минимум — computed ip_space_name (string).

4. v_ip_configure как JSON-строка — реально ли бьёт

🟢 В этом сценарии практически безвредно. Значение фактически write-only (computed read-back нет), потребителю нужно только имя, которое пользователь сам задал. Поэлементное сравнение/адресация не требуются → прошлый 🔴 понижается.

Остаточный 🟡: спурьёзный diff при переформатировании JSON пользователем; если count растёт со временем — семантика «полная замена списка» без гранулярного diff может ре-триггерить modify/ре-аллокацию. Но без read-back и при связке по имени это не блокер.

5. Delete modifier = no-op

🟡 No-op допустим для association/one-shot модификатора, если обратного payload нет — это известный паттерн.

Реальные последствия destroy: Terraform удалит запись из state, но бэкенд НЕ освободит IP и НЕ отключит SNAT → утечка ресурсов + расхождение state↔бэкенд.

  • Если бэкенд поддерживает обратную операцию (де-аллокация / disable SNAT) — no-op это реальный пробел 🔴, надо подключить reverse-op.
  • НЕДОСТАТОЧНО ДАННЫХ: есть ли у vc_org/vc_nsxt обратный payload. Если его действительно нет — no-op единственный вариант, но обязательно задокументировать поведение destroy.

6. Нужен ли ещё «int64 desired_count на инстансе»

🟢 Нет, прошлый п.2 отменяется. modifier — правильная единица для чередующейся зависимости (org аллоцирует → nsxt потребляет → org доаллоцирует). Счётчик на инстанс-ресурсе связал бы жизненный цикл пула с жизненным циклом инстанса и сломал бы взаимное чередование org↔nsxt. Отдельный modifier строго лучше.

7. k8sShturval create — что остаётся критичным

Помимо резолва refSvc (display name ИЛИ uid) и корректности required/default в map-fixed/array-map-fixed:

  • 🟡 read-back computed после create (kubeconfig/vault-выводы) — что реально возвращается в state.
  • 🟡 обработка failure-состояния в поллинге: dtFinish должен различать «готово успешно» и «завершилось с ошибкой», иначе провал маскируется под успех. НЕДОСТАТОЧНО ДАННЫХ по логике ошибки в poll-цикле.
  • 🟢 timeout-бюджет long-create vs интервал поллинга.

Итог пересмотра: обе задачи 1 и 2 укладываются в существующий modifier-паттерн без новых абстракций. Единственная стоящая доработка — добавить computed-экспорт ip_space_name на org-модификатор (п.2/3), чтобы заменить свободную строку ссылкой и построить граф зависимостей. Проблема JSON-строки в этом сценарии не критична (п.4). Главный остаточный риск — no-op delete и утечка IP/SNAT при destroy (п.5), требует проверки наличия обратного payload.


Раунд 3 — дрейф сгенерированного кода и защитный механизм

Отдельная тема (не roadmap): расхождение generated/{stand}/goprovider/internal/resources_gen, как предотвратить и сделать видимым.

Контекст, переданный Opus

Go-провайдер Terraform. Ресурсы генерируются из YAML-спеков генератором. Цепочка: YAML → generated/{stand}/go/.go → (копирование) → provider/internal/resources_gen/.go → сборка бинаря.

Факты:

  • generated/ и provider/internal/resources_gen/ внесены в .gitignore → дрейф между ними НЕ виден в git status/diff.
  • Реальный дрейф уже случился: копия в provider/ устарела (сгенерирована до двух последних фиксов генератора).
  • Есть два скрипта сборки: (а) правильный — копирует свежий generated во временный каталог provider и собирает оттуда; (б) рисковый — собирает напрямую из provider/ (берёт потенциально устаревшую локальную копию).

Вопросы: 1) коммитить автоген или .gitignore+проверка; 2) два пути сборки — корень ли; 3) что предотвращает дрейф, а не детектит; 4) убрать вторую копию и генерировать при сборке; 5) как гарантированно «видно сразу при изменении кода».

Ответ Opus

1. Коммитить сгенерированный код или .gitignore + проверка дрейфа

🟢 Коммитить сгенерированный код (для соло/малой команды это надёжнее). Обоснование:

  • Расхождение всплывает в git diff/PR-ревью — там, куда уже смотришь, а не в отдельной проверке, которую надо не забыть запустить.
  • CI-гейт regenerate && git diff --exit-code превращает дрейф в жёсткую ошибку сборки.
  • Минус — шум в диффах от больших автогенов, но это приемлемая цена за видимость.

.gitignore + внешняя проверка дрейфа 🟡 — работает, только если проверку невозможно обойти; сейчас её обходят вторым скриптом. Скрытый код = скрытый дрейф (уже случилось).

2. Два пути сборки — корень проблемы?

🔴 Да, это корень. Проблема не в дрейфе как таковом, а в том, что существует путь, собирающий из потенциально устаревшей копии. Пока рисковый скрипт (б) доступен, дрейф рано или поздно попадёт в бинарь.

Оставить ровно один канонический путь. Скрипт (б) удалить или сделать так, чтобы он физически не мог собрать из несвежей копии (см. п.3–4).

3. Что ПРЕДОТВРАЩАЕТ дрейф, а не только детектит

Механизм Роль
Жёсткий fail на старте сборки 🟡 детект — не даёт собрать плохой бинарь, но дрейф уже есть
Авто-синхронизация/генерация перед сборкой 🟢 предотвращение — копия физически не может быть устаревшей
git pre-commit hook 🟡 детект на этапе коммита

Предотвращает только генерация/синк как обязательная часть сборки (копия всегда свежая). Fail и hook — детекторы. Рекомендуемая комбинация: генерация-при-сборке (предотвращение) + CI git diff --exit-code после регенерации (страховка от обхода).

4. Более сильный паттерн: убрать вторую копию, генерировать при сборке

🟢 Да, это сильнейший вариант. Нет постоянной копии → нечему рассинхронизироваться. Единственный источник истины = YAML + генератор (go generate / шаг Makefile перед go build).

Минусы:

  • 🟡 Сборка зависит от наличия и детерминированности генератора (версию генератора надо пинить).
  • 🟡 Медленнее сборка.
  • 🔴 Провайдер перестаёт быть чистым Go-модулем: нельзя go get/собрать без тулчейна генератора; воспроизводимость завязана на версию генератора.
  • Сложнее просто «посмотреть» сгенерированный код без сборки.
  • НЕДОСТАТОЧНО ДАННЫХ: детерминирован ли генератор (стабильный порядок полей/файлов) — без этого diff-гейт будет шуметь.

Компромисс: генерировать при сборке И коммитить результат (генерация переписывает закоммиченную копию, CI ловит расхождение) — совмещает «невозможно устареть» с «видно в диффе».

5. «Видно сразу при изменении кода, не закопано в доках»

Гарантированно это даёт только закоммиченный сгенерированный код: расхождение появляется в git diff того же PR, где меняется генератор/YAML. Усилить:

  • pre-commit hook regenerate → расхождение всплывает до коммита;
  • CI-гейт regenerate && git diff --exit-code → нельзя смёржить с дрейфом.

Вариант «генерировать при сборке без копии» даёт другую гарантию — «устареть невозможно», но там нет диффа для обзора. Если требование именно «видно в изменениях кода» — нужен закоммиченный автоген.


Итог — рекомендация по приоритету:

  1. 🔴 Убрать рисковый путь сборки (б); оставить один канонический.
  2. 🟢 Сделать генерацию/синк обязательным шагом канонической сборки (предотвращение).
  3. 🟢 Закоммитить сгенерированный код + CI-гейт regenerate && git diff --exit-code (видимость + защита от обхода).
  4. 🟡 Запинить версию генератора и обеспечить детерминированный вывод, иначе гейт будет ложно срабатывать.

Раунд 4 — универсальность и реестр исключений

Тема: принцип «универсально по умолчанию, исключения — только если API не ложится», и как кодировать исключения, чтобы они были однозначно понимаемы при каждой генерации.

Контекст, переданный Opus (кратко)

Пайплайн: YAML-спека сервиса (из API) → универсальный генератор → Go-ресурсы → сборка. Код/скрипты/логика генерации — одни для всех стендов и сервисов; различается только YAML. Механизм modifier-ресурсов уже универсален (дженерик читает kind: modifier + имя). Не универсально сейчас: хардкод в yaml-generator (svc.ID == 19 → ip_space, svc.ID == 22 → network) и в docs-generator (ServiceID == 90, спец-вывод postgres). Отдельно был инцидент дрейфа git-ignored сгенерированного кода (фон).

Выработанный принцип (просили оценить): «Универсально по умолчанию. Исключение — только если API реально не ложится. Каждое исключение — одна явная таблица/структура, которую КОД ЧИТАЕТ, с полями ЧТО/ЧТО ДЕЛАЕТ/ПОЧЕМУ/ГДЕ. Канонический список — в ARCHITECTURE-доке, код ссылается. Изменение = изменение таблицы → видно в диффе.»

Ответ Opus

1. Оценка принципа

🟢 Принцип корректен долгосрочно — «convention over configuration + escape hatch».

Подводные камни:

  • 🟡 «API не ложится» субъективно → нужен объективный тест-триггер, иначе exception creep.
  • 🔴 Исключения не возвращаются в ядро: когда паттерн повторился 2–3 раза, нужен ритуал «промоушена» в ядро.
  • 🟡 Обратный перекос: обобщать реально одноразовый случай — раздувает ядро.

2. Как кодировать исключения

Вариант Видимость в diff Нельзя «проспать» Поддержка Рассинхрон с YAML
(а) именованная таблица в коде, код её читает 🟢 🟢 (если итерирует и падает на неучтённом) 🟡 нужна пересборка 🟢 низкий
(б) отдельный yaml-конфиг 🟢 🟡 легко забыть подключить 🟢 без пересборки 🟡 средний
(в) аннотации в YAML-спеке 🔴 🔴 🔴 🔴 фатально: YAML регенерится → аннотации затираются

(в) отклонить. Рекомендация: (а) — именованная структура, которую код итерирует и ассертит.

3. Граница «логика» vs «данные»

  • В ядре — механизм/алгоритм (как модификатор генерится, маппинг схемы). Никогда не per-service.
  • В реестре — чистые данные («сервис X → имя модификатора Y», «сервис 90 → набор полей Z»).

Признаки: 1) if id == N, меняющий поток исполнения → извлечь данные; 2) убрать пункт → меняются только значения, не поведение → данные; 3) copy-paste кода → механизм (обобщать), разные строки таблицы → данные.

4. Паттерн «override registry» в кодогенераторах

  • tfplugingen-openapi: generator_config.yml отдельно от спеки + IR (terraform-plugin-codegen-spec).
  • OpenAPI Generator: vendor extensions x-* + template-оверрайды + config-json.
  • protoc-плагины: custom options (напр. google.api.http).

Что перенять: 1) отдельный версионируемый конфиг оверрайдов; 2) IR-слой; 3) fail на неучтённом; 4) стабильное символьное имя, не сырой ID.

5. Не противоречит ли реестр «YAML — единственный источник»

Не противоречит — при разделении двух доменов истины:

  • API-YAML = истина про «что есть сервис» (машинно-владеемый, регенерится).
  • Реестр оверрайдов = истина про «наши провайдер-специфичные решения» (человеко-владеемый).

Теневой источник — только если ОДИН факт лежит в обоих. 🔴 Нельзя аннотировать API-YAML. Конвейер: API-YAML + overrides → merged IR → codegen.

6. Риски «таблица + раздел в ARCHITECTURE.md»

  • 🔴 ARCHITECTURE.md дрейфует от таблицы → возврат к if id==19, если кто-то добавит ветку в обход.
  • 🔴 Числовые ID (19/22/90) непрозрачны.

Как закрыть: 1) 🔴 единая точка маршрутизации + CI-lint/grep-гейт против svc.ID == вне реестра; 2) раздел ARCHITECTURE генерировать ИЗ реестра (golden-test); 3) 4 поля — поля структуры, а не комментарии; 4) стабильные символьные ключи; 5) fail-fast: генератор падает, если спец-обработка без записи в реестре.


Итог: сильнейшая реализация — отдельный человеко-владеемый override-реестр (данные, не логика), стабильные ключи, IR-слой применения, CI-гейт против хардкодов вне реестра, раздел ARCHITECTURE генерируется из реестра. Это устраняет и if id==N, и дрейф доки.