Skip to content

Conversation

@rh-amarin
Copy link
Contributor

@rh-amarin rh-amarin commented Dec 17, 2025

In order to test other CLM components they need a working hyperfleet-api which adds friction to the experience

Adding a make dev target so it:

  • Starts a testcontainer with PostgreSQL
  • Does hyperfleet-api migrate on the container
  • Starts the hyperfleet-api with --enable-auth=false --enable-jwt=false

Summary by CodeRabbit

  • New Features

    • Added make dev command for local development, which runs the API with an embedded PostgreSQL database via testcontainers, with JWT and authorization disabled for easier testing.
    • Implemented graceful shutdown handling for the API server.
  • Documentation

    • Added guides for the new local development workflow and embedded PostgreSQL setup.

✏️ Tip: You can customize this high-level summary in your review settings.

@openshift-ci
Copy link

openshift-ci bot commented Dec 17, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign crizzo71 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link

coderabbitai bot commented Dec 17, 2025

Walkthrough

This pull request introduces a new embedded development environment for local development that uses testcontainers to run a PostgreSQL database at runtime, bypassing JWT and authorization requirements. The changes add a Makefile target dev to launch the API with this environment, implement the corresponding environment configuration in Go, and update documentation. Graceful shutdown handling is added via signal interception (SIGINT/SIGTERM) to ensure proper resource cleanup of the testcontainer before exit.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Signal handling in cmd.go: Verify that environments.Environment().Teardown() is called correctly and that the graceful shutdown doesn't leave dangling processes or containers.
  • Environment overrides in e_embedded_development.go: Ensure all security feature disables (JWT, authz, HTTPS) are intentional and appropriate for development mode only.
  • Testcontainers integration: Confirm that the embedded PostgreSQL setup works reliably across different local environments and that migrations run correctly.

Suggested labels

lgtm, approved

Suggested reviewers

  • ciaranRoche
  • vkareh

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly aligns with the main objective: enabling a single make dev command for easy API development. It captures the core purpose of reducing friction for development workflows.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@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: 0

🧹 Nitpick comments (1)
cmd/hyperfleet-api/servecmd/cmd.go (1)

55-62: Consider graceful server shutdown before exit.

The signal handling correctly triggers environment teardown, but the servers (API, metrics, healthcheck) started in goroutines are not gracefully shut down. In-flight requests may be interrupted. While acceptable for the development workflow, consider implementing graceful server shutdown (e.g., using context cancellation or server.Shutdown() calls) to drain connections before exit.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0520ff0 and deae427.

📒 Files selected for processing (7)
  • Makefile (2 hunks)
  • cmd/hyperfleet-api/environments/e_embedded_development.go (1 hunks)
  • cmd/hyperfleet-api/environments/framework.go (1 hunks)
  • cmd/hyperfleet-api/environments/types.go (1 hunks)
  • cmd/hyperfleet-api/servecmd/cmd.go (2 hunks)
  • docs/development.md (2 hunks)
  • docs/testcontainers.md (1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T11:04:29.971Z
Learning: Applies to test/integration/**/*.go : Use Testcontainers to spin up isolated PostgreSQL instances for integration tests
📚 Learning: 2025-12-12T11:04:29.971Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T11:04:29.971Z
Learning: Applies to cmd/hyperfleet/environments/**/*.go : Use environment-based configuration via OCM_ENV to select development, unit_testing, integration_testing, or production settings

Applied to files:

  • cmd/hyperfleet-api/servecmd/cmd.go
  • cmd/hyperfleet-api/environments/types.go
  • cmd/hyperfleet-api/environments/framework.go
  • cmd/hyperfleet-api/environments/e_embedded_development.go
📚 Learning: 2025-12-12T11:04:29.971Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T11:04:29.971Z
Learning: Applies to test/integration/**/*.go : Use Testcontainers to spin up isolated PostgreSQL instances for integration tests

Applied to files:

  • Makefile
  • docs/testcontainers.md
  • docs/development.md
  • cmd/hyperfleet-api/environments/e_embedded_development.go
📚 Learning: 2025-12-12T11:04:29.971Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T11:04:29.971Z
Learning: Run make test-integration with OCM_ENV=integration_testing for tests using Testcontainers

Applied to files:

  • docs/testcontainers.md
  • docs/development.md
📚 Learning: 2025-12-12T11:04:29.971Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T11:04:29.971Z
Learning: Applies to pkg/api/openapi_embed.go : Embed OpenAPI specification at compile time using //go:embed directive

Applied to files:

  • cmd/hyperfleet-api/environments/types.go
📚 Learning: 2025-12-12T11:04:29.971Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T11:04:29.971Z
Learning: Mark OCM_ENV environment variable with the appropriate value (development, unit_testing, integration_testing, production) for each execution context

Applied to files:

  • cmd/hyperfleet-api/environments/types.go
🧬 Code graph analysis (2)
cmd/hyperfleet-api/servecmd/cmd.go (1)
cmd/hyperfleet-api/environments/framework.go (1)
  • Environment (54-56)
cmd/hyperfleet-api/environments/framework.go (1)
cmd/hyperfleet-api/environments/types.go (5)
  • DevelopmentEnv (15-15)
  • EmbeddedDevelopmentEnv (16-16)
  • UnitTestingEnv (13-13)
  • IntegrationTestingEnv (14-14)
  • ProductionEnv (17-17)
🔇 Additional comments (13)
cmd/hyperfleet-api/servecmd/cmd.go (1)

4-7: LGTM!

The signal handling imports are appropriate for implementing graceful shutdown.

docs/testcontainers.md (1)

7-15: LGTM!

The documentation clearly describes the embedded PostgreSQL setup for local development and how to use the new make dev command.

Makefile (2)

58-58: LGTM!

The help text clearly documents the new dev target and its purpose.


262-269: The Makefile comment is accurate. Migrations run automatically inside the testcontainer factory during initialization—NewTestcontainerFactory() calls Init(), which invokes db.Migrate() before returning the factory instance. No verification required.

docs/development.md (2)

94-102: LGTM!

The documentation clearly describes the embedded PostgreSQL workflow and its benefits for autonomous local development.


193-193: LGTM!

The build targets table correctly documents the new dev command.

cmd/hyperfleet-api/environments/types.go (1)

13-17: LGTM!

The new EmbeddedDevelopmentEnv constant follows the established naming convention and is properly integrated with existing environment types.

cmd/hyperfleet-api/environments/framework.go (1)

25-29: LGTM!

The new environment is properly registered in the environments map following the established pattern.

cmd/hyperfleet-api/environments/e_embedded_development.go (5)

8-14: LGTM!

The type definition and compile-time interface check follow Go best practices and are consistent with other environment implementations.


16-19: LGTM!

The database override correctly uses the testcontainer factory. Verification of automatic migration handling has been requested in the Makefile review.


21-26: LGTM!

Disabling security features (JWT, authz, HTTPS) is appropriate for the embedded development environment. The code comments clearly indicate this is for "local/dev usage" only.


28-38: LGTM!

The no-op override implementations are appropriate, allowing the environment to use default service, handler, and client initialization from the framework.


40-53: LGTM!

The flag defaults are well-suited for local development: verbose logging for debugging, security features disabled, mock OCM client to avoid external dependencies, and localhost binding for isolation.

@rh-amarin rh-amarin marked this pull request as draft December 17, 2025 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant