dumpcerts.sh: Fix call to "base64" for Alpine #2344

nknapp · 2017-10-29T16:46:01Z

Alpine Linux' "base64" does not support the --decode option. Only -d.
In order to run dumpcerts.sh inside the office "traefik:1.4-alpine" docker-image,
base64 must be called with -d

timoreimann · 2017-10-29T17:25:35Z

contrib/scripts/dumpcerts.sh

@@ -149,10 +149,10 @@ for domain in $(jq -r '.DomainsCertificate.Certs[].Certificate.Domain' ${acmefil
 	echo "Extracting cert bundle for ${domain}"
 	cert=$(jq -e -r --arg domain "$domain" '.DomainsCertificate.Certs[].Certificate |
         	select (.Domain == $domain )| .Certificate' ${acmefile}) || bad_acme
-	echo "${cert}" | base64 --decode > "${cdir}/${domain}.crt"
+	echo "${cert}" | base64 -d > "${cdir}/${domain}.crt"


Is -d supported in non-Alpine contexts as well?

If yes: cool 👍

If no: shouldn't our script be smart enough to handle both option styles?

The man page states both options. I have verified the help output of Ubuntu 10.04, Ubuntu 16.04, Centos 7 and Fedora 26 and they all support the short-hand option.

Alpine is a bit special, because the installed binary for base64 is actually the busybox binary. I guess that's the reason. I believe it is safe to use -d without checking for the environment.

The base64 binary shipped with Mac OS X seems to deviate:

$ base64 --help Usage: base64 [-hvD] [-b num] [-i in_file] [-o out_file] -h, --help display this message -D, --decode decodes input -b, --break break encoded string into num character lines -i, --input input file (default: "-" for stdin) -o, --output output file (default: "-" for stdout)

:(

That's sad... I'll have to check if the is another base64 binary for alpine.

The alternative would be to check if the base64 binary is actually a symlink to busybox and then revert to the -d option. If that's an acceptable solution, I would change my PR.

How about just checking in the script whether we run on Darwin and adjust the parameter accordingly?

timoreimann · 2017-10-31T21:54:59Z

contrib/scripts/dumpcerts.sh

+		alias decode_base64='base64 -d'
+		;;
+	*)
+		# Max OS-X supports --decode and -D, but --decode may be supported by other platforms as well.


Just to complete the picture: We could also invert the logic and switch-case on Darwin to set --decode, and use -d in all other cases (Linux, FreeBSD, you name it).

No idea what might be better. I'd go with your current approach when in doubt.

nknapp · 2017-10-31T21:58:25Z

Sorry for the force-push. My last version did not work out (aliases don't work in bash-scripts, it seems). Now I'm using a variable to store the base64-decode command.

timoreimann · 2017-10-31T22:02:35Z

contrib/scripts/dumpcerts.sh

@@ -42,6 +42,18 @@ set -o nounset

 USAGE="$(basename "$0") <path to acme> <destination cert directory>"

+# Platform variations
+case `uname` in


Could you please use $() instead of backticks, just to follow best practices?

Done. If you want me to squash my commits, just tell me.

Would by nicer with quotes around, I guess

True -- let's add quotes for complete happiness. :)

No need to squash, our bot does that.

timoreimann

Thanks a lot, @nknapp -- the PR looks good to me at this point. 🎉

nmengin

LGTM 👍

ldez

LGTM

Alpine Linux' "base64" does not support the "--decode" option. Only "-d". In order to run dumpcerts.sh inside the office "traefik:1.4-alpine" docker-image, "base64" must be called with "-d"

Detects if this is a Mac OSX (FreeBSD) or Linux and uses '--decode' or '-d' depending on the outcome. Mac OSX does not support '-d', but '-D' as shortcut. fixup: remove redundant colon

traefiker added size/S status/0-needs-triage labels Oct 29, 2017

timoreimann reviewed Oct 29, 2017

View reviewed changes

ldez added status/2-needs-review contributor/waiting-for-corrections and removed status/0-needs-triage labels Oct 29, 2017

timoreimann reviewed Oct 31, 2017

View reviewed changes

nknapp force-pushed the patch-2 branch from f6066c3 to 5c498dd Compare October 31, 2017 21:55

timoreimann suggested changes Oct 31, 2017

View reviewed changes

nknapp force-pushed the patch-2 branch 2 times, most recently from c4e1d82 to 371d552 Compare October 31, 2017 22:56

timoreimann approved these changes Oct 31, 2017

View reviewed changes

traefiker removed the contributor/waiting-for-corrections label Oct 31, 2017

ldez added the kind/enhancement a new or improved feature. label Nov 1, 2017

nmengin approved these changes Nov 2, 2017

View reviewed changes

ldez approved these changes Nov 2, 2017

View reviewed changes

mmatur added this to the 1.5 milestone Nov 2, 2017

mmatur added status/3-needs-merge and removed status/2-needs-review labels Nov 2, 2017

traefiker added the status/4-merge-in-progress label Nov 2, 2017

nknapp and others added 4 commits November 2, 2017 14:40

dumpcerts.sh: Fix call to "base64" for Alpine

2d5efea

Alpine Linux' "base64" does not support the "--decode" option. Only "-d". In order to run dumpcerts.sh inside the office "traefik:1.4-alpine" docker-image, "base64" must be called with "-d"

dumpcerts.sh: Determine base64-decode param via platforms "uname"

c1dec61

Detects if this is a Mac OSX (FreeBSD) or Linux and uses '--decode' or '-d' depending on the outcome. Mac OSX does not support '-d', but '-D' as shortcut. fixup: remove redundant colon

dumpcerts: Use $() instead of backticks

2d06f4a

dumpCerts.sh: Surround $(uname) with quotes

7ad8aa4

traefiker force-pushed the patch-2 branch from 371d552 to 7ad8aa4 Compare November 2, 2017 14:40

traefiker removed the status/4-merge-in-progress label Nov 2, 2017

traefiker merged commit 91ff94e into traefik:master Nov 2, 2017

traefiker removed the status/3-needs-merge label Nov 2, 2017

ldez removed the kind/enhancement a new or improved feature. label Nov 24, 2017

ldez added the area/acme label Nov 28, 2017

jantari mentioned this pull request Jun 23, 2022

Use base64 -d on all Linux platforms and --decode on others tvesely/terraform-external-shell-resource#1

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dumpcerts.sh: Fix call to "base64" for Alpine #2344

dumpcerts.sh: Fix call to "base64" for Alpine #2344

nknapp commented Oct 29, 2017 •

edited by ldez

Loading

timoreimann Oct 29, 2017

nknapp Oct 29, 2017 •

edited

Loading

timoreimann Oct 29, 2017 •

edited

Loading

nknapp Oct 31, 2017

timoreimann Oct 31, 2017

timoreimann Oct 31, 2017

nknapp commented Oct 31, 2017

timoreimann Oct 31, 2017

nknapp Oct 31, 2017

nknapp Oct 31, 2017

timoreimann Oct 31, 2017 •

edited

Loading

timoreimann left a comment

nmengin left a comment

ldez left a comment

dumpcerts.sh: Fix call to "base64" for Alpine #2344

dumpcerts.sh: Fix call to "base64" for Alpine #2344

Conversation

nknapp commented Oct 29, 2017 • edited by ldez Loading

timoreimann Oct 29, 2017

Choose a reason for hiding this comment

nknapp Oct 29, 2017 • edited Loading

Choose a reason for hiding this comment

timoreimann Oct 29, 2017 • edited Loading

Choose a reason for hiding this comment

nknapp Oct 31, 2017

Choose a reason for hiding this comment

timoreimann Oct 31, 2017

Choose a reason for hiding this comment

timoreimann Oct 31, 2017

Choose a reason for hiding this comment

nknapp commented Oct 31, 2017

timoreimann Oct 31, 2017

Choose a reason for hiding this comment

nknapp Oct 31, 2017

Choose a reason for hiding this comment

nknapp Oct 31, 2017

Choose a reason for hiding this comment

timoreimann Oct 31, 2017 • edited Loading

Choose a reason for hiding this comment

timoreimann left a comment

Choose a reason for hiding this comment

nmengin left a comment

Choose a reason for hiding this comment

ldez left a comment

Choose a reason for hiding this comment

nknapp commented Oct 29, 2017 •

edited by ldez

Loading

nknapp Oct 29, 2017 •

edited

Loading

timoreimann Oct 29, 2017 •

edited

Loading

timoreimann Oct 31, 2017 •

edited

Loading