Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use docker buildx bake to build images #669

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
218 changes: 218 additions & 0 deletions docker-bake.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
group "all" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can eliminate the need to duplicate the target names in the sub-groups (maintained, practice, vulnerable) and the main all group by using variables and concat:

variable "maintained_targets" {
  default = ["bitcoin-28", "bitcoin-27", "bitcoin-26"]
}

variable "practice_targets" {
  default = ["bitcoin-unknown-message", "bitcoin-invalid-blocks", "bitcoin-50-orphans", "bitcoin-no-mp-trim", "bitcoin-disabled-opcodes", "bitcoin-5k-inv"]
}

variable "vulnerable_targets" {
  default = ["v0-21-1", "v0-20-0", "v0-19-2", "v0-17-0", "v0-16-1"]
}

group "maintained" {
  targets = maintained_targets
}

group "practice" {
  targets = practice_targets
}

group "vulnerable" {
  targets = vulnerable_targets
}

group "all" {
  targets = concat(maintained_targets, practice_targets, vulnerable_targets)
}

targets = [
"bitcoin-28",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referring to a comment in src/warnet/constants.py (replicated below), do you think somehow referencing or parsing the targets here would be a potential solution?

# Constants used throughout the project
# Storing as constants for now but we might want a more sophisticated config management
# at some point.
SUPPORTED_TAGS = ["27.0", "26.0", "25.1", "24.2", "23.2", "22.2"]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could leverage the tags of @Onyekachukwu-Nweke's suggestion to achieve this: #669 (comment)

"bitcoin-27",
"bitcoin-26",
"v0-21-1",
"v0-20-0",
"v0-19-2",
"v0-17-0",
"v0-16-1",
"bitcoin-unknown-message",
"bitcoin-invalid-blocks",
"bitcoin-50-orphans",
"bitcoin-no-mp-trim",
"bitcoin-disabled-opcodes",
"bitcoin-5k-inv"
]
}

group "maintained" {
targets = [
"bitcoin-28",
"bitcoin-27",
"bitcoin-26"
]
}

group "practice" {
targets = [
"bitcoin-unknown-message",
"bitcoin-invalid-blocks",
"bitcoin-50-orphans",
"bitcoin-no-mp-trim",
"bitcoin-disabled-opcodes",
"bitcoin-5k-inv"
]
}

group "vulnerable" {
targets = [
"v0-21-1",
"v0-20-0",
"v0-19-2",
"v0-17-0",
"v0-16-1",
]
}

