Skip to content

Conversation

kvaps
Copy link
Member

@kvaps kvaps commented Sep 5, 2025

Signed-off-by: Andrei Kvapil kvapss@gmail.com

What this PR does

It does not work well anyway.

Release note

[dx] Remove BUILDER and PLATFORM autodetection logic

Summary by CodeRabbit

  • Refactor
    • Consolidated Docker Buildx flags into a single configurable argument across all image build targets, keeping tagging, caching, and metadata behavior unchanged.
  • Chores
    • Added configurable environment variables for builds (e.g., builder, platform, extra args, tag) to standardize and simplify configuration.
    • Removed automatic builder/platform detection; these can now be explicitly set when needed, making builds more predictable and customizable.

Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Copy link
Contributor

coderabbitai bot commented Sep 5, 2025

Walkthrough

Makefile targets across apps, core, extra, and system replace explicit docker buildx flags with a single $(BUILDX_ARGS). scripts/common-envs.mk introduces TAG/BUILDER/PLATFORM/BUILDX_EXTRA_ARGS and composes BUILDX_ARGS, removing previous builder/platform auto-detection. Existing tagging, caching, metadata, and post-build digest handling remain unchanged.

Changes

Cohort / File(s) Summary of changes
Buildx args consolidation — Apps
packages/apps/clickhouse/Makefile, packages/apps/http-cache/Makefile, packages/apps/kubernetes/Makefile, packages/apps/mysql/Makefile
Replaced per-target hard-coded docker buildx flags (provenance, builder, platform, push, label, load) with $(BUILDX_ARGS); retained tag/cache/metadata and digest/tag logic.
Buildx args consolidation — Core
packages/core/installer/Makefile, packages/core/testing/Makefile
Switched build invocations to $(BUILDX_ARGS); kept existing tag/cache/metadata and digest processing unchanged.
Buildx args consolidation — Extra
packages/extra/monitoring/Makefile
Centralized buildx options into $(BUILDX_ARGS); other build steps unchanged.
Buildx args consolidation — System
packages/system/bucket/Makefile, packages/system/cilium/Makefile, packages/system/cozystack-api/Makefile, packages/system/cozystack-controller/Makefile, packages/system/dashboard/Makefile, packages/system/kamaji/Makefile, packages/system/kubeovn-webhook/Makefile, packages/system/kubeovn/Makefile, packages/system/metallb/Makefile, packages/system/objectstorage-controller/Makefile
Unified docker buildx flags via $(BUILDX_ARGS) across image targets; preserved tag/cache/metadata and post-build digest usage and value updates.
Common build envs refactor
scripts/common-envs.mk
Added TAG, BUILDER, PLATFORM, BUILDX_EXTRA_ARGS, and composite BUILDX_ARGS; removed builder/platform auto-detection; moved TAG definition earlier; BUILDX_ARGS now standardizes provenance/push/load/label and optional builder/platform.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev/CI
  participant Make as Makefile Target
  participant Env as scripts/common-envs.mk
  participant Buildx as docker buildx
  participant Reg as Registry

  Dev/CI->>Make: make image-*
  Make->>Env: include and resolve TAG/BUILDER/PLATFORM/BUILDX_ARGS
  Env-->>Make: export BUILDX_ARGS (provenance/push/load/label + optional builder/platform + extra)
  Make->>Buildx: buildx build $(BUILDX_ARGS) --tag ... --cache-* --metadata-file ...
  alt push enabled
    Buildx->>Reg: push image
  else load enabled
    Buildx-->>Make: load image locally
  end
  Buildx-->>Make: metadata (digest)
  Make->>Make: parse digest, write tag files / update values.yaml
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

ok-to-test

Suggested reviewers

  • klinch0
  • lllamnyp

Poem

I thump my paw at make-time cheer,
One flag to rule the builds this year.
From apps to systems, hops align,
Args in a bundle—oh how fine!
I nibble tags, digest in sight,
Cache warmed, images take flight. 🥕🐇

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor-makefiles

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

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

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

