Re: [PATCH v3 7/7] ssh signing: verify ssh-keygen in test prereq

[Junio C Hamano <gitster@pobox.com>]

Fabian Stelzer <fs@gigacodes.de> writes:

> @@ -123,12 +119,19 @@ test_lazy_prereq GPGSSH '
>  	echo "\"principal with number 2\" $(cat "${GPGSSH_KEY_SECONDARY}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
>  	ssh-keygen -t ed25519 -N "${GPGSSH_KEY_PASSPHRASE}" -C "git ed25519 encrypted key" -f "${GPGSSH_KEY_WITH_PASSPHRASE}" >/dev/null &&
>  	echo "\"principal with number 3\" $(cat "${GPGSSH_KEY_WITH_PASSPHRASE}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
> -	ssh-keygen -t ed25519 -N "" -C "git ed25519 key" -f "${GPGSSH_KEY_UNTRUSTED}" >/dev/null
> +	ssh-keygen -t ed25519 -N "" -C "git ed25519 key" -f "${GPGSSH_KEY_UNTRUSTED}" >/dev/null &&
> +
> +	# Verify if at least one key and ssh-keygen works as expected
> +	echo "testpayload" | ssh-keygen -Y sign -n "git" -f "${GPGSSH_KEY_PRIMARY}" > gpgssh_prereq.sig &&

Style:
 . Avoid overlong line by breaking the line after pipe.
 . No SP between redirection operator '>' and redirection target.

	echo "testpayload" |
	ssh-keygen -Y sign -n "git" -f "${GPGSSH_KEY_PRIMARY}" >gpgssh_prereq.sig &&

Also I wonder if preparation of GPGSSH_ALLOWED_SIGNERS file can be
simplified and made easier to read.  Instead of adding one line at a
time like this:

>  	echo "\"principal with not yet valid key\" valid-after=\"29990101000000\" $(cat "${GPGSSH_KEY_NOTYETVALID}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}"

if you define variables with meaningful name and used here document,
e.g.

	ts2005a=200504070000 ts2005b=200504100000 &&
	key_timeboxedvalid=$(cat "${GPGSSH_KEY_TIMEBOXEDVALID}.pub") &&

	ts2999=299901010000 &&
	key_notyetvalid=$(cat "${GPGSSH_KEY_NOTYETVALID}.pub") &&

	cat >"${GPGSSH_ALLOWED_SIGNERS}" <<-EOF &&
	"principal with not yet valid key" valid-after="$ts2999" $key_notyetvalid
	"timeboxed valid key" valid-after="$ts2005a",valid-before="$ts2005b" $key_timeboxedvalid
	...
	EOF