Skip to content

Commit

Permalink
twoliter: consistently provide image override view
Browse files Browse the repository at this point in the history
Twoliter had a bug in which overridden SDK images were resolved, but then
the wrong SDK was being passed into builds.

This change:
* Uses the the sealed trait/typestate pattern to modify the Project with
  interfaces to fetch images based on the lock level.
* Provides clarified public interfaces for getting images from pre/post
  lock resolution.
* Forces public and private callers to deal with possibly-overridden
  images.
  • Loading branch information
cbgbt committed Sep 11, 2024
1 parent 593209d commit 10af17f
Show file tree
Hide file tree
Showing 13 changed files with 462 additions and 334 deletions.
2 changes: 2 additions & 0 deletions tests/projects/local-kit/Twoliter.override
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[bottlerocket.bottlerocket-sdk]
registry = "public.ecr.aws/bottlerocket"
4 changes: 3 additions & 1 deletion tests/projects/local-kit/Twoliter.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@ vendor = "bottlerocket"
version = "0.41.0"

[vendor.bottlerocket]
registry = "public.ecr.aws/bottlerocket"
# This test will fail if Twoliter overrides aren't working
# See `Twoliter.override`
registry = "this-definitely-wont-resolve/bottlerocket"
2 changes: 1 addition & 1 deletion tools/oci-cli-wrapper/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ impl ImageTool {
}

#[async_trait]
pub trait ImageToolImpl: std::fmt::Debug {
pub trait ImageToolImpl: std::fmt::Debug + Send + Sync + 'static {
/// Pull an image archive to disk
async fn pull_oci_image(&self, path: &Path, uri: &str) -> Result<()>;
/// Fetch the image config
Expand Down
10 changes: 5 additions & 5 deletions twoliter/src/cmd/build.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::build_clean::BuildClean;
use crate::cargo_make::CargoMake;
use crate::common::fs;
use crate::project;
use crate::project::{self, Locked};
use crate::tools::install_tools;
use anyhow::{Context, Result};
use clap::Parser;
Expand Down Expand Up @@ -52,7 +52,7 @@ pub(crate) struct BuildKit {
impl BuildKit {
pub(super) async fn run(&self) -> Result<()> {
let project = project::load_or_find_project(self.project_path.clone()).await?;
let lock = project.load_lock().await?;
let project = project.load_lock::<Locked>().await?;
let toolsdir = project.project_dir().join("build/tools");
install_tools(&toolsdir).await?;
let makefile_path = toolsdir.join("Makefile.toml");
Expand All @@ -63,7 +63,7 @@ impl BuildKit {
optional_envs.push(("BUILDSYS_LOOKASIDE_CACHE", lookaside_cache))
}

CargoMake::new(&lock.sdk.source)?
CargoMake::new(&project.sdk_image().project_image_uri().to_string())?
.env("TWOLITER_TOOLS_DIR", toolsdir.display().to_string())
.env("BUILDSYS_ARCH", &self.arch)
.env("BUILDSYS_KIT", &self.kit)
Expand Down Expand Up @@ -112,7 +112,7 @@ pub(crate) struct BuildVariant {
impl BuildVariant {
pub(super) async fn run(&self) -> Result<()> {
let project = project::load_or_find_project(self.project_path.clone()).await?;
let lock = project.load_lock().await?;
let project = project.load_lock::<Locked>().await?;
let toolsdir = project.project_dir().join("build/tools");
install_tools(&toolsdir).await?;
let makefile_path = toolsdir.join("Makefile.toml");
Expand All @@ -135,7 +135,7 @@ impl BuildVariant {
))
}

CargoMake::new(&lock.sdk.source)?
CargoMake::new(&project.sdk_image().project_image_uri().to_string())?
.env("TWOLITER_TOOLS_DIR", toolsdir.display().to_string())
.env("BUILDSYS_ARCH", &self.arch)
.env("BUILDSYS_VARIANT", &self.variant)
Expand Down
6 changes: 3 additions & 3 deletions twoliter/src/cmd/build_clean.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::cargo_make::CargoMake;
use crate::project;
use crate::project::{self, Locked};
use crate::tools;
use anyhow::Result;
use clap::Parser;
Expand All @@ -15,12 +15,12 @@ pub(crate) struct BuildClean {
impl BuildClean {
pub(super) async fn run(&self) -> Result<()> {
let project = project::load_or_find_project(self.project_path.clone()).await?;
let lock = project.load_lock().await?;
let project = project.load_lock::<Locked>().await?;
let toolsdir = project.project_dir().join("build/tools");
tools::install_tools(&toolsdir).await?;
let makefile_path = toolsdir.join("Makefile.toml");

CargoMake::new(&lock.sdk.source)?
CargoMake::new(&project.sdk_image().project_image_uri().to_string())?
.env("TWOLITER_TOOLS_DIR", toolsdir.display().to_string())
.makefile(makefile_path)
.project_dir(project.project_dir())
Expand Down
6 changes: 3 additions & 3 deletions twoliter/src/cmd/fetch.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::project;
use crate::project::{self, Locked};
use anyhow::Result;
use clap::Parser;
use std::path::PathBuf;
Expand All @@ -17,8 +17,8 @@ pub(crate) struct Fetch {
impl Fetch {
pub(super) async fn run(&self) -> Result<()> {
let project = project::load_or_find_project(self.project_path.clone()).await?;
let lock_file = project.load_lock().await?;
lock_file.fetch(&project, self.arch.as_str()).await?;
let project = project.load_lock::<Locked>().await?;
project.fetch(self.arch.as_str()).await?;
Ok(())
}
}
24 changes: 12 additions & 12 deletions twoliter/src/cmd/make.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::cargo_make::CargoMake;
use crate::project::{self};
use crate::project::{self, Locked, SDKLocked, Unlocked};
use crate::tools::install_tools;
use anyhow::Result;
use clap::Parser;
Expand Down Expand Up @@ -68,24 +68,23 @@ impl Make {
.await
}

fn can_skip_kit_verification(&self, project: &project::Project) -> bool {
fn can_skip_kit_verification(&self, project: &project::Project<Unlocked>) -> bool {
let target_allows_kit_verification_skip =
!MUST_VALIDATE_KITS_TARGETS.contains(&self.makefile_task.as_str());
let project_has_explicit_sdk_dep = project.sdk_image().is_some();
let project_has_explicit_sdk_dep = project.direct_sdk_image_dep().is_some();

target_allows_kit_verification_skip && project_has_explicit_sdk_dep
}

/// Returns the locked SDK image for the project.
async fn locked_sdk(&self, project: &project::Project) -> Result<String> {
let sdk_source = if self.can_skip_kit_verification(project) {
let lock = project.load_locked_sdk().await?;
lock.0.source
async fn locked_sdk(&self, project: &project::Project<Unlocked>) -> Result<String> {
Ok(if self.can_skip_kit_verification(project) {
project.load_lock::<SDKLocked>().await?.sdk_image()
} else {
let lock = project.load_lock().await?;
lock.sdk.source
};
Ok(sdk_source)
project.load_lock::<Locked>().await?.sdk_image()
}
.project_image_uri()
.to_string())
}
}

Expand Down Expand Up @@ -214,7 +213,8 @@ mod test {
let project = project::load_or_find_project(Some(project_path))
.await
.unwrap();
let sdk_source = project.load_locked_sdk().await.unwrap().0.source;
let project = project.load_lock::<SDKLocked>().await.unwrap();
let sdk_source = project.sdk_image().project_image_uri().to_string();

if delete_verifier_tags {
// Clean up tags so that the build fails
Expand Down
6 changes: 3 additions & 3 deletions twoliter/src/cmd/publish_kit.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::cargo_make::CargoMake;
use crate::project;
use crate::project::{self, Locked};
use crate::tools::install_tools;
use anyhow::Result;
use clap::Parser;
Expand Down Expand Up @@ -36,12 +36,12 @@ pub(crate) struct PublishKit {
impl PublishKit {
pub(super) async fn run(&self) -> Result<()> {
let project = project::load_or_find_project(self.project_path.clone()).await?;
let lock = project.load_lock().await?;
let project = project.load_lock::<Locked>().await?;
let toolsdir = project.project_dir().join("build/tools");
install_tools(&toolsdir).await?;
let makefile_path = toolsdir.join("Makefile.toml");

CargoMake::new(&lock.sdk.source)?
CargoMake::new(project.sdk_image().project_image_uri().to_string().as_str())?
.env("TWOLITER_TOOLS_DIR", toolsdir.display().to_string())
.env("BUILDSYS_KIT", &self.kit_name)
.env("BUILDSYS_VERSION_IMAGE", project.release_version())
Expand Down
Loading

0 comments on commit 10af17f

Please sign in to comment.