Add image Makefile targets and Helm chart (HYPERFLEET-312) #14
Conversation
- Add image, image-push, image-dev Makefile targets for container builds - Update Dockerfile to multi-stage build (consistent with sentinel/adapter) - Add Helm chart with configurable PostgreSQL support: - Built-in PostgreSQL for development (default) - External database support for production (GCP Cloud SQL, etc.) - Update README with container image and Helm deployment docs 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughAdds a multi-stage Docker build that generates OpenAPI client code, compiles a static Go binary, and produces a distroless runtime image (binary at /app/hyperfleet-api, OPENAPI_SCHEMA_PATH set). Introduces Makefile targets/variables for building and pushing container images (including a personal-registry flow). Adds a Helm chart (charts/Chart.yaml, values.yaml, .helmignore) and templates (helpers, deployment, service, HPA, postgresql, serviceaccount, service) with conditional logic for autoscaling, DB init/migrations, env/config, probes, volumes, and RBAC. Expands README with image and Helm deployment instructions. Sequence Diagram(s)(Section intentionally omitted.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (5)📓 Common learnings📚 Learning: 2025-12-03T15:47:39.989ZApplied to files:
📚 Learning: 2025-12-03T15:47:39.989ZApplied to files:
📚 Learning: 2025-12-03T15:47:39.989ZApplied to files:
📚 Learning: 2025-12-03T15:47:39.989ZApplied to files:
🪛 YAMLlint (1.37.1)charts/templates/deployment.yaml[error] 6-6: syntax error: expected the node content, but found '-' (syntax) charts/templates/service.yaml[error] 6-6: syntax error: expected the node content, but found '-' (syntax) 🔇 Additional comments (7)
Comment |
VerificationBuildHelm InstallPods RunningIgnore the restarted pod 😸 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
257-259: Missing.PHONYdeclaration forrun-no-authtarget.The
run-no-authtarget (lines 257–259) is not marked as.PHONY, which means Make will only rebuild it if a file namedrun-no-authdoesn't exist in the working directory. This can lead to unexpected behavior where the target is skipped if such a file exists.Add
.PHONYdeclaration:run-no-auth: binary ./hyperfleet-api migrate ./hyperfleet-api serve --enable-authz=false --enable-jwt=false +.PHONY: run-no-auth
🧹 Nitpick comments (5)
charts/values.yaml (2)
27-39: Security context is hardened; consider enabling readOnlyRootFilesystem if application permits.Pod security context is well-configured:
runAsNonRoot: true,fsGroup: 65532,seccompProfile: RuntimeDefault. However,readOnlyRootFilesystem: false(line 37) allows the container to write to the root filesystem. If the application doesn't require write access to/, enabling read-only would improve security posture.If the application doesn't write to the filesystem:
securityContext: allowPrivilegeEscalation: false capabilities: drop: - ALL - readOnlyRootFilesystem: false + readOnlyRootFilesystem: true seccompProfile: type: RuntimeDefault
95-96: Persistence is disabled by default; data will be lost on pod restart.For development deployments with
persistence.enabled: false(line 96), the PostgreSQL database will lose all data when the pod restarts. This is acceptable for ephemeral dev environments, but users should understand this limitation.Consider adding a warning in the values comments:
persistence: + # ⚠️ WARNING: If disabled, database data is lost on pod restart enabled: false size: 1Gi storageClass: ""charts/templates/deployment.yaml (1)
30-35: Init container assumesncis present inbusybox:1.36Lines 32–34 rely on
nc -zbeing available inbusybox:1.36. If this image is ever swapped for a more minimal or differently built BusyBox, the init container could crash-loop becausencis missing.Consider either:
- Parameterizing the init image/command via values, or
- Using an image you control (or a standard postgres client) where the presence of the probe tool is guaranteed.
charts/templates/postgresql.yaml (2)
56-69: Ensure the PostgreSQL image version matches the project’s standardThe Postgres container image is driven by
.Values.database.postgresql.image(Line 56). Given the project guidance to use PostgreSQL 14.2 for DB operations, it would be good to ensure that this value (invalues.yamlor overlays) pins to a 14.2 image tag, or at least documents that expectation.If it’s not already pinned, consider updating the default image/tag or adding a brief comment in
values.yamlclarifying the expected version.Based on learnings, this keeps the chart aligned with the preferred Postgres version.
10-15: Plaintext dev DB credentials are acceptable but should stay dev‑onlyThe Secret uses
stringDatawithdb.useranddb.passwordpopulated directly from values. That’s fine for local/dev workflows, but make sure any real credentials are supplied via overrides or external secrets and that defaults invalues.yamlremain non‑sensitive.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Dockerfile(1 hunks)Makefile(3 hunks)README.md(1 hunks)charts/Chart.yaml(1 hunks)charts/templates/_helpers.tpl(1 hunks)charts/templates/deployment.yaml(1 hunks)charts/templates/hpa.yaml(1 hunks)charts/templates/postgresql.yaml(1 hunks)charts/templates/service.yaml(1 hunks)charts/templates/serviceaccount.yaml(1 hunks)charts/values.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-03T15:47:39.989Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T15:47:39.989Z
Learning: Applies to pkg/db/**/*.go : Use PostgreSQL 14.2 with GORM ORM for all database operations
Applied to files:
charts/templates/postgresql.yaml
📚 Learning: 2025-12-03T15:47:39.989Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T15:47:39.989Z
Learning: Applies to pkg/api/openapi/**/*.go : Use make generate to regenerate Go models from openapi/openapi.yaml via openapi-generator-cli v7.16.0 in Podman
Applied to files:
DockerfileMakefile
📚 Learning: 2025-12-03T15:47:39.989Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T15:47:39.989Z
Learning: Applies to openapi/openapi.yaml : TypeSpec definitions are maintained separately in hyperfleet-api-spec repository; only edit openapi/openapi.yaml in this repository for spec changes
Applied to files:
charts/values.yamlcharts/Chart.yaml
🪛 checkmake (0.2.2)
Makefile
[warning] 315-315: Missing required phony target "all"
(minphony)
🪛 YAMLlint (1.37.1)
charts/templates/serviceaccount.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/templates/postgresql.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/templates/service.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
charts/templates/hpa.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/templates/deployment.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (13)
README.md (1)
467-550: Documentation for container images and Helm deployment is clear and comprehensive.The section effectively guides users through both development (built-in PostgreSQL) and production (external database) deployment scenarios with concrete examples. Examples align well with the actual Makefile targets and Helm chart configuration.
charts/Chart.yaml (1)
1-15: Helm chart metadata is well-structured and complete.The chart includes all standard metadata (name, description, version, appVersion, maintainers, keywords, home). Versioning strategy (version: 1.0.0, appVersion: "1.0.0") is reasonable for an initial release.
charts/templates/serviceaccount.yaml (1)
1-12: Helm template structure is correct; static analysis false positive on Helm conditionals.The ServiceAccount template uses proper Helm conditional rendering and helper templates. The YAMLlint "syntax error" is a false positive—it doesn't understand Helm template syntax. The template will render correctly when processed by Helm.
Ensure the helper templates
hyperfleet-api.serviceAccountNameandhyperfleet-api.labelsare defined incharts/templates/_helpers.tpland match the references here.charts/templates/hpa.yaml (1)
1-32: HPA template is well-formed with proper conditional metric blocks.The HorizontalPodAutoscaler v2 uses correct API version and syntax. Conditional rendering for CPU and memory metrics (lines 16–31) allows flexible autoscaling configuration. The scaleTargetRef correctly targets the Deployment created by the chart. YAMLlint's "syntax error" is a false positive on Helm conditionals.
Verify that the Deployment template (
charts/templates/deployment.yaml) defines the same name viahyperfleet-api.fullnamehelper to ensure the HPA targets the correct Deployment.charts/templates/service.yaml (1)
1-15: Service template is well-formed; verify containerPort naming matches deployment.The Service exposes port 8000 via ClusterIP and routes to
targetPort: http(line 11). Ensure the Deployment template defines a containerPort withname: httpto match this selector.Verify that
charts/templates/deployment.yamldefines a containerPort named "http" to match the Service's targetPort.Dockerfile (1)
1-33: Multi-stage Dockerfile follows security and performance best practices.Excellent use of:
- Builder stage (Alpine 1.24): minimizes layer size and compilation overhead
- Distroless runtime (gcr.io/distroless/static-debian12:nonroot): removes package manager, shell, and uid 0 attack surface
- Static binary (CGO_ENABLED=0, ldflags -w -s): enables distroless compatibility and reduces final image size
- Layer caching strategy: go.mod/go.sum copied before source code
The resulting image will be minimal, hardened, and suitable for production Kubernetes deployments.
Makefile (2)
18-26: Image build targets are well-designed with good ergonomics and error handling.Configuration variables are clear (IMAGE_REGISTRY, IMAGE_NAME, IMAGE_TAG), and the
image-devtarget properly validates theQUAY_USERenvironment variable before attempting a build. The help text (lines 66–68) clearly documents the new targets and usage patterns.
300-330: Image targets follow best practices for Helm deployments.
make image: builds with full registry pathmake image-push: depends on image target to ensure it's built firstmake image-dev: validates QUAY_USER and clearly communicates the push destinationDefaults align with Helm
values.yaml(quay.io/openshift-hyperfleet/hyperfleet-api:latest), enabling seamless integration between build and deployment workflows.charts/templates/deployment.yaml (2)
1-29: Deployment metadata, labels, and basic spec look consistent with Helm best practicesName, labels, selectors, optional replica count, pod annotations, imagePullSecrets, service account, and pod security context wiring all look correct and consistent with the helpers. Nothing blocking here.
47-79: Env configuration and probes look reasonable for the API containerJWKS_URL is optional, JWT/AuthZ flags are configurable, extra envs are supported via
.Values.env, and liveness/readiness probes against/healthzon thehttpport are standard and should work fine as long as the app exposes that endpoint. No changes needed here from a chart perspective.charts/templates/postgresql.yaml (2)
1-100: PostgreSQL Secret, Service, Deployment, and persistence wiring look coherentThe conditional rendering on
.Values.database.postgresql.enabled, consistent naming ({{ fullname }}-postgresql), label/selector pairing,db.*secret keys,pg_isreadyprobes, and optional PVC/persistence hooks are all wired up sensibly. From a Helm/Kubernetes perspective this should behave as expected for the built‑in dev database.
95-119: PVC template and volume attachment look correct for optional persistenceThe PVC spec and its conditional wiring into the Deployment via the
datavolume (Lines 70–99, 102–119) look correct:ReadWriteOnce, configurable size, and optionalstorageClassName. No issues from a chart standpoint.charts/templates/_helpers.tpl (1)
1-62: Helper templates are idiomatic and provide a solid base for the chartThe
name,fullname,chart,labels,selectorLabels, andserviceAccountNamehelpers follow the usual Helm patterns and keep naming/labeling consistent across templates. This should make future templates and overrides much easier to maintain.
- Guard secrets volume against both DB modes being disabled - Use simple name for Service (hyperfleet-api) instead of fullname for predictable service discovery across components - Add JWKS URL override comment for custom identity providers - Disable enableAuthz by default (not required for MVP) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix bind addresses to use :PORT format for all interfaces - Add health and metrics ports to deployment - Fix health probe path from /healthz to /healthcheck - Add app.kubernetes.io/component label to differentiate from PostgreSQL - Add .helmignore to exclude .git from chart packaging - Add configurable server bind addresses in values.yaml 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
charts/.helmignore(1 hunks)charts/templates/_helpers.tpl(1 hunks)charts/templates/deployment.yaml(1 hunks)charts/values.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- charts/.helmignore
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/values.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T15:47:39.989Z
Learning: Applies to pkg/api/openapi/**/*.go : Use make generate to regenerate Go models from openapi/openapi.yaml via openapi-generator-cli v7.16.0 in Podman
📚 Learning: 2025-12-03T15:47:39.989Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T15:47:39.989Z
Learning: Applies to openapi/openapi.yaml : TypeSpec definitions are maintained separately in hyperfleet-api-spec repository; only edit openapi/openapi.yaml in this repository for spec changes
Applied to files:
charts/templates/deployment.yaml
🪛 YAMLlint (1.37.1)
charts/templates/deployment.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (1)
charts/templates/_helpers.tpl (1)
1-63: Helper templates follow standard Helm conventions and look solidThe name/fullname/chart/labels/selectorLabels/serviceAccountName helpers match the usual
helm createpatterns and give you consistent naming and labeling across templates. I don’t see any issues here.
- Add OpenAPI generation stage to Dockerfile (multi-stage build) - Add health (8083) and metrics (8080) ports to service.yaml - Add db-migrate init container to run migrations before serve - Add /tmp emptyDir volume for readOnlyRootFilesystem support - Enable readOnlyRootFilesystem: true in security context 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy openapi/openapi.yaml into the runtime container and set OPENAPI_SCHEMA_PATH environment variable to enable schema validation in production deployments. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Following the latest changes, deploying with the helm chart Sentinel and API all running good 😸 |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yasun1 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 67e2059
into
openshift-hyperfleet:main
3 participants
Summary
make image,make image-push, andmake image-devMakefile targets for container buildsTest plan
make imagebuilds container image successfullymake image-devbuilds and pushes to personal Quay registryhelm templaterenders valid Kubernetes manifests🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.