265 lines
28 KiB
Markdown
265 lines
28 KiB
Markdown
# Ответ 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_space` → `nubes_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}/go` ↔ `provider/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`, и дрейф доки.
|