caddyserver / caddy-docker

Source for the official Caddy v2 Docker Image
https://hub.docker.com/_/caddy
Apache License 2.0
409 stars 74 forks source link

Make CADDY_VERSION an ARG instead of ENV #207

Closed ThinkChaos closed 3 years ago

ThinkChaos commented 3 years ago

This change allows overriding it's value when building as ARGs can be set with --build-arg but ENVs can't.

I think it's safe to change CADDY_VERSION like so because xcaddy can build multiple caddy versions. I didn't change XCADDY_VERSION as that would break the checksum verification.

francislavoie commented 3 years ago

If you want to use a different version, you should probably just specify it in the xcaddy command, from your own Dockerfile.

xcaddy build <version> \
    --with <plugin>

https://github.com/caddyserver/xcaddy#command-usage

ThinkChaos commented 3 years ago

Yeah that's what I do, but my Dockerfile takes an ARG CADDY_VERSION which doesn't work because ENV always overrides ARG. So to avoid hardcoding the version in the Dockerfile, I have to change this (or use a different name - but meh).

EDIT: more specifically in my Dockerfile I call xcaddy $CADDY_VERSION which ignores my ARG and uses the ENV defined here.

francislavoie commented 3 years ago

@ThinkChaos just use a different variable then, and do xcaddy build $CUSTOM_CADDY_VERSION or something. xcaddy always takes the argument as priority over the CADDY_VERSION env.

ThinkChaos commented 3 years ago

@francislavoie I know, but I'm not satisfied with another name. "Custom" makes it sound like it's not an official Caddy release. I don't understand why this change would pose any issue as it makes the Dockerfile more flexible for downstream users with almost no cost. The Dockerfile reference even recommends the ARG+ENV approach.

hairyhenderson commented 3 years ago

Sorry @ThinkChaos but we can't accept this. Official images can't be built with ARG instructions.

See also: https://github.com/caddyserver/caddy-docker/issues/108 and https://github.com/caddyserver/caddy-docker/pull/110