Remove 0.2 version checks in composition for headers & connect-on-type

apollo-federation/Cargo.toml

@@ -17,7 +17,6 @@ autotests = false                                      # Integration tests are m
 snapshot_tracing = ["ron"]
 # `correctness` feature enables the `correctness` module.
 correctness = []
-"connect_v0.2" = []
 
 [dependencies]
 apollo-compiler.workspace = true

apollo-federation/src/sources/connect/models.rs

@@ -25,7 +25,6 @@ use super::id::ConnectorPosition;
 use super::json_selection::ExternalVarPaths;
 use super::spec::schema::ConnectDirectiveArguments;
 use super::spec::schema::SourceDirectiveArguments;
-use super::spec::versions::VersionInfo;
 use super::variable::Namespace;
 use super::variable::VariableReference;
 use crate::error::FederationError;
@@ -113,14 +112,11 @@ impl Connector {
             return Ok(Default::default());
         };
 
-        let version: VersionInfo = spec.into();
-
         let source_name = ConnectSpec::source_directive_name(&link);
-        let source_arguments = extract_source_directive_arguments(schema, &source_name, &version)?;
+        let source_arguments = extract_source_directive_arguments(schema, &source_name)?;
 
         let connect_name = ConnectSpec::connect_directive_name(&link);
-        let connect_arguments =
-            extract_connect_directive_arguments(schema, &connect_name, &version)?;
+        let connect_arguments = extract_connect_directive_arguments(schema, &connect_name)?;
 
         connect_arguments
             .into_iter()

apollo-federation/src/sources/connect/models/http_json_transport.rs

@@ -12,6 +12,7 @@ use apollo_compiler::parser::SourceSpan;
 use either::Either;
 use http::HeaderName;
 use http::Uri;
+use http::header;
 use http::uri::InvalidUri;
 use http::uri::InvalidUriParts;
 use http::uri::Parts;
@@ -34,7 +35,6 @@ use crate::sources::connect::spec::schema::HEADERS_ARGUMENT_NAME;
 use crate::sources::connect::spec::schema::HTTP_HEADER_MAPPING_FROM_ARGUMENT_NAME;
 use crate::sources::connect::spec::schema::HTTP_HEADER_MAPPING_NAME_ARGUMENT_NAME;
 use crate::sources::connect::spec::schema::HTTP_HEADER_MAPPING_VALUE_ARGUMENT_NAME;
-use crate::sources::connect::spec::versions::AllowedHeaders;
 use crate::sources::connect::string_template;
 use crate::sources::connect::string_template::UriString;
 use crate::sources::connect::string_template::write_value;