target "maintained-base" {
dockerfile = "./Dockerfile"
context = "./resources/images/bitcoin"
args = {
REPO = "bitcoin"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be as shown below?

Suggested change
REPO = "bitcoin"
REPO = "bitcoin/bitcoin"

BUILD_ARGS = "--disable-tests --without-gui --disable-bench --disable-fuzz-binary --enable-suppress-external-warnings "
}
platforms = ["linux/amd64", "linux/arm64", "linux/arm/v7"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For improved modularity, consider implementing this as a reusable variable."

variable "DEFAULT_PLATFORMS" {
  default = ["linux/amd64", "linux/arm64", "linux/arm/v7"]
}

platforms = "${variable.DEFAULT_PLATFORMS}"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

variable. is not needed from my experience to reference a variable

variable "DEFAULT_PLATFORMS" {
  default = ["linux/amd64", "linux/arm64", "linux/arm/v7"]
}

platforms = "${DEFAULT_PLATFORMS}"

*Note: platform can be overridden as needed as well when baking
docker buildx bake <target> --set "*.platform="linux/arm64""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, works like @macgyver13 suggested

}

target "bitcoin-28" {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implement a matrix pattern for version management

I recommend implementing a matrix pattern for targets that differ primarily by version and commit hash:

// Define a matrix of maintained versions
variable "MAINTAINED_VERSIONS" {
  default = {
    "28" = {
      tag = "28.0"
      commit = "110183746150428e6385880c79f8c5733b1361ba"
    }
    "27" = {
      tag = "27.2"
      commit = "bf03c458e994abab9be85486ed8a6d8813313579" 
    }
    "26" = {
      tag = "26.2"
      commit = "7b7041019ba5e7df7bde1416aa6916414a04f3db"
    }
  }
}

// Then reference in target definitions:
target "bitcoin-maintained" {
  name = "bitcoin-${key}"
  inherits = ["maintained-base"]
  matrix = {
    key = keys(variable.MAINTAINED_VERSIONS)
  }
  tags = ["bitcoindevproject/bitcoin:${variable.MAINTAINED_VERSIONS[key].tag}"]
  args = {
    COMMIT_SHA = "${variable.MAINTAINED_VERSIONS[key].commit}"
  }
}

This pattern could be extended to both practice-base and cve-base targets.

This approach:

  1. Makes the configuration more maintainable
  2. Reduces duplication
  3. Makes it easier to add new versions
  4. It creates a clear separation between configuration and implementation

inherits = ["maintained-base"]
tags = ["bitcoindevproject/bitcoin:28.0"]
args = {
COMMIT_SHA = "110183746150428e6385880c79f8c5733b1361ba"
}
}

target "bitcoin-27" {
inherits = ["maintained-base"]
tags = ["bitcoindevproject/bitcoin:27.2"]
args = {
COMMIT_SHA = "bf03c458e994abab9be85486ed8a6d8813313579"
}
}

target "bitcoin-26" {
inherits = ["maintained-base"]
tags = ["bitcoindevproject/bitcoin:26.2"]
args = {
COMMIT_SHA = "7b7041019ba5e7df7bde1416aa6916414a04f3db"
}
}

target "practice-base" {
dockerfile = "./Dockerfile"
context = "./resources/images/bitcoin/insecure"
contexts = {
bitcoin-src = "."
}
args = {
ALPINE_VERSION = "3.20"
BITCOIN_VERSION = "28.1.1"
EXTRA_PACKAGES = "sqlite-dev"
EXTRA_RUNTIME_PACKAGES = ""
REPO = "willcl-ark/bitcoin"
}
platforms = ["linux/amd64", "linux/armhf"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following the same pattern as my previous comment, I recommend making this a reusable variable as well.

}

target "bitcoin-unknown-message" {
inherits = ["practice-base"]
tags = ["bitcoindevproject/bitcoin:99.0.0-unknown-message"]
args = {
COMMIT_SHA = "ae999611026e941eca5c0b61f22012c3b3f3d8dc"
}
}

target "bitcoin-invalid-blocks" {
inherits = ["practice-base"]
tags = ["bitcoindevproject/bitcoin:98.0.0-invalid-blocks"]
args = {
COMMIT_SHA = "9713324368e5a966ec330389a533ae8ad7a0ea8f"
}
}

target "bitcoin-50-orphans" {
inherits = ["practice-base"]
tags = ["bitcoindevproject/bitcoin:97.0.0-50-orphans"]
args = {
COMMIT_SHA = "cbcb308eb29621c0db3a105e1a1c1788fb0dab6b"
}
}

target "bitcoin-no-mp-trim" {
inherits = ["practice-base"]
tags = ["bitcoindevproject/bitcoin:96.0.0-no-mp-trim"]
args = {
COMMIT_SHA = "a3a15a9a06dd541d1dafba068c00eedf07e1d5f8"
}
}

target "bitcoin-disabled-opcodes" {
inherits = ["practice-base"]
tags = ["bitcoindevproject/bitcoin:95.0.0-disabled-opcodes"]
args = {
COMMIT_SHA = "5bdb8c52a8612cac9aa928c84a499dd701542b2a"
}
}

target "bitcoin-5k-inv" {
inherits = ["practice-base"]
tags = ["bitcoindevproject/bitcoin:94.0.0-5k-inv"]
args = {
COMMIT_SHA = "e70e610e07eea3aeb0c49ae0bd9f4049ffc1b88c"
}
}

target "CVE-base" {
dockerfile = "./Dockerfile"
context = "./resources/images/bitcoin/insecure"
contexts = {
bitcoin-src = "."
}
platforms = ["linux/amd64", "linux/armhf"]
args = {
REPO = "josibake/bitcoin"
}
}

target "v0-16-1" {
inherits = ["CVE-base"]
tags = ["bitcoindevproject/bitcoin:0.16.1"]
args = {
ALPINE_VERSION = "3.7"
BITCOIN_VERSION = "0.16.1"
COMMIT_SHA = "dc94c00e58c60412a4e1a540abdf0b56093179e8"
EXTRA_PACKAGES = "protobuf-dev libressl-dev"
EXTRA_RUNTIME_PACKAGES = "boost boost-program_options libressl"
PRE_CONFIGURE_COMMANDS = "sed -i '/AC_PREREQ/a\\AR_FLAGS=cr' src/univalue/configure.ac && sed -i '/AX_PROG_CC_FOR_BUILD/a\\AR_FLAGS=cr' src/secp256k1/configure.ac && sed -i 's:sys/fcntl.h:fcntl.h:' src/compat.h"
}
}

target "v0-17-0" {
inherits = ["CVE-base"]
tags = ["bitcoindevproject/bitcoin:0.17.0"]
args = {
ALPINE_VERSION = "3.9"
BITCOIN_VERSION = "0.17.0"
COMMIT_SHA = "f6b2db49a707e7ad433d958aee25ce561c66521a"
EXTRA_PACKAGES = "protobuf-dev libressl-dev"
EXTRA_RUNTIME_PACKAGES = "boost boost-program_options libressl sqlite-dev"
}
}

target "v0-19-2" {
inherits = ["CVE-base"]
tags = ["bitcoindevproject/bitcoin:0.19.2"]
args = {
ALPINE_VERSION = "3.12.12"
BITCOIN_VERSION = "0.19.2"
COMMIT_SHA = "e20f83eb5466a7d68227af14a9d0cf66fb520ffc"
EXTRA_PACKAGES = "sqlite-dev libressl-dev"
EXTRA_RUNTIME_PACKAGES = "boost boost-program_options libressl sqlite-dev"
}
}

target "v0-20-0" {
inherits = ["CVE-base"]
tags = ["bitcoindevproject/bitcoin:0.20.0"]
args = {
ALPINE_VERSION = "3.12.12"
BITCOIN_VERSION = "0.20.0"
COMMIT_SHA = "0bbff8feff0acf1693dfe41184d9a4fd52001d3f"
EXTRA_PACKAGES = "sqlite-dev miniupnpc-dev"
EXTRA_RUNTIME_PACKAGES = "boost-filesystem miniupnpc-dev sqlite-dev"
}
}

target "v0-21-1" {
inherits = ["CVE-base"]
tags = ["bitcoindevproject/bitcoin:0.21.1"]
args = {
ALPINE_VERSION = "3.17"
BITCOIN_VERSION = "0.21.1"
COMMIT_SHA = "e0a22f14c15b4877ef6221f9ee2dfe510092d734"
EXTRA_PACKAGES = "sqlite-dev"
EXTRA_RUNTIME_PACKAGES = "boost-filesystem sqlite-dev"
}
}
17 changes: 17 additions & 0 deletions docs/developer-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,20 @@ python3 -m build
# Upload to Pypi
python3 -m twine upload dist/*
```

## Building docker images

The Bitcoin Core docker images used by warnet are specified in the *docker-bake.hcl* file.
This uses the (experimental) `bake` build functionality of docker buildx.
We use [HCL language](https://github.com/hashicorp/hcl) in the declaration file itself.
See the `bake` [documentation](https://docs.docker.com/build/bake/) for more information on specifications, and how to e.g. override arguments.

In order to build (or "bake") a certain image, find the image's target (name) in the *docker-bake.hcl* file, and then run `docker buildx bake <target>`.

```bash
# build the dummy image that will crash on 5k invs
docker buildx bake bitcoin-5k-inv

# build the same image, but set platform to only linux/amd64
docker buildx bake bitcoin-5k-inv --set bitcoin-5k-inv.platform=linux/amd64
Comment on lines +77 to +90
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm running the commands provided as example, but can we add a reference on how to create a builder before running the buildx command? Mostly for folks not used to build images for multiple platforms.
Even that I'm used to it I always the forget the commands.

https://docs.docker.com/reference/cli/docker/buildx/create/
https://www.docker.com/blog/how-to-rapidly-build-multi-architecture-images-with-buildx/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, the instructions should outline all required steps. Assuming we're ok with the docker-container build driver (which supports multi-arch image builds), can simply add this as a preliminary step

docker buildx create --name=warnet-builder --use --bootstrap

```
7 changes: 6 additions & 1 deletion resources/images/bitcoin/insecure/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ RUN mkdir -p ${BERKELEYDB_PREFIX}

WORKDIR /${BERKELEYDB_VERSION}/build_unix

RUN ../dist/configure --enable-cxx --disable-shared --with-pic --prefix=${BERKELEYDB_PREFIX}
ARG TARGETPLATFORM
RUN if [ "$TARGETPLATFORM" = "linux/arm64" ]; then \
../dist/configure --enable-cxx --disable-shared --with-pic --prefix=${BERKELEYDB_PREFIX} --build=aarch64-unknown-linux-gnu; \
else \
../dist/configure --enable-cxx --disable-shared --with-pic --prefix=${BERKELEYDB_PREFIX}; \
fi
RUN make -j$(nproc)
RUN make install
RUN rm -rf ${BERKELEYDB_PREFIX}/docs
Expand Down
Loading