From 0edea0994b848f43a608f71f7a4e5ab810281bd0 Mon Sep 17 00:00:00 2001 From: Ben Cressey Date: Sun, 24 Sep 2023 15:52:52 +0000 Subject: [PATCH 1/5] twoliter: switch to fixed directory for tools Secrets have a maximum size of 500 KiB, which is not sufficient to pass binaries to the build. This would prevent us from using native code rather than shell scripts. Install the tools into `build/tools` instead, taking care to clean up any previous installs and to reset the file modification times so as not to bust any caches or trigger unnecessary rebuilds. Signed-off-by: Ben Cressey --- Cargo.lock | 1 + tools/buildsys/src/builder.rs | 28 ------------------ twoliter/Cargo.toml | 1 + twoliter/embedded/Dockerfile | 16 ++--------- twoliter/src/cmd/build.rs | 12 ++++---- twoliter/src/cmd/make.rs | 14 +++++---- twoliter/src/tools.rs | 53 +++++++++++++++++++++++------------ 7 files changed, 55 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e0ff83f1c..9d156f538 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3913,6 +3913,7 @@ dependencies = [ "bytes", "clap", "env_logger", + "filetime", "flate2", "hex", "log", diff --git a/tools/buildsys/src/builder.rs b/tools/buildsys/src/builder.rs index 74a2ba4f4..e963795f6 100644 --- a/tools/buildsys/src/builder.rs +++ b/tools/buildsys/src/builder.rs @@ -86,15 +86,6 @@ lazy_static! { static DOCKER_BUILD_MAX_ATTEMPTS: NonZeroU16 = nonzero!(10u16); -/// The list of tools. -const TOOLS: &[&str] = &[ - "docker-go", - "partyplanner", - "rpm2img", - "rpm2kmodkit", - "rpm2migrations", -]; - pub(crate) struct PackageBuilder; impl PackageBuilder { @@ -133,11 +124,6 @@ impl PackageBuilder { args.build_arg(target_env_var, src_env_val); } - // Add the tools directory as a secret since it is not in the context directory. - args.extend(tools_secret_args(&PathBuf::from( - env::var(TOOLS_DIR).context(error::EnvironmentSnafu { var: TOOLS_DIR })?, - ))); - let tag = format!( "buildsys-pkg-{package}-{arch}", package = package, @@ -236,11 +222,6 @@ impl VariantBuilder { // Add known secrets to the build argments. add_secrets(&mut args)?; - // Add the tools directory as a secret since it is not in the context directory. - args.extend(tools_secret_args(&PathBuf::from( - env::var(TOOLS_DIR).context(error::EnvironmentSnafu { var: TOOLS_DIR })?, - ))); - // Always rebuild variants since they are located in a different workspace, // and don't directly track changes in the underlying packages. getenv("BUILDSYS_TIMESTAMP")?; @@ -676,12 +657,3 @@ where self.as_ref().split(' ').map(String::from).collect() } } - -fn tools_secret_args(tools_dir: &Path) -> impl Iterator { - let mut secrets = Vec::new(); - for &tool in TOOLS { - let id = format!("tools-{}", tool.to_lowercase().replace('.', "-")); - secrets.build_secret("file", &id, &tools_dir.join(tool).display().to_string()) - } - secrets.into_iter() -} diff --git a/twoliter/Cargo.toml b/twoliter/Cargo.toml index db7a8c81a..d4cdace88 100644 --- a/twoliter/Cargo.toml +++ b/twoliter/Cargo.toml @@ -14,6 +14,7 @@ anyhow = "1" async-recursion = "1" clap = { version = "4", features = ["derive", "env", "std"] } env_logger = "0.10" +filetime = "0.2" flate2 = "1" hex = "0.4" log = "0.4" diff --git a/twoliter/embedded/Dockerfile b/twoliter/embedded/Dockerfile index 5abe8119a..9e78e4704 100644 --- a/twoliter/embedded/Dockerfile +++ b/twoliter/embedded/Dockerfile @@ -13,16 +13,6 @@ ARG ARCH ARG GOARCH FROM ${SDK} as sdk -USER root -RUN \ - --mount=type=secret,id=tools-docker-go,mode=755,target=/tmp/tools/docker-go \ - --mount=type=secret,id=tools-partyplanner,mode=755,target=/tmp/tools/partyplanner \ - --mount=type=secret,id=tools-rpm2img,mode=755,target=/tmp/tools/rpm2img \ - --mount=type=secret,id=tools-rpm2kmodkit,mode=755,target=/tmp/tools/rpm2kmodkit \ - --mount=type=secret,id=tools-rpm2migrations,mode=755,target=/tmp/tools/rpm2migrations \ - cp -a /tmp/tools/ /tools/ -USER builder - FROM --platform=linux/${GOARCH} ${TOOLCHAIN}-${ARCH} as toolchain ############################################################################################ @@ -232,7 +222,7 @@ RUN --mount=target=/host \ --mount=type=secret,id=aws-access-key-id.env,target=/root/.aws/aws-access-key-id.env \ --mount=type=secret,id=aws-secret-access-key.env,target=/root/.aws/aws-secret-access-key.env \ --mount=type=secret,id=aws-session-token.env,target=/root/.aws/aws-session-token.env \ - /tools/rpm2img \ + /host/build/tools/rpm2img \ --package-dir=/local/rpms \ --output-dir=/local/output \ --output-fmt="${IMAGE_FORMAT}" \ @@ -265,7 +255,7 @@ RUN --mount=target=/host \ -name "bottlerocket-migrations-*.rpm" \ -not -iname '*debuginfo*' \ -exec cp '{}' '/local/migrations/' ';' \ - && /tools/rpm2migrations \ + && /host/build/tools/rpm2migrations \ --package-dir=/local/migrations \ --output-dir=/local/output \ && echo ${NOCACHE} @@ -291,7 +281,7 @@ RUN --mount=target=/host \ && find /host/build/rpms/ -maxdepth 1 -type f \ -name "bottlerocket-${KERNEL}-archive-*.rpm" \ -exec cp '{}' '/local/archives/' ';' \ - && /tools/rpm2kmodkit \ + && /host/build/tools/rpm2kmodkit \ --archive-dir=/local/archives \ --toolchain-dir=/local/toolchain \ --output-dir=/local/output \ diff --git a/twoliter/src/cmd/build.rs b/twoliter/src/cmd/build.rs index 2dc3f6b32..37e9d114f 100644 --- a/twoliter/src/cmd/build.rs +++ b/twoliter/src/cmd/build.rs @@ -1,7 +1,7 @@ use crate::cargo_make::CargoMake; use crate::docker::DockerContainer; use crate::project; -use crate::tools::{install_tools, tools_tempdir}; +use crate::tools::install_tools; use anyhow::{Context, Result}; use clap::Parser; use log::debug; @@ -42,9 +42,11 @@ impl BuildVariant { pub(super) async fn run(&self) -> Result<()> { let project = project::load_or_find_project(self.project_path.clone()).await?; let token = project.token(); - let tempdir = tools_tempdir()?; - install_tools(&tempdir).await?; - let makefile_path = tempdir.path().join("Makefile.toml"); + let toolsdir = project.project_dir().join("build/tools"); + tokio::fs::remove_dir_all(&toolsdir).await?; + tokio::fs::create_dir_all(&toolsdir).await?; + install_tools(&toolsdir).await?; + let makefile_path = toolsdir.join("Makefile.toml"); // A temporary directory in the `build` directory let build_temp_dir = TempDir::new_in(project.project_dir()) .context("Unable to create a tempdir for Twoliter's build")?; @@ -103,7 +105,7 @@ impl BuildVariant { // Hold the result of the cargo make call so we can clean up the project directory first. let res = CargoMake::new(&project, &self.arch)? - .env("TWOLITER_TOOLS_DIR", tempdir.path().display().to_string()) + .env("TWOLITER_TOOLS_DIR", toolsdir.display().to_string()) .env("BUILDSYS_ARCH", &self.arch) .env("BUILDSYS_VARIANT", &self.variant) .env("BUILDSYS_SBKEYS_DIR", sbkeys_dir.display().to_string()) diff --git a/twoliter/src/cmd/make.rs b/twoliter/src/cmd/make.rs index f297bf63c..13ec0127e 100644 --- a/twoliter/src/cmd/make.rs +++ b/twoliter/src/cmd/make.rs @@ -1,9 +1,10 @@ use crate::cargo_make::CargoMake; -use crate::project; -use crate::tools::{install_tools, tools_tempdir}; +use crate::project::{self}; +use crate::tools::install_tools; use anyhow::Result; use clap::Parser; use std::path::PathBuf; +use tokio::fs; /// Run a cargo make command in Twoliter's build environment. Known Makefile.toml environment /// variables will be passed-through to the cargo make invocation. @@ -33,12 +34,13 @@ pub(crate) struct Make { impl Make { pub(super) async fn run(&self) -> Result<()> { let project = project::load_or_find_project(self.project_path.clone()).await?; - let tempdir = tools_tempdir()?; - install_tools(&tempdir).await?; - let makefile_path = tempdir.path().join("Makefile.toml"); + let toolsdir = project.project_dir().join("build/tools"); + fs::remove_dir_all(&toolsdir).await?; + install_tools(&toolsdir).await?; + let makefile_path = toolsdir.join("Makefile.toml"); CargoMake::new(&project, &self.arch)? .env("CARGO_HOME", self.cargo_home.display().to_string()) - .env("TWOLITER_TOOLS_DIR", tempdir.path().display().to_string()) + .env("TWOLITER_TOOLS_DIR", toolsdir.display().to_string()) .env("BUILDSYS_VERSION_IMAGE", project.release_version()) .makefile(makefile_path) .project_dir(project.project_dir()) diff --git a/twoliter/src/tools.rs b/twoliter/src/tools.rs index bb26a0b7b..43a54e1a2 100644 --- a/twoliter/src/tools.rs +++ b/twoliter/src/tools.rs @@ -1,9 +1,9 @@ use anyhow::{Context, Result}; +use filetime::{set_file_handle_times, set_file_mtime, FileTime}; use flate2::read::ZlibDecoder; use log::debug; use std::path::Path; use tar::Archive; -use tempfile::TempDir; use tokio::fs; use tokio::io::AsyncWriteExt; @@ -16,13 +16,6 @@ const PUBSYS_SETUP: &[u8] = include_bytes!(env!("CARGO_BIN_FILE_PUBSYS_SETUP")); const TESTSYS: &[u8] = include_bytes!(env!("CARGO_BIN_FILE_TESTSYS")); const TUFTOOL: &[u8] = include_bytes!(env!("CARGO_BIN_FILE_TUFTOOL")); -/// Create a `TempDir` object and provide a tools-centric error message if it fails. Make sure you -/// hang on to the `TempDir` for as long as you need it. It will be deleted when it goes out of -/// scope. -pub(crate) fn tools_tempdir() -> Result { - TempDir::new().context("Unable to create a tempdir for Twoliter's tools") -} - /// Install tools into the given `tools_dir`. If you use a `TempDir` object, make sure to pass it by /// reference and hold on to it until you no longer need the tools to still be installed (it will /// auto delete when it goes out of scope). @@ -30,22 +23,31 @@ pub(crate) async fn install_tools(tools_dir: impl AsRef) -> Result<()> { let dir = tools_dir.as_ref(); debug!("Installing tools to '{}'", dir.display()); - write_bin("bottlerocket-variant", BOTTLEROCKET_VARIANT, &dir).await?; - write_bin("buildsys", BUILDSYS, &dir).await?; - write_bin("pubsys", PUBSYS, &dir).await?; - write_bin("pubsys-setup", PUBSYS_SETUP, &dir).await?; - write_bin("testsys", TESTSYS, &dir).await?; - write_bin("tuftool", TUFTOOL, &dir).await?; - // Write out the embedded tools and scripts. unpack_tarball(&dir) .await .context("Unable to install tools")?; + // Pick one of the embedded files for use as the canonical mtime. + let metadata = fs::metadata(dir.join("Dockerfile")) + .await + .context("Unable to get Dockerfile metadata")?; + let mtime = FileTime::from_last_modification_time(&metadata); + + write_bin("bottlerocket-variant", BOTTLEROCKET_VARIANT, &dir, mtime).await?; + write_bin("buildsys", BUILDSYS, &dir, mtime).await?; + write_bin("pubsys", PUBSYS, &dir, mtime).await?; + write_bin("pubsys-setup", PUBSYS_SETUP, &dir, mtime).await?; + write_bin("testsys", TESTSYS, &dir, mtime).await?; + write_bin("tuftool", TUFTOOL, &dir, mtime).await?; + + // Apply the mtime to the directory now that the writes are done. + set_file_mtime(dir, mtime).context(format!("Unable to set mtime for '{}'", dir.display()))?; + Ok(()) } -async fn write_bin(name: &str, data: &[u8], dir: impl AsRef) -> Result<()> { +async fn write_bin(name: &str, data: &[u8], dir: impl AsRef, mtime: FileTime) -> Result<()> { let path = dir.as_ref().join(name); let mut f = fs::OpenOptions::new() .create(true) @@ -60,7 +62,12 @@ async fn write_bin(name: &str, data: &[u8], dir: impl AsRef) -> Result<()> .context(format!("Unable to write to '{}'", path.display()))?; f.flush() .await - .context(format!("Unable to finalize '{}'", path.display())) + .context(format!("Unable to finalize '{}'", path.display()))?; + + let f = f.into_std().await; + set_file_handle_times(&f, None, Some(mtime)) + .context(format!("Unable to set mtime for '{}'", path.display()))?; + Ok(()) } async fn unpack_tarball(tools_dir: impl AsRef) -> Result<()> { @@ -77,7 +84,7 @@ async fn unpack_tarball(tools_dir: impl AsRef) -> Result<()> { #[tokio::test] async fn test_install_tools() { - let tempdir = tools_tempdir().unwrap(); + let tempdir = tempfile::TempDir::new().unwrap(); install_tools(&tempdir).await.unwrap(); // Assert that the expected files exist in the tools directory. @@ -98,4 +105,14 @@ async fn test_install_tools() { assert!(tempdir.path().join("pubsys-setup").is_file()); assert!(tempdir.path().join("testsys").is_file()); assert!(tempdir.path().join("tuftool").is_file()); + + // Check that the mtimes match. + let dockerfile_metadata = fs::metadata(tempdir.path().join("Dockerfile")) + .await + .unwrap(); + let buildsys_metadata = fs::metadata(tempdir.path().join("buildsys")).await.unwrap(); + let dockerfile_mtime = FileTime::from_last_modification_time(&dockerfile_metadata); + let buildsys_mtime = FileTime::from_last_modification_time(&buildsys_metadata); + + assert_eq!(dockerfile_mtime, buildsys_mtime); } From cb1aa1764fc7ea44f7513c9303d073008de3e9de Mon Sep 17 00:00:00 2001 From: Ben Cressey Date: Fri, 6 Oct 2023 18:27:16 +0000 Subject: [PATCH 2/5] twoliter: add target to clean tools directory This leaves an empty build directory when `cargo make clean` runs. Signed-off-by: Ben Cressey --- twoliter/embedded/Makefile.toml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/twoliter/embedded/Makefile.toml b/twoliter/embedded/Makefile.toml index 044960d49..aeec7fc38 100644 --- a/twoliter/embedded/Makefile.toml +++ b/twoliter/embedded/Makefile.toml @@ -1495,6 +1495,7 @@ dependencies = [ "clean-images", "clean-repos", "clean-state", + "clean-tools", ] [tasks.clean-sources] @@ -1545,6 +1546,16 @@ rm -rf ${BUILDSYS_STATE_DIR} ''' ] +[tasks.clean-tools] +script_runner = "bash" +script = [ +''' +if [ -n "${TWOLITER_TOOLS_DIR}" ] ; then + rm -rf "${TWOLITER_TOOLS_DIR}" +fi +''' +] + # Deletes cached code used for Bottlerocket builds [tasks.purge-cache] dependencies = [ From 8419d5c7dc2655680abe7fdfd72ce4e2956d8189 Mon Sep 17 00:00:00 2001 From: Ben Cressey Date: Sun, 24 Sep 2023 20:07:31 +0000 Subject: [PATCH 3/5] twoliter: fix whitespace in embedded Dockerfile Signed-off-by: Ben Cressey --- twoliter/embedded/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/twoliter/embedded/Dockerfile b/twoliter/embedded/Dockerfile index 9e78e4704..859c3d0de 100644 --- a/twoliter/embedded/Dockerfile +++ b/twoliter/embedded/Dockerfile @@ -125,7 +125,7 @@ RUN --mount=target=/host \ --repofrompath repo,./rpmbuild/RPMS \ --enablerepo 'repo' \ --nogpgcheck \ - --forcearch "${ARCH}" \ + --forcearch "${ARCH}" \ builddep rpmbuild/SPECS/${PACKAGE}.spec # We use the "nocache" writable space to generate code where necessary, like the variant- @@ -176,7 +176,7 @@ RUN --mount=target=/host \ --nogpgcheck \ --downloadonly \ --downloaddir . \ - --forcearch "${ARCH}" \ + --forcearch "${ARCH}" \ install $(printf "bottlerocket-%s\n" ${PACKAGES}) \ && mv *.rpm /local/rpms \ && createrepo_c /local/rpms \ From 5490dae0e85d4d2c740c13390e1de672ea167e6c Mon Sep 17 00:00:00 2001 From: Ben Cressey Date: Mon, 25 Sep 2023 14:53:10 +0000 Subject: [PATCH 4/5] twoliter: embed .dockerignore The build system has very specific requirements on what to include in the Docker build context in order to work as expected. Embedding the file avoids the need for a synchronized change in other projects when updating Twoliter. Signed-off-by: Ben Cressey --- twoliter/build.rs | 1 + twoliter/embedded/Dockerfile | 4 ++-- twoliter/embedded/Dockerfile.dockerignore | 12 ++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 twoliter/embedded/Dockerfile.dockerignore diff --git a/twoliter/build.rs b/twoliter/build.rs index aa8e9f08a..40eabc335 100644 --- a/twoliter/build.rs +++ b/twoliter/build.rs @@ -26,6 +26,7 @@ fn main() { )); paths.copy_file("Dockerfile"); + paths.copy_file("Dockerfile.dockerignore"); paths.copy_file("Makefile.toml"); paths.copy_file("docker-go"); paths.copy_file("partyplanner"); diff --git a/twoliter/embedded/Dockerfile b/twoliter/embedded/Dockerfile index 859c3d0de..2d0100305 100644 --- a/twoliter/embedded/Dockerfile +++ b/twoliter/embedded/Dockerfile @@ -31,7 +31,7 @@ ARG TOKEN # We can't create directories via RUN in a scratch container, so take an existing one. COPY --chown=1000:1000 --from=sdk /tmp /cache # Ensure the ARG variables are used in the layer to prevent reuse by other builds. -COPY --chown=1000:1000 .dockerignore /cache/.${PACKAGE}.${ARCH}.${TOKEN} +COPY --chown=1000:1000 .gitignore /cache/.${PACKAGE}.${ARCH}.${TOKEN} # =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= # Some builds need to modify files in the source directory, for example Rust software using @@ -52,7 +52,7 @@ ARG TOKEN # We can't create directories via RUN in a scratch container, so take an existing one. COPY --chown=1000:1000 --from=sdk /tmp /variantcache # Ensure the ARG variables are used in the layer to prevent reuse by other builds. -COPY --chown=1000:1000 .dockerignore /variantcache/.${PACKAGE}.${ARCH}.${VARIANT}.${TOKEN} +COPY --chown=1000:1000 .gitignore /variantcache/.${PACKAGE}.${ARCH}.${VARIANT}.${TOKEN} # =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= # Builds an RPM package from a spec file. diff --git a/twoliter/embedded/Dockerfile.dockerignore b/twoliter/embedded/Dockerfile.dockerignore new file mode 100644 index 000000000..c2bf40d23 --- /dev/null +++ b/twoliter/embedded/Dockerfile.dockerignore @@ -0,0 +1,12 @@ +/.git +/.gomodcache +/build/* +!/build/rpms/ +/build/rpms/* +!/build/rpms/*.rpm +/build/rpms/*-debuginfo-*.rpm +/build/rpms/*-debugsource-*.rpm +!/build/tools/* +**/target/* +/sbkeys +/tests From d7aba2510e637df589ffdce5054abc0957d789ac Mon Sep 17 00:00:00 2001 From: Ben Cressey Date: Mon, 9 Oct 2023 17:57:44 +0000 Subject: [PATCH 5/5] twoliter: clean up prep dir between builds Otherwise, files that were present in an earlier build that have been renamed or removed might still be in the prep directory, where they will be added to the new archive. Signed-off-by: Ben Cressey --- twoliter/build.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/twoliter/build.rs b/twoliter/build.rs index 40eabc335..51410b577 100644 --- a/twoliter/build.rs +++ b/twoliter/build.rs @@ -20,6 +20,7 @@ fn main() { let paths = Paths::new(); println!("cargo:rerun-if-changed={}", paths.data_input_dir.display()); + let _ = fs::remove_dir_all(&paths.prep_dir); fs::create_dir_all(&paths.prep_dir).expect(&format!( "Unable to create directory '{}'", paths.prep_dir.display()