HYPERFLEET-509 - feat: Standardize Dockerfiles and Makefile for building images#59
HYPERFLEET-509 - feat: Standardize Dockerfiles and Makefile for building images#59xueli181114 wants to merge 1 commit intoopenshift-hyperfleet:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces the Docker builder base image with Red Hat UBI9 go-toolset, adds BUILD_DATE and VERSION build args, installs make in the builder, enforces non-root build/runtime users, sets GOBIN and retains module/build caches, and embeds version/commit/build-time via ldflags. The Makefile adds CONTAINER_TOOL detection (podman/docker), centralizes build metadata (GIT_SHA, GIT_DIRTY, VERSION, BUILD_DATE, LDFLAGS), derives IMAGE_TAG from VERSION, and introduces many new targets (image/image-push/image-dev, db/, generate/, test, lint, run/docs, etc.). Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Make as Makefile
participant CT as CONTAINER_TOOL (podman/docker)
participant Builder as Builder Container (ubi9/go-toolset)
participant Runtime as Runtime Image
participant Reg as Image Registry
Dev->>Make: invoke "make image" (VERSION, BUILD_DATE, GIT_SHA)
Make->>Make: compute metadata, set LDFLAGS, prepare build context
Make->>CT: request container build (builder stage)
CT->>Builder: start builder (ubi9/go-toolset)
Builder->>Builder: install make, set GOBIN, restore caches, go mod download
Builder->>Builder: build binary with CGO_ENABLED=0 and ldflags (GIT_SHA,GIT_DIRTY,BUILD_DATE,VERSION)
Builder-->>CT: output binary + OpenAPI schema
CT->>Runtime: assemble final image (copy binary, set non-root user, labels)
CT->>Reg: push image:IMAGE_TAG (if requested)
Reg-->>Dev: image available
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Line 4: Update the builder base image FROM to use Go 1.24 (replace
registry.access.redhat.com/ubi9/go-toolset:1.25 with
registry.access.redhat.com/ubi9/go-toolset:1.24) and change the build
environment to enable CGO and boringcrypto (set CGO_ENABLED=1 and add
GOEXPERIMENT=boringcrypto in the build stage where CGO is currently disabled);
also change the runtime BASE_IMAGE from the distroless nonroot image to a
glibc-based UBI minimal image (e.g.,
registry.access.redhat.com/ubi9/ubi-minimal:latest) so the binary built with
boringcrypto runs correctly at runtime.
In `@Makefile`:
- Line 259: The Makefile target that runs the Swagger UI uses $(container_tool)
run -d -p 80:8080 ... which fails for rootless Podman because ports <1024 are
privileged; update that docker/podman run invocation to map an unprivileged host
port (e.g., change -p 80:8080 to -p 8080:8080 or -p 8081:8080) so non-root users
can bind the port, leaving the rest of the command (environment var SWAGGER_JSON
and volume mount openapi/hyperfleet.yaml) unchanged.
|
|
||
| # OpenAPI generation stage | ||
| FROM golang:1.25 AS builder | ||
| FROM registry.access.redhat.com/ubi9/go-toolset:1.25 AS builder |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "Dockerfile" -type f 2>/dev/null | head -20Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 61
🏁 Script executed:
git ls-files | grep -i dockerfileRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 89
🏁 Script executed:
cat -n Dockerfile | head -35Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 1414
🌐 Web query:
registry.access.redhat.com ubi9 go-toolset 1.24 available tags versions
💡 Result:
For registry.access.redhat.com/ubi9/go-toolset (Go Toolset 1.24 stream), the tags follow Red Hat’s standard “floating stream + pinned patch + build-id” pattern:
-
Floating stream tags (recommended if you want 1.24.x updates automatically):
1.24latest(typically points at the current supported stream/build)
-
Pinned patch tags (example currently shown in Red Hat’s catalog for Go Toolset 1.24 images):
1.24.6(patch-level pin) [1]
-
Build-specific tags (examples from the catalog; these pin to a specific rebuild):
1.24-17618418781.24.6-1761841878[1]
Independent confirmation that the 1.24 tag is in active use for the UBI9 Go Toolset image can be seen in OpenShift release content referencing an update to registry.access.redhat.com/ubi9/go-toolset “Docker tag to v1.24”. [2]
If you need the exact current full tag list as returned by the registry, the usual way is:
skopeo list-tags docker://registry.access.redhat.com/ubi9/go-toolsetSources: [1] [2]
🌐 Web query:
Go 1.24.9 release golang version availability
💡 Result:
Go 1.24.9 was an official Go patch release, released on October 13, 2025 [1]. It’s still available for download (source tarball and installers/archives for multiple OS/architectures) on the official downloads page at go.dev/dl under “go1.24.9” [2].
Note: Go 1.24.9 is not the latest 1.24 patch anymore (the 1.24 line continued past it) [1].
Sources: [1] [2]
🌐 Web query:
registry.access.redhat.com ubi9 ubi-micro FIPS glibc boringcrypto
💡 Result:
- UBI 9 images (including
ubi9/ubi-micro) are RHEL 9 userspace, i.e., glibc-based (not musl). Example from a UBI9 container shows glibc 2.34. [1] - RHEL 9 FIPS behavior is OpenSSL 3 provider-based: when the kernel FIPS flag is set, OpenSSL automatically loads the FIPS provider and restricts algorithms accordingly. [2]
- Go “boringcrypto” / FIPS-capable builds on RHEL (Red Hat Go Toolset) rely on CGO + glibc
dlopen+ OpenSSL at runtime, and require dynamic linking with glibc (not static). [3] - Practical impact for
ubi9/ubi-micro: UBI micro is designed to be extremely small and has no package manager, so you typically won’t have (or be able to easily add) the needed OpenSSL bits unless you add/copy them during a multi-stage build. [4] - If your goal is “FIPS + glibc + Go boringcrypto” in a container, the simplest path is usually to run on
ubi9/ubi-minimalorubi9/ubiand ensure OpenSSL is present, then run the container on a FIPS-enabled RHEL host so the FIPS provider is activated. [2][3][4]
Sources:
[1] https://access.redhat.com/discussions/7097037
[2] https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/epub/9.0_release_notes/bug-fix_high-availability-and-clusters
[3] https://developers.redhat.com/articles/2025/01/23/fips-mode-red-hat-go-toolset
[4] https://www.redhat.com/en/blog/introducing-red-hat-universal-base-image
Align to Go 1.24 with FIPS-compliant build settings.
Line 4 currently uses Go 1.25, and line 29 sets CGO_ENABLED=0. This deviates from the project requirement: Go 1.24 with FIPS-compliant crypto (CGO_ENABLED=1 + GOEXPERIMENT=boringcrypto).
Update line 4 to use registry.access.redhat.com/ubi9/go-toolset:1.24 (available in the registry; exact patch level may vary) and line 29 to enable CGO and boringcrypto:
Suggested alignment
-FROM registry.access.redhat.com/ubi9/go-toolset:1.25 AS builder
+FROM registry.access.redhat.com/ubi9/go-toolset:1.24 AS builder
RUN export PATH="$(go env GOPATH)/bin:$PATH" && \
- CGO_ENABLED=0 GOOS=linux GIT_SHA=${GIT_SHA} GIT_DIRTY=${GIT_DIRTY} make build
+ CGO_ENABLED=1 GOEXPERIMENT=boringcrypto GOOS=linux GIT_SHA=${GIT_SHA} GIT_DIRTY=${GIT_DIRTY} make buildFor the runtime stage (line 1), switch BASE_IMAGE from gcr.io/distroless/static-debian12:nonroot to a glibc-based image (e.g., registry.access.redhat.com/ubi9/ubi-minimal:latest) to support FIPS boringcrypto at runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` at line 4, Update the builder base image FROM to use Go 1.24
(replace registry.access.redhat.com/ubi9/go-toolset:1.25 with
registry.access.redhat.com/ubi9/go-toolset:1.24) and change the build
environment to enable CGO and boringcrypto (set CGO_ENABLED=1 and add
GOEXPERIMENT=boringcrypto in the build stage where CGO is currently disabled);
also change the runtime BASE_IMAGE from the distroless nonroot image to a
glibc-based UBI minimal image (e.g.,
registry.access.redhat.com/ubi9/ubi-minimal:latest) so the binary built with
boringcrypto runs correctly at runtime.
There was a problem hiding this comment.
1.25 is what I needed.
"Enable CGO_ENABLED=1 + boringcrypto" — Valid concern, but premature
this is a deliberate tradeoff I already documented in the Dockerfile
There was a problem hiding this comment.
@xueli181114, you're absolutely right—I apologize for the confusion. I see now that Go 1.25 is the intended version for this project, and the Dockerfile already documents the deliberate tradeoff at lines 27-29: CGO_ENABLED=0 produces a static binary for the distroless runtime, with clear guidance that FIPS-compliant builds would require switching to CGO_ENABLED=1 + boringcrypto + a glibc-based runtime image when needed. My review comment was premature. Thank you for the clarification!
✏️ Learnings added
Learnt from: xueli181114
Repo: openshift-hyperfleet/hyperfleet-api PR: 59
File: Dockerfile:3-3
Timestamp: 2026-02-28T09:25:06.472Z
Learning: In openshift-hyperfleet/hyperfleet-api Dockerfile, CGO_ENABLED=0 is intentionally used to produce static binaries for the distroless runtime; the Dockerfile documents that FIPS-compliant builds (CGO_ENABLED=1 + GOEXPERIMENT=boringcrypto) would require switching to a glibc-based runtime image like ubi9-micro instead of distroless.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Learnt from: xueli181114
Repo: openshift-hyperfleet/hyperfleet-api PR: 59
File: Makefile:42-42
Timestamp: 2026-02-28T08:46:17.822Z
Learning: The openshift-hyperfleet/hyperfleet-api project uses Go 1.25 (updated from 1.24.9) for builds with FIPS-compliant crypto requirements.
xueli181114
force-pushed
the
HYPERFLEET-509
branch
from
13aae70 to
ac5b9e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
Makefile (1)
142-144:⚠️ Potential issue | 🟠 Major
run/docsuses privileged host port 80, which breaks common rootless Podman setups.Line 144 maps
-p 80:8080; default non-root workflows typically need an unprivileged host port.Suggested fix
- `@echo` "Please open http://localhost/" - $(CONTAINER_TOOL) run -d -p 80:8080 -e SWAGGER_JSON=/hyperfleet.yaml -v $(PWD)/openapi/hyperfleet.yaml:/hyperfleet.yaml swaggerapi/swagger-ui + `@echo` "Please open http://localhost:8080/" + $(CONTAINER_TOOL) run -d -p 8080:8080 -e SWAGGER_JSON=/hyperfleet.yaml -v $(PWD)/openapi/hyperfleet.yaml:/hyperfleet.yaml swaggerapi/swagger-uiOn Linux rootless Podman, can an unprivileged user publish host port 80 using `-p 80:8080`, or must they use a port >= 1024?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 142 - 144, The make target "run/docs" currently publishes privileged host port via the docker/podman flag "-p 80:8080", which fails for rootless Podman; update the target to use an unprivileged host port (>=1024) or make the host port configurable (e.g., use a variable like DOC_PORT with a default of 8080/8081) and replace the hard-coded "-p 80:8080" with "-p $(DOC_PORT):8080" (or another unprivileged port) so the command using $(CONTAINER_TOOL) works for non-root workflows.Dockerfile (1)
3-3:⚠️ Potential issue | 🔴 CriticalBuilder toolchain/crypto mode is not aligned with the required FIPS baseline.
Line 3 uses
go-toolset:1.25, and Line 29 setsCGO_ENABLED=0; this conflicts with the required Go 1.24.9 + CGO + boringcrypto baseline.Suggested fix
-ARG BASE_IMAGE=gcr.io/distroless/static-debian12:nonroot +ARG BASE_IMAGE=registry.access.redhat.com/ubi9/ubi-minimal:latest @@ -FROM registry.access.redhat.com/ubi9/go-toolset:1.25 AS builder +FROM registry.access.redhat.com/ubi9/go-toolset:1.24 AS builder @@ - CGO_ENABLED=0 GOOS=linux \ + CGO_ENABLED=1 GOEXPERIMENT=boringcrypto GOOS=linux \ GIT_SHA=${GIT_SHA} GIT_DIRTY=${GIT_DIRTY} BUILD_DATE=${BUILD_DATE} VERSION=${VERSION} \ make build#!/bin/bash set -euo pipefail echo "Dockerfile build/runtime settings:" rg -n 'ARG BASE_IMAGE|FROM .*go-toolset|CGO_ENABLED|GOEXPERIMENT' Dockerfile echo echo "Makefile build policy signals:" rg -n 'GO_VERSION|CGO_ENABLED|GOEXPERIMENT' Makefile echo echo "Project policy (if AGENTS.md exists):" fd -a '^AGENTS\.md$' --exec rg -n 'Go 1\.24\.9|CGO_ENABLED=1|boringcrypto' {}Based on learnings: Use Go 1.24.9 with FIPS-compliant crypto (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto) for all builds.
Also applies to: 23-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` at line 3, The builder uses registry.access.redhat.com/ubi9/go-toolset:1.25 and sets CGO_ENABLED=0 which conflicts with the required FIPS baseline; update the Dockerfile to use a Go 1.24.9 toolset base image (replace the FROM image reference), ensure build args/ENV set CGO_ENABLED=1 (remove or override any CGO_ENABLED=0), and enable GOEXPERIMENT=boringcrypto so the builder performs CGO builds with BoringCrypto; look for and update the BASE_IMAGE/FROM line, any CGO_ENABLED assignments, and GOEXPERIMENT settings to align with Go 1.24.9 + CGO + boringcrypto.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 241-248: The makefile check for QUAY_USER using "ifndef QUAY_USER"
doesn't catch an explicitly set-but-empty variable (QUAY_USER ?=), so change the
validation to test for an empty/whitespace-only QUAY_USER (for example using an
ifeq/ifneq with $(strip $(QUAY_USER))) in the image-dev validation block; update
the conditional that currently references QUAY_USER so it triggers the error
block and exits when QUAY_USER is empty or only whitespace, leaving the existing
error messages and exit behavior intact.
- Line 42: Update the GO_VERSION variable to "go1.24.9" (replace the current
GO_VERSION := go1.25.) and change the verify target logic so it fails the build
on mismatch instead of just warning: in the verify target (the block around the
current lines 88-95) add an exit with non-zero status (e.g., exit 1) when the
detected Go version does not equal $(GO_VERSION) so make verify will fail on
unsupported toolchains.
---
Duplicate comments:
In `@Dockerfile`:
- Line 3: The builder uses registry.access.redhat.com/ubi9/go-toolset:1.25 and
sets CGO_ENABLED=0 which conflicts with the required FIPS baseline; update the
Dockerfile to use a Go 1.24.9 toolset base image (replace the FROM image
reference), ensure build args/ENV set CGO_ENABLED=1 (remove or override any
CGO_ENABLED=0), and enable GOEXPERIMENT=boringcrypto so the builder performs CGO
builds with BoringCrypto; look for and update the BASE_IMAGE/FROM line, any
CGO_ENABLED assignments, and GOEXPERIMENT settings to align with Go 1.24.9 + CGO
+ boringcrypto.
In `@Makefile`:
- Around line 142-144: The make target "run/docs" currently publishes privileged
host port via the docker/podman flag "-p 80:8080", which fails for rootless
Podman; update the target to use an unprivileged host port (>=1024) or make the
host port configurable (e.g., use a variable like DOC_PORT with a default of
8080/8081) and replace the hard-coded "-p 80:8080" with "-p $(DOC_PORT):8080"
(or another unprivileged port) so the command using $(CONTAINER_TOOL) works for
non-root workflows.
| glog_v:=10 | ||
| # Encourage consistent tool versions | ||
| OPENAPI_GENERATOR_VERSION := 5.4.0 | ||
| GO_VERSION := go1.25. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Examining Makefile at line 42 (GO_VERSION) ==="
sed -n '40,45p' Makefile
echo
echo "=== Examining Makefile at lines 88-95 (verify target) ==="
sed -n '85,100p' Makefile
echo
echo "=== Checking for CGO_ENABLED and GOEXPERIMENT flags ==="
rg -n 'CGO_ENABLED|GOEXPERIMENT' Makefile
echo
echo "=== Checking AGENTS.md for Go version policy ==="
if [ -f AGENTS.md ]; then
rg -n 'Go 1\.24|boringcrypto' AGENTS.md
else
echo "AGENTS.md not found"
fiRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 1776
Go version must be updated to 1.24.9 and verify target must block on mismatch.
Line 42 sets GO_VERSION to go1.25., which violates the project policy in AGENTS.md requiring Go 1.24.9 for FIPS compliance. Additionally, Lines 88-95 only print a warning on mismatch—make verify exits successfully even with an unsupported toolchain. Add exit 1; to enforce the version requirement.
Suggested fix
-GO_VERSION := go1.25.
+GO_VERSION := go1.24.9 @ ${GO} version | grep -q "$(GO_VERSION)" || \
( \
printf '\033[41m\033[97m\n'; \
echo "* Your go version is not the expected $(GO_VERSION) *" | sed 's/./*/g'; \
echo "* Your go version is not the expected $(GO_VERSION) *"; \
echo "* Your go version is not the expected $(GO_VERSION) *" | sed 's/./*/g'; \
printf '\033[0m'; \
+ exit 1; \
)📝 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.
| GO_VERSION := go1.25. | |
| GO_VERSION := go1.24.9 |
| GO_VERSION := go1.25. | |
| @ ${GO} version | grep -q "$(GO_VERSION)" || \ | |
| ( \ | |
| printf '\033[41m\033[97m\n'; \ | |
| echo "* Your go version is not the expected $(GO_VERSION) *" | sed 's/./*/g'; \ | |
| echo "* Your go version is not the expected $(GO_VERSION) *"; \ | |
| echo "* Your go version is not the expected $(GO_VERSION) *" | sed 's/./*/g'; \ | |
| printf '\033[0m'; \ | |
| exit 1; \ | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` at line 42, Update the GO_VERSION variable to "go1.24.9" (replace
the current GO_VERSION := go1.25.) and change the verify target logic so it
fails the build on mismatch instead of just warning: in the verify target (the
block around the current lines 88-95) add an exit with non-zero status (e.g.,
exit 1) when the detected Go version does not equal $(GO_VERSION) so make verify
will fail on unsupported toolchains.
There was a problem hiding this comment.
But why? I need go 1.25
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking AGENTS.md for Go version requirements ==="
if [ -f AGENTS.md ]; then
cat AGENTS.md
else
echo "AGENTS.md not found"
fi
echo -e "\n=== Searching for Go version references in repository ==="
rg -i 'go.?1\.(24|25)' --type md --type yaml --type txt -C 2 || echo "No Go version references found"
echo -e "\n=== Checking Dockerfile for Go version ==="
if [ -f Dockerfile ]; then
rg -n 'go-toolset|golang:' Dockerfile || echo "No Go base image found"
fi
echo -e "\n=== Checking go.mod for Go version ==="
if [ -f go.mod ]; then
head -n 10 go.mod
fi
Length of output: 28383
🌐 Web query:
Does Go 1.25 support FIPS-compliant crypto with GOEXPERIMENT=boringcrypto
💡 Result:
Go 1.25 still contains the GOEXPERIMENT=boringcrypto code path (the “Go+BoringCrypto” mode), but it is not an officially supported or documented way to claim FIPS compliance, and the Go team explicitly makes no representations about its suitability for FIPS use outside Google. [1][2]
For a supported FIPS story in Go 1.25, upstream Go’s intended path is the native Go Cryptographic Module introduced in Go 1.24 and controlled via GOFIPS140 (build-time selection) and GODEBUG=fips140=on|only (runtime mode). [3][4]
Practical takeaway: If you need a supported FIPS mode in Go 1.25, use GOFIPS140=... + GODEBUG=fips140=..., not GOEXPERIMENT=boringcrypto. [3][4]
Sources: [1] https://groups.google.com/g/golang-dev/c/fqwZgtzHbzk [2] https://www.mail-archive.com/golang-nuts%40googlegroups.com/msg53105.html [3] https://go.dev/blog/fips140 [4] https://tip.golang.org/doc/security/fips140
@xueli181114, that makes sense! I see the Dockerfile in this PR is already using go-toolset:1.25. However, there's a documentation mismatch:
Documentation currently states Go 1.24.9:
- AGENTS.md (line specifies "Go 1.24.9 with FIPS-compliant crypto")
- README.md
- PREREQUISITES.md
More importantly, I found that Go 1.24+ introduced a new native FIPS approach that replaces GOEXPERIMENT=boringcrypto. According to the official Go blog, the supported FIPS compliance path in Go 1.25 is:
- Build-time:
GOFIPS140=... - Runtime:
GODEBUG=fips140=on|only
The old GOEXPERIMENT=boringcrypto path still exists but is not officially supported for FIPS compliance outside Google.
Suggestions:
- Update the documentation files to reflect Go 1.25
- Consider migrating from
GOEXPERIMENT=boringcryptoto the newGOFIPS140+GODEBUG=fips140approach for better FIPS support
Would you like me to help identify all the places that need updating, or do you prefer to handle this separately?
🧠 Learnings used
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T02:03:46.333Z
Learning: Use Go 1.24.9 with FIPS-compliant crypto (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto) for all builds
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
There was a problem hiding this comment.
help identify all the places that need updating
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Searching for Go 1.24 version references ==="
rg -n '1\.24' --type-add 'doc:*.{md,txt,rst}' --type doc --type yaml --type toml --type makefile || echo "No 1.24 references found in docs/config"
echo -e "\n=== Searching for Go version in all files (including code) ==="
rg -n 'go.?1\.24|golang.*1\.24' -g '!vendor/' -g '!*.sum' || echo "No go 1.24 references found"
echo -e "\n=== Searching for GOEXPERIMENT=boringcrypto usage ==="
rg -n 'GOEXPERIMENT.*boringcrypto|boringcrypto' -g '!vendor/' || echo "No boringcrypto references found"
echo -e "\n=== Checking for FIPS-related configuration ==="
rg -n 'FIPS|fips140|GOFIPS' -g '!vendor/' || echo "No FIPS configuration found"
echo -e "\n=== Listing all markdown documentation files ==="
fd -e md -t f
echo -e "\n=== Checking go.mod for Go version directive ==="
if [ -f go.mod ]; then
head -n 5 go.mod
fi
echo -e "\n=== Checking for any CI/CD configuration files ==="
fd -t f -e yml -e yaml . .github .gitlab-ci workflows || echo "No CI/CD config found"
echo -e "\n=== Checking Dockerfile for Go version ==="
if [ -f Dockerfile ]; then
rg -n 'go-toolset|golang|FROM.*go' Dockerfile
fi
Length of output: 2542
@xueli181114, here's a comprehensive list of all files that need updating to reflect Go 1.25:
Files Requiring Go Version Updates
1. go.mod (lines 3, 5)
Current:
go 1.24.0
toolchain go1.24.9Update to:
go 1.25.0
toolchain go1.252. AGENTS.md
- Line 27: Update
Go 1.24.9→Go 1.25 - Line 36: Update
Go 1.24→Go 1.25
3. PREREQUISITES.md (line 10)
Current: Go 1.24 or higher
Update to: Go 1.25 or higher
4. docs/metrics.md (lines 29, 34)
Update example metrics from go1.24.0 → go1.25.0
5. Makefile (line 42) ✅ Already Updated
Currently: GO_VERSION := go1.25.
Optional: Remove trailing dot → GO_VERSION := go1.25
6. Dockerfile (line 3) ✅ Already Correct
Already using: registry.access.redhat.com/ubi9/go-toolset:1.25
FIPS Compliance Migration (Separate Consideration)
The following files currently use GOEXPERIMENT=boringcrypto. Consider migrating to the new official FIPS approach (GOFIPS140 + GODEBUG=fips140):
Files to Review:
- Makefile (lines 125, 129, 150): Build commands
- Dockerfile (line 24): FIPS build comment
- AGENTS.md (line 27): Documentation
Migration would involve:
- Build flags:
GOEXPERIMENT=boringcrypto→GOFIPS140=latest - Runtime: Add
GODEBUG=fips140=on(or=onlyfor strict mode)
This is a more significant change that affects build and runtime behavior, so you may want to test thoroughly before migrating.
Would you like me to help draft the specific changes for the FIPS migration as well?
✏️ Learnings added
Learnt from: xueli181114
Repo: openshift-hyperfleet/hyperfleet-api PR: 59
File: Makefile:42-42
Timestamp: 2026-02-28T08:46:17.822Z
Learning: The openshift-hyperfleet/hyperfleet-api project uses Go 1.25 (updated from 1.24.9) for builds with FIPS-compliant crypto requirements.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
⛔ Learnings removed
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T02:03:46.333Z
Learning: Use Go 1.24.9 with FIPS-compliant crypto (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto) for all builds
🧠 Learnings used
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T02:03:46.333Z
Learning: Use Go 1.24.9 with FIPS-compliant crypto (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto) for all builds
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
There was a problem hiding this comment.
Stop it now. I will use another ticket to bump the whole go version including update the docs. Right now go.mod is using 1.24, current bump may affect local development who is using 1.24
There was a problem hiding this comment.
@xueli181114, understood! That makes perfect sense—keeping go.mod at 1.24 for now ensures developers don't hit version mismatches during local development. I'll leave this as-is, and you can coordinate the full version bump (go.mod, docs, etc.) in the separate ticket.
Thanks for clarifying the approach! 👍
🧠 Learnings used
Learnt from: xueli181114
Repo: openshift-hyperfleet/hyperfleet-api PR: 59
File: Makefile:42-42
Timestamp: 2026-02-28T08:46:17.822Z
Learning: In Makefile(s) for the openshift-hyperfleet/hyperfleet-api repo, pin and enforce Go 1.25 for builds and ensure FIPS-compliant crypto requirements are met. Update CI/build scripts to use Go 1.25, set the appropriate image/Go toolchain, and document any FIPS-related flags or environment variables. Add a verification step that prints the Go version and asserts it equals 1.25, and ensure crypto libraries used are FIPS-compliant.
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
ac5b9e2 to
3f84848
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
Makefile (2)
88-95:⚠️ Potential issue | 🟠 MajorFail
verifyon Go-version mismatch, not just print a warning.At Line 88, the
|| ( ... )branch ends successfully, somake verifystill exits 0 on mismatch.Suggested fix
@ ${GO} version | grep -q "$(GO_VERSION)" || \ ( \ printf '\033[41m\033[97m\n'; \ echo "* Your go version is not the expected $(GO_VERSION) *" | sed 's/./*/g'; \ echo "* Your go version is not the expected $(GO_VERSION) *"; \ echo "* Your go version is not the expected $(GO_VERSION) *" | sed 's/./*/g'; \ printf '\033[0m'; \ + exit 1; \ )#!/bin/bash set -euo pipefail echo "Inspecting verify gate:" nl -ba Makefile | sed -n '88,95p' if sed -n '88,95p' Makefile | rg -q 'exit 1'; then echo "PASS: verify mismatch branch contains exit 1" else echo "FAIL: verify mismatch branch does not fail the target" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 88 - 95, The Go version check in the Makefile uses "@ ${GO} version | grep -q "$(GO_VERSION)" || ( ... )" but the failure subshell currently only prints warnings and exits successfully, so the verify target doesn't fail; modify the failure branch of that check (the subshell after the ||) to terminate with a non-zero exit (e.g., add an exit 1) so that when GO version mismatches the verify target fails; update the block that references GO and GO_VERSION accordingly and ensure the subshell prints the formatted message then calls exit 1.
142-145:⚠️ Potential issue | 🟠 Major
run/docscommand uses a privileged host port and likely stale spec path.Line 144 still maps
-p 80:8080(problematic for rootless Podman) and mountsopenapi/hyperfleet.yaml, while the standardized spec path isopenapi/openapi.yaml.
Based on learnings: Store API specification inopenapi/openapi.yaml(TypeSpec-generated source) - never manually editpkg/api/openapi/api/openapi.yaml(fully resolved, auto-generated).Suggested fix
run/docs: ## Run swagger and host the api spec - `@echo` "Please open http://localhost/" - $(CONTAINER_TOOL) run -d -p 80:8080 -e SWAGGER_JSON=/hyperfleet.yaml -v $(PWD)/openapi/hyperfleet.yaml:/hyperfleet.yaml swaggerapi/swagger-ui + `@echo` "Please open http://localhost:8080/" + $(CONTAINER_TOOL) run -d -p 8080:8080 -e SWAGGER_JSON=/openapi.yaml -v $(PWD)/openapi/openapi.yaml:/openapi.yaml swaggerapi/swagger-ui#!/bin/bash set -euo pipefail echo "Inspecting run/docs target:" nl -ba Makefile | sed -n '142,145p' echo echo "Checking for privileged host-port mapping or stale spec filename:" sed -n '142,145p' Makefile | rg -n -- '-p 80:8080|openapi/hyperfleet.yaml' || true echo echo "Listing OpenAPI files under openapi/:" fd -a 'openapi\.yaml|hyperfleet\.yaml' openapi || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 142 - 145, The run/docs Makefile target currently maps a privileged host port and mounts a stale spec filename; update the run/docs recipe (target name: run/docs) to avoid host port 80 (use a non-privileged mapping such as -p 8080:8080 or use a CONTAINER_PORT variable) and change the mounted OpenAPI source from openapi/hyperfleet.yaml to the canonical TypeSpec source openapi/openapi.yaml (keep SWAGGER_JSON=/hyperfleet.yaml if you want the container to read /hyperfleet.yaml but mount the host openapi/openapi.yaml to that path); do not point the mount at the generated pkg/api/openapi/api/openapi.yaml. Ensure the CONTAINER_TOOL run command reflects these two fixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Makefile`:
- Around line 88-95: The Go version check in the Makefile uses "@ ${GO} version
| grep -q "$(GO_VERSION)" || ( ... )" but the failure subshell currently only
prints warnings and exits successfully, so the verify target doesn't fail;
modify the failure branch of that check (the subshell after the ||) to terminate
with a non-zero exit (e.g., add an exit 1) so that when GO version mismatches
the verify target fails; update the block that references GO and GO_VERSION
accordingly and ensure the subshell prints the formatted message then calls exit
1.
- Around line 142-145: The run/docs Makefile target currently maps a privileged
host port and mounts a stale spec filename; update the run/docs recipe (target
name: run/docs) to avoid host port 80 (use a non-privileged mapping such as -p
8080:8080 or use a CONTAINER_PORT variable) and change the mounted OpenAPI
source from openapi/hyperfleet.yaml to the canonical TypeSpec source
openapi/openapi.yaml (keep SWAGGER_JSON=/hyperfleet.yaml if you want the
container to read /hyperfleet.yaml but mount the host openapi/openapi.yaml to
that path); do not point the mount at the generated
pkg/api/openapi/api/openapi.yaml. Ensure the CONTAINER_TOOL run command reflects
these two fixes.
3f84848 to
8d297ae
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Makefile (1)
141-144:⚠️ Potential issue | 🟡 MinorRootless Podman cannot bind to host port 80.
Unprivileged users on Linux cannot bind to ports below 1024. Since Podman is preferred (line 13) and typically runs rootless,
-p 80:8080will fail for non-root users. Use an unprivileged host port instead.Suggested fix
.PHONY: run/docs run/docs: ## Run swagger and host the api spec - `@echo` "Please open http://localhost/" - $(CONTAINER_TOOL) run -d -p 80:8080 -e SWAGGER_JSON=/hyperfleet.yaml -v $(PWD)/openapi/hyperfleet.yaml:/hyperfleet.yaml swaggerapi/swagger-ui + `@echo` "Please open http://localhost:8080/" + $(CONTAINER_TOOL) run -d -p 8080:8080 -e SWAGGER_JSON=/hyperfleet.yaml -v $(PWD)/openapi/hyperfleet.yaml:/hyperfleet.yaml swaggerapi/swagger-ui🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 141 - 144, The Makefile target run/docs uses CONTAINER_TOOL to run a container with host port mapping -p 80:8080 which fails for rootless Podman (unprivileged users cannot bind <1024); update the run/docs recipe to use an unprivileged host port (e.g., 8080:8080 or a configurable variable like SWAGGER_HOST_PORT) instead of 80:8080 so rootless Podman users can start the swagger-ui container; ensure you update any references or documentation that expect http://localhost/ to the new port (e.g., http://localhost:8080) if needed.
🧹 Nitpick comments (3)
Makefile (3)
88-95: Go version check warns but does not fail the build.The
verifytarget prints a warning when the Go version doesn't match but allows the build to continue. This could lead to builds with unsupported toolchains. Consider addingexit 1if strict enforcement is desired.Optional: Make version check fail
@ ${GO} version | grep -q "$(GO_VERSION)" || \ ( \ printf '\033[41m\033[97m\n'; \ echo "* Your go version is not the expected $(GO_VERSION) *" | sed 's/./*/g'; \ echo "* Your go version is not the expected $(GO_VERSION) *"; \ echo "* Your go version is not the expected $(GO_VERSION) *" | sed 's/./*/g'; \ printf '\033[0m'; \ + exit 1; \ )Based on learnings: "Add a verification step that prints the Go version and asserts it equals 1.25."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 88 - 95, The Go version check in the Makefile's verify target currently only warns; update the check that uses ${GO} and $(GO_VERSION) so it exits non‑zero on mismatch (e.g., add an exit 1 after the colored warning) and print the detected Go version for clarity; also assert the version equals the required "1.25" (compare output of `${GO} version` to "go1.25" or normalize and fail) so the verify target enforces the toolchain strictly.
12-13: Container tool detection lacks fallback handling.If neither
podmannordockeris installed,CONTAINER_TOOLwill be empty, causing cryptic errors when container targets are invoked. Consider adding a validation or helpful error message.Suggested validation
Add a guard target or inline check in container-dependent targets:
# Add after line 13 ifeq ($(CONTAINER_TOOL),) $(warning No container tool found. Install podman or docker for container targets.) endifOr validate at the start of container targets:
.PHONY: image image: ## Build container image with configurable registry/tag + `@test` -n "$(CONTAINER_TOOL)" || { echo "Error: No container tool (podman/docker) found"; exit 1; } `@echo` "Building container image $(IMAGE_REGISTRY)/$(IMAGE_NAME):$(IMAGE_TAG)..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 12 - 13, The Makefile currently sets CONTAINER_TOOL but provides no fallback or check when both podman and docker are missing; add a validation that detects when CONTAINER_TOOL is empty and prints a clear warning or fails early for container-dependent targets (e.g., emit a $(warning ...) or use a conditional that errors out) so invoking targets that rely on CONTAINER_TOOL gives a helpful message; locate the CONTAINER_TOOL assignment and add the guard/validation immediately after it to ensure container targets don't proceed with an empty value.
49-50: Hardcoded database password in Makefile.The default password
foobar-bizz-buzzis committed to version control. While this appears to be for local development only, consider:
- Adding a comment clarifying this is for local dev only
- Using environment variable override:
db_password ?= $${DB_PASSWORD:-foobar-bizz-buzz}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 49 - 50, The Makefile currently hardcodes db_password := foobar-bizz-buzz which exposes a default credential; update the assignment to allow environment overrides and document it as a local-dev default: replace the hardcoded assignment for the db_password variable (and keep db_password_file) with a conditional assignment that uses an environment variable (DB_PASSWORD) and falls back to the local default, and add a short comment above the db_password variable that this default is for local development only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Line 44: The Dockerfile currently forces USER 65532:65532 which breaks builds
when BASE_IMAGE is overridden to alpine:3.21 (used by make image-dev); update
the Dockerfile/build process to handle non-distroless bases: either (A)
conditionally emit the USER directive only for the distroless base by testing
the build-arg BASE_IMAGE in the Dockerfile/Makefile, or (B) when BASE_IMAGE is
Alpine, create a nonroot user/group with UID/GID 65532 (using addgroup/adduser)
before the USER 65532:65532 line so the account exists, or (C) switch to a
portable nonroot approach (e.g., create and use a project-specific nonroot user
with a configurable UID via build-arg); pick one approach and implement it
around the existing USER 65532:65532 directive and the make image-dev build
path.
---
Duplicate comments:
In `@Makefile`:
- Around line 141-144: The Makefile target run/docs uses CONTAINER_TOOL to run a
container with host port mapping -p 80:8080 which fails for rootless Podman
(unprivileged users cannot bind <1024); update the run/docs recipe to use an
unprivileged host port (e.g., 8080:8080 or a configurable variable like
SWAGGER_HOST_PORT) instead of 80:8080 so rootless Podman users can start the
swagger-ui container; ensure you update any references or documentation that
expect http://localhost/ to the new port (e.g., http://localhost:8080) if
needed.
---
Nitpick comments:
In `@Makefile`:
- Around line 88-95: The Go version check in the Makefile's verify target
currently only warns; update the check that uses ${GO} and $(GO_VERSION) so it
exits non‑zero on mismatch (e.g., add an exit 1 after the colored warning) and
print the detected Go version for clarity; also assert the version equals the
required "1.25" (compare output of `${GO} version` to "go1.25" or normalize and
fail) so the verify target enforces the toolchain strictly.
- Around line 12-13: The Makefile currently sets CONTAINER_TOOL but provides no
fallback or check when both podman and docker are missing; add a validation that
detects when CONTAINER_TOOL is empty and prints a clear warning or fails early
for container-dependent targets (e.g., emit a $(warning ...) or use a
conditional that errors out) so invoking targets that rely on CONTAINER_TOOL
gives a helpful message; locate the CONTAINER_TOOL assignment and add the
guard/validation immediately after it to ensure container targets don't proceed
with an empty value.
- Around line 49-50: The Makefile currently hardcodes db_password :=
foobar-bizz-buzz which exposes a default credential; update the assignment to
allow environment overrides and document it as a local-dev default: replace the
hardcoded assignment for the db_password variable (and keep db_password_file)
with a conditional assignment that uses an environment variable (DB_PASSWORD)
and falls back to the local default, and add a short comment above the
db_password variable that this default is for local development only.
8d297ae to
b10b702
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Makefile (1)
12-13: Add validation for missing container tool.If neither
podmannordockeris installed,CONTAINER_TOOLwill be empty, causing cryptic failures in targets likeimage,db/setup, etc.Suggested validation
# Auto-detect container tool (podman preferred when available) CONTAINER_TOOL ?= $(shell command -v podman 2>/dev/null || command -v docker 2>/dev/null) + +# Validate container tool is available (checked by targets that need it) +.PHONY: check-container-tool +check-container-tool: +ifeq ($(strip $(CONTAINER_TOOL)),) + `@echo` "Error: No container tool found. Please install podman or docker." + `@exit` 1 +endifThen add
check-container-toolas a dependency to container-related targets (image,image-push,image-dev,db/setup,run/docs).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 12 - 13, Add a validation step for CONTAINER_TOOL: create a new make target (e.g. check-container-tool) that fails with a clear error message if CONTAINER_TOOL is empty (i.e. neither podman nor docker detected), and then add this target as a prerequisite to all container-related targets (image, image-push, image-dev, db/setup, run/docs) so they depend on check-container-tool before running; reference the existing CONTAINER_TOOL variable in the new target to perform the check and emit guidance about installing podman or docker.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 160-166: The clean target currently deletes the secrets directory;
update the Makefile to stop removing secrets/ from the default clean target and
instead create a separate target (e.g., clean-secrets) that explicitly removes
the secrets directory and is marked .PHONY; modify the clean rule (referencing
the clean target and .PHONY: clean) to only remove build artifacts (bin,
pkg/api/openapi, data/generated/openapi/*.json) and add a new .PHONY:
clean-secrets rule that performs removal of secrets/ so users must opt in to
that destructive action.
- Around line 44-46: The Makefile defines db_host := hyperfleet-db.$(namespace)
but namespace is undefined; either remove the unused db_host variable or add a
default namespace value to avoid empty expansion. Locate the db_host assignment
and either delete that line (if db_host is dead code) or add a namespace
variable definition (e.g., namespace ?= <default>) earlier in the Makefile so
db_host expands correctly; ensure references to db_host (if any) still work
after the change.
---
Nitpick comments:
In `@Makefile`:
- Around line 12-13: Add a validation step for CONTAINER_TOOL: create a new make
target (e.g. check-container-tool) that fails with a clear error message if
CONTAINER_TOOL is empty (i.e. neither podman nor docker detected), and then add
this target as a prerequisite to all container-related targets (image,
image-push, image-dev, db/setup, run/docs) so they depend on
check-container-tool before running; reference the existing CONTAINER_TOOL
variable in the new target to perform the check and emit guidance about
installing podman or docker.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
DockerfileMakefile
🚧 Files skipped from review as they are similar to previous changes (1)
- Dockerfile
Makefile
Outdated
| # Database connection details | ||
| db_name := hyperfleet | ||
| db_host = hyperfleet-db.$(namespace) |
There was a problem hiding this comment.
db_host references undefined namespace variable.
db_host uses $(namespace) but namespace is never defined in this Makefile. This will expand to hyperfleet-db. (empty suffix). While db_host doesn't appear to be used by any target currently (db/setup writes "localhost" to secrets), this is likely dead code or a leftover.
Options
- Remove
db_hostif unused:
-db_host = hyperfleet-db.$(namespace)- Or define
namespacewith a default:
+namespace ?= default
db_host = hyperfleet-db.$(namespace)📝 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.
| # Database connection details | |
| db_name := hyperfleet | |
| db_host = hyperfleet-db.$(namespace) | |
| # Database connection details | |
| db_name := hyperfleet |
| # Database connection details | |
| db_name := hyperfleet | |
| db_host = hyperfleet-db.$(namespace) | |
| # Database connection details | |
| namespace ?= default | |
| db_name := hyperfleet | |
| db_host = hyperfleet-db.$(namespace) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 44 - 46, The Makefile defines db_host :=
hyperfleet-db.$(namespace) but namespace is undefined; either remove the unused
db_host variable or add a default namespace value to avoid empty expansion.
Locate the db_host assignment and either delete that line (if db_host is dead
code) or add a namespace variable definition (e.g., namespace ?= <default>)
earlier in the Makefile so db_host expands correctly; ensure references to
db_host (if any) still work after the change.
| .PHONY: clean | ||
| clean: ## Delete temporary generated files | ||
| rm -rf \ | ||
| bin \ | ||
| pkg/api/openapi \ | ||
| data/generated/openapi/*.json \ | ||
| secrets \ |
There was a problem hiding this comment.
clean removes secrets/ which may contain user-configured credentials.
The secrets directory can contain user-customized values (e.g., ocm-service.clientSecret, ocm-service.token). Removing it in clean could surprise users who expect only build artifacts to be deleted.
Suggestion: separate secrets cleanup
.PHONY: clean
clean: ## Delete temporary generated files
rm -rf \
bin \
pkg/api/openapi \
- data/generated/openapi/*.json \
- secrets \
+ data/generated/openapi/*.json
+
+.PHONY: clean-secrets
+clean-secrets: ## Remove secrets directory (use with caution)
+ rm -rf secrets
+
+.PHONY: clean-all
+clean-all: clean clean-secrets ## Delete all generated files including secrets🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 160 - 166, The clean target currently deletes the
secrets directory; update the Makefile to stop removing secrets/ from the
default clean target and instead create a separate target (e.g., clean-secrets)
that explicitly removes the secrets directory and is marked .PHONY; modify the
clean rule (referencing the clean target and .PHONY: clean) to only remove build
artifacts (bin, pkg/api/openapi, data/generated/openapi/*.json) and add a new
.PHONY: clean-secrets rule that performs removal of secrets/ so users must opt
in to that destructive action.
xueli181114
force-pushed
the
HYPERFLEET-509
branch
from
b10b702 to
1fc767a
Compare
1 participant
Summary by CodeRabbit
New Features
Chores