From 2f6e3c91f9b983ddb514696d2a5e0b7505232bd9 Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Sun, 4 Dec 2022 14:52:29 +0100
Subject: [PATCH] bakefile: try to keep defaults in the Dockerfile

This is an attempt to keep defaults in the Dockerfile, without having to
re-define them in the bakefile. Before this patch, build-args would always
be set, but either with a default (specified in the bakefile) or an empty
string.

When setting a build-arg to an empty string, the default from the Dockerfile
is ignored (and a literal empty string used instead).

Ideally, bakefiles would have some option for this, but I don't think there
currently is, but thinking something along the lines of;

- a list of build-arg names to consider (taking them from the environment,
  if present)
- possibly changing args to a list of strings instead of a map (so that
  `key` can be set without `=value`), although this would still require
  some mangling of the values (must be set without `=` to take environment
  variables).
- maybe other options?

Other things I looked at (but I'm very much not familiar with the HCL syntax);

- custom conditions (this could reduce the boilerplating `somevar != ""`
  see https://developer.hashicorp.com/terraform/language/expressions/custom-conditions
- local values; trying to use `local` (or `locals`) didn't work; trying to
  reference them through `local.variable_name` caused an error (but so did
  referencing variables through `var.variable_name`), perhaps it's just not
  supported by `bake` (again, I'm very unfamiliar with HCL so maybe I just
  did it wrong); https://developer.hashicorp.com/terraform/language/values/locals
  The HCL docs seem to mention them, so perhaps it's just not supported by
  bake; https://github.com/hashicorp/hcl/blob/27df1ec5fcbc64f6799707e98c7d0433b05ef140/guide/language_design.rst#blocks-vs-object-values

I also looked at the current bakefile, and was a bit confused how merging
of maps happens (looks like `inherits` does an implicit merge of maps).

Either way, with this patch:

The default is taken from the Dockerfile;

```bash
docker buildx bake --set binary.platform=darwin/amd64,darwin/arm64 binary
...
=> [linux/arm64/v8 internal] load metadata for docker.io/library/golang:1.19.3-alpine   2.4s
...
```

But can still be overridden;

```bash
GO_VERSION=1.19.2 docker buildx bake --set binary.platform=darwin/amd64,darwin/arm64 binary
...
=> [linux/arm64/v8 internal] load metadata for docker.io/library/golang:1.19.2-alpine   2.4s
...
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 docker-bake.hcl | 63 ++++++++++++++++++++++++-------------------------
 1 file changed, 31 insertions(+), 32 deletions(-)

diff --git a/docker-bake.hcl b/docker-bake.hcl
index d789dca7e246..bffac207ec11 100644
--- a/docker-bake.hcl
+++ b/docker-bake.hcl
@@ -1,29 +1,40 @@
-variable "GO_VERSION" {
-    default = "1.19.3"
+variable "GO_VERSION" {}
+variable "VERSION" {}
+variable "USE_GLIBC" {}
+variable "STRIP_TARGET" {}
+variable "IMAGE_NAME" {
+    default = "docker-cli"
 }
-variable "VERSION" {
-    default = ""
+
+# Sets the name of the company that produced the windows binary.
+variable "PACKAGER_NAME" {}
+
+variable "binary_args" {
+    default = {}
 }
-variable "USE_GLIBC" {
-    default = ""
+
+variable "go_version" {
+    default = GO_VERSION != "" ? { GO_VERSION = GO_VERSION } : {}
 }
-variable "STRIP_TARGET" {
-    default = ""
+
+variable "go_strip" {
+    default = STRIP_TARGET != "" ? { GO_STRIP = STRIP_TARGET } : {}
 }
-variable "IMAGE_NAME" {
-    default = "docker-cli"
+
+variable "variant" {
+    default = USE_GLIBC != "" ? { BASE_VARIANT = "bullseye" } : {}
 }
 
-# Sets the name of the company that produced the windows binary.
-variable "PACKAGER_NAME" {
-    default = ""
+variable "version" {
+    default = VERSION != "" ? { VERSION = VERSION } : {}
+}
+
+variable "packager_name" {
+    default = PACKAGER_NAME != "" ? { PACKAGER_NAME = PACKAGER_NAME } : {}
 }
 
 target "_common" {
-    args = {
-        GO_VERSION = GO_VERSION
-        BUILDKIT_CONTEXT_KEEP_GIT_DIR = 1
-    }
+    args = merge(go_version, { BUILDKIT_CONTEXT_KEEP_GIT_DIR = 1 })
 }
 
 target "_platforms" {
@@ -51,12 +62,7 @@ target "binary" {
     target = "binary"
     platforms = ["local"]
     output = ["build"]
-    args = {
-        BASE_VARIANT = USE_GLIBC != "" ? "bullseye" : "alpine"
-        VERSION = VERSION
-        PACKAGER_NAME = PACKAGER_NAME
-        GO_STRIP = STRIP_TARGET
-    }
+    args = merge(go_strip, variant, version, packager_name)
 }
 
 target "dynbinary" {
@@ -71,11 +77,7 @@ target "plugins" {
     target = "plugins"
     platforms = ["local"]
     output = ["build"]
-    args = {
-        BASE_VARIANT = USE_GLIBC != "" ? "bullseye" : "alpine"
-        VERSION = VERSION
-        GO_STRIP = STRIP_TARGET
-    }
+    args = merge(go_strip, variant, version)
 }
 
 target "cross" {
@@ -154,8 +156,5 @@ target "e2e-image" {
     target = "e2e"
     output = ["type=docker"]
     tags = ["${IMAGE_NAME}"]
-    args = {
-        BASE_VARIANT = USE_GLIBC != "" ? "bullseye" : "alpine"
-        VERSION = VERSION
-    }
+    args = merge(go_version, variant, version)
 }