Skip to content

refactor: use docker-compose with testcontainers#3476

Merged
chronark merged 15 commits intomainfrom
use-docker-compose-with-testcontainers
Jul 9, 2025
Merged

refactor: use docker-compose with testcontainers#3476
chronark merged 15 commits intomainfrom
use-docker-compose-with-testcontainers

Conversation

@chronark
Copy link
Collaborator

@chronark chronark commented Jul 8, 2025

This is a partial PR, I undid/changed most of it in the next one. sorry for the noise

What does this PR do?

Modernizes the testing infrastructure by replacing the custom container management with TestContainers, improving test reliability and maintainability. The changes include:

  • Migrates from custom container management to TestContainers for dynamic service management
  • Updates MySQL image from fixed version to latest in Dockerfile.mysql
  • Adds container_name attributes to services in docker-compose.yaml for better identification
  • Refactors the Makefile to use docker-compose for pulling and building images
  • Implements a new test harness with chaos testing capabilities for API containers
  • Simplifies container setup in tests by using standardized container access methods

Type of change

  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)

How should this be tested?

  • Run make test-unit to verify the new container management works correctly
  • Run make test-full to ensure integration tests pass with the new infrastructure
  • Verify that the API chaos testing capabilities work by running the integration tests

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Summary by CodeRabbit

  • New Features

    • Introduced two new services: AssetManagerd for VM asset management and Billaged for VM usage billing aggregation, each with full lifecycle management, observability, and secure multi-tenant support.
    • Added CLI tools for both services, enabling asset and billing operations from the command line.
    • Provided detailed documentation, environment templates, and systemd integration for easy deployment and configuration.
    • Implemented local storage backend for asset management, with extensible support for future backends.
  • Documentation

    • Comprehensive READMEs, changelogs, TODOs, and deployment guides added for all new services.
    • Included API references, architecture diagrams, operational instructions, and developer setup guides.
    • Added example Grafana dashboards and Prometheus integration documentation for monitoring.
  • Configuration

    • Environment variable templates and sample files provided for both services.
    • Makefiles included for build, test, install, and service management, supporting unified workflows and systemd integration.
  • Bug Fixes

    • Improved initialization and validation of metadata fields in identity-related tests to ensure consistent JSON object usage.
  • Style

    • Cleaned up formatting and removed redundant blank lines in documentation and workflow files.
  • Refactor

    • Streamlined test setup and container management in integration tests for reliability and maintainability.
  • Chores

    • Updated .gitignore files across new service directories to prevent accidental commits of sensitive or temporary files.

@changeset-bot
Copy link

changeset-bot bot commented Jul 8, 2025

⚠️ No Changeset found

Latest commit: 1b14fbb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Jul 8, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 9, 2025 0:09am
engineering ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 9, 2025 0:09am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 8, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

This change introduces the initial implementation of the Unkey Deploy system, adding new services (assetmanagerd, billaged, builderd), their clients, CLI tools, systemd units, observability, multi-tenant security, and supporting documentation. It also refactors dashboard onboarding and key creation flows, enhances UI components, updates Docker and Makefile orchestration, and improves test harnesses.

Changes

Files/Paths Change Summary
go/deploy/assetmanagerd/**, go/deploy/billaged/**, go/deploy/builderd/** New services: Added code, configs, Makefiles, systemd units, environment templates, changelogs, TODOs, documentation, and Go modules for assetmanagerd, billaged, and builderd. Includes service logic, client libraries, CLI tools, observability, and storage/aggregation backends.
go/deploy/Makefile, go/deploy/.gitignore, go/deploy/CLAUDE.md, go/deploy/LOCAL_DEPLOYMENT_GUIDE.md Deployment orchestration: New root Makefile for multi-service management, .gitignore, developer/AI guidelines, and a detailed local deployment guide.
deployment/Dockerfile.mysql, deployment/docker-compose.yaml Docker image tag for MySQL changed to latest; added explicit container_name for several services in compose file.
go/Makefile Refactored to use Docker Compose for orchestration; added up/down targets, migration retry logic, and updated test targets.
go/.golangci.yaml Updated linter config: new struct exclusions and explicit goconst thresholds.
go/go.mod Added/updated many indirect dependencies, reflecting new modules and tooling.
go/apps/api/integration/harness.go, go/apps/api/integration/http.go, go/apps/api/cancel_test.go, go/pkg/counter/redis_test.go Refactored API integration test harness to run API nodes in-process, simplified container setup, and improved URL handling.
go/apps/api/routes/v2_identities_*/**_test.go, go/apps/api/routes/v2_identities_create_identity/handler.go Test and handler updates: consistently initialize Meta field as empty JSON object ([]byte("{}")) instead of nil.
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/*-setup.tsx Updated *Setup components to accept overrideEnabled prop, conditionally rendering protection switches.
apps/dashboard/app/(app)/apis/[apiId]/_overview/components/controls/components/logs-search/index.tsx, ... (all logs-search/index.tsx in dashboard) Switched from local LogsLLMSearch to LLMSearch from @unkey/ui across all logs search UIs.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/selection-controls/index.tsx Replaced framer-motion-based animations with CSS-based keyframe animations for selection controls.
apps/dashboard/app/new-2/components/expandable-settings.tsx Added new ExpandableSettings component for toggleable settings panels.
apps/dashboard/app/new-2/hooks/use-key-creation-step.tsx Added new hook useKeyCreationStep for onboarding API key creation step with form logic and expandable settings.
apps/dashboard/app/new-2/page.tsx, apps/dashboard/app/new-2/constants.ts Refactored onboarding page to use new key creation step hook; updated step text.
apps/dashboard/app/new-2/components/onboarding-wizard.tsx Restructured onboarding wizard layout for scrollable content and fixed footer.
apps/dashboard/components/virtual-table/components/loading-indicator.tsx Enhanced LoadMoreFooter with collapsible/minimizable UI and animations.
apps/dashboard/components/logs/datetime/datetime-popover.tsx Changed date validation logic to discard out-of-range dates instead of clamping.
apps/engineering/content/design/components/search/llm-search.examples.tsx, apps/engineering/content/design/components/search/llm-search.mdx Added new documentation and example usages for LLMSearch UI component.
apps/engineering/content/design/components/*/*.mdx (multiple files) Removed or reworded introductory sections and improved component documentation clarity/structure.
Other new/modified files Added/updated configuration, environment templates, documentation, and supporting files for new services.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant DashboardUI
    participant API
    participant AssetManagerd
    participant Billaged
    participant Builderd
    participant Storage
    participant Registry

    User->>DashboardUI: Start onboarding/key creation
    DashboardUI->>API: Submit key creation form
    API->>AssetManagerd: Register asset (e.g. rootfs)
    AssetManagerd->>Registry: Store asset metadata
    AssetManagerd->>Storage: Store asset data
    AssetManagerd->>Builderd: (If needed) Trigger rootfs build
    Builderd->>AssetManagerd: Notify build completion
    AssetManagerd->>API: Respond with asset info
    API->>DashboardUI: Confirm key creation

    Note over AssetManagerd, Billaged: Periodically, Billaged aggregates VM usage from metald and reports billing stats.
Loading

Possibly related PRs

Suggested reviewers

  • imeyer
  • mcstepp
  • ogzhanolguncu
  • perkinsjr
  • Flo4604
  • MichaelUnkey
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Collaborator Author

chronark commented Jul 8, 2025

@vercel vercel bot temporarily deployed to Preview – engineering July 8, 2025 12:15 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard July 8, 2025 12:15 Inactive
@chronark chronark marked this pull request as ready for review July 8, 2025 12:17
@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2025

Thank you for following the naming conventions for pull request titles! 🙏

@chronark chronark changed the title refactor: use docker-compose with testcontainerrefactos refactor: use docker-compose with testcontainers Jul 8, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 11

🔭 Outside diff range comments (1)
go/go.mod (1)

53-318: Update container-related and Docker dependencies to latest patch versions and run a vulnerability scan.

