Skip to content

Commit cfa9d69

Browse files
committed
fix(deploy): keep an explicit app.dockerfile out of the .stacker rewrite
normalize_generated_compose_paths rewrote the app build section to `dockerfile: .stacker/Dockerfile` unconditionally, but that file is only generated when app.dockerfile is unset — a config declaring `app.dockerfile: Dockerfile` failed the local build with 'open .stacker/Dockerfile: no such file or directory'. - normalize now receives app.dockerfile: unset → point the build at the generated file, set → keep the configured path (and repair a stale .stacker/Dockerfile left by the previous behaviour) - reroot_relative_context() re-roots every relative build context for the .stacker/ compose location ('.' → '..', './backend' → '../backend'); absolute and already-normalized contexts are left alone - generated_dockerfile_ref() expresses the generated Dockerfile relative to the context directory, which is where Compose resolves it from
1 parent 809ea90 commit cfa9d69

1 file changed

Lines changed: 254 additions & 24 deletions

File tree

‎src/console/commands/cli/deploy.rs‎

Lines changed: 254 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -719,16 +719,66 @@ fn normalize_service_bind_sources(
719719
changed
720720
}
721721

722+
/// Re-root a `build.context` that was authored relative to the project root
723+
/// (`app.path`, default `.`) so it resolves correctly from a compose file
724+
/// living in `.stacker/`.
725+
///
726+
/// Returns `(compose_context, context_relative_to_project_root)`, or `None`
727+
/// for absolute paths that need no rewriting. A context of `..` is returned
728+
/// unchanged (it already points at the project root from `.stacker/`).
729+
fn reroot_relative_context(context: &str) -> Option<(String, String)> {
730+
if Path::new(context).is_absolute() {
731+
return None;
732+
}
733+
let normalized = context.trim_start_matches("./");
734+
if normalized.starts_with("..") {
735+
// Already expressed relative to `.stacker/` (a previous normalization).
736+
return (context == "..").then(|| ("..".to_string(), ".".to_string()));
737+
}
738+
let root_rel = if normalized.is_empty() || normalized == "." {
739+
".".to_string()
740+
} else {
741+
normalized.to_string()
742+
};
743+
let compose_context = if root_rel == "." {
744+
"..".to_string()
745+
} else {
746+
format!("../{root_rel}")
747+
};
748+
Some((compose_context, root_rel))
749+
}
750+
751+
/// Compose resolves `build.dockerfile` relative to `build.context`. Given a
752+
/// context path relative to the project root (`.` = the root itself), return
753+
/// the value pointing at the generated `<root>/.stacker/Dockerfile`.
754+
fn generated_dockerfile_ref(context_relative_to_root: &str) -> String {
755+
let depth = context_relative_to_root
756+
.split('/')
757+
.filter(|segment| !segment.is_empty() && *segment != ".")
758+
.count();
759+
let mut reference = String::new();
760+
for _ in 0..depth {
761+
reference.push_str("../");
762+
}
763+
reference.push_str(".stacker/Dockerfile");
764+
reference
765+
}
766+
722767
/// Normalize the generated `.stacker/docker-compose.yml` in place.
723768
///
724769
/// Besides the obsolete `version:` key and `build.context`/`dockerfile`
725770
/// rewrites, relative bind-mount sources are expressed relative to the project
726771
/// root, because the compose file lives in `.stacker/` while every path in it
727772
/// was authored in `stacker.yml` relative to the project root. See
728773
/// [`restage_relative_path`] for the per-target direction of that rewrite.
774+
///
775+
/// `app_dockerfile` is `app.dockerfile` from `stacker.yml`: when it is set, the
776+
/// generator never writes `.stacker/Dockerfile` (the user's own file is the
777+
/// build input), so the `dockerfile:` rewrite must be skipped for it.
729778
fn normalize_generated_compose_paths(
730779
compose_path: &Path,
731780
deploy_target: DeployTarget,
781+
app_dockerfile: Option<&Path>,
732782
) -> Result<(), CliError> {
733783
let is_stacker_compose = compose_path
734784
.components()
@@ -817,23 +867,49 @@ fn normalize_generated_compose_paths(
817867
changed = true;
818868
}
819869

820-
if service_name == "app" && (current_context == "." || current_context == "./") {
821-
build_map.insert(context_key, serde_yaml::Value::String("..".to_string()));
870+
if service_name == "app" {
871+
// Build contexts are authored relative to the project root
872+
// (`app.path`, default `.`) while the compose file lives in
873+
// `.stacker/` — re-root them (`.` → `..`, `./backend` →
874+
// `../backend`).
875+
if let Some((compose_context, root_rel)) =
876+
reroot_relative_context(&current_context)
877+
{
878+
if compose_context != current_context {
879+
build_map.insert(
880+
context_key.clone(),
881+
serde_yaml::Value::String(compose_context),
882+
);
883+
changed = true;
884+
}
822885

823-
let dockerfile_needs_rewrite = match dockerfile.as_deref() {
824-
None => true,
825-
Some("Dockerfile") | Some("./Dockerfile") => true,
826-
_ => false,
827-
};
886+
// `.stacker/Dockerfile` only exists when `app.dockerfile`
887+
// is unset (the generator skips it otherwise), and Compose
888+
// resolves `dockerfile:` relative to `context`:
889+
// - no `app.dockerfile` → point the build at the file
890+
// Stacker generates;
891+
// - an explicit `app.dockerfile` → keep/restore the
892+
// configured path (it is relative to the project root
893+
// the context now points at), repairing a compose a
894+
// previous version rewrote to `.stacker/Dockerfile`.
895+
let rewritten_dockerfile = match (app_dockerfile, dockerfile.as_deref()) {
896+
(None, None | Some("Dockerfile") | Some("./Dockerfile")) => {
897+
Some(generated_dockerfile_ref(&root_rel))
898+
}
899+
(Some(configured), Some(".stacker/Dockerfile")) => {
900+
Some(configured.to_string_lossy().into_owned())
901+
}
902+
_ => None,
903+
};
828904

829-
if dockerfile_needs_rewrite {
830-
build_map.insert(
831-
dockerfile_key,
832-
serde_yaml::Value::String(".stacker/Dockerfile".to_string()),
833-
);
905+
if let Some(rewritten) = rewritten_dockerfile {
906+
build_map.insert(
907+
dockerfile_key.clone(),
908+
serde_yaml::Value::String(rewritten),
909+
);
910+
changed = true;
911+
}
834912
}
835-
836-
changed = true;
837913
}
838914
}
839915
}
@@ -3791,7 +3867,11 @@ fn run_deploy_with_credentials_manager<S: CredentialStore>(
37913867
}
37923868
}
37933869

3794-
normalize_generated_compose_paths(&compose_path, deploy_target)?;
3870+
normalize_generated_compose_paths(
3871+
&compose_path,
3872+
deploy_target,
3873+
config.app.dockerfile.as_deref(),
3874+
)?;
37953875
validate_compose_for_deploy(&compose_path)?;
37963876
reject_build_sections_for_cloud(
37973877
&compose_path,
@@ -6706,7 +6786,7 @@ services:
67066786
"#;
67076787
std::fs::write(&compose_path, compose).unwrap();
67086788

6709-
normalize_generated_compose_paths(&compose_path, DeployTarget::Local).unwrap();
6789+
normalize_generated_compose_paths(&compose_path, DeployTarget::Local, None).unwrap();
67106790

67116791
let normalized = std::fs::read_to_string(&compose_path).unwrap();
67126792
assert!(!normalized.contains("version:"));
@@ -6729,7 +6809,7 @@ services:
67296809
"#;
67306810
std::fs::write(&compose_path, compose).unwrap();
67316811

6732-
normalize_generated_compose_paths(&compose_path, DeployTarget::Local).unwrap();
6812+
normalize_generated_compose_paths(&compose_path, DeployTarget::Local, None).unwrap();
67336813

67346814
let normalized = std::fs::read_to_string(&compose_path).unwrap();
67356815
assert!(normalized.contains("context: .."));
@@ -6773,7 +6853,7 @@ services:
67736853
"#;
67746854
std::fs::write(&compose_path, compose).unwrap();
67756855

6776-
normalize_generated_compose_paths(&compose_path, DeployTarget::Local).unwrap();
6856+
normalize_generated_compose_paths(&compose_path, DeployTarget::Local, None).unwrap();
67776857

67786858
assert_eq!(
67796859
normalized_volumes(&compose_path, "app"),
@@ -6805,7 +6885,7 @@ services:
68056885
"#;
68066886
std::fs::write(&compose_path, compose).unwrap();
68076887

6808-
normalize_generated_compose_paths(&compose_path, DeployTarget::Local).unwrap();
6888+
normalize_generated_compose_paths(&compose_path, DeployTarget::Local, None).unwrap();
68096889

68106890
assert_eq!(
68116891
normalized_volumes(&compose_path, "nginx"),
@@ -6834,7 +6914,7 @@ services:
68346914
"#;
68356915
std::fs::write(&compose_path, compose).unwrap();
68366916

6837-
normalize_generated_compose_paths(&compose_path, DeployTarget::Local).unwrap();
6917+
normalize_generated_compose_paths(&compose_path, DeployTarget::Local, None).unwrap();
68386918

68396919
let doc: serde_yaml::Value =
68406920
serde_yaml::from_str(&std::fs::read_to_string(&compose_path).unwrap()).unwrap();
@@ -6864,7 +6944,7 @@ services:
68646944
"#;
68656945
std::fs::write(&compose_path, compose).unwrap();
68666946

6867-
normalize_generated_compose_paths(&compose_path, DeployTarget::Local).unwrap();
6947+
normalize_generated_compose_paths(&compose_path, DeployTarget::Local, None).unwrap();
68686948

68696949
let doc: serde_yaml::Value =
68706950
serde_yaml::from_str(&std::fs::read_to_string(&compose_path).unwrap()).unwrap();
@@ -6890,7 +6970,7 @@ services:
68906970
"#;
68916971
std::fs::write(&compose_path, compose).unwrap();
68926972

6893-
normalize_generated_compose_paths(&compose_path, target).unwrap();
6973+
normalize_generated_compose_paths(&compose_path, target, None).unwrap();
68946974

68956975
assert_eq!(
68966976
normalized_volumes(&compose_path, "app"),
@@ -6916,9 +6996,9 @@ services:
69166996
"#;
69176997
std::fs::write(&compose_path, compose).unwrap();
69186998

6919-
normalize_generated_compose_paths(&compose_path, DeployTarget::Local).unwrap();
6999+
normalize_generated_compose_paths(&compose_path, DeployTarget::Local, None).unwrap();
69207000
let first = std::fs::read_to_string(&compose_path).unwrap();
6921-
normalize_generated_compose_paths(&compose_path, DeployTarget::Local).unwrap();
7001+
normalize_generated_compose_paths(&compose_path, DeployTarget::Local, None).unwrap();
69227002
let second = std::fs::read_to_string(&compose_path).unwrap();
69237003

69247004
assert_eq!(first, second);
@@ -6928,6 +7008,156 @@ services:
69287008
);
69297009
}
69307010

7011+
/// Read `services.<name>.build` back out of a normalized compose file.
7012+
fn normalized_build(compose_path: &Path, service: &str) -> (String, Option<String>) {
7013+
let doc: serde_yaml::Value =
7014+
serde_yaml::from_str(&std::fs::read_to_string(compose_path).unwrap()).unwrap();
7015+
let build = &doc["services"][service]["build"];
7016+
let context = build["context"].as_str().unwrap_or_default().to_string();
7017+
let dockerfile = build["dockerfile"].as_str().map(str::to_string);
7018+
(context, dockerfile)
7019+
}
7020+
7021+
#[test]
7022+
fn test_normalize_local_keeps_explicit_app_dockerfile() {
7023+
// `app.dockerfile: Dockerfile` used to be rewritten to
7024+
// `.stacker/Dockerfile`, which the generator never writes when
7025+
// `app.dockerfile` is set — the local build then failed with
7026+
// `open .stacker/Dockerfile: no such file or directory`.
7027+
let dir = TempDir::new().unwrap();
7028+
let stacker_dir = dir.path().join(".stacker");
7029+
std::fs::create_dir_all(&stacker_dir).unwrap();
7030+
std::fs::write(dir.path().join("Dockerfile"), "FROM nginx:alpine\n").unwrap();
7031+
7032+
let compose_path = stacker_dir.join("docker-compose.yml");
7033+
let compose = r#"
7034+
services:
7035+
app:
7036+
build:
7037+
context: .
7038+
dockerfile: Dockerfile
7039+
"#;
7040+
std::fs::write(&compose_path, compose).unwrap();
7041+
7042+
normalize_generated_compose_paths(
7043+
&compose_path,
7044+
DeployTarget::Local,
7045+
Some(Path::new("Dockerfile")),
7046+
)
7047+
.unwrap();
7048+
7049+
assert_eq!(
7050+
normalized_build(&compose_path, "app"),
7051+
("..".to_string(), Some("Dockerfile".to_string()))
7052+
);
7053+
}
7054+
7055+
#[test]
7056+
fn test_normalize_local_repairs_stacker_dockerfile_pointing_at_missing_file() {
7057+
// A compose normalized by an earlier version keeps pointing at
7058+
// `.stacker/Dockerfile` even after `app.dockerfile` is set — the
7059+
// rewrite heals it back to the configured path.
7060+
let dir = TempDir::new().unwrap();
7061+
let stacker_dir = dir.path().join(".stacker");
7062+
std::fs::create_dir_all(&stacker_dir).unwrap();
7063+
std::fs::write(dir.path().join("Dockerfile"), "FROM nginx:alpine\n").unwrap();
7064+
7065+
let compose_path = stacker_dir.join("docker-compose.yml");
7066+
let compose = r#"
7067+
services:
7068+
app:
7069+
build:
7070+
context: ..
7071+
dockerfile: .stacker/Dockerfile
7072+
"#;
7073+
std::fs::write(&compose_path, compose).unwrap();
7074+
7075+
normalize_generated_compose_paths(
7076+
&compose_path,
7077+
DeployTarget::Local,
7078+
Some(Path::new("Dockerfile")),
7079+
)
7080+
.unwrap();
7081+
7082+
assert_eq!(
7083+
normalized_build(&compose_path, "app"),
7084+
("..".to_string(), Some("Dockerfile".to_string()))
7085+
);
7086+
}
7087+
7088+
#[test]
7089+
fn test_normalize_local_points_build_at_generated_stacker_dockerfile() {
7090+
let dir = TempDir::new().unwrap();
7091+
let stacker_dir = dir.path().join(".stacker");
7092+
std::fs::create_dir_all(&stacker_dir).unwrap();
7093+
7094+
let compose_path = stacker_dir.join("docker-compose.yml");
7095+
7096+
// Root context (the default `app.path: .`).
7097+
std::fs::write(
7098+
&compose_path,
7099+
"services:\n app:\n build:\n context: .\n",
7100+
)
7101+
.unwrap();
7102+
normalize_generated_compose_paths(&compose_path, DeployTarget::Local, None).unwrap();
7103+
assert_eq!(
7104+
normalized_build(&compose_path, "app"),
7105+
("..".to_string(), Some(".stacker/Dockerfile".to_string()))
7106+
);
7107+
7108+
// Nested `app.path`: Compose resolves `dockerfile:` against the
7109+
// context, so the generated file needs one `../` per path segment.
7110+
std::fs::write(
7111+
&compose_path,
7112+
"services:\n app:\n build:\n context: ./backend\n",
7113+
)
7114+
.unwrap();
7115+
normalize_generated_compose_paths(&compose_path, DeployTarget::Local, None).unwrap();
7116+
assert_eq!(
7117+
normalized_build(&compose_path, "app"),
7118+
(
7119+
"../backend".to_string(),
7120+
Some("../.stacker/Dockerfile".to_string())
7121+
)
7122+
);
7123+
7124+
// Already normalized by a previous run: only the missing dockerfile
7125+
// reference is completed, the context stays as-is.
7126+
std::fs::write(
7127+
&compose_path,
7128+
"services:\n app:\n build:\n context: ..\n",
7129+
)
7130+
.unwrap();
7131+
normalize_generated_compose_paths(&compose_path, DeployTarget::Local, None).unwrap();
7132+
assert_eq!(
7133+
normalized_build(&compose_path, "app"),
7134+
("..".to_string(), Some(".stacker/Dockerfile".to_string()))
7135+
);
7136+
}
7137+
7138+
#[test]
7139+
fn test_normalize_leaves_absolute_build_context_alone() {
7140+
let dir = TempDir::new().unwrap();
7141+
let stacker_dir = dir.path().join(".stacker");
7142+
std::fs::create_dir_all(&stacker_dir).unwrap();
7143+
7144+
let compose_path = stacker_dir.join("docker-compose.yml");
7145+
let compose = r#"
7146+
services:
7147+
app:
7148+
build:
7149+
context: /srv/app
7150+
"#;
7151+
std::fs::write(&compose_path, compose).unwrap();
7152+
7153+
normalize_generated_compose_paths(&compose_path, DeployTarget::Local, None).unwrap();
7154+
7155+
assert_eq!(
7156+
normalized_build(&compose_path, "app"),
7157+
("/srv/app".to_string(), None)
7158+
);
7159+
}
7160+
69317161
#[test]
69327162
fn test_validate_compose_for_deploy_allows_unique_published_ports() {
69337163
let dir = TempDir::new().unwrap();

0 commit comments

Comments
 (0)