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

32 KiB
Raw Blame History

Что сказать 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 , lines 210 to 350

Searched for regex buildSubresourceForceNewCodes|computeCreateOnly|analyzeJsonPlanMod (tf_provider · universal_rebuild/tools/gen_v2/generate_resources_v2.go), 10 results

Read

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 , lines 2040 to 2150

Read

Read

Изучил архитектуру, ядро, генератор, историю багов и тесты. Сохраню анализ и представлю его.

Created memory file

Анализ 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 /instancesPOST /instanceOperationsGET cfsParams → resolve refs → POST каждого param → validate-cfsrunwaitForOperationFinish.

Хорошо. Завершение по 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-сбой на шаге 4–5 рвёт 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: jsonIsJson+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, а не законченный продукт.