@@ -362,14 +362,10 @@ impl<'a> Header<'a> {
     /// Get a list of headers from the `headers` argument in a `@connect` or `@source` directive.
     pub(crate) fn from_headers_arg(
         node: &'a Node<ast::Value>,
-        allowed_headers: &AllowedHeaders,
     ) -> Vec<Result<Self, HeaderParseError<'a>>> {
         match (node.as_list(), node.as_object()) {
-            (Some(values), _) => values
-                .iter()
-                .map(|v| Self::from_single(v, allowed_headers))
-                .collect(),
-            (None, Some(_)) => vec![Self::from_single(node, allowed_headers)],
+            (Some(values), _) => values.iter().map(Self::from_single).collect(),
+            (None, Some(_)) => vec![Self::from_single(node)],
             _ => vec![Err(HeaderParseError::Other {
                 message: format!("`{HEADERS_ARGUMENT_NAME}` must be an object or list of objects"),
                 node,
@@ -378,10 +374,7 @@ impl<'a> Header<'a> {
     }
 
     /// Build a single [`Self`] from a single entry in the `headers` arg.
-    fn from_single(
-        node: &'a Node<ast::Value>,
-        allowed_headers: &AllowedHeaders,
-    ) -> Result<Self, HeaderParseError<'a>> {
+    fn from_single(node: &'a Node<ast::Value>) -> Result<Self, HeaderParseError<'a>> {
         let mappings = node.as_object().ok_or_else(|| HeaderParseError::Other {
             message: "the HTTP header mapping is not an object".to_string(),
             node,
@@ -407,7 +400,7 @@ impl<'a> Header<'a> {
                 node: name_node,
             })?;
 
-        if allowed_headers.header_name_is_reserved(&name) {
+        if RESERVED_HEADERS.contains(&name) {
             return Err(HeaderParseError::Other {
                 message: format!("header '{name}' is reserved and cannot be set by a connector"),
                 node: name_node,
@@ -422,7 +415,7 @@ impl<'a> Header<'a> {
             .find(|(name, _value)| *name == HTTP_HEADER_MAPPING_VALUE_ARGUMENT_NAME);
 
         match (from, value) {
-            (Some(_), None) if allowed_headers.header_name_allowed_static(&name) => {
+            (Some(_), None) if STATIC_HEADERS.contains(&name) => {
                 Err(HeaderParseError::Other{ message: format!(
                     "header '{name}' can't be set with `{HTTP_HEADER_MAPPING_FROM_ARGUMENT_NAME}`, only with `{HTTP_HEADER_MAPPING_VALUE_ARGUMENT_NAME}`"
                 ), node: name_node})
@@ -507,6 +500,22 @@ impl Display for HeaderParseError<'_> {
 
 impl Error for HeaderParseError<'_> {}
 
+const RESERVED_HEADERS: [HeaderName; 11] = [
+    header::CONNECTION,
+    header::PROXY_AUTHENTICATE,
+    header::PROXY_AUTHORIZATION,
+    header::TE,
+    header::TRAILER,
+    header::TRANSFER_ENCODING,
+    header::UPGRADE,
+    header::CONTENT_LENGTH,
+    header::CONTENT_ENCODING,
+    header::ACCEPT_ENCODING,
+    HeaderName::from_static("keep-alive"),
+];
+
+const STATIC_HEADERS: [HeaderName; 3] = [header::CONTENT_TYPE, header::ACCEPT, header::HOST];
+
 #[cfg(test)]
 mod test_make_uri {
     use std::str::FromStr;

apollo-federation/src/sources/connect/spec/directives.rs

@@ -20,7 +20,6 @@ use super::schema::SOURCE_BASE_URL_ARGUMENT_NAME;
 use super::schema::SOURCE_NAME_ARGUMENT_NAME;
 use super::schema::SourceDirectiveArguments;
 use super::schema::SourceHTTPArguments;
-use super::versions::VersionInfo;
 use crate::error::FederationError;
 use crate::schema::position::InterfaceFieldDefinitionPosition;
 use crate::schema::position::ObjectOrInterfaceFieldDefinitionPosition;
@@ -41,21 +40,19 @@ macro_rules! internal {
 pub(crate) fn extract_source_directive_arguments(
     schema: &Schema,
     name: &Name,
-    version_info: &VersionInfo,
 ) -> Result<Vec<SourceDirectiveArguments>, FederationError> {
     schema
         .schema_definition
         .directives
         .iter()
         .filter(|directive| directive.name == *name)
-        .map(|d| SourceDirectiveArguments::from_directive(d, version_info))
+        .map(SourceDirectiveArguments::from_directive)
         .collect()
 }
 
 pub(crate) fn extract_connect_directive_arguments(
     schema: &Schema,
     name: &Name,
-    version_info: &VersionInfo,
 ) -> Result<Vec<ConnectDirectiveArguments>, FederationError> {
     // connect on fields
     schema
@@ -100,11 +97,7 @@ pub(crate) fn extract_connect_directive_arguments(
                                 directive_name: directive.name.clone(),
                                 directive_index: i,
                             });
-                        ConnectDirectiveArguments::from_position_and_directive(
-                            position,
-                            directive,
-                            version_info,
-                        )
+                        ConnectDirectiveArguments::from_position_and_directive(position, directive)
                     })
             })
         })
@@ -127,9 +120,7 @@ pub(crate) fn extract_connect_directive_arguments(
                                     directive_index: i,
                                 });
                             ConnectDirectiveArguments::from_position_and_directive(
-                                position,
-                                directive,
-                                version_info,
+                                position, directive,
                             )
                         })
                 }),
@@ -141,10 +132,7 @@ pub(crate) fn extract_connect_directive_arguments(
 type ObjectNode = [(Name, Node<Value>)];
 
 impl SourceDirectiveArguments {
-    fn from_directive(
-        value: &Component<Directive>,
-        version_info: &VersionInfo,
-    ) -> Result<Self, FederationError> {
+    fn from_directive(value: &Component<Directive>) -> Result<Self, FederationError> {
         let args = &value.arguments;
 
         // We'll have to iterate over the arg list and keep the properties by their name
@@ -161,7 +149,7 @@ impl SourceDirectiveArguments {
                 let http_value = arg.value.as_object().ok_or_else(|| {
                     internal!("`http` field in `@source` directive is not an object")
                 })?;
-                let http_value = SourceHTTPArguments::from_values(http_value, version_info)?;
+                let http_value = SourceHTTPArguments::from_values(http_value)?;
 
                 http = Some(http_value);
             } else {
@@ -181,10 +169,7 @@ impl SourceDirectiveArguments {
 }
 
 impl SourceHTTPArguments {
-    fn from_values(
-        values: &ObjectNode,
-        version_info: &VersionInfo,
-    ) -> Result<Self, FederationError> {
+    fn from_values(values: &ObjectNode) -> Result<Self, FederationError> {
         let mut base_url = None;
         let mut headers = None;
         let mut path = None;
@@ -204,7 +189,7 @@ impl SourceHTTPArguments {
                 );
             } else if name == HEADERS_ARGUMENT_NAME.as_str() {
                 headers = Some(
-                    Header::from_headers_arg(value, &version_info.allowed_headers)
+                    Header::from_headers_arg(value)
                         .into_iter()
                         .map_ok(|Header { name, source, .. }| (name, source))
                         .try_collect()
@@ -246,7 +231,6 @@ impl ConnectDirectiveArguments {
     fn from_position_and_directive(
         position: ConnectorPosition,
         value: &Node<Directive>,
-        version_info: &VersionInfo,
     ) -> Result<Self, FederationError> {
         let args = &value.arguments;
 
@@ -270,7 +254,7 @@ impl ConnectDirectiveArguments {
                     internal!("`http` field in `@connect` directive is not an object")
                 })?;
 
-                http = Some(ConnectHTTPArguments::from_values(http_value, version_info)?);
+                http = Some(ConnectHTTPArguments::from_values(http_value)?);
             } else if arg_name == "batch" {
                 let http_value = arg.value.as_object().ok_or_else(|| {
                     internal!("`http` field in `@connect` directive is not an object")
@@ -309,10 +293,7 @@ impl ConnectDirectiveArguments {
 }
 
 impl ConnectHTTPArguments {
-    fn from_values(
-        values: &ObjectNode,
-        version_info: &VersionInfo,
-    ) -> Result<Self, FederationError> {
+    fn from_values(values: &ObjectNode) -> Result<Self, FederationError> {
         let mut get = None;
         let mut post = None;
         let mut patch = None;
@@ -332,7 +313,7 @@ impl ConnectHTTPArguments {
                 body = Some(JSONSelection::parse(body_value).map_err(|e| internal!(e.message))?);
             } else if name == HEADERS_ARGUMENT_NAME.as_str() {
                 headers = Some(
-                    Header::from_headers_arg(value, &version_info.allowed_headers)
+                    Header::from_headers_arg(value)
                         .into_iter()
                         .map_ok(|Header { name, source, .. }| (name, source))
                         .try_collect()
@@ -420,7 +401,6 @@ mod tests {
 
     use crate::ValidFederationSubgraphs;
     use crate::schema::FederationSchema;
-    use crate::sources::connect::ConnectSpec;
     use crate::sources::connect::spec::schema::CONNECT_DIRECTIVE_NAME_IN_SPEC;
     use crate::sources::connect::spec::schema::SOURCE_DIRECTIVE_NAME_IN_SPEC;
     use crate::sources::connect::spec::schema::SourceDirectiveArguments;
@@ -490,7 +470,7 @@ mod tests {
 
         insta::assert_snapshot!(
             actual_definition.to_string(),
-            @"directive @connect(source: String, http: connect__ConnectHTTP, batch: connect__ConnectBatch, selection: connect__JSONSelection!, entity: Boolean = false) repeatable on FIELD_DEFINITION"
+            @"directive @connect(source: String, http: connect__ConnectHTTP, batch: connect__ConnectBatch, selection: connect__JSONSelection!, entity: Boolean = false) repeatable on FIELD_DEFINITION | OBJECT"
         );
 
         let fields = schema
@@ -532,7 +512,7 @@ mod tests {
             .directives
             .iter()
             .filter(|directive| directive.name == SOURCE_DIRECTIVE_NAME_IN_SPEC)
-            .map(|d| SourceDirectiveArguments::from_directive(d, &ConnectSpec::V0_1.into()))
+            .map(SourceDirectiveArguments::from_directive)
             .collect();
 
         insta::assert_debug_snapshot!(
@@ -578,11 +558,7 @@ mod tests {
         let schema = &subgraph.schema;
 
         // Extract the connects from the schema definition and map them to their `Connect` equivalent
-        let connects = super::extract_connect_directive_arguments(
-            schema.schema(),
-            &name!(connect),
-            &ConnectSpec::V0_1.into(),
-        );
+        let connects = super::extract_connect_directive_arguments(schema.schema(), &name!(connect));
 
         insta::assert_debug_snapshot!(
             connects.unwrap(),

apollo-federation/src/sources/connect/spec/mod.rs

@@ -17,15 +17,13 @@
 mod directives;
 pub(crate) mod schema;
 mod type_and_directive_specifications;
-pub(crate) mod versions;
 
 use std::fmt::Display;
 
 use apollo_compiler::Name;
 use apollo_compiler::Schema;
 use apollo_compiler::ast::Argument;
 use apollo_compiler::ast::Directive;
-use apollo_compiler::ast::DirectiveLocation;
 use apollo_compiler::ast::Value;
 use apollo_compiler::name;
 pub(crate) use directives::extract_connect_directive_arguments;
@@ -118,9 +116,7 @@ impl ConnectSpec {
             return Ok(());
         };
 
-        let spec = Self::try_from(&link.url.version)?;
-
-        type_and_directive_specifications::check_or_add(&link, &spec, schema)
+        type_and_directive_specifications::check_or_add(&link, schema)
     }
 
     pub(crate) fn source_directive_name(link: &Link) -> Name {
@@ -157,29 +153,13 @@ impl ConnectSpec {
             ],
         }
     }
-
-    pub(crate) const fn connect_directive_locations(&self) -> &'static [DirectiveLocation] {
-        match self {
-            ConnectSpec::V0_1 => CONNECT_V0_1_LOCATIONS,
-            ConnectSpec::V0_2 => CONNECT_V0_2_LOCATIONS,
-        }
-    }
-
-    pub(crate) const fn available() -> &'static [ConnectSpec] {
-        if cfg!(any(feature = "connect_v0.2", test)) {
-            &[ConnectSpec::V0_1, ConnectSpec::V0_2]
-        } else {
-            &[ConnectSpec::V0_1]
-        }
-    }
 }
 
 impl TryFrom<&Version> for ConnectSpec {
     type Error = SingleFederationError;
     fn try_from(version: &Version) -> Result<Self, Self::Error> {
         match (version.major, version.minor) {
             (0, 1) => Ok(Self::V0_1),
-            #[cfg(any(feature = "connect_v0.2", test))]
             (0, 2) => Ok(Self::V0_2),
             _ => Err(SingleFederationError::UnknownLinkVersion {
                 message: format!("Unknown connect version: {version}"),
@@ -202,9 +182,3 @@ impl From<ConnectSpec> for Version {
         }
     }
 }
-
-const CONNECT_V0_1_LOCATIONS: &[DirectiveLocation] = &[DirectiveLocation::FieldDefinition];
-const CONNECT_V0_2_LOCATIONS: &[DirectiveLocation] = &[
-    DirectiveLocation::FieldDefinition,
-    DirectiveLocation::Object,
-];