The following modules have newer patch releases—please bump them in go/go.mod:

  • github.com/containerd/console v1.0.4 → v1.0.5
  • github.com/containerd/containerd/api v1.8.0 → v1.9.0
  • github.com/containerd/containerd/v2 v2.0.4 → v2.1.3
  • github.com/containerd/fuse-overlayfs-snapshotter/v2 v2.1.1 → v2.1.6
  • github.com/containerd/nydus-snapshotter v0.15.0 → v0.15.2
  • github.com/docker/cli v28.2.2 → v28.3.1
  • github.com/docker/cli-docs-tool v0.9.0 → v0.10.0
  • github.com/docker/compose/v2 v2.35.0 → v2.38.2
  • github.com/docker/docker v28.2.2 → v28.3.1
  • github.com/docker/docker-credential-helpers v0.8.2 → v0.9.3
  • github.com/moby/buildkit v0.20.1 → v0.23.2
  • github.com/moby/spdystream v0.4.0 → v0.5.0

After updating, run:

cd go
go mod tidy
go list -u -m all | grep -E "(containerd|docker|moby)"
go run golang.org/x/vuln/cmd/govulncheck@latest ./...

to verify no other updates are available and scan for known vulnerabilities. Integrating govulncheck into your CI pipeline is also recommended.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1821e10 and 94c216a.

