From b414c054004ffb76b83eb0416bd0dd5a4bc75a17 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sat, 6 Jun 2026 19:41:55 +0100 Subject: [PATCH] Clean up --- .forgejo/workflows/docker-publish.yml | 78 +++++++ .github/workflows/docker-publish.yml | 126 ----------- Cargo.lock | 19 +- Cargo.toml | 2 +- Dockerfile | 19 +- config.example.yaml | 2 +- docker-compose.yml | 29 +-- src/compose/parser.rs | 211 ++++++++++++------- src/compose/updater.rs | 287 ++++++++++++++++---------- src/config.rs | 53 ++--- src/health.rs | 112 +++++----- src/registry.rs | 151 ++++++++------ src/scheduler.rs | 126 ++++++----- src/strategy.rs | 59 ++---- src/version.rs | 9 +- tests/integration_tests.rs | 205 ++++++++---------- tests/test_config_validation.rs | 195 ++++++----------- tests/test_error_handling.rs | 207 ++++++++----------- 18 files changed, 905 insertions(+), 985 deletions(-) create mode 100644 .forgejo/workflows/docker-publish.yml delete mode 100644 .github/workflows/docker-publish.yml diff --git a/.forgejo/workflows/docker-publish.yml b/.forgejo/workflows/docker-publish.yml new file mode 100644 index 0000000..fcc7abc --- /dev/null +++ b/.forgejo/workflows/docker-publish.yml @@ -0,0 +1,78 @@ +name: Build and Publish Docker Image + +on: + push: + branches: [main] + tags: ["v*"] + pull_request: + branches: [main] + +env: + REGISTRY: ${{ forgejo.server_url }} + IMAGE_NAME: ${{ forgejo.repository }} + +jobs: + test: + runs-on: docker + container: + image: rust:1.88 + steps: + - name: Checkout repository + uses: https://code.forgejo.org/actions/checkout@v4 + + - name: Install components + run: rustup component add rustfmt clippy + + - name: Cache cargo registry + uses: https://code.forgejo.org/actions/cache@v4 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + target + key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + + - name: Run tests + run: cargo test --verbose + + - name: Run clippy + run: cargo clippy -- -D warnings + + - name: Check formatting + run: cargo fmt -- --check + + build-and-push: + needs: test + runs-on: docker + steps: + - name: Checkout repository + uses: https://code.forgejo.org/actions/checkout@v4 + + - name: Build and publish image + uses: http://forgejo:3000/andras/ci-actions/docker-publish@main + with: + context: . + push: ${{ github.event_name != 'pull_request' }} + token: ${{ secrets.FORGEJO_TOKEN }} + + security-scan: + needs: build-and-push + runs-on: docker + if: github.event_name != 'pull_request' + steps: + - name: Resolve registry host + id: registry + run: | + host="$(echo '${{ github.server_url }}' | sed -E 's|https?://||; s|/$||')" + [ "$host" = "forgejo:3000" ] && host="127.0.0.1:13000" + echo "image=${host}/$(echo '${{ github.repository }}' | tr '[:upper:]' '[:lower:]')" >> "$GITHUB_OUTPUT" + + - name: Run Trivy vulnerability scanner + # Pinned to the commit behind trivy-action v0.36.0 for a reproducible, + # supply-chain-safe reference instead of the moving @master branch. + uses: https://github.com/aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 + with: + image-ref: ${{ steps.registry.outputs.image }}:latest + format: "table" + exit-code: "1" + severity: "CRITICAL,HIGH" diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml deleted file mode 100644 index d9fb338..0000000 --- a/.github/workflows/docker-publish.yml +++ /dev/null @@ -1,126 +0,0 @@ -name: Build and Publish Docker Image - -on: - push: - branches: [main] - tags: ["v*"] - pull_request: - branches: [main] - -env: - REGISTRY: ghcr.io - IMAGE_NAME: ${{ github.repository }} - -jobs: - test: - runs-on: ubuntu-latest - steps: - - name: Checkout repository - uses: actions/checkout@v4 - - - name: Setup Rust toolchain - uses: actions-rs/toolchain@v1 - with: - toolchain: 1.88 - override: true - components: rustfmt, clippy - - - name: Cache cargo registry - uses: actions/cache@v3 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} - - - name: Run tests - run: GITHUB_TOKEN="${{ secrets.GITHUB_TOKEN }}"cargo test --verbose - - - name: Run clippy - run: cargo clippy -- -D warnings - - - name: Check formatting - run: cargo fmt -- --check - - build-and-push: - needs: test - runs-on: ubuntu-latest - permissions: - contents: read - packages: write - outputs: - version: ${{ steps.meta.outputs.version }} - tags: ${{ steps.meta.outputs.tags }} - - steps: - - name: Checkout repository - uses: actions/checkout@v4 - - - name: Setup Docker Buildx - uses: docker/setup-buildx-action@v3 - - - name: Log in to Container Registry - if: github.event_name != 'pull_request' - uses: docker/login-action@v3 - with: - registry: ${{ env.REGISTRY }} - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Extract metadata - id: meta - uses: docker/metadata-action@v5 - with: - images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} - tags: | - type=ref,event=branch - type=ref,event=pr - type=semver,pattern={{version}} - type=semver,pattern={{major}}.{{minor}} - type=semver,pattern={{major}} - type=sha - - - name: Build and push Docker image - uses: docker/build-push-action@v5 - with: - context: . - platforms: linux/amd64,linux/arm64 - push: ${{ github.event_name != 'pull_request' }} - tags: ${{ steps.meta.outputs.tags }} - labels: ${{ steps.meta.outputs.labels }} - cache-from: type=gha - cache-to: type=gha,mode=max - - - name: Test Docker image - if: github.event_name != 'pull_request' - run: | - docker run --rm \ - -e RUST_LOG=info \ - ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ steps.meta.outputs.version }} \ - docker-compose-updater check - - security-scan: - needs: build-and-push - runs-on: ubuntu-latest - if: github.event_name != 'pull_request' - permissions: - contents: read - packages: read - security-events: write - - steps: - - name: Checkout repository - uses: actions/checkout@v4 - - - name: Run Trivy vulnerability scanner - uses: aquasecurity/trivy-action@master - with: - image-ref: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:${{ needs.build-and-push.outputs.version }} - format: "sarif" - output: "trivy-results.sarif" - - - name: Upload Trivy scan results to GitHub Security tab - uses: github/codeql-action/upload-sarif@v2 - with: - sarif_file: "trivy-results.sarif" diff --git a/Cargo.lock b/Cargo.lock index 9a23f46..3760599 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -97,17 +97,6 @@ version = "1.0.98" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487" -[[package]] -name = "async-trait" -version = "0.1.88" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e539d3fca749fcee5236ab05e93a52867dd549cc157c8cb7f99595f3cedffdb5" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "atomic-waker" version = "1.1.2" @@ -277,7 +266,6 @@ name = "docker-compose-updater" version = "0.1.0" dependencies = [ "anyhow", - "async-trait", "chrono", "clap", "cron", @@ -291,6 +279,7 @@ dependencies = [ "tokio", "tracing", "tracing-subscriber", + "urlencoding", ] [[package]] @@ -1583,6 +1572,12 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "urlencoding" +version = "2.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "daf8dba3b7eb870caf1ddeed7bc9d2a049f3cfdfae7cb521b087cc33ae4c49da" + [[package]] name = "utf8_iter" version = "1.0.4" diff --git a/Cargo.toml b/Cargo.toml index 772148f..67daeaf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ anyhow = "1.0" regex = "1.10" cron = "0.12" clap = { version = "4.0", features = ["derive"] } -async-trait = "0.1" +urlencoding = "2" tracing = "0.1" tracing-subscriber = "0.3" chrono = "0.4" diff --git a/Dockerfile b/Dockerfile index 9738f3e..b1299ea 100644 --- a/Dockerfile +++ b/Dockerfile @@ -7,39 +7,24 @@ WORKDIR /app COPY Cargo.toml Cargo.lock ./ COPY src ./src -# Build the application RUN cargo build --release -# Runtime stage -FROM alpine:latest +FROM alpine:3.21 -# Install runtime dependencies -RUN apk add --no-cache \ - ca-certificates \ - tzdata \ - curl +RUN apk add --no-cache ca-certificates tzdata curl -# Create non-root user RUN addgroup -g 1000 updater && \ adduser -u 1000 -G updater -s /bin/sh -D updater -# Create necessary directories RUN mkdir -p /app/config && \ chown -R updater:updater /app -# Copy binary from builder stage COPY --from=builder /app/target/release/docker-compose-updater /usr/local/bin/docker-compose-updater -# Switch to non-root user USER updater - -# Set working directory WORKDIR /app - -# Expose health check port EXPOSE 8080 -# Health check HEALTHCHECK --interval=30s --timeout=5s --start-period=5s --retries=3 \ CMD curl -f http://localhost:8080/health || exit 1 diff --git a/config.example.yaml b/config.example.yaml index 04f67b6..06a93c3 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -33,7 +33,7 @@ registries: # auth_token: "${CUSTOM_REGISTRY_TOKEN}" # Update strategy -# Options: LatestPatchOfPreviousMinor (default), LatestPatchOfPreviousMinor, Latest +# Options: LatestPatchOfPreviousMinor (default), Latest update_strategy: "LatestPatchOfPreviousMinor" # Images to ignore (substring matching) diff --git a/docker-compose.yml b/docker-compose.yml index 4f807e6..4a27260 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,44 +1,27 @@ -version: '3.8' - services: docker-compose-updater: image: ghcr.io/your-username/docker-compose-updater:latest container_name: docker-compose-updater restart: unless-stopped - + environment: - GITHUB_TOKEN=${GITHUB_TOKEN} - GITLAB_TOKEN=${GITLAB_TOKEN} - - RUST_LOG=info - TZ=UTC - + volumes: - # Mount Docker socket for container management - - /var/run/docker.sock:/var/run/docker.sock:ro - # Mount directory containing your compose files - ./compose-files:/app/compose-files:rw - + # Mount configuration - ./config.yaml:/app/config/config.yaml:ro - - # Optional: Mount additional compose files - - ./docker-compose.yml:/app/docker-compose.yml:rw - + ports: - "8080:8080" - - networks: - - updater-network - + healthcheck: test: ["CMD", "curl", "-f", "http://localhost:8080/health"] interval: 30s timeout: 5s retries: 3 - start_period: 10s - - -networks: - updater-network: - driver: bridge \ No newline at end of file + start_period: 10s \ No newline at end of file diff --git a/src/compose/parser.rs b/src/compose/parser.rs index 6163058..e693382 100644 --- a/src/compose/parser.rs +++ b/src/compose/parser.rs @@ -2,8 +2,13 @@ use crate::registry::ImageRef; use anyhow::Result; use regex::Regex; use std::fs; +use std::sync::LazyLock; use tracing::{info, warn}; +static IMAGE_REGEX: LazyLock = LazyLock::new(|| { + Regex::new(r#"^\s*image:\s*(?:["']([^"']+)["']|([^\s#]+))\s*(#.*)?$"#).unwrap() +}); + #[derive(Debug, Clone)] pub struct ServiceImage { pub service_name: String, @@ -27,7 +32,6 @@ impl ComposeParser { pub fn parse_file(&self, file_path: &str) -> Result { let content = fs::read_to_string(file_path)?; let services = self.extract_services(&content)?; - Ok(ComposeFile { content, services }) } @@ -36,41 +40,77 @@ impl ComposeParser { let lines: Vec<&str> = content.lines().collect(); let mut in_services = false; let mut current_service: Option = None; - let image_regex = Regex::new(r#"^\s*image:\s*(?:["']([^"']+)["']|([^\s#]+))\s*(#.*)?$"#)?; + let mut service_indent: Option = None; + let mut service_child_indent: Option = None; for (line_number, line) in lines.iter().enumerate() { - if line.trim_start().starts_with("services:") { - in_services = true; + let trimmed = line.trim_start(); + + if trimmed.is_empty() || trimmed.starts_with('#') { continue; } - if in_services - && line.chars().next().is_some_and(|c| c.is_alphabetic()) - && !line.starts_with(" ") - { + let indent = line.len() - trimmed.len(); + + if indent == 0 && trimmed.starts_with("services:") { + in_services = true; + current_service = None; + service_indent = None; + service_child_indent = None; + continue; + } + + if in_services && indent == 0 { in_services = false; current_service = None; + service_indent = None; + service_child_indent = None; continue; } - if in_services { - if let Some(service_name) = self.extract_service_name(line) { - current_service = Some(service_name); - continue; - } + if !in_services { + continue; + } - if let Some(ref service_name) = current_service { - if let Some(image_ref) = self.extract_image_from_line(line, &image_regex)? { - info!( - "Found service '{}' with image '{}'", - service_name, image_ref - ); - services.push(ServiceImage { - service_name: service_name.clone(), - image_ref, - original_line: line.to_string(), - line_number, - }); + if (service_indent.is_none() || indent == service_indent.unwrap()) + && trimmed.ends_with(':') + && !trimmed.contains(' ') + && trimmed + .trim_end_matches(':') + .chars() + .all(|c| c.is_alphanumeric() || c == '_' || c == '-' || c == '.') + { + let name = trimmed.trim_end_matches(':'); + if !name.is_empty() { + service_indent = Some(indent); + service_child_indent = None; + current_service = Some(name.to_string()); + } + continue; + } + + if let Some(ref service_name) = current_service { + if let Some(si) = service_indent { + if indent > si { + // The first key under a service fixes the direct-child + // indent; only keys at that exact level are the service's + // own properties. This avoids picking up an `image:` nested + // under `build:` or other deeper mappings. + let child_indent = *service_child_indent.get_or_insert(indent); + if indent == child_indent { + if let Some(image_ref) = self.extract_image_from_line(line)? { + info!( + "Found service '{}' with image '{}'", + service_name, image_ref + ); + services.push(ServiceImage { + service_name: service_name.clone(), + image_ref, + original_line: line.to_string(), + line_number, + }); + } + } } } } @@ -79,62 +119,37 @@ impl ComposeParser { Ok(services) } - fn extract_service_name(&self, line: &str) -> Option { + fn extract_image_from_line(&self, line: &str) -> Result> { let trimmed = line.trim_start(); - let indent_level = line.len() - trimmed.len(); - - if indent_level > 0 && indent_level <= 8 && trimmed.ends_with(':') && !trimmed.contains(' ') - { - let potential_service = trimmed.trim_end_matches(':'); - if !potential_service.is_empty() - && potential_service - .chars() - .all(|c| c.is_alphanumeric() || c == '_' || c == '-') - { - return Some(potential_service.to_string()); - } + if !trimmed.starts_with("image:") { + return Ok(None); } - None - } + let Some(captures) = IMAGE_REGEX.captures(line) else { + return Ok(None); + }; - fn extract_image_from_line(&self, line: &str, image_regex: &Regex) -> Result> { - let trimmed = line.trim_start(); - let indent_level = line.len() - trimmed.len(); + let image_str = captures + .get(1) + .or_else(|| captures.get(2)) + .unwrap() + .as_str(); - if indent_level > 2 && trimmed.starts_with("image:") { - if let Some(captures) = image_regex.captures(line) { - let image_str = captures - .get(1) - .or_else(|| captures.get(2)) - .unwrap() - .as_str(); + if image_str.contains('@') { + info!("Skipping digest-pinned image: {}", image_str); + return Ok(None); + } - if image_str.find('@').is_some() { - info!("Found digest in image string, skipping: {}", image_str); - return Ok(None); - } + if image_str.trim().is_empty() { + return Ok(None); + } - if image_str.trim().is_empty() - || image_str.trim() == "\"\"" - || image_str.trim() == "''" - { - info!("Empty image string found in line: {}, skipping", line); - return Ok(None); - } - - match ImageRef::parse(image_str) { - Ok(image_ref) => Ok(Some(image_ref)), - Err(e) => { - warn!("Failed to parse image '{}': {}", image_str, e); - Ok(None) - } - } - } else { + match ImageRef::parse(image_str) { + Ok(image_ref) => Ok(Some(image_ref)), + Err(e) => { + warn!("Failed to parse image '{}': {}", image_str, e); Ok(None) } - } else { - Ok(None) } } } @@ -160,12 +175,12 @@ services: image: nginx:1.21.0 # Web server ports: - "80:80" - + db: image: postgres:13.7 environment: POSTGRES_DB: myapp - + redis: image: redis:6.2-alpine "#; @@ -183,4 +198,52 @@ services: assert_eq!(result.services[0].image_ref.name, "nginx"); assert_eq!(result.services[0].image_ref.tag, "1.21.0"); } + + #[test] + fn test_service_names_with_dots() { + let compose_content = r#" +services: + my.service: + image: nginx:1.21.0 + another-service: + image: redis:7.0 +"#; + + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(compose_content.as_bytes()).unwrap(); + + let parser = ComposeParser::new(); + let result = parser + .parse_file(temp_file.path().to_str().unwrap()) + .unwrap(); + + assert_eq!(result.services.len(), 2); + assert_eq!(result.services[0].service_name, "my.service"); + assert_eq!(result.services[1].service_name, "another-service"); + } + + #[test] + fn test_ignores_image_nested_under_build() { + let compose_content = r#" +services: + web: + build: + context: . + image: should-not-match:9.9.9 + image: nginx:1.21.0 +"#; + + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(compose_content.as_bytes()).unwrap(); + + let parser = ComposeParser::new(); + let result = parser + .parse_file(temp_file.path().to_str().unwrap()) + .unwrap(); + + assert_eq!(result.services.len(), 1); + assert_eq!(result.services[0].service_name, "web"); + assert_eq!(result.services[0].image_ref.name, "nginx"); + assert_eq!(result.services[0].image_ref.tag, "1.21.0"); + } } diff --git a/src/compose/updater.rs b/src/compose/updater.rs index 6b098a3..dec4b05 100644 --- a/src/compose/updater.rs +++ b/src/compose/updater.rs @@ -1,38 +1,49 @@ use super::parser::{ComposeFile, ComposeParser, ServiceImage}; -use crate::config::Config; -use crate::registry::Client as RegistryClient; +use crate::config::{Config, UpdateStrategy}; +use crate::registry::{Client as RegistryClient, ImageRef}; use crate::strategy::create_selector; -use crate::version::parse_version_tag; +use crate::version::{parse_version_tag, VersionInfo}; use anyhow::{anyhow, Result}; use regex::Regex; +use semver::Version; use std::collections::HashSet; use std::fs; use std::path::{Path, PathBuf}; +use std::sync::LazyLock; use tracing::{debug, info, warn}; +static IMAGE_LINE_REGEX: LazyLock = LazyLock::new(|| { + Regex::new(r#"^(\s*image:\s*)(?:["']([^"']+)["']|([^\s#]+))(\s*(?:#.*)?)$"#).unwrap() +}); + pub struct ComposeUpdater { config: Config, registry_client: RegistryClient, parser: ComposeParser, } +pub struct UpdateReport { + pub files_found: usize, + pub updated_files: Vec, +} + impl ComposeUpdater { pub fn new(config: Config) -> Self { let registry_client = RegistryClient::new(config.clone()); - let parser = ComposeParser::new(); - Self { config, registry_client, - parser, + parser: ComposeParser::new(), } } - pub async fn update_all_compose_files(&self) -> Result> { + pub async fn update_all_compose_files(&self) -> Result { let mut updated_files = Vec::new(); + let mut files_found = 0; for compose_path in &self.config.compose_paths { let compose_files = self.find_compose_files(compose_path)?; + files_found += compose_files.len(); for file_path in compose_files { let updated = self.update_compose_file(&file_path).await?; @@ -42,7 +53,10 @@ impl ComposeUpdater { } } - Ok(updated_files) + Ok(UpdateReport { + files_found, + updated_files, + }) } pub fn parse_compose_file(&self, file_path: &str) -> Result { @@ -58,7 +72,7 @@ impl ComposeUpdater { for service in &compose_file.services { if self.config.is_image_ignored(&service.image_ref.to_string()) { - info!("Skipping ignored image: {}", service.image_ref.to_string()); + debug!("Skipping ignored image: {}", service.image_ref); continue; } @@ -69,16 +83,13 @@ impl ComposeUpdater { updated = true; info!( "Updated {}: {} -> {}", - service.service_name, - service.image_ref.to_string(), - new_image + service.service_name, service.image_ref, new_image ); } Ok(None) => { debug!( "No update needed for {}: {}", - service.service_name, - service.image_ref.to_string() + service.service_name, service.image_ref ); } Err(e) => { @@ -97,57 +108,45 @@ impl ComposeUpdater { } fn write_updated_content(&self, file_path: &str, content: String) -> Result<()> { - if !self.config.dry_run { + if self.config.dry_run { + info!("Dry run: would update {}", file_path); + } else { fs::write(file_path, content)?; info!("Updated compose file: {}", file_path); - } else { - info!("Dry run: Would update compose file: {}", file_path); } Ok(()) } async fn update_service_image(&self, service: &ServiceImage) -> Result> { - info!( - "Checking for updates for service: {} (current image: {})", - service.service_name, - service.image_ref.to_string() - ); let (current_version, current_prefix, current_suffix, _) = parse_version_tag(&service.image_ref.tag); + let Some(current_version) = current_version else { + debug!( + "Skipping non-semver tag '{}' for service {}", + service.image_ref.tag, service.service_name + ); + return Ok(None); + }; + let available_versions = self .registry_client .get_available_versions(&service.image_ref) .await?; if available_versions.is_empty() { - warn!("No versions available for selection"); + warn!("No versions available for {}", service.image_ref); return Ok(None); } - let selector = create_selector(&self.config.update_strategy); - if let Some(target_version_info) = - selector.select_target_version(&available_versions, current_prefix, current_suffix) - { - // Only update if the target version is different AND higher than the current version - // This prevents downgrades - if Some(target_version_info.version.clone()) != current_version { - if let Some(ref current_ver) = current_version { - if target_version_info.version < *current_ver { - info!( - "Skipping downgrade from {} to {}", - current_ver, target_version_info.version - ); - return Ok(None); - } - } - let mut new_image_ref = service.image_ref.clone(); - new_image_ref.tag = target_version_info.to_string(); - return Ok(Some(new_image_ref.to_string())); - } - } - - Ok(None) + Ok(choose_new_tag( + &service.image_ref, + ¤t_version, + &available_versions, + current_prefix, + current_suffix, + &self.config.update_strategy, + )) } pub fn replace_image_in_content( @@ -156,49 +155,44 @@ impl ComposeUpdater { service: &ServiceImage, new_image: &str, ) -> Result { - let image_regex = - Regex::new(r#"^(\s*image:\s*)(?:["']([^"']+)["']|([^\s#]+))(\s*(?:#.*)?)$"#)?; - - if let Some(captures) = image_regex.captures(&service.original_line) { - let prefix = captures.get(1).unwrap().as_str(); - let _old_image = captures - .get(2) - .or_else(|| captures.get(3)) - .unwrap() - .as_str(); - let suffix = captures.get(4).unwrap().as_str(); - - let image_part = if captures.get(2).is_some() { - format!("\"{new_image}\"") - } else { - new_image.to_string() - }; - - let new_line = format!("{prefix}{image_part}{suffix}"); - - let lines: Vec<&str> = content.lines().collect(); - if service.line_number < lines.len() - && lines[service.line_number] == service.original_line - { - let mut result_lines = lines; - result_lines[service.line_number] = &new_line; - let mut result = result_lines.join("\n"); - - if content.ends_with('\n') { - result.push('\n'); - } - - Ok(result) - } else { - Err(anyhow!( - "Line number mismatch or content changed for service: {}", - service.service_name - )) - } - } else { - Err(anyhow!( + let Some(captures) = IMAGE_LINE_REGEX.captures(&service.original_line) else { + return Err(anyhow!( "Could not parse image line: {}", service.original_line + )); + }; + + let prefix = captures.get(1).unwrap().as_str(); + let suffix = captures.get(4).unwrap().as_str(); + let was_quoted = captures.get(2).is_some(); + + let image_part = if was_quoted { + format!("\"{new_image}\"") + } else { + new_image.to_string() + }; + + let new_line = format!("{prefix}{image_part}{suffix}"); + + let lines: Vec<&str> = content.lines().collect(); + if service.line_number < lines.len() && lines[service.line_number] == service.original_line + { + let mut result_lines = lines; + result_lines[service.line_number] = &new_line; + let mut result = result_lines.join("\n"); + + if content.ends_with('\n') { + result.push('\n'); + } + + Ok(result) + } else { + Err(anyhow!( + "Line mismatch for service '{}': line {} expected '{}', got '{}'", + service.service_name, + service.line_number, + service.original_line, + lines.get(service.line_number).unwrap_or(&"") )) } } @@ -256,38 +250,111 @@ impl ComposeUpdater { } } +/// Chooses the new image string for `image_ref`, or `None` when no suitable +/// upgrade exists. The selected version must share the current tag's prefix and +/// suffix and be strictly higher than `current_version`, which prevents both +/// downgrades and no-op rewrites to the same version. +fn choose_new_tag( + image_ref: &ImageRef, + current_version: &Version, + available_versions: &[VersionInfo], + current_prefix: Option, + current_suffix: Option, + strategy: &UpdateStrategy, +) -> Option { + let selector = create_selector(strategy); + let target = + selector.select_target_version(available_versions, current_prefix, current_suffix)?; + + if target.version <= *current_version { + return None; + } + + let mut new_image_ref = image_ref.clone(); + new_image_ref.tag = target.to_string(); + Some(new_image_ref.to_string()) +} + #[cfg(test)] mod tests { use super::*; - use crate::config::{Config, UpdateStrategy}; - use std::io::Write; - use tempfile::NamedTempFile; - #[tokio::test] - async fn test_prevents_downgrade() { - let mut config = Config::default(); - config.update_strategy = UpdateStrategy::Latest; - config.dry_run = true; + fn version_infos(tags: &[&str]) -> Vec { + tags.iter() + .map(|t| VersionInfo::from_tag(t).unwrap()) + .collect() + } - let updater = ComposeUpdater::new(config); + #[test] + fn test_choose_new_tag_upgrades_to_higher_version() { + let image = ImageRef::parse("nginx:1.25.0").unwrap(); + let current = Version::parse("1.25.0").unwrap(); + let available = version_infos(&["1.25.0", "1.26.0", "1.24.0"]); - // Create a temporary compose file with a higher version - let mut temp_file = NamedTempFile::new().unwrap(); - writeln!(temp_file, "services:\n web:\n image: nginx:1.25.0").unwrap(); + let result = choose_new_tag( + &image, + ¤t, + &available, + None, + None, + &UpdateStrategy::Latest, + ); + assert_eq!(result, Some("nginx:1.26.0".to_string())); + } - let file_path = temp_file.path().to_str().unwrap(); - let compose_file = updater.parse_compose_file(file_path).unwrap(); + #[test] + fn test_choose_new_tag_prevents_downgrade() { + let image = ImageRef::parse("nginx:1.25.0").unwrap(); + let current = Version::parse("1.25.0").unwrap(); + // Registry only offers older versions. + let available = version_infos(&["1.24.0", "1.23.5"]); - assert_eq!(compose_file.services.len(), 1); - let service = &compose_file.services[0]; + let result = choose_new_tag( + &image, + ¤t, + &available, + None, + None, + &UpdateStrategy::Latest, + ); + assert!(result.is_none(), "should never downgrade"); + } - // Mock a scenario where the strategy selects a lower version - // This would happen if available versions only include older versions - let (current_version, _, _, _) = parse_version_tag(&service.image_ref.tag); - assert!(current_version.is_some()); + #[test] + fn test_choose_new_tag_skips_equal_version() { + let image = ImageRef::parse("nginx:1.25.0").unwrap(); + let current = Version::parse("1.25.0").unwrap(); + let available = version_infos(&["1.25.0"]); - // The actual test would need mocked registry responses, but we can verify - // the logic by checking that current version is properly extracted - assert_eq!(current_version.unwrap().to_string(), "1.25.0"); + let result = choose_new_tag( + &image, + ¤t, + &available, + None, + None, + &UpdateStrategy::Latest, + ); + assert!(result.is_none(), "no rewrite when already on the latest"); + } + + #[test] + fn test_choose_new_tag_respects_prefix_and_suffix() { + let image = ImageRef::parse("nginx:v1.25.0-alpine").unwrap(); + let current = Version::parse("1.25.0").unwrap(); + let available = vec![ + VersionInfo::from_tag("v1.26.0-alpine").unwrap(), + VersionInfo::from_tag("1.27.0").unwrap(), // no matching prefix/suffix + VersionInfo::from_tag("v1.26.0").unwrap(), // missing suffix + ]; + + let result = choose_new_tag( + &image, + ¤t, + &available, + Some("v".to_string()), + Some("-alpine".to_string()), + &UpdateStrategy::Latest, + ); + assert_eq!(result, Some("nginx:v1.26.0-alpine".to_string())); } } diff --git a/src/config.rs b/src/config.rs index c8db59b..5aced8d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,22 +1,22 @@ use anyhow::Result; +use regex::Regex; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::path::PathBuf; -use tracing::info; +use std::sync::LazyLock; +use tracing::{info, warn}; + +static ENV_VAR_REGEX: LazyLock = + LazyLock::new(|| Regex::new(r"\$\{([^}]+)\}|\$([A-Za-z_][A-Za-z0-9_]*)").unwrap()); #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(default)] pub struct Config { - #[serde(default)] pub compose_paths: Vec, - #[serde(default)] pub schedule: String, - #[serde(default)] pub registries: HashMap, - #[serde(default)] pub update_strategy: UpdateStrategy, - #[serde(default)] pub ignore_images: Vec, - #[serde(default)] pub dry_run: bool, } @@ -53,7 +53,7 @@ impl Default for Config { Self { compose_paths: vec![PathBuf::from(".")], - schedule: "0 0 2 * * *".to_string(), // Daily at 2 AM + schedule: "0 0 2 * * *".to_string(), registries, update_strategy: UpdateStrategy::LatestPatchOfPreviousMinor, ignore_images: vec![], @@ -68,31 +68,32 @@ impl Config { let content = std::fs::read_to_string(config_path)?; let expanded_content = Self::expand_env_vars(&content); let mut config: Self = serde_yaml::from_str(&expanded_content)?; - config.resolve_env_tokens(); + config.normalize_auth_tokens(); Ok(config) } - /// Expand environment variable placeholders like ${VAR} in the config content pub fn expand_env_vars(content: &str) -> String { - let env_var_pattern = regex::Regex::new(r"\$\{([^}]+)\}").unwrap(); - - env_var_pattern + ENV_VAR_REGEX .replace_all(content, |caps: ®ex::Captures| { - let var_name = &caps[1]; - std::env::var(var_name).unwrap_or_else(|_| format!("${{{var_name}}}")) + let var_name = caps.get(1).or_else(|| caps.get(2)).unwrap().as_str(); + std::env::var(var_name).unwrap_or_else(|_| { + warn!( + "Environment variable `{}` referenced in config is not set; substituting an empty string", + var_name + ); + String::new() + }) }) - .to_string() + .into_owned() } - /// Resolve environment variable tokens after deserialization - fn resolve_env_tokens(&mut self) { - for registry_config in self.registries.values_mut() { - if let Some(token) = ®istry_config.auth_token { - if token.starts_with("$") { - // Handle direct env var references like "$GITHUB_TOKEN" - let env_var_name = token.trim_start_matches('$'); - registry_config.auth_token = std::env::var(env_var_name).ok(); - } + /// Treat a registry `auth_token` that expanded to an empty string (e.g. an + /// unset `${TOKEN}`) as no token at all, so we fall back to unauthenticated + /// access with a clear error instead of attempting auth with an empty token. + fn normalize_auth_tokens(&mut self) { + for registry in self.registries.values_mut() { + if registry.auth_token.as_deref() == Some("") { + registry.auth_token = None; } } } @@ -100,6 +101,6 @@ impl Config { pub fn is_image_ignored(&self, image: &str) -> bool { self.ignore_images .iter() - .any(|ignored| image.contains(ignored)) + .any(|pattern| image.contains(pattern)) } } diff --git a/src/health.rs b/src/health.rs index 1e66651..7de9891 100644 --- a/src/health.rs +++ b/src/health.rs @@ -14,22 +14,18 @@ pub struct HealthHandle { last_update_success: Arc>, } -impl Default for HealthServer { - fn default() -> Self { - Self::new().0 - } -} - impl HealthServer { pub fn new() -> (Self, HealthHandle) { let last_update_success = Arc::new(Mutex::new(true)); - let server = Self { + let handle = HealthHandle { last_update_success: last_update_success.clone(), }; - let handle = HealthHandle { - last_update_success, - }; - (server, handle) + ( + Self { + last_update_success, + }, + handle, + ) } pub async fn start(self) -> anyhow::Result<()> { @@ -37,64 +33,56 @@ impl HealthServer { info!("Health server listening on port 8080"); loop { - let (mut socket, _) = listener.accept().await?; + let (socket, _) = match listener.accept().await { + Ok(conn) => conn, + Err(e) => { + warn!("Failed to accept connection: {}", e); + continue; + } + }; let health_status = self.last_update_success.clone(); tokio::spawn(async move { - // Set timeout for the entire request handling (5 seconds) - let handle_request = async { - let mut buffer = [0; 1024]; - - // Read with timeout to prevent hanging connections - match timeout(Duration::from_secs(5), socket.read(&mut buffer)).await { - Ok(Ok(_)) => { - // Successfully read request (we don't need to parse it for health check) - } - Ok(Err(_)) | Err(_) => { - warn!("Health check request read timeout or error"); - return; - } - } - - // Safely access health status without panicking - let is_healthy = match health_status.lock() { - Ok(status) => *status, - Err(_) => { - warn!("Health status mutex poisoned, defaulting to unhealthy"); - false - } - }; - - let (status_line, json_body) = if is_healthy { - ("HTTP/1.1 200 OK", "{\"status\":\"healthy\"}") - } else { - ( - "HTTP/1.1 503 Service Unavailable", - "{\"status\":\"unhealthy\"}", - ) - }; - - let response = format!( - "{}\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", - status_line, - json_body.len(), - json_body - ); - - let _ = timeout( - Duration::from_secs(5), - socket.write_all(response.as_bytes()), - ) - .await; - }; - - // Overall timeout for the entire connection handling - let _ = timeout(Duration::from_secs(10), handle_request).await; + let _ = timeout(Duration::from_secs(10), async { + handle_connection(socket, health_status).await; + }) + .await; }); } } } +async fn handle_connection(mut socket: tokio::net::TcpStream, health_status: Arc>) { + let mut buffer = [0; 1024]; + + match timeout(Duration::from_secs(5), socket.read(&mut buffer)).await { + Ok(Ok(0)) | Ok(Err(_)) | Err(_) => return, + Ok(Ok(_)) => {} + } + + let is_healthy = health_status.lock().map(|s| *s).unwrap_or(false); + + let (status_line, json_body) = if is_healthy { + ("HTTP/1.1 200 OK", r#"{"status":"healthy"}"#) + } else { + ( + "HTTP/1.1 503 Service Unavailable", + r#"{"status":"unhealthy"}"#, + ) + }; + + let response = format!( + "{status_line}\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{json_body}", + json_body.len(), + ); + + let _ = timeout( + Duration::from_secs(5), + socket.write_all(response.as_bytes()), + ) + .await; +} + impl HealthHandle { pub fn set_health_status(&self, is_healthy: bool) { if let Ok(mut status) = self.last_update_success.lock() { @@ -103,12 +91,10 @@ impl HealthHandle { } pub fn report_update_success(&self) { - info!("Update succeeded - marking health as healthy"); self.set_health_status(true); } pub fn report_update_failure(&self) { - info!("Update failed - marking health as unhealthy"); self.set_health_status(false); } } diff --git a/src/registry.rs b/src/registry.rs index dfa9f67..24e9586 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -4,13 +4,18 @@ use anyhow::{anyhow, Result}; use chrono::{DateTime, Utc}; use reqwest::{Client as HttpClient, Response, StatusCode}; use serde::Deserialize; +use std::sync::LazyLock; use std::time::Duration; use tokio::time::sleep; use tracing::{debug, warn}; const PAGE_SIZE: usize = 500; +const MAX_PAGES: usize = 100; const MAX_RETRY_ATTEMPTS: u32 = 5; +const MAX_AUTH_ATTEMPTS: u32 = 2; const INITIAL_RETRY_DELAY_SECS: u64 = 1; +const HTTP_TIMEOUT_SECS: u64 = 30; +const HTTP_CONNECT_TIMEOUT_SECS: u64 = 10; #[derive(Debug, Clone)] pub struct ImageRef { @@ -49,7 +54,6 @@ impl ImageRef { } } _ => { - // 3 or more parts: first is registry, last is name, everything in between is namespace let registry = registry_parts[0].to_string(); let name = registry_parts[registry_parts.len() - 1].to_string(); let namespace = Some(registry_parts[1..registry_parts.len() - 1].join("/")); @@ -68,23 +72,20 @@ impl ImageRef { impl std::fmt::Display for ImageRef { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let base = match &self.namespace { - Some(ns) => format!("{}/{}/{}", self.registry, ns, self.name), - None => format!("{}/{}", self.registry, self.name), - }; - if self.registry == "docker.io" { match &self.namespace { Some(ns) => write!(f, "{}/{}:{}", ns, self.name, self.tag), None => write!(f, "{}:{}", self.name, self.tag), } } else { - write!(f, "{}:{}", base, self.tag) + match &self.namespace { + Some(ns) => write!(f, "{}/{}/{}:{}", self.registry, ns, self.name, self.tag), + None => write!(f, "{}/{}:{}", self.registry, self.name, self.tag), + } } } } -/// Docker Hub API response for tag list #[derive(Deserialize)] struct DockerHubTagsResponse { results: Vec, @@ -96,8 +97,6 @@ struct DockerHubTag { name: String, } -/// Unified registry client with hybrid approach -/// Uses Docker Hub's public API for Docker Hub, standard v2 API for others pub struct Client { http_client: HttpClient, config: Config, @@ -105,21 +104,23 @@ pub struct Client { impl Client { pub fn new(config: Config) -> Self { + let http_client = HttpClient::builder() + .timeout(Duration::from_secs(HTTP_TIMEOUT_SECS)) + .connect_timeout(Duration::from_secs(HTTP_CONNECT_TIMEOUT_SECS)) + .build() + .expect("failed to build HTTP client"); + Self { - http_client: HttpClient::new(), + http_client, config, } } - /// Parse Retry-After header to determine wait duration - /// Supports both delay-seconds (integer) and HTTP-date formats fn parse_retry_after(&self, retry_after: &str) -> Option { - // Try parsing as seconds first if let Ok(seconds) = retry_after.parse::() { return Some(Duration::from_secs(seconds)); } - // Try parsing as HTTP date (RFC 2822 or RFC 3339) if let Ok(date) = DateTime::parse_from_rfc2822(retry_after) { let now = Utc::now(); let wait_time = date.signed_duration_since(now); @@ -131,7 +132,6 @@ impl Client { None } - /// Perform HTTP request with retry logic for rate limiting (429) async fn request_with_retry( &self, request_fn: F, @@ -158,7 +158,6 @@ impl Client { )); } - // Check for Retry-After header let wait_duration = if let Some(retry_after) = response .headers() .get("retry-after") @@ -176,7 +175,6 @@ impl Client { sleep(wait_duration).await; - // Exponential backoff for next attempt if no Retry-After header delay = delay.saturating_mul(2); } else { return Ok(response); @@ -194,15 +192,10 @@ impl Client { fn build_repository_path(&self, image_ref: &ImageRef) -> String { match &image_ref.namespace { Some(namespace) => format!("{}/{}", namespace, image_ref.name), - None => { - if image_ref.registry == "docker.io" { - // For Docker Hub official images, use 'library' namespace in public API - format!("library/{}", image_ref.name) - } else { - // For other registries, no namespace means just the image name - image_ref.name.clone() - } + None if image_ref.registry == "docker.io" => { + format!("library/{}", image_ref.name) } + None => image_ref.name.clone(), } } @@ -255,7 +248,7 @@ impl Client { async fn get_dockerhub_versions(&self, image_ref: &ImageRef) -> Result> { let repo_path = self.build_repository_path(image_ref); let mut results = Vec::new(); - // Use Docker Hub's public REST API which doesn't require authentication for public repos + let mut page_count: usize = 0; let mut url = format!( "https://hub.docker.com/v2/repositories/{repo_path}/tags/?page_size={PAGE_SIZE}" ); @@ -286,6 +279,15 @@ impl Client { results.extend(new_tags); + page_count += 1; + if page_count >= MAX_PAGES { + warn!( + "Reached maximum page count ({}) for {}, stopping pagination", + MAX_PAGES, image_ref + ); + break; + } + if let Some(next) = next_page { url = next; } else { @@ -304,12 +306,16 @@ impl Client { let mut last_tag: Option = None; let mut next_url: Option = None; let mut bearer_token: Option = None; + let mut auth_attempts: u32 = 0; + let mut page_count: usize = 0; - // Reference: https://github.com/opencontainers/distribution-spec/blob/main/spec.md#endpoints loop { - // Use next URL from Link header if available, otherwise build URL with last parameter let url = if let Some(next) = next_url.take() { - format!("{}{next}", registry_config.url) + if next.starts_with("http://") || next.starts_with("https://") { + next + } else { + format!("{}{next}", registry_config.url) + } } else { format!( "{}/v2/{repo_path}/tags/list?n={PAGE_SIZE}{}", @@ -324,7 +330,6 @@ impl Client { debug!("Registry API URL: {}", url); - // Try unauthenticated request first to potentially get auth challenge let url_clone = url.clone(); let bearer_token_clone = bearer_token.clone(); let response = self @@ -341,13 +346,21 @@ impl Client { .await?; let (new_tags, link_next) = if response.status() == reqwest::StatusCode::UNAUTHORIZED { + if auth_attempts >= MAX_AUTH_ATTEMPTS { + return Err(anyhow!( + "Authentication failed after {} attempts for registry {}", + auth_attempts, + image_ref.registry + )); + } if let Some(token) = ®istry_config.auth_token { - // Try Docker Registry v2 auth flow if we get auth challenge if let Some(auth_header) = response.headers().get("www-authenticate") { - bearer_token = self - .try_registry_v2_auth(auth_header.to_str().unwrap(), token) - .await?; - continue; // Retry with new token + let auth_str = auth_header.to_str().map_err(|e| { + anyhow!("Invalid WWW-Authenticate header encoding: {}", e) + })?; + bearer_token = self.try_registry_v2_auth(auth_str, token).await?; + auth_attempts += 1; + continue; } else { return Err(anyhow!( "Unauthorized request but no WWW-Authenticate header found" @@ -357,7 +370,6 @@ impl Client { return Err(anyhow!("Unauthorized request but no auth token configured")); } } else if response.status().is_success() { - // Check for Link header for pagination let link_next = response .headers() .get("link") @@ -377,14 +389,26 @@ impl Client { )); }; + page_count += 1; + if page_count >= MAX_PAGES { + warn!( + "Reached maximum page count ({}) for {}, stopping pagination", + MAX_PAGES, image_ref + ); + results.extend(new_tags); + break; + } + let maybe_last_tag = new_tags.last().map(|v| v.original.clone()); results.extend(new_tags); - // Use Link header next URL if available, otherwise fall back to last parameter pagination if let Some(next) = link_next { next_url = Some(next); - } else if let Some(last) = maybe_last_tag { - last_tag = Some(last); + } else if let Some(ref last) = maybe_last_tag { + if last_tag.as_ref() == Some(last) { + break; + } + last_tag = maybe_last_tag; } else { break; } @@ -394,13 +418,15 @@ impl Client { } async fn try_registry_v2_auth(&self, auth_str: &str, token: &str) -> Result> { - // Parse WWW-Authenticate header to extract auth parameters - let realm = self.extract_auth_param(auth_str, "realm")?; - let service = self.extract_auth_param(auth_str, "service")?; - let scope = self.extract_auth_param(auth_str, "scope")?; + let realm = extract_auth_param(auth_str, "realm")?; + let service = extract_auth_param(auth_str, "service")?; + let scope = extract_auth_param(auth_str, "scope")?; - // Request token from auth endpoint - let auth_url = format!("{realm}?service={service}&scope={scope}"); + let auth_url = format!( + "{realm}?service={}&scope={}", + urlencoding::encode(&service), + urlencoding::encode(&scope), + ); debug!("Getting registry token from: {}", auth_url); let auth_url_clone = auth_url.clone(); @@ -432,29 +458,16 @@ impl Client { return Ok(Some(registry_token.to_string())); } - Err(anyhow!("Auth flow didn't work, caller can try fallback")) - } - - fn extract_auth_param(&self, auth_str: &str, param: &str) -> Result { - let pattern = format!(r#"{param}="([^"]+)""#); - let re = regex::Regex::new(&pattern)?; - - re.captures(auth_str) - .and_then(|caps| caps.get(1)) - .map(|m| m.as_str().to_string()) - .ok_or_else(|| anyhow!("Missing {} in auth challenge", param)) + Err(anyhow!("Auth response missing 'token' field")) } fn parse_link_header(&self, link_header: &str) -> Option { - // Parse RFC 5988 Link header to find "next" relation - // Format: ; rel="next", ; rel="last" for link in link_header.split(',') { let parts: Vec<&str> = link.trim().split(';').collect(); if parts.len() >= 2 { let url = parts[0].trim(); if url.starts_with('<') && url.ends_with('>') { - let url = &url[1..url.len() - 1]; // Remove < and > - + let url = &url[1..url.len() - 1]; for param in &parts[1..] { let param = param.trim(); if param == "rel=\"next\"" || param == "rel=next" { @@ -468,6 +481,22 @@ impl Client { } } +static AUTH_PARAM_REGEX: LazyLock = + LazyLock::new(|| regex::Regex::new(r#"(\w+)="([^"]+)""#).unwrap()); + +fn extract_auth_param(auth_str: &str, param: &str) -> Result { + for caps in AUTH_PARAM_REGEX.captures_iter(auth_str) { + if caps.get(1).map(|m| m.as_str()) == Some(param) { + return Ok(caps[2].to_string()); + } + } + Err(anyhow!( + "Missing '{}' in auth challenge: {}", + param, + auth_str + )) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/scheduler.rs b/src/scheduler.rs index 8da6e27..8dabc51 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -5,8 +5,7 @@ use cron::Schedule; use std::str::FromStr; use std::time::Duration; use tokio::time::{sleep, Instant}; -use tracing::error; -use tracing::info; +use tracing::{error, info}; pub struct Scheduler { config: Config, @@ -38,24 +37,26 @@ impl Scheduler { loop { if let Err(err) = self.run_update().await.context("Failed to run update") { - error!("{:?}", err) + error!("{:#}", err); } - if let Some(next_run) = self.schedule.upcoming(chrono::Utc).take(1).next() { - let now = chrono::Utc::now(); - let duration_until_next = next_run.signed_duration_since(now); + let sleep_duration = + if let Some(next_run) = self.schedule.upcoming(chrono::Utc).take(1).next() { + let now = chrono::Utc::now(); + let duration_until_next = next_run.signed_duration_since(now); + let millis = duration_until_next.num_milliseconds().max(0) as u64; + let duration = Duration::from_millis(millis).max(Duration::from_secs(1)); - if duration_until_next.num_seconds() > 0 { info!( "Next update scheduled for: {}", next_run.format("%Y-%m-%d %H:%M:%S UTC") ); + duration + } else { + Duration::from_secs(60) + }; - let sleep_duration = - Duration::from_secs(duration_until_next.num_seconds() as u64); - sleep(sleep_duration).await; - } - } + sleep(sleep_duration).await; } } @@ -66,48 +67,55 @@ impl Scheduler { async fn run_update(&self) -> Result<()> { let start_time = Instant::now(); - info!("Starting Docker Compose update cycle"); + info!("Starting update cycle"); match self.updater.update_all_compose_files().await { - Ok(updated_files) => { + Ok(report) => { let duration = start_time.elapsed(); - if updated_files.is_empty() { + + if report.files_found == 0 { + if let Some(health) = &self.health_handle { + health.report_update_failure(); + } + return Err(anyhow!( + "No compose files found under configured compose_paths: {:?}", + self.config.compose_paths + )); + } + + if report.updated_files.is_empty() { info!( - "Update cycle completed in {:?} - no files updated", - duration + "Update cycle completed in {:?} - scanned {} files, none updated", + duration, report.files_found ); } else { + let verb = if self.config.dry_run { + "would update" + } else { + "updated" + }; info!( - "Update cycle completed in {:?} - {} {} files:", + "Update cycle completed in {:?} - scanned {} files, {} {} files:", duration, - if self.config.dry_run { - "would update" - } else { - "updated" - }, - updated_files.len() + report.files_found, + verb, + report.updated_files.len() ); - for file in &updated_files { + for file in &report.updated_files { info!(" - {}", file); } } - // Report success to health monitor - if let Some(ref health) = self.health_handle { + if let Some(health) = &self.health_handle { health.report_update_success(); } - Ok(()) } Err(e) => { - let error = e.context("Failed to update Docker Compose files"); - - // Report failure to health monitor - if let Some(ref health) = self.health_handle { + if let Some(health) = &self.health_handle { health.report_update_failure(); } - - Err(error) + Err(e.context("Failed to update Docker Compose files")) } } } @@ -120,18 +128,20 @@ mod tests { use std::collections::HashMap; use std::path::PathBuf; + fn test_config(schedule: &str) -> Config { + Config { + compose_paths: vec![PathBuf::from("./test")], + schedule: schedule.to_string(), + registries: HashMap::new(), + update_strategy: UpdateStrategy::LatestPatchOfPreviousMinor, + ignore_images: vec![], + dry_run: true, + } + } + #[test] fn test_scheduler_creation() { - let config = Config { - compose_paths: vec![PathBuf::from("./test")], - schedule: "0 0 2 * * *".to_string(), - registries: HashMap::new(), - update_strategy: UpdateStrategy::LatestPatchOfPreviousMinor, - ignore_images: vec![], - dry_run: true, - }; - - let scheduler = Scheduler::new(config, None).unwrap(); + let scheduler = Scheduler::new(test_config("0 0 2 * * *"), None).unwrap(); assert!(scheduler .schedule .upcoming(chrono::Utc) @@ -141,19 +151,8 @@ mod tests { } #[test] - fn test_cron_parsing() { - let config = Config { - compose_paths: vec![PathBuf::from("./test")], - schedule: "0 30 1 * * *".to_string(), // 1:30 AM daily - registries: HashMap::new(), - update_strategy: UpdateStrategy::LatestPatchOfPreviousMinor, - ignore_images: vec![], - dry_run: true, - }; - - let scheduler = Scheduler::new(config, None).unwrap(); - - // Just verify the scheduler can be created + fn test_scheduler_with_different_cron() { + let scheduler = Scheduler::new(test_config("0 30 1 * * *"), None).unwrap(); assert!(scheduler .schedule .upcoming(chrono::Utc) @@ -161,4 +160,17 @@ mod tests { .next() .is_some()); } + + #[tokio::test] + async fn test_run_once_errors_when_no_compose_files_found() { + let mut config = test_config("0 0 2 * * *"); + config.compose_paths = vec![]; + let scheduler = Scheduler::new(config, None).unwrap(); + + let result = scheduler.run_once().await; + assert!( + result.is_err(), + "an update cycle that finds no compose files should report an error" + ); + } } diff --git a/src/strategy.rs b/src/strategy.rs index 878c812..f45251b 100644 --- a/src/strategy.rs +++ b/src/strategy.rs @@ -1,6 +1,6 @@ use crate::config::UpdateStrategy; use crate::version::VersionInfo; -use tracing::{info, warn}; +use tracing::{debug, warn}; pub fn create_selector(strategy: &UpdateStrategy) -> Box { match strategy { @@ -27,17 +27,15 @@ impl VersionSelector for LatestVersionSelector { current_prefix: Option, current_suffix: Option, ) -> Option { - info!("Using update strategy: LatestVersionSelector"); - let versions = - get_filtered_and_soreted_matching_versions(available, current_prefix, current_suffix); + get_filtered_and_sorted_matching_versions(available, current_prefix, current_suffix); let latest = versions.first().cloned(); if let Some(ref selected) = latest { - info!("Selected {} as latest version", selected.version); + debug!("Latest version selected: {}", selected.version); } else { - warn!("No versions available for selection"); + warn!("No matching versions available"); } latest @@ -53,25 +51,19 @@ impl VersionSelector for SmartPreviousMinorSelector { current_prefix: Option, current_suffix: Option, ) -> Option { - info!("Using update strategy: SmartPreviousMinorSelector"); - let versions = - get_filtered_and_soreted_matching_versions(available, current_prefix, current_suffix); + get_filtered_and_sorted_matching_versions(available, current_prefix, current_suffix); let latest = &versions.first()?.version; - info!("Latest version available: {}", latest); + debug!("Latest version available: {}", latest); let (target_major, max_minor) = if latest.minor == 0 { - // If current minor is 0, look for the latest minor of the previous major if latest.major == 0 { - info!("Cannot go to previous version of 0.0.x, skipping update"); + debug!("Cannot go to previous version of 0.0.x"); return None; } - - info!("The latest version has a minor version of 0, looking for the latest minor of the previous major"); (latest.major - 1, None) } else { - info!("The latest version has a minor version of {}, looking for the latest patch of the previous minor", latest.minor); (latest.major, Some(latest.minor - 1)) }; @@ -83,46 +75,35 @@ impl VersionSelector for SmartPreviousMinorSelector { .cloned(); if let Some(ref selected) = selected { - info!("Selected {} as latest version", selected.version); + debug!("Selected version: {}", selected.version); } else { - warn!("No versions available for selection"); + warn!("No matching versions available for previous minor strategy"); } selected } } -fn get_filtered_and_soreted_matching_versions( +fn get_filtered_and_sorted_matching_versions( available: &[VersionInfo], current_prefix: Option, current_suffix: Option, ) -> Vec { - let mut sorted_versions = available.to_vec(); - sorted_versions.sort_by(|a, b| b.version.cmp(&a.version)); - - info!( - "Available versions for selection: {}", - sorted_versions - .iter() - .map(|v| v.to_string()) - .collect::>() - .join(", ") - ); - let filtered_versions: Vec = sorted_versions - .into_iter() + let mut versions: Vec = available + .iter() .filter(|v| v.prefix == current_prefix && v.suffix == current_suffix) + .cloned() .collect(); + versions.sort_by(|a, b| b.version.cmp(&a.version)); - info!( - "Filtered versions with matching prefix and suffix: {}", - filtered_versions - .iter() - .map(|v| v.to_string()) - .collect::>() - .join(", ") + debug!( + "Filtered versions (prefix={:?}, suffix={:?}): {}", + current_prefix, + current_suffix, + versions.len() ); - filtered_versions + versions } #[cfg(test)] diff --git a/src/version.rs b/src/version.rs index b00fa9d..8a40d07 100644 --- a/src/version.rs +++ b/src/version.rs @@ -1,6 +1,11 @@ use regex::Regex; use semver::Version; use std::fmt; +use std::sync::LazyLock; + +static VERSION_REGEX: LazyLock = LazyLock::new(|| { + Regex::new(r"^(?P.*?)(?P(?:\d+\.)+\d+)(?P.*?)$").unwrap() +}); #[derive(Debug, Clone, PartialEq)] pub struct VersionInfo { @@ -24,9 +29,7 @@ impl VersionInfo { /// assert_eq!(version_info.suffix, Some("-alpine-slim".to_string())); /// ``` pub fn from_tag(tag: &str) -> Option { - let re = Regex::new(r"^(?P.*?)(?P(?:\d+\.)+\d+)(?P.*?)$").unwrap(); - - if let Some(captures) = re.captures(tag) { + if let Some(captures) = VERSION_REGEX.captures(tag) { let prefix_part = captures.name("prefix").map_or("", |m| m.as_str()); let version_part = captures.name("version").map_or("", |m| m.as_str()); let suffix_part = captures.name("suffix").map_or("", |m| m.as_str()); diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 1324680..8f598ff 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -3,11 +3,11 @@ use docker_compose_updater::config::{Config, RegistryConfig, UpdateStrategy}; use docker_compose_updater::registry::{Client, ImageRef}; use std::collections::HashMap; use std::io::Write; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use tempfile::NamedTempFile; #[tokio::test] -async fn test_end_to_end_compose_file_parsing_and_updating() { +async fn test_compose_file_parsing() { let compose_content = r#" version: '3.8' services: @@ -15,12 +15,12 @@ services: image: nginx:1.21.0 # Web server ports: - "80:80" - + db: image: postgres:13.7 environment: POSTGRES_DB: myapp - + redis: image: redis:6.2.1-alpine ports: @@ -30,9 +30,7 @@ services: let mut temp_file = NamedTempFile::new().unwrap(); temp_file.write_all(compose_content.as_bytes()).unwrap(); - let config = create_test_config(); - let updater = ComposeUpdater::new(config); - + let updater = ComposeUpdater::new(create_test_config()); let result = updater .parse_compose_file(temp_file.path().to_str().unwrap()) .unwrap(); @@ -54,7 +52,7 @@ services: } #[tokio::test] -async fn test_comment_and_formatting_preservation_during_updates() { +async fn test_comment_and_formatting_preservation() { let compose_content = r#" version: '3.8' services: @@ -62,7 +60,7 @@ services: image: nginx:1.21.0 # This is a comment ports: - "80:80" - + # This is another comment db: image: "postgres:13.7" # Database comment with quotes @@ -73,9 +71,7 @@ services: let mut temp_file = NamedTempFile::new().unwrap(); temp_file.write_all(compose_content.as_bytes()).unwrap(); - let config = create_test_config(); - let updater = ComposeUpdater::new(config); - + let updater = ComposeUpdater::new(create_test_config()); let result = updater .parse_compose_file(temp_file.path().to_str().unwrap()) .unwrap(); @@ -87,34 +83,34 @@ services: .original_line .contains("# Database comment")); + // Replace unquoted image -> stays unquoted, preserves comment let new_content = updater .replace_image_in_content(&result.content, &result.services[0], "nginx:1.20.0") .unwrap(); - assert!(new_content.contains("# This is a comment")); - assert!(new_content.contains("nginx:1.20.0")); + assert!(new_content.contains("nginx:1.20.0 # This is a comment")); assert!(!new_content.contains("nginx:1.21.0")); + // Replace quoted image -> stays quoted, preserves comment let new_content = updater .replace_image_in_content(&new_content, &result.services[1], "postgres:14.0") .unwrap(); - assert!(new_content.contains("# Database comment")); - assert!(new_content.contains("\"postgres:14.0\"")); + assert!(new_content.contains("\"postgres:14.0\" # Database comment")); assert!(!new_content.contains("postgres:13.7")); } #[tokio::test] -async fn test_empty_compose_paths_handling() { +async fn test_empty_compose_paths() { let config = Config { compose_paths: vec![], ..create_test_config() }; let updater = ComposeUpdater::new(config); - let result = updater.update_all_compose_files().await; - assert!(result.is_ok()); - assert!(result.unwrap().is_empty()); + let result = updater.update_all_compose_files().await.unwrap(); + assert_eq!(result.files_found, 0); + assert!(result.updated_files.is_empty()); } #[test] @@ -140,6 +136,77 @@ fn test_config_defaults_and_image_filtering() { assert!(!config_with_ignores.is_image_ignored("nginx:1.21.0")); } +#[test] +fn test_env_var_substitution() { + std::env::set_var("TEST_TOKEN", "test_value_123"); + + let test_yaml = r#" +registries: + "test.registry": + url: "https://test.registry" + auth_token: "${TEST_TOKEN}" +"#; + + let expanded = Config::expand_env_vars(test_yaml); + assert!(expanded.contains("test_value_123")); + assert!(!expanded.contains("${TEST_TOKEN}")); + + std::env::remove_var("TEST_TOKEN"); +} + +#[test] +fn test_dollar_var_and_unset_var_expansion() { + std::env::set_var("TEST_DOLLAR_TOKEN", "dollar_value_456"); + let expanded = Config::expand_env_vars("token: $TEST_DOLLAR_TOKEN"); + assert!(expanded.contains("dollar_value_456")); + std::env::remove_var("TEST_DOLLAR_TOKEN"); + + let expanded = Config::expand_env_vars("token: ${DEFINITELY_UNSET_VAR}"); + assert!(!expanded.contains("DEFINITELY_UNSET_VAR")); + assert!(expanded.contains("token: ")); +} + +#[tokio::test] +#[ignore = "Only run in CI with GITHUB_TOKEN set"] +async fn test_ghcr_authentication_e2e() { + let github_token = std::env::var("GITHUB_TOKEN").expect("GITHUB_TOKEN must be set"); + + let mut registries = HashMap::new(); + registries.insert( + "docker.io".to_string(), + RegistryConfig { + url: "https://registry-1.docker.io".to_string(), + auth_token: None, + }, + ); + registries.insert( + "ghcr.io".to_string(), + RegistryConfig { + url: "https://ghcr.io".to_string(), + auth_token: Some(github_token), + }, + ); + + let config = Config { + compose_paths: vec![PathBuf::from("./test")], + schedule: "0 0 2 * * *".to_string(), + registries, + update_strategy: UpdateStrategy::LatestPatchOfPreviousMinor, + ignore_images: vec!["localhost".to_string()], + dry_run: true, + }; + + let client = Client::new(config); + let image_ref = ImageRef::parse("ghcr.io/schmelczer/vault-link:0.5.1").unwrap(); + + let versions = client.get_available_versions(&image_ref).await.unwrap(); + assert!(!versions.is_empty()); + assert!( + versions.iter().any(|v| v.version.to_string() == "0.5.1"), + "Should find version 0.5.1" + ); +} + fn create_test_config() -> Config { let mut registries = HashMap::new(); registries.insert( @@ -159,101 +226,3 @@ fn create_test_config() -> Config { dry_run: true, } } - -#[test] -fn test_env_var_substitution() { - // Test environment variable substitution in config - std::env::set_var("TEST_TOKEN", "test_value_123"); - - let test_yaml = r#" -registries: - "test.registry": - url: "https://test.registry" - auth_token: "${TEST_TOKEN}" -"#; - - let expanded = Config::expand_env_vars(test_yaml); - assert!(expanded.contains("test_value_123")); - assert!(!expanded.contains("${TEST_TOKEN}")); - - // Clean up - std::env::remove_var("TEST_TOKEN"); -} - -#[test] -fn test_config_loading_with_env_vars() { - // Set up test environment variable - std::env::set_var("GITHUB_TOKEN", "ghp_test_token_for_testing"); - - // Test that config loading works with our test config - if std::path::Path::new("config.test.yaml").exists() { - let config = Config::load(Path::new("config.test.yaml").to_path_buf()).unwrap(); - - if let Some(ghcr_config) = config.registries.get("ghcr.io") { - // Should have resolved the environment variable - assert_eq!( - ghcr_config.auth_token, - Some("ghp_test_token_for_testing".to_string()) - ); - } - } - - // Clean up - std::env::remove_var("GITHUB_TOKEN"); -} - -#[tokio::test] -#[ignore = "Only run in CI with GITHUB_TOKEN set"] -async fn test_ghcr_authentication_e2e() { - let github_token = std::env::var("GITHUB_TOKEN").unwrap(); - - // This test validates that Docker Registry v2 authentication works with GHCR - let mut registries = HashMap::new(); - registries.insert( - "docker.io".to_string(), - RegistryConfig { - url: "https://registry-1.docker.io".to_string(), - auth_token: None, - }, - ); - registries.insert( - "ghcr.io".to_string(), - RegistryConfig { - url: "https://ghcr.io".to_string(), - // Use the token we validated above - auth_token: Some(github_token), - }, - ); - - let config = Config { - compose_paths: vec![PathBuf::from("./test")], - schedule: "0 0 2 * * *".to_string(), - registries, - update_strategy: UpdateStrategy::LatestPatchOfPreviousMinor, - ignore_images: vec!["localhost".to_string()], - dry_run: true, - }; - - let client = Client::new(config); - - // Test with the actual image that was failing - let image_ref = ImageRef::parse("ghcr.io/schmelczer/vault-link:0.5.1").unwrap(); - - let result = client.get_available_versions(&image_ref).await; - - match result { - Ok(versions) => { - assert!(!versions.is_empty(), "Should return at least one version"); - - // Verify that we can find the specific version 0.5.1 - let has_target_version = versions.iter().any(|v| v.version.to_string() == "0.5.1"); - assert!( - has_target_version, - "Should find target version 0.5.1 in available versions" - ); - } - Err(e) => { - panic!("GHCR authentication test failed: {e}"); - } - } -} diff --git a/tests/test_config_validation.rs b/tests/test_config_validation.rs index 501896e..a3a8787 100644 --- a/tests/test_config_validation.rs +++ b/tests/test_config_validation.rs @@ -8,7 +8,6 @@ use tempfile::NamedTempFile; fn test_config_default_values() { let config = Config::default(); - // Verify all default values are reasonable assert_eq!(config.schedule, "0 0 2 * * *"); assert_eq!( config.update_strategy, @@ -16,10 +15,8 @@ fn test_config_default_values() { ); assert!(!config.dry_run); assert!(config.ignore_images.is_empty()); - // compose_paths should be a valid Vec (length check is always true for usize) - let _paths_count = config.compose_paths.len(); + assert!(!config.compose_paths.is_empty()); - // Should have default registries configured assert!(config.registries.contains_key("docker.io")); assert!(config.registries.contains_key("ghcr.io")); @@ -29,12 +26,10 @@ fn test_config_default_values() { let ghcr_registry = &config.registries["ghcr.io"]; assert_eq!(ghcr_registry.url, "https://ghcr.io"); - assert!(ghcr_registry.auth_token.is_none()); } #[test] -fn test_config_from_different_file_formats() { - // Test loading from a properly formatted YAML file +fn test_config_from_yaml_file() { let valid_yaml = r#" compose_paths: - "./docker-compose.yml" @@ -57,10 +52,7 @@ registries: temp_file.write_all(valid_yaml.as_bytes()).unwrap(); temp_file.flush().unwrap(); - let result = Config::load(temp_file.path().to_path_buf()); - assert!(result.is_ok(), "Should load valid YAML config"); - - let config = result.unwrap(); + let config = Config::load(temp_file.path().to_path_buf()).unwrap(); assert_eq!(config.compose_paths.len(), 2); assert_eq!(config.schedule, "0 0 3 * * *"); assert_eq!(config.update_strategy, UpdateStrategy::Latest); @@ -74,45 +66,18 @@ registries: } #[test] -fn test_config_with_invalid_yaml_syntax() { - let invalid_yamls = [ - // Invalid YAML syntax - r#" -compose_paths: - - "./docker-compose.yml" -invalid_indent: -schedule: "0 0 2 * * *" -"#, - // Invalid field types - r#" -compose_paths: "not an array" -schedule: 123 -dry_run: "not a boolean" -"#, - // Completely invalid YAML - r#" -{ invalid yaml syntax [ -"#, - ]; +fn test_config_rejects_invalid_yaml() { + let invalid_yaml = r#"{ invalid yaml syntax ["#; - for (i, invalid_yaml) in invalid_yamls.iter().enumerate() { - let mut temp_file = NamedTempFile::new().unwrap(); - temp_file.write_all(invalid_yaml.as_bytes()).unwrap(); - temp_file.flush().unwrap(); + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(invalid_yaml.as_bytes()).unwrap(); + temp_file.flush().unwrap(); - let result = Config::load(temp_file.path().to_path_buf()); - - // Should either fail gracefully or fall back to defaults - if result.is_ok() { - let config = result.unwrap(); - // If it succeeds, should have reasonable defaults - assert!( - !config.schedule.is_empty(), - "Schedule should not be empty for test case {i}" - ); - } - // If it fails, that's also acceptable behavior for invalid YAML - } + let result = Config::load(temp_file.path().to_path_buf()); + assert!( + result.is_err(), + "Completely invalid YAML should fail to load" + ); } #[test] @@ -123,10 +88,6 @@ fn test_config_update_strategy_parsing() { "LatestPatchOfPreviousMinor", UpdateStrategy::LatestPatchOfPreviousMinor, ), - ( - "LatestPatchOfPreviousMinor", - UpdateStrategy::LatestPatchOfPreviousMinor, - ), ]; for (strategy_str, expected_strategy) in strategies { @@ -142,19 +103,13 @@ update_strategy: "{strategy_str}" temp_file.write_all(yaml_config.as_bytes()).unwrap(); temp_file.flush().unwrap(); - let result = Config::load(temp_file.path().to_path_buf()); - assert!( - result.is_ok(), - "Should parse valid strategy: {strategy_str}" - ); - - let config = result.unwrap(); + let config = Config::load(temp_file.path().to_path_buf()).unwrap(); assert_eq!(config.update_strategy, expected_strategy); } } #[test] -fn test_config_with_invalid_update_strategy() { +fn test_config_rejects_invalid_update_strategy() { let yaml_config = r#" compose_paths: [] schedule: "0 0 2 * * *" @@ -166,15 +121,12 @@ update_strategy: "InvalidStrategy" temp_file.flush().unwrap(); let result = Config::load(temp_file.path().to_path_buf()); - - assert!(result.is_err(), "Should not parse invalid update strategy"); + assert!(result.is_err()); } #[test] -fn test_image_ignore_patterns_functionality() { - // Test various ignore patterns +fn test_image_ignore_patterns() { let test_cases = vec![ - // (ignore_pattern, image_to_test, should_be_ignored) ("localhost", "localhost:5000/app:latest", true), ("localhost", "nginx:latest", false), ("127.0.0.1", "127.0.0.1:5000/app:latest", true), @@ -182,6 +134,7 @@ fn test_image_ignore_patterns_functionality() { ("ghcr.io", "ghcr.io/user/app:latest", true), ("ghcr.io", "docker.io/nginx:latest", false), ("test-", "test-app:latest", true), + // "app-test:latest" does NOT contain "test-" (it has "test:" after the dash) ("test-", "app-test:latest", false), ]; @@ -191,10 +144,13 @@ fn test_image_ignore_patterns_functionality() { ..Config::default() }; - let result = config.is_image_ignored(image_to_test); assert_eq!( - result, should_be_ignored, - "Pattern '{ignore_pattern}' with image '{image_to_test}' should be ignored: {should_be_ignored}" + config.is_image_ignored(image_to_test), + should_be_ignored, + "Pattern '{}' with image '{}': expected ignored={}", + ignore_pattern, + image_to_test, + should_be_ignored ); } } @@ -211,31 +167,17 @@ fn test_multiple_ignore_patterns() { ..Config::default() }; - let test_cases = vec![ - ("localhost:5000/app:latest", true), - ("127.0.0.1:5000/app:latest", true), - ("test-app:latest", true), - ("ghcr.io/user/app:latest", true), - ("docker.io/nginx:latest", false), - ("registry.example.com/app:latest", false), - ("app-test:latest", false), - ]; - - for (image, should_be_ignored) in test_cases { - let result = config.is_image_ignored(image); - assert_eq!( - result, should_be_ignored, - "Image '{image}' should be ignored: {should_be_ignored}" - ); - } + assert!(config.is_image_ignored("localhost:5000/app:latest")); + assert!(config.is_image_ignored("127.0.0.1:5000/app:latest")); + assert!(config.is_image_ignored("test-app:latest")); + assert!(config.is_image_ignored("ghcr.io/user/app:latest")); + assert!(!config.is_image_ignored("docker.io/nginx:latest")); + assert!(!config.is_image_ignored("registry.example.com/app:latest")); } #[test] -fn test_registry_configuration_validation() { - // Test registry configs with various configurations +fn test_registry_configuration() { let mut registries = HashMap::new(); - - // Registry with just URL registries.insert( "simple.registry.com".to_string(), RegistryConfig { @@ -243,8 +185,6 @@ fn test_registry_configuration_validation() { auth_token: None, }, ); - - // Registry with auth token registries.insert( "auth.registry.com".to_string(), RegistryConfig { @@ -258,17 +198,13 @@ fn test_registry_configuration_validation() { ..Config::default() }; - // Verify registries are properly configured - assert!(config.registries.contains_key("simple.registry.com")); - assert!(config.registries.contains_key("auth.registry.com")); - - let simple_registry = &config.registries["simple.registry.com"]; - assert_eq!(simple_registry.url, "https://simple.registry.com"); - assert!(simple_registry.auth_token.is_none()); - - let auth_registry = &config.registries["auth.registry.com"]; - assert_eq!(auth_registry.url, "https://auth.registry.com"); - assert_eq!(auth_registry.auth_token, Some("secret-token".to_string())); + assert!(config.registries["simple.registry.com"] + .auth_token + .is_none()); + assert_eq!( + config.registries["auth.registry.com"].auth_token, + Some("secret-token".to_string()) + ); } #[test] @@ -284,36 +220,19 @@ registries: {} temp_file.write_all(yaml_config.as_bytes()).unwrap(); temp_file.flush().unwrap(); - let result = Config::load(temp_file.path().to_path_buf()); - - if result.is_ok() { - let config = result.unwrap(); - assert!(config.compose_paths.is_empty()); - assert!(config.ignore_images.is_empty()); - // Empty schedule should either be rejected or have a default - assert!(!config.schedule.is_empty() || config.schedule.is_empty()); // Either is acceptable - } - // If loading fails for empty schedule, that's also acceptable + let config = Config::load(temp_file.path().to_path_buf()).unwrap(); + assert!(config.compose_paths.is_empty()); + assert!(config.ignore_images.is_empty()); + assert!(config.schedule.is_empty()); + assert!(config.registries.is_empty()); } #[test] fn test_config_compose_paths_handling() { - // Test various compose path configurations let test_cases = vec![ - // Single file path vec!["./docker-compose.yml"], - // Multiple file paths vec!["./docker-compose.yml", "./docker-compose.override.yml"], - // Directory paths vec!["./services/", "./environments/"], - // Mixed paths - vec!["./docker-compose.yml", "./services/", "./override.yml"], - // Relative and absolute-looking paths - vec![ - "docker-compose.yml", - "/app/config/compose.yml", - "../compose.yml", - ], ]; for paths in test_cases { @@ -334,13 +253,33 @@ schedule: "0 0 2 * * *" temp_file.write_all(yaml_config.as_bytes()).unwrap(); temp_file.flush().unwrap(); - let result = Config::load(temp_file.path().to_path_buf()); - assert!(result.is_ok(), "Should handle paths: {paths:?}"); - - let config = result.unwrap(); + let config = Config::load(temp_file.path().to_path_buf()).unwrap(); assert_eq!(config.compose_paths.len(), paths.len()); for (i, expected_path) in paths.iter().enumerate() { assert_eq!(config.compose_paths[i], PathBuf::from(expected_path)); } } } + +#[test] +fn test_load_normalizes_empty_auth_token_to_none() { + // An auth_token referencing an unset variable expands to "" and should be + // treated as "no token" rather than an empty credential. + let yaml_config = r#" +compose_paths: [] +schedule: "0 0 2 * * *" +registries: + private.registry.com: + url: "https://private.registry.com" + auth_token: "${DCU_DEFINITELY_UNSET_TOKEN}" +"#; + + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(yaml_config.as_bytes()).unwrap(); + temp_file.flush().unwrap(); + + let config = Config::load(temp_file.path().to_path_buf()).unwrap(); + assert!(config.registries["private.registry.com"] + .auth_token + .is_none()); +} diff --git a/tests/test_error_handling.rs b/tests/test_error_handling.rs index 8f3dbca..5045000 100644 --- a/tests/test_error_handling.rs +++ b/tests/test_error_handling.rs @@ -9,153 +9,125 @@ use tempfile::{NamedTempFile, TempDir}; #[test] fn test_scheduler_invalid_cron_expressions() { - // Test that scheduler now properly rejects invalid cron expressions instead of falling back - let invalid_cron_configs = [ + let invalid_crons = [ "invalid cron", - "0 0 25 * * *", // Invalid hour - "60 0 2 * * *", // Invalid minute - "", // Empty string - "0 0 2 * *", // Missing field + "0 0 25 * * *", + "60 0 2 * * *", + "", + "0 0 2 * *", ]; - for invalid_cron in invalid_cron_configs { + for cron in invalid_crons { let config = Config { compose_paths: vec![PathBuf::from("./test")], - schedule: invalid_cron.to_string(), + schedule: cron.to_string(), registries: HashMap::new(), update_strategy: UpdateStrategy::Latest, ignore_images: vec![], dry_run: true, }; - let result = Scheduler::new(config, None); assert!( - result.is_err(), - "Expected error for invalid cron expression: '{invalid_cron}'" + Scheduler::new(config, None).is_err(), + "Expected error for invalid cron expression: '{cron}'" ); } } #[test] fn test_scheduler_valid_cron_expressions() { - // Test that scheduler accepts valid cron expressions - let valid_cron_configs = [ - "0 0 2 * * *", // Default - 2 AM daily - "0 30 1 * * *", // 1:30 AM daily - "0 0 */6 * * *", // Every 6 hours - "0 0 2 * * MON", // Mondays at 2 AM - "0 15 14 1 * *", // 1st of month at 2:15 PM + let valid_crons = [ + "0 0 2 * * *", + "0 30 1 * * *", + "0 0 */6 * * *", + "0 0 2 * * MON", + "0 15 14 1 * *", ]; - for valid_cron in valid_cron_configs { + for cron in valid_crons { let config = Config { compose_paths: vec![PathBuf::from("./test")], - schedule: valid_cron.to_string(), + schedule: cron.to_string(), registries: HashMap::new(), update_strategy: UpdateStrategy::Latest, ignore_images: vec![], dry_run: true, }; - let result = Scheduler::new(config, None); assert!( - result.is_ok(), - "Expected success for valid cron expression: '{valid_cron}'" + Scheduler::new(config, None).is_ok(), + "Expected success for valid cron expression: '{cron}'" ); } } #[test] fn test_compose_invalid_file_paths() { - let config = create_test_config(); - let updater = ComposeUpdater::new(config); + let updater = ComposeUpdater::new(create_test_config()); - // Test with completely invalid path - let result = updater.parse_compose_file("/nonexistent/path/file.yml"); - assert!(result.is_err(), "Should fail for nonexistent file"); + assert!(updater + .parse_compose_file("/nonexistent/path/file.yml") + .is_err()); - // Test with directory instead of file let temp_dir = TempDir::new().unwrap(); - let result = updater.parse_compose_file(temp_dir.path().to_str().unwrap()); - assert!(result.is_err(), "Should fail when path is a directory"); + assert!(updater + .parse_compose_file(temp_dir.path().to_str().unwrap()) + .is_err()); } #[test] fn test_compose_malformed_yaml() { - let config = create_test_config(); - let updater = ComposeUpdater::new(config); + let updater = ComposeUpdater::new(create_test_config()); - let malformed_yamls = [ - // Invalid YAML syntax - r#" -version: '3.8' -services: - web: - image: nginx:1.21.0 - invalid_indent - ports: - - "80:80" -"#, - // Missing required fields - r#" -version: '3.8' -# Missing services section -"#, - // Completely empty file - "", - // Invalid version format - r#" -version: invalid -services: - web: - image: nginx:1.21.0 -"#, - ]; + // Missing services section -> no services found + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"version: '3.8'\n").unwrap(); + let result = updater + .parse_compose_file(temp_file.path().to_str().unwrap()) + .unwrap(); + assert!(result.services.is_empty()); - for (i, malformed_yaml) in malformed_yamls.iter().enumerate() { - let mut temp_file = NamedTempFile::new().unwrap(); - temp_file.write_all(malformed_yaml.as_bytes()).unwrap(); + // Empty file -> no services found + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"").unwrap(); + let result = updater + .parse_compose_file(temp_file.path().to_str().unwrap()) + .unwrap(); + assert!(result.services.is_empty()); - let result = updater.parse_compose_file(temp_file.path().to_str().unwrap()); - - // We expect these to either fail or return empty services (graceful degradation) - if result.is_ok() { - let compose_file = result.unwrap(); - // If it parses successfully, it should at least not crash - assert!( - compose_file.services.is_empty() || !compose_file.services.is_empty(), - "Test case {i} should either fail or succeed gracefully" - ); - } - // If it fails, that's also acceptable behavior for malformed YAML - } + // Valid services section with invalid version field -> still finds services + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file + .write_all(b"version: invalid\nservices:\n web:\n image: nginx:1.21.0\n") + .unwrap(); + let result = updater + .parse_compose_file(temp_file.path().to_str().unwrap()) + .unwrap(); + assert_eq!(result.services.len(), 1); } #[test] fn test_image_ref_parsing_edge_cases() { - // Test that image parsing doesn't crash on edge cases - let long_name = "a".repeat(300); - let edge_case_refs = [ - "", // Empty string - ":", // Just colon - "image:", // Missing tag - ":tag", // Missing image - "registry.com/", // Missing image name - &long_name, // Extremely long name - ]; + // Empty string parses but produces empty name (no panic) + let result = ImageRef::parse("").unwrap(); + assert_eq!(result.name, ""); + assert_eq!(result.tag, "latest"); - for edge_case_ref in edge_case_refs { - let result = ImageRef::parse(edge_case_ref); - // The main goal is that parsing doesn't crash and returns a result - // Some might succeed (with defaults) or fail - both are acceptable - if result.is_ok() { - let _image_ref = result.unwrap(); - // If it succeeds, that's fine - the parser is robust - // We don't make strict assertions about the content since - // the parser may use defaults for edge cases - } - // If they fail, that's also perfectly acceptable behavior - } + // Missing tag defaults to "latest" + let result = ImageRef::parse("nginx").unwrap(); + assert_eq!(result.tag, "latest"); + assert_eq!(result.name, "nginx"); + + // Registry with namespace + let result = ImageRef::parse("ghcr.io/user/app:v1").unwrap(); + assert_eq!(result.registry, "ghcr.io"); + assert_eq!(result.namespace, Some("user".to_string())); + assert_eq!(result.name, "app"); + assert_eq!(result.tag, "v1"); + + // Very long name shouldn't panic + let long_name = "a".repeat(300); + let _ = ImageRef::parse(&long_name); } #[test] @@ -163,7 +135,7 @@ fn test_registry_unknown_registry() { let config = Config { compose_paths: vec![], schedule: "0 0 2 * * *".to_string(), - registries: HashMap::new(), // Empty registries + registries: HashMap::new(), update_strategy: UpdateStrategy::Latest, ignore_images: vec![], dry_run: true, @@ -171,7 +143,6 @@ fn test_registry_unknown_registry() { let registry_client = RegistryClient::new(config); - // Test with an unknown registry let image_ref = ImageRef { registry: "unknown.registry.com".to_string(), namespace: Some("user".to_string()), @@ -179,57 +150,43 @@ fn test_registry_unknown_registry() { tag: "1.0.0".to_string(), }; - // This should fail because the registry is not configured let rt = tokio::runtime::Runtime::new().unwrap(); let result = rt.block_on(registry_client.get_available_versions(&image_ref)); - assert!(result.is_err(), "Should fail for unknown registry"); + assert!(result.is_err()); let error_msg = result.unwrap_err().to_string(); assert!( error_msg.contains("Unknown registry"), - "Error should mention unknown registry, got: {error_msg}" + "Expected 'Unknown registry' in error, got: {error_msg}" ); } #[tokio::test] async fn test_compose_update_with_empty_paths() { let config = Config { - compose_paths: vec![], // Empty paths + compose_paths: vec![], ..create_test_config() }; let updater = ComposeUpdater::new(config); - // Should handle empty paths gracefully - let result = updater.update_all_compose_files().await; - assert!(result.is_ok(), "Should handle empty paths gracefully"); - assert!( - result.unwrap().is_empty(), - "Should return empty list for empty paths" - ); + let result = updater.update_all_compose_files().await.unwrap(); + assert_eq!(result.files_found, 0); + assert!(result.updated_files.is_empty()); } #[test] fn test_config_edge_cases() { - // Test config with minimal valid data let minimal_config = Config { - compose_paths: vec![], // Empty paths + compose_paths: vec![], schedule: "0 0 2 * * *".to_string(), - registries: HashMap::new(), // No registries + registries: HashMap::new(), update_strategy: UpdateStrategy::Latest, ignore_images: vec![], dry_run: true, }; - // Should be able to create scheduler with minimal config - let scheduler_result = Scheduler::new(minimal_config.clone(), None); - assert!( - scheduler_result.is_ok(), - "Should accept minimal valid config" - ); - - // Should be able to create compose updater with minimal config + assert!(Scheduler::new(minimal_config.clone(), None).is_ok()); let _updater = ComposeUpdater::new(minimal_config.clone()); - // This should not panic assert!(!minimal_config.is_image_ignored("any-image:tag")); } @@ -238,8 +195,7 @@ fn test_version_selection_with_mismatched_prefixes_suffixes() { use docker_compose_updater::strategy::create_selector; use docker_compose_updater::version::VersionInfo; - let config = create_test_config(); - let selector = create_selector(&config.update_strategy); + let selector = create_selector(&create_test_config().update_strategy); let current_prefix = Some("v".to_string()); let current_suffix = Some("-alpine".to_string()); @@ -251,10 +207,9 @@ fn test_version_selection_with_mismatched_prefixes_suffixes() { ]; let target = selector.select_target_version(&available, current_prefix, current_suffix); - assert!( target.is_none(), - "Should not find target when prefix/suffix don't match" + "No versions match both prefix 'v' and suffix '-alpine'" ); }