diff --git a/TOOLS/ARCHITECTURE.md b/TOOLS/ARCHITECTURE.md index ded1d37..133db9c 100644 --- a/TOOLS/ARCHITECTURE.md +++ b/TOOLS/ARCHITECTURE.md @@ -212,3 +212,31 @@ From the unified YAML, generate: - No manual edits to generated YAML or generated Go code. - Any change must come from API or generator logic updates. - The generator must enforce these rules and fail fast on drift. + +## Exception Registry (service-specific DATA, never logic) + +Principle: provider core and generator logic are universal for all stands and +services. The ONLY allowed deviations are DATA entries, and they MUST live in +exactly two named registries: + +| Registry | File | Declares | +|---|---|---| +| `serviceSpecificModifiers` | `TOOLS/yaml-generator/main.go` | which service `modify` op becomes a modifier resource and its name (key = normalized service name) | +| `serviceSpecificDocExamples` | `TOOLS/docs-generator/internal/writers/writers.go` | per-service doc examples, gated on service name + required state/vault keys | + +Rules: + +- Key by stable service NAME (slug), never by raw numeric ID. +- Each entry answers WHAT / WHAT IT DOES / WHY / WHERE (see code comments). +- Adding an exception = editing one of these two registries → visible in diff. +- Never annotate API-YAML: it is machine-regenerated and edits would be lost. + +Enforced by scripts (run before build/commit): + +- `TOOLS/scripts/check_generated_drift.sh ` — generated Go vs provider copy. +- `TOOLS/scripts/check_hardcoded_service_ids.sh` — forbids `svc.ID == N` / + `ServiceID == N` outside the registries. + +Build rule: `provider/internal/resources_gen` is a transient sync copy, never a +build source. Canonical build is `03_build_and_upload_provider.sh` (temp copy from +`generated//go`); `build-provider.sh` refuses direct build from `provider/`. diff --git a/docs/opus_review_answer.md b/docs/opus_review_answer.md new file mode 100644 index 0000000..108bcbc --- /dev/null +++ b/docs/opus_review_answer.md @@ -0,0 +1,264 @@ +# Ответ 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.` | 🟢 лучший фит. 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_`), subresource, action (только redeploy), modifier (`nubes__`). +- 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`, и дрейф доки.