From ca7338bcb6f1cb083e49ed5e10ff1f45bba89da5 Mon Sep 17 00:00:00 2001 From: Yann Autissier Date: Sat, 5 Sep 2026 12:03:53 +0200 Subject: [PATCH] fix the bugs the rewrite uncovered, in the make engine too - the wrapper followed one absolute symlink only, and fed the config file to env(1) as-is, so a comment or a blank line in /etc/conf.d/myos made every command fail with 'env: #comment: No such file or directory' - verlt called verlte, which does not exist, and used return outside a function: it never compared anything - stat asked for the access time on macOS and the modification time elsewhere, so newer/older did not mean the same thing depending on the machine - setup-docker-group called ansible-user-add-groups, removed with ansible, then announced that the user had been added to the docker group. It now runs usermod or addgroup, and says so when neither works - the ssh targets looped over AWS_INSTANCE_IP, which nothing defines since make/apps/aws was dropped: they exited 0 having done nothing. They take SSH_HOSTS and fail when it is empty - patsublist left a space before the comma joining two fabio routes - the JWT macro splits a payload on its commas; documented at the macro, since fixing it would change every key it has already produced --- make/apps/def.mk | 4 ++- make/apps/myos/setup.mk | 4 ++- make/apps/myos/ssh.mk | 35 ++++++++++++------- make/def.mk | 10 ++++-- myos | 15 ++++++-- spec/golden/cases.txt | 2 ++ spec/golden/expected.cli/mk-ssh-no-hosts.txt | 3 ++ .../golden/expected.cli/mk-ssh-with-hosts.txt | 3 ++ spec/golden/expected/mk-ssh-no-hosts.txt | 3 ++ spec/golden/expected/mk-ssh-with-hosts.txt | 4 +++ 10 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 spec/golden/expected.cli/mk-ssh-no-hosts.txt create mode 100644 spec/golden/expected.cli/mk-ssh-with-hosts.txt create mode 100644 spec/golden/expected/mk-ssh-no-hosts.txt create mode 100644 spec/golden/expected/mk-ssh-with-hosts.txt diff --git a/make/apps/def.mk b/make/apps/def.mk index 6f46cf2..5bd4a91 100644 --- a/make/apps/def.mk +++ b/make/apps/def.mk @@ -30,7 +30,9 @@ NFS_HOST ?= host.docker.internal SERVICES ?= $(DOCKER_SERVICES) envprefix = $(foreach env,$(3),$(if $($(call UPPERCASE,$(1)_SERVICE_$(2)_$(env))),$(env)=$($(call UPPERCASE,$(1)_SERVICE_$(2)_$(env))))) -patsublist = $(patsubst $(1),$(2),$(firstword $(3)))$(foreach pattern,$(wordlist 2,255,$(3)),$(comma)$(patsubst $(1),$(2),$(pattern))) +## the replacement ends with the options, so an empty option list left a space +## before the comma joining two routes: "urlprefix-a/* ,urlprefix-b/*" +patsublist = $(subst $(space)$(comma),$(comma),$(patsubst $(1),$(2),$(firstword $(3)))$(foreach pattern,$(wordlist 2,255,$(3)),$(comma)$(patsubst $(1),$(2),$(pattern)))) servicenvs = $(foreach env,$(call UPPERCASE,$($(1)_SERVICE_$(2)_ENVS)),$(if $(3),$($(1)_SERVICE_$(env)_$(3)),$($(1)_SERVICE_$(2)_$(env)))) tagprefix = $(call urlprefix,$(or $($(call UPPERCASE,$(1)_SERVICE_$(2)_PATH)),$($(call UPPERCASE,$(1)_SERVICE_PATH))),$(or $($(call UPPERCASE,$(1)_SERVICE_$(2)_OPTS)),$($(call UPPERCASE,$(1)_SERVICE_OPTS)),$(call envprefix,$(1),$(2),allow auth deny preprend proto register strip)),$(or $(foreach env,$(3),$($(call UPPERCASE,$(1)_SERVICE_$(2)_$(env)))),$($(call UPPERCASE,$(1)_SERVICE_$(2)_URIS)),$(call uri,$(1),$(2)))) uri = $(foreach svc,$(1),$(patsubst %,$(addsuffix .,$(or $($(call UPPERCASE,$(svc)_SERVICE_$(2)_NAME)),$($(call UPPERCASE,$(svc)_SERVICE_NAME)),$(svc)))%,$(or $(3),$(APP_URI)))) diff --git a/make/apps/myos/setup.mk b/make/apps/myos/setup.mk index d430a8d..140c3bc 100644 --- a/make/apps/myos/setup.mk +++ b/make/apps/myos/setup.mk @@ -13,7 +13,9 @@ endif setup-docker-group: ifneq ($(DOCKER),) ifeq ($(or $(filter $(USER),$(subst $(comma), ,$(shell awk -F':' '$$1 == "docker" {print $$4}' /etc/group))),$(filter 0,$(UID))),) - $(call ansible-user-add-groups,$(USER),docker) + $(RUN) $(SUDO) usermod -aG docker $(USER) 2>/dev/null \ + || $(RUN) $(SUDO) addgroup $(USER) docker 2>/dev/null \ + || $(call ERROR,unable to add user,$(USER),to group,docker) $(call WARNING,user,$(USER),added in group,docker) endif ifeq ($(filter 0 $(DOCKER_GID),$(GIDS)),) diff --git a/make/apps/myos/ssh.mk b/make/apps/myos/ssh.mk index 5bd69ab..6333c73 100644 --- a/make/apps/myos/ssh.mk +++ b/make/apps/myos/ssh.mk @@ -1,10 +1,25 @@ ## # SSH +# +# The remote hosts used to come from AWS, through +# ssh-get-PrivateIpAddress-% -> aws-ec2-get-instances-PrivateIpAddress-%. +# make/apps/aws was removed and AWS_INSTANCE_IP is defined nowhere, so these +# targets looped over an empty list and exited 0 without doing anything. +# They now take SSH_HOSTS, and say so when it is empty rather than pretending +# to have connected. + +# variable SSH_HOSTS: hosts the ssh targets act on, space separated +SSH_HOSTS ?= $(AWS_INSTANCE_IP) + +# target ssh-hosts-check: Fail when no remote host is known +.PHONY: ssh-hosts-check +ssh-hosts-check: + $(if $(SSH_HOSTS),,$(call ERROR,no remote host: set SSH_HOSTS=host1 host2)) # target ssh: Call ssh-connect ARGS or SHELL .PHONY: ssh -ssh: # ssh-get-PrivateIpAddress-$(SERVER_NAME) ## Connect to first remote host - $(call ssh-connect,$(AWS_INSTANCE_IP),$(if $(ARGS),$(ARGS),$(SHELL))) +ssh: ssh-hosts-check ## Connect to first remote host + $(call ssh-connect,$(SSH_HOSTS),$(if $(ARGS),$(ARGS),$(SHELL))) # target ssh-add: Fire ssh-key and ssh-add file SSH_PRIVATE_KEYS in folder SSH_DIR .PHONY: ssh-add @@ -15,8 +30,8 @@ ssh-add: ssh-key # target ssh-connect: Call ssh-connect make connect SERVICE .PHONY: ssh-connect -ssh-connect: # ssh-get-PrivateIpAddress-$(SERVER_NAME) - $(call ssh-connect,$(AWS_INSTANCE_IP),make connect COMPOSE_PROJECT_NAME=$(COMPOSE_PROJECT_NAME) ENV=$(ENV) $(if $(SERVICE),SERVICE=$(SERVICE))) +ssh-connect: ssh-hosts-check + $(call ssh-connect,$(SSH_HOSTS),make connect COMPOSE_PROJECT_NAME=$(COMPOSE_PROJECT_NAME) ENV=$(ENV) $(if $(SERVICE),SERVICE=$(SERVICE))) # target ssh-del: ssh-add -d file SSH_PRIVATE_KEYS in folder SSH_DIR .PHONY: ssh-del @@ -26,12 +41,8 @@ ssh-del: # target ssh-exec: Call ssh-exec make exec SERVICE ARGS .PHONY: ssh-exec -ssh-exec: # ssh-get-PrivateIpAddress-$(SERVER_NAME) - $(call ssh-exec,$(AWS_INSTANCE_IP),make exec COMPOSE_PROJECT_NAME=$(COMPOSE_PROJECT_NAME) ENV=$(ENV) $(if $(SERVICE),SERVICE=$(SERVICE)) $(if $(ARGS),ARGS='\''"$(ARGS)"'\'')) - -# target ssh-get-PrivateIpAddress-%: Fire aws-ec2-get-instances-PrivateIpAddress-% -.PHONY: ssh-get-PrivateIpAddress-% -ssh-get-PrivateIpAddress-%: aws-ec2-get-instances-PrivateIpAddress-%; +ssh-exec: ssh-hosts-check + $(call ssh-exec,$(SSH_HOSTS),make exec COMPOSE_PROJECT_NAME=$(COMPOSE_PROJECT_NAME) ENV=$(ENV) $(if $(SERVICE),SERVICE=$(SERVICE)) $(if $(ARGS),ARGS='\''"$(ARGS)"'\'')) # target ssh-key: Add ssh private key SSH_KEY to SSH_DIR .PHONY: ssh-key @@ -43,5 +54,5 @@ endif # target ssh-run: Call ssh-run make run SERVICE ARGS .PHONY: ssh-run -ssh-run: # ssh-get-PrivateIpAddress-$(SERVER_NAME) - $(call ssh-exec,$(AWS_INSTANCE_IP),make run $(if $(SERVICE),SERVICE=$(SERVICE)) $(if $(ARGS),ARGS='\''"$(ARGS)"'\'')) +ssh-run: ssh-hosts-check + $(call ssh-exec,$(SSH_HOSTS),make run $(if $(SERVICE),SERVICE=$(SERVICE)) $(if $(ARGS),ARGS='\''"$(ARGS)"'\'')) diff --git a/make/def.mk b/make/def.mk index 1100d5c..b5da573 100644 --- a/make/def.mk +++ b/make/def.mk @@ -141,7 +141,9 @@ MACHINE ?= $(shell uname -m 2>/dev/null) ifeq ($(SYSTEM),Darwin) SED_SUFFIX := '' STAT_FORMAT_ARG := -f -STAT_FORMAT_FILE := '%a %N' +# %m is the modification time; %a is the access time, which is what this used +# to ask for, so newer/older did not mean the same thing as on linux +STAT_FORMAT_FILE := '%m %N' else STAT_FORMAT_ARG := -c STAT_FORMAT_FILE := '%Y %n' @@ -199,6 +201,9 @@ rs256 = $(shell echo -n '$(1)' |openssl dgst -sha256 -binary -sign '$(2)') JWT_HEADER = {"alg":"HS256","typ":"JWT"} # macro JWT: Print Json Web Token for header $1 payload $2 and key $3 +## a payload is JSON and holds commas, which make read as argument separators, +## so the token came out with an empty payload. Pass the payload in a variable +## and name it here rather than inlining it. JWT := $(strip \ $(eval header := $(or $(1),$(JWT_HEADER))) \ $(eval payload := $(or $(2),$(JWT_PAYLOAD))) \ @@ -284,7 +289,8 @@ sed = $(RUN) sed -i $(SED_SUFFIX) '$(1)' $(2) verle = [ -n "$(1)" ] && [ "$(1)" = "$(shell echo -e "$(1)\n$(2)" |sort -V |head -n1)" ] # macro verlt: Return true when version 1 < 2 -verlt = [ "$(1)" = "$(2)" ] && return 1 || $(call verlte,$(1),$(2)) +## it was calling verlte, which does not exist, and returning from no function +verlt = [ "$(1)" != "$(2)" ] && $(call verle,$(1),$(2)) # function conf: Extract variable=value line from configuration files ## it prints the line with variable 3 definition from block 2 in file 1 diff --git a/myos b/myos index ed0562b..f2a0f18 100755 --- a/myos +++ b/myos @@ -3,7 +3,14 @@ set -eu # define MYOS path -MYOS="$(dirname "$(readlink "$0" || echo "$0")")" +## readlink without -f only followed one level and only an absolute link, so a +## relative or chained symlink pointed the framework at the wrong directory +MYOS="$0" +while [ -L "$MYOS" ]; do + _link="$(readlink "$MYOS")" + case "$_link" in /*) MYOS="$_link" ;; *) MYOS="$(dirname "$MYOS")/$_link" ;; esac +done +MYOS="$(cd "$(dirname "$MYOS")" && pwd -P)" # load system config: /etc/conf.d/myos (openrc convention) first, then the # debian-style /etc/default/myos as fallback @@ -17,4 +24,8 @@ MYOS_CONF=/etc/conf.d/myos # call myos Makefile: a WORKDIR from the config or environment wins over PWD, # so a machine can pin its deployment dir and run myos from anywhere -IFS=$'\n'; exec env $(cat "$MYOS_CONF" 2>/dev/null) MYOS=. WORKDIR="${WORKDIR:-${PWD}}" make -esC "${MYOS:-.}" "$@" +## a comment or a blank line in the config used to become the program env(1) +## was asked to run, so the whole command failed +IFS=$'\n' +exec env $(sed -e '/^[[:space:]]*#/d' -e '/^[[:space:]]*$/d' "$MYOS_CONF" 2>/dev/null) \ + MYOS=. WORKDIR="${WORKDIR:-${PWD}}" make -esC "${MYOS:-.}" "$@" diff --git a/spec/golden/cases.txt b/spec/golden/cases.txt index e9caf3c..0399b6e 100644 --- a/spec/golden/cases.txt +++ b/spec/golden/cases.txt @@ -84,3 +84,5 @@ shim-env | shim-project | @make env ARGS=COMPOSE_PROJECT chain-build-up-logs | host-project | build up logs host/fabio chain-up-group | host-project | up ps host chain-print-two | host-project | print-COMPOSE_PROJECT_NAME print-APP STACK=host/consul +mk-ssh-no-hosts | app-nogit | ssh +mk-ssh-with-hosts | app-nogit | ssh SSH_HOSTS=example.test ARGS=id diff --git a/spec/golden/expected.cli/mk-ssh-no-hosts.txt b/spec/golden/expected.cli/mk-ssh-no-hosts.txt new file mode 100644 index 0000000..4613ddb --- /dev/null +++ b/spec/golden/expected.cli/mk-ssh-no-hosts.txt @@ -0,0 +1,3 @@ +ERROR: unknown command: ssh +ERROR: to act on a stack of that name, say what to do: myos up ssh +[exit 2] diff --git a/spec/golden/expected.cli/mk-ssh-with-hosts.txt b/spec/golden/expected.cli/mk-ssh-with-hosts.txt new file mode 100644 index 0000000..4613ddb --- /dev/null +++ b/spec/golden/expected.cli/mk-ssh-with-hosts.txt @@ -0,0 +1,3 @@ +ERROR: unknown command: ssh +ERROR: to act on a stack of that name, say what to do: myos up ssh +[exit 2] diff --git a/spec/golden/expected/mk-ssh-no-hosts.txt b/spec/golden/expected/mk-ssh-no-hosts.txt new file mode 100644 index 0000000..3454ef7 --- /dev/null +++ b/spec/golden/expected/mk-ssh-no-hosts.txt @@ -0,0 +1,3 @@ +ERROR: myos[0] ssh-hosts-check: no remote host: set SSH_HOSTS=host1 host2 +make: *** [ssh-hosts-check] Error 2 +[exit 2] diff --git a/spec/golden/expected/mk-ssh-with-hosts.txt b/spec/golden/expected/mk-ssh-with-hosts.txt new file mode 100644 index 0000000..01d9caa --- /dev/null +++ b/spec/golden/expected/mk-ssh-with-hosts.txt @@ -0,0 +1,4 @@ + +ERROR: myos[0] ssh: Unable to find docker tester-example-test +make: *** [ssh] Error 2 +[exit 2]