Skip to content

Conversation

@jcabrero
Copy link
Member

@jcabrero jcabrero commented May 19, 2025

After Phase 0 finishes, there will be a transition period where API keys and NUCs may live together. In this PR, we duplicate api and postgres containers and add caddy redirection under /nuc subdomain to that NUC instances. This makes it so that users can choose to connect to nilAI through both NUCs and API keys during the transition period where both NUCs and API Keys are both working.

Furthermore, to make sure both authentication methods remain operational, we modified ci and tests so that both are passed.

@jcabrero jcabrero self-assigned this May 19, 2025
@jcabrero jcabrero force-pushed the feat/dual_authentication branch from 45976d4 to 973e20a Compare May 21, 2025 10:36
@jcabrero jcabrero force-pushed the feat/dual_authentication branch from 0be7c36 to 79b718f Compare May 22, 2025 10:03
@jcabrero jcabrero force-pushed the feat/dual_authentication branch from 9072f82 to 4bf1007 Compare May 22, 2025 15:19
@jcabrero jcabrero force-pushed the feat/dual_authentication branch from 4bf1007 to 53c075c Compare May 22, 2025 15:35
@jcabrero jcabrero requested a review from Copilot May 22, 2025 15:56
@jcabrero jcabrero marked this pull request as ready for review May 22, 2025 15:57
@jcabrero jcabrero requested review from lumasepa and mfontanini May 22, 2025 15:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enables a parallel transition period where API key and NUC authentication coexist by duplicating containers, updating routing, and adapting tests and CI to handle both methods.

  • Refactored E2E tests to use a unified api_key_getter, disable SSL verification in test clients, and add skips based on AUTH_STRATEGY.
  • Enhanced configuration (config.py) with a match on AUTH_STRATEGY to set BASE_URL and token retrieval dynamically.
  • Updated Docker Compose and Caddy to include NUC-specific nuc-postgres and nuc-api services with path-based routing, and expanded CI to run tests for both auth strategies.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/e2e/test_openai.py Updated fixtures to call api_key_getter(), added custom HTTP clients with verify=False, and skip markers.
tests/e2e/test_http.py Switched to api_key_getter(), added verify=False flags, improved assertion messages, and skip markers.
tests/e2e/config.py Introduced AUTH_STRATEGY, refactored BASE_URL and api_key_getter definitions via match.
scripts/wait_for_ci_services.sh Added health check logic for the new nilai-nuc-api container.
docker-compose.yml Added nuc-postgres and nuc-api services, volumes, and healthchecks for NUC support.
docker-compose.dev.yml Exposed nuc-api and nuc-postgres ports for local development.
caddy/Caddyfile Configured /nuc and /nuc/* routing to the nuc-api service.
.github/workflows/ci.yml Installed python3.12-dev, fixed typos, and added separate CI steps for NUC and API key E2E tests.
.env.ci Adjusted NILAI_GUNICORN_WORKERS and default AUTH_STRATEGY for CI.

@jcabrero jcabrero changed the base branch from feat/nuc_specific_rate_limits to main May 26, 2025 12:26
@jcabrero jcabrero merged commit a8f5257 into main May 26, 2025
4 checks passed
@jcabrero jcabrero deleted the feat/dual_authentication branch July 1, 2025 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants