Files
tf_provider/HISTORY/OPUS/3006_0_analysis.md
T

306 lines
32 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
Что сказать Opus — кратко
Задача: глубокий анализ проекта, найти слабые места, предложить улучшения.
10 вопросов:
Два провайдера (Legacy + Universal) — как и когда выпилить Legacy?
Генерационный конвейер — API→YAML→Go→S3. Надёжен ли? Что если API поменяет формат?
API Flow V6 — 6 шагов без транзакционности. Что при обрыве посредине?
CRUD/Adopt — сложнейшая логика в adoptExistingInstanceOnCreate. Все ли краевые кейзы покрыты?
Генератор gen_v2 — шаблоны вшиты в бинарь. Как расширять? Что с новыми kind'ами операций?
Баги — 23 файла истории. Какие системные? Фильтрация deleted — главная проблема.
Матрица состояний — 16 состояний, все ли обрабатываются?
Тесты — crud_test.go, client_test.go. Достаточно ли?
Безопасность — токены, GPG-ключ в репозитории, InsecureSkipVerify.
Конкурентность — два terraform apply одновременно?
Что читать (приоритет):
ARCHITECTURE.md
client.go
crud.go
generate_resources_v2.go
Searched for regex `.*` (`tf_provider` · `universal_rebuild/internal/resources_core/crud_test.go`), 56 results
Read [](file:///home/naeel/tf_provider/universal_rebuild/tools/gen_v2/generate_resources_v2.go#210-210), lines 210 to 350
Searched for regex `buildSubresourceForceNewCodes|computeCreateOnly|analyzeJsonPlanMod` (`tf_provider` · `universal_rebuild/tools/gen_v2/generate_resources_v2.go`), 10 results
Read [](file:///home/naeel/tf_provider/universal_rebuild/internal/core/client_test.go)
Searched for regex `func.*IsJson|func.*RefSvc|func.*computeCreateOnly|func.*analyzeJsonPlanMod` (`tf_provider` · `universal_rebuild/tools/gen_v2/generate_resources_v2.go`), 3 results
Read [](file:///home/naeel/tf_provider/universal_rebuild/tools/gen_v2/generate_resources_v2.go#2040-2040), lines 2040 to 2150
Read [](file:///home/naeel/tf_provider/.gitignore)
Read [](file:///home/naeel/.vscode-server/data/User/workspaceStorage/358cf615179442f06eab7245b983b753/GitHub.copilot-chat/chat-session-resources/2c7367dd-6d7f-48b4-84d0-25bbb06f7cfb/toolu_01RcjLgQg9SVXsgzGtT1p8Wa__vscode-1782817912278/content.txt)
Изучил архитектуру, ядро, генератор, историю багов и тесты. Сохраню анализ и представлю его.
Created memory file [](file:///home/naeel/.vscode-server/data/User/workspaceStorage/358cf615179442f06eab7245b983b753/GitHub.copilot-chat/memory-tool/memories/MmM3MzY3ZGQtNmQ3Zi00OGI0LTg0ZDAtMjViYmIwNmY3Y2Zi/plan.md)
# Анализ Terraform Provider для Nubes Cloud
Изучил архитектуру (ARCHITECTURE.md), ядро (client.go, crud.go), генератор (generate_resources_v2.go), транспорт (provider.go), 24 файла истории и тесты. Ниже — ответы по всем 10 пунктам: текущее состояние → плюсы → минусы → предложения → риски.
---
## 1. Два провайдера: Legacy + Universal
**Состояние.** Legacy (internal, v5.0.52, реестр `registry.terraform.io/nubes/nubes`, 13 ручных ресурсов) и Universal (universal_rebuild, v5.0.51, реестр `terra.k8c.ru/nubes/nubes`, ~50 генерируемых). Оба на `terraform-plugin-framework`, но это **полностью независимые деревья кода** с дублирующимся ядром (core существует в обоих).
**Хорошо.** Universal — целевая архитектура, чёткие принципы в ARCHITECTURE.md (YAML = source of truth). Разные реестры → можно мигрировать без коллизий.
**Плохо.** Дублирование ядра (client.go, `instance_lookup.go`) — баги фиксятся в двух местах (см. баг #1: правка в client.go И instance_lookup.go). Версии разъехались (5.0.52 vs 5.0.51) — непонятно, какая «свежее». Нет формального deprecation-плана с датой.
**Предложения.**
- Зафиксировать **матрицу соответствия ресурсов** Legacy→Universal: какие 13 ресурсов уже перекрыты Universal, какие нет.
- Объявить Legacy *frozen* (только critical-фиксы), завести `DEPRECATED.md` с целевой версией снятия.
- Перенести уникальную логику Legacy (VM/vApp/edge/vdc) в YAML-спеки, проверить паритет, затем archive Legacy в отдельную ветку/тег.
**Риск.** VM/vApp в Legacy содержат ручную логику (FW-rules, 500-фикс из 04_vm_hang_fix_and_500_error), которую генератор может не воспроизвести. Нужен паритетный прогон на тестовом стенде до снятия Legacy.
---
## 2. Генерационный конвейер (API→YAML→Go→S3)
**Состояние.** 4 шага: `service_spec_gen` (API→YAML, ATTEMPTS=3, REQUEST_DELAY=0.5) → `gen_v2` (YAML→Go) → build+GPG+S3 → mkdocs. Включение сервиса = строка в `services_list.txt`.
**Хорошо.** Чёткое разделение, профили dev/test/prod изолируют артефакты, retry на шаге сбора YAML.
**Плохо — главное расхождение с собственными принципами.** ARCHITECTURE.md декларирует «*The generator must enforce these rules and fail fast on drift*», но фактически:
- **Неизвестный `kind` операции тихо игнорируется** (generate_resources_v2.go — три `if op.Kind != "..." { continue }`). Если API введёт новый kind — ресурс молча пропадёт из провайдера, без ошибки.
- **YAML почти не валидируется**: проверяется только синтаксис (`yaml.Unmarshal`). Отсутствие `op.Kind`/`op.Action` → zero-value → тихое игнорирование. Нет проверки уникальности param ID, наличия required-полей, валидности `RefSvcId`.
- При ошибке `format.Source` генератор пишет **неформатированный (возможно битый) код** как fallback вместо остановки.
- Исключение сервиса только комментированием в `services_list.txt` → риск рассинхрона (закомментировали в test, забыли в prod).
**Предложения.**
- Добавить фазу `validateSpec()` перед генерацией: required-поля, уникальность ID, известность `kind`/`action`, ссылочная целостность `RefSvcId`. **Fail fast** на неизвестном kind.
- Убрать fallback на неформатированный код — при `format.Source` error → паника с понятным сообщением.
- CI-шаг «генерация без diff»: прогон генератора → `git diff --exit-code` (детект дрейфа, как требует ARCHITECTURE.md).
- Go для генераторов оправдан (один язык со сгенерированным кодом, `text/template`, `format.Source`); bash-обёртки — лишь оркестрация. Менять не нужно.
**Риск.** Изменение формата ответа API на шаге 1 не обнаружится до runtime у пользователя. Сейчас единственная защита — `ATTEMPTS=3`, что не ловит *семантический* дрейф (поле переименовали, а не пропало).
---
## 3. API Flow V6 — отсутствие транзакционности
**Состояние.** 6 шагов в `CreateGenericInstanceUniversalV6`: `POST /instances``POST /instanceOperations``GET cfsParams` → resolve refs → `POST` каждого param → `validate-cfs``run``waitForOperationFinish`.
**Хорошо.** Завершение по `dtFinish` — надёжный контракт (выстрадан в 02, 05). Нормализация пустых map/json/array есть.
**Плохо.**
- **Orphan при обрыве.** Если процесс упал/таймаут после `POST /instances`, но до `run` — в облаке остаётся инстанс в состоянии `not_created`/`creating`, **не попавший в Terraform state**. Следующий apply найдёт его через `FindInstanceByDisplayName` и упрётся в ошибку «найден в состоянии Not Created; удалите вручную». То есть пользователь обязан чистить руками.
- **`doRequest` без retry** (client.go) — нет обработки 429/503/сетевых сбоев. Любой transient-сбой на шаге 45 рвёт create.
- `req.Close = true` — новое TCP+TLS соединение на каждый запрос (на длинном поллинге дорого).
**Предложения.**
- Retry с экспоненциальным backoff в `doRequest` для идемпотентных GET и для 429/503/сетевых ошибок (с уважением `Retry-After`).
- Идемпотентность create: перед `POST /instances` делать `FindInstanceByDisplayName` (уже есть в `CreateResource`) — но также **обрабатывать «недосозданный» инстанс**: предлагать авто-cleanup `not_created`-инстанса при `adopt_existing_on_create=true`, а не только ручное удаление.
- Рассмотреть keep-alive (убрать `req.Close=true`) для поллинга — меньше TLS-handshake.
**Риск.** Авто-cleanup `not_created` — операция удаления, требует явного флага и подтверждения семантики (нельзя удалять то, что пользователь мог создавать вручную параллельно).
---
## 4. CRUD / Adopt — `adoptExistingInstanceOnCreate`
**Состояние.** Покрытые ветки в crud.go:
1. `operation_in_progress`/`pending` → ошибка «дождитесь».
2. `!adopt_existing_on_create` → ошибка с подсказкой про import.
3. `not_created` → ошибка.
4. `running` → ref-валидация → adopt (возврат UUID).
5. `suspended` → required-params check → resume → проверка статуса после resume → ref-валидация → adopt.
6. Иначе (`creating`/`failed`/`error`) → общая ошибка «статус не подходит для авто-усыновления».
**Хорошо.** Логика соответствует decision-matrix из ARCHITECTURE.md. Ref-валидация при adopt (баг 22) закрыта. Диагностики подробные.
**Плохо.**
- **Конкурентность не покрыта** (см. п.10): между `FindInstanceByDisplayName` и `CreateGenericInstanceUniversalV6` нет блокировки.
- `creating`/`failed` падают в *общую* ветку с менее информативным сообщением, чем требует ARCHITECTURE.md (там для `creating`/`pending`/`failed` предписана отдельная диагностика).
- Функция ~100 строк, глубокая вложенность, ref-валидация дублируется в двух ветках (running и после resume) — риск рассинхрона при правках.
- `state_only`/`detach` destroy → `return nil` без API-вызова: инстанс остаётся в облаке (**это by design** из 20, но «orphaned» с т.з. биллинга — пользователь должен понимать).
**Предложения.**
- Вынести классификацию статуса в один `switch` с явными ветками для каждого из 16 состояний (см. п.7), убрать дублирование ref-валидации в helper.
- Для `creating`/`failed` — отдельные сообщения по контракту ARCHITECTURE.md.
- Покрыть adopt-матрицу таблично-управляемыми тестами (сейчас 0 тестов на adopt, п.8).
**Риск.** Рефакторинг самой сложной функции без тестов опасен — сначала тесты, потом рефакторинг.
---
## 5. Генератор gen_v2 — шаблоны вшиты в бинарь
**Состояние.** 3 inline-шаблона `text/template`: `instanceTemplate` (569 строк), `subresourceTemplate` (443), `actionTemplate` (174). `computeCreateOnly` = эвристика (param в create, но не в modify). Identity подресурса: нет modify → все params ForceNew; есть modify → createOnly+delete params ForceNew.
**Хорошо.** Эвристика createOnly опирается на данные API (не хардкод), `format.Source` гарантирует валидный Go при успехе, восстановление регистра UUID решает «inconsistent result».
**Плохо.**
- Шаблоны как строковые константы внутри `.go` (1186 строк шаблонов) — тяжело поддерживать, нет подсветки/линтинга шаблонов, любая правка = пересборка генератора.
- **Новый kind → тихое выпадение ресурса** (см. п.2).
- `data_type: json``IsJson`+`JsonNormalize` plan-modifier — но **0 тестов** на это, а нормализация JSON исторически проблемная (V2→V6 цикл, баг 13).
- Immutable определяется только через отсутствие в modify — если API *временно* не отдаёт modify-параметр (сбой/неполный YAML), параметр ошибочно станет ForceNew → пересоздание ресурса.
**Предложения.**
- Вынести шаблоны в `embed.FS` (`//go:embed templates/*.tmpl`) — поддерживаемость без потери single-binary.
- Fail fast на неизвестном kind + лог числа сгенерированных ресурсов на сервис (детект «пропал ресурс»).
- Снапшот-тесты генератора: эталонный YAML → ожидаемый `.go` (golden files).
- Защита от «исчезнувшего modify-параметра»: предупреждать, если у сервиса есть create-params, но 0 modify-params (подозрительно).
**Риск.** Если `gen_v2` сломается — ломается **весь** Universal-провайдер. Сейчас единственная страховка — `format.Source`, который при ошибке всё равно пишет битый код.
---
## 6. Баги из истории — что системное, что осталось
**Системные классы (порождали серии багов):**
1. **Фильтрация deleted** (23, 22, 21) — API возвращает deleted-инстансы, провайдер их не отсеивал. **Закрыт 5.0.50** (`isDeleted=false` + `isInstanceDeleted()` + ошибка при >1 совпадении).
2. **Определение конца операции** (05, 02, 04) — зависание поллинга. **Закрыт** (критерий `dtFinish`).
3. **Динамические param ID** (create ID ≠ modify ID, 06) — **закрыт** runtime-discovery.
4. **Create-only параметры** (16) — **закрыт 5.0.38**.
**Не до конца решённые / ограничения:**
- **Нормализация map/json/list** — потребовала 5 итераций (V2→V6), помечена как *частично*; новые типы параметров могут снова всплыть.
- **Realm-валидация отключена** (21) из-за бага бэкенда `/resourceRealms/available` — валидации realm до деплоя нет.
- **FW-rules 500** (04) — **platform-side bug**, воспроизводится и в Cloud Console; провайдер не может починить.
- **Disk shrink** (06) — ограничение платформы (только увеличение), провайдер корректно прокидывает ошибку.
**Главная системная проблема** — именно фильтрация deleted была корнем 3+ багов. Сейчас закрыта, но **отсутствие тестов** означает, что регрессия не будет поймана автоматически.
**Предложения.** Regression-тесты на deleted-фильтрацию и multi-match; превратить known-limitations в явные диагностики (например, предупреждать про realm «валидация недоступна»).
---
## 7. Матрица состояний — 16 состояний
**Состояние.** `INSTANCE_STATES.md` / `STATE_TRANSITIONS.md` описывают полную матрицу. В коде adopt обрабатывает: `not_created`, `running`, `suspended`, `in_progress`/`pending`; остальные → общая ошибка.
**Плохо.** Хелперы `isStatusSuspended`/`isStatusNonAdoptable`/`isStatusNotCreated` работают через `strings.Contains` по тексту `explainedStatus`**хрупко**: изменение формулировки статуса в API сломает классификацию молча. Промежуточные (`creating`, `failed`, `error`) сваливаются в одну ветку без индивидуальных подсказок, хотя ARCHITECTURE.md требует разные диагностики.
**Предложения.** Завести enum состояний и единую функцию `classifyStatus(raw) → State`, маппинг raw→enum в одном месте, exhaustive `switch` по всем 16 (с `default → явная ошибка «неизвестный статус X»`). Тесты на каждый статус.
**Риск.** Строковое сопоставление — самое уязвимое место к молчаливому дрейфу API.
---
## 8. Тесты — достаточно ли
**Состояние.** **8 unit-тестов всего**: crud_test.go (3: `isStatusSuspended`, `isStatusNonAdoptable`, delete-default) и client_test.go (5: нормализация значений). Без моков, без integration.
**Не покрыто (критично):** adopt-логика, ref-валидация, polling/`waitForOperationFinish`, `FindInstanceByDisplayName` (deleted+multi-match), генератор целиком, JSON plan-modifier, suspend/resume, required-params compare.
**Предложения.**
- `httptest.Server` мок Nubes API → тесты Flow V6, поллинга по `dtFinish`, обрыва на шаге N, 429/503.
- Табличные тесты adopt-матрицы (все 16 состояний × `adopt_existing_on_create` true/false).
- Golden-тесты генератора (YAML→Go).
- Контракт-тесты на основе HAR (см. п. ниже).
**Риск.** Все закрытые системные баги (deleted, polling, param-ID) **не защищены от регрессии**. Любой рефакторинг ядра/генератора — рулетка.
---
## 9. Безопасность
**Состояние.**
- Токены: `*.token` в .gitignore ✅; private_key.asc в .gitignore ✅; id_ed25519.txt ✅.
- `InsecureSkipVerify` default = **false** ✅, включается только явно/`NUBES_INSECURE=true`. TLS 1.2 min ✅.
- `api_token` помечен `Sensitive: true` ✅.
**Хорошо.** Базовая гигиена соблюдена — ключи и токены не коммитятся, TLS-проверка по умолчанию включена.
**Плохо / проверить.**
- **GPG-ключ физически лежит в secrets** — да, в .gitignore, но стоит проверить `git log --all -- secrets/private_key.asc`, что он не попал в историю ранее. .gitignore не вычищает уже закоммиченное.
- public_key.asc, id_ed25519.pub — публичные, ок; но prod.token/`dev.token`/`test.token` существуют локально — убедиться, что покрыты `*.token` (да) и не было коммита до добавления правила.
- **Утечка токена в логи**: `formatAPIError` форматирует тело ответа API в ошибку — если API эхает заголовки/токен в body ошибки, он попадёт в диагностику Terraform. `doRequest` сам токен не логирует. Стоит маскировать `Bearer ...` в любых сообщениях.
- `ttyOut()` пишет напрямую в tty минуя Terraform — в debug-режиме `StageMsg` может содержать чувствительные данные; они идут в терминал в обход TF-логирования.
**Предложения.** `git log` аудит секретов; явная маскировка токена в `formatAPIError`/диагностиках; политика ротации `*.token`; вынести секреты из репо в внешний secret-store (для CI).
**Риск.** Если ключ/токен попал в git-историю до .gitignore — он уже скомпрометирован, .gitignore не поможет. Это надо проверить первым делом.
---
## 10. Конкурентность — два `terraform apply`
**Состояние.** Никаких блокировок. `FindInstanceByDisplayName` + `CreateGenericInstanceUniversalV6`**не атомарны**. `waitForInstanceIdle` ждёт `operationIsPending/InProgress`, но это не защищает от гонки create.
**Плохо.**
- Два apply с одинаковым `displayName` одновременно: оба проходят `FindInstanceByDisplayName` (никого нет) → оба `POST /instances`**два инстанса с одним именем**. После этого `FindInstanceByDisplayName` начнёт возвращать ошибку «найдено 2 инстанса» (баг 22 только *детектирует* это, но не предотвращает).
- Между modify из двух окружений — гонка на `instanceOperations`; частично гасится `waitForInstanceIdle`, но окно остаётся.
**Предложения.**
- Полагаться на **Terraform state locking** (backend lock) как первичную защиту — это ответственность пользователя, задокументировать.
- На стороне API — проверить, есть ли уникальность `displayName` на бэкенде; если нет, провайдер не может гарантировать атомарность.
- Минимально: после `POST /instances` сразу повторный `FindInstanceByDisplayName` и, если найдено >1, откатить свой (требует delete — осторожно).
**Риск.** Полноценная защита возможна только при поддержке со стороны API (уникальность имени или conditional create). Провайдер в одиночку гонку не закрывает.
---
## Сводный план улучшений (по приоритету)
**Steps**
**P0 — Надёжность runtime (блокеры для прода)**
1. Retry + backoff в `doRequest` для 429/503/сетевых сбоев (уважать `Retry-After`).
2. Обработка orphan-инстанса `not_created`: авто-cleanup при явном флаге вместо «удалите вручную».
3. Документировать и протестировать поведение при конкурентном apply (state-lock + повторная проверка после create).
**P1 — Защита от дрейфа и регрессий** (*parallel с P0*)
4. `validateSpec()` в генераторе + **fail fast на неизвестном kind**; убрать fallback на неформатированный код.
5. CI-шаг «генерация без diff» (детект дрейфа per ARCHITECTURE.md).
6. Тесты: `httptest` мок API (Flow V6, polling, обрыв, 429), табличные тесты adopt-матрицы, golden-тесты генератора, regression на deleted-фильтрацию.
**P2 — Снижение тех-долга**
7. Единый `classifyStatus → enum` для 16 состояний, exhaustive switch, убрать `strings.Contains`-классификацию.
8. Рефакторинг `adoptExistingInstanceOnCreate` (после тестов): вынести ref-валидацию в helper, отдельные диагностики для `creating`/`failed`.
9. Шаблоны генератора в `embed.FS`.
10. План вывода Legacy: матрица паритета ресурсов, `DEPRECATED.md`, целевая версия снятия.
**P3 — Безопасность и наблюдаемость**
11. `git log` аудит secrets; маскировка `Bearer`-токена в `formatAPIError`/диагностиках.
12. Realm-валидация — включить, когда бэкенд починит `/resourceRealms/available`.
13. HAR-based контракт-тесты (верификация соответствия провайдера реальному API).
**Relevant files**
- client.go — `doRequest` (retry), `CreateGenericInstanceUniversalV6` (orphan/идемпотентность), `FindInstanceByDisplayName` (конкурентность).
- crud.go — `adoptExistingInstanceOnCreate` (рефакторинг + тесты), статус-хелперы.
- generate_resources_v2.go — валидация YAML, fail-fast на kind, вынос шаблонов.
- provider.go — транспорт (keep-alive, маскировка токена).
- Тесты: рядом с crud.go и client.go.
**Decisions / scope**
- Это аналитический отчёт + план; **код не менялся** (режим Plan, и правила проекта запрещают изменения без «делай»).
- Большинство критических багов из истории **уже закрыты** (5.0.50). Главный системный риск сейчас — **отсутствие тестов** (регрессии не ловятся) и **дрейф API** (генератор/статусы молча ломаются).
**Further Considerations**
1. С чего начать реализацию? **A)** P0 (надёжность runtime) / **B)** P1 (тесты+защита от дрейфа — фундамент для безопасного рефакторинга) / **C)** аудит безопасности секретов (быстро, но критично, если ключ в истории git).
2. Вывод Legacy — делать сейчас параллельно или после стабилизации Universal тестами? Рекомендую **после** (нужен паритет VM/vApp, который без тестов рискован).
3. Нужен ли вам этот анализ как отдельный markdown-файл в docs (например `docs/50_history/25_opus_deep_analysis.md`), или достаточно ответа в чате?
Какой блок развернуть детальнее или с какого приоритета начинать?
Создано задач: 5
Список задач обновлен
## Общее мнение
**Архитектурно — сильное, зрелое решение.** Идея «YAML как source of truth → генерация Go-кода и доков» правильная и редко встречается в self-made провайдерах. Универсальное ядро без сервис-специфичной логики + декларативные спеки = можно добавить сервис без программиста. Это уровень выше типичного «руками написанного» провайдера.
**Что реально хорошо:**
- Выстраданный контракт с API (`dtFinish` для конца операции, динамические param-ID, фильтрация deleted) — видно, что прошли через боль и закрыли системные баги.
- Чистое разделение pipeline (API→YAML→Go→S3), профили dev/test/prod.
- Базовая безопасность в порядке (токены/ключи в .gitignore, TLS-проверка по умолчанию).
**Главные слабости (по убыванию):**
1. **Тесты почти отсутствуют** (8 unit-тестов, без моков, без integration). Все закрытые баги не защищены от регрессии — это самый большой риск.
2. **Генератор молча ломается** при дрейфе API (неизвестный kind тихо игнорируется, YAML не валидируется) — при этом сам же ARCHITECTURE.md требует «fail fast on drift». Декларация ≠ реализация.
3. **Нет транзакционности create и retry** — обрыв посередине оставляет orphan, transient-сбой рвёт apply.
4. **Два провайдера с дублированным ядром** — тех-долг, баги фиксятся в двух местах.
**Вердикт:** концепция и инженерная зрелость ядра — выше среднего, видно опытную руку. Но проект держится на «героике» (ручное тестирование на реальных стендах, комментарии «НЕ МЕНЯТЬ»), а не на автоматических гарантиях. Один слой тестов + fail-fast в генераторе превратили бы его из «работает, потому что мы знаем где не трогать» в по-настоящему надёжный. Сейчас это **крепкий прототип на пути к продакшн-grade**, а не законченный продукт.