From d71e833633e117622b6f97bd81fea31abc70370b Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Thu, 28 Sep 2023 17:27:13 -0700 Subject: [PATCH] use project file sdk in twoliter make Require the SDK to be specified in Twoliter.toml when using twoliter make. --- tools/buildsys/src/builder.rs | 8 ++-- tools/buildsys/src/constants.rs | 4 ++ tools/buildsys/src/gomod.rs | 5 +-- tools/buildsys/src/main.rs | 1 + twoliter/embedded/Makefile.toml | 66 ++++++++++++++++++--------------- twoliter/src/cmd/make.rs | 62 ++++++++++++++++++++++++++++++- 6 files changed, 108 insertions(+), 38 deletions(-) create mode 100644 tools/buildsys/src/constants.rs diff --git a/tools/buildsys/src/builder.rs b/tools/buildsys/src/builder.rs index 38f8bb727..aaba9811a 100644 --- a/tools/buildsys/src/builder.rs +++ b/tools/buildsys/src/builder.rs @@ -7,6 +7,8 @@ the repository's top-level Dockerfile. pub(crate) mod error; use error::Result; +use crate::constants::{SDK_VAR, TOOLCHAIN_VAR}; +use buildsys::manifest::{ImageFeature, ImageFormat, ImageLayout, PartitionPlan, SupportedArch}; use duct::cmd; use lazy_static::lazy_static; use nonzero_ext::nonzero; @@ -22,8 +24,6 @@ use std::path::{Path, PathBuf}; use std::process::Output; use walkdir::{DirEntry, WalkDir}; -use buildsys::manifest::{ImageFeature, ImageFormat, ImageLayout, PartitionPlan, SupportedArch}; - const TOOLS_DIR: &str = "TWOLITER_TOOLS_DIR"; /* @@ -283,8 +283,8 @@ fn build( let tag = format!("{}-{}", tag, token); // Our SDK and toolchain are picked by the external `cargo make` invocation. - let sdk = getenv("BUILDSYS_SDK_IMAGE")?; - let toolchain = getenv("BUILDSYS_TOOLCHAIN")?; + let sdk = getenv(SDK_VAR)?; + let toolchain = getenv(TOOLCHAIN_VAR)?; // Avoid using a cached layer from a previous build. let nocache = rand::thread_rng().gen::(); diff --git a/tools/buildsys/src/constants.rs b/tools/buildsys/src/constants.rs new file mode 100644 index 000000000..b561aaa64 --- /dev/null +++ b/tools/buildsys/src/constants.rs @@ -0,0 +1,4 @@ +///! Constants that are used in more than one module. + +pub(crate) const SDK_VAR: &str = "TLPRIVATE_SDK_IMAGE"; +pub(crate) const TOOLCHAIN_VAR: &str = "TLPRIVATE_TOOLCHAIN"; diff --git a/tools/buildsys/src/gomod.rs b/tools/buildsys/src/gomod.rs index 59d2ebcb3..c0a835a7e 100644 --- a/tools/buildsys/src/gomod.rs +++ b/tools/buildsys/src/gomod.rs @@ -18,6 +18,7 @@ when the docker-go script is invoked. pub(crate) mod error; use error::Result; +use crate::constants::SDK_VAR; use buildsys::manifest; use duct::cmd; use snafu::{ensure, OptionExt, ResultExt}; @@ -111,9 +112,7 @@ impl GoMod { ); // Our SDK and toolchain are picked by the external `cargo make` invocation. - let sdk = env::var("BUILDSYS_SDK_IMAGE").context(error::EnvironmentSnafu { - var: "BUILDSYS_SDK_IMAGE", - })?; + let sdk = env::var(SDK_VAR).context(error::EnvironmentSnafu { var: SDK_VAR })?; let args = DockerGoArgs { module_path: package_dir, diff --git a/tools/buildsys/src/main.rs b/tools/buildsys/src/main.rs index 5ea01121f..8f457305b 100644 --- a/tools/buildsys/src/main.rs +++ b/tools/buildsys/src/main.rs @@ -10,6 +10,7 @@ The implementation is closely tied to the top-level Dockerfile. */ mod builder; mod cache; +mod constants; mod gomod; mod project; mod spec; diff --git a/twoliter/embedded/Makefile.toml b/twoliter/embedded/Makefile.toml index 4eaf43b08..eaf60f21d 100644 --- a/twoliter/embedded/Makefile.toml +++ b/twoliter/embedded/Makefile.toml @@ -31,12 +31,6 @@ BUILDSYS_NAME = "bottlerocket" # If you're building a Bottlerocket remix, you'd want to set this to something like # "Bottlerocket Remix by ${CORP}" or "${CORP}'s Bottlerocket Remix" BUILDSYS_PRETTY_NAME = "Bottlerocket OS" -# SDK name used for building -BUILDSYS_SDK_NAME="bottlerocket" -# SDK version used for building -BUILDSYS_SDK_VERSION="v0.33.0" -# Site for fetching the SDK -BUILDSYS_REGISTRY="public.ecr.aws/bottlerocket" # These can be overridden with -e to change configuration for pubsys (`cargo # make repo`). In addition, you can set RELEASE_START_TIME to determine when @@ -142,11 +136,6 @@ TESTSYS_LOG_LEVEL = "info" # Certain variables are defined here to allow us to override a component value # on the command line. -# Depends on ${BUILDSYS_ARCH}, ${BUILDSYS_REGISTRY}, ${BUILDSYS_SDK_NAME}, and -# ${BUILDSYS_SDK_VERSION}. -BUILDSYS_SDK_IMAGE = { script = [ "echo ${BUILDSYS_REGISTRY}/${BUILDSYS_SDK_NAME}-sdk-${BUILDSYS_ARCH}:${BUILDSYS_SDK_VERSION}" ] } -BUILDSYS_TOOLCHAIN = { script = [ "echo ${BUILDSYS_REGISTRY}/${BUILDSYS_SDK_NAME}-toolchain-${BUILDSYS_ARCH}:${BUILDSYS_SDK_VERSION}" ] } - # Depends on ${BUILDSYS_JOBS}. CARGO_MAKE_CARGO_LIMIT_JOBS = "--jobs ${BUILDSYS_JOBS}" CARGO_MAKE_CARGO_ARGS = "--offline --locked" @@ -238,7 +227,17 @@ fi ''' ] } +# These are variables that are not meant to be set by users of `twoliter make`. These are intended +# to be set only by Twoliter itself when it invokes `cargo make`. +[env.private] +# The URIs for the SDK image and the toolchain image must be provided. +TLPRIVATE_SDK_IMAGE = "" +TLPRIVATE_TOOLCHAIN = "" + +#################################################################################################### + [tasks.setup] +script_runner = "bash" script = [ ''' # Ensure we use a supported architecture @@ -256,6 +255,13 @@ if [ -z "${TWOLITER_TOOLS_DIR}" ];then exit 1 fi +# Ensure TLPRIVATE_SDK_IMAGE and TLPRIVATE_TOOLCHAIN are set +if [[ -z "${TLPRIVATE_SDK_IMAGE}" || -z "{TLPRIVATE_TOOLCHAIN}" ]];then + echo "TLPRIVATE_SDK_IMAGE and TLPRIVATE_TOOLCHAIN must be defined and must be non-zero in length." + echo "Are you using Twoliter? It is a bug if Twoliter has invoked cargo make without these." + exit 1 +fi + mkdir -p ${BUILDSYS_BUILD_DIR} mkdir -p ${BUILDSYS_OUTPUT_DIR} mkdir -p ${BUILDSYS_PACKAGES_DIR} @@ -290,9 +296,9 @@ dependencies = ["setup-build"] script_runner = "bash" script = [ ''' -if ! docker image inspect "${BUILDSYS_SDK_IMAGE}" >/dev/null 2>&1 ; then - if ! docker pull "${BUILDSYS_SDK_IMAGE}" ; then - echo "failed to pull '${BUILDSYS_SDK_IMAGE}'" >&2 +if ! docker image inspect "${TLPRIVATE_SDK_IMAGE}" >/dev/null 2>&1 ; then + if ! docker pull "${TLPRIVATE_SDK_IMAGE}" ; then + echo "failed to pull '${TLPRIVATE_SDK_IMAGE}'" >&2 exit 1 fi fi @@ -304,7 +310,7 @@ dependencies = ["setup-build"] script_runner = "bash" script = [ ''' -if docker image inspect "${BUILDSYS_TOOLCHAIN}-${BUILDSYS_ARCH}" >/dev/null 2>&1 ; then +if docker image inspect "${TLPRIVATE_TOOLCHAIN}-${BUILDSYS_ARCH}" >/dev/null 2>&1 ; then exit 0 fi @@ -315,14 +321,14 @@ esac # We want the image with the target's native toolchain, rather than one that matches the # host architecture. -if ! docker pull --platform "${docker_arch}" "${BUILDSYS_TOOLCHAIN}" ; then - echo "could not pull '${BUILDSYS_TOOLCHAIN}' for ${docker_arch}" >&2 +if ! docker pull --platform "${docker_arch}" "${TLPRIVATE_TOOLCHAIN}" ; then + echo "could not pull '${TLPRIVATE_TOOLCHAIN}' for ${docker_arch}" >&2 exit 1 fi # Apply a tag to distinguish the image from other architectures. -if ! docker tag "${BUILDSYS_TOOLCHAIN}" "${BUILDSYS_TOOLCHAIN}-${BUILDSYS_ARCH}" ; then - echo "could not tag '${BUILDSYS_TOOLCHAIN}-${BUILDSYS_ARCH}'" >&2 +if ! docker tag "${TLPRIVATE_TOOLCHAIN}" "${TLPRIVATE_TOOLCHAIN}-${BUILDSYS_ARCH}" ; then + echo "could not tag '${TLPRIVATE_TOOLCHAIN}-${BUILDSYS_ARCH}'" >&2 exit 1 fi ''' @@ -350,7 +356,7 @@ go_fetch() { module="${1:?}" ${TWOLITER_TOOLS_DIR}/docker-go \ --module-path "${BUILDSYS_SOURCES_DIR}/${module}" \ - --sdk-image ${BUILDSYS_SDK_IMAGE} \ + --sdk-image ${TLPRIVATE_SDK_IMAGE} \ --go-mod-cache ${GO_MOD_CACHE} \ --command "go list -mod=readonly ./... >/dev/null && go mod vendor" } @@ -379,7 +385,7 @@ test_go_module() { module="${1:?}" ${TWOLITER_TOOLS_DIR}/docker-go \ --module-path "${BUILDSYS_SOURCES_DIR}/${module}" \ - --sdk-image ${BUILDSYS_SDK_IMAGE} \ + --sdk-image ${TLPRIVATE_SDK_IMAGE} \ --go-mod-cache ${GO_MOD_CACHE} \ --command "cd cmd/$module; go test -v" } @@ -410,7 +416,7 @@ go_fmt() { module="${1:?}" ${TWOLITER_TOOLS_DIR}/docker-go \ --module-path "${BUILDSYS_SOURCES_DIR}/${module}" \ - --sdk-image ${BUILDSYS_SDK_IMAGE} \ + --sdk-image ${TLPRIVATE_SDK_IMAGE} \ --go-mod-cache ${GO_MOD_CACHE} \ --command "gofmt -l cmd/$module" } @@ -429,7 +435,7 @@ if ! docker run --rm \ -e CARGO_HOME="/tmp/.cargo" \ -v "${CARGO_HOME}":/tmp/.cargo \ -v "${BUILDSYS_ROOT_DIR}/sources":/tmp/sources \ - "${BUILDSYS_SDK_IMAGE}" \ + "${TLPRIVATE_SDK_IMAGE}" \ cargo fmt \ --manifest-path /tmp/sources/Cargo.toml \ --message-format short \ @@ -466,7 +472,7 @@ if ! docker run --rm \ -v "${CARGO_HOME}":/tmp/.cargo \ -v "${BUILDSYS_ROOT_DIR}/sources":/tmp/sources \ -e VARIANT \ - "${BUILDSYS_SDK_IMAGE}" \ + "${TLPRIVATE_SDK_IMAGE}" \ cargo clippy \ --manifest-path /tmp/sources/Cargo.toml \ --locked -- -D warnings --no-deps; then @@ -491,7 +497,7 @@ if ! docker run --rm \ --user "$(id -u):$(id -g)" \ --security-opt="label=disable" \ -v "${BUILDSYS_TOOLS_DIR}":/tmp/tools \ - "${BUILDSYS_SDK_IMAGE}" \ + "${TLPRIVATE_SDK_IMAGE}" \ bash -c \ 'flagged_scripts=0 && \ cd /tmp/tools && \ @@ -657,7 +663,7 @@ echo "Generating local keys." >&2 mkdir -p "${BUILDSYS_SBKEYS_PROFILE_DIR}" ${BUILDSYS_SBKEYS_DIR}/generate-local-sbkeys \ - --sdk-image "${BUILDSYS_SDK_IMAGE}" \ + --sdk-image "${TLPRIVATE_SDK_IMAGE}" \ --output-dir "${BUILDSYS_SBKEYS_PROFILE_DIR}" ''' ] @@ -714,7 +720,7 @@ docker run --rm \ --security-opt="label=disable" \ -v "${BOOT_CONFIG_INPUT}":/tmp/bootconfig-input \ -v "${boot_config}":/tmp/bootconfig.data \ - "${BUILDSYS_SDK_IMAGE}" \ + "${TLPRIVATE_SDK_IMAGE}" \ bootconfig -a /tmp/bootconfig-input /tmp/bootconfig.data if [ -e "${boot_config_tmp}" ] ; then @@ -734,7 +740,7 @@ docker run --rm \ --user "$(id -u):$(id -g)" \ --security-opt="label=disable" \ -v "${BOOT_CONFIG}":/tmp/bootconfig.data \ - "${BUILDSYS_SDK_IMAGE}" \ + "${TLPRIVATE_SDK_IMAGE}" \ bootconfig -l /tmp/bootconfig.data ''' ] @@ -816,7 +822,7 @@ docker run --rm \ -e CARGO_HOME="/tmp/.cargo" \ -v "${CARGO_HOME}":/tmp/.cargo \ -v "${BUILDSYS_ROOT_DIR}/sources":/tmp/sources \ - "${BUILDSYS_SDK_IMAGE}" \ + "${TLPRIVATE_SDK_IMAGE}" \ bash -c "${run_cargo_deny}" [ "${?}" -eq 0 ] || [ "${BUILDSYS_ALLOW_FAILED_LICENSE_CHECK}" = "true" ] ''' @@ -854,7 +860,7 @@ docker run --rm \ -v "${CARGO_HOME}":/tmp/.cargo \ -v "${BUILDSYS_ROOT_DIR}/licenses:/tmp/licenses" \ -v "${BUILDSYS_ROOT_DIR}/Licenses.toml:/tmp/Licenses.toml" \ - "${BUILDSYS_SDK_IMAGE}" \ + "${TLPRIVATE_SDK_IMAGE}" \ bash -c "${run_fetch_licenses}" ''' ] diff --git a/twoliter/src/cmd/make.rs b/twoliter/src/cmd/make.rs index b77d9b274..6e35e8061 100644 --- a/twoliter/src/cmd/make.rs +++ b/twoliter/src/cmd/make.rs @@ -1,7 +1,9 @@ use crate::common::exec; +use crate::docker::ImageArchUri; use crate::project; +use crate::project::Project; use crate::tools::{install_tools, tools_tempdir}; -use anyhow::Result; +use anyhow::{bail, ensure, Result}; use clap::Parser; use log::trace; use std::path::PathBuf; @@ -45,14 +47,34 @@ impl Make { project.project_dir().display().to_string(), ]; + let mut arch = String::new(); + for (key, val) in std::env::vars() { if is_build_system_env(key.as_str()) { trace!("Passing env var {} to cargo make", key); args.push("-e".to_string()); args.push(format!("{}={}", key, val)); } + + // To avoid confusion, environment variables whose values have been moved to + // Twoliter.toml are expressly disallowed here. + check_for_disallowed_var(&key)?; + + if key == "BUILDSYS_ARCH" { + arch = val.clone(); + } } + ensure!( + !arch.is_empty(), + "It is required to pass a non-zero string as the value of environment variable \ + 'BUILDSYS_ARCH' when running twoliter make" + ); + + let (sdk, toolchain) = require_sdk(&project, &arch)?; + + args.push(format!("-e=TLPRIVATE_SDK_IMAGE={}", sdk)); + args.push(format!("-e=TLPRIVATE_TOOLCHAIN={}", toolchain)); args.push(format!("-e=CARGO_HOME={}", self.cargo_home.display())); args.push(format!( "-e=TWOLITER_TOOLS_DIR={}", @@ -86,6 +108,13 @@ const ENV_VARS: [&str; 12] = [ "VMWARE_VM_NAME_DEFAULT", ]; +const DISALLOWED_SDK_VARS: [&str; 4] = [ + "BUILDSYS_SDK_NAME", + "BUILDSYS_SDK_VERSION", + "BUILDSYS_REGISTRY", + "BUILDSYS_TOOLCHAIN", +]; + /// Returns `true` if `key` is an environment variable that needs to be passed to `cargo make`. fn is_build_system_env(key: impl AsRef) -> bool { let key = key.as_ref(); @@ -98,6 +127,31 @@ fn is_build_system_env(key: impl AsRef) -> bool { || ENV_VARS.contains(&key) } +fn check_for_disallowed_var(key: &str) -> Result<()> { + if DISALLOWED_SDK_VARS.contains(&key) { + bail!( + "The environment variable '{}' can no longer be used. Specify the SDK in Twoliter.toml", + key + ) + } + Ok(()) +} + +fn require_sdk(project: &Project, arch: &str) -> Result<(ImageArchUri, ImageArchUri)> { + match (project.sdk(arch), project.toolchain(arch)) { + (Some(s), Some(t)) => { + ensure!( + !s.to_string().is_empty() && !t.to_string().is_empty(), + "The SDK fields cannot be zero length strings" + ); + Ok((s, t)) + } + _ => bail!( + "When using twoliter make, it is required that the SDK be specified in Twoliter.toml" + ), + } +} + #[test] fn test_is_build_system_env() { assert!(is_build_system_env( @@ -113,3 +167,9 @@ fn test_is_build_system_env() { assert!(!is_build_system_env("HOME")); assert!(!is_build_system_env("COLORTERM")); } + +#[test] +fn test_check_for_disallowed_var() { + assert!(check_for_disallowed_var("BUILDSYS_REGISTRY").is_err()); + assert!(check_for_disallowed_var("BUILDSYS_PRETTY_NAME").is_ok()); +}