chore(config): fix invalid enum schemas + provide more enum metadata (#…

…14586)

lib/codecs/Cargo.toml

@@ -27,6 +27,7 @@ tracing = { version = "0.1", default-features = false }
 value = { path = "../value", default-features = false }
 vector-common = { path = "../vector-common", default-features = false }
 vector-config = { path = "../vector-config", default-features = false }
+vector-config-common = { path = "../vector-config-common", default-features = false }
 vector-config-macros = { path = "../vector-config-macros", default-features = false }
 vector-core = { path = "../vector-core", default-features = false }
 

lib/vector-common/src/datetime.rs

@@ -3,10 +3,27 @@ use std::fmt::Debug;
 use chrono::{DateTime, Local, ParseError, TimeZone as _, Utc};
 use chrono_tz::Tz;
 use derivative::Derivative;
-use vector_config::configurable_component;
+use vector_config::{
+    schema::{
+        apply_metadata, generate_const_string_schema, generate_one_of_schema,
+        get_or_generate_schema,
+    },
+    schemars::{gen::SchemaGenerator, schema::SchemaObject},
+    Configurable, GenerateError, Metadata,
+};
+use vector_config_common::attributes::CustomAttribute;
 
 /// Timezone reference.
-#[configurable_component(no_deser, no_ser)]
+///
+/// This can refer to any valid timezone as defined in the [TZ database][tzdb], or "local" which
+/// refers to the system local timezone.
+///
+/// [tzdb]: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
+#[cfg_attr(
+    feature = "serde",
+    derive(::serde::Deserialize, ::serde::Serialize),
+    serde(try_from = "String", into = "String")
+)]
 #[derive(Clone, Copy, Debug, Derivative, Eq, PartialEq)]
 #[derivative(Default)]
 pub enum TimeZone {
@@ -19,7 +36,7 @@ pub enum TimeZone {
     /// Must be a valid name in the [TZ database][tzdb].
     ///
     /// [tzdb]: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
-    Named(#[configurable(transparent)] Tz),
+    Named(Tz),
 }
 
 /// This is a wrapper trait to allow `TimeZone` types to be passed generically.
@@ -54,41 +71,63 @@ pub(super) fn datetime_to_utc<TZ: chrono::TimeZone>(ts: &DateTime<TZ>) -> DateTi
     Utc.timestamp(ts.timestamp(), ts.timestamp_subsec_nanos())
 }
 
-#[cfg(feature = "serde")]
-pub mod ser_de {
-    use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
+impl From<TimeZone> for String {
+    fn from(tz: TimeZone) -> Self {
+        match tz {
+            TimeZone::Local => "local".to_string(),
+            TimeZone::Named(tz) => tz.name().to_string(),
+        }
+    }
+}
 
-    use super::TimeZone;
+impl TryFrom<String> for TimeZone {
+    type Error = String;
 
-    impl Serialize for TimeZone {
-        fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
-            match self {
-                Self::Local => serializer.serialize_str("local"),
-                Self::Named(tz) => serializer.serialize_str(tz.name()),
-            }
+    fn try_from(value: String) -> Result<Self, Self::Error> {
+        match TimeZone::parse(&value) {
+            Some(tz) => Ok(tz),
+            None => Err("No such time zone".to_string()),
         }
     }
+}
 
-    impl<'de> Deserialize<'de> for TimeZone {
-        fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
-            deserializer.deserialize_str(TimeZoneVisitor)
-        }
+// TODO: Consider an approach for generating schema of "fixed string value, or remainder" structure
+// used by this type.
+impl Configurable for TimeZone {
+    fn referenceable_name() -> Option<&'static str> {
+        Some(std::any::type_name::<Self>())
     }
 
-    struct TimeZoneVisitor;
+    fn metadata() -> vector_config::Metadata<Self> {
+        let mut metadata = vector_config::Metadata::default();
+        metadata.set_title("Timezone reference.");
+        metadata.set_description(r#"This can refer to any valid timezone as defined in the [TZ database][tzdb], or "local" which refers to the system local timezone.
 
-    impl<'de> de::Visitor<'de> for TimeZoneVisitor {
-        type Value = TimeZone;
+[tzdb]: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones"#);
+        metadata
+    }
 
-        fn expecting(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-            write!(f, "a time zone name")
-        }
+    fn generate_schema(gen: &mut SchemaGenerator) -> Result<SchemaObject, GenerateError> {
+        let mut local_schema = generate_const_string_schema("local".to_string());
+        let mut local_metadata = Metadata::<()>::with_description("System local timezone.");
+        local_metadata.add_custom_attribute(CustomAttribute::KeyValue {
+            key: "logical_name".to_string(),
+            value: "Local".to_string(),
+        });
+        apply_metadata(&mut local_schema, local_metadata);
 
-        fn visit_str<E: de::Error>(self, s: &str) -> Result<Self::Value, E> {
-            match TimeZone::parse(s) {
-                Some(tz) => Ok(tz),
-                None => Err(de::Error::custom("No such time zone")),
-            }
-        }
+        let mut tz_metadata = Metadata::with_title("A named timezone.");
+        tz_metadata.set_description(
+            r#"Must be a valid name in the [TZ database][tzdb].
+
+[tzdb]: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones"#,
+        );
+        tz_metadata.add_custom_attribute(CustomAttribute::KeyValue {
+            key: "logical_name".to_string(),
+            value: "Named".to_string(),
+        });
+        let tz_schema = get_or_generate_schema::<Tz>(gen, tz_metadata)?;
+
+        Ok(generate_one_of_schema(&[local_schema, tz_schema]))
     }
 }

lib/vector-config-macros/src/ast/variant.rs

@@ -1,4 +1,5 @@
 use darling::{error::Accumulator, util::Flag, FromAttributes};
+use proc_macro2::Ident;
 use serde_derive_internals::ast as serde_ast;
 use syn::spanned::Spanned;
 use vector_config_common::attributes::CustomAttribute;
@@ -50,6 +51,11 @@ impl<'a> Variant<'a> {
         accumulator.finish_with(variant)
     }
 
+    /// Ident of the variant.
+    pub fn ident(&self) -> &Ident {
+        &self.original.ident
+    }
+
     /// Style of the variant.
     ///
     /// This comes directly from `serde`, but effectively represents common terminology used outside

lib/vector-config-macros/src/configurable.rs

@@ -141,7 +141,7 @@ fn build_enum_generate_schema_fn(variants: &[Variant<'_>]) -> proc_macro2::Token
             let schema_metadata = <Self as ::vector_config::Configurable>::metadata();
             #(#mapped_variants)*
 
-            let mut schema = ::vector_config::schema::generate_composite_schema(&subschemas);
+            let mut schema = ::vector_config::schema::generate_one_of_schema(&subschemas);
             ::vector_config::schema::apply_metadata(&mut schema, schema_metadata);
 
             Ok(schema)
@@ -402,6 +402,20 @@ fn generate_variant_metadata(
         get_metadata_transparent(meta_ident, variant.tagging() == &Tagging::None);
     let maybe_custom_attributes = get_metadata_custom_attributes(meta_ident, variant.metadata());
 
+    // We add a special metadata key (`logical_name`) that informs consumers of the schema what the
+    // variant name is for this variant's subschema. While the doc comments being coerced into title
+    // and/or description are typically good enough, sometimes we need a more mechanical mapping of
+    // the variant's name since shoving it into the title would mean doc comments with redundant
+    // information.
+    //
+    // You can think of this as an enum-specific additional title.
+    let logical_name_attrs = vec![CustomAttribute::KeyValue {
+        key: "logical_name".to_string(),
+        value: variant.ident().to_string(),
+    }];
+    let variant_logical_name =
+        get_metadata_custom_attributes(meta_ident, logical_name_attrs.into_iter());
+
     // We specifically use `()` as the type here because we need to generate the metadata for this
     // variant, but there's no unique concrete type for a variant, only the type of the enum
     // container it exists within. We also don't want to use the metadata of the enum container, as
@@ -413,6 +427,7 @@ fn generate_variant_metadata(
         #maybe_deprecated
         #maybe_transparent
         #maybe_custom_attributes
+        #variant_logical_name
     }
 }
 

lib/vector-config/src/schema.rs

@@ -75,10 +75,6 @@ where
 }
 
 pub fn convert_to_flattened_schema(primary: &mut SchemaObject, mut subschemas: Vec<SchemaObject>) {
-    // Now we need to extract our object validation portion into a new schema object, add it to the list of subschemas,
-    // and then update the primary schema to use `allOf`. It is not valid to "extend" a schema via `allOf`, hence why we
-    // have to extract the primary schema object validation first.
-
     // First, we replace the primary schema with an empty schema, because we need to push it the actual primary schema
     // into the list of `allOf` schemas. This is due to the fact that it's not valid to "extend" a schema using `allOf`,
     // so everything has to be in there.
@@ -274,7 +270,7 @@ where
                 // outer field wrapping this schema, and it looks wonky to have it nested within the composite schema.
                 schema.metadata().description = None;
 
-                return Ok(generate_composite_schema(&[null, schema]))
+                return Ok(generate_one_of_schema(&[null, schema]))
             }
         },
         Some(sov) => match sov {
@@ -291,7 +287,7 @@ where
     Ok(schema)
 }
 
-pub fn generate_composite_schema(subschemas: &[SchemaObject]) -> SchemaObject {
+pub fn generate_one_of_schema(subschemas: &[SchemaObject]) -> SchemaObject {
     let subschemas = subschemas
         .iter()
         .map(|s| Schema::Object(s.clone()))
@@ -306,6 +302,21 @@ pub fn generate_composite_schema(subschemas: &[SchemaObject]) -> SchemaObject {
     }
 }
 
+pub fn generate_all_of_schema(subschemas: &[SchemaObject]) -> SchemaObject {
+    let subschemas = subschemas
+        .iter()
+        .map(|s| Schema::Object(s.clone()))
+        .collect::<Vec<_>>();
+
+    SchemaObject {
+        subschemas: Some(Box::new(SubschemaValidation {
+            all_of: Some(subschemas),
+            ..Default::default()
+        })),
+        ..Default::default()
+    }
+}
+
 pub fn generate_tuple_schema(subschemas: &[SchemaObject]) -> SchemaObject {
     let subschemas = subschemas
         .iter()
@@ -325,6 +336,13 @@ pub fn generate_tuple_schema(subschemas: &[SchemaObject]) -> SchemaObject {
     }
 }
 
+pub fn generate_enum_schema(values: Vec<Value>) -> SchemaObject {
+    SchemaObject {
+        enum_values: Some(values),
+        ..Default::default()
+    }
+}
+
 pub fn generate_const_string_schema(value: String) -> SchemaObject {
     SchemaObject {
         const_value: Some(Value::String(value)),

lib/vector-core/Cargo.toml

@@ -62,6 +62,7 @@ value = { path = "../value", default-features = false, features = ["lua", "toml"
 vector-buffers = { path = "../vector-buffers", default-features = false }
 vector-common = { path = "../vector-common" }
 vector-config = { path = "../vector-config" }
+vector-config-common = { path = "../vector-config-common" }
 vector-config-macros = { path = "../vector-config-macros" }
 # Rename to "vrl" once we use a release with stable `-Z namespaced-features`:
 # https://doc.rust-lang.org/cargo/reference/unstable.html#namespaced-features

lib/vrl/compiler/Cargo.toml

@@ -34,6 +34,7 @@ parser = { package = "vrl-parser", path = "../parser" }
 lookup = { path = "../../lookup" }
 vector-common = { path = "../../vector-common", default-features = false, features = ["conversion", "serde"] }
 vector-config = { path = "../../vector-config" }
+vector-config-common = { path = "../../vector-config-common" }
 vector-config-macros = { path = "../../vector-config-macros" }
 value = { path = "../../value" }
 

src/aws/auth.rs

@@ -18,7 +18,7 @@ const DEFAULT_LOAD_TIMEOUT: Duration = Duration::from_secs(5);
 #[serde(deny_unknown_fields, untagged)]
 pub enum AwsAuthentication {
     /// Authenticate using a fixed access key and secret pair.
-    Static {
+    AccessKey {
         /// The AWS access key ID.
         access_key_id: SensitiveString,
 
@@ -66,7 +66,7 @@ impl AwsAuthentication {
         service_region: Region,
     ) -> crate::Result<SharedCredentialsProvider> {
         match self {
-            Self::Static {
+            Self::AccessKey {
                 access_key_id,
                 secret_access_key,
             } => Ok(SharedCredentialsProvider::new(Credentials::from_keys(
@@ -97,7 +97,7 @@ impl AwsAuthentication {
 
     #[cfg(test)]
     pub fn test_auth() -> AwsAuthentication {
-        AwsAuthentication::Static {
+        AwsAuthentication::AccessKey {
             access_key_id: "dummy".to_string().into(),
             secret_access_key: "dummy".to_string().into(),
         }
@@ -220,7 +220,7 @@ mod tests {
         )
         .unwrap();
 
-        assert!(matches!(config.auth, AwsAuthentication::Static { .. }));
+        assert!(matches!(config.auth, AwsAuthentication::AccessKey { .. }));
     }
 
     #[test]