⛔ Files ignored due to path filters (1)
  • go/go.sum is excluded by !**/*.sum
📒 Files selected for processing (26)
  • deployment/Dockerfile.mysql (1 hunks)
  • deployment/docker-compose.yaml (4 hunks)
  • go/.golangci.yaml (2 hunks)
  • go/Makefile (1 hunks)
  • go/apps/api/cancel_test.go (1 hunks)
  • go/apps/api/integration/harness.go (4 hunks)
  • go/apps/api/integration/http.go (2 hunks)
  • go/go.mod (5 hunks)
  • go/pkg/counter/redis_test.go (2 hunks)
  • go/pkg/hydra/test_helpers.go (1 hunks)
  • go/pkg/testutil/containers/api.go (0 hunks)
  • go/pkg/testutil/containers/clickhouse.go (0 hunks)
  • go/pkg/testutil/containers/constants.go (0 hunks)
  • go/pkg/testutil/containers/containers.go (1 hunks)
  • go/pkg/testutil/containers/doc.go (1 hunks)
  • go/pkg/testutil/containers/mysql.go (0 hunks)
  • go/pkg/testutil/containers/otel.go (0 hunks)
  • go/pkg/testutil/containers/redis.go (0 hunks)
  • go/pkg/testutil/containers/s3.go (0 hunks)
  • go/pkg/testutil/http.go (4 hunks)
  • go/pkg/testutil/testservices.go (1 hunks)
  • go/pkg/vault/integration/coldstart_test.go (1 hunks)
  • go/pkg/vault/integration/migrate_deks_test.go (1 hunks)
  • go/pkg/vault/integration/reencryption_test.go (1 hunks)
  • go/pkg/vault/integration/reusing_deks_test.go (2 hunks)
  • go/pkg/vault/storage/s3.go (2 hunks)
💤 Files with no reviewable changes (7)
  • go/pkg/testutil/containers/s3.go
  • go/pkg/testutil/containers/constants.go
  • go/pkg/testutil/containers/redis.go
  • go/pkg/testutil/containers/otel.go
  • go/pkg/testutil/containers/api.go
  • go/pkg/testutil/containers/mysql.go
  • go/pkg/testutil/containers/clickhouse.go
🧰 Additional context used
🧠 Learnings (2)
deployment/Dockerfile.mysql (1)
Learnt from: chronark
PR: unkeyed/unkey#3426
File: deployment/init-databases.sql:0-0
Timestamp: 2025-07-07T13:22:10.960Z
Learning: The database initialization script in deployment/init-databases.sql is specifically for local development environments, not production. The permissive settings (wildcard host '%' and ALL PRIVILEGES) are intentionally used for local dev convenience.
go/pkg/hydra/test_helpers.go (1)
Learnt from: chronark
PR: unkeyed/unkey#3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
🧬 Code Graph Analysis (7)
go/pkg/vault/integration/reencryption_test.go (2)
go/pkg/testutil/containers/containers.go (2)
  • S3 (153-160)
  • S3Config (18-34)
go/pkg/vault/storage/s3.go (2)
  • NewS3 (33-73)
  • S3Config (25-31)
go/pkg/vault/integration/migrate_deks_test.go (2)
go/pkg/testutil/containers/containers.go (2)
  • S3 (153-160)
  • S3Config (18-34)
go/pkg/vault/storage/s3.go (2)
  • NewS3 (33-73)
  • S3Config (25-31)
go/pkg/hydra/test_helpers.go (1)
go/pkg/testutil/containers/containers.go (1)
  • MySQL (88-99)
go/pkg/counter/redis_test.go (1)
go/pkg/testutil/containers/containers.go (1)
  • Redis (109-111)
go/apps/api/cancel_test.go (1)
go/pkg/testutil/containers/containers.go (2)
  • MySQL (88-99)
  • Redis (109-111)
go/pkg/testutil/http.go (5)
go/pkg/zen/server.go (1)
  • Server (22-33)
go/pkg/zen/validation/validator.go (1)
  • Validator (24-26)
go/pkg/sim/simulation.go (1)
  • Validator (13-13)
go/pkg/testutil/containers/containers.go (6)
  • StartAllServices (67-70)
  • MySQL (88-99)
  • Redis (109-111)
  • ClickHouse (130-132)
  • S3 (153-160)
  • S3Config (18-34)
go/pkg/vault/storage/s3.go (2)
  • NewS3 (33-73)
  • S3Config (25-31)
go/pkg/testutil/testservices.go (1)
go/pkg/testutil/containers/containers.go (7)
  • MySQL (88-99)
  • Redis (109-111)
  • ClickHouse (130-132)
  • S3 (153-160)
  • S3Config (18-34)
  • OTEL (181-187)
  • OTELConfig (41-53)
🪛 Checkov (3.2.334)
deployment/Dockerfile.mysql

[LOW] 1-1: Ensure the base image uses a non latest version tag

(CKV_DOCKER_7)


[LOW] 1-9: Ensure that HEALTHCHECK instructions have been added to container images

(CKV_DOCKER_2)


[LOW] 1-9: Ensure that a user for the container has been created

(CKV_DOCKER_3)

🪛 Hadolint (2.12.0)
deployment/Dockerfile.mysql

[warning] 1-1: Using latest is prone to errors if the image will ever update. Pin the version explicitly to a release tag

(DL3007)

🪛 checkmake (0.2.2)
go/Makefile

[warning] 13-13: Target body for "up" exceeds allowed length of 5 (12).

(maxbodylength)


[warning] 1-1: Missing required phony target "all"

(minphony)


[warning] 1-1: Missing required phony target "clean"

(minphony)


[warning] 1-1: Missing required phony target "test"

(minphony)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
  • GitHub Check: Test Go API Local / Test (Shard 6/8)
  • GitHub Check: Test Go API Local / Test (Shard 3/8)
  • GitHub Check: Build / Build
  • GitHub Check: Test Go API Local / Test (Shard 4/8)
  • GitHub Check: Test Agent Local / test_agent_local
  • GitHub Check: Test Go API Local / Test (Shard 8/8)
  • GitHub Check: Test Go API Local / Test (Shard 7/8)
  • GitHub Check: Test Go API Local / Test (Shard 1/8)
  • GitHub Check: Test Go API Local / Test (Shard 5/8)
  • GitHub Check: Test Go API Local / Test (Shard 2/8)
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Test Packages / Test ./packages/cache
  • GitHub Check: Test Packages / Test ./packages/api
  • GitHub Check: Test Packages / Test ./packages/rbac
  • GitHub Check: Test Packages / Test ./packages/nextjs
  • GitHub Check: Test Packages / Test ./apps/dashboard
  • GitHub Check: Test Packages / Test ./internal/resend
  • GitHub Check: Test Packages / Test ./packages/hono
  • GitHub Check: Test Packages / Test ./internal/id
  • GitHub Check: Test Packages / Test ./internal/keys
  • GitHub Check: Test Packages / Test ./internal/hash
  • GitHub Check: Test Packages / Test ./internal/clickhouse
  • GitHub Check: Test Packages / Test ./internal/billing
  • GitHub Check: Test Packages / Test ./internal/encryption
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (26)
go/.golangci.yaml (2)

73-74: LGTM - Redis client exclusions align with container refactoring.

The addition of Redis client option exclusions from the exhaustruct linter is consistent with the move to static container configurations described in the PR objectives.


96-98: LGTM - Reasonable goconst configuration.

The minimum constant length of 4 characters and minimum occurrence count of 5 are sensible defaults for detecting repeated constants.

go/pkg/counter/redis_test.go (2)

17-17: LGTM - Simplified Redis container setup.

The change from container manager instantiation to direct containers.Redis(t) call aligns with the PR's goal of simplifying container management by relying on externally managed docker-compose services.


327-327: LGTM - Consistent Redis container setup.

Same beneficial simplification as in the previous test function.

go/pkg/vault/integration/migrate_deks_test.go (2)

25-25: LGTM - Simplified S3 container setup.

The change from container manager instantiation to direct containers.S3(t) call is consistent with the PR's container management simplification.


30-30: LGTM - Corrected field name capitalization.

The change from S3AccessKeyId to S3AccessKeyID follows Go naming conventions where "ID" should be capitalized as a complete acronym.

go/pkg/vault/storage/s3.go (2)

28-28: LGTM - Corrected field name capitalization.

The change from S3AccessKeyId to S3AccessKeyID follows Go naming conventions where "ID" should be capitalized as a complete acronym.


51-51: LGTM - Updated field usage to match renamed struct field.

The usage of the field is correctly updated to match the renamed struct field.

go/pkg/hydra/test_helpers.go (1)

22-23: LGTM! Clean simplification of container setup.

The migration from explicit container management to the simplified containers.MySQL(t) call effectively reduces test complexity while maintaining the same functionality. The approach aligns well with using external docker-compose services.

go/pkg/vault/integration/coldstart_test.go (2)

22-22: LGTM! Simplified S3 container setup.

The migration from explicit container management to containers.S3(t) follows the same pattern as other test files and aligns with the docker-compose-based approach.


29-29: Good catch! Field name corrected to follow Go naming conventions.

The change from S3AccessKeyId to S3AccessKeyID properly follows Go naming conventions for acronyms and matches the updated field name in the storage.S3Config struct.

go/pkg/vault/integration/reencryption_test.go (1)

25-25: LGTM! Consistent changes align with the container refactor.

The S3 container setup simplification and field name correction (S3AccessKeyIdS3AccessKeyID) are consistent with the changes in other vault integration tests, demonstrating a systematic approach to the refactor.

Also applies to: 30-30

go/apps/api/cancel_test.go (1)

22-22: LGTM! Consistent simplification of MySQL and Redis container setup.

The migration to containers.MySQL(t) and containers.Redis(t) effectively removes explicit container management complexity while maintaining the same test functionality. This aligns well with the docker-compose-based approach adopted across the codebase.

Also applies to: 25-25

go/pkg/vault/integration/reusing_deks_test.go (1)

24-24: LGTM! Consistent S3 container setup changes applied to both test functions.

The S3 container setup simplification and field name correction (S3AccessKeyIdS3AccessKeyID) are consistently applied across both test functions, maintaining the same systematic approach to the container refactor seen in other vault integration tests.

Also applies to: 29-29, 67-67, 72-72

go/apps/api/integration/http.go (1)

65-69: LGTM! Improved URL scheme handling.

The updated logic correctly handles cases where addr already includes a scheme, preventing duplicate prefixes like "http://http://...". This defensive approach improves compatibility with the new in-process API server cluster architecture.

deployment/docker-compose.yaml (1)

93-93: LGTM! Explicit container naming improves predictability.

Adding explicit container_name attributes provides consistent, predictable naming that aligns with the service names. This supports the migration to static configuration by ensuring containers can be reliably referenced by name.

Also applies to: 99-99, 122-122, 152-152, 174-174

go/pkg/testutil/containers/doc.go (1)

1-91: Excellent comprehensive documentation!

The rewritten documentation provides clear guidance on the package's new static configuration approach. The explanation of design decisions (hardcoded ports vs. dynamic discovery), usage examples, and prerequisites makes the package easily adoptable. The distinction between host and docker configurations is particularly valuable.

go/pkg/testutil/http.go (2)

58-66: LGTM! Simplified harness initialization.

The refactoring successfully removes dynamic container management complexity in favor of static configurations. Using containers.StartAllServices(t) as a no-op and the simplified accessor functions (MySQL(t), Redis(t)) makes the code cleaner and more maintainable.


137-137: Good fix for Go naming convention.

Correcting S3AccessKeyId to S3AccessKeyID properly follows Go naming conventions where "ID" should be capitalized as a whole when it's an acronym.

go/pkg/testutil/containers/containers.go (4)

9-54: Well-structured configuration types with excellent documentation.

The S3Config and OTELConfig structs are well-designed with clear field documentation that explains the dual URL pattern for host vs docker connections. The field naming follows Go conventions correctly.


72-99: Well-implemented MySQL configuration with good practices.

The function correctly configures MySQL with ParseTime enabled and uses NopLogger to reduce test output noise. The empty DBName with clear documentation is a good practice to avoid test conflicts.


101-132: Simple and effective service configuration functions.

Both Redis and ClickHouse functions provide appropriate connection strings with good documentation. The ClickHouse DSN includes appropriate security settings for testing environments.


134-187: Comprehensive service configurations with excellent documentation.

Both S3 and OTEL functions return complete configurations with all necessary endpoints and credentials. The documentation includes helpful usage examples, and the credentials properly match the docker-compose setup.

go/Makefile (2)

6-11: Good improvements to fmt and pull targets.

Running golangci-lint after formatting ensures code quality, and using docker compose pull is more maintainable than managing individual image pulls.


30-37: Well-configured test targets with proper dependencies.

Both test targets correctly depend on the up target to ensure services are running. Good practices include race detection for unit tests and appropriate timeout settings.

go/apps/api/integration/harness.go (1)

253-259: Properly implemented getter method.

Good defensive programming by checking for nil apiCluster.

Comment on lines +227 to +251
// StopContainer stops a specific API container (for chaos testing)
func (h *Harness) StopContainer(index int) error {
if h.apiCluster == nil || index >= len(h.apiCluster.Resources) {
return fmt.Errorf("invalid container index: %d", index)
}

pool, err := dockertest.NewPool("")
if err != nil {
return err
}
return pool.Client.StopContainer(h.apiCluster.Resources[index].Container.ID, 10)
}

// StartContainer starts a stopped API container (for chaos testing)
func (h *Harness) StartContainer(index int) error {
if h.apiCluster == nil || index >= len(h.apiCluster.Resources) {
return fmt.Errorf("invalid container index: %d", index)
}

pool, err := dockertest.NewPool("")
if err != nil {
return err
}
return pool.Client.StartContainer(h.apiCluster.Resources[index].Container.ID, nil)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

⚠️ Potential issue

Chaos testing methods are broken - they reference non-existent Docker containers.

The StopContainer and StartContainer methods attempt to control Docker containers, but the API nodes now run in-process as goroutines, not as Docker containers. These methods will fail if called.

Either remove these methods entirely or reimplement them to work with in-process servers. For example, you could:

  1. Remove the methods if chaos testing is no longer needed:
-// StopContainer stops a specific API container (for chaos testing)
-func (h *Harness) StopContainer(index int) error {
-	// ... entire method
-}
-
-// StartContainer starts a stopped API container (for chaos testing) 
-func (h *Harness) StartContainer(index int) error {
-	// ... entire method
-}
  1. Or reimplement to control in-process servers (would require significant refactoring to track contexts/cancel functions for each server)

Would you like me to help implement a proper chaos testing mechanism for in-process servers, or should these methods be removed?

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// StopContainer stops a specific API container (for chaos testing)
func (h *Harness) StopContainer(index int) error {
if h.apiCluster == nil || index >= len(h.apiCluster.Resources) {
return fmt.Errorf("invalid container index: %d", index)
}
pool, err := dockertest.NewPool("")
if err != nil {
return err
}
return pool.Client.StopContainer(h.apiCluster.Resources[index].Container.ID, 10)
}
// StartContainer starts a stopped API container (for chaos testing)
func (h *Harness) StartContainer(index int) error {
if h.apiCluster == nil || index >= len(h.apiCluster.Resources) {
return fmt.Errorf("invalid container index: %d", index)
}
pool, err := dockertest.NewPool("")
if err != nil {
return err
}
return pool.Client.StartContainer(h.apiCluster.Resources[index].Container.ID, nil)
}
🤖 Prompt for AI Agents
In go/apps/api/integration/harness.go around lines 227 to 251, the StopContainer
and StartContainer methods are broken because they try to control Docker
containers that no longer exist; the API nodes now run as in-process goroutines.
To fix this, either remove these methods entirely if chaos testing is no longer
needed, or refactor the harness to track and manage the in-process server
instances with contexts and cancel functions to enable stopping and starting
them programmatically. Decide which approach fits the current testing strategy
and implement accordingly.

Comment on lines +121 to +126
// RunAPI creates a cluster of API containers for chaos testing
func (h *Harness) RunAPI(config ApiConfig) *ApiCluster {
cluster := &ApiCluster{
Addrs: make([]string, config.Nodes),
Resources: make([]*dockertest.Resource, config.Nodes), // Not used but kept for compatibility
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove Resources initialization if not used.

Following up on the previous comment about removing the Resources field.

 cluster := &ApiCluster{
 	Addrs:     make([]string, config.Nodes),
-	Resources: make([]*dockertest.Resource, config.Nodes), // Not used but kept for compatibility
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// RunAPI creates a cluster of API containers for chaos testing
func (h *Harness) RunAPI(config ApiConfig) *ApiCluster {
cluster := &ApiCluster{
Addrs: make([]string, config.Nodes),
Resources: make([]*dockertest.Resource, config.Nodes), // Not used but kept for compatibility
}
// RunAPI creates a cluster of API containers for chaos testing
func (h *Harness) RunAPI(config ApiConfig) *ApiCluster {
cluster := &ApiCluster{
Addrs: make([]string, config.Nodes),
}
🤖 Prompt for AI Agents
In go/apps/api/integration/harness.go around lines 121 to 126, the Resources
field in ApiCluster is initialized but not used. Remove the initialization of
the Resources slice from the RunAPI function and also remove the Resources field
from the ApiCluster struct definition to clean up unused code.

ogzhanolguncu and others added 4 commits July 8, 2025 13:12
* feat: add new expandable configs

* feat: add form submit with conditionals

* feat: add description for each section and disable form

---------

Co-authored-by: James P <james@unkey.dev>
* refactor: add minimized load more

* chore: fmt

* refactor: get rid of useEffect animation

* chore: fmt
* llm search moved with docs and replaced in dashboard

* eng docs change

* [autofix.ci] apply automated fixes

* minor refactor from c-rabbit

* rabbit

* [autofix.ci] apply automated fixes

* rabbit

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: James P <james@unkey.dev>
* fix: first time selection

* fix: resetting to initial date of the month

* fix: date validation issue

* fix: day selection by bypassing react-day-picker
chronark and others added 6 commits July 8, 2025 19:19
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* feat: Initial implementation of Unkey Deploy

- Introduce four core services: assetmanagerd, billaged, builderd, metald
- Implement VM lifecycle management with Firecracker/Cloud Hypervisor support
- Add SPIFFE/SPIRE integration for mTLS inter-service communication
- Include systemd service files and CLI tools for each service
- Set up observability with OpenTelemetry and Grafana LGTM stack
- Implement tenant isolation, resource management, and usage billing

Signed-off-by: Ian Meyer <k@imeyer.io>

* [autofix.ci] apply automated fixes

---------

Signed-off-by: Ian Meyer <k@imeyer.io>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Copy link
Collaborator Author

chronark commented Jul 9, 2025

yeah, I’ll add that back in

@graphite-app
Copy link

graphite-app bot commented Jul 9, 2025

Merge activity

  • Jul 9, 10:00 AM UTC: This pull request can not be added to the Graphite merge queue. Please try rebasing and resubmitting to merge when ready.
  • Jul 9, 10:00 AM UTC: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..

@chronark chronark merged commit 53902ef into main Jul 9, 2025
24 of 37 checks passed
@chronark chronark deleted the use-docker-compose-with-testcontainers branch July 9, 2025 12:08
@vercel vercel bot temporarily deployed to Preview – engineering July 9, 2025 12:09 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard July 9, 2025 12:09 Inactive
chronark added a commit that referenced this pull request Jul 9, 2025
* feat: get rid of testcontainers and just rawdog docker-compose

* feat: onboarding key creation (#3459)

* feat: add new expandable configs

* feat: add form submit with conditionals

* feat: add description for each section and disable form

---------

Co-authored-by: James P <james@unkey.dev>

* refactor: add minimized load more (#3399)

* refactor: add minimized load more

* chore: fmt

* refactor: get rid of useEffect animation

* chore: fmt

* chore: llm search moved to ui (#3409)

* llm search moved with docs and replaced in dashboard

* eng docs change

* [autofix.ci] apply automated fixes

* minor refactor from c-rabbit

* rabbit

* [autofix.ci] apply automated fixes

* rabbit

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: James P <james@unkey.dev>

* fix: calendar bug (#3478)

* fix: first time selection

* fix: resetting to initial date of the month

* fix: date validation issue

* fix: day selection by bypassing react-day-picker

* feat: return permission slugs (#3481)

* chore(release): version packages (#3482)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* feat: add listPermissions and listRoles (#3483)

* chore(release): version packages (#3484)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* feat: Initial implementation of Unkey Deploy (#3488)

* feat: Initial implementation of Unkey Deploy

- Introduce four core services: assetmanagerd, billaged, builderd, metald
- Implement VM lifecycle management with Firecracker/Cloud Hypervisor support
- Add SPIFFE/SPIRE integration for mTLS inter-service communication
- Include systemd service files and CLI tools for each service
- Set up observability with OpenTelemetry and Grafana LGTM stack
- Implement tenant isolation, resource management, and usage billing

Signed-off-by: Ian Meyer <k@imeyer.io>

* [autofix.ci] apply automated fixes

---------

Signed-off-by: Ian Meyer <k@imeyer.io>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* fix: cast JSON for interpolate params (#3492)

* refactor: use docker-compose with testcontainers (#3476)

* refactor: use docker-compose with testcontainerrefactos

* feat: get rid of testcontainers and just rawdog docker-compose

* feat: onboarding key creation (#3459)

* feat: add new expandable configs

* feat: add form submit with conditionals

* feat: add description for each section and disable form

---------

Co-authored-by: James P <james@unkey.dev>

* refactor: add minimized load more (#3399)

* refactor: add minimized load more

* chore: fmt

* refactor: get rid of useEffect animation

* chore: fmt

* chore: llm search moved to ui (#3409)

* llm search moved with docs and replaced in dashboard

* eng docs change

* [autofix.ci] apply automated fixes

* minor refactor from c-rabbit

* rabbit

* [autofix.ci] apply automated fixes

* rabbit

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: James P <james@unkey.dev>

* fix: calendar bug (#3478)

* fix: first time selection

* fix: resetting to initial date of the month

* fix: date validation issue

* fix: day selection by bypassing react-day-picker

* feat: return permission slugs (#3481)

* chore(release): version packages (#3482)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* feat: add listPermissions and listRoles (#3483)

* chore(release): version packages (#3484)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* feat: Initial implementation of Unkey Deploy (#3488)

* feat: Initial implementation of Unkey Deploy

- Introduce four core services: assetmanagerd, billaged, builderd, metald
- Implement VM lifecycle management with Firecracker/Cloud Hypervisor support
- Add SPIFFE/SPIRE integration for mTLS inter-service communication
- Include systemd service files and CLI tools for each service
- Set up observability with OpenTelemetry and Grafana LGTM stack
- Implement tenant isolation, resource management, and usage billing

Signed-off-by: Ian Meyer <k@imeyer.io>

* [autofix.ci] apply automated fixes

---------

Signed-off-by: Ian Meyer <k@imeyer.io>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* fix: cast JSON for interpolate params (#3492)

* refactor: use docker-compose with testcontainerrefactos

* feat: get rid of testcontainers and just rawdog docker-compose

---------

Signed-off-by: Ian Meyer <k@imeyer.io>
Co-authored-by: Oğuzhan Olguncu <21091016+ogzhanolguncu@users.noreply.github.com>
Co-authored-by: James P <james@unkey.dev>
Co-authored-by: CodeReaper <148160799+MichaelUnkey@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Ian Meyer <imeyer@users.noreply.github.com>
Co-authored-by: Flo <53355483+Flo4604@users.noreply.github.com>

* fix: clean up

---------

Signed-off-by: Ian Meyer <k@imeyer.io>
Co-authored-by: Oğuzhan Olguncu <21091016+ogzhanolguncu@users.noreply.github.com>
Co-authored-by: James P <james@unkey.dev>
Co-authored-by: CodeReaper <148160799+MichaelUnkey@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Ian Meyer <imeyer@users.noreply.github.com>
Co-authored-by: Flo <53355483+Flo4604@users.noreply.github.com>
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.

5 participants