use project file sdk in twoliter make

Require the SDK to be specified in Twoliter.toml when using twoliter make.
bottlerocket-os · Sep 29, 2023 · d71e833 · d71e833
1 parent 9e1f8db
commit d71e833
Show file tree

Hide file tree

Showing 6 changed files with 108 additions and 38 deletions.
diff --git 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::<u32>();

diff --git a/tools/buildsys/src/constants.rs 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
@@ -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
@@ -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
@@ -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
@@ -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<str>) -> bool {
     let key = key.as_ref();
@@ -98,6 +127,31 @@ fn is_build_system_env(key: impl AsRef<str>) -> 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());
+}