[codex] Wire customer VM IPv6 provisioning

[Svaag]

Summary

Wires the Hyrule customer VM IPv6 provisioning settings into production deploy configuration and documents the tactical per-VM /64 model.

Depends on AS215932/hyrule-cloud#33 for the application behavior. After that PR merges, production promotion should happen through the existing promote-apps workflow with the merged Hyrule SHA, not by manually editing app pins.

Changes

• Add HYRULE_CUSTOMER_IPV6_SUPERNET, HYRULE_CUSTOMER_IPV6_GATEWAY, and HYRULE_CUSTOMER_IPV6_DNS to plain and Vault-rendered Hyrule env templates.
• Add those keys to the Vault render-hook required env contract and IaC tests.
• Run uv run alembic upgrade head before the deterministic Hyrule service restart.
• Document that rtr enX3 intentionally does not serve RA/DHCPv6 for paid VMs because one /64 per VM is handled through static XO networkConfig.
• Link the long-term proper IPAM issue: Set up proper IPAM for customer VM address allocation #346.

Validation

• uv run --with jinja2 --with pyyaml pytest tests/iac/test_mock_render.py tests/iac/test_vault_and_runner_contracts.py passed: 43 tests.
• scripts/ci/iac-static.sh passed. Local systemd/Caddy warnings were advisory from missing target binaries/plugins on the runner.
• git diff --check passed.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit dc921d8)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌ 33 - Not compliant Non-compliant requirements: None of the requirements are addressed in this PR 346 - Partially compliant Compliant requirements: Documents the tactical per-VM /64 model and links to the long-term IPAM issue Adds environment variables for supernet, gateway, and DNS that the tactical allocator will use Non-compliant requirements: No IPAM deployment or deployment plan No source-of-truth modeling of customer /48 or existing reservations No API/automation integration for Hyrule provisioning beyond env vars No auditability mechanism No reconciliation against DNS, inventory, or router config No migration path from tactical allocator to IPAM

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🏅 Score: 85

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Migration environment isolation The database migration step runs uv run alembic upgrade head with only PYTHONPATH set in the environment. It does not load the service's environment file (which contains HYRULE_DATABASE_URL and other secrets). If the migration relies on environment variables from the service's env file (e.g., database URL), it will fail at runtime because those variables are not available in the migration process's environment. The test only checks the command structure, not its actual execution. This could cause the deploy to silently skip migrations or fail during the health check phase. - name: Run hyrule-cloud database migrations ansible.builtin.command: cmd: /usr/local/bin/uv run alembic upgrade head chdir: "{{ hyrule_cloud_install_dir }}" become: true become_user: "{{ hyrule_cloud_app_user }}" environment: PYTHONPATH: "{{ hyrule_cloud_install_dir }}" changed_when: false when: hyrule_cloud_pyproject.stat.exists

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to dc921d8
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add missing stat task for condition The when condition references hyrule_cloud_pyproject.stat.exists, but no stat task to populate this fact is visible in the diff. Without a preceding ansible.builtin.stat task, this variable will be undefined and the task will fail. Add a stat task before the migration to check for the pyproject.toml file, or change the condition to a more reliable check such as ansible_facts.stat or file existence with a stat module. ansible/roles/hyrule_cloud/tasks/health.yml [13-22] +- name: Stat hyrule-cloud pyproject.toml + ansible.builtin.stat: + path: "{{ hyrule_cloud_install_dir }}/pyproject.toml" + register: hyrule_cloud_pyproject + - name: Run hyrule-cloud database migrations ansible.builtin.command: cmd: /usr/local/bin/uv run alembic upgrade head chdir: "{{ hyrule_cloud_install_dir }}" become: true become_user: "{{ hyrule_cloud_app_user }}" environment: PYTHONPATH: "{{ hyrule_cloud_install_dir }}" changed_when: false when: hyrule_cloud_pyproject.stat.exists Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the when condition references hyrule_cloud_pyproject.stat.exists without a preceding stat task, which would cause a failure. This is a critical bug that would break the playbook, so the high score is warranted. The improved_code accurately adds the missing stat task. However, the suggestion does not consider that the stat task might be defined elsewhere in the role (e.g., in a parent playbook), but based on the diff alone, it is a valid and important fix.	Medium

---

Previous suggestions

Suggestions up to commit dc921d8

Category	Suggestion	Impact
General	Add database URL env variable The migration task does not include the database connection environment variable (HYRULE_DATABASE_URL). Alembic will fail to connect if the database URL is not set in the process environment. Add the HYRULE_DATABASE_URL environment variable, sourced from the same vault/secret that the application uses, to ensure the migration can access the database. ansible/roles/hyrule_cloud/tasks/health.yml [13-22] - name: Run hyrule-cloud database migrations ansible.builtin.command: cmd: /usr/local/bin/uv run alembic upgrade head chdir: "{{ hyrule_cloud_install_dir }}" become: true become_user: "{{ hyrule_cloud_app_user }}" environment: PYTHONPATH: "{{ hyrule_cloud_install_dir }}" + HYRULE_DATABASE_URL: "{{ hyrule_database_url }}" changed_when: false when: hyrule_cloud_pyproject.stat.exists Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a critical missing environment variable (HYRULE_DATABASE_URL) that is required for Alembic to connect to the database. Without it, the migration task will fail at runtime. The improved code accurately reflects the change and aligns with the PR's context where HYRULE_DATABASE_URL is defined as a required env key.	Medium

[Svaag]

Reviewed PR-Agent suggestions:

• The HYRULE_DATABASE_URL migration concern is covered by task context: the migration command runs with chdir: {{ hyrule_cloud_install_dir }} after Vault has rendered /opt/hyrule-cloud/.env; Alembic's env.py instantiates HyruleConfig, which reads .env from that working directory. We should not duplicate the database secret into an Ansible variable.
• The ticket-compliance note for Set up proper IPAM for customer VM address allocation #346 is expected: this PR intentionally references the proper IPAM issue while wiring the tactical allocator/deploy path. It does not claim to implement the full IPAM acceptance criteria.

[github-actions]

Persistent review updated to latest commit dc921d8

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dc921d8c1e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Load the rendered env before running migrations

In production apply this new task runs outside systemd, so it does not inherit EnvironmentFile=/opt/hyrule-cloud/.env that the existing service ExecStartPre uses. The role only supplies HYRULE_DATABASE_URL via that rendered env, and uv run only loads env files when passed --env-file (checked uv help run), so Alembic will fail before the restart/health check whenever it reads the app DB URL. Please invoke Alembic with the rendered env file or keep relying on the systemd pre-start migration.

Useful? React with 👍 / 👎.