Use installed packages, segregate executables by platform (#375)

Author: Nuru

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

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 "<!-- markdownlint-disable -->" > $@
-	@terraform-docs md . >> $@
-	@echo "<!-- markdownlint-restore -->" >> $@
+	@echo "<!-- markdownlint-disable -->" > $@ ; \
+	terraform-docs md . >> $@ ; \
+	echo "<!-- markdownlint-restore -->" >> $@
 
 .PHONY : docs/github-action.md
 ## Update `docs/github-action.md` from `action.yaml`
 docs/github-action.md: docs/deps packages/install/gomplate
-	@echo "<!-- markdownlint-disable -->" > $@
-	@gomplate -d action=./action.yml -f $(BUILD_HARNESS_PATH)/templates/docs-github-action.gotmpl --config $(BUILD_HARNESS_PATH)/configs/gomplate.yaml >> $@
-	@echo "<!-- markdownlint-restore -->" >> $@
+	@echo "<!-- markdownlint-disable -->" > $@ ; \
+	gomplate -d action=./action.yml -f $(BUILD_HARNESS_PATH)/templates/docs-github-action.gotmpl --config $(BUILD_HARNESS_PATH)/configs/gomplate.yaml >> $@ ; \
+	echo "<!-- markdownlint-restore -->" >> $@
 
 .PHONY : docs/github-actions-reusable-workflows.md
 ## Update `docs/github-actions-reusable-workflows.md` from `.github/workflows/*.yaml`

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)" && \

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

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