diff --git a/Cargo.lock b/Cargo.lock index 9e8a9bf..bdaf720 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -774,6 +774,7 @@ dependencies = [ "gitlab-runner-mock", "glob", "hmac 0.13.0", + "normalize-path", "parking_lot", "pin-project", "rand 0.10.1", @@ -1359,6 +1360,12 @@ dependencies = [ "version_check", ] +[[package]] +name = "normalize-path" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f5438dd2b2ff4c6df6e1ce22d825ed2fa93ee2922235cc45186991717f0a892d" + [[package]] name = "nu-ansi-term" version = "0.50.3" diff --git a/gitlab-runner/Cargo.toml b/gitlab-runner/Cargo.toml index 40f6403..fbf8f0a 100644 --- a/gitlab-runner/Cargo.toml +++ b/gitlab-runner/Cargo.toml @@ -36,6 +36,7 @@ hmac = "0.13.0" rand = "0.10.1" tokio-util = { version = "0.7.18", features = [ "io" ] } tokio-retry2 = { version = "0.9.1", features = ["jitter"] } +normalize-path = "0.2.1" [dev-dependencies] tokio = { version = "1.50.0", features = [ "full", "test-util" ] } diff --git a/gitlab-runner/src/run.rs b/gitlab-runner/src/run.rs index 7c60298..a9d5a61 100644 --- a/gitlab-runner/src/run.rs +++ b/gitlab-runner/src/run.rs @@ -1,4 +1,5 @@ use bytes::Bytes; +use normalize_path::NormalizePath; use std::future::Future; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -85,6 +86,48 @@ where overall_result } +fn artifact_path_matches(file_path: &Path, patterns: I) -> bool +where + I::Item: AsRef, +{ + patterns.into_iter().any(|test_path| { + let test_path = Path::new(test_path.as_ref()); + let Some(test_path) = test_path.try_normalize() else { + return false; + }; + match glob::Pattern::new(&test_path.to_string_lossy()) { + Ok(pattern) => file_path.ancestors().any(|p| { + pattern.matches_path_with( + p, + glob::MatchOptions { + require_literal_separator: true, + ..glob::MatchOptions::new() + }, + ) + }), + Err(_) => file_path.ancestors().any(|p| p == test_path), + } + }) +} + +#[test] +fn test_artifact_path_matches() { + assert!(artifact_path_matches(Path::new("abc"), &["abc"])); + assert!(artifact_path_matches(Path::new("abc"), &["a*c"])); + assert!(artifact_path_matches(Path::new("abc/d"), &["abc"])); + assert!(artifact_path_matches(Path::new("abc/d"), &["a*c"])); + assert!(!artifact_path_matches(Path::new("ab/c"), &["a*c"])); + assert!(artifact_path_matches(Path::new("abc"), &["."])); + assert!(artifact_path_matches(Path::new("abc"), &["./abc//"])); + assert!(artifact_path_matches(Path::new("a/b/c"), &["a/**/c"])); + assert!(artifact_path_matches(Path::new("a/b/d/e/f/c"), &["a/**/c"])); + assert!(artifact_path_matches( + Path::new("a/b/c.yaml"), + &["**/*.yaml"] + )); + assert!(artifact_path_matches(Path::new("["), &["["])); +} + async fn process_artifact( artifact: &JobArtifact, script_result: JobResult, @@ -124,16 +167,13 @@ where let mut uploaded = 0; for file in handler.get_uploadable_files().await? { - if artifact - .paths - .iter() - .any(|path| match glob::Pattern::new(path) { - Ok(pattern) => pattern.matches(&file.get_path()), - Err(_) => path == &file.get_path(), - }) - { - let path = file.get_path(); - match uploader.file(path.to_string()).await { + let file_path = Path::new(&file.get_path().as_ref()).normalize(); + + if artifact_path_matches(&file_path, &artifact.paths) { + match uploader + .file(file_path.to_string_lossy().into_owned()) + .await + { Ok(mut upload) => { let mut data = file.get_data().await?; match futures::io::copy(&mut data, &mut upload).await { diff --git a/gitlab-runner/tests/artifacts.rs b/gitlab-runner/tests/artifacts.rs index 83aea70..d9820a7 100644 --- a/gitlab-runner/tests/artifacts.rs +++ b/gitlab-runner/tests/artifacts.rs @@ -17,6 +17,7 @@ use gitlab_runner_mock::{ enum TestFile { Test, Test2, + TestDir, } #[async_trait::async_trait] @@ -26,13 +27,15 @@ impl UploadableFile for TestFile { fn get_path(&self) -> Cow<'_, str> { match self { TestFile::Test => "test".into(), - TestFile::Test2 => "test2".into(), + TestFile::Test2 => "./test2".into(), + TestFile::TestDir => "dir/test".into(), } } async fn get_data(&self) -> Result<&'_ [u8], ()> { Ok(match self { TestFile::Test => b"testdata".as_slice(), TestFile::Test2 => b"testdata2".as_slice(), + TestFile::TestDir => b"testdata-dir".as_slice(), }) } } @@ -48,7 +51,9 @@ impl JobHandler for Upload { async fn get_uploadable_files( &mut self, ) -> Result + Send>, ()> { - Ok(Box::new([TestFile::Test, TestFile::Test2].into_iter())) + Ok(Box::new( + [TestFile::Test, TestFile::Test2, TestFile::TestDir].into_iter(), + )) } } @@ -65,22 +70,20 @@ impl Download { .expect("No artifacts to download"); let mut names: Vec<_> = artifact.file_names().collect(); names.sort_unstable(); - assert_eq!(2, names.len()); - assert_eq!(&["test", "test2"], names.as_slice()); - - let mut f = artifact.file("test").expect("Testfile not available"); - assert_eq!(f.name(), "test"); - let mut data = Vec::new(); - f.read_to_end(&mut data).expect("Failed to read content"); - assert_eq!("testdata".as_bytes(), &*data); - drop(f); - - let mut f = artifact.file("test2").expect("Testfile not available"); - assert_eq!(f.name(), "test2"); - let mut data = Vec::new(); - f.read_to_end(&mut data).expect("Failed to read content"); - assert_eq!("testdata2".as_bytes(), &*data); - drop(f); + assert_eq!(3, names.len()); + assert_eq!(&["dir/test", "test", "test2"], names.as_slice()); + + for (name, expected) in [ + ("test", "testdata"), + ("test2", "testdata2"), + ("dir/test", "testdata-dir"), + ] { + let mut f = artifact.file(name).expect("Testfile not available"); + assert_eq!(f.name(), name); + let mut data = Vec::new(); + f.read_to_end(&mut data).expect("Failed to read content"); + assert_eq!(expected.as_bytes(), &*data); + } /* Try reading first data file a second time */ let mut f = artifact.file("test").expect("Testfile not available"); @@ -113,7 +116,11 @@ async fn upload_download() { MockJobStepWhen::OnSuccess, false, ); - upload.add_artifact_paths(vec!["*".to_string()]); + upload.add_artifact_paths(vec![ + "dir".to_string(), + "./*2".to_string(), + "test/".to_string(), + ]); let upload = upload.build(); mock.enqueue_job(upload.clone()); @@ -236,7 +243,7 @@ async fn multiple_upload() { let mut z = ZipArchive::new(std::io::Cursor::new(artifact.data.as_slice())) .expect("failed to open zip archive"); saw_zip = true; - assert_eq!(z.len(), 2); + assert_eq!(z.len(), 3); for ix in 0..z.len() { let mut f = z.by_index(ix).expect("failed to get zip file"); match f.name() { @@ -250,6 +257,11 @@ async fn multiple_upload() { f.read_to_end(&mut data).expect("failed to read zip file"); assert_eq!(b"testdata2", data.as_slice()); } + "dir/test" => { + let mut data = Vec::new(); + f.read_to_end(&mut data).expect("failed to read zip file"); + assert_eq!(b"testdata-dir", data.as_slice()); + } _ => { unreachable!("unknown file in archive"); }