Merge branch 'main' into feat/crd-versioning-merged-crds

Author: Techassi

.github/workflows/pr_pre-commit.yaml

@@ -7,17 +7,38 @@ on:
 env:
   CARGO_TERM_COLOR: always
   RUST_TOOLCHAIN_VERSION: "1.80.1"
+  HADOLINT_VERSION: "v1.17.6"
 
 jobs:
   pre-commit:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+        with:
+          fetch-depth: 0
       - uses: actions/setup-python@82c7e631bb3cdc910f68e0081d67478d79c6982d # v5.1.0
         with:
           python-version: '3.12'
       - uses: dtolnay/rust-toolchain@master
         with:
           toolchain: ${{ env.RUST_TOOLCHAIN_VERSION }}
           components: rustfmt,clippy
+      - name: Setup Hadolint
+        shell: bash
+        run: |
+          set -euo pipefail
+
+          LOCATION_DIR="$HOME/.local/bin"
+          LOCATION_BIN="$LOCATION_DIR/hadolint"
+
+          SYSTEM=$(uname -s)
+          ARCH=$(uname -m)
+
+          mkdir -p "$LOCATION_DIR"
+          curl -sL -o "${LOCATION_BIN}" "https://github.com/hadolint/hadolint/releases/download/${{ env.HADOLINT_VERSION }}/hadolint-$SYSTEM-$ARCH"
+          chmod 700 "${LOCATION_BIN}"
+
+          echo "$LOCATION_DIR" >> "$GITHUB_PATH"
       - uses: pre-commit/action@2c7b3805fd2a0fd8c1884dcaebf91fc102a13ecd # v3.0.1
+        with:
+          extra_args: "--from-ref ${{ github.event.pull_request.base.sha }} --to-ref ${{ github.event.pull_request.head.sha }}"

.github/workflows/reviewdog.yaml

@@ -1,9 +1,10 @@
 ---
-# See https://pre-commit.com for more information
-# See https://pre-commit.com/hooks.html for more hooks
+default_language_version:
+  node: system
+
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.2.0
+    rev: 2c9f875913ee60ca25ce70243dc24d5b6415598c # 4.6.0
     hooks:
       - id: trailing-whitespace
       - id: end-of-file-fixer
@@ -12,28 +13,38 @@ repos:
       - id: detect-private-key
 
   - repo: https://github.com/doublify/pre-commit-rust
-    rev: v1.0
+    rev: eeee35a89e69d5772bdee97db1a6a898467b686e # 1.0
     hooks:
       - id: fmt
         args: ["--all", "--", "--check"]
       - id: clippy
         args: ["--all-targets", "--", "-D", "warnings"]
   - repo: https://github.com/adrienverge/yamllint
-    rev: v1.26.3
+    rev: 81e9f98ffd059efe8aa9c1b1a42e5cce61b640c6 # 1.35.1
     hooks:
       - id: yamllint
 
   - repo: https://github.com/igorshubovych/markdownlint-cli
-    rev: v0.31.1
+    rev: f295829140d25717bc79368d3f966fc1f67a824f # 0.41.0
     hooks:
       - id: markdownlint
 
   - repo: https://github.com/koalaman/shellcheck-precommit
-    rev: v0.10.0
+    rev: 2491238703a5d3415bb2b7ff11388bf775372f29 # 0.10.0
     hooks:
       - id: shellcheck
         args: ["--severity=info"]
 
+  - repo: https://github.com/rhysd/actionlint
+    rev: 62dc61a45fc95efe8c800af7a557ab0b9165d63b # 1.7.1
+    hooks:
+      - id: actionlint
+
+  - repo: https://github.com/hadolint/hadolint
+    rev: b3555ba9c2bfd9401e79f2f0da68dd1ae38e10c7 # 2.12.0
+    hooks:
+      - id: hadolint
+
   - repo: local
     hooks:
       - id: .scripts/verify-crate-versions

.pre-commit-config.yaml

@@ -15,9 +15,11 @@ All notable changes to this project will be documented in this file.
 ### Fixed
 
 - Fix the CRD description of `ClientAuthenticationDetails` to not contain internal Rust doc, but a public CRD description ([#846]).
+- `StackableAffinity` fields are no longer erroneously marked as required ([#855]).
 
 [#846]: https://github.com/stackabletech/operator-rs/pull/846
 [#851]: https://github.com/stackabletech/operator-rs/pull/851
+[#855]: https://github.com/stackabletech/operator-rs/pull/855
 
 ## [0.74.0] - 2024-08-22
 

crates/stackable-operator/CHANGELOG.md

@@ -38,15 +38,15 @@ pub const TOPOLOGY_KEY_HOSTNAME: &str = "kubernetes.io/hostname";
 )]
 pub struct StackableAffinity {
     /// Same as the `spec.affinity.podAffinity` field on the Pod, see the [Kubernetes docs](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node)
-    #[fragment_attrs(schemars(schema_with = "raw_optional_object_schema"))]
+    #[fragment_attrs(serde(default), schemars(schema_with = "raw_optional_object_schema"))]
     pub pod_affinity: Option<PodAffinity>,
 
     /// Same as the `spec.affinity.podAntiAffinity` field on the Pod, see the [Kubernetes docs](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node)
-    #[fragment_attrs(schemars(schema_with = "raw_optional_object_schema"))]
+    #[fragment_attrs(serde(default), schemars(schema_with = "raw_optional_object_schema"))]
     pub pod_anti_affinity: Option<PodAntiAffinity>,
 
     /// Same as the `spec.affinity.nodeAffinity` field on the Pod, see the [Kubernetes docs](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node)
-    #[fragment_attrs(schemars(schema_with = "raw_optional_object_schema"))]
+    #[fragment_attrs(serde(default), schemars(schema_with = "raw_optional_object_schema"))]
     pub node_affinity: Option<NodeAffinity>,
 
     // This schema isn't big, so it can stay

crates/stackable-operator/src/commons/affinity.rs

@@ -1,7 +1,7 @@
 use darling::{util::SpannedValue, Error, FromMeta};
 use k8s_version::Version;
 use proc_macro2::Span;
-use syn::{spanned::Spanned, Attribute, Ident, Path};
+use syn::{spanned::Spanned, Attribute, Ident, Path, Type};
 
 use crate::{
     attrs::common::ContainerAttributes,
@@ -55,14 +55,14 @@ where
             }
         }
 
-        for rename in &*self.common_attributes().renames {
+        for change in &*self.common_attributes().changes {
             if !container_attrs
                 .versions
                 .iter()
-                .any(|v| v.name == *rename.since)
+                .any(|v| v.name == *change.since)
             {
                 errors.push(
-                   Error::custom("variant action `renamed` uses version which was not declared via #[versioned(version)]")
+                   Error::custom("variant action `changed` uses version which was not declared via #[versioned(version)]")
                    .with_span(item)
                );
             }
@@ -97,15 +97,20 @@ pub(crate) enum ItemType {
     Variant,
 }
 
+// TODO (@Techassi): Shower thought: Track actions as a Vec of an ActionAttribute
+// enum and implement Ord on it. This would allow us to order the items by type
+// of action (added < changed < deprecated) as well as sort changed action to
+// each other by specified version (which already implements Ord)
+
 /// These attributes are meant to be used in super structs, which add
 /// [`Field`](syn::Field) or [`Variant`](syn::Variant) specific attributes via
 /// darling's flatten feature. This struct only provides shared attributes.
 ///
 /// ### Shared Item Rules
 ///
 /// - An item can only ever be added once at most. An item not marked as 'added'
-///   is part of the container in every version until renamed or deprecated.
-/// - An item can be renamed many times. That's why renames are stored in a
+///   is part of the container in every version until changed or deprecated.
+/// - An item can be changed many times. That's why changes are stored in a
 ///   [`Vec`].
 /// - An item can only be deprecated once. A field or variant not marked as
 ///   'deprecated' will be included up until the latest version.
@@ -115,10 +120,10 @@ pub(crate) struct ItemAttributes {
     /// only be present at most once.
     pub(crate) added: Option<AddedAttributes>,
 
-    /// This parses the `renamed` attribute on items (fields or variants). It
+    /// This parses the `changed` attribute on items (fields or variants). It
     /// can be present 0..n times.
-    #[darling(multiple, rename = "renamed")]
-    pub(crate) renames: Vec<RenamedAttributes>,
+    #[darling(multiple, rename = "changed")]
+    pub(crate) changes: Vec<ChangedAttributes>,
 
     /// This parses the `deprecated` attribute on items (fields or variants). It
     /// can only be present at most once.
@@ -138,20 +143,6 @@ impl ItemAttributes {
 
         let mut errors = Error::accumulator();
 
-        // TODO (@Techassi): Make the field or variant 'note' optional, because
-        // in the future, the macro will generate parts of the deprecation note
-        // automatically. The user-provided note will then be appended to the
-        // auto-generated one.
-
-        if let Some(deprecated) = &self.deprecated {
-            if deprecated.note.is_empty() {
-                errors.push(
-                    Error::custom("deprecation note must not be empty")
-                        .with_span(&deprecated.note.span()),
-                );
-            }
-        }
-
         // Semantic validation
         errors.handle(self.validate_action_combinations(item_ident, item_type));
         errors.handle(self.validate_action_order(item_ident, item_type));
@@ -175,34 +166,34 @@ impl ItemAttributes {
     ///   cannot be marked as added in a particular version and then marked as
     ///   deprecated immediately after. Fields and variants must be included for
     ///   at least one version before being marked deprecated.
-    /// - `added` and `renamed` using the same version: The same reasoning from
+    /// - `added` and `changed` using the same version: The same reasoning from
     ///   above applies here as well. Fields and variants must be included for
-    ///   at least one version before being renamed.
-    /// - `renamed` and `deprecated` using the same version: Again, the same
+    ///   at least one version before being changed.
+    /// - `changed` and `deprecated` using the same version: Again, the same
     ///   rules from above apply here as well.
     fn validate_action_combinations(
         &self,
         item_ident: &Ident,
         item_type: &ItemType,
     ) -> Result<(), Error> {
-        match (&self.added, &self.renames, &self.deprecated) {
+        match (&self.added, &self.changes, &self.deprecated) {
             (Some(added), _, Some(deprecated)) if *added.since == *deprecated.since => {
                 Err(Error::custom(format!(
                     "{item_type} cannot be marked as `added` and `deprecated` in the same version"
                 ))
                 .with_span(item_ident))
             }
-            (Some(added), renamed, _) if renamed.iter().any(|r| *r.since == *added.since) => {
+            (Some(added), changed, _) if changed.iter().any(|r| *r.since == *added.since) => {
                 Err(Error::custom(format!(
-                    "{item_type} cannot be marked as `added` and `renamed` in the same version"
+                    "{item_type} cannot be marked as `added` and `changed` in the same version"
                 ))
                 .with_span(item_ident))
             }
-            (_, renamed, Some(deprecated))
-                if renamed.iter().any(|r| *r.since == *deprecated.since) =>
+            (_, changed, Some(deprecated))
+                if changed.iter().any(|r| *r.since == *deprecated.since) =>
             {
                 Err(Error::custom(
-                    format!("{item_type} cannot be marked as `deprecated` and `renamed` in the same version"),
+                    format!("{item_type} cannot be marked as `deprecated` and `changed` in the same version"),
                 )
                 .with_span(item_ident))
             }
@@ -220,7 +211,7 @@ impl ItemAttributes {
     ///   ensures that these versions are chronologically sound, that means,
     ///   that the version of the deprecated action must be greater than the
     ///   version of the added action.
-    /// - All `renamed` actions must use a greater version than `added` but a
+    /// - All `changed` actions must use a greater version than `added` but a
     ///   lesser version than `deprecated`.
     fn validate_action_order(&self, item_ident: &Ident, item_type: &ItemType) -> Result<(), Error> {
         let added_version = self.added.as_ref().map(|a| *a.since);
@@ -238,14 +229,14 @@ impl ItemAttributes {
             }
         }
 
-        // Now, iterate over all renames and ensure that their versions are
+        // Now, iterate over all changes and ensure that their versions are
         // between the added and deprecated version.
-        if !self.renames.iter().all(|r| {
+        if !self.changes.iter().all(|r| {
             added_version.map_or(true, |a| a < *r.since)
                 && deprecated_version.map_or(true, |d| d > *r.since)
         }) {
             return Err(Error::custom(
-                "all renames must use versions higher than `added` and lower than `deprecated`",
+                "all changes must use versions higher than `added` and lower than `deprecated`",
             )
             .with_span(item_ident));
         }
@@ -320,27 +311,32 @@ pub(crate) struct AddedAttributes {
 
 fn default_default_fn() -> SpannedValue<Path> {
     SpannedValue::new(
-        syn::parse_str("std::default::Default::default").expect("internal error: path must parse"),
+        syn::parse_str("::std::default::Default::default")
+            .expect("internal error: path must parse"),
         Span::call_site(),
     )
 }
 
-/// For the renamed() action
+/// For the changed() action
 ///
 /// Example usage:
-/// - `renamed(since = "...", from = "...")`
+/// - `changed(since = "...", from_name = "...")`
+/// - `changed(since = "...", from_name = "..." from_type="...")`
 #[derive(Clone, Debug, FromMeta)]
-pub(crate) struct RenamedAttributes {
+pub(crate) struct ChangedAttributes {
     pub(crate) since: SpannedValue<Version>,
-    pub(crate) from: SpannedValue<String>,
+    pub(crate) from_name: Option<SpannedValue<String>>,
+
+    pub(crate) from_type: Option<SpannedValue<Type>>,
 }
 
 /// For the deprecated() action
 ///
 /// Example usage:
+/// - `deprecated(since = "...")`
 /// - `deprecated(since = "...", note = "...")`
 #[derive(Clone, Debug, FromMeta)]
 pub(crate) struct DeprecatedAttributes {
     pub(crate) since: SpannedValue<Version>,
-    pub(crate) note: SpannedValue<String>,
+    pub(crate) note: Option<SpannedValue<String>>,
 }

crates/stackable-versioned-macros/src/attrs/common/item.rs

@@ -50,8 +50,8 @@ impl FieldAttributes {
             .ident
             .as_ref()
             .expect("internal error: field must have an ident");
-        self.common.validate(ident, &ItemType::Field, &self.attrs)?;
 
+        self.common.validate(ident, &ItemType::Field, &self.attrs)?;
         Ok(self)
     }
 }