Skip to content

refactor: Further simplify Makefiles #2073

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.

Sign up for GitHub

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

Open
Camila-B wants to merge 3 commits into
base: main
Choose a base branch
from
Open

refactor: Further simplify Makefiles #2073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
80b2edb
Remove deprecated aliases
Camila-B Feb 25, 2026
7b0a62a
Create pattern rule for %-in-docker
Camila-B Feb 25, 2026
d930343
Remove unused variables
Camila-B Feb 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,6 @@ IMAGES := \
$(ASKPASS_IMAGE) \
$(RESOURCE_GROUP_IMAGE)

# nomos binary for local run.
NOMOS_LOCAL := $(BIN_DIR)/linux_amd64/nomos

# Allows an interactive docker build or test session to be interrupted
# by Ctrl-C. This must be turned off in case of non-interactive runs,
# like in CI/CD.
Expand Down Expand Up @@ -299,6 +296,11 @@ all: buildenv-dirs
@docker run $(DOCKER_RUN_ARGS) \
make all-local

# Run any make target in the docker buildenv container
# e.g. make clientgen-in-docker -> docker run ... make clientgen
%-in-docker: buildenv-dirs
@docker run $(DOCKER_RUN_ARGS) make $*
Copy link

@gemini-code-assist gemini-code-assist bot Feb 25, 2026

Choose a reason for hiding this comment

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

security-medium medium

The pattern rule %-in-docker uses the stem $* directly in a shell command without quoting. If the target name contains shell metacharacters (e.g., ;, &, |), it can lead to arbitrary command execution on the host running make. For example, running make 'foo; touch /tmp/pwned-in-docker' would execute touch /tmp/pwned on the host. To remediate this, quote the $* variable.

	@docker run $(DOCKER_RUN_ARGS) make "$*"

Copy link
Contributor

@tiffanny29631 tiffanny29631 Mar 2, 2026

Choose a reason for hiding this comment

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

Mostly lgtm, please address this.


.PHONY: all-local
# Run tests, cleanup dependencies, and generate CRDs locally
all-local: test deps clientgen configsync-crds
Expand Down Expand Up @@ -352,8 +354,7 @@ __test-presubmit: all-local

# This is the entrypoint used by the ProwJob - runs using docker-in-docker.
.PHONY: test-presubmit
test-presubmit: pull-buildenv
@docker run $(DOCKER_RUN_ARGS) make __test-presubmit
test-presubmit: pull-buildenv __test-presubmit-in-docker

# Runs all tests.
# This only runs on local dev environment not CI environment.
Expand Down
16 changes: 0 additions & 16 deletions Makefile.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,6 @@ $(BUILD_IMAGE_TARGETS): "$(HELM)" "$(KUSTOMIZE)"
.PHONY: build-images
build-images: $(BUILD_IMAGE_TARGETS)

# Deprecated alias of build-images. Remove this once unused.
.PHONY: build-images-multirepo
build-images-multirepo: build-images

.PHONY: auth-docker
auth-docker:
@echo "+++ Using account:"
Expand All @@ -102,10 +98,6 @@ $(PUSH_IMAGE_TARGETS): auth-docker
.PHONY: push-images
push-images: $(PUSH_IMAGE_TARGETS)

# Deprecated alias of push-images. Remove this once unused.
.PHONY: push-images-multirepo
push-images-multirepo: push-images

# Targets for pulling individual images
PULL_IMAGE_TARGETS := $(patsubst %,__pull-image-%,$(IMAGES))

Expand All @@ -117,10 +109,6 @@ $(PULL_IMAGE_TARGETS): auth-docker
.PHONY: pull-images
pull-images: $(PULL_IMAGE_TARGETS)

# Deprecated alias of pull-images. Remove this once unused.
.PHONY: pull-images-multirepo
pull-images-multirepo: pull-images

# Targets for retagging individual images
RETAG_IMAGE_TARGETS := $(patsubst %,__retag-image-%,$(IMAGES))

Expand All @@ -132,10 +120,6 @@ $(RETAG_IMAGE_TARGETS):
.PHONY: retag-images
retag-images: $(RETAG_IMAGE_TARGETS)

# Deprecated alias of retag-images. Remove this once unused.
.PHONY: retag-images-multirepo
retag-images-multirepo: retag-images

###################################
# Config Sync manifest
###################################
Expand Down
2 changes: 0 additions & 2 deletions Makefile.e2e
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ E2E_PARAMS := \
GCP_PROJECT=$(GCP_PROJECT) \
REGISTRY=$(REGISTRY) \

GOTOPT2_BINARY := docker run -i -u $(UID):$(GID) $(BUILDENV_IMAGE) /bin/gotopt2

GCP_ZONE ?= us-central1-a
GKE_E2E_TIMEOUT ?= 6h
# Configurable timeout for running the e2e tests on kind clusters.
Expand Down
6 changes: 0 additions & 6 deletions Makefile.gen
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,6 @@ clientgen: symlink-gopath $(INSTALL_CLIENTGEN_TOOLS)
@echo "+++ Generating clientgen directory"
./scripts/generate-clientset.sh

.PHONY: clientgen-in-docker
# Run make clientgen in the docker buildenv container
clientgen-in-docker: buildenv-dirs
@docker run $(DOCKER_RUN_ARGS) \
make clientgen

# To regenerate api_resources_1_15.txt:
# 1) Create a fresh 1.15 cluster that DOES NOT have ACM installed.
# 2) Run the below command:
Expand Down
27 changes: 7 additions & 20 deletions Makefile.reconcilermanager
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@

CONTROLLER_GEN_PATHS := \
paths="./pkg/api/configsync/v1alpha1" \
paths="./pkg/api/configsync/v1beta1" \
paths="./pkg/api/configmanagement/v1" \
paths="./pkg/api/kpt.dev/v1alpha1"

.PHONY: generate
# Generate DeepCopy and runtime.Object implementation methods.
generate: "$(CONTROLLER_GEN)"
"$(CONTROLLER_GEN)" \
object:headerFile="hack/boilerplate.txt" \
paths="./pkg/api/configsync/v1alpha1" \
paths="./pkg/api/configsync/v1beta1" \
paths="./pkg/api/kpt.dev/v1alpha1" \
paths="./pkg/api/configmanagement/v1"
$(CONTROLLER_GEN_PATHS)

.PHONY: configsync-crds
# Generate configsync CRDs and then patch them with kustomize
configsync-crds: "$(CONTROLLER_GEN)" "$(KUSTOMIZE)" "$(ADDLICENSE)"
"$(CONTROLLER_GEN)" crd \
paths="./pkg/api/configsync/v1alpha1" \
paths="./pkg/api/configsync/v1beta1" \
paths="./pkg/api/configmanagement/v1" \
paths="./pkg/api/kpt.dev/v1alpha1" \
$(CONTROLLER_GEN_PATHS) \
output:artifacts:config=./manifests \
&& mv ./manifests/configsync.gke.io_reposyncs.yaml ./manifests/patch/reposync-crd.yaml \
&& mv ./manifests/configsync.gke.io_rootsyncs.yaml ./manifests/patch/rootsync-crd.yaml \
Expand Down Expand Up @@ -51,15 +50,3 @@ install-controller-gen: "$(CONTROLLER_GEN)"
.PHONY: clean-controller-gen
clean-controller-gen:
@rm -rf $(CONTROLLER_GEN)

.PHONY: generate-in-docker
# Run make generate-in-docker in the docker buildenv container
generate-in-docker: buildenv-dirs
@docker run $(DOCKER_RUN_ARGS) \
make generate

.PHONY: configsync-crds-in-docker
# Run make configsync-crds in the docker buildenv container
configsync-crds-in-docker: buildenv-dirs
@docker run $(DOCKER_RUN_ARGS) \
make configsync-crds