From 743c047631f2e78e88fc559a2ecf5141d66eb169 Mon Sep 17 00:00:00 2001 From: Nuru Date: Fri, 2 Feb 2024 12:23:20 -0800 Subject: [PATCH] Use installed packages, segregate executables by platform (#375) --- Makefile | 24 ++++++++++++++++- modules/docs/Makefile | 12 ++++----- modules/packages/Makefile | 45 ++++++++++++++++++++++++-------- modules/readme/Makefile | 4 +-- templates/Makefile.build-harness | 2 +- 5 files changed, 66 insertions(+), 21 deletions(-) diff --git a/Makefile b/Makefile index a2a0c14f..5cc09444 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,6 @@ export BUILD_HARNESS_EXTENSIONS_PATH ?= $(BUILD_HARNESS_PATH)/../build-harness-e export BUILD_HARNESS_OS ?= $(OS) export BUILD_HARNESS_ARCH ?= $(shell uname -m | sed 's/x86_64/amd64/g') export SELF ?= $(MAKE) -export PATH := $(BUILD_HARNESS_PATH)/vendor:$(PATH) # Forces auto-init off to avoid invoking the macro on recursive $(MAKE) export BUILD_HARNESS_AUTO_INIT := false @@ -61,6 +60,29 @@ ifneq ($(wildcard $(BUILD_HARNESS_EXTENSIONS_PATH)/modules/*/Makefile*),) -include $(BUILD_HARNESS_EXTENSIONS_PATH)/modules/*/Makefile* endif +# Unless PACKAGES_PREFER_HOST is not "false", add the INSTALL_PATH, which +# is where build-harness installs needed tools, to the PATH, but wait +# until it is set, which may not be the first time through this Makefile. +# There is an incredibly subtle behavior here. Changes to PATH do not +# affect `make` itself, so $(shell ...) will not see the new PATH. +# Even more subtle, simple recipes that do not require a subshell, +# such as `kubectl version`, will NOT see the new PATH. To use binaries +# installed in the INSTALL_PATH, you must use a recipe that forces a subshell, +# such as by using a pipe or compound command, or if nothing else is needed, +# using a no-op command such as `: && kubectl version`. +# To make things even more subtle, this is inconsistent across different +# versions of Gnu make, with disagreement about the correct behavior and +# bugs in the implementation. The above behavior is what we have observed +# with Gnu make 3.81, which is what Apple ships with macOS. Gnu make 4.4.1 +# updates PATH everywhere. We suspect some versions in between update the +# PATH for recipes but not for $(shell ...). +# See: +# - https://savannah.gnu.org/bugs/?10593#comment5 +# - https://savannah.gnu.org/bugs/?56834 +ifneq ($(INSTALL_PATH),) +export PATH := $(if $(subst false,,$(PACKAGES_PREFER_HOST)),$(PATH),$(INSTALL_PATH):$(PATH)) +endif + # For backwards compatibility with all of our other projects that use build-harness init:: exit 0 diff --git a/modules/docs/Makefile b/modules/docs/Makefile index 0ef27290..165e49c2 100644 --- a/modules/docs/Makefile +++ b/modules/docs/Makefile @@ -17,16 +17,16 @@ docs/targets.md: docs/deps .PHONY : docs/terraform.md ## Update `docs/terraform.md` from `terraform-docs` docs/terraform.md: docs/deps packages/install/terraform-docs - @echo "" > $@ - @terraform-docs md . >> $@ - @echo "" >> $@ + @echo "" > $@ ; \ + terraform-docs md . >> $@ ; \ + echo "" >> $@ .PHONY : docs/github-action.md ## Update `docs/github-action.md` from `action.yaml` docs/github-action.md: docs/deps packages/install/gomplate - @echo "" > $@ - @gomplate -d action=./action.yml -f $(BUILD_HARNESS_PATH)/templates/docs-github-action.gotmpl --config $(BUILD_HARNESS_PATH)/configs/gomplate.yaml >> $@ - @echo "" >> $@ + @echo "" > $@ ; \ + gomplate -d action=./action.yml -f $(BUILD_HARNESS_PATH)/templates/docs-github-action.gotmpl --config $(BUILD_HARNESS_PATH)/configs/gomplate.yaml >> $@ ; \ + echo "" >> $@ .PHONY : docs/github-actions-reusable-workflows.md ## Update `docs/github-actions-reusable-workflows.md` from `.github/workflows/*.yaml` diff --git a/modules/packages/Makefile b/modules/packages/Makefile index cba2979d..81ec5fdf 100644 --- a/modules/packages/Makefile +++ b/modules/packages/Makefile @@ -1,32 +1,55 @@ -export INSTALL_PATH ?= $(BUILD_HARNESS_PATH)/vendor +export VENDOR_DIR ?= $(BUILD_HARNESS_PATH)/vendor +export VENDOR_SUBDIR := $(shell uname -s)/$(shell uname -m) +export INSTALL_PATH ?= $(VENDOR_DIR)/$(VENDOR_SUBDIR) export PACKAGES_VERSION ?= master -export PACKAGES_PATH ?= $(BUILD_HARNESS_PATH)/vendor/packages +export PACKAGES_PATH ?= $(VENDOR_DIR)/packages +# PACKAGES_PREFER_HOST is used to force the use of the host's tools +# rather than the tools installed by build-harness in the git repo tree. +# This is to guard against the possibility that a malicious PR could install +# a compromised version of a tool that would be used by subsequent CI runs. export PACKAGES_PREFER_HOST ?= false ## Delete packages packages/delete: - rm -rf $(PACKAGES_PATH) + @# Do some checking to guard against running something like `rm -rf /` by mistake. + @# Check if packages is a subdirectory of build-harness and is a valid directory before deleting it. + @# Also, do not delete it if PRESERVE_PACKAGES is not empty. + @# Use realpath to resolve symlinks and relative paths and compare the actual paths. + @# Do not use realpath with [ -d ] because it returns an empty string if the path does not exist. + @if [ -n "$(findstring $(realpath $(BUILD_HARNESS_PATH)),$(realpath $(PACKAGES_PATH)))" ] \ + && [ ! "$(realpath $(BUILD_HARNESS_PATH))" = "$(realpath $(PACKAGES_PATH))" ] \ + && [ -d "$(PACKAGES_PATH)" ] && [ -z "$(PRESERVE_PACKAGES)" ]; then \ + printf "* Removing existing packages cache under %s ...\n" "$(realpath $(PACKAGES_PATH))"; \ + rm -rf "$(realpath $(PACKAGES_PATH))"; \ + fi ## Reinstall packages packages/reinstall: packages/delete packages/install @exit 0 +# Set PRESERVE_PACKAGES to a non-empty value to preserve the packages cache if it is less than a day old +packages/install: PRESERVE_PACKAGES ?= $(shell [ -d "$(PACKAGES_PATH)" ] && find "$(PACKAGES_PATH)" -maxdepth 0 -mtime 0) ## Install packages -packages/install: - @if [ ! -d $(PACKAGES_PATH) ]; then \ +packages/install: packages/delete + @if [ ! -d "$(PACKAGES_PATH)" ]; then \ echo "* Installing packages $(PACKAGES_VERSION)..."; \ - rm -rf $(PACKAGES_PATH); \ - $(GIT) clone -c advice.detachedHead=false --depth=1 -b $(PACKAGES_VERSION) https://github.com/cloudposse/packages.git $(PACKAGES_PATH); \ - rm -rf $(PACKAGES_PATH)/.git; \ + $(GIT) clone -c advice.detachedHead=false --depth=1 -b $(PACKAGES_VERSION) https://github.com/cloudposse/packages.git "$(PACKAGES_PATH)"; \ + rm -rf "$(realpath $(PACKAGES_PATH))"/.git; \ fi ## Install package (e.g. helm, helmfile, kubectl) packages/install/%: @binary="$*"; \ - if [ -x "$(INSTALL_PATH)/$$binary" ]; then \ + if [ "$(PACKAGES_PREFER_HOST)" = "true" ]; then \ + if installed=$$(command -v $* 2>/dev/null); then \ + echo Using "$*" from "$$installed" ; \ + else \ + echo "* Package $$binary not found on the host" >&2; \ + echo "* NOT Installing $* because PACKAGES_PREFER_HOST is true" >&2; \ + exit 1; \ + fi; \ + elif [ -x "$(INSTALL_PATH)/$$binary" ]; then \ echo "* Package $$binary already installed"; \ - elif [ "$(PACKAGES_PREFER_HOST)" = "true" ] && installed=$$(command -v $* 2>/dev/null); then \ - echo Using "$*" from "$$installed" ; \ else \ $(MAKE) packages/install && \ echo "* Installing $* to $(INSTALL_PATH)" && \ diff --git a/modules/readme/Makefile b/modules/readme/Makefile index 455224ff..341a0294 100644 --- a/modules/readme/Makefile +++ b/modules/readme/Makefile @@ -65,8 +65,8 @@ readme/lint: ## Create README.md by building it from README.yaml readme/build: readme/deps $(README_TEMPLATE_FILE) $(README_DEPS) - @gomplate --file $(README_TEMPLATE_FILE) --out $(README_FILE) --config $(BUILD_HARNESS_PATH)/configs/gomplate.yaml - @echo "Generated $(README_FILE) from $(README_TEMPLATE_FILE) using data from $(README_TEMPLATE_YAML)" + @gomplate --file $(README_TEMPLATE_FILE) --out $(README_FILE) --config $(BUILD_HARNESS_PATH)/configs/gomplate.yaml && \ + echo "Generated $(README_FILE) from $(README_TEMPLATE_FILE) using data from $(README_TEMPLATE_YAML)" readme/generate-related-references: @$(BUILD_HARNESS_PATH)/bin/generate_related_references.py diff --git a/templates/Makefile.build-harness b/templates/Makefile.build-harness index 5c841cee..e43451b9 100644 --- a/templates/Makefile.build-harness +++ b/templates/Makefile.build-harness @@ -160,7 +160,7 @@ precommit/terraform pr/auto-format precommit/terraform/host pr/auto-format/host: pr/readme pr/readme/host: ARGS := readme/deps readme pr/github-update pr/github-update/host: ARGS := github/update precommit/terraform pr/auto-format pr/readme pr/github-update: build-harness/runner -precommit/terraform/host pr/auto-format/host pr/readme/host pr/github-update/host: git-safe-directgory +precommit/terraform/host pr/auto-format/host pr/readme/host pr/github-update/host: git-safe-directory $(MAKE) $(ARGS) pr/pre-commit: ARGS := pre-commit/run