From 81a31415f73cb87ae19d7b42b35552accc3060e8 Mon Sep 17 00:00:00 2001 From: t4lz Date: Tue, 13 Feb 2024 13:22:45 +0100 Subject: [PATCH] Add new features enum and field for operator The old enum cannot be extended as it would break old clients. So we define a new extensible enum and a new field for it. We make all of those fields private so that users of that struct only access them via a getter that handles objects deserialized from either old or new versions. --- Cargo.lock | 365 ++++++++++++++++++++++++++++++++- mirrord/operator/src/client.rs | 11 +- mirrord/operator/src/crd.rs | 98 ++++++++- 3 files changed, 465 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e5ce871cbf1..f9a888fad07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -535,6 +535,321 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" +[[package]] +name = "aws-config" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7af266887e24cd5f6d2ea7433cacd25dcd4773b7f70e488701968a7cdf51df57" +dependencies = [ + "aws-credential-types", + "aws-runtime", + "aws-sdk-sso", + "aws-sdk-ssooidc", + "aws-sdk-sts", + "aws-smithy-async", + "aws-smithy-http", + "aws-smithy-json", + "aws-smithy-runtime", + "aws-smithy-runtime-api", + "aws-smithy-types", + "aws-types", + "bytes", + "fastrand", + "hex", + "http 0.2.11", + "hyper 0.14.28", + "ring 0.17.7", + "time", + "tokio", + "tracing", + "zeroize", +] + +[[package]] +name = "aws-credential-types" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d56f287a9e65e4914bfedb5b22c056b65e4c232fca512d5509a9df36386759f" +dependencies = [ + "aws-smithy-async", + "aws-smithy-runtime-api", + "aws-smithy-types", + "zeroize", +] + +[[package]] +name = "aws-runtime" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d6a29eca8ea8982028a4df81883e7001e250a21d323b86418884b5345950a4b" +dependencies = [ + "aws-credential-types", + "aws-sigv4", + "aws-smithy-async", + "aws-smithy-http", + "aws-smithy-runtime-api", + "aws-smithy-types", + "aws-types", + "bytes", + "fastrand", + "http 0.2.11", + "http-body 0.4.6", + "percent-encoding", + "pin-project-lite", + "tracing", + "uuid", +] + +[[package]] +name = "aws-sdk-sqs" +version = "1.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a16637754ae72aba8d157ad559a75685f93e60e37e008d5197d809b74b37b2e6" +dependencies = [ + "aws-credential-types", + "aws-runtime", + "aws-smithy-async", + "aws-smithy-http", + "aws-smithy-json", + "aws-smithy-runtime", + "aws-smithy-runtime-api", + "aws-smithy-types", + "aws-types", + "bytes", + "http 0.2.11", + "once_cell", + "regex-lite", + "tracing", +] + +[[package]] +name = "aws-sdk-sso" +version = "1.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e2d7f527c7b28af1a641f7d89f9e6a4863e8ec00f39d2b731b056fc5ec5ce829" +dependencies = [ + "aws-credential-types", + "aws-runtime", + "aws-smithy-async", + "aws-smithy-http", + "aws-smithy-json", + "aws-smithy-runtime", + "aws-smithy-runtime-api", + "aws-smithy-types", + "aws-types", + "bytes", + "http 0.2.11", + "once_cell", + "regex-lite", + "tracing", +] + +[[package]] +name = "aws-sdk-ssooidc" +version = "1.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0d0be3224cd574ee8ab5fd7c32087876f25c134c27ac603fcb38669ed8d346b0" +dependencies = [ + "aws-credential-types", + "aws-runtime", + "aws-smithy-async", + "aws-smithy-http", + "aws-smithy-json", + "aws-smithy-runtime", + "aws-smithy-runtime-api", + "aws-smithy-types", + "aws-types", + "bytes", + "http 0.2.11", + "once_cell", + "regex-lite", + "tracing", +] + +[[package]] +name = "aws-sdk-sts" +version = "1.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b3167c60d82a13bbaef569da06041644ff41e85c6377e5dad53fa2526ccfe9d" +dependencies = [ + "aws-credential-types", + "aws-runtime", + "aws-smithy-async", + "aws-smithy-http", + "aws-smithy-json", + "aws-smithy-query", + "aws-smithy-runtime", + "aws-smithy-runtime-api", + "aws-smithy-types", + "aws-smithy-xml", + "aws-types", + "http 0.2.11", + "once_cell", + "regex-lite", + "tracing", +] + +[[package]] +name = "aws-sigv4" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "54b1cbe0eee57a213039088dbdeca7be9352f24e0d72332d961e8a1cb388f82d" +dependencies = [ + "aws-credential-types", + "aws-smithy-http", + "aws-smithy-runtime-api", + "aws-smithy-types", + "bytes", + "form_urlencoded", + "hex", + "hmac", + "http 0.2.11", + "http 1.0.0", + "once_cell", + "percent-encoding", + "sha2", + "time", + "tracing", +] + +[[package]] +name = "aws-smithy-async" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "426a5bc369ca7c8d3686439e46edc727f397a47ab3696b13f3ae8c81b3b36132" +dependencies = [ + "futures-util", + "pin-project-lite", + "tokio", +] + +[[package]] +name = "aws-smithy-http" +version = "0.60.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85d6a0619f7b67183067fa3b558f94f90753da2df8c04aeb7336d673f804b0b8" +dependencies = [ + "aws-smithy-runtime-api", + "aws-smithy-types", + "bytes", + "bytes-utils", + "futures-core", + "http 0.2.11", + "http-body 0.4.6", + "once_cell", + "percent-encoding", + "pin-project-lite", + "pin-utils", + "tracing", +] + +[[package]] +name = "aws-smithy-json" +version = "0.60.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1c1b5186b6f5c579bf0de1bcca9dd3d946d6d51361ea1d18131f6a0b64e13ae" +dependencies = [ + "aws-smithy-types", +] + +[[package]] +name = "aws-smithy-query" +version = "0.60.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1c0a2ce65882e788d2cf83ff28b9b16918de0460c47bf66c5da4f6c17b4c9694" +dependencies = [ + "aws-smithy-types", + "urlencoding", +] + +[[package]] +name = "aws-smithy-runtime" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4cb6b3afa5fc9825a75675975dcc3e21764b5476bc91dbc63df4ea3d30a576e" +dependencies = [ + "aws-smithy-async", + "aws-smithy-http", + "aws-smithy-runtime-api", + "aws-smithy-types", + "bytes", + "fastrand", + "h2", + "http 0.2.11", + "http-body 0.4.6", + "hyper 0.14.28", + "hyper-rustls", + "once_cell", + "pin-project-lite", + "pin-utils", + "rustls 0.21.10", + "tokio", + "tracing", +] + +[[package]] +name = "aws-smithy-runtime-api" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23165433e80c04e8c09cee66d171292ae7234bae05fa9d5636e33095eae416b2" +dependencies = [ + "aws-smithy-async", + "aws-smithy-types", + "bytes", + "http 0.2.11", + "pin-project-lite", + "tokio", + "tracing", + "zeroize", +] + +[[package]] +name = "aws-smithy-types" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c94a5bec34850b92c9a054dad57b95c1d47f25125f55973e19f6ad788f0381ff" +dependencies = [ + "base64-simd", + "bytes", + "bytes-utils", + "futures-core", + "http 0.2.11", + "http-body 0.4.6", + "itoa", + "num-integer", + "pin-project-lite", + "pin-utils", + "ryu", + "serde", + "time", + "tokio", + "tokio-util", +] + +[[package]] +name = "aws-smithy-xml" +version = "0.60.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d16f94c9673412b7a72e3c3efec8de89081c320bf59ea12eed34c417a62ad600" +dependencies = [ + "xmlparser", +] + +[[package]] +name = "aws-types" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ff7e122ee50ca962e9de91f5850cc37e2184b1219611eef6d44aa85929b54f6" +dependencies = [ + "aws-credential-types", + "aws-smithy-async", + "aws-smithy-runtime-api", + "aws-smithy-types", + "http 0.2.11", + "rustc_version", + "tracing", +] + [[package]] name = "axum" version = "0.6.20" @@ -633,6 +948,16 @@ version = "0.21.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" +[[package]] +name = "base64-simd" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "339abbe78e73178762e23bea9dfd08e697eb3f3301cd4be981c0f78ba5859195" +dependencies = [ + "outref", + "vsimd", +] + [[package]] name = "base64ct" version = "1.6.0" @@ -878,6 +1203,16 @@ version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2bd12c1caf447e69cd4528f47f94d203fd2582878ecb9e9465484c4148a8223" +[[package]] +name = "bytes-utils" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7dafe3a8757b027e2be6e4e5601ed563c55989fcf1546e933c66c8eb3a058d35" +dependencies = [ + "bytes", + "either", +] + [[package]] name = "bytesize" version = "1.3.0" @@ -3502,7 +3837,7 @@ dependencies = [ "mirrord-protocol", "rand", "regex", - "rstest 0.17.0", + "rstest 0.18.2", "serde", "serde_json", "shellexpand", @@ -3551,7 +3886,7 @@ dependencies = [ "os_info", "rand", "regex", - "rstest 0.17.0", + "rstest 0.18.2", "serde", "serde_json", "socket2 0.5.5", @@ -3977,6 +4312,12 @@ dependencies = [ name = "outgoing" version = "3.86.1" +[[package]] +name = "outref" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4030760ffd992bef45b0ae3f10ce1aba99e33464c90d14dd7c039884963ddc7a" + [[package]] name = "overload" version = "0.1.1" @@ -4787,6 +5128,12 @@ dependencies = [ "regex-syntax 0.8.2", ] +[[package]] +name = "regex-lite" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "30b661b2f27137bdbc16f00eda72866a92bb28af1753ffbd56744fb6e2e9cd8e" + [[package]] name = "regex-syntax" version = "0.6.29" @@ -5860,6 +6207,8 @@ checksum = "8e7a7de15468c6e65dd7db81cf3822c1ec94c71b2a3c1a976ea8e4696c91115c" name = "tests" version = "0.1.0" dependencies = [ + "aws-config", + "aws-sdk-sqs", "bytes", "chrono", "const_format", @@ -6664,6 +7013,12 @@ version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d" +[[package]] +name = "vsimd" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c3082ca00d5a5ef149bb8b555a72ae84c9c59f7250f013ac822ac2e49b19c64" + [[package]] name = "walkdir" version = "2.4.0" @@ -7141,6 +7496,12 @@ version = "0.8.19" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fcb9cbac069e033553e8bb871be2fbdffcab578eb25bd0f7c508cedc6dcd75a" +[[package]] +name = "xmlparser" +version = "0.13.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "66fee0b777b0f5ac1c69bb06d361268faafa61cd4682ae064a171c16c433e9e4" + [[package]] name = "xz" version = "0.1.0" diff --git a/mirrord/operator/src/client.rs b/mirrord/operator/src/client.rs index 3c06e3c4083..f8851f5d795 100644 --- a/mirrord/operator/src/client.rs +++ b/mirrord/operator/src/client.rs @@ -34,7 +34,7 @@ use tokio_tungstenite::tungstenite::{Error as TungsteniteError, Message}; use tracing::{debug, error, warn}; use crate::crd::{ - CopyTargetCrd, CopyTargetSpec, MirrordOperatorCrd, OperatorFeatures, TargetCrd, + CopyTargetCrd, CopyTargetSpec, MirrordOperatorCrd, NewOperatorFeature, TargetCrd, OPERATOR_STATUS_NAME, }; @@ -95,7 +95,7 @@ pub struct OperatorSessionMetadata { client_certificate: Option, session_id: u64, fingerprint: Option, - operator_features: Vec, + operator_features: Vec, protocol_version: Option, copy_pod_enabled: Option, } @@ -104,7 +104,7 @@ impl OperatorSessionMetadata { fn new( client_certificate: Option, fingerprint: Option, - operator_features: Vec, + operator_features: Vec, protocol_version: Option, copy_pod_enabled: Option, ) -> Self { @@ -140,7 +140,8 @@ impl OperatorSessionMetadata { } fn proxy_feature_enabled(&self) -> bool { - self.operator_features.contains(&OperatorFeatures::ProxyApi) + self.operator_features + .contains(&NewOperatorFeature::ProxyApi) } } @@ -267,7 +268,7 @@ impl OperatorApi { let metadata = OperatorSessionMetadata::new( client_certificate, operator.spec.license.fingerprint, - operator.spec.features.unwrap_or_default(), + operator.spec.supported_features(), operator .spec .protocol_version diff --git a/mirrord/operator/src/crd.rs b/mirrord/operator/src/crd.rs index fd68ceefaac..853b0206555 100644 --- a/mirrord/operator/src/crd.rs +++ b/mirrord/operator/src/crd.rs @@ -91,10 +91,79 @@ pub static OPERATOR_STATUS_NAME: &str = "operator"; pub struct MirrordOperatorSpec { pub operator_version: String, pub default_namespace: String, - pub features: Option>, + /// Should be removed when we can stop supporting compatibility with versions from before the + /// `supported_features` field was added. + /// "Breaking" that compatibility by removing this field and then running with one old (from + /// before the `supported_features` field) side (client or operator) would make the client + /// think `ProxyApi` is not supported even if it is. + /// + /// Access this info only via `supported_features()`. + features: Option>, + /// Replaces both `features` and `copy_target_enabled`. Operator versions that use a version + /// of this code that has both this and the old fields are expected to populate this field with + /// the full set of features they support, and the old fields with their limited info they + /// support, for old clients. + /// + /// Access this info only via `supported_features()`. + supported_features: Option>, pub license: LicenseInfoOwned, pub protocol_version: Option, - pub copy_target_enabled: Option, + /// Should be removed when we can stop supporting compatibility with versions from before the + /// `supported_features` field was added. + /// "Breaking" that compatibility by removing this field and then running with one old (from + /// before the `supported_features` field) side (client or operator) would make the client + /// think copy target is not enabled even if it is. + /// + /// Access this info only via `supported_features()`. + copy_target_enabled: Option, +} + +impl MirrordOperatorSpec { + pub fn new( + operator_version: String, + default_namespace: String, + features: Option>, + supported_features: Option>, + license: LicenseInfoOwned, + protocol_version: Option, + copy_target_enabled: Option, + ) -> Self { + Self { + operator_version, + default_namespace, + features, + supported_features, + license, + protocol_version, + copy_target_enabled, + } + } + + /// Get a vector with the features the operator supports. + /// Handles objects sent from old and new operators. + // When the deprecated fields are removed, this can be changed to just return + // `self.supported_features.unwrap_or_default()`. + pub fn supported_features(&self) -> Vec { + self.supported_features + // if supported_features was sent, just use that. If not we are dealing with an older + // operator, so we build a vector of new features from the old fields. + .or_else(|| { + self.features.map(|features| { + features + .iter() + .map(From::from) + .chain( + self.copy_target_enabled.and_then(|enabled| { + enabled.then_some(NewOperatorFeature::CopyTarget) + }), + ) + .collect() + }) + }) + // Convert `None` to empty vector since we don't expect this to often be + // `None` (although it's ok if it is) and that way the return type is simpler. + .unwrap_or_default() + } } #[derive(Clone, Debug, Default, Deserialize, Serialize, JsonSchema)] @@ -134,9 +203,34 @@ pub struct LicenseInfoOwned { pub subscription_id: Option, } +/// Since this enum does not have a variant marked with `#[serde(other)]`, and is present like that +/// in released clients, existing clients would fail to parse any new variant. This means the +/// operator can never send anything but the one existing variant, otherwise the client will error +/// out. #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] pub enum OperatorFeatures { ProxyApi, + // DON'T ADD VARIANTS - old clients won't be able to deserialize them. + // Add new features in NewOperatorFeature +} + +#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] +pub enum NewOperatorFeature { + ProxyApi, + CopyTarget, + Sqs, + /// This variant is what a client sees when the operator includes a feature the client is not + /// yet aware of, because it was introduced in a version newer than the client's. + #[serde(other)] + FeatureFromTheFuture, +} + +impl From for NewOperatorFeature { + fn from(value: OperatorFeatures) -> Self { + match value { + OperatorFeatures::ProxyApi => NewOperatorFeature::ProxyApi, + } + } } /// This [`Resource`](kube::Resource) represents a copy pod created from an existing [`Target`]