review feedback

adwk67 · adwk67 · commit d83dfa01ecae · 2025-06-23T10:52:59.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -30,6 +30,7 @@
   - The `runAsUser` and `runAsGroup` fields will not be set anymore by the operator
   - The defaults from the docker images itself will now apply, which will be different from 1000/0 going forward
   - This is marked as breaking because tools and policies might exist, which require these fields to be set
+- Changed listener class to be role-only ([#645]).
 
 ### Fixed
 
@@ -48,6 +49,7 @@
 [#625]: https://github.com/stackabletech/airflow-operator/pull/625
 [#630]: https://github.com/stackabletech/airflow-operator/pull/630
 [#636]: https://github.com/stackabletech/airflow-operator/pull/636
+[#645]: https://github.com/stackabletech/airflow-operator/pull/645
 
 ## [25.3.0] - 2025-03-21
 
diff --git a/docs/modules/airflow/examples/example-airflow-dags-configmap.yaml b/docs/modules/airflow/examples/example-airflow-dags-configmap.yaml
@@ -19,7 +19,7 @@ spec:
         mountPath: /dags/test_airflow_dag.py # <6>
         subPath: test_airflow_dag.py # <7>
   webservers:
-    config:
+    roleConfig:
       listenerClass: external-unstable
     roleGroups:
       default:
diff --git a/docs/modules/airflow/examples/example-airflow-incluster.yaml b/docs/modules/airflow/examples/example-airflow-incluster.yaml
@@ -11,7 +11,7 @@ spec:
     exposeConfig: false
     credentialsSecret: simple-airflow-credentials
   webservers:
-    config:
+    roleConfig:
       listenerClass: external-unstable
     roleGroups:
       default:
diff --git a/docs/modules/airflow/examples/example-airflow-kubernetes-executor-s3-logging.yaml b/docs/modules/airflow/examples/example-airflow-kubernetes-executor-s3-logging.yaml
@@ -7,7 +7,7 @@ spec:
     productVersion: 2.10.5
   clusterConfig: {}
   webservers:
-    config:
+    roleConfig:
       listenerClass: external-unstable
     envOverrides: &envOverrides
       AIRFLOW__LOGGING__REMOTE_LOGGING: "True"
diff --git a/docs/modules/airflow/examples/example-airflow-kubernetes-executor-s3-xcom.yaml b/docs/modules/airflow/examples/example-airflow-kubernetes-executor-s3-xcom.yaml
@@ -7,7 +7,7 @@ spec:
     productVersion: 2.10.5
   clusterConfig: {}
   webservers:
-    config:
+    roleConfig:
       listenerClass: external-unstable
     envOverrides: &envOverrides
       AIRFLOW__CORE__XCOM_BACKEND: airflow.providers.common.io.xcom.backend.XComObjectStorageBackend
diff --git a/docs/modules/airflow/examples/getting_started/code/airflow.yaml b/docs/modules/airflow/examples/getting_started/code/airflow.yaml
@@ -11,7 +11,7 @@ spec:
     exposeConfig: false
     credentialsSecret: simple-airflow-credentials
   webservers:
-    config:
+    roleConfig:
       listenerClass: external-unstable
     roleGroups:
       default:
diff --git a/docs/modules/airflow/pages/usage-guide/listenerclass.adoc b/docs/modules/airflow/pages/usage-guide/listenerclass.adoc
@@ -2,7 +2,7 @@
 :description: Configure Airflow service exposure with ListenerClasses: cluster-internal, external-unstable, or external-stable.
 
 The operator deploys a xref:listener-operator:listener.adoc[Listener] for the Webserver pod.
-The listener defaults to only being accessible from within the Kubernetes cluster, but this can be changed by setting `.spec.webservers.config.listenerClass`:
+The listener defaults to only being accessible from within the Kubernetes cluster, but this can be changed by setting `.spec.webservers.roleConfig.listenerClass`:
 
 [source,yaml]
 ----
diff --git a/examples/simple-airflow-cluster-dags-cmap.yaml b/examples/simple-airflow-cluster-dags-cmap.yaml
@@ -96,7 +96,7 @@ spec:
         mountPath: /dags/test_airflow_dag.py
         subPath: test_airflow_dag.py
   webservers:
-    config:
+    roleConfig:
       listenerClass: external-unstable
     roleGroups:
       default:
diff --git a/examples/simple-airflow-cluster-ldap-insecure-tls.yaml b/examples/simple-airflow-cluster-ldap-insecure-tls.yaml
@@ -157,7 +157,7 @@ spec:
       - authenticationClass: airflow-with-ldap-insecure-tls-ldap
         userRegistrationRole: Admin
   webservers:
-    config:
+    roleConfig:
       listenerClass: external-unstable
     roleGroups:
       default:
diff --git a/examples/simple-airflow-cluster-ldap.yaml b/examples/simple-airflow-cluster-ldap.yaml
@@ -155,7 +155,7 @@ spec:
       - authenticationClass: airflow-with-ldap-server-veri-tls-ldap
         userRegistrationRole: Admin
   webservers:
-    config:
+    roleConfig:
       listenerClass: external-unstable
     roleGroups:
       default:
diff --git a/examples/simple-airflow-cluster.yaml b/examples/simple-airflow-cluster.yaml
@@ -28,7 +28,7 @@ spec:
     exposeConfig: false
     credentialsSecret: simple-airflow-credentials
   webservers:
-    config:
+    roleConfig:
       listenerClass: external-unstable
     roleGroups:
       default:
diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs
@@ -45,13 +45,16 @@ use stackable_operator::{
 };
 use strum::{Display, EnumIter, EnumString, IntoEnumIterator};
 
-use crate::crd::{
-    affinity::{get_affinity, get_executor_affinity},
-    authentication::{
-        AirflowAuthenticationClassResolved, AirflowClientAuthenticationDetails,
-        AirflowClientAuthenticationDetailsResolved,
+use crate::{
+    crd::{
+        affinity::{get_affinity, get_executor_affinity},
+        authentication::{
+            AirflowAuthenticationClassResolved, AirflowClientAuthenticationDetails,
+            AirflowClientAuthenticationDetailsResolved,
+        },
+        v1alpha1::WebserverRoleConfig,
     },
-    v1alpha1::WebserverRoleConfig,
+    util::role_service_name,
 };
 
 pub mod affinity;
@@ -310,10 +313,7 @@ impl v1alpha1::AirflowCluster {
     /// is needed for that role.
     pub fn group_listener_name(&self, role: &AirflowRole) -> Option<String> {
         match role {
-            AirflowRole::Webserver => Some(format!(
-                "{cluster_name}-{role}",
-                cluster_name = self.name_any()
-            )),
+            AirflowRole::Webserver => Some(role_service_name(&self.name_any(), &role.to_string())),
             AirflowRole::Scheduler | AirflowRole::Worker => None,
         }
     }
diff --git a/rust/operator-binary/src/env_vars.rs b/rust/operator-binary/src/env_vars.rs
@@ -23,7 +23,7 @@ use crate::{
         authorization::AirflowAuthorizationResolved,
         v1alpha1,
     },
-    util::env_var_from_secret,
+    util::{env_var_from_secret, role_service_name},
 };
 
 const AIRFLOW_CORE_AUTH_MANAGER: &str = "AIRFLOW__CORE__AUTH_MANAGER";
@@ -576,7 +576,7 @@ fn execution_server_env_vars(airflow: &v1alpha1::AirflowCluster) -> BTreeMap<Str
         // are multiple ones). Parse the list of webservers in a deterministic
         // way by iterating over a BTree map rather than the HashMap.
         if airflow.spec.webservers.as_ref().is_some() {
-            let webserver = format!("{name}-webserver", name = name,);
+            let webserver = role_service_name(name, "webserver");
             tracing::debug!("Webserver set [{webserver}]");
             // These settings are new in 3.x and will have no affect with earlier versions.
             env.insert(
diff --git a/rust/operator-binary/src/util.rs b/rust/operator-binary/src/util.rs
@@ -14,3 +14,7 @@ pub fn env_var_from_secret(var_name: &str, secret: &str, secret_key: &str) -> En
         ..Default::default()
     }
 }
+
+pub fn role_service_name(name: &str, role: &str) -> String {
+    format!("{name}-{role}")
+}
diff --git a/tests/templates/kuttl/commons/health.py b/tests/templates/kuttl/commons/health.py
@@ -14,9 +14,6 @@
     )
 
     parser = argparse.ArgumentParser(description="Health check script")
-    parser.add_argument(
-        "--role-group", type=str, default="default", help="Role group to check"
-    )
     parser.add_argument("--airflow-version", type=str, help="Airflow version")
     opts = parser.parse_args()
 
