From 68677a506d5b71d0c6c3defae23b9af57326cb68 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 11 Apr 2025 18:18:10 +0200 Subject: [PATCH 01/18] Add git-sync support --- crates/stackable-operator/CHANGELOG.md | 5 + .../src/git_sync/framework.rs | 779 ++++++++++++++++++ crates/stackable-operator/src/git_sync/mod.rs | 4 + .../stackable-operator/src/git_sync/spec.rs | 67 ++ crates/stackable-operator/src/lib.rs | 1 + .../src/product_config_utils.rs | 151 ++++ 6 files changed, 1007 insertions(+) create mode 100644 crates/stackable-operator/src/git_sync/framework.rs create mode 100644 crates/stackable-operator/src/git_sync/mod.rs create mode 100644 crates/stackable-operator/src/git_sync/spec.rs diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index ec0fdd5e..e145163c 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- Add git-sync support ([#1024]). + ### Changed - BREAKING: Version common CRD structs and enums ([#968]). @@ -13,6 +17,7 @@ All notable changes to this project will be documented in this file. - Import are now more granular in general. [#968]: https://github.com/stackabletech/operator-rs/pull/968 +[#1024]: https://github.com/stackabletech/operator-rs/pull/1024 ## [0.92.0] - 2025-04-14 diff --git a/crates/stackable-operator/src/git_sync/framework.rs b/crates/stackable-operator/src/git_sync/framework.rs new file mode 100644 index 00000000..c2872df5 --- /dev/null +++ b/crates/stackable-operator/src/git_sync/framework.rs @@ -0,0 +1,779 @@ +use std::{collections::BTreeMap, path::PathBuf}; + +use k8s_openapi::api::core::v1::{ + Container, EmptyDirVolumeSource, EnvVar, EnvVarSource, SecretKeySelector, Volume, VolumeMount, +}; +use snafu::{ResultExt, Snafu}; +use strum::{EnumDiscriminants, IntoStaticStr}; + +use super::spec::GitSync; +use crate::{ + builder::pod::{ + container::ContainerBuilder, resources::ResourceRequirementsBuilder, volume::VolumeBuilder, + }, + commons::product_image_selection::ResolvedProductImage, + product_config_utils::insert_or_update_env_vars, + utils::COMMON_BASH_TRAP_FUNCTIONS, +}; + +pub const CONTAINER_NAME_PREFIX: &str = "git-sync"; +pub const VOLUME_NAME_PREFIX: &str = "content-from-git"; +pub const MOUNT_PATH_PREFIX: &str = "/stackable/app/git"; +pub const GIT_SYNC_SAFE_DIR_OPTION: &str = "safe.directory"; +pub const GIT_SYNC_ROOT_DIR: &str = "/tmp/git"; +pub const GIT_SYNC_LINK: &str = "current"; + +#[derive(Snafu, Debug, EnumDiscriminants)] +#[strum_discriminants(derive(IntoStaticStr))] +pub enum Error { + #[snafu(display("invalid container name"))] + InvalidContainerName { + source: crate::builder::pod::container::Error, + }, + + #[snafu(display("failed to add needed volumeMount"))] + AddVolumeMount { + source: crate::builder::pod::container::Error, + }, +} + +/// Kubernetes resources generated from `GitSync` specifications which should be added to the Pod. +#[derive(Default)] +pub struct GitSyncResources { + /// GitSync containers with regular synchronizations + pub git_sync_containers: Vec, + /// GitSync init containers with a one-time synchronizations + pub git_sync_init_containers: Vec, + /// GitSync volumes containing the synchronized repository + pub git_content_volumes: Vec, + /// Volume mounts for the GitSync volumes + pub git_content_volume_mounts: Vec, + /// Absolute paths to the Git contents in the mounted volumes + pub git_content_folders: Vec, +} + +impl GitSyncResources { + /// Returns whether or not GitSync is enabled. + pub fn is_git_sync_enabled(&self) -> bool { + !self.git_sync_containers.is_empty() + } + + /// Returns the Git content folders as strings + pub fn git_content_folders_as_string(&self) -> Vec { + self.git_content_folders + .iter() + .map(|path| path.to_str().expect("The path names of the git_content_folders are created as valid UTF-8 strings, so Path::to_str should not fail.").to_string()) + .collect() + } + + /// Creates `GitSyncResources` from the given `GitSync` specifications. + pub fn new( + git_syncs: &[GitSync], + resolved_product_image: &ResolvedProductImage, + extra_env_vars: &[EnvVar], + extra_volume_mounts: &[VolumeMount], + ) -> Result { + let mut resources = GitSyncResources::default(); + + for (i, git_sync) in git_syncs.iter().enumerate() { + let mut env_vars = vec![]; + if let Some(git_credentials_secret) = &git_sync.credentials_secret { + env_vars.push(GitSyncResources::env_var_from_secret( + "GITSYNC_USERNAME", + git_credentials_secret, + "user", + )); + env_vars.push(GitSyncResources::env_var_from_secret( + "GITSYNC_PASSWORD", + git_credentials_secret, + "password", + )); + } + env_vars = insert_or_update_env_vars(&env_vars, extra_env_vars); + + let volume_name = format!("{VOLUME_NAME_PREFIX}-{i}"); + let mount_path = format!("{MOUNT_PATH_PREFIX}-{i}"); + + let git_sync_root_volume_mount = VolumeMount { + name: volume_name.to_owned(), + mount_path: GIT_SYNC_ROOT_DIR.to_string(), + ..VolumeMount::default() + }; + let mut git_sync_container_volume_mounts = vec![git_sync_root_volume_mount]; + git_sync_container_volume_mounts.extend_from_slice(extra_volume_mounts); + + let container = Self::create_git_sync_container( + &format!("{CONTAINER_NAME_PREFIX}-{i}"), + resolved_product_image, + git_sync, + false, + &env_vars, + &git_sync_container_volume_mounts, + )?; + + let init_container = Self::create_git_sync_container( + &format!("{CONTAINER_NAME_PREFIX}-{i}-init"), + resolved_product_image, + git_sync, + true, + &env_vars, + &git_sync_container_volume_mounts, + )?; + + let volume = VolumeBuilder::new(volume_name.to_owned()) + .empty_dir(EmptyDirVolumeSource::default()) + .build(); + + let git_content_volume_mount = VolumeMount { + name: volume_name.to_owned(), + mount_path: mount_path.to_owned(), + ..VolumeMount::default() + }; + + let mut git_content_folder = PathBuf::from(mount_path); + let relative_git_folder = git_sync + .git_folder + .strip_prefix("/") + .unwrap_or(&git_sync.git_folder); + git_content_folder.push(GIT_SYNC_LINK); + git_content_folder.push(relative_git_folder); + + resources.git_sync_containers.push(container); + resources.git_sync_init_containers.push(init_container); + resources.git_content_volumes.push(volume); + resources + .git_content_volume_mounts + .push(git_content_volume_mount); + resources.git_content_folders.push(git_content_folder); + } + + Ok(resources) + } + + fn create_git_sync_container( + container_name: &str, + resolved_product_image: &ResolvedProductImage, + git_sync: &GitSync, + one_time: bool, + env_vars: &[EnvVar], + volume_mounts: &[VolumeMount], + ) -> Result { + let container = ContainerBuilder::new(container_name) + .context(InvalidContainerNameSnafu)? + .image_from_product_image(resolved_product_image) + .command(vec![ + "/bin/bash".to_string(), + "-x".to_string(), + "-euo".to_string(), + "pipefail".to_string(), + "-c".to_string(), + ]) + .args(vec![Self::create_git_sync_command(git_sync, one_time)]) + .add_env_vars(env_vars.into()) + .add_volume_mounts(volume_mounts.to_vec()) + .context(AddVolumeMountSnafu)? + .resources( + ResourceRequirementsBuilder::new() + .with_cpu_request("100m") + .with_cpu_limit("200m") + .with_memory_request("64Mi") + .with_memory_limit("64Mi") + .build(), + ) + .build(); + Ok(container) + } + + fn create_git_sync_command(git_sync: &GitSync, one_time: bool) -> String { + let internal_args = [ + Some(("--repo".to_string(), git_sync.repo.to_owned())), + Some(("--ref".to_string(), git_sync.branch.to_owned())), + Some(("--depth".to_string(), git_sync.depth.to_string())), + Some(( + "--period".to_string(), + format!("{}s", git_sync.wait.as_secs()), + )), + Some(("--link".to_string(), GIT_SYNC_LINK.to_string())), + Some(("--root".to_string(), GIT_SYNC_ROOT_DIR.to_string())), + one_time.then_some(("--one-time".to_string(), "true".to_string())), + ] + .into_iter() + .flatten() + .collect::>(); + + let internal_git_config = [( + GIT_SYNC_SAFE_DIR_OPTION.to_string(), + GIT_SYNC_ROOT_DIR.to_string(), + )] + .into_iter() + .collect::>(); + + let mut user_defined_args = BTreeMap::new(); + // The key and value in Git configs are separated by a colon, but both + // can contain either escaped colons or unescaped colons if enclosed in + // quotes. To avoid parsing, just a vector is used instead of a map. + let mut user_defined_git_configs = Vec::new(); + + for (key, value) in &git_sync.git_sync_conf { + // The initial git-sync implementation in the airflow-operator + // (https://github.com/stackabletech/airflow-operator/pull/381) + // used this condition to find Git configs. It is also used here + // for backwards-compatibility: + if key.to_lowercase().ends_with("-git-config") { + // Roughly check if the user defined config contains an + // internally defined config and emit a warning in case. + if internal_git_config.keys().any(|key| value.contains(key)) { + tracing::warn!("Config option {value:?} contains a value for {GIT_SYNC_SAFE_DIR_OPTION} that overrides + the value of this operator. Git-sync functionality will probably not work as expected!"); + } + user_defined_git_configs.push(value.to_owned()); + } else if internal_args.contains_key(key) { + tracing::warn!( + "The git-sync option {key:?} is already internally defined and will be ignored." + ); + } else { + // The user-defined arguments are not validated. + user_defined_args.insert(key.to_owned(), value.to_owned()); + } + } + + // The user-defined Git config is just appended. + // The user is responsible for escaping special characters like `:` and `,`. + let git_config = internal_git_config + .into_iter() + .map(|(key, value)| format!("{key}:{value}")) + .chain(user_defined_git_configs) + .collect::>() + .join(","); + + let mut args = internal_args; + args.extend(user_defined_args); + args.insert("--git-config".to_string(), format!("'{git_config}'")); + + let args_string = args + .into_iter() + .map(|(key, value)| format!("{key}={value}")) + .collect::>() + .join(" "); + + let git_sync_command = format!("/stackable/git-sync {args_string}"); + + if one_time { + git_sync_command + } else { + // Run the git-sync command in the background + format!( + "{COMMON_BASH_TRAP_FUNCTIONS} +prepare_signal_handlers +{git_sync_command} & +wait_for_termination $!" + ) + } + } + + fn env_var_from_secret(var_name: &str, secret: &str, secret_key: &str) -> EnvVar { + EnvVar { + name: var_name.to_string(), + value_from: Some(EnvVarSource { + secret_key_ref: Some(SecretKeySelector { + name: secret.to_string(), + key: secret_key.to_string(), + ..Default::default() + }), + ..Default::default() + }), + ..Default::default() + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::product_config_utils::env_vars_from; + + #[test] + fn test_no_git_sync() { + let git_syncs = []; + + let resolved_product_image = ResolvedProductImage { + image: "oci.stackable.tech/sdp/product:latest".to_string(), + app_version_label: "1.0.0-latest".to_string(), + product_version: "1.0.0".to_string(), + image_pull_policy: "Always".to_string(), + pull_secrets: None, + }; + + let extra_env_vars = []; + + let extra_volume_mounts = []; + + let git_sync_resources = GitSyncResources::new( + &git_syncs, + &resolved_product_image, + &extra_env_vars, + &extra_volume_mounts, + ) + .unwrap(); + + assert!(!git_sync_resources.is_git_sync_enabled()); + assert!(git_sync_resources.git_sync_containers.is_empty()); + assert!(git_sync_resources.git_sync_init_containers.is_empty()); + assert!(git_sync_resources.git_content_volumes.is_empty()); + assert!(git_sync_resources.git_content_volume_mounts.is_empty()); + assert!(git_sync_resources.git_content_folders.is_empty()); + } + + #[test] + fn test_multiple_git_syncs() { + let git_sync_spec = r#" + # GitSync with defaults + - repo: https://github.com/stackabletech/repo1 + + # GitSync with usual configuration + - repo: https://github.com/stackabletech/repo2 + branch: trunk + gitFolder: "" + depth: 3 + wait: 1m + credentialsSecret: git-credentials + gitSyncConf: + --rev: HEAD + --git-config: http.sslCAInfo:/tmp/ca-cert/ca.crt + + # GitSync with unusual configuration + - repo: https://github.com/stackabletech/repo3 + branch: feat/git-sync + # leading slashes should be removed + gitFolder: ////folder + gitSyncConf: + --depth: internal option which should be ignored + --link: internal option which should be ignored + --period: internal option which should be ignored + --ref: internal option which should be ignored + --repo: internal option which should be ignored + --root: internal option which should be ignored + --GIT-CONFIG: k1:v1 + # safe.directory should be accepted but a warning will be emitted + --git-config: k2:v2,safe.directory:/safe-dir + -GIT-CONFIG: k3:v3 + -git-config: k4:v4 + "#; + + let deserializer = serde_yaml::Deserializer::from_str(git_sync_spec); + let git_syncs: Vec = + serde_yaml::with::singleton_map_recursive::deserialize(deserializer).unwrap(); + + let resolved_product_image = ResolvedProductImage { + image: "oci.stackable.tech/sdp/product:latest".to_string(), + app_version_label: "1.0.0-latest".to_string(), + product_version: "1.0.0".to_string(), + image_pull_policy: "Always".to_string(), + pull_secrets: None, + }; + + let extra_env_vars = env_vars_from([ + ("VAR1", "value1"), + ("GITSYNC_USERNAME", "overriden-username"), + ]); + + let extra_volume_mounts = [VolumeMount { + name: "extra-volume".to_string(), + mount_path: "/mnt/extra-volume".to_string(), + ..VolumeMount::default() + }]; + + let git_sync_resources = GitSyncResources::new( + &git_syncs, + &resolved_product_image, + &extra_env_vars, + &extra_volume_mounts, + ) + .unwrap(); + + assert!(git_sync_resources.is_git_sync_enabled()); + + assert_eq!(3, git_sync_resources.git_sync_containers.len()); + + assert_eq!( + r#"args: +- |2- + + prepare_signal_handlers() + { + unset term_child_pid + unset term_kill_needed + trap 'handle_term_signal' TERM + } + + handle_term_signal() + { + if [ "${term_child_pid}" ]; then + kill -TERM "${term_child_pid}" 2>/dev/null + else + term_kill_needed="yes" + fi + } + + wait_for_termination() + { + set +e + term_child_pid=$1 + if [[ -v term_kill_needed ]]; then + kill -TERM "${term_child_pid}" 2>/dev/null + fi + wait ${term_child_pid} 2>/dev/null + trap - TERM + wait ${term_child_pid} 2>/dev/null + set -e + } + + prepare_signal_handlers + /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git' --link=current --period=20s --ref=main --repo=https://github.com/stackabletech/repo1 --root=/tmp/git & + wait_for_termination $! +command: +- /bin/bash +- -x +- -euo +- pipefail +- -c +env: +- name: GITSYNC_USERNAME + value: overriden-username +- name: VAR1 + value: value1 +image: oci.stackable.tech/sdp/product:latest +imagePullPolicy: Always +name: git-sync-0 +resources: + limits: + cpu: 200m + memory: 64Mi + requests: + cpu: 100m + memory: 64Mi +volumeMounts: +- mountPath: /tmp/git + name: content-from-git-0 +- mountPath: /mnt/extra-volume + name: extra-volume +"#, + serde_yaml::to_string(&git_sync_resources.git_sync_containers.first()).unwrap() + ); + + assert_eq!( + r#"args: +- |2- + + prepare_signal_handlers() + { + unset term_child_pid + unset term_kill_needed + trap 'handle_term_signal' TERM + } + + handle_term_signal() + { + if [ "${term_child_pid}" ]; then + kill -TERM "${term_child_pid}" 2>/dev/null + else + term_kill_needed="yes" + fi + } + + wait_for_termination() + { + set +e + term_child_pid=$1 + if [[ -v term_kill_needed ]]; then + kill -TERM "${term_child_pid}" 2>/dev/null + fi + wait ${term_child_pid} 2>/dev/null + trap - TERM + wait ${term_child_pid} 2>/dev/null + set -e + } + + prepare_signal_handlers + /stackable/git-sync --depth=3 --git-config='safe.directory:/tmp/git,http.sslCAInfo:/tmp/ca-cert/ca.crt' --link=current --period=60s --ref=trunk --repo=https://github.com/stackabletech/repo2 --rev=HEAD --root=/tmp/git & + wait_for_termination $! +command: +- /bin/bash +- -x +- -euo +- pipefail +- -c +env: +- name: GITSYNC_PASSWORD + valueFrom: + secretKeyRef: + key: password + name: git-credentials +- name: GITSYNC_USERNAME + value: overriden-username +- name: VAR1 + value: value1 +image: oci.stackable.tech/sdp/product:latest +imagePullPolicy: Always +name: git-sync-1 +resources: + limits: + cpu: 200m + memory: 64Mi + requests: + cpu: 100m + memory: 64Mi +volumeMounts: +- mountPath: /tmp/git + name: content-from-git-1 +- mountPath: /mnt/extra-volume + name: extra-volume +"#, + serde_yaml::to_string(&git_sync_resources.git_sync_containers.get(1)).unwrap() + ); + + assert_eq!( + r#"args: +- |2- + + prepare_signal_handlers() + { + unset term_child_pid + unset term_kill_needed + trap 'handle_term_signal' TERM + } + + handle_term_signal() + { + if [ "${term_child_pid}" ]; then + kill -TERM "${term_child_pid}" 2>/dev/null + else + term_kill_needed="yes" + fi + } + + wait_for_termination() + { + set +e + term_child_pid=$1 + if [[ -v term_kill_needed ]]; then + kill -TERM "${term_child_pid}" 2>/dev/null + fi + wait ${term_child_pid} 2>/dev/null + trap - TERM + wait ${term_child_pid} 2>/dev/null + set -e + } + + prepare_signal_handlers + /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git,k1:v1,k2:v2,safe.directory:/safe-dir,k3:v3,k4:v4' --link=current --period=20s --ref=feat/git-sync --repo=https://github.com/stackabletech/repo3 --root=/tmp/git & + wait_for_termination $! +command: +- /bin/bash +- -x +- -euo +- pipefail +- -c +env: +- name: GITSYNC_USERNAME + value: overriden-username +- name: VAR1 + value: value1 +image: oci.stackable.tech/sdp/product:latest +imagePullPolicy: Always +name: git-sync-2 +resources: + limits: + cpu: 200m + memory: 64Mi + requests: + cpu: 100m + memory: 64Mi +volumeMounts: +- mountPath: /tmp/git + name: content-from-git-2 +- mountPath: /mnt/extra-volume + name: extra-volume +"#, + serde_yaml::to_string(&git_sync_resources.git_sync_containers.get(2)).unwrap() + ); + + assert_eq!(3, git_sync_resources.git_sync_init_containers.len()); + + assert_eq!( + r#"args: +- /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git' --link=current --one-time=true --period=20s --ref=main --repo=https://github.com/stackabletech/repo1 --root=/tmp/git +command: +- /bin/bash +- -x +- -euo +- pipefail +- -c +env: +- name: GITSYNC_USERNAME + value: overriden-username +- name: VAR1 + value: value1 +image: oci.stackable.tech/sdp/product:latest +imagePullPolicy: Always +name: git-sync-0-init +resources: + limits: + cpu: 200m + memory: 64Mi + requests: + cpu: 100m + memory: 64Mi +volumeMounts: +- mountPath: /tmp/git + name: content-from-git-0 +- mountPath: /mnt/extra-volume + name: extra-volume +"#, + serde_yaml::to_string(&git_sync_resources.git_sync_init_containers.first()).unwrap() + ); + + assert_eq!( + r#"args: +- /stackable/git-sync --depth=3 --git-config='safe.directory:/tmp/git,http.sslCAInfo:/tmp/ca-cert/ca.crt' --link=current --one-time=true --period=60s --ref=trunk --repo=https://github.com/stackabletech/repo2 --rev=HEAD --root=/tmp/git +command: +- /bin/bash +- -x +- -euo +- pipefail +- -c +env: +- name: GITSYNC_PASSWORD + valueFrom: + secretKeyRef: + key: password + name: git-credentials +- name: GITSYNC_USERNAME + value: overriden-username +- name: VAR1 + value: value1 +image: oci.stackable.tech/sdp/product:latest +imagePullPolicy: Always +name: git-sync-1-init +resources: + limits: + cpu: 200m + memory: 64Mi + requests: + cpu: 100m + memory: 64Mi +volumeMounts: +- mountPath: /tmp/git + name: content-from-git-1 +- mountPath: /mnt/extra-volume + name: extra-volume +"#, + serde_yaml::to_string(&git_sync_resources.git_sync_init_containers.get(1)).unwrap() + ); + + assert_eq!( + r#"args: +- /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git,k1:v1,k2:v2,safe.directory:/safe-dir,k3:v3,k4:v4' --link=current --one-time=true --period=20s --ref=feat/git-sync --repo=https://github.com/stackabletech/repo3 --root=/tmp/git +command: +- /bin/bash +- -x +- -euo +- pipefail +- -c +env: +- name: GITSYNC_USERNAME + value: overriden-username +- name: VAR1 + value: value1 +image: oci.stackable.tech/sdp/product:latest +imagePullPolicy: Always +name: git-sync-2-init +resources: + limits: + cpu: 200m + memory: 64Mi + requests: + cpu: 100m + memory: 64Mi +volumeMounts: +- mountPath: /tmp/git + name: content-from-git-2 +- mountPath: /mnt/extra-volume + name: extra-volume +"#, + serde_yaml::to_string(&git_sync_resources.git_sync_init_containers.get(2)).unwrap() + ); + + assert_eq!(3, git_sync_resources.git_content_volumes.len()); + + assert_eq!( + "emptyDir: {} +name: content-from-git-0 +", + serde_yaml::to_string(&git_sync_resources.git_content_volumes.first()).unwrap() + ); + + assert_eq!( + "emptyDir: {} +name: content-from-git-1 +", + serde_yaml::to_string(&git_sync_resources.git_content_volumes.get(1)).unwrap() + ); + + assert_eq!( + "emptyDir: {} +name: content-from-git-2 +", + serde_yaml::to_string(&git_sync_resources.git_content_volumes.get(2)).unwrap() + ); + + assert_eq!(3, git_sync_resources.git_content_volume_mounts.len()); + + assert_eq!( + "mountPath: /stackable/app/git-0 +name: content-from-git-0 +", + serde_yaml::to_string(&git_sync_resources.git_content_volume_mounts.first()).unwrap() + ); + + assert_eq!( + "mountPath: /stackable/app/git-1 +name: content-from-git-1 +", + serde_yaml::to_string(&git_sync_resources.git_content_volume_mounts.get(1)).unwrap() + ); + + assert_eq!( + "mountPath: /stackable/app/git-2 +name: content-from-git-2 +", + serde_yaml::to_string(&git_sync_resources.git_content_volume_mounts.get(2)).unwrap() + ); + + assert_eq!(3, git_sync_resources.git_content_folders.len()); + + assert_eq!( + "/stackable/app/git-0/current/", + git_sync_resources + .git_content_folders_as_string() + .first() + .unwrap() + ); + + assert_eq!( + "/stackable/app/git-1/current/", + git_sync_resources + .git_content_folders_as_string() + .get(1) + .unwrap() + ); + + assert_eq!( + "/stackable/app/git-2/current/folder", + git_sync_resources + .git_content_folders_as_string() + .get(2) + .unwrap() + ); + } +} diff --git a/crates/stackable-operator/src/git_sync/mod.rs b/crates/stackable-operator/src/git_sync/mod.rs new file mode 100644 index 00000000..5641ec25 --- /dev/null +++ b/crates/stackable-operator/src/git_sync/mod.rs @@ -0,0 +1,4 @@ +//! Modules for git-sync + +pub mod framework; +pub mod spec; diff --git a/crates/stackable-operator/src/git_sync/spec.rs b/crates/stackable-operator/src/git_sync/spec.rs new file mode 100644 index 00000000..6c57b12b --- /dev/null +++ b/crates/stackable-operator/src/git_sync/spec.rs @@ -0,0 +1,67 @@ +//! GitSync structure for CRDs + +use std::{collections::BTreeMap, path::PathBuf}; + +use schemars::{self, JsonSchema}; +use serde::{Deserialize, Serialize}; + +use crate::time::Duration; + +#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct GitSync { + /// The git repository URL that will be cloned, for example: `https://github.com/stackabletech/airflow-operator`. + pub repo: String, + + /// The branch to clone; defaults to `main`. + /// + /// Since git-sync v4.x.x this field is mapped to the flag `--ref`. + #[serde(default = "GitSync::default_branch")] + pub branch: String, + + /// The location of the DAG folder, relative to the synced repository root. + /// + /// It can optionally start with `/`, however, no trailing slash is recommended. + /// An empty string (``) or slash (`/`) corresponds to the root folder in Git. + #[serde(default = "GitSync::default_git_folder")] + pub git_folder: PathBuf, + + /// The depth of syncing, i.e. the number of commits to clone; defaults to 1. + #[serde(default = "GitSync::default_depth")] + pub depth: u32, + + /// The synchronization interval, e.g. `20s` or `5m`; defaults to `20s`. + /// + /// Since git-sync v4.x.x this field is mapped to the flag `--period`. + #[serde(default = "GitSync::default_wait")] + pub wait: Duration, + + /// The name of the Secret used to access the repository if it is not public. + /// This should include two fields: `user` and `password`. + /// The `password` field can either be an actual password (not recommended) or a GitHub token, + /// as described [here](https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual). + pub credentials_secret: Option, + + /// A map of optional configuration settings that are listed in the [git-sync documentation](https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual). + /// Read the [git sync example](DOCS_BASE_URL_PLACEHOLDER/airflow/usage-guide/mounting-dags#_example). + #[serde(default)] + pub git_sync_conf: BTreeMap, +} + +impl GitSync { + fn default_branch() -> String { + "main".to_string() + } + + fn default_git_folder() -> PathBuf { + PathBuf::from("/") + } + + fn default_depth() -> u32 { + 1 + } + + fn default_wait() -> Duration { + Duration::from_secs(20) + } +} diff --git a/crates/stackable-operator/src/lib.rs b/crates/stackable-operator/src/lib.rs index f0ccc599..1dcd8d56 100644 --- a/crates/stackable-operator/src/lib.rs +++ b/crates/stackable-operator/src/lib.rs @@ -15,6 +15,7 @@ pub mod config; pub mod constants; pub mod cpu; pub mod crd; +pub mod git_sync; pub mod helm; pub mod iter; pub mod kvp; diff --git a/crates/stackable-operator/src/product_config_utils.rs b/crates/stackable-operator/src/product_config_utils.rs index cf6ea870..0406b451 100644 --- a/crates/stackable-operator/src/product_config_utils.rs +++ b/crates/stackable-operator/src/product_config_utils.rs @@ -1,5 +1,6 @@ use std::collections::{BTreeMap, HashMap}; +use k8s_openapi::api::core::v1::EnvVar; use product_config::{ProductConfigManager, PropertyValidationResult, types::PropertyNameKind}; use schemars::JsonSchema; use serde::Serialize; @@ -528,6 +529,156 @@ where Ok(final_overrides) } +/// Extract the environment variables of a rolegroup config into a vector of EnvVars. +/// +/// # Example +/// +/// ``` +/// use std::collections::{BTreeMap, HashMap}; +/// +/// use k8s_openapi::api::core::v1::EnvVar; +/// use product_config::types::PropertyNameKind; +/// use stackable_operator::product_config_utils::env_vars_from_rolegroup_config; +/// +/// let rolegroup_config = [( +/// PropertyNameKind::Env, +/// [ +/// ("VAR1".to_string(), "value 1".to_string()), +/// ("VAR2".to_string(), "value 2".to_string()), +/// ] +/// .into_iter() +/// .collect::>(), +/// )] +/// .into_iter() +/// .collect::>(); +/// +/// let expected_env_vars = vec![ +/// EnvVar { +/// name: "VAR1".to_string(), +/// value: Some("value 1".to_string()), +/// value_from: None, +/// }, +/// EnvVar { +/// name: "VAR2".to_string(), +/// value: Some("value 2".to_string()), +/// value_from: None, +/// }, +/// ]; +/// assert_eq!( +/// expected_env_vars, +/// env_vars_from_rolegroup_config(&rolegroup_config) +/// ); +/// ``` +pub fn env_vars_from_rolegroup_config( + rolegroup_config: &HashMap>, +) -> Vec { + env_vars_from( + rolegroup_config + .get(&PropertyNameKind::Env) + .cloned() + .unwrap_or_default(), + ) +} + +/// Convert key-value structures into a vector of EnvVars. +/// +/// # Example +/// +/// ``` +/// use k8s_openapi::api::core::v1::EnvVar; +/// use stackable_operator::{product_config_utils::env_vars_from, role_utils::CommonConfiguration}; +/// +/// let common_config = CommonConfiguration::<(), ()> { +/// env_overrides: [("VAR".to_string(), "value".to_string())] +/// .into_iter() +/// .collect(), +/// ..Default::default() +/// }; +/// +/// let env_vars = env_vars_from(common_config.env_overrides); +/// +/// let expected_env_vars = vec![EnvVar { +/// name: "VAR".to_string(), +/// value: Some("value".to_string()), +/// value_from: None +/// }]; +/// +/// assert_eq!(expected_env_vars, env_vars); +/// ``` +pub fn env_vars_from(env_vars: I) -> Vec +where + I: IntoIterator, + K: Clone + Into, + V: Clone + Into, +{ + env_vars.into_iter().map(env_var_from_tuple).collect() +} + +/// Convert a tuple of strings into an EnvVar +/// +/// # Example +/// +/// ``` +/// use k8s_openapi::api::core::v1::EnvVar; +/// use stackable_operator::product_config_utils::env_var_from_tuple; +/// +/// let tuple = ("VAR", "value"); +/// +/// let env_var = env_var_from_tuple(tuple); +/// +/// let expected_env_var = EnvVar { +/// name: "VAR".to_string(), +/// value: Some("value".to_string()), +/// value_from: None, +/// }; +/// assert_eq!(expected_env_var, env_var); +/// ``` +pub fn env_var_from_tuple(entry: (impl Into, impl Into)) -> EnvVar { + EnvVar { + name: entry.0.into(), + value: Some(entry.1.into()), + value_from: None, + } +} + +/// Inserts or updates the EnvVars from `env_overrides` in `env_vars`. +/// +/// The resulting vector is sorted by the EnvVar names. +/// +/// # Example +/// +/// ``` +/// use stackable_operator::product_config_utils::{env_vars_from, insert_or_update_env_vars}; +/// +/// let env_vars = env_vars_from([ +/// ("VAR1", "original value 1"), +/// ("VAR2", "original value 2") +/// ]); +/// let env_overrides = env_vars_from([ +/// ("VAR2", "overriden value 2"), +/// ("VAR3", "new value 3") +/// ]); +/// +/// let combined_env_vars = insert_or_update_env_vars(&env_vars, &env_overrides); +/// +/// let expected_result = env_vars_from([ +/// ("VAR1", "original value 1"), +/// ("VAR2", "overriden value 2"), +/// ("VAR3", "new value 3"), +/// ]); +/// +/// assert_eq!(expected_result, combined_env_vars); +/// ``` +pub fn insert_or_update_env_vars(env_vars: &[EnvVar], env_overrides: &[EnvVar]) -> Vec { + let mut combined = BTreeMap::new(); + + for env_var in env_vars.iter().chain(env_overrides) { + combined.insert(env_var.name.to_owned(), env_var.to_owned()); + } + + combined.into_values().collect() +} + #[cfg(test)] mod tests { macro_rules! collection { From 5d7a0076b1e51b6105375e46e44e79198d6e233a Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Thu, 8 May 2025 15:49:33 +0200 Subject: [PATCH 02/18] Version the git_sync structure as v1alpha1 --- .../src/crd/git_sync/mod.rs | 58 ++++++++++++++++ .../git_sync/v1alpha1_impl.rs} | 21 +++++- crates/stackable-operator/src/crd/mod.rs | 1 + crates/stackable-operator/src/git_sync/mod.rs | 4 -- .../stackable-operator/src/git_sync/spec.rs | 67 ------------------- crates/stackable-operator/src/lib.rs | 1 - 6 files changed, 79 insertions(+), 73 deletions(-) create mode 100644 crates/stackable-operator/src/crd/git_sync/mod.rs rename crates/stackable-operator/src/{git_sync/framework.rs => crd/git_sync/v1alpha1_impl.rs} (98%) delete mode 100644 crates/stackable-operator/src/git_sync/mod.rs delete mode 100644 crates/stackable-operator/src/git_sync/spec.rs diff --git a/crates/stackable-operator/src/crd/git_sync/mod.rs b/crates/stackable-operator/src/crd/git_sync/mod.rs new file mode 100644 index 00000000..2e9aac8f --- /dev/null +++ b/crates/stackable-operator/src/crd/git_sync/mod.rs @@ -0,0 +1,58 @@ +//! GitSync structure for CRDs + +use std::{collections::BTreeMap, path::PathBuf}; + +use schemars::{self, JsonSchema}; +use serde::{Deserialize, Serialize}; + +use crate::{time::Duration, versioned::versioned}; + +mod v1alpha1_impl; + +#[versioned(version(name = "v1alpha1"))] +pub mod versioned { + pub mod v1alpha1 { + pub use v1alpha1_impl::{Error, GitSyncResources}; + } + + #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Eq, Serialize)] + #[serde(rename_all = "camelCase")] + pub struct GitSync { + /// The git repository URL that will be cloned, for example: `https://github.com/stackabletech/airflow-operator`. + pub repo: String, + + /// The branch to clone; defaults to `main`. + /// + /// Since git-sync v4.x.x this field is mapped to the flag `--ref`. + #[serde(default = "GitSync::default_branch")] + pub branch: String, + + /// The location of the DAG folder, relative to the synced repository root. + /// + /// It can optionally start with `/`, however, no trailing slash is recommended. + /// An empty string (``) or slash (`/`) corresponds to the root folder in Git. + #[serde(default = "GitSync::default_git_folder")] + pub git_folder: PathBuf, + + /// The depth of syncing, i.e. the number of commits to clone; defaults to 1. + #[serde(default = "GitSync::default_depth")] + pub depth: u32, + + /// The synchronization interval, e.g. `20s` or `5m`; defaults to `20s`. + /// + /// Since git-sync v4.x.x this field is mapped to the flag `--period`. + #[serde(default = "GitSync::default_wait")] + pub wait: Duration, + + /// The name of the Secret used to access the repository if it is not public. + /// This should include two fields: `user` and `password`. + /// The `password` field can either be an actual password (not recommended) or a GitHub token, + /// as described [here](https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual). + pub credentials_secret: Option, + + /// A map of optional configuration settings that are listed in the [git-sync documentation](https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual). + /// Read the [git sync example](DOCS_BASE_URL_PLACEHOLDER/airflow/usage-guide/mounting-dags#_example). + #[serde(default)] + pub git_sync_conf: BTreeMap, + } +} diff --git a/crates/stackable-operator/src/git_sync/framework.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs similarity index 98% rename from crates/stackable-operator/src/git_sync/framework.rs rename to crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index c2872df5..07f2d0c3 100644 --- a/crates/stackable-operator/src/git_sync/framework.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -6,13 +6,14 @@ use k8s_openapi::api::core::v1::{ use snafu::{ResultExt, Snafu}; use strum::{EnumDiscriminants, IntoStaticStr}; -use super::spec::GitSync; use crate::{ builder::pod::{ container::ContainerBuilder, resources::ResourceRequirementsBuilder, volume::VolumeBuilder, }, commons::product_image_selection::ResolvedProductImage, + crd::git_sync::v1alpha1::GitSync, product_config_utils::insert_or_update_env_vars, + time::Duration, utils::COMMON_BASH_TRAP_FUNCTIONS, }; @@ -37,6 +38,24 @@ pub enum Error { }, } +impl GitSync { + pub(crate) fn default_branch() -> String { + "main".to_string() + } + + pub(crate) fn default_git_folder() -> PathBuf { + PathBuf::from("/") + } + + pub(crate) fn default_depth() -> u32 { + 1 + } + + pub(crate) fn default_wait() -> Duration { + Duration::from_secs(20) + } +} + /// Kubernetes resources generated from `GitSync` specifications which should be added to the Pod. #[derive(Default)] pub struct GitSyncResources { diff --git a/crates/stackable-operator/src/crd/mod.rs b/crates/stackable-operator/src/crd/mod.rs index 399b5d98..3beb69aa 100644 --- a/crates/stackable-operator/src/crd/mod.rs +++ b/crates/stackable-operator/src/crd/mod.rs @@ -5,6 +5,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; pub mod authentication; +pub mod git_sync; pub mod listener; pub mod s3; diff --git a/crates/stackable-operator/src/git_sync/mod.rs b/crates/stackable-operator/src/git_sync/mod.rs deleted file mode 100644 index 5641ec25..00000000 --- a/crates/stackable-operator/src/git_sync/mod.rs +++ /dev/null @@ -1,4 +0,0 @@ -//! Modules for git-sync - -pub mod framework; -pub mod spec; diff --git a/crates/stackable-operator/src/git_sync/spec.rs b/crates/stackable-operator/src/git_sync/spec.rs deleted file mode 100644 index 6c57b12b..00000000 --- a/crates/stackable-operator/src/git_sync/spec.rs +++ /dev/null @@ -1,67 +0,0 @@ -//! GitSync structure for CRDs - -use std::{collections::BTreeMap, path::PathBuf}; - -use schemars::{self, JsonSchema}; -use serde::{Deserialize, Serialize}; - -use crate::time::Duration; - -#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Eq, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct GitSync { - /// The git repository URL that will be cloned, for example: `https://github.com/stackabletech/airflow-operator`. - pub repo: String, - - /// The branch to clone; defaults to `main`. - /// - /// Since git-sync v4.x.x this field is mapped to the flag `--ref`. - #[serde(default = "GitSync::default_branch")] - pub branch: String, - - /// The location of the DAG folder, relative to the synced repository root. - /// - /// It can optionally start with `/`, however, no trailing slash is recommended. - /// An empty string (``) or slash (`/`) corresponds to the root folder in Git. - #[serde(default = "GitSync::default_git_folder")] - pub git_folder: PathBuf, - - /// The depth of syncing, i.e. the number of commits to clone; defaults to 1. - #[serde(default = "GitSync::default_depth")] - pub depth: u32, - - /// The synchronization interval, e.g. `20s` or `5m`; defaults to `20s`. - /// - /// Since git-sync v4.x.x this field is mapped to the flag `--period`. - #[serde(default = "GitSync::default_wait")] - pub wait: Duration, - - /// The name of the Secret used to access the repository if it is not public. - /// This should include two fields: `user` and `password`. - /// The `password` field can either be an actual password (not recommended) or a GitHub token, - /// as described [here](https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual). - pub credentials_secret: Option, - - /// A map of optional configuration settings that are listed in the [git-sync documentation](https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual). - /// Read the [git sync example](DOCS_BASE_URL_PLACEHOLDER/airflow/usage-guide/mounting-dags#_example). - #[serde(default)] - pub git_sync_conf: BTreeMap, -} - -impl GitSync { - fn default_branch() -> String { - "main".to_string() - } - - fn default_git_folder() -> PathBuf { - PathBuf::from("/") - } - - fn default_depth() -> u32 { - 1 - } - - fn default_wait() -> Duration { - Duration::from_secs(20) - } -} diff --git a/crates/stackable-operator/src/lib.rs b/crates/stackable-operator/src/lib.rs index 1dcd8d56..f0ccc599 100644 --- a/crates/stackable-operator/src/lib.rs +++ b/crates/stackable-operator/src/lib.rs @@ -15,7 +15,6 @@ pub mod config; pub mod constants; pub mod cpu; pub mod crd; -pub mod git_sync; pub mod helm; pub mod iter; pub mod kvp; From 613e58de117218c49296a3da100994c8b9fedcb0 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 9 May 2025 09:32:12 +0200 Subject: [PATCH 03/18] Rephrase the comment for GitSync::git_folder --- crates/stackable-operator/src/crd/git_sync/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-operator/src/crd/git_sync/mod.rs b/crates/stackable-operator/src/crd/git_sync/mod.rs index 2e9aac8f..b6b9013d 100644 --- a/crates/stackable-operator/src/crd/git_sync/mod.rs +++ b/crates/stackable-operator/src/crd/git_sync/mod.rs @@ -27,7 +27,7 @@ pub mod versioned { #[serde(default = "GitSync::default_branch")] pub branch: String, - /// The location of the DAG folder, relative to the synced repository root. + /// Location in the Git repository containing the resource. /// /// It can optionally start with `/`, however, no trailing slash is recommended. /// An empty string (``) or slash (`/`) corresponds to the root folder in Git. From 6619fe4cc96ba1fbe97b99de5ddf43e7f3d14647 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Mon, 12 May 2025 20:24:47 +0200 Subject: [PATCH 04/18] Add logging support to git-sync containers --- .../src/crd/git_sync/v1alpha1_impl.rs | 99 ++++++++++++++++--- 1 file changed, 86 insertions(+), 13 deletions(-) diff --git a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index 07f2d0c3..711e3c85 100644 --- a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -13,6 +13,10 @@ use crate::{ commons::product_image_selection::ResolvedProductImage, crd::git_sync::v1alpha1::GitSync, product_config_utils::insert_or_update_env_vars, + product_logging::{ + framework::capture_shell_output, + spec::{ContainerLogConfig, ContainerLogConfigChoice}, + }, time::Duration, utils::COMMON_BASH_TRAP_FUNCTIONS, }; @@ -72,6 +76,8 @@ pub struct GitSyncResources { } impl GitSyncResources { + const LOG_VOLUME_MOUNT_PATH: &str = "/stackable/log"; + /// Returns whether or not GitSync is enabled. pub fn is_git_sync_enabled(&self) -> bool { !self.git_sync_containers.is_empty() @@ -91,6 +97,8 @@ impl GitSyncResources { resolved_product_image: &ResolvedProductImage, extra_env_vars: &[EnvVar], extra_volume_mounts: &[VolumeMount], + log_volume_name: &str, + container_log_config: &ContainerLogConfig, ) -> Result { let mut resources = GitSyncResources::default(); @@ -118,7 +126,15 @@ impl GitSyncResources { mount_path: GIT_SYNC_ROOT_DIR.to_string(), ..VolumeMount::default() }; - let mut git_sync_container_volume_mounts = vec![git_sync_root_volume_mount]; + + let log_volume_mount = VolumeMount { + name: log_volume_name.to_string(), + mount_path: Self::LOG_VOLUME_MOUNT_PATH.to_string(), + ..VolumeMount::default() + }; + + let mut git_sync_container_volume_mounts = + vec![git_sync_root_volume_mount, log_volume_mount]; git_sync_container_volume_mounts.extend_from_slice(extra_volume_mounts); let container = Self::create_git_sync_container( @@ -128,6 +144,7 @@ impl GitSyncResources { false, &env_vars, &git_sync_container_volume_mounts, + container_log_config, )?; let init_container = Self::create_git_sync_container( @@ -137,6 +154,7 @@ impl GitSyncResources { true, &env_vars, &git_sync_container_volume_mounts, + container_log_config, )?; let volume = VolumeBuilder::new(volume_name.to_owned()) @@ -176,6 +194,7 @@ impl GitSyncResources { one_time: bool, env_vars: &[EnvVar], volume_mounts: &[VolumeMount], + container_log_config: &ContainerLogConfig, ) -> Result { let container = ContainerBuilder::new(container_name) .context(InvalidContainerNameSnafu)? @@ -187,7 +206,12 @@ impl GitSyncResources { "pipefail".to_string(), "-c".to_string(), ]) - .args(vec![Self::create_git_sync_command(git_sync, one_time)]) + .args(vec![Self::create_git_sync_shell_script( + container_name, + git_sync, + one_time, + container_log_config, + )]) .add_env_vars(env_vars.into()) .add_volume_mounts(volume_mounts.to_vec()) .context(AddVolumeMountSnafu)? @@ -203,7 +227,12 @@ impl GitSyncResources { Ok(container) } - fn create_git_sync_command(git_sync: &GitSync, one_time: bool) -> String { + fn create_git_sync_shell_script( + container_name: &str, + git_sync: &GitSync, + one_time: bool, + container_log_config: &ContainerLogConfig, + ) -> String { let internal_args = [ Some(("--repo".to_string(), git_sync.repo.to_owned())), Some(("--ref".to_string(), git_sync.branch.to_owned())), @@ -275,19 +304,35 @@ impl GitSyncResources { .collect::>() .join(" "); + let mut shell_script = String::new(); + + if let ContainerLogConfig { + choice: Some(ContainerLogConfigChoice::Automatic(log_config)), + } = container_log_config + { + shell_script.push_str(&capture_shell_output( + Self::LOG_VOLUME_MOUNT_PATH, + container_name, + log_config, + )); + shell_script.push('\n'); + }; + let git_sync_command = format!("/stackable/git-sync {args_string}"); if one_time { - git_sync_command + shell_script.push_str(&git_sync_command); } else { // Run the git-sync command in the background - format!( + shell_script.push_str(&format!( "{COMMON_BASH_TRAP_FUNCTIONS} prepare_signal_handlers {git_sync_command} & wait_for_termination $!" - ) + )) } + + shell_script } fn env_var_from_secret(var_name: &str, secret: &str, secret_key: &str) -> EnvVar { @@ -309,7 +354,10 @@ wait_for_termination $!" #[cfg(test)] mod tests { use super::*; - use crate::product_config_utils::env_vars_from; + use crate::{ + config::fragment::validate, product_config_utils::env_vars_from, + product_logging::spec::default_container_log_config, + }; #[test] fn test_no_git_sync() { @@ -332,6 +380,8 @@ mod tests { &resolved_product_image, &extra_env_vars, &extra_volume_mounts, + "log-volume", + &validate(default_container_log_config()).unwrap(), ) .unwrap(); @@ -407,6 +457,8 @@ mod tests { &resolved_product_image, &extra_env_vars, &extra_volume_mounts, + "log-volume", + &validate(default_container_log_config()).unwrap(), ) .unwrap(); @@ -416,7 +468,8 @@ mod tests { assert_eq!( r#"args: -- |2- +- |- + mkdir --parents /stackable/log/git-sync-0 && exec > >(tee /stackable/log/git-sync-0/container.stdout.log) 2> >(tee /stackable/log/git-sync-0/container.stderr.log >&2) prepare_signal_handlers() { @@ -474,6 +527,8 @@ resources: volumeMounts: - mountPath: /tmp/git name: content-from-git-0 +- mountPath: /stackable/log + name: log-volume - mountPath: /mnt/extra-volume name: extra-volume "#, @@ -482,7 +537,8 @@ volumeMounts: assert_eq!( r#"args: -- |2- +- |- + mkdir --parents /stackable/log/git-sync-1 && exec > >(tee /stackable/log/git-sync-1/container.stdout.log) 2> >(tee /stackable/log/git-sync-1/container.stderr.log >&2) prepare_signal_handlers() { @@ -545,6 +601,8 @@ resources: volumeMounts: - mountPath: /tmp/git name: content-from-git-1 +- mountPath: /stackable/log + name: log-volume - mountPath: /mnt/extra-volume name: extra-volume "#, @@ -553,7 +611,8 @@ volumeMounts: assert_eq!( r#"args: -- |2- +- |- + mkdir --parents /stackable/log/git-sync-2 && exec > >(tee /stackable/log/git-sync-2/container.stdout.log) 2> >(tee /stackable/log/git-sync-2/container.stderr.log >&2) prepare_signal_handlers() { @@ -611,6 +670,8 @@ resources: volumeMounts: - mountPath: /tmp/git name: content-from-git-2 +- mountPath: /stackable/log + name: log-volume - mountPath: /mnt/extra-volume name: extra-volume "#, @@ -621,7 +682,9 @@ volumeMounts: assert_eq!( r#"args: -- /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git' --link=current --one-time=true --period=20s --ref=main --repo=https://github.com/stackabletech/repo1 --root=/tmp/git +- |- + mkdir --parents /stackable/log/git-sync-0-init && exec > >(tee /stackable/log/git-sync-0-init/container.stdout.log) 2> >(tee /stackable/log/git-sync-0-init/container.stderr.log >&2) + /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git' --link=current --one-time=true --period=20s --ref=main --repo=https://github.com/stackabletech/repo1 --root=/tmp/git command: - /bin/bash - -x @@ -646,6 +709,8 @@ resources: volumeMounts: - mountPath: /tmp/git name: content-from-git-0 +- mountPath: /stackable/log + name: log-volume - mountPath: /mnt/extra-volume name: extra-volume "#, @@ -654,7 +719,9 @@ volumeMounts: assert_eq!( r#"args: -- /stackable/git-sync --depth=3 --git-config='safe.directory:/tmp/git,http.sslCAInfo:/tmp/ca-cert/ca.crt' --link=current --one-time=true --period=60s --ref=trunk --repo=https://github.com/stackabletech/repo2 --rev=HEAD --root=/tmp/git +- |- + mkdir --parents /stackable/log/git-sync-1-init && exec > >(tee /stackable/log/git-sync-1-init/container.stdout.log) 2> >(tee /stackable/log/git-sync-1-init/container.stderr.log >&2) + /stackable/git-sync --depth=3 --git-config='safe.directory:/tmp/git,http.sslCAInfo:/tmp/ca-cert/ca.crt' --link=current --one-time=true --period=60s --ref=trunk --repo=https://github.com/stackabletech/repo2 --rev=HEAD --root=/tmp/git command: - /bin/bash - -x @@ -684,6 +751,8 @@ resources: volumeMounts: - mountPath: /tmp/git name: content-from-git-1 +- mountPath: /stackable/log + name: log-volume - mountPath: /mnt/extra-volume name: extra-volume "#, @@ -692,7 +761,9 @@ volumeMounts: assert_eq!( r#"args: -- /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git,k1:v1,k2:v2,safe.directory:/safe-dir,k3:v3,k4:v4' --link=current --one-time=true --period=20s --ref=feat/git-sync --repo=https://github.com/stackabletech/repo3 --root=/tmp/git +- |- + mkdir --parents /stackable/log/git-sync-2-init && exec > >(tee /stackable/log/git-sync-2-init/container.stdout.log) 2> >(tee /stackable/log/git-sync-2-init/container.stderr.log >&2) + /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git,k1:v1,k2:v2,safe.directory:/safe-dir,k3:v3,k4:v4' --link=current --one-time=true --period=20s --ref=feat/git-sync --repo=https://github.com/stackabletech/repo3 --root=/tmp/git command: - /bin/bash - -x @@ -717,6 +788,8 @@ resources: volumeMounts: - mountPath: /tmp/git name: content-from-git-2 +- mountPath: /stackable/log + name: log-volume - mountPath: /mnt/extra-volume name: extra-volume "#, From 5bba49d4a28a1cf6c4b4dc3d768464aa37e54b34 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Wed, 14 May 2025 17:28:25 +0200 Subject: [PATCH 05/18] Use the type Url for the git-sync repository URL --- crates/stackable-operator/src/crd/git_sync/mod.rs | 3 ++- crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/stackable-operator/src/crd/git_sync/mod.rs b/crates/stackable-operator/src/crd/git_sync/mod.rs index b6b9013d..fbe4f3ca 100644 --- a/crates/stackable-operator/src/crd/git_sync/mod.rs +++ b/crates/stackable-operator/src/crd/git_sync/mod.rs @@ -4,6 +4,7 @@ use std::{collections::BTreeMap, path::PathBuf}; use schemars::{self, JsonSchema}; use serde::{Deserialize, Serialize}; +use url::Url; use crate::{time::Duration, versioned::versioned}; @@ -19,7 +20,7 @@ pub mod versioned { #[serde(rename_all = "camelCase")] pub struct GitSync { /// The git repository URL that will be cloned, for example: `https://github.com/stackabletech/airflow-operator`. - pub repo: String, + pub repo: Url, /// The branch to clone; defaults to `main`. /// diff --git a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index 711e3c85..90bae4f8 100644 --- a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -234,7 +234,7 @@ impl GitSyncResources { container_log_config: &ContainerLogConfig, ) -> String { let internal_args = [ - Some(("--repo".to_string(), git_sync.repo.to_owned())), + Some(("--repo".to_string(), git_sync.repo.as_str().to_owned())), Some(("--ref".to_string(), git_sync.branch.to_owned())), Some(("--depth".to_string(), git_sync.depth.to_string())), Some(( From e8ad04b49f8c561c2440dd000bcf116f5db2bd4c Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 16 May 2025 09:22:20 +0200 Subject: [PATCH 06/18] Extend the comment of the GitSync structure --- crates/stackable-operator/src/crd/git_sync/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-operator/src/crd/git_sync/mod.rs b/crates/stackable-operator/src/crd/git_sync/mod.rs index fbe4f3ca..5ed5e849 100644 --- a/crates/stackable-operator/src/crd/git_sync/mod.rs +++ b/crates/stackable-operator/src/crd/git_sync/mod.rs @@ -28,7 +28,7 @@ pub mod versioned { #[serde(default = "GitSync::default_branch")] pub branch: String, - /// Location in the Git repository containing the resource. + /// Location in the Git repository containing the resource; defaults to the root folder. /// /// It can optionally start with `/`, however, no trailing slash is recommended. /// An empty string (``) or slash (`/`) corresponds to the root folder in Git. From 2cf42ff7fe4a4516c718f6f289aab9da0b5b9c2f Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 16 May 2025 09:28:15 +0200 Subject: [PATCH 07/18] Improve a comment in the GitSync structure Co-authored-by: Techassi --- crates/stackable-operator/src/crd/git_sync/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/stackable-operator/src/crd/git_sync/mod.rs b/crates/stackable-operator/src/crd/git_sync/mod.rs index 5ed5e849..5a35b5ed 100644 --- a/crates/stackable-operator/src/crd/git_sync/mod.rs +++ b/crates/stackable-operator/src/crd/git_sync/mod.rs @@ -46,9 +46,12 @@ pub mod versioned { pub wait: Duration, /// The name of the Secret used to access the repository if it is not public. - /// This should include two fields: `user` and `password`. + /// + /// The referenced Secret must include two fields: `user` and `password`. /// The `password` field can either be an actual password (not recommended) or a GitHub token, - /// as described [here](https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual). + /// as described in the git-sync [documentation]. + /// + /// [documentation]: https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual pub credentials_secret: Option, /// A map of optional configuration settings that are listed in the [git-sync documentation](https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual). From 80c75f6be093ac2a780f819591156d095b4aead9 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 16 May 2025 09:30:09 +0200 Subject: [PATCH 08/18] Extend the comment of the GitSync structure --- crates/stackable-operator/src/crd/git_sync/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/stackable-operator/src/crd/git_sync/mod.rs b/crates/stackable-operator/src/crd/git_sync/mod.rs index 5ed5e849..09af12c9 100644 --- a/crates/stackable-operator/src/crd/git_sync/mod.rs +++ b/crates/stackable-operator/src/crd/git_sync/mod.rs @@ -51,8 +51,12 @@ pub mod versioned { /// as described [here](https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual). pub credentials_secret: Option, - /// A map of optional configuration settings that are listed in the [git-sync documentation](https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual). - /// Read the [git sync example](DOCS_BASE_URL_PLACEHOLDER/airflow/usage-guide/mounting-dags#_example). + /// A map of optional configuration settings that are listed in the git-sync [documentation]. + /// + /// Also read the git-sync [example] in our documentation. + /// + /// [documentation]: https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual + /// [example]: DOCS_BASE_URL_PLACEHOLDER/airflow/usage-guide/mounting-dags#_example #[serde(default)] pub git_sync_conf: BTreeMap, } From 7dc792698295932c2d29cb4e4cf4d027ac6a44a8 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 16 May 2025 09:34:07 +0200 Subject: [PATCH 09/18] Reformat the code Co-authored-by: Techassi --- crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index 90bae4f8..263c1a84 100644 --- a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -65,12 +65,16 @@ impl GitSync { pub struct GitSyncResources { /// GitSync containers with regular synchronizations pub git_sync_containers: Vec, + /// GitSync init containers with a one-time synchronizations pub git_sync_init_containers: Vec, + /// GitSync volumes containing the synchronized repository pub git_content_volumes: Vec, + /// Volume mounts for the GitSync volumes pub git_content_volume_mounts: Vec, + /// Absolute paths to the Git contents in the mounted volumes pub git_content_folders: Vec, } From 84aa733b46e8d5f95104e306bcaa8415046fdd46 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 16 May 2025 09:35:10 +0200 Subject: [PATCH 10/18] Improve the code style Co-authored-by: Techassi --- crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index 263c1a84..ab7a4de4 100644 --- a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -91,7 +91,7 @@ impl GitSyncResources { pub fn git_content_folders_as_string(&self) -> Vec { self.git_content_folders .iter() - .map(|path| path.to_str().expect("The path names of the git_content_folders are created as valid UTF-8 strings, so Path::to_str should not fail.").to_string()) + .map(|path| path.to_str().expect("The path names of the git_content_folders are created as valid UTF-8 strings, so Path::to_str should not fail.").to_owned()) .collect() } From d8a8b455fb04d165ce6ebb8dbe4de884b4f8475a Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 16 May 2025 09:57:17 +0200 Subject: [PATCH 11/18] Improve the code style Co-authored-by: Techassi --- crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index ab7a4de4..a74ba089 100644 --- a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -126,8 +126,8 @@ impl GitSyncResources { let mount_path = format!("{MOUNT_PATH_PREFIX}-{i}"); let git_sync_root_volume_mount = VolumeMount { - name: volume_name.to_owned(), - mount_path: GIT_SYNC_ROOT_DIR.to_string(), + name: volume_name.clone(), + mount_path: GIT_SYNC_ROOT_DIR.to_owned(), ..VolumeMount::default() }; From 8a13bbaa2cce4e0b8c48a0564070b474ef1e4b97 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 16 May 2025 09:57:59 +0200 Subject: [PATCH 12/18] Improve the code style Co-authored-by: Techassi --- crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index a74ba089..8b9b7185 100644 --- a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -161,7 +161,7 @@ impl GitSyncResources { container_log_config, )?; - let volume = VolumeBuilder::new(volume_name.to_owned()) + let volume = VolumeBuilder::new(volume_name.clone()) .empty_dir(EmptyDirVolumeSource::default()) .build(); From ea4e63f4fbba5256ce40d282cd3a0527d064fbc9 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 16 May 2025 09:59:01 +0200 Subject: [PATCH 13/18] Improve the code style Co-authored-by: Techassi --- crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index 8b9b7185..db05d32d 100644 --- a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -166,8 +166,8 @@ impl GitSyncResources { .build(); let git_content_volume_mount = VolumeMount { - name: volume_name.to_owned(), - mount_path: mount_path.to_owned(), + name: volume_name.clone(), + mount_path: mount_path.clone(), ..VolumeMount::default() }; From dccf07d649e55d6dc276e4984e138fc507f84c37 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 16 May 2025 10:54:40 +0200 Subject: [PATCH 14/18] Add always the git-sync parameter one-time --- .../src/crd/git_sync/v1alpha1_impl.rs | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index ab7a4de4..bdd27f82 100644 --- a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -237,21 +237,18 @@ impl GitSyncResources { one_time: bool, container_log_config: &ContainerLogConfig, ) -> String { - let internal_args = [ - Some(("--repo".to_string(), git_sync.repo.as_str().to_owned())), - Some(("--ref".to_string(), git_sync.branch.to_owned())), - Some(("--depth".to_string(), git_sync.depth.to_string())), - Some(( + let internal_args = BTreeMap::from([ + ("--repo".to_string(), git_sync.repo.as_str().to_owned()), + ("--ref".to_string(), git_sync.branch.to_owned()), + ("--depth".to_string(), git_sync.depth.to_string()), + ( "--period".to_string(), format!("{}s", git_sync.wait.as_secs()), - )), - Some(("--link".to_string(), GIT_SYNC_LINK.to_string())), - Some(("--root".to_string(), GIT_SYNC_ROOT_DIR.to_string())), - one_time.then_some(("--one-time".to_string(), "true".to_string())), - ] - .into_iter() - .flatten() - .collect::>(); + ), + ("--link".to_string(), GIT_SYNC_LINK.to_string()), + ("--root".to_string(), GIT_SYNC_ROOT_DIR.to_string()), + ("--one-time".to_string(), one_time.to_string()), + ]); let internal_git_config = [( GIT_SYNC_SAFE_DIR_OPTION.to_string(), @@ -505,7 +502,7 @@ mod tests { } prepare_signal_handlers - /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git' --link=current --period=20s --ref=main --repo=https://github.com/stackabletech/repo1 --root=/tmp/git & + /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git' --link=current --one-time=false --period=20s --ref=main --repo=https://github.com/stackabletech/repo1 --root=/tmp/git & wait_for_termination $! command: - /bin/bash @@ -574,7 +571,7 @@ volumeMounts: } prepare_signal_handlers - /stackable/git-sync --depth=3 --git-config='safe.directory:/tmp/git,http.sslCAInfo:/tmp/ca-cert/ca.crt' --link=current --period=60s --ref=trunk --repo=https://github.com/stackabletech/repo2 --rev=HEAD --root=/tmp/git & + /stackable/git-sync --depth=3 --git-config='safe.directory:/tmp/git,http.sslCAInfo:/tmp/ca-cert/ca.crt' --link=current --one-time=false --period=60s --ref=trunk --repo=https://github.com/stackabletech/repo2 --rev=HEAD --root=/tmp/git & wait_for_termination $! command: - /bin/bash @@ -648,7 +645,7 @@ volumeMounts: } prepare_signal_handlers - /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git,k1:v1,k2:v2,safe.directory:/safe-dir,k3:v3,k4:v4' --link=current --period=20s --ref=feat/git-sync --repo=https://github.com/stackabletech/repo3 --root=/tmp/git & + /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git,k1:v1,k2:v2,safe.directory:/safe-dir,k3:v3,k4:v4' --link=current --one-time=false --period=20s --ref=feat/git-sync --repo=https://github.com/stackabletech/repo3 --root=/tmp/git & wait_for_termination $! command: - /bin/bash From c69b2d2dfbbfb2eda06b23150bce075c921c2c38 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 16 May 2025 10:56:23 +0200 Subject: [PATCH 15/18] Improve the code style Co-authored-by: Techassi --- .../src/crd/git_sync/v1alpha1_impl.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index 711c6587..9148ad1c 100644 --- a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -250,12 +250,10 @@ impl GitSyncResources { ("--one-time".to_string(), one_time.to_string()), ]); - let internal_git_config = [( - GIT_SYNC_SAFE_DIR_OPTION.to_string(), - GIT_SYNC_ROOT_DIR.to_string(), - )] - .into_iter() - .collect::>(); + let internal_git_config = BTreeMap::from([( + GIT_SYNC_SAFE_DIR_OPTION.to_owned(), + GIT_SYNC_ROOT_DIR.to_owned(), + )]); let mut user_defined_args = BTreeMap::new(); // The key and value in Git configs are separated by a colon, but both From a2baa65d419a4dfaafbe38676c1c81093fa3cd79 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 16 May 2025 12:22:38 +0200 Subject: [PATCH 16/18] Improve the code style --- .../src/crd/git_sync/v1alpha1_impl.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index 9148ad1c..614a38ea 100644 --- a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -334,13 +334,17 @@ wait_for_termination $!" shell_script } - fn env_var_from_secret(var_name: &str, secret: &str, secret_key: &str) -> EnvVar { + fn env_var_from_secret( + var_name: impl Into, + secret: impl Into, + secret_key: impl Into, + ) -> EnvVar { EnvVar { - name: var_name.to_string(), + name: var_name.into(), value_from: Some(EnvVarSource { secret_key_ref: Some(SecretKeySelector { - name: secret.to_string(), - key: secret_key.to_string(), + name: secret.into(), + key: secret_key.into(), ..Default::default() }), ..Default::default() From 80d7b3e5a421c2cd171abce68aa41bab1b6143c7 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Tue, 20 May 2025 10:47:34 +0200 Subject: [PATCH 17/18] Disallow variants of the option `--git-config` --- .../src/crd/git_sync/v1alpha1_impl.rs | 71 ++++++++++--------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs index 614a38ea..87b62179 100644 --- a/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs +++ b/crates/stackable-operator/src/crd/git_sync/v1alpha1_impl.rs @@ -255,33 +255,26 @@ impl GitSyncResources { GIT_SYNC_ROOT_DIR.to_owned(), )]); - let mut user_defined_args = BTreeMap::new(); - // The key and value in Git configs are separated by a colon, but both - // can contain either escaped colons or unescaped colons if enclosed in - // quotes. To avoid parsing, just a vector is used instead of a map. - let mut user_defined_git_configs = Vec::new(); - - for (key, value) in &git_sync.git_sync_conf { - // The initial git-sync implementation in the airflow-operator - // (https://github.com/stackabletech/airflow-operator/pull/381) - // used this condition to find Git configs. It is also used here - // for backwards-compatibility: - if key.to_lowercase().ends_with("-git-config") { - // Roughly check if the user defined config contains an - // internally defined config and emit a warning in case. - if internal_git_config.keys().any(|key| value.contains(key)) { - tracing::warn!("Config option {value:?} contains a value for {GIT_SYNC_SAFE_DIR_OPTION} that overrides - the value of this operator. Git-sync functionality will probably not work as expected!"); - } - user_defined_git_configs.push(value.to_owned()); - } else if internal_args.contains_key(key) { - tracing::warn!( - "The git-sync option {key:?} is already internally defined and will be ignored." - ); - } else { - // The user-defined arguments are not validated. - user_defined_args.insert(key.to_owned(), value.to_owned()); - } + let mut git_sync_config = git_sync.git_sync_conf.clone(); + + // The key and value in Git configs are separated by a colon, but both can contain either + // escaped colons or unescaped colons if enclosed in quotes. To avoid parsing, just a String + // is used instead of a key-value pair. + let user_defined_git_config = git_sync_config.remove("--git-config"); + + if let Some(git_config) = &user_defined_git_config { + // Roughly check if the user defined Git config contains an internally defined config + // and emit a warning in case. + internal_git_config + .keys() + .filter(|key| git_config.contains(*key)) + .for_each(|key| { + tracing::warn!( + "The Git config option {git_config:?} contains a value for {key} that \ + overrides the value of this operator. Git-sync functionality will probably \ + not work as expected!" + ); + }); } // The user-defined Git config is just appended. @@ -289,10 +282,23 @@ impl GitSyncResources { let git_config = internal_git_config .into_iter() .map(|(key, value)| format!("{key}:{value}")) - .chain(user_defined_git_configs) + .chain(user_defined_git_config) .collect::>() .join(","); + let mut user_defined_args = BTreeMap::new(); + + for (key, value) in git_sync_config { + if internal_args.contains_key(&key) { + tracing::warn!( + "The git-sync option {key:?} is already internally defined and will be ignored." + ); + } else { + // The user-defined arguments are not validated. + user_defined_args.insert(key, value); + } + } + let mut args = internal_args; args.extend(user_defined_args); args.insert("--git-config".to_string(), format!("'{git_config}'")); @@ -425,11 +431,8 @@ mod tests { --ref: internal option which should be ignored --repo: internal option which should be ignored --root: internal option which should be ignored - --GIT-CONFIG: k1:v1 # safe.directory should be accepted but a warning will be emitted - --git-config: k2:v2,safe.directory:/safe-dir - -GIT-CONFIG: k3:v3 - -git-config: k4:v4 + --git-config: key:value,safe.directory:/safe-dir "#; let deserializer = serde_yaml::Deserializer::from_str(git_sync_spec); @@ -647,7 +650,7 @@ volumeMounts: } prepare_signal_handlers - /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git,k1:v1,k2:v2,safe.directory:/safe-dir,k3:v3,k4:v4' --link=current --one-time=false --period=20s --ref=feat/git-sync --repo=https://github.com/stackabletech/repo3 --root=/tmp/git & + /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git,key:value,safe.directory:/safe-dir' --link=current --one-time=false --period=20s --ref=feat/git-sync --repo=https://github.com/stackabletech/repo3 --root=/tmp/git & wait_for_termination $! command: - /bin/bash @@ -766,7 +769,7 @@ volumeMounts: r#"args: - |- mkdir --parents /stackable/log/git-sync-2-init && exec > >(tee /stackable/log/git-sync-2-init/container.stdout.log) 2> >(tee /stackable/log/git-sync-2-init/container.stderr.log >&2) - /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git,k1:v1,k2:v2,safe.directory:/safe-dir,k3:v3,k4:v4' --link=current --one-time=true --period=20s --ref=feat/git-sync --repo=https://github.com/stackabletech/repo3 --root=/tmp/git + /stackable/git-sync --depth=1 --git-config='safe.directory:/tmp/git,key:value,safe.directory:/safe-dir' --link=current --one-time=true --period=20s --ref=feat/git-sync --repo=https://github.com/stackabletech/repo3 --root=/tmp/git command: - /bin/bash - -x From 19a0ac9145aeb22aa4544b824b31de80f47ef674 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Tue, 20 May 2025 15:20:55 +0200 Subject: [PATCH 18/18] Extend a comment in the GitSync structure --- crates/stackable-operator/src/crd/git_sync/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-operator/src/crd/git_sync/mod.rs b/crates/stackable-operator/src/crd/git_sync/mod.rs index a4cc13bb..bc63e2a1 100644 --- a/crates/stackable-operator/src/crd/git_sync/mod.rs +++ b/crates/stackable-operator/src/crd/git_sync/mod.rs @@ -56,7 +56,7 @@ pub mod versioned { /// A map of optional configuration settings that are listed in the git-sync [documentation]. /// - /// Also read the git-sync [example] in our documentation. + /// Also read the git-sync [example] in our documentation. These settings are not verified. /// /// [documentation]: https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual /// [example]: DOCS_BASE_URL_PLACEHOLDER/airflow/usage-guide/mounting-dags#_example