Support

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

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

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

CodeRabbit Configuration File (.coderabbit.yaml)

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

Status, Documentation and Community

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

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @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 refactors the image building process by centralizing Docker Buildx arguments into a single variable and removing the automatic detection of builder and platform settings. The change aims to simplify and standardize build commands across various components, making the build system more maintainable and predictable.

Highlights

  • Buildx Argument Consolidation: The pull request introduces a new BUILDX_ARGS variable in scripts/common-envs.mk to centralize and manage common docker buildx build arguments. This simplifies individual Makefile targets by replacing multiple explicit flags with a single variable.
  • Removal of Autodetection Logic: The previous autodetection logic for BUILDER and PLATFORM variables, which relied on docker buildx inspect and jq, has been removed from scripts/common-envs.mk. These variables are now optionally set, and if not provided, buildx will use its default behavior.
  • Streamlined Makefile Targets: Across numerous Makefile targets responsible for building container images, redundant docker buildx build flags such as --provenance, --builder, --platform, --push, --label, and --load have been removed and replaced with the new $(BUILDX_ARGS) variable. This significantly cleans up and standardizes the image build commands.
Using Gemini Code Assist

The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a 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 refactors the Docker image build process by removing the automatic detection of BUILDER and PLATFORM from the Makefiles. Instead, it introduces a centralized BUILDX_ARGS variable in scripts/common-envs.mk to hold common docker buildx arguments. This change simplifies the individual Makefiles, reduces code duplication, and makes the build process more explicit and maintainable. The refactoring is applied consistently across all relevant packages. As a positive side effect, this change also fixes latent bugs in a few Makefiles where build arguments were incorrectly specified.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/system/dashboard/Makefile (1)

21-27: BUG: ‘tag’ variable assignment is commented out, subsequent uses are empty
Lines 22 and 26 use $${tag} but Line 21 is commented, so tag is unset → wget/grep/sed break.

Apply:

-	#tag=$$(git ls-remote --tags --sort="v:refname" https://github.com/vmware-tanzu/kubeapps  | awk -F'[/^]' 'END{print $$3}') && \
+	tag=$$(git ls-remote --tags --sort="v:refname" https://github.com/vmware-tanzu/kubeapps  | awk -F'[/^]' 'END{print $$3}') && \

Optional hardening:

-	wget https://github.com/vmware-tanzu/kubeapps/raw/$${tag}/cmd/kubeapps-apis/Dockerfile -O images/kubeapps-apis/Dockerfile && \
+	test -n "$$tag" || { echo "No kubeapps tag found"; exit 1; } ; \
+	wget https://github.com/vmware-tanzu/kubeapps/raw/$${tag}/cmd/kubeapps-apis/Dockerfile -O images/kubeapps-apis/Dockerfile && \
🧹 Nitpick comments (8)
scripts/common-envs.mk (2)

5-7: Expose new knobs clearly (docs/help)
With autodetection removed, users must set BUILDER/PLATFORM explicitly. Please add a short README blurb or a make “help” target showing examples:

  • BUILDER=mybuilder
  • PLATFORM=linux/amd64,linux/arm64
  • BUILDX_EXTRA_ARGS=--progress=plain

6-6: Strip trailing whitespace
Minor style nit; “PLATFORM ?=” currently has a trailing space which is easy to propagate in diffs.

-PLATFORM ?= 
+PLATFORM ?=
packages/apps/mysql/Makefile (1)

15-15: Consistent adoption of $(BUILDX_ARGS) — check digest JSON compatibility

Looks good. Ensure the metadata JSON still contains "containerimage.digest" under your current buildx/provenance settings; otherwise yq extraction will break.

If you foresee per-target extras (e.g., build-arg overrides), confirm BUILDX_ARGS appends BUILDX_EXTRA_ARGS so targets can opt-in without touching Makefiles.

packages/core/testing/Makefile (1)

23-23: e2e-sandbox developer UX — default to --load for local runs

For this dev-heavy target, ensure BUILDX_ARGS chooses --load by default when REGISTRY or PUSH isn’t set, so docker run later can pull from the local daemon without requiring a registry push.

Document in scripts/common-envs.mk how to flip between load/push (e.g., PUSH=1 make image-e2e-sandbox).

packages/extra/monitoring/Makefile (1)

17-17: Centralized flags: parity check and minor Make hygiene

Change looks fine; same parity checks as other targets apply.

Consider marking phony targets to avoid timestamp pitfalls:

  • .PHONY: image generate

Optional, low impact.

packages/system/kamaji/Makefile (1)

20-20: Add a fail-fast guard for missing containerimage.digest
Ensure images/kamaji.json contains .containerimage.digest after the build to avoid writing an invalid tag. For example, right before updating values.yaml add:

DIGEST="$(yq -r '."containerimage.digest" // empty' images/kamaji.json)"
[ -n "$DIGEST" ] || { echo "Missing containerimage.digest (did you --push?)"; exit 1; }
packages/apps/http-cache/Makefile (1)

14-14: Same here: keep command robust if BUILDX_ARGS is empty and validate digest availability

  • Prevent a stray backslash if BUILDX_ARGS is empty by moving it before the last flag.
-        --metadata-file images/nginx-cache.json \
-        $(BUILDX_ARGS)
+        $(BUILDX_ARGS) \
+        --metadata-file images/nginx-cache.json
  • The subsequent echo relies on containerimage.digest. Verify BUILDX_ARGS ensures a push/registry export; otherwise images/nginx-cache.json may lack the digest.
packages/system/cozystack-api/Makefile (1)

15-15: Consolidation looks good; add two sanity checks

  • Move BUILDX_ARGS before the final flag to avoid a trailing backslash hazard if it’s empty.
-        --metadata-file images/cozystack-api.json \
-        $(BUILDX_ARGS)
+        $(BUILDX_ARGS) \
+        --metadata-file images/cozystack-api.json
  • Ensure scripts/common-envs.mk composes BUILDX_ARGS with:
    • a source label (org.opencontainers.image.source) and desired provenance policy,
    • mutually compatible options (e.g., disallow --load with multi-arch --platform lists),
    • a push/registry export so images/cozystack-api.json has containerimage.digest used in values.yaml.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7090b8a and 9f2b98d.

📒 Files selected for processing (18)
  • packages/apps/clickhouse/Makefile (1 hunks)
  • packages/apps/http-cache/Makefile (1 hunks)
  • packages/apps/kubernetes/Makefile (2 hunks)
  • packages/apps/mysql/Makefile (1 hunks)
  • packages/core/installer/Makefile (2 hunks)
  • packages/core/testing/Makefile (1 hunks)
  • packages/extra/monitoring/Makefile (1 hunks)
  • packages/system/bucket/Makefile (1 hunks)
  • packages/system/cilium/Makefile (1 hunks)
  • packages/system/cozystack-api/Makefile (1 hunks)
  • packages/system/cozystack-controller/Makefile (1 hunks)
  • packages/system/dashboard/Makefile (2 hunks)
  • packages/system/kamaji/Makefile (1 hunks)
  • packages/system/kubeovn-webhook/Makefile (1 hunks)
  • packages/system/kubeovn/Makefile (1 hunks)
  • packages/system/metallb/Makefile (1 hunks)
  • packages/system/objectstorage-controller/Makefile (1 hunks)
  • scripts/common-envs.mk (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: nbykov0
PR: cozystack/cozystack#932
File: packages/system/dashboard/images/kubeapps-apis/Dockerfile:15-17
Timestamp: 2025-05-14T16:58:01.478Z
Learning: When working with Dockerfiles for cross-architecture builds in the kubeapps-apis component, the `TARGETOS` and `TARGETARCH` build arguments should be declared without default values so that Docker's buildx system can automatically set these variables based on the specified target platform.
Learnt from: nbykov0
PR: cozystack/cozystack#932
File: packages/core/testing/images/e2e-sandbox/Dockerfile:7-8
Timestamp: 2025-05-14T16:57:29.651Z
Learning: In Dockerfiles supporting cross-architecture builds, `TARGETOS` and `TARGETARCH` are special build arguments that should be declared without default values. Docker's buildx system automatically sets these variables based on the specified target platform during cross-platform builds.
Learnt from: nbykov0
PR: cozystack/cozystack#932
File: packages/apps/kubernetes/images/cluster-autoscaler/Dockerfile:3-4
Timestamp: 2025-05-14T16:54:50.368Z
Learning: In Dockerfiles for the cozystack project, ARG ordering where a variable like ${TARGETARCH} is referenced before being declared can still work correctly in practice because Docker's BuildKit may evaluate the variable only when it's actually used in a FROM statement.
📚 Learning: 2025-05-14T16:58:01.478Z
Learnt from: nbykov0
PR: cozystack/cozystack#932
File: packages/system/dashboard/images/kubeapps-apis/Dockerfile:15-17
Timestamp: 2025-05-14T16:58:01.478Z
Learning: When working with Dockerfiles for cross-architecture builds in the kubeapps-apis component, the `TARGETOS` and `TARGETARCH` build arguments should be declared without default values so that Docker's buildx system can automatically set these variables based on the specified target platform.

Applied to files:

  • packages/apps/http-cache/Makefile
  • packages/system/cilium/Makefile
  • packages/system/kubeovn-webhook/Makefile
  • packages/extra/monitoring/Makefile
  • packages/system/objectstorage-controller/Makefile
  • packages/apps/mysql/Makefile
  • packages/system/cozystack-api/Makefile
  • packages/apps/clickhouse/Makefile
  • packages/core/installer/Makefile
  • packages/system/dashboard/Makefile
  • packages/system/kamaji/Makefile
  • packages/system/cozystack-controller/Makefile
  • scripts/common-envs.mk
  • packages/system/kubeovn/Makefile
  • packages/system/metallb/Makefile
  • packages/apps/kubernetes/Makefile
📚 Learning: 2025-05-14T16:57:29.651Z
Learnt from: nbykov0
PR: cozystack/cozystack#932
File: packages/core/testing/images/e2e-sandbox/Dockerfile:7-8
Timestamp: 2025-05-14T16:57:29.651Z
Learning: In Dockerfiles supporting cross-architecture builds, `TARGETOS` and `TARGETARCH` are special build arguments that should be declared without default values. Docker's buildx system automatically sets these variables based on the specified target platform during cross-platform builds.

Applied to files:

  • packages/system/cilium/Makefile
  • packages/extra/monitoring/Makefile
  • packages/core/installer/Makefile
  • packages/system/dashboard/Makefile
  • scripts/common-envs.mk
  • packages/system/metallb/Makefile
📚 Learning: 2025-05-14T16:54:50.368Z
Learnt from: nbykov0
PR: cozystack/cozystack#932
File: packages/apps/kubernetes/images/cluster-autoscaler/Dockerfile:3-4
Timestamp: 2025-05-14T16:54:50.368Z
Learning: In Dockerfiles for the cozystack project, ARG ordering where a variable like ${TARGETARCH} is referenced before being declared can still work correctly in practice because Docker's BuildKit may evaluate the variable only when it's actually used in a FROM statement.

Applied to files:

  • packages/system/cozystack-api/Makefile
  • packages/system/cozystack-controller/Makefile
  • scripts/common-envs.mk
⏰ 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 (14)
scripts/common-envs.mk (1)

2-2: LGTM: TAG computation
Exact-match tag or “latest” fallback is sane and consistent with existing targets.

packages/system/dashboard/Makefile (2)

35-35: LGTM: Centralized Buildx flags
Switching to $(BUILDX_ARGS) keeps the command concise and consistent.


52-52: LGTM: Centralized Buildx flags
Consistent with the repo-wide pattern.

packages/system/cozystack-controller/Makefile (1)

15-15: LGTM: $(BUILDX_ARGS) adoption
Keeps flags centralized; digest wiring remains intact.

packages/system/objectstorage-controller/Makefile (1)

25-25: LGTM: $(BUILDX_ARGS) adoption for multi-target build
Consistent with the refactor; TARGET/stage selection unaffected.

packages/system/kubeovn/Makefile (1)

27-27: LGTM: $(BUILDX_ARGS) adoption
Centralizes build flags; existing dual tagging and digest update remain unchanged.

packages/apps/kubernetes/Makefile (1)

21-21: Centralized Buildx flags verified
All build targets now use $(BUILDX_ARGS) exclusively; scripts/common-envs.mk defines --provenance=false, --push/--load (defaults to exactly one) and optional --platform/--builder, with no residual flags in individual Makefiles.

packages/system/cilium/Makefile (1)

25-25: No changes needed: BUILDX_ARGS doesn’t include or override --metadata-file, and the explicit --metadata-file images/cilium.json remains in the Makefile.

packages/apps/clickhouse/Makefile (1)

15-15: Verify default values for PUSH/LOAD
BUILDX_ARGS uses --push=$(PUSH) and --load=$(LOAD) as defined in scripts/common-envs.mk. Ensure that PUSH and LOAD default to the same values as before (e.g., CI wasn’t implicitly setting PUSH=true or LOAD=false). If previous CI runs relied on unconditionally pushing, explicitly set PUSH=1 (or true) in the Make invocation.

packages/system/metallb/Makefile (1)

24-24: Ensure CI explicitly passes PLATFORM or BUILDX_EXTRA_ARGS for multi-arch builds
The default PLATFORM is unset (see common-envs.mk), so $(BUILDX_ARGS) won’t include --platform=… unless CI exports PLATFORM or adds it via BUILDX_EXTRA_ARGS. Verify your pipeline invokes make with PLATFORM set (or injects it through BUILDX_EXTRA_ARGS) to restore multi-arch outputs.

packages/system/bucket/Makefile (1)

19-19: Add PUSH/LOAD environment variables to CI workflow
I don’t see PUSH or LOAD defined in any .github/workflows/*.yml; add them under env: in your build job so BUILDX_ARGS := --push=$(PUSH) --load=$(LOAD)… receives the correct modes.

packages/system/kubeovn-webhook/Makefile (1)

13-13: BUILDX_ARGS parity verified — no changes needed. Definition in scripts/common-envs.mk still includes --provenance=false, the org.opencontainers.image.source label, respects PUSH/LOAD defaults, and conditionally adds --builder/--platform.

packages/core/installer/Makefile (2)

31-31: image-cozystack: Confirm push/load behavior under $(BUILDX_ARGS)

This target feeds values.yaml used by apply flows; a silent switch from --load to --push (or vice versa) would be disruptive. Verify BUILDX_ARGS defaults match prior behavior used by contributors running this target locally.

Reuse the script from the kubeovn-webhook comment to print PUSH/LOAD defaults and BUILDX_ARGS.


49-49: image-matchbox: Multi-tag build — ensure digest mapping is stable

With two --tag flags, the metadata-file should still report a single image digest. Validate that:

  • images/matchbox.json includes containerimage.digest,
  • both tags reference the same digest (buildx manifest logic unchanged).

After a dry run in CI or locally, cat images/matchbox.json and confirm the yq path resolves and matches the pushed tag(s).

Comment on lines +10 to +14
BUILDX_ARGS := --provenance=false --push=$(PUSH) --load=$(LOAD) \
--label org.opencontainers.image.source=https://github.com/cozystack/cozystack \
$(if $(strip $(BUILDER)),--builder=$(BUILDER)) \
$(if $(strip $(PLATFORM)),--platform=$(PLATFORM)) \
$(BUILDX_EXTRA_ARGS)
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Make --push/--load mutually exclusive and avoid boolean flags with values

Buildx treats these as presence flags; passing both or using “=1/0” can misbehave. Gate each flag and error if both are enabled.

Apply within this hunk:

-BUILDX_ARGS := --provenance=false --push=$(PUSH) --load=$(LOAD) \
+BUILDX_ARGS := --provenance=false \
+  $(if $(filter 1 true yes,$(PUSH)),--push) \
+  $(if $(filter 1 true yes,$(LOAD)),--load) \
   --label org.opencontainers.image.source=https://github.com/cozystack/cozystack \
   $(if $(strip $(BUILDER)),--builder=$(BUILDER)) \
   $(if $(strip $(PLATFORM)),--platform=$(PLATFORM)) \
   $(BUILDX_EXTRA_ARGS)

Add guard (outside this hunk, e.g., after Line 7):

# Ensure only one of PUSH/LOAD is enabled
ifneq ($(and $(filter 1 true yes,$(PUSH)),$(filter 1 true yes,$(LOAD))),)
  $(error Both PUSH and LOAD are enabled; enable only one of PUSH or LOAD)
endif
🤖 Prompt for AI Agents
In scripts/common-envs.mk around lines 10 to 14, the Makefile currently passes
--push=$(PUSH) and --load=$(LOAD) which treats these as boolean-with-value flags
and can result in both being passed; add a guard after line 7 that errors when
both PUSH and LOAD are truthy (1/true/yes) to make them mutually exclusive, and
change the BUILDX_ARGS hunk to emit presence-only flags (e.g., use conditionals
to include --push when PUSH is set and --load when LOAD is set) instead of using
=$(PUSH)/=$(LOAD) so buildx receives the flags only when enabled.

💡 Verification agent

🧩 Analysis chain

Repo-wide adoption sanity check
Run this to confirm no stragglers keep passing buildx flags directly and all builds include $(BUILDX_ARGS).


🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "1) Find buildx calls missing BUILDX_ARGS (might show context next line):"
rg -nP --type=make -C2 'docker\s+buildx\s+build(?!.*\$\((?:BUILDX_ARGS)\))'

echo -e "\n2) Find direct flag usage outside scripts/common-envs.mk:"
rg -nP --type=make -g '!scripts/common-envs.mk' -- '(--platform|--builder|--provenance|--push(=| )|--load(=| ))'

Length of output: 9489


Include $(BUILDX_ARGS) in every docker buildx build invocation
Numerous Makefiles (e.g. packages/system/cozystack-api/Makefile:10, packages/system/objectstorage-controller/Makefile:19, packages/system/metallb/Makefile:17, and many more under packages/…) still call docker buildx build with hard-coded flags instead of using the shared $(BUILDX_ARGS); update them to docker buildx build $(BUILDX_ARGS) ….

🤖 Prompt for AI Agents
In scripts/common-envs.mk around lines 10 to 14, the shared BUILDX_ARGS variable
is defined but many Makefiles still call docker buildx build with hard-coded
flags; update every docker buildx build invocation across the repo (e.g.,
packages/system/cozystack-api/Makefile:10,
packages/system/objectstorage-controller/Makefile:19,
packages/system/metallb/Makefile:17 and other packages/* Makefiles) to use
docker buildx build $(BUILDX_ARGS) … instead of repeating flags, ensuring any
additional per-target flags are appended after $(BUILDX_ARGS).

@kvaps kvaps requested a review from nbykov0 September 5, 2025 10:02
@kvaps kvaps added the backport Should change be backported on previus release label Sep 5, 2025
@kvaps kvaps merged commit 49d69a5 into main Sep 5, 2025
19 of 20 checks passed
@kvaps kvaps deleted the refactor-makefiles branch September 5, 2025 10:53
Copy link

github-actions bot commented Sep 5, 2025

Successfully created backport PR for release-0.35:

kvaps added a commit that referenced this pull request Sep 5, 2025
…n logic (#1392)

# Description
Backport of #1391 to `release-0.35`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Should change be backported on previus release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant