-
Notifications
You must be signed in to change notification settings - Fork 136
[ci] Add cross-platform cozypkg build targets with version injection #1862
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (7)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughReplaces the single "installer" artifact with separate CRDs and Operator manifests across build, release, and test flows; adds multi-platform cozypkg packaging and a build-time CLI Version variable. Workflow steps, asset names/outputs, uploads/downloads, and test/apply scripts updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
actor Developer
participant CI as GitHub Actions Runner
participant Release as GitHub Release API
participant Storage as Release Assets
participant Cluster as Kubernetes Cluster
Developer ->> CI: push PR / create release
CI ->> CI: run Makefile -> generate `cozystack-crds.yaml`, `cozystack-operator.yaml`, cozypkg archives
CI ->> Release: upload assets (`crds`, `operator`, `cozypkg` archives, checksums)
Release ->> Storage: store uploaded assets
CI ->> Release: resolve asset IDs (crds_id, operator_id, disk_id)
CI ->> CI: download assets using resolved IDs
CI ->> Cluster: apply `cozystack-crds.yaml`
CI ->> Cluster: apply `cozystack-operator.yaml`
Cluster -->> CI: readiness/status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
Summary of ChangesHello @kvaps, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refines the build and deployment pipeline by enabling the creation of cross-platform Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces cross-platform builds for cozypkg, injects version information, and splits the installer manifests. The changes are well-structured, particularly in the Makefile with new build targets. The Go code modification for version injection is correct, and the updates across tests, manifests, and scripts are consistent with the new asset structure. I have one suggestion to improve the robustness of the build process in the Makefile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@Makefile`:
- Around line 40-46: The build-deps dependency check is missing the cozyhr tool
used by the manifests target; update the build-deps check by adding "cozyhr" to
the list of required tools (the tools array/string on the build-deps line ~6) so
the Makefile verifies cozyhr is installed alongside helm, docker, jq, etc.
- Around line 59-62: The assets-cozypkg-% makefile rule needs to ensure output
directories exist and append .exe for Windows builds: modify the
assets-cozypkg-% recipe to create directories before building (ensure
_out/bin/cozypkg-$* and _out/assets exist via mkdir -p or similar) and when
computing the output filename for go build (used in the -o flag and later
filenames like cozypkg and tar contents) append .exe when GOOS is windows
(detect via GOOS=$(firstword $(subst -, ,$*)) and set BINNAME=cozypkg$(if
$(filter windows,$(GOOS)),.exe,)); use that BINNAME in the go build -o, cp, and
tar commands so Windows builds produce cozypkg.exe and directories are created.
In `@scripts/common-envs.mk`:
- Around line 28-32: COZYSTACK_VERSION can remain empty if git describe --tags
fails; update the scripts/common-envs.mk logic that sets COZYSTACK_VERSION so it
provides a safe fallback or fails early: after the existing git describe
assignment, check if COZYSTACK_VERSION is empty and either set a default like
"0.0.0" or "dev" (so helm package --version always receives a value) or print a
clear error and exit; reference the COZYSTACK_VERSION variable and the git
describe --tags invocation and ensure the chosen behavior prevents downstream
helm package commands from receiving an empty --version.
🧹 Nitpick comments (1)
Makefile (1)
48-49: Consider ensuring output directory exists.The
go buildcommand outputs to_out/bin/cozypkg, but unlike themanifeststarget (which hasmkdir -p _out/assets), there's no directory creation here. This could fail on a clean build.Proposed fix
cozypkg: + mkdir -p _out/bin go build -ldflags "-X github.com/cozystack/cozystack/cmd/cozypkg/cmd.Version=v$(COZYSTACK_VERSION)" -o _out/bin/cozypkg ./cmd/cozypkg
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/pull-requests.yamlMakefilecmd/cozypkg/cmd/root.gohack/e2e-install-cozystack.batshack/upload-assets.shpackages/core/installer/templates/crds.yamlpackages/core/testing/Makefilescripts/common-envs.mk
💤 Files with no reviewable changes (1)
- packages/core/installer/templates/crds.yaml
🧰 Additional context used
📓 Path-based instructions (3)
cmd/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Follow controller-runtime patterns and kubebuilder style for Go code in cmd directory
Files:
cmd/cozypkg/cmd/root.go
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Do NOT manually edit vendor/ directory or auto-generated files (zz_generated.*.go)
Files:
cmd/cozypkg/cmd/root.go
packages/core/**
📄 CodeRabbit inference engine (AGENTS.md)
Use Helm Chart umbrella pattern with vendored upstream charts in
charts/directory
Files:
packages/core/testing/Makefile
🧠 Learnings (2)
📓 Common learnings
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1117
File: packages/apps/mysql/Makefile:8-8
Timestamp: 2025-06-26T04:29:24.830Z
Learning: The cozystack project uses yq v4+ on their CI runner, so yq v4 syntax (-o json --indent 4) is compatible and version checks are not needed.
📚 Learning: 2025-06-26T04:29:24.830Z
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1117
File: packages/apps/mysql/Makefile:8-8
Timestamp: 2025-06-26T04:29:24.830Z
Learning: The cozystack project uses yq v4+ on their CI runner, so yq v4 syntax (-o json --indent 4) is compatible and version checks are not needed.
Applied to files:
scripts/common-envs.mkhack/upload-assets.shhack/e2e-install-cozystack.bats
🔇 Additional comments (13)
cmd/cozypkg/cmd/root.go (2)
26-28: LGTM!The exported
Versionvariable with a sensible default follows the standard Go pattern for build-time version injection via-ldflags "-X <package>.Version=<value>".
49-51: LGTM!Wiring the
VersiontorootCmd.Versionininit()correctly enables Cobra's built-in--versionflag support..github/workflows/pull-requests.yaml (3)
74-84: LGTM!The upload steps are correctly split to handle CRDs and operator manifests as separate artifacts. The naming convention (
cozystack-crdsandcozystack-operator) is consistent with the new asset structure.
92-151: LGTM!The
resolve_assetsjob correctly updates the outputs and asset lookups to reflect the new CRDs and operator split. The validation at line 145 properly ensures all three required assets exist before proceeding.
236-262: LGTM!The download steps for both regular PR and release PR paths are correctly updated to fetch the CRDs and operator manifests separately. The asset ID references align with the outputs from
resolve_assets.hack/upload-assets.sh (2)
6-7: LGTM!The upload commands correctly reflect the new asset split from installer to CRDs and operator manifests.
13-14: Glob expansion is safe—cozypkg artifacts are always built.The
assetstarget in the Makefile hasassets-cozypkgas an explicit mandatory dependency (line 51), which in turn builds 6 platform variants. The glob patterncozypkg-*.tar.gzwill reliably match all built artifacts. The Makefile itself uses the same glob pattern when generating the checksums file, confirming it's intentional and safe. The release workflow also enforces this sequence by runningmake assetsbeforemake upload_assets.scripts/common-envs.mk (1)
15-15: LGTM!Suppressing stderr with
2>/dev/nullis appropriate here to avoid noisy output when no tags exist. The fallback logic at lines 28-32 handles the empty case by fetching upstream tags.packages/core/testing/Makefile (1)
33-35: LGTM!The target correctly copies both manifest files to the sandbox container. The target name
copy-installer-manifestis slightly outdated given the split, but renaming it would require updating dependent targets (install-cozystack,test-cluster), which may not be worth the churn.hack/e2e-install-cozystack.bats (2)
3-12: LGTM!The prerequisite check correctly validates both manifest files exist before proceeding, with clear error messages for each missing file.
14-20: LGTM!The installation order is correct: CRDs are applied first (line 19) followed by the operator (line 20). This ensures the Custom Resource Definitions are registered before the operator attempts to watch them.
Makefile (2)
8-10: LGTM!Good addition of GNU tool checks. These ensure consistent behavior across different environments since GNU-specific features may be used in the build scripts.
56-57: No action needed. Thesha256sumtool is already actively used in the CI workflows (pull-requests.yaml lines 200, 266, 316), confirming its availability in the self-hosted runner environment. The CI infrastructure uses self-hosted Linux runners, not macOS, so the portability concern does not apply.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
ccbb2bf to
7bcb25c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@hack/upload-assets.sh`:
- Around line 13-14: The Makefile target upload_assets currently only depends on
manifests, which allows hack/upload-assets.sh to run before assets-cozypkg is
built and may cause gh release upload to fail on unmatched glob patterns; update
the Makefile so the upload_assets target declares assets-cozypkg as an explicit
dependency (i.e., make upload_assets depend on both manifests and
assets-cozypkg) so the tar.gz artifacts are built before hack/upload-assets.sh
runs.
In `@Makefile`:
- Around line 59-64: The assets-cozypkg-% target uses GO build to write to
_out/bin/cozypkg-$*/cozypkg$(EXT) and then cp LICENSE there, but only
_out/assets is created; add a step to create the output directory (e.g., mkdir
-p _out/bin/cozypkg-$*) before the go build and cp so the build output and copy
succeed; update the assets-cozypkg-% recipe (referencing the target name and
variables EXT and the path _out/bin/cozypkg-$*/) to ensure the directory exists.
🧹 Nitpick comments (1)
Makefile (1)
48-49: Add directory creation forcozypkgtarget.The
cozypkgtarget outputs to_out/bin/cozypkg, but_out/binmay not exist on a clean build.Proposed fix
cozypkg: + mkdir -p _out/bin go build -ldflags "-X github.com/cozystack/cozystack/cmd/cozypkg/cmd.Version=v$(COZYSTACK_VERSION)" -o _out/bin/cozypkg ./cmd/cozypkg
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/pull-requests.yamlMakefilecmd/cozypkg/cmd/root.gohack/e2e-install-cozystack.batshack/upload-assets.shpackages/core/installer/templates/crds.yamlpackages/core/testing/Makefile
💤 Files with no reviewable changes (1)
- packages/core/installer/templates/crds.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/testing/Makefile
- cmd/cozypkg/cmd/root.go
- hack/e2e-install-cozystack.bats
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-26T04:29:24.830Z
Learnt from: NickVolynkin
Repo: cozystack/cozystack PR: 1117
File: packages/apps/mysql/Makefile:8-8
Timestamp: 2025-06-26T04:29:24.830Z
Learning: The cozystack project uses yq v4+ on their CI runner, so yq v4 syntax (-o json --indent 4) is compatible and version checks are not needed.
Applied to files:
hack/upload-assets.sh
📚 Learning: 2025-12-25T09:45:26.524Z
Learnt from: CR
Repo: cozystack/cozystack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T09:45:26.524Z
Learning: Applies to pkg/**/*.go : Follow controller-runtime patterns and kubebuilder style for Go code in pkg directory
Applied to files:
Makefile
⏰ 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). (1)
- GitHub Check: Build
🔇 Additional comments (10)
Makefile (4)
38-46: LGTM!The split of the manifests target into separate CRDs and operator YAML files is clean and follows the PR objectives. Directory creation is handled on line 39.
56-57: LGTM!The checksum generation using a subshell with
cdensures the checksum file contains relative paths, which is the correct approach for portable checksums.
3-3: LGTM!The include for common environment variables centralizes version management.
51-54: LGTM!The assets target properly aggregates sub-targets, and assets-talos correctly delegates to the package-specific Makefile.
hack/upload-assets.sh (1)
6-7: LGTM!The upload script correctly reflects the split from a single installer manifest to separate CRDs and operator manifests, aligning with the Makefile changes.
.github/workflows/pull-requests.yaml (5)
74-84: LGTM!The upload steps are correctly split into separate artifacts for CRDs and operator, with clear naming that aligns with the Makefile output paths.
97-99: LGTM!The output declarations are correctly updated to expose the three asset IDs needed by downstream jobs.
142-151: LGTM!The asset lookup and validation logic correctly handles the split assets. The validation on line 145 ensures all three required assets are present before proceeding.
236-248: LGTM!The download steps for regular PRs correctly fetch both CRDs and operator artifacts into the expected
_out/assetsdirectory.
251-262: LGTM!The release PR path correctly downloads both assets from the draft release using the resolved asset IDs. The curl commands properly handle authentication and output paths.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| gh release upload --clobber $version _out/assets/cozypkg-*.tar.gz | ||
| gh release upload --clobber $version _out/assets/cozypkg-checksums.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the upload-assets.sh file
fd upload-assets.shRepository: cozystack/cozystack
Length of output: 84
🏁 Script executed:
# Read the full script to understand context
cat -n hack/upload-assets.shRepository: cozystack/cozystack
Length of output: 843
🏁 Script executed:
# Check if there's a Makefile or build script that shows dependencies
fd -e Makefile -e makefile | head -5Repository: cozystack/cozystack
Length of output: 45
🏁 Script executed:
# Let's also check the gh release upload documentation and behavior
web_search
gh release upload command glob pattern behavior shell expansionRepository: cozystack/cozystack
Length of output: 139
🏁 Script executed:
# Look for Makefile or build scripts
find . -maxdepth 3 -type f \( -name "Makefile" -o -name "*.mk" -o -name "*.make" \) | head -10Repository: cozystack/cozystack
Length of output: 225
🏁 Script executed:
# Check if there's a target that creates cozypkg assets
grep -r "cozypkg-" --include="Makefile" --include="*.mk" | head -20Repository: cozystack/cozystack
Length of output: 749
🏁 Script executed:
# Check the _out/assets directory structure if it exists or is documented
find . -path "*_out/assets*" -o -name "*assets*" -type d | head -10Repository: cozystack/cozystack
Length of output: 45
🏁 Script executed:
# Search for how cozypkg assets are generated
rg "cozypkg.*tar\.gz" --type sh --type makefile -B 3 -A 3Repository: cozystack/cozystack
Length of output: 95
🏁 Script executed:
# Search for how upload-assets.sh is invoked and whether assets-cozypkg is a dependency
grep -r "upload-assets" --include="Makefile" -B 5 -A 5Repository: cozystack/cozystack
Length of output: 200
🏁 Script executed:
# Check the main Makefile to see the relationship between asset targets and upload
cat -n Makefile | head -100Repository: cozystack/cozystack
Length of output: 3774
🏁 Script executed:
# Look for any CI/CD files that might show the sequence of commands
find . -maxdepth 2 -type f \( -name "*.yml" -o -name "*.yaml" \) | head -10Repository: cozystack/cozystack
Length of output: 119
🏁 Script executed:
# Let's verify the exact shell behavior when glob doesn't match
# Create a test scenario
bash -c 'set -xe; mkdir -p /tmp/test_glob && cd /tmp/test_glob && echo "Testing glob with no matches:" && ls *.nonexistent 2>&1; echo "Exit code: $?"'Repository: cozystack/cozystack
Length of output: 276
🏁 Script executed:
# Also check what happens when passing the glob string to a command
bash -c 'set -xe; mkdir -p /tmp/test_glob && cd /tmp/test_glob && echo "file1.tar.gz" > file1.tar.gz && echo "Testing glob with matches:" && echo _out/assets/cozypkg-*.tar.gz; echo "Testing glob with no matches:" && echo _out/assets/cozypkg-*.tar.gz 2>&1 || true'Repository: cozystack/cozystack
Length of output: 411
🏁 Script executed:
# Verify the upload_assets target dependencies in the Makefile
grep -A 3 "^upload_assets:" MakefileRepository: cozystack/cozystack
Length of output: 110
🏁 Script executed:
# Check if there's any other build orchestration that might ensure assets-cozypkg runs first
grep -r "upload.assets" . --include="*.yml" --include="*.yaml" --include="Makefile" -B 5 -A 5Repository: cozystack/cozystack
Length of output: 1284
Add assets-cozypkg as an explicit dependency for the upload_assets Makefile target.
The glob pattern expansion is correct in the shell, but the upload_assets target in the Makefile only depends on manifests and not on assets-cozypkg, allowing hack/upload-assets.sh to be invoked without the required tar.gz files being built. With set -xe in the script, this would cause gh release upload to fail when called with the literal unmatched glob string. The GitHub workflow mitigates this by explicitly running make assets first, but the Makefile target itself should declare the dependency. Change line 82 in the Makefile from:
upload_assets: manifests
to:
upload_assets: manifests assets-cozypkg
🤖 Prompt for AI Agents
In `@hack/upload-assets.sh` around lines 13 - 14, The Makefile target
upload_assets currently only depends on manifests, which allows
hack/upload-assets.sh to run before assets-cozypkg is built and may cause gh
release upload to fail on unmatched glob patterns; update the Makefile so the
upload_assets target declares assets-cozypkg as an explicit dependency (i.e.,
make upload_assets depend on both manifests and assets-cozypkg) so the tar.gz
artifacts are built before hack/upload-assets.sh runs.
| assets-cozypkg-%: | ||
| $(eval EXT := $(if $(filter windows,$(firstword $(subst -, ,$*))),.exe,)) | ||
| mkdir -p _out/assets | ||
| GOOS=$(firstword $(subst -, ,$*)) GOARCH=$(lastword $(subst -, ,$*)) go build -ldflags "-X github.com/cozystack/cozystack/cmd/cozypkg/cmd.Version=v$(COZYSTACK_VERSION)" -o _out/bin/cozypkg-$*/cozypkg$(EXT) ./cmd/cozypkg | ||
| cp LICENSE _out/bin/cozypkg-$*/LICENSE | ||
| tar -C _out/bin/cozypkg-$* -czf _out/assets/cozypkg-$*.tar.gz LICENSE cozypkg$(EXT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing directory creation for _out/bin/cozypkg-$*.
The mkdir -p on line 61 only creates _out/assets, but the go build on line 62 outputs to _out/bin/cozypkg-$*/cozypkg$(EXT), which will fail if that directory doesn't exist. Similarly, the cp LICENSE on line 63 will fail.
Proposed fix
assets-cozypkg-%:
$(eval EXT := $(if $(filter windows,$(firstword $(subst -, ,$*))),.exe,))
- mkdir -p _out/assets
+ mkdir -p _out/assets _out/bin/cozypkg-$*
GOOS=$(firstword $(subst -, ,$*)) GOARCH=$(lastword $(subst -, ,$*)) go build -ldflags "-X github.com/cozystack/cozystack/cmd/cozypkg/cmd.Version=v$(COZYSTACK_VERSION)" -o _out/bin/cozypkg-$*/cozypkg$(EXT) ./cmd/cozypkg
cp LICENSE _out/bin/cozypkg-$*/LICENSE
tar -C _out/bin/cozypkg-$* -czf _out/assets/cozypkg-$*.tar.gz LICENSE cozypkg$(EXT)📝 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.
| assets-cozypkg-%: | |
| $(eval EXT := $(if $(filter windows,$(firstword $(subst -, ,$*))),.exe,)) | |
| mkdir -p _out/assets | |
| GOOS=$(firstword $(subst -, ,$*)) GOARCH=$(lastword $(subst -, ,$*)) go build -ldflags "-X github.com/cozystack/cozystack/cmd/cozypkg/cmd.Version=v$(COZYSTACK_VERSION)" -o _out/bin/cozypkg-$*/cozypkg$(EXT) ./cmd/cozypkg | |
| cp LICENSE _out/bin/cozypkg-$*/LICENSE | |
| tar -C _out/bin/cozypkg-$* -czf _out/assets/cozypkg-$*.tar.gz LICENSE cozypkg$(EXT) | |
| assets-cozypkg-%: | |
| $(eval EXT := $(if $(filter windows,$(firstword $(subst -, ,$*))),.exe,)) | |
| mkdir -p _out/assets _out/bin/cozypkg-$* | |
| GOOS=$(firstword $(subst -, ,$*)) GOARCH=$(lastword $(subst -, ,$*)) go build -ldflags "-X github.com/cozystack/cozystack/cmd/cozypkg/cmd.Version=v$(COZYSTACK_VERSION)" -o _out/bin/cozypkg-$*/cozypkg$(EXT) ./cmd/cozypkg | |
| cp LICENSE _out/bin/cozypkg-$*/LICENSE | |
| tar -C _out/bin/cozypkg-$* -czf _out/assets/cozypkg-$*.tar.gz LICENSE cozypkg$(EXT) |
🤖 Prompt for AI Agents
In `@Makefile` around lines 59 - 64, The assets-cozypkg-% target uses GO build to
write to _out/bin/cozypkg-$*/cozypkg$(EXT) and then cp LICENSE there, but only
_out/assets is created; add a step to create the output directory (e.g., mkdir
-p _out/bin/cozypkg-$*) before the go build and cp so the build output and copy
succeed; update the assets-cozypkg-% recipe (referencing the target name and
variables EXT and the path _out/bin/cozypkg-$*/) to ensure the directory exists.
- Add pattern rule for building cozypkg binaries per platform (assets-cozypkg-<os>-<arch>) with checksums generation - Add Version variable to cozypkg CLI injected via ldflags - Split manifests into separate cozystack-crds.yaml and cozystack-operator.yaml files - Update CI workflow to handle CRDs and operator as separate artifacts - Update e2e tests and upload script for new manifest structure - Suppress git describe stderr when no tags exist Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
e557b23 to
6dbfcbb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Makefile`:
- Around line 48-50: The cozypkg Makefile target currently runs go build to
output _out/bin/cozypkg but doesn't ensure the _out/bin directory exists; modify
the cozypkg target (target name: cozypkg) to create the directory first (e.g.,
run a mkdir -p _out/bin) before invoking go build so the output path is
guaranteed to exist and the build won't fail on a clean workspace.
♻️ Duplicate comments (1)
Makefile (1)
59-65: Missing directory creation for_out/bin/cozypkg-$*.Line 61 only creates
_out/assets, but thego buildon line 62 andcpon line 63 write to_out/bin/cozypkg-$*/, which doesn't exist. This will cause the build to fail.Proposed fix
assets-cozypkg-%: $(eval EXT := $(if $(filter windows,$(firstword $(subst -, ,$*))),.exe,)) - mkdir -p _out/assets + mkdir -p _out/assets _out/bin/cozypkg-$* GOOS=$(firstword $(subst -, ,$*)) GOARCH=$(lastword $(subst -, ,$*)) go build -ldflags "-X github.com/cozystack/cozystack/cmd/cozypkg/cmd.Version=v$(COZYSTACK_VERSION)" -o _out/bin/cozypkg-$*/cozypkg$(EXT) ./cmd/cozypkg cp LICENSE _out/bin/cozypkg-$*/LICENSE tar -C _out/bin/cozypkg-$* -czf _out/assets/cozypkg-$*.tar.gz LICENSE cozypkg$(EXT)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Makefilepackages/core/testing/Makefile
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/testing/Makefile
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-25T09:45:26.524Z
Learnt from: CR
Repo: cozystack/cozystack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T09:45:26.524Z
Learning: Applies to pkg/**/*.go : Follow controller-runtime patterns and kubebuilder style for Go code in pkg directory
Applied to files:
Makefile
⏰ 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). (1)
- GitHub Check: Build
🔇 Additional comments (3)
Makefile (3)
40-46: LGTM!The manifest generation is cleanly split into CRDs and operator components. The
-sflag usage correctly selects specific templates for each output file.
51-54: LGTM!Clean composition of asset targets with proper delegation.
56-57: LGTM!Good coverage of common platforms and architectures. The checksum generation with relative paths is the right approach for release artifacts.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| cozypkg: | ||
| go build -ldflags "-X github.com/cozystack/cozystack/cmd/cozypkg/cmd.Version=v$(COZYSTACK_VERSION)" -o _out/bin/cozypkg ./cmd/cozypkg | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing directory creation for _out/bin.
The go build command outputs to _out/bin/cozypkg, but this directory may not exist on a clean build, causing the build to fail.
Proposed fix
cozypkg:
+ mkdir -p _out/bin
go build -ldflags "-X github.com/cozystack/cozystack/cmd/cozypkg/cmd.Version=v$(COZYSTACK_VERSION)" -o _out/bin/cozypkg ./cmd/cozypkg📝 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.
| cozypkg: | |
| go build -ldflags "-X github.com/cozystack/cozystack/cmd/cozypkg/cmd.Version=v$(COZYSTACK_VERSION)" -o _out/bin/cozypkg ./cmd/cozypkg | |
| cozypkg: | |
| mkdir -p _out/bin | |
| go build -ldflags "-X github.com/cozystack/cozystack/cmd/cozypkg/cmd.Version=v$(COZYSTACK_VERSION)" -o _out/bin/cozypkg ./cmd/cozypkg |
🤖 Prompt for AI Agents
In `@Makefile` around lines 48 - 50, The cozypkg Makefile target currently runs go
build to output _out/bin/cozypkg but doesn't ensure the _out/bin directory
exists; modify the cozypkg target (target name: cozypkg) to create the directory
first (e.g., run a mkdir -p _out/bin) before invoking go build so the output
path is guaranteed to exist and the build won't fail on a clean workspace.
What this PR does
assets-cozypkg-<os>-<arch>) with checksums generationcozystack-crds.yamlandcozystack-operator.yamlfilesRelease note
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.