fix: Pass gitsync credentials through properly

CHANGELOG.md

@@ -10,7 +10,12 @@
   - `volumes`
   - `volumeMounts`
 
+### Fixed
+
+- Pass gitsync credentials through properly and use a fine-grained access token ([#489]).
+
 [#488]: https://github.com/stackabletech/airflow-operator/pull/488
+[#489]: https://github.com/stackabletech/airflow-operator/pull/489
 
 ## [24.7.0] - 2024-07-24
 

deploy/helm/airflow-operator/crds/crds.yaml

@@ -497,7 +497,7 @@ spec:
                             nullable: true
                             type: string
                           credentialsSecret:
-                            description: '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/v3.6.4#flags-which-configure-authentication).'
+                            description: '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).'
                             nullable: true
                             type: string
                           depth:
@@ -513,7 +513,7 @@ spec:
                           gitSyncConf:
                             additionalProperties:
                               type: string
-                            description: A map of optional configuration settings that are listed in the [git-sync documentation](https://github.com/kubernetes/git-sync/tree/v3.6.4#primary-flags). Read the [git sync example](https://docs.stackable.tech/home/nightly/airflow/usage-guide/mounting-dags#_example).
+                            description: 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](https://docs.stackable.tech/home/nightly/airflow/usage-guide/mounting-dags#_example).
                             nullable: true
                             type: object
                           repo:

rust/crd/src/git_sync.rs

@@ -23,9 +23,9 @@ pub struct GitSync {
     /// 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/v3.6.4#flags-which-configure-authentication).
+    /// as described [here](https://github.com/kubernetes/git-sync/tree/v4.2.4?tab=readme-ov-file#manual).
     pub credentials_secret: Option<String>,
-    /// A map of optional configuration settings that are listed in the [git-sync documentation](https://github.com/kubernetes/git-sync/tree/v3.6.4#primary-flags).
+    /// 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).
     pub git_sync_conf: Option<BTreeMap<String, String>>,
 }

rust/crd/src/lib.rs

@@ -709,14 +709,6 @@ impl Configuration for AirflowConfigFragment {
             AirflowConfig::CREDENTIALS_SECRET_PROPERTY.to_string(),
             Some(cluster.spec.cluster_config.credentials_secret.clone()),
         );
-        if let Some(git_sync) = &cluster.git_sync() {
-            if let Some(credentials_secret) = &git_sync.credentials_secret {
-                env.insert(
-                    AirflowConfig::GIT_CREDENTIALS_SECRET_PROPERTY.to_string(),
-                    Some(credentials_secret.to_string()),
-                );
-            }
-        }
         Ok(env)
     }
 

rust/operator-binary/src/airflow_controller.rs

@@ -749,7 +749,7 @@ fn build_rolegroup_metadata(
 ) -> Result<ObjectMeta, Error> {
     let metadata = ObjectMetaBuilder::new()
         .name_and_namespace(airflow)
-        .name(&rolegroup.object_name())
+        .name(rolegroup.object_name())
         .ownerreference_from_resource(airflow, None, Some(true))
         .context(ObjectMissingMetadataForOwnerRefSnafu)?
         .with_recommended_labels(build_recommended_labels(
@@ -924,7 +924,7 @@ fn build_server_rolegroup_statefulset(
             &gitsync,
             false,
             &format!("{}-{}", GIT_SYNC_NAME, 1),
-            build_gitsync_statefulset_envs(rolegroup_config),
+            build_gitsync_statefulset_envs(rolegroup_config, &gitsync.credentials_secret),
             airflow.volume_mounts(),
         )?;
 
@@ -942,7 +942,7 @@ fn build_server_rolegroup_statefulset(
                 &gitsync,
                 true,
                 &format!("{}-{}", GIT_SYNC_NAME, 0),
-                build_gitsync_statefulset_envs(rolegroup_config),
+                build_gitsync_statefulset_envs(rolegroup_config, &gitsync.credentials_secret),
                 airflow.volume_mounts(),
             )?;
             // If the DAG is modularized we may encounter a timing issue whereby the celery worker has started
@@ -1112,7 +1112,7 @@ fn build_executor_template_config_map(
             &gitsync,
             true,
             &format!("{}-{}", GIT_SYNC_NAME, 0),
-            build_gitsync_template(env_overrides),
+            build_gitsync_template(env_overrides, &gitsync.credentials_secret),
             airflow.volume_mounts(),
         )?;
         pb.add_volume(

rust/operator-binary/src/env_vars.rs

@@ -295,12 +295,17 @@ fn static_envs(airflow: &AirflowCluster) -> BTreeMap<String, EnvVar> {
 }
 
 /// Return environment variables to be applied to the gitsync container in the statefulset for the scheduler,
-/// webserver (and worker, for clusters utilizing `celeryExecutor`).
+/// webserver (and worker, for clusters utilizing `celeryExecutor`). N.B. the git credentials-secret is passed
+/// explicitly here: it is no longer added to the role config (lib/compute_env) as the kubenertes executor wraps
+/// `CommonConfiguration<ExecutorConfigFragment>` that is not linked to a role.
 pub fn build_gitsync_statefulset_envs(
     rolegroup_config: &HashMap<PropertyNameKind, BTreeMap<String, String>>,
+    git_credentials_secret: &Option<String>,
 ) -> Vec<EnvVar> {
     let mut env: BTreeMap<String, EnvVar> = BTreeMap::new();
 
+    add_gitsync_credentials(git_credentials_secret, &mut env);
+
     if let Some(git_env) = rolegroup_config.get(&PropertyNameKind::Env) {
         for (k, v) in git_env.iter() {
             gitsync_vars_map(k, &mut env, v);
@@ -311,6 +316,22 @@ pub fn build_gitsync_statefulset_envs(
     transform_map_to_vec(env)
 }
 
+fn add_gitsync_credentials(
+    git_credentials_secret: &Option<String>,
+    env: &mut BTreeMap<String, EnvVar>,
+) {
+    if let Some(git_credentials_secret) = &git_credentials_secret {
+        env.insert(
+            GITSYNC_USERNAME.to_string(),
+            env_var_from_secret(GITSYNC_USERNAME, git_credentials_secret, "user"),
+        );
+        env.insert(
+            GITSYNC_PASSWORD.to_string(),
+            env_var_from_secret(GITSYNC_PASSWORD, git_credentials_secret, "password"),
+        );
+    }
+}
+
 /// Return environment variables to be applied to the configuration map used in conjunction with
 /// the `kubernetesExecutor` worker.
 pub fn build_airflow_template_envs(
@@ -401,9 +422,14 @@ pub fn build_airflow_template_envs(
 
 /// Return environment variables to be applied to the configuration map used in conjunction with
 /// the `kubernetesExecutor` worker: applied to the gitsync `initContainer`.
-pub fn build_gitsync_template(env_overrides: &HashMap<String, String>) -> Vec<EnvVar> {
+pub fn build_gitsync_template(
+    env_overrides: &HashMap<String, String>,
+    git_credentials_secret: &Option<String>,
+) -> Vec<EnvVar> {
     let mut env: BTreeMap<String, EnvVar> = BTreeMap::new();
 
+    add_gitsync_credentials(git_credentials_secret, &mut env);
+
     for (k, v) in env_overrides.iter().collect::<BTreeMap<_, _>>() {
         gitsync_vars_map(k, &mut env, v);
     }
@@ -413,25 +439,14 @@ pub fn build_gitsync_template(env_overrides: &HashMap<String, String>) -> Vec<En
 }
 
 fn gitsync_vars_map(k: &String, env: &mut BTreeMap<String, EnvVar>, v: &String) {
-    if k.eq_ignore_ascii_case(AirflowConfig::GIT_CREDENTIALS_SECRET_PROPERTY) {
-        env.insert(
-            GITSYNC_USERNAME.to_string(),
-            env_var_from_secret(GITSYNC_USERNAME, k, "user"),
-        );
-        env.insert(
-            GITSYNC_PASSWORD.to_string(),
-            env_var_from_secret(GITSYNC_PASSWORD, k, "password"),
-        );
-    } else {
-        env.insert(
-            k.to_string(),
-            EnvVar {
-                name: k.to_string(),
-                value: Some(v.to_string()),
-                ..Default::default()
-            },
-        );
-    }
+    env.insert(
+        k.to_string(),
+        EnvVar {
+            name: k.to_string(),
+            value: Some(v.to_string()),
+            ..Default::default()
+        },
+    );
 }
 
 // Internally the environment variable collection uses a map so that overrides can actually

tests/templates/kuttl/mount-dags-gitsync/30-install-airflow-cluster.yaml.j2

@@ -23,6 +23,18 @@ stringData:
 {% endif %}
 ---
 apiVersion: v1
+kind: Secret
+metadata:
+  name: git-credentials
+type: Opaque
+data:
+  # This is a fine-grained access token for the owner of the repo (stackable-airflow/dags) which has read only access
+  # to *only* this repo. Contact github users @razvan or @adwk67 for details.
+  # It is valid for one year and will expire on 12.08.2025.
+  user: c3RhY2thYmxlLWFpcmZsb3c=
+  password: Z2l0aHViX3BhdF8xMUJLUURCRVkwSFFPMkpjRlJ1ekxLX0FLTGhqT0xvZ0JQMzZnTENFUFo0RUNETTM2TndjdTJDQUJvVVFueFBiVGhDTTZKQkhJNFFIZVdJb1dECg==
+---
+apiVersion: v1
 kind: ConfigMap
 metadata:
   name: test-cm-gitsync
@@ -49,13 +61,14 @@ spec:
 {% endif %}
     credentialsSecret: test-airflow-credentials
     dagsGitSync:
-      - repo: https://github.com/stackabletech/airflow-operator
+      - repo: https://github.com/stackable-airflow/dags
         branch: "main"
         wait: 20
+        credentialsSecret: git-credentials
         gitSyncConf:
           # supply some config to check that safe.directory is correctly set
           --git-config: http.sslVerify:false
-        gitFolder: "tests/templates/kuttl/mount-dags-gitsync/dags"
+        gitFolder: "mount-dags-gitsync/dags"
     volumeMounts:
       - name: test-cm-gitsync
         mountPath: /tmp/test.txt

tests/templates/kuttl/mount-dags-gitsync/31-assert.yaml.j2

@@ -10,10 +10,16 @@ commands:
   - script: kubectl -n $NAMESPACE get cm airflow-executor-pod-template -o json | jq -r '.data."airflow_executor_pod_template.yaml"'  | grep "test-cm-gitsync" | wc -l | grep 4
   # will expect 2 (two containers, base and gitsync)
   - script: kubectl -n $NAMESPACE get cm airflow-executor-pod-template -o json | jq -r '.data."airflow_executor_pod_template.yaml"'  | grep "AIRFLOW_TEST_VAR" | wc -l | grep 2
+  # will expect 1 (one container, gitsync)
+  - script: kubectl -n $NAMESPACE get cm airflow-executor-pod-template -o json | jq -r '.data."airflow_executor_pod_template.yaml"'  | grep "GITSYNC_USERNAME" | wc -l | grep 1
+  - script: kubectl -n $NAMESPACE get cm airflow-executor-pod-template -o json | jq -r '.data."airflow_executor_pod_template.yaml"'  | grep "GITSYNC_PASSWORD" | wc -l | grep 1
 {% else %}
   # check that the statefulset contains mounts and envs
   # will expect 6 (2 from from the volume declaration + mounts to 3 containers, base and 2 gitsyncs, plus configmap restarter)
   - script: kubectl -n $NAMESPACE get sts airflow-worker-default -o json | grep "test-cm-gitsync" | wc -l | grep 6
   # will expect 3 (two containers, base and gitsync-1, and one initContainer gitsync-0)
   - script: kubectl -n $NAMESPACE get sts airflow-worker-default -o json | grep "AIRFLOW_TEST_VAR" | wc -l | grep 3
+  # will expect 2 (one container, gitsync-1, and one initContainer gitsync-0)
+  - script: kubectl -n $NAMESPACE get sts airflow-worker-default -o json | grep "GITSYNC_USERNAME" | wc -l | grep 2
+  - script: kubectl -n $NAMESPACE get sts airflow-worker-default -o json | grep "GITSYNC_PASSWORD" | wc -l | grep 2
 {% endif %}