diff --git a/Cargo.lock b/Cargo.lock index 0de243037..153f8169a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4232,6 +4232,7 @@ name = "xtask" version = "0.0.0" dependencies = [ "clap", + "indoc", "paste", "serde", "serde_json", diff --git a/crates/stackable-operator/src/client.rs b/crates/stackable-operator/src/client.rs index bb9ceb122..3c27863ba 100644 --- a/crates/stackable-operator/src/client.rs +++ b/crates/stackable-operator/src/client.rs @@ -590,7 +590,7 @@ impl Client { pub trait GetApi: Resource + Sized { /// The namespace type for `Self`'s scope. /// - /// This will be [`str`] for namespaced resource, and [`()`] for cluster-scoped resources. + /// This will be [`str`] for namespaced resource, and `()` for cluster-scoped resources. type Namespace: ?Sized; /// Get a [`kube::Api`] for `Self`'s native scope.. fn get_api(client: kube::Client, ns: &Self::Namespace) -> kube::Api diff --git a/crates/stackable-operator/src/crd/authentication/core/mod.rs b/crates/stackable-operator/src/crd/authentication/core/mod.rs index f466faf5f..884d474af 100644 --- a/crates/stackable-operator/src/crd/authentication/core/mod.rs +++ b/crates/stackable-operator/src/crd/authentication/core/mod.rs @@ -155,3 +155,59 @@ pub mod versioned { oidc: Option>, } } + +#[cfg(test)] +impl stackable_versioned::test_utils::RoundtripTestData for v1alpha1::AuthenticationClassSpec { + fn roundtrip_test_data() -> Vec { + crate::utils::yaml_from_str_singleton_map(indoc::indoc! {" + - provider: + static: + userCredentialsSecret: + name: simple-users-credentials + - provider: + ldap: + hostname: my.ldap.server + port: 389 + searchBase: ou=users,dc=example,dc=org + searchFilter: foo + bindCredentials: + secretClass: openldap-bind-credentials + ldapFieldNames: + email: email + givenName: givenName + group: group + surname: surname + uid: uid + tls: + verification: + server: + caCert: + secretClass: s3-cert + - provider: + oidc: + hostname: my.keycloak.server + port: 8080 + rootPath: /realms/master + scopes: + - email + - openid + - profile + principalClaim: preferred_username + providerHint: Keycloak + tls: + verification: + server: + caCert: + secretClass: s3-cert + - provider: + tls: {} + - provider: + tls: + clientCertSecretClass: client-auth-tls + - provider: + kerberos: + kerberosSecretClass: kerberos-auth + "}) + .expect("Failed to parse AuthenticationClassSpec YAML") + } +} diff --git a/crates/stackable-operator/src/crd/listener/class/mod.rs b/crates/stackable-operator/src/crd/listener/class/mod.rs index 491d09512..1a7b1e966 100644 --- a/crates/stackable-operator/src/crd/listener/class/mod.rs +++ b/crates/stackable-operator/src/crd/listener/class/mod.rs @@ -119,3 +119,22 @@ pub mod versioned { pub service_overrides: Service, } } + +#[cfg(test)] +impl stackable_versioned::test_utils::RoundtripTestData for v1alpha1::ListenerClassSpec { + fn roundtrip_test_data() -> Vec { + crate::utils::yaml_from_str_singleton_map(indoc::indoc! {" + - serviceType: ClusterIP + - serviceType: NodePort + - serviceType: LoadBalancer + - serviceType: ClusterIP + loadBalancerAllocateNodePorts: false + loadBalancerClass: foo + serviceAnnotations: + foo: bar + serviceExternalTrafficPolicy: Local + preferredAddressType: HostnameConservative + "}) + .expect("Failed to parse ListenerClassSpec YAML") + } +} diff --git a/crates/stackable-operator/src/crd/listener/listeners/mod.rs b/crates/stackable-operator/src/crd/listener/listeners/mod.rs index 2c4f45eaf..1542d3258 100644 --- a/crates/stackable-operator/src/crd/listener/listeners/mod.rs +++ b/crates/stackable-operator/src/crd/listener/listeners/mod.rs @@ -167,3 +167,54 @@ pub mod versioned { Cluster, } } + +#[cfg(test)] +impl stackable_versioned::test_utils::RoundtripTestData for v1alpha1::ListenerSpec { + fn roundtrip_test_data() -> Vec { + crate::utils::yaml_from_str_singleton_map(indoc::indoc! {" + - {} + - className: cluster-internal + extraPodLabelSelectorLabels: {} + ports: [] + publishNotReadyAddresses: true + - className: external-unstable + extraPodLabelSelectorLabels: + foo: bar + ports: + - name: http + port: 8080 + protocol: TCP + publishNotReadyAddresses: true + "}) + .expect("Failed to parse ListenerSpec YAML") + } +} + +#[cfg(test)] +impl stackable_versioned::test_utils::RoundtripTestData for v1alpha1::PodListenersSpec { + fn roundtrip_test_data() -> Vec { + crate::utils::yaml_from_str_singleton_map(indoc::indoc! {" + - listeners: {} + - listeners: + foo: + scope: Node + - listeners: + foo: + scope: Cluster + ingressAddresses: + - address: 1.2.3.4 + addressType: IP + ports: {} + - listeners: + foo: + scope: Cluster + ingressAddresses: + - address: foo.bar + addressType: Hostname + ports: + http: 8080 + https: 8443 + "}) + .expect("Failed to parse PodListenersSpec YAML") + } +} diff --git a/crates/stackable-operator/src/crd/s3/bucket/mod.rs b/crates/stackable-operator/src/crd/s3/bucket/mod.rs index 9ec56d2a8..5672c04fc 100644 --- a/crates/stackable-operator/src/crd/s3/bucket/mod.rs +++ b/crates/stackable-operator/src/crd/s3/bucket/mod.rs @@ -51,3 +51,34 @@ pub mod versioned { pub connection: conn_v1alpha1::ConnectionSpec, } } + +#[cfg(test)] +impl stackable_versioned::test_utils::RoundtripTestData for v1alpha1::BucketSpec { + fn roundtrip_test_data() -> Vec { + crate::utils::yaml_from_str_singleton_map(indoc::indoc! {" + - bucketName: my-example-bucket + connection: + reference: my-connection-resource + - bucketName: foo + connection: + inline: + host: s3.example.com + - bucketName: foo + connection: + inline: + host: s3.example.com + port: 1234 + accessStyle: VirtualHosted + credentials: + secretClass: s3-credentials + region: + name: eu-west-1 + tls: + verification: + server: + caCert: + secretClass: s3-cert + "}) + .expect("Failed to parse BucketSpec YAML") + } +} diff --git a/crates/stackable-operator/src/crd/s3/connection/mod.rs b/crates/stackable-operator/src/crd/s3/connection/mod.rs index b41c280d8..09cd1ac56 100644 --- a/crates/stackable-operator/src/crd/s3/connection/mod.rs +++ b/crates/stackable-operator/src/crd/s3/connection/mod.rs @@ -103,6 +103,36 @@ pub mod versioned { } } +#[cfg(test)] +impl stackable_versioned::test_utils::RoundtripTestData for v1alpha1::ConnectionSpec { + fn roundtrip_test_data() -> Vec { + crate::utils::yaml_from_str_singleton_map(indoc::indoc! {" + - host: s3.example.com + - host: s3.example.com + port: 1234 + accessStyle: VirtualHosted + credentials: + secretClass: s3-credentials + region: + name: eu-west-1 + tls: null + - host: s3.example.com + region: + name: us-east-1 + tls: + verification: + none: {} + - host: s3.example.com + tls: + verification: + server: + caCert: + secretClass: s3-cert + "}) + .expect("Failed to parse ConnectionSpec YAML") + } +} + #[cfg(test)] mod tests { use std::collections::BTreeMap; diff --git a/crates/stackable-versioned-macros/src/codegen/container/mod.rs b/crates/stackable-versioned-macros/src/codegen/container/mod.rs index 326e10a5b..5fc2b9c67 100644 --- a/crates/stackable-versioned-macros/src/codegen/container/mod.rs +++ b/crates/stackable-versioned-macros/src/codegen/container/mod.rs @@ -20,6 +20,7 @@ mod r#enum; mod r#struct; /// Contains common container data shared between structs and enums. +#[derive(Debug)] pub struct CommonContainerData { /// Original attributes placed on the container, like `#[derive()]` or `#[cfg()]`. pub original_attributes: Vec, diff --git a/crates/stackable-versioned-macros/src/codegen/container/struct/conversion.rs b/crates/stackable-versioned-macros/src/codegen/container/struct/conversion.rs index 7d6c5e593..4d64ff728 100644 --- a/crates/stackable-versioned-macros/src/codegen/container/struct/conversion.rs +++ b/crates/stackable-versioned-macros/src/codegen/container/struct/conversion.rs @@ -3,7 +3,7 @@ use std::{borrow::Cow, cmp::Ordering}; use indoc::formatdoc; use itertools::Itertools as _; use proc_macro2::TokenStream; -use quote::quote; +use quote::{format_ident, quote}; use syn::parse_quote; use crate::{ @@ -530,6 +530,71 @@ impl Struct { }) } + pub(super) fn generate_conversion_roundtrip_test( + &self, + versions: &[VersionDefinition], + mod_gen_ctx: ModuleGenerationContext<'_>, + spec_gen_ctx: &SpecGenerationContext<'_>, + ) -> TokenStream { + let versioned_path = &*mod_gen_ctx.crates.versioned; + let struct_ident = &self.common.idents.original; + let kind_ident = &spec_gen_ctx.kubernetes_idents.kind; + + let api_versions = spec_gen_ctx + .version_strings + .iter() + .map(|version| { + format!( + "{group}/{version}", + group = &spec_gen_ctx.kubernetes_arguments.group + ) + }) + .collect::>(); + + let earliest_version_module_ident = &versions + .first() + .expect("there must be at least one version present") + .idents + .module; + let latest_version_module_ident = &versions + .last() + .expect("there must be at least one version present") + .idents + .module; + let earliest_api_version = api_versions + .first() + .expect("there must be at least one version present"); + let latest_api_version = api_versions + .last() + .expect("there must be at least one version present"); + let test_function_down_up = format_ident!("{struct_ident}_roundtrip_down_up"); + let test_function_up_down = format_ident!("{struct_ident}_roundtrip_up_down"); + + quote! { + #[cfg(test)] + #[test] + fn #test_function_down_up() { + #versioned_path::test_utils::test_roundtrip::<#latest_version_module_ident::#struct_ident>( + stringify!(#kind_ident), + #latest_api_version, + #earliest_api_version, + #kind_ident::try_convert, + ); + } + + #[cfg(test)] + #[test] + fn #test_function_up_down() { + #versioned_path::test_utils::test_roundtrip::<#earliest_version_module_ident::#struct_ident>( + stringify!(#kind_ident), + #earliest_api_version, + #latest_api_version, + #kind_ident::try_convert, + ); + } + } + } + pub(super) fn generate_from_json_object_fn( &self, mod_gen_ctx: ModuleGenerationContext<'_>, diff --git a/crates/stackable-versioned-macros/src/codegen/container/struct/mod.rs b/crates/stackable-versioned-macros/src/codegen/container/struct/mod.rs index 021f523fa..5b05c08f2 100644 --- a/crates/stackable-versioned-macros/src/codegen/container/struct/mod.rs +++ b/crates/stackable-versioned-macros/src/codegen/container/struct/mod.rs @@ -87,6 +87,7 @@ impl Container { } /// A versioned struct. +#[derive(Debug)] pub struct Struct { /// List of fields defined in the original struct. How, and if, an item /// should generate code, is decided by the currently generated version. @@ -101,6 +102,7 @@ pub struct Struct { pub generics: Generics, } +#[derive(Debug)] pub struct KubernetesData { pub kubernetes_arguments: StructCrdArguments, pub kubernetes_idents: KubernetesIdents, @@ -164,11 +166,14 @@ impl Struct { self.generate_entry_impl_block(versions, mod_gen_ctx, &spec_gen_ctx); let version_enum = self.generate_version_enum(mod_gen_ctx, &spec_gen_ctx); let status_struct = self.generate_status_struct(mod_gen_ctx, &spec_gen_ctx); + let conversion_roundtrip_test = + self.generate_conversion_roundtrip_test(versions, mod_gen_ctx, &spec_gen_ctx); container_tokens .extend_outer(entry_enum) .extend_outer(entry_enum_impl) .extend_outer(version_enum) + .extend_outer(conversion_roundtrip_test) .extend_outer(status_struct); } diff --git a/crates/stackable-versioned-macros/src/codegen/item/field.rs b/crates/stackable-versioned-macros/src/codegen/item/field.rs index 9c51b196d..5ffb475af 100644 --- a/crates/stackable-versioned-macros/src/codegen/item/field.rs +++ b/crates/stackable-versioned-macros/src/codegen/item/field.rs @@ -17,6 +17,7 @@ use crate::{ utils::{ItemIdentExt, ItemIdents}, }; +#[derive(Debug)] pub struct VersionedField { pub original_attributes: Vec, pub changes: Option>, diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@basic.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@basic.rs.snap index 904083f27..2fc3903b2 100644 --- a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@basic.rs.snap +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@basic.rs.snap @@ -434,3 +434,17 @@ impl FooVersion { } } } +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_down_up() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1::FooSpec, + >(stringify!(Foo), "stackable.tech/v1", "stackable.tech/v1alpha1", Foo::try_convert); +} +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_up_down() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1alpha1::FooSpec, + >(stringify!(Foo), "stackable.tech/v1alpha1", "stackable.tech/v1", Foo::try_convert); +} diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking.rs.snap index 88e9eeb82..febe62803 100644 --- a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking.rs.snap +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@conversion_tracking.rs.snap @@ -478,6 +478,20 @@ impl FooVersion { } } } +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_down_up() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1::FooSpec, + >(stringify!(Foo), "stackable.tech/v1", "stackable.tech/v1alpha1", Foo::try_convert); +} +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_up_down() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1alpha1::FooSpec, + >(stringify!(Foo), "stackable.tech/v1alpha1", "stackable.tech/v1", Foo::try_convert); +} #[automatically_derived] #[derive( ::core::clone::Clone, diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@crate_overrides.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@crate_overrides.rs.snap index f62143573..cc950ec37 100644 --- a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@crate_overrides.rs.snap +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@crate_overrides.rs.snap @@ -421,3 +421,27 @@ impl FooVersion { } } } +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_down_up() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1::FooSpec, + >( + stringify!(Foo), + "foo.example.org/v1", + "foo.example.org/v1alpha1", + Foo::try_convert, + ); +} +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_up_down() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1alpha1::FooSpec, + >( + stringify!(Foo), + "foo.example.org/v1alpha1", + "foo.example.org/v1", + Foo::try_convert, + ); +} diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@module.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@module.rs.snap index e86d884fa..80162cebe 100644 --- a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@module.rs.snap +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@module.rs.snap @@ -629,6 +629,30 @@ impl FooVersion { } } } +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_down_up() { + ::stackable_versioned::test_utils::test_roundtrip::< + v2alpha1::FooSpec, + >( + stringify!(Foo), + "foo.example.org/v2alpha1", + "foo.example.org/v1alpha1", + Foo::try_convert, + ); +} +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_up_down() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1alpha1::FooSpec, + >( + stringify!(Foo), + "foo.example.org/v1alpha1", + "foo.example.org/v2alpha1", + Foo::try_convert, + ); +} #[automatically_derived] #[derive(::core::fmt::Debug)] pub(crate) enum Bar { @@ -911,3 +935,27 @@ impl BarVersion { } } } +#[cfg(test)] +#[test] +fn BarSpec_roundtrip_down_up() { + ::stackable_versioned::test_utils::test_roundtrip::< + v2alpha1::BarSpec, + >( + stringify!(Bar), + "bar.example.org/v2alpha1", + "bar.example.org/v1alpha1", + Bar::try_convert, + ); +} +#[cfg(test)] +#[test] +fn BarSpec_roundtrip_up_down() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1alpha1::BarSpec, + >( + stringify!(Bar), + "bar.example.org/v1alpha1", + "bar.example.org/v2alpha1", + Bar::try_convert, + ); +} diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@module_preserve.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@module_preserve.rs.snap index 261792f53..b9d3c6e2b 100644 --- a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@module_preserve.rs.snap +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@module_preserve.rs.snap @@ -599,6 +599,30 @@ pub(crate) mod versioned { } } } + #[cfg(test)] + #[test] + fn FooSpec_roundtrip_down_up() { + ::stackable_versioned::test_utils::test_roundtrip::< + v2alpha1::FooSpec, + >( + stringify!(Foo), + "foo.example.org/v2alpha1", + "foo.example.org/v1alpha1", + Foo::try_convert, + ); + } + #[cfg(test)] + #[test] + fn FooSpec_roundtrip_up_down() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1alpha1::FooSpec, + >( + stringify!(Foo), + "foo.example.org/v1alpha1", + "foo.example.org/v2alpha1", + Foo::try_convert, + ); + } #[derive(::core::fmt::Debug)] pub enum Bar { V1Alpha1(v1alpha1::Bar), @@ -876,4 +900,28 @@ pub(crate) mod versioned { } } } + #[cfg(test)] + #[test] + fn BarSpec_roundtrip_down_up() { + ::stackable_versioned::test_utils::test_roundtrip::< + v2alpha1::BarSpec, + >( + stringify!(Bar), + "bar.example.org/v2alpha1", + "bar.example.org/v1alpha1", + Bar::try_convert, + ); + } + #[cfg(test)] + #[test] + fn BarSpec_roundtrip_up_down() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1alpha1::BarSpec, + >( + stringify!(Bar), + "bar.example.org/v1alpha1", + "bar.example.org/v2alpha1", + Bar::try_convert, + ); + } } diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@renamed_kind.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@renamed_kind.rs.snap index a4b97d5ae..5e18325a0 100644 --- a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@renamed_kind.rs.snap +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@renamed_kind.rs.snap @@ -406,3 +406,27 @@ impl FooBarVersion { } } } +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_down_up() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1::FooSpec, + >( + stringify!(FooBar), + "stackable.tech/v1", + "stackable.tech/v1alpha1", + FooBar::try_convert, + ); +} +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_up_down() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1alpha1::FooSpec, + >( + stringify!(FooBar), + "stackable.tech/v1alpha1", + "stackable.tech/v1", + FooBar::try_convert, + ); +} diff --git a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@shortnames.rs.snap b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@shortnames.rs.snap index c20892b16..2d3472af9 100644 --- a/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@shortnames.rs.snap +++ b/crates/stackable-versioned-macros/tests/snapshots/stackable_versioned_macros__snapshots__pass@shortnames.rs.snap @@ -213,3 +213,27 @@ impl FooVersion { } } } +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_down_up() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1alpha1::FooSpec, + >( + stringify!(Foo), + "stackable.tech/v1alpha1", + "stackable.tech/v1alpha1", + Foo::try_convert, + ); +} +#[cfg(test)] +#[test] +fn FooSpec_roundtrip_up_down() { + ::stackable_versioned::test_utils::test_roundtrip::< + v1alpha1::FooSpec, + >( + stringify!(Foo), + "stackable.tech/v1alpha1", + "stackable.tech/v1alpha1", + Foo::try_convert, + ); +} diff --git a/crates/stackable-versioned/CHANGELOG.md b/crates/stackable-versioned/CHANGELOG.md index d388a4d7d..02b0767f8 100644 --- a/crates/stackable-versioned/CHANGELOG.md +++ b/crates/stackable-versioned/CHANGELOG.md @@ -4,6 +4,15 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- BREAKING: The `versioned` macro now automatically generates tests to test roundtrip converts for + every versioned struct. This requires you to implement the `RoundtripTestData` trait for all + structs, which returns some test data to be used for the roundtrip tests. + Read on the `RoundtripTestData` documentation for details ([#1172]). + +[#1172]: https://github.com/stackabletech/operator-rs/pull/1172 + ## [0.8.3] - 2025-10-23 ### Fixed diff --git a/crates/stackable-versioned/Cargo.toml b/crates/stackable-versioned/Cargo.toml index 2bb8d43bd..32853328e 100644 --- a/crates/stackable-versioned/Cargo.toml +++ b/crates/stackable-versioned/Cargo.toml @@ -13,6 +13,7 @@ all-features = true [dependencies] stackable-versioned-macros = { path = "../stackable-versioned-macros" } +kube.workspace = true schemars.workspace = true serde.workspace = true serde_json.workspace = true @@ -22,7 +23,6 @@ snafu.workspace = true [dev-dependencies] insta.workspace = true k8s-openapi.workspace = true -kube.workspace = true [lints] workspace = true diff --git a/crates/stackable-versioned/src/lib.rs b/crates/stackable-versioned/src/lib.rs index 7b7445a4b..3cf3df4bf 100644 --- a/crates/stackable-versioned/src/lib.rs +++ b/crates/stackable-versioned/src/lib.rs @@ -42,6 +42,10 @@ use snafu::{ErrorCompat, Snafu}; // Re-export pub use stackable_versioned_macros::versioned; +// We can not put this behind `#[cfg(test)]`, as it seems like the `test` flag is not enabled, when +// a *dependant* crate compiles tests. +pub mod test_utils; + /// A value-to-value conversion that consumes the input value while tracking changes via a /// Kubernetes status. /// diff --git a/crates/stackable-versioned/src/test_utils.rs b/crates/stackable-versioned/src/test_utils.rs new file mode 100644 index 000000000..84da9c4ee --- /dev/null +++ b/crates/stackable-versioned/src/test_utils.rs @@ -0,0 +1,153 @@ +use kube::{ + api::TypeMeta, + core::{ + conversion::{ConversionRequest, ConversionReview}, + response::StatusSummary, + }, +}; +use serde::Serialize; + +const TEST_CONVERSION_UUID: &str = "9980028f-816b-4b38-a521-5f087266f76c"; + +/// One very important requirement for CRD conversions is that they support roundtrips, which means +/// that we can not loose data when starting at version A, converting to version B and back to +/// version A. +/// +/// As it's very hard to make sure the roundtrips never loos data, the +/// [`crate::versioned`] macro automatically generates tests that test roundtrips. +/// However, for that to work it needs some test data to run through the conversions, hence it +/// requires you to provide test data for the earliest and latest version of the CRD struct. +/// +/// You can provide test data e.g. as follows +/// ``` +/// #[cfg(test)] +/// impl stackable_operator::versioned::test_utils::RoundtripTestData for v1alpha1::ListenerClassSpec { +/// fn roundtrip_test_data() -> Vec { +/// stackable_operator::utils::yaml_from_str_singleton_map(indoc::indoc! {" +/// - serviceType: ClusterIP +/// - serviceType: NodePort +/// - serviceType: LoadBalancer +/// - serviceType: ClusterIP +/// loadBalancerAllocateNodePorts: false +/// loadBalancerClass: foo +/// serviceAnnotations: +/// foo: bar +/// serviceExternalTrafficPolicy: Local +/// preferredAddressType: HostnameConservative +/// "}) +/// .expect("Failed to parse ListenerClassSpec YAML") +/// } +/// } +/// ``` +pub trait RoundtripTestData: Sized + Serialize { + fn roundtrip_test_data() -> Vec; +} + +/// Tests a roundtrip `start_version` -> `middle_version` -> `start_version` and asserts that it +/// produces the same output as input. +pub fn test_roundtrip( + kind: &str, + start_version: &str, + middle_version: &str, + convert_fn: fn(ConversionReview) -> ConversionReview, +) { + // Construct test data + let original_specs = StartVersion::roundtrip_test_data() + .iter() + .map(|spec| serde_json::to_value(spec).expect("Failed to serialize inout roundtrip data")) + .collect::>(); + let original_objects = specs_to_objects(original_specs.clone(), start_version, kind); + + // Downgrade to the middle version + let downgrade_conversion_review = conversion_review(original_objects, middle_version); + let downgraded = convert_fn(downgrade_conversion_review); + let downgraded_objects = objects_from_conversion_review(downgraded); + + // Upgrade to start version again + let upgrade_conversion_review = conversion_review(downgraded_objects, start_version); + let upgraded = convert_fn(upgrade_conversion_review); + let upgraded_objects = objects_from_conversion_review(upgraded); + let upgraded_specs = objects_to_specs(upgraded_objects); + + // Assert the same output as input + assert_eq!(upgraded_specs.len(), original_specs.len()); + assert_eq!( + upgraded_specs, original_specs, + "The object spec must be the same before and after the roundtrip!" + ); +} + +/// Crates a [`ConversionReview`] that requests to convert the passed `objects` to the desired +/// apiVersion +fn conversion_review( + objects: impl IntoIterator, + desired_api_version: impl Into, +) -> ConversionReview { + let conversion_request = ConversionRequest { + types: Some(conversion_type()), + uid: TEST_CONVERSION_UUID.to_string(), + desired_api_version: desired_api_version.into(), + objects: objects.into_iter().collect(), + }; + ConversionReview { + types: conversion_type(), + request: Some(conversion_request), + response: None, + } +} + +/// Converts from a `.spec` field (as [`serde_json::Value`]) to the entire Kubernetes object +/// (also as [`serde_json::Value`]). +fn specs_to_objects( + specs: impl IntoIterator, + api_version: &str, + kind: &str, +) -> Vec { + specs + .into_iter() + .map(|spec| { + serde_json::json!({ + "apiVersion": api_version, + "kind": kind, + "spec": spec, + "metadata": {}, + }) + }) + .collect() +} + +/// Extracts the `.spec` field out of Kubernetes objects +fn objects_to_specs( + objects: impl IntoIterator, +) -> Vec { + objects + .into_iter() + .map(|obj| { + obj.get("spec") + .expect("The downgraded objects need to have a spec") + .to_owned() + }) + .collect() +} + +/// Asserts that the [`ConversionReview`] was successful and extracts the `converted_objects` +fn objects_from_conversion_review(conversion_review: ConversionReview) -> Vec { + let conversion_result = conversion_review + .response + .expect("The ConversionReview needs to have a result"); + + assert_eq!( + conversion_result.result.status, + Some(StatusSummary::Success), + "The conversion failed: {conversion_result:?}" + ); + + conversion_result.converted_objects +} + +fn conversion_type() -> TypeMeta { + TypeMeta { + api_version: "apiextensions.k8s.io/v1".to_string(), + kind: "ConversionReview".to_string(), + } +} diff --git a/crates/stackable-versioned/tests/person.rs b/crates/stackable-versioned/tests/person.rs index fbbe930b4..bd502cfc6 100644 --- a/crates/stackable-versioned/tests/person.rs +++ b/crates/stackable-versioned/tests/person.rs @@ -66,6 +66,8 @@ pub mod versioned { // We started out with a enum. As we *need* to provide a default, we have a Unknown variant. // Afterwards we figured let's be more flexible and accept any arbitrary String. + + // FIXME: The roundtrips are currently broken, see the `roundtrip_test_data` below. #[versioned(added(since = "v2"), changed(since = "v3", from_type = "Gender"))] gender: String, @@ -82,6 +84,68 @@ pub mod versioned { } } +#[cfg(test)] +impl stackable_versioned::test_utils::RoundtripTestData for v1alpha1::PersonSpec { + fn roundtrip_test_data() -> Vec { + vec![ + Self { + username: "jdoe".to_string(), + socials: v1alpha1::Socials { + email: "jdoe@example.com".to_owned(), + }, + }, + Self { + username: "".to_string(), + socials: v1alpha1::Socials { + email: "".to_owned(), + }, + }, + ] + } +} + +#[cfg(test)] +impl stackable_versioned::test_utils::RoundtripTestData for v3::PersonSpec { + fn roundtrip_test_data() -> Vec { + vec![ + Self { + username: "jdoe".to_owned(), + first_name: "John".to_owned(), + last_name: "Doe".to_owned(), + gender: "Unknown".to_owned(), + socials: v3::Socials { + email: "jdoe@example.com".to_owned(), + mastodon: "@jdoe@example.com".to_owned(), + }, + }, + Self { + username: "jdoe".to_owned(), + first_name: "John".to_owned(), + last_name: "Doe".to_owned(), + gender: "Male".to_owned(), + socials: v3::Socials { + email: "jdoe@example.com".to_owned(), + mastodon: "@jdoe@example.com".to_owned(), + }, + }, + // FIXME: The following test case fails. See the docs on the `versioned` macro, as of + // writing it only supports tracking `added` fields. + // Hence, the firstName, lastName, socials.mastodon work, while gender is broken. + // Self { + // username: "".to_owned(), + // first_name: "".to_owned(), + // last_name: "".to_owned(), + // // FIXME: Currently, the roundtrip results in "Unknown", although it should be "It's complicated" + // gender: "It's complicated".to_owned(), + // socials: v3::Socials { + // email: "".to_owned(), + // mastodon: "".to_owned(), + // }, + // }, + ] + } +} + #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct PersonStatus { pub alive: bool, diff --git a/crates/xtask/Cargo.toml b/crates/xtask/Cargo.toml index b64675e9f..c03ef78b4 100644 --- a/crates/xtask/Cargo.toml +++ b/crates/xtask/Cargo.toml @@ -7,6 +7,7 @@ publish = false stackable-operator = { path = "../stackable-operator", features = ["crds"] } clap.workspace = true +indoc.workspace = true paste.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/crates/xtask/src/crd/dummy.rs b/crates/xtask/src/crd/dummy.rs index 28ed0581c..c27313559 100644 --- a/crates/xtask/src/crd/dummy.rs +++ b/crates/xtask/src/crd/dummy.rs @@ -115,3 +115,11 @@ pub mod versioned { pub conditions: Vec, } } + +#[cfg(test)] +impl stackable_operator::versioned::test_utils::RoundtripTestData for v1alpha1::DummyClusterSpec { + fn roundtrip_test_data() -> Vec { + // In the DummyCluster we only have v1alpha1, so there is no point in testing roundtrips. + vec![] + } +}