fix: Fixed various issues in the CRD structure (#353)

* fix: Fixed various issues in the CRD structure * changelog * fix tests
stackabletech · Dec 5, 2023 · d14a62d · d14a62d
1 parent c038e9e
commit d14a62d
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- BREAKING: Fixed various issues in the CRD structure. `clusterConfig.credentialsSecret` is now mandatory ([#353]).
+
+[#353]: https://github.com/stackabletech/airflow-operator/pull/353
+
 ## [23.11.0] - 2023-11-24
 
 ### Added

diff --git a/deploy/helm/airflow-operator/crds/crds.yaml b/deploy/helm/airflow-operator/crds/crds.yaml
@@ -6930,11 +6930,6 @@ spec:
                     - roleGroups
                   type: object
                 clusterConfig:
-                  default:
-                    authentication: []
-                    credentialsSecret: ''
-                    dagsGitSync: []
-                    listenerClass: cluster-internal
                   description: Global cluster configuration that applies to all roles and role groups
                   properties:
                     authentication:
@@ -7000,7 +6995,7 @@ spec:
                         type: object
                       type: array
                     exposeConfig:
-                      nullable: true
+                      default: false
                       type: boolean
                     listenerClass:
                       default: cluster-internal
@@ -7020,7 +7015,7 @@ spec:
                         - external-stable
                       type: string
                     loadExamples:
-                      nullable: true
+                      default: false
                       type: boolean
                     vectorAggregatorConfigMapName:
                       description: Name of the Vector aggregator discovery ConfigMap. It must contain the key `ADDRESS` with the address of the Vector aggregator.
@@ -24940,6 +24935,7 @@ spec:
                     - roleGroups
                   type: object
               required:
+                - clusterConfig
                 - image
               type: object
             status:

diff --git a/rust/crd/src/affinity.rs b/rust/crd/src/affinity.rs
@@ -63,9 +63,8 @@ mod tests {
         spec:
           image:
             productVersion: 2.7.2
-          loadExamples: true
-          exposeConfig: false
-          credentialsSecret: simple-airflow-credentials
+          clusterConfig:
+            credentialsSecret: airflow-credentials
           webservers:
             roleGroups:
               default:
@@ -160,9 +159,8 @@ mod tests {
         spec:
           image:
             productVersion: 2.7.2
-          loadExamples: true
-          exposeConfig: false
-          credentialsSecret: simple-airflow-credentials
+          clusterConfig:
+            credentialsSecret: airflow-credentials
           webservers:
             roleGroups:
               default:

diff --git a/rust/crd/src/authentication.rs b/rust/crd/src/authentication.rs
@@ -39,7 +39,7 @@ pub struct AirflowAuthenticationConfigResolved {
     pub sync_roles_at: FlaskRolesSyncMoment,
 }
 
-#[derive(Clone, Debug, Default, Deserialize, Eq, JsonSchema, PartialEq, Serialize)]
+#[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct AirflowAuthentication {
     /// The Airflow authentication settings.

diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs
@@ -151,32 +151,41 @@ impl FlaskAppConfigOptions for AirflowConfigOptions {
 pub struct AirflowClusterSpec {
     /// The Airflow image to use
     pub image: ProductImage,
+
     /// Global cluster configuration that applies to all roles and role groups
-    #[serde(default)]
     pub cluster_config: AirflowClusterConfig,
+
     /// Cluster operations like pause reconciliation or cluster stop.
     #[serde(default)]
     pub cluster_operation: ClusterOperation,
+
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub webservers: Option<Role<AirflowConfigFragment>>,
+
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub schedulers: Option<Role<AirflowConfigFragment>>,
+
     #[serde(flatten)]
     pub executor: AirflowExecutor,
 }
 
-#[derive(Clone, Deserialize, Default, Debug, JsonSchema, PartialEq, Serialize)]
+#[derive(Clone, Deserialize, Debug, JsonSchema, PartialEq, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct AirflowClusterConfig {
     #[serde(flatten)]
     pub authentication: AirflowAuthentication,
+
     pub credentials_secret: String,
+
     #[serde(default)]
     pub dags_git_sync: Vec<GitSync>,
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub expose_config: Option<bool>,
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub load_examples: Option<bool>,
+
+    #[serde(default)]
+    pub expose_config: bool,
+
+    #[serde(default)]
+    pub load_examples: bool,
+
     /// This field controls which type of Service the Operator creates for this AirflowCluster:
     ///
     /// * cluster-internal: Use a ClusterIP service
@@ -190,12 +199,15 @@ pub struct AirflowClusterConfig {
     /// will be used to expose the service, and ListenerClass names will stay the same, allowing for a non-breaking change.
     #[serde(default)]
     pub listener_class: CurrentlySupportedListenerClasses,
+
     /// Name of the Vector aggregator discovery ConfigMap.
     /// It must contain the key `ADDRESS` with the address of the Vector aggregator.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub vector_aggregator_config_map_name: Option<String>,
+
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub volumes: Option<Vec<Volume>>,
+
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub volume_mounts: Option<Vec<VolumeMount>>,
 }
@@ -207,8 +219,10 @@ pub enum CurrentlySupportedListenerClasses {
     #[default]
     #[serde(rename = "cluster-internal")]
     ClusterInternal,
+
     #[serde(rename = "external-unstable")]
     ExternalUnstable,
+
     #[serde(rename = "external-stable")]
     ExternalStable,
 }
@@ -263,6 +277,7 @@ pub struct Connections {
 pub enum AirflowRole {
     #[strum(serialize = "webserver")]
     Webserver,
+
     #[strum(serialize = "scheduler")]
     Scheduler,
     #[strum(serialize = "worker")]
@@ -342,6 +357,7 @@ pub enum AirflowExecutor {
         #[serde(flatten)]
         config: Role<AirflowConfigFragment>,
     },
+
     #[serde(rename = "kubernetesExecutors")]
     KubernetesExecutor {
         #[serde(flatten)]
@@ -726,16 +742,6 @@ pub fn build_recommended_labels<'a, T>(
     }
 }
 
-/// A reference to a [`AirflowCluster`]
-#[derive(Clone, Default, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)]
-#[serde(rename_all = "camelCase")]
-pub struct AirflowClusterRef {
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub name: Option<String>,
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    pub namespace: Option<String>,
-}
-
 #[cfg(test)]
 mod tests {
     use crate::AirflowCluster;
@@ -777,7 +783,7 @@ mod tests {
         assert_eq!("2.7.2", &resolved_airflow_image.product_version);
 
         assert_eq!("KubernetesExecutor", cluster.spec.executor.to_string());
-        assert!(cluster.spec.cluster_config.load_examples.unwrap_or(false));
-        assert!(cluster.spec.cluster_config.expose_config.unwrap_or(false));
+        assert!(cluster.spec.cluster_config.load_examples);
+        assert!(cluster.spec.cluster_config.expose_config);
     }
 }
diff --git a/rust/operator-binary/src/airflow_controller.rs b/rust/operator-binary/src/airflow_controller.rs
@@ -1157,7 +1157,7 @@ fn build_mapped_envs(
             })
         }
     }
-    if let Some(true) = airflow.spec.cluster_config.load_examples {
+    if airflow.spec.cluster_config.load_examples {
         env.push(EnvVar {
             name: "AIRFLOW__CORE__LOAD_EXAMPLES".into(),
             value: Some("True".into()),
@@ -1171,7 +1171,7 @@ fn build_mapped_envs(
         })
     }
 
-    if let Some(true) = airflow.spec.cluster_config.expose_config {
+    if airflow.spec.cluster_config.expose_config {
         env.push(EnvVar {
             name: "AIRFLOW__WEBSERVER__EXPOSE_CONFIG".into(),
             value: Some("True".into()),