diff --git a/CODEOWNERS b/CODEOWNERS index 618354eee063..231f6e898236 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -379,6 +379,7 @@ extensions/filters/http/oauth2 @derekargueta @mattklein123 /*/extensions/clusters/logical_dns/ @UNOWNED @UNOWNED /*/extensions/clusters/common/ @UNOWNED @UNOWNED /*/extensions/clusters/eds/ @UNOWNED @UNOWNED +/*/extensions/clusters/dns @UNOWNED @UNOWNED /*/source/extensions/listener_managers/listener_manager @alyssawilk @ggreenway /*/source/extensions/listener_managers/validation_listener_manager @alyssawilk @ggreenway /*/source/extensions/config_subscription/ @adisuissa @UNOWNED diff --git a/api/BUILD b/api/BUILD index c71b978a6317..1638d2d1f699 100644 --- a/api/BUILD +++ b/api/BUILD @@ -137,6 +137,8 @@ proto_library( "//envoy/extensions/access_loggers/wasm/v3:pkg", "//envoy/extensions/bootstrap/internal_listener/v3:pkg", "//envoy/extensions/clusters/aggregate/v3:pkg", + "//envoy/extensions/clusters/common/dns/v3:pkg", + "//envoy/extensions/clusters/dns/v3:pkg", "//envoy/extensions/clusters/dynamic_forward_proxy/v3:pkg", "//envoy/extensions/clusters/redis/v3:pkg", "//envoy/extensions/common/async_files/v3:pkg", diff --git a/api/envoy/config/cluster/v3/cluster.proto b/api/envoy/config/cluster/v3/cluster.proto index 079a1e497758..f4c8e4b6c957 100644 --- a/api/envoy/config/cluster/v3/cluster.proto +++ b/api/envoy/config/cluster/v3/cluster.proto @@ -953,8 +953,15 @@ message Cluster { // :ref:`STRICT_DNS` // and :ref:`LOGICAL_DNS` // this setting is ignored. - google.protobuf.Duration dns_refresh_rate = 16 - [(validate.rules).duration = {gt {nanos: 1000000}}]; + // This field is deprecated in favor of using the :ref:`cluster_type` + // extension point and configuring it with :ref:`DnsCluster`. + // If :ref:`cluster_type` is configured with + // :ref:`DnsCluster`, this field will be ignored. + google.protobuf.Duration dns_refresh_rate = 16 [ + deprecated = true, + (validate.rules).duration = {gt {nanos: 1000000}}, + (envoy.annotations.deprecated_at_minor_version) = "3.0" + ]; // DNS jitter can be optionally specified if the cluster type is either // :ref:`STRICT_DNS`, @@ -965,7 +972,15 @@ message Cluster { // :ref:`STRICT_DNS` // and :ref:`LOGICAL_DNS` // this setting is ignored. - google.protobuf.Duration dns_jitter = 58 [(validate.rules).duration = {gte {}}]; + // This field is deprecated in favor of using the :ref:`cluster_type` + // extension point and configuring it with :ref:`DnsCluster`. + // If :ref:`cluster_type` is configured with + // :ref:`DnsCluster`, this field will be ignored. + google.protobuf.Duration dns_jitter = 58 [ + deprecated = true, + (validate.rules).duration = {gte {}}, + (envoy.annotations.deprecated_at_minor_version) = "3.0" + ]; // If the DNS failure refresh rate is specified and the cluster type is either // :ref:`STRICT_DNS`, @@ -975,16 +990,31 @@ message Cluster { // other than :ref:`STRICT_DNS` and // :ref:`LOGICAL_DNS` this setting is // ignored. - RefreshRate dns_failure_refresh_rate = 44; + // This field is deprecated in favor of using the :ref:`cluster_type` + // extension point and configuring it with :ref:`DnsCluster`. + // If :ref:`cluster_type` is configured with + // :ref:`DnsCluster`, this field will be ignored. + RefreshRate dns_failure_refresh_rate = 44 + [deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"]; // Optional configuration for setting cluster's DNS refresh rate. If the value is set to true, // cluster's DNS refresh rate will be set to resource record's TTL which comes from DNS // resolution. - bool respect_dns_ttl = 39; + // This field is deprecated in favor of using the :ref:`cluster_type` + // extension point and configuring it with :ref:`DnsCluster`. + // If :ref:`cluster_type` is configured with + // :ref:`DnsCluster`, this field will be ignored. + bool respect_dns_ttl = 39 + [deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"]; // The DNS IP address resolution policy. If this setting is not specified, the // value defaults to // :ref:`AUTO`. + // For logical and strict dns cluster, this field is deprecated in favor of using the + // :ref:`cluster_type` + // extension point and configuring it with :ref:`DnsCluster`. + // If :ref:`cluster_type` is configured with + // :ref:`DnsCluster`, this field will be ignored. DnsLookupFamily dns_lookup_family = 17 [(validate.rules).enum = {defined_only: true}]; // If DNS resolvers are specified and the cluster type is either @@ -1024,6 +1054,9 @@ message Cluster { // During the transition period when both ``dns_resolution_config`` and ``typed_dns_resolver_config`` exists, // when ``typed_dns_resolver_config`` is in place, Envoy will use it and ignore ``dns_resolution_config``. // When ``typed_dns_resolver_config`` is missing, the default behavior is in place. + // Also note that this field is deprecated for logical dns and strict dns clusters and will be ignored when + // :ref:`cluster_type` is configured with + // :ref:`DnsCluster`. // [#extension-category: envoy.network.dns_resolver] core.v3.TypedExtensionConfig typed_dns_resolver_config = 55; diff --git a/api/envoy/extensions/clusters/common/dns/v3/BUILD b/api/envoy/extensions/clusters/common/dns/v3/BUILD new file mode 100644 index 000000000000..29ebf0741406 --- /dev/null +++ b/api/envoy/extensions/clusters/common/dns/v3/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_xds//udpa/annotations:pkg"], +) diff --git a/api/envoy/extensions/clusters/common/dns/v3/dns.proto b/api/envoy/extensions/clusters/common/dns/v3/dns.proto new file mode 100644 index 000000000000..db4e31fb4d98 --- /dev/null +++ b/api/envoy/extensions/clusters/common/dns/v3/dns.proto @@ -0,0 +1,22 @@ +syntax = "proto3"; + +package envoy.extensions.clusters.common.dns.v3; + +import "udpa/annotations/status.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.clusters.common.dns.v3"; +option java_outer_classname = "DnsProto"; +option java_multiple_files = true; +option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/clusters/common/dns/v3;dnsv3"; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: DNS configuration for clusters] + +enum DnsLookupFamily { + UNSPECIFIED = 0; + AUTO = 1; + V4_ONLY = 2; + V6_ONLY = 3; + V4_PREFERRED = 4; + ALL = 5; +} diff --git a/api/envoy/extensions/clusters/dns/v3/BUILD b/api/envoy/extensions/clusters/dns/v3/BUILD new file mode 100644 index 000000000000..c8b8444932e9 --- /dev/null +++ b/api/envoy/extensions/clusters/dns/v3/BUILD @@ -0,0 +1,13 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = [ + "//envoy/config/core/v3:pkg", + "//envoy/extensions/clusters/common/dns/v3:pkg", + "@com_github_cncf_xds//udpa/annotations:pkg", + ], +) diff --git a/api/envoy/extensions/clusters/dns/v3/dns_cluster.proto b/api/envoy/extensions/clusters/dns/v3/dns_cluster.proto new file mode 100644 index 000000000000..4266541e67b8 --- /dev/null +++ b/api/envoy/extensions/clusters/dns/v3/dns_cluster.proto @@ -0,0 +1,92 @@ +syntax = "proto3"; + +package envoy.extensions.clusters.dns.v3; + +import "envoy/config/core/v3/extension.proto"; +import "envoy/extensions/clusters/common/dns/v3/dns.proto"; + +import "google/protobuf/duration.proto"; + +import "udpa/annotations/status.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.clusters.dns.v3"; +option java_outer_classname = "DnsClusterProto"; +option java_multiple_files = true; +option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/clusters/dns/v3;dnsv3"; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: DNS cluster configuration] + +// Configuration for DNS discovery clusters. +// [#extension: envoy.clusters.dns] + +// [#next-free-field: 10] +message DnsCluster { + message RefreshRate { + // Specifies the base interval between refreshes. This parameter is required and must be greater + // than zero and less than + // :ref:`max_interval `. + google.protobuf.Duration base_interval = 1 [(validate.rules).duration = { + required: true + gt {nanos: 1000000} + }]; + + // Specifies the maximum interval between refreshes. This parameter is optional, but must be + // greater than or equal to the + // :ref:`base_interval ` if set. The default + // is 10 times the :ref:`base_interval `. + google.protobuf.Duration max_interval = 2 [(validate.rules).duration = {gt {nanos: 1000000}}]; + } + + // This value is the cluster’s DNS refresh rate. The value configured must be at least 1ms. + // If this setting is not specified, the + // value defaults to 5000ms. + google.protobuf.Duration dns_refresh_rate = 3 [(validate.rules).duration = {gt {nanos: 1000000}}]; + + // This is the cluster’s DNS refresh rate when requests are failing. If this setting is + // not specified, the failure refresh rate defaults to the DNS refresh rate. + RefreshRate dns_failure_refresh_rate = 4; + + // Optional configuration for setting cluster's DNS refresh rate. If the value is set to true, + // cluster's DNS refresh rate will be set to resource record's TTL which comes from DNS + // resolution. + bool respect_dns_ttl = 5; + + // DNS jitter causes the cluster to refresh DNS entries later by a random amount of time to avoid a + // stampede of DNS requests. This value sets the upper bound (exclusive) for the random amount. + // There will be no jitter if this value is omitted. + google.protobuf.Duration dns_jitter = 6 [(validate.rules).duration = {gte {}}]; + + // DNS resolver type configuration extension. This extension can be used to configure c-ares, apple, + // or any other DNS resolver types and the related parameters. + // For example, an object of + // :ref:`CaresDnsResolverConfig` + // can be packed into this ``typed_dns_resolver_config``. This configuration replaces the + // :ref:`Cluster.typed_dns_resolver_config` + // configuration which replaces :ref:`Cluster.dns_resolution_config`. + // During the transition period when + // :ref:`DnsCluster.typed_dns_resolver_config`, + // :ref:`Cluster.typed_dns_resolver_config`, + // and :ref:`Cluster.dns_resolution_config` + // exist, Envoy will use + // :ref:`DnsCluster.typed_dns_resolver_config` + // and ignore + // DNS resolver-related fields in :ref:`Cluster` if the cluster is configured via the + // :ref:`Cluster.cluster_type` extension point with the + // :ref:`DnsCluster` extension type. + // Otherwise, see :ref:`Cluster.typed_dns_resolver_config`. + // [#extension-category: envoy.network.dns_resolver] + config.core.v3.TypedExtensionConfig typed_dns_resolver_config = 7; + + // The DNS IP address resolution policy. If this setting is not specified, the + // value defaults to + // :ref:`AUTO`. + common.dns.v3.DnsLookupFamily dns_lookup_family = 8; + + // If true, all returned addresses are considered to be associated with a single endpoint, + // which maps to :ref:`logical DNS discovery ` + // semantics. Otherwise, each address is considered to be a separate endpoint, which maps to + // :ref:`strict DNS discovery ` semantics. + bool all_addresses_in_single_endpoint = 9; +} diff --git a/api/versioning/BUILD b/api/versioning/BUILD index 7628d664bf65..f3aa2c40e691 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -75,6 +75,8 @@ proto_library( "//envoy/extensions/access_loggers/wasm/v3:pkg", "//envoy/extensions/bootstrap/internal_listener/v3:pkg", "//envoy/extensions/clusters/aggregate/v3:pkg", + "//envoy/extensions/clusters/common/dns/v3:pkg", + "//envoy/extensions/clusters/dns/v3:pkg", "//envoy/extensions/clusters/dynamic_forward_proxy/v3:pkg", "//envoy/extensions/clusters/redis/v3:pkg", "//envoy/extensions/common/async_files/v3:pkg", diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 030fddfa2054..bc553270271a 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -28,6 +28,7 @@ behavior_changes: but also means shadowing may take place for requests which are canceled mid-stream. This behavior change can be temporarily reverted by flipping ``envoy.reloadable_features.streaming_shadow`` to false. + minor_behavior_changes: # *Changes that may cause incompatibilities for some users, but should not for most* - area: access_log @@ -85,6 +86,15 @@ minor_behavior_changes: change: | Local replies now traverse the filter chain if 1xx headers have been sent to the client. This change can be reverted by setting the runtime guard ``envoy.reloadable_features.local_reply_traverses_filter_chain_after_1xx`` to false. +- area: cluster + change: | + Clusters can no longer use unregistered extension types in + :ref:`cluster_type`. +- area: cluster + change: | + Clusters factories are registered by configuration type for + :ref:`cluster_type` + and will use configuration type to lookup the corresponding factory when available. - area: dns change: | Patched c-ares to address CVE-2024-25629. @@ -341,6 +351,12 @@ deprecated: change: | metadata :ref:`metadata ` is now deprecated in the favor of :ref:`sourced_metadata `. +- area: cluster + change: | + `DNS-related fields in :ref:`Cluster are deprecated when using + strict and logical dns clusters. Instead, use the :ref:`cluster_type ` of type + :ref:`DnsCluster `. - area: aws_iam change: | The :ref:`aws_iam extension ` is deprecated and will be diff --git a/docs/root/api-v3/config/cluster/cluster.rst b/docs/root/api-v3/config/cluster/cluster.rst index f31111382c6c..53209c2677e4 100644 --- a/docs/root/api-v3/config/cluster/cluster.rst +++ b/docs/root/api-v3/config/cluster/cluster.rst @@ -5,4 +5,4 @@ Cluster :glob: :maxdepth: 2 - ../../extensions/clusters/*/v3/* + ../../extensions/clusters/**/v3/* diff --git a/source/common/common/BUILD b/source/common/common/BUILD index 4b168b30601a..ad3a0c273ff1 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -109,6 +109,7 @@ envoy_cc_library( "//envoy/network:dns_interface", "//source/common/network:utility_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/clusters/dns/v3:pkg_cc_proto", ], ) diff --git a/source/common/common/dns_utils.cc b/source/common/common/dns_utils.cc index f13531396427..40ec21ebc873 100644 --- a/source/common/common/dns_utils.cc +++ b/source/common/common/dns_utils.cc @@ -31,6 +31,27 @@ getDnsLookupFamilyFromEnum(envoy::config::cluster::v3::Cluster::DnsLookupFamily return Network::DnsLookupFamily::All; } +Network::DnsLookupFamily +getDnsLookupFamilyFromEnum(envoy::extensions::clusters::common::dns::v3::DnsLookupFamily family) { + switch (family) { + PANIC_ON_PROTO_ENUM_SENTINEL_VALUES; + case envoy::extensions::clusters::common::dns::v3::V6_ONLY: + return Network::DnsLookupFamily::V6Only; + case envoy::extensions::clusters::common::dns::v3::V4_ONLY: + return Network::DnsLookupFamily::V4Only; + case envoy::extensions::clusters::common::dns::v3::AUTO: + case envoy::extensions::clusters::common::dns::v3::UNSPECIFIED: + return Network::DnsLookupFamily::Auto; + case envoy::extensions::clusters::common::dns::v3::V4_PREFERRED: + return Network::DnsLookupFamily::V4Preferred; + case envoy::extensions::clusters::common::dns::v3::ALL: + return Network::DnsLookupFamily::All; + break; + } + IS_ENVOY_BUG("unexpected dns lookup family enum"); + return Network::DnsLookupFamily::All; +} + std::vector generateAddressList(const std::list& responses, uint32_t port) { std::vector addresses; diff --git a/source/common/common/dns_utils.h b/source/common/common/dns_utils.h index 869a91d2a236..04d7b1a67dcb 100644 --- a/source/common/common/dns_utils.h +++ b/source/common/common/dns_utils.h @@ -1,6 +1,7 @@ #pragma once #include "envoy/config/cluster/v3/cluster.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.h" #include "envoy/network/dns.h" namespace Envoy { @@ -13,6 +14,8 @@ Network::DnsLookupFamily getDnsLookupFamilyFromCluster(const envoy::config::cluster::v3::Cluster& cluster); Network::DnsLookupFamily getDnsLookupFamilyFromEnum(envoy::config::cluster::v3::Cluster::DnsLookupFamily family); +Network::DnsLookupFamily +getDnsLookupFamilyFromEnum(envoy::extensions::clusters::common::dns::v3::DnsLookupFamily family); // Generates a list of InstanceConstSharedPtr from the DNS responses provided. std::vector diff --git a/source/common/upstream/cluster_factory_impl.cc b/source/common/upstream/cluster_factory_impl.cc index a4324b8d472d..2d1754d7e6de 100644 --- a/source/common/upstream/cluster_factory_impl.cc +++ b/source/common/upstream/cluster_factory_impl.cc @@ -8,6 +8,7 @@ #include "source/common/network/dns_resolver/dns_factory_util.h" #include "source/common/network/resolver_impl.h" #include "source/common/network/socket_option_factory.h" +#include "source/common/protobuf/protobuf.h" #include "source/common/upstream/health_checker_impl.h" #include "source/server/transport_socket_config_impl.h" @@ -21,29 +22,50 @@ ClusterFactoryImplBase::create(const envoy::config::cluster::v3::Cluster& cluste Ssl::ContextManager& ssl_context_manager, Outlier::EventLoggerSharedPtr outlier_event_logger, bool added_via_api) { - std::string cluster_type; - - if (!cluster.has_cluster_type()) { - switch (cluster.type()) { - PANIC_ON_PROTO_ENUM_SENTINEL_VALUES; - case envoy::config::cluster::v3::Cluster::STATIC: - cluster_type = "envoy.cluster.static"; - break; - case envoy::config::cluster::v3::Cluster::STRICT_DNS: - cluster_type = "envoy.cluster.strict_dns"; - break; - case envoy::config::cluster::v3::Cluster::LOGICAL_DNS: - cluster_type = "envoy.cluster.logical_dns"; - break; - case envoy::config::cluster::v3::Cluster::ORIGINAL_DST: - cluster_type = "envoy.cluster.original_dst"; - break; - case envoy::config::cluster::v3::Cluster::EDS: - cluster_type = "envoy.cluster.eds"; - break; + std::string cluster_name; + std::string cluster_config_type_name; + + ClusterFactory* factory; + // try to look up by typed_config + if (cluster.has_cluster_type() && cluster.cluster_type().has_typed_config() && + (TypeUtil::typeUrlToDescriptorFullName(cluster.cluster_type().typed_config().type_url()) != + ProtobufWkt::Struct::GetDescriptor()->full_name())) { + cluster_config_type_name = + TypeUtil::typeUrlToDescriptorFullName(cluster.cluster_type().typed_config().type_url()); + factory = Registry::FactoryRegistry::getFactoryByType(cluster_config_type_name); + if (factory == nullptr) { + return absl::InvalidArgumentError( + fmt::format("Didn't find a registered cluster factory implementation for type: '{}'", + cluster_config_type_name)); } } else { - cluster_type = cluster.cluster_type().name(); + if (!cluster.has_cluster_type()) { + switch (cluster.type()) { + PANIC_ON_PROTO_ENUM_SENTINEL_VALUES; + case envoy::config::cluster::v3::Cluster::STATIC: + cluster_name = "envoy.cluster.static"; + break; + case envoy::config::cluster::v3::Cluster::STRICT_DNS: + cluster_name = "envoy.cluster.strict_dns"; + break; + case envoy::config::cluster::v3::Cluster::LOGICAL_DNS: + cluster_name = "envoy.cluster.logical_dns"; + break; + case envoy::config::cluster::v3::Cluster::ORIGINAL_DST: + cluster_name = "envoy.cluster.original_dst"; + break; + case envoy::config::cluster::v3::Cluster::EDS: + cluster_name = "envoy.cluster.eds"; + break; + } + } else { + cluster_name = cluster.cluster_type().name(); + } + factory = Registry::FactoryRegistry::getFactory(cluster_name); + if (factory == nullptr) { + return absl::InvalidArgumentError(fmt::format( + "Didn't find a registered cluster factory implementation for name: '{}'", cluster_name)); + } } if (cluster.common_lb_config().has_consistent_hashing_lb_config() && @@ -51,13 +73,7 @@ ClusterFactoryImplBase::create(const envoy::config::cluster::v3::Cluster& cluste cluster.type() != envoy::config::cluster::v3::Cluster::STRICT_DNS) { return absl::InvalidArgumentError(fmt::format( "Cannot use hostname for consistent hashing loadbalancing for cluster of type: '{}'", - cluster_type)); - } - ClusterFactory* factory = Registry::FactoryRegistry::getFactory(cluster_type); - - if (factory == nullptr) { - return absl::InvalidArgumentError(fmt::format( - "Didn't find a registered cluster factory implementation for name: '{}'", cluster_type)); + cluster_name)); } ClusterFactoryContextImpl context(server_context, cm, dns_resolver_fn, ssl_context_manager, diff --git a/source/common/upstream/cluster_factory_impl.h b/source/common/upstream/cluster_factory_impl.h index 8844dc629a3d..fe7da2846eb2 100644 --- a/source/common/upstream/cluster_factory_impl.h +++ b/source/common/upstream/cluster_factory_impl.h @@ -149,6 +149,13 @@ template class ConfigurableClusterFactoryBase : public Clust return std::make_unique(); } + std::set configTypes() override { + auto ptr = createEmptyConfigProto(); + ASSERT(ptr != nullptr); + Protobuf::ReflectableMessage reflectable_message = createReflectableMessage(*ptr); + return {std::string(reflectable_message->GetDescriptor()->full_name())}; + } + protected: ConfigurableClusterFactoryBase(const std::string& name) : ClusterFactoryImplBase(name) {} diff --git a/source/extensions/clusters/common/BUILD b/source/extensions/clusters/common/BUILD index cf520569c34b..b36c45e6924a 100644 --- a/source/extensions/clusters/common/BUILD +++ b/source/extensions/clusters/common/BUILD @@ -18,3 +18,15 @@ envoy_cc_library( "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", ], ) + +envoy_cc_library( + name = "dns_cluster_backcompat_lib", + srcs = ["dns_cluster_backcompat.cc"], + hdrs = ["dns_cluster_backcompat.h"], + deps = [ + "//source/common/upstream:cluster_factory_includes", + "//source/common/upstream:upstream_includes", + "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/clusters/dns/v3:pkg_cc_proto", + ], +) diff --git a/source/extensions/clusters/common/dns_cluster_backcompat.cc b/source/extensions/clusters/common/dns_cluster_backcompat.cc new file mode 100644 index 000000000000..c3f4a6fa4941 --- /dev/null +++ b/source/extensions/clusters/common/dns_cluster_backcompat.cc @@ -0,0 +1,59 @@ +#include "source/extensions/clusters/common/dns_cluster_backcompat.h" + +#include "envoy/common/exception.h" +#include "envoy/config/cluster/v3/cluster.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.h" + +namespace Envoy { +namespace Upstream { + +void createDnsClusterFromLegacyFields( + const envoy::config::cluster::v3::Cluster& cluster, + envoy::extensions::clusters::dns::v3::DnsCluster& dns_cluster) { + + // We have to add all these guards because otherwise dns_cluster.mutable_FIELD will initialize the + // field and dns_cluster.has_FIELD will return true + if (cluster.has_dns_refresh_rate()) { + dns_cluster.mutable_dns_refresh_rate()->CopyFrom(cluster.dns_refresh_rate()); + } + + if (cluster.has_dns_failure_refresh_rate()) { + auto* new_refresh_rate = dns_cluster.mutable_dns_failure_refresh_rate(); + const auto& old_refresh_rate = cluster.dns_failure_refresh_rate(); + + if (old_refresh_rate.has_max_interval()) { + new_refresh_rate->mutable_max_interval()->CopyFrom(old_refresh_rate.max_interval()); + } + if (old_refresh_rate.has_base_interval()) { + new_refresh_rate->mutable_base_interval()->CopyFrom(old_refresh_rate.base_interval()); + } + } + + dns_cluster.set_respect_dns_ttl(cluster.respect_dns_ttl()); + + if (cluster.has_dns_jitter()) { + dns_cluster.mutable_dns_jitter()->CopyFrom(cluster.dns_jitter()); + } + + switch (cluster.dns_lookup_family()) { + PANIC_ON_PROTO_ENUM_SENTINEL_VALUES; + case envoy::config::cluster::v3::Cluster::AUTO: + dns_cluster.set_dns_lookup_family(envoy::extensions::clusters::common::dns::v3::AUTO); + break; + case envoy::config::cluster::v3::Cluster::V4_ONLY: + dns_cluster.set_dns_lookup_family(envoy::extensions::clusters::common::dns::v3::V4_ONLY); + break; + case envoy::config::cluster::v3::Cluster::V6_ONLY: + dns_cluster.set_dns_lookup_family(envoy::extensions::clusters::common::dns::v3::V6_ONLY); + break; + case envoy::config::cluster::v3::Cluster::V4_PREFERRED: + dns_cluster.set_dns_lookup_family(envoy::extensions::clusters::common::dns::v3::V4_PREFERRED); + break; + case envoy::config::cluster::v3::Cluster::ALL: + dns_cluster.set_dns_lookup_family(envoy::extensions::clusters::common::dns::v3::ALL); + break; + } +} + +} // namespace Upstream +} // namespace Envoy diff --git a/source/extensions/clusters/common/dns_cluster_backcompat.h b/source/extensions/clusters/common/dns_cluster_backcompat.h new file mode 100644 index 000000000000..d274dd737cc8 --- /dev/null +++ b/source/extensions/clusters/common/dns_cluster_backcompat.h @@ -0,0 +1,19 @@ +#pragma once + +#include "envoy/config/cluster/v3/cluster.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.h" + +namespace Envoy { +namespace Upstream { + +/** + * create a DnsCluster from the legacy Cluster options so that we only have to worry about one API. + * NOTE: this does not consider the `typed_dns_resolver_config` field. + * because choosing the dns resolver is done by the factory. + */ +void createDnsClusterFromLegacyFields( + const envoy::config::cluster::v3::Cluster& cluster, + envoy::extensions::clusters::dns::v3::DnsCluster& new_proto_config); + +} // namespace Upstream +} // namespace Envoy diff --git a/source/extensions/clusters/dns/BUILD b/source/extensions/clusters/dns/BUILD new file mode 100644 index 000000000000..29df85897b31 --- /dev/null +++ b/source/extensions/clusters/dns/BUILD @@ -0,0 +1,25 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_extension", + "envoy_extension_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_extension_package() + +envoy_cc_extension( + name = "dns_cluster_lib", + srcs = ["dns_cluster.cc"], + hdrs = ["dns_cluster.h"], + visibility = ["//visibility:public"], + deps = [ + "//source/common/upstream:cluster_factory_includes", + "//source/common/upstream:upstream_includes", + "//source/extensions/clusters/logical_dns:logical_dns_cluster_lib", + "//source/extensions/clusters/strict_dns:strict_dns_cluster_lib", + "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", + "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/clusters/dns/v3:pkg_cc_proto", + ], +) diff --git a/source/extensions/clusters/dns/dns_cluster.cc b/source/extensions/clusters/dns/dns_cluster.cc new file mode 100644 index 000000000000..7aa22aa3a464 --- /dev/null +++ b/source/extensions/clusters/dns/dns_cluster.cc @@ -0,0 +1,56 @@ +#include "source/extensions/clusters/dns/dns_cluster.h" + +#include + +#include "envoy/common/exception.h" +#include "envoy/config/cluster/v3/cluster.pb.h" +#include "envoy/config/endpoint/v3/endpoint.pb.h" +#include "envoy/config/endpoint/v3/endpoint_components.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.h" + +#include "source/common/network/dns_resolver/dns_factory_util.h" +#include "source/extensions/clusters/common/dns_cluster_backcompat.h" +#include "source/extensions/clusters/logical_dns/logical_dns_cluster.h" +#include "source/extensions/clusters/strict_dns/strict_dns_cluster.h" + +namespace Envoy { +namespace Upstream { + +absl::StatusOr> +DnsClusterFactory::createClusterWithConfig( + const envoy::config::cluster::v3::Cluster& cluster, + const envoy::extensions::clusters::dns::v3::DnsCluster& proto_config, + Upstream::ClusterFactoryContext& context) { + + absl::StatusOr dns_resolver_or_error; + if (proto_config.has_typed_dns_resolver_config()) { + Network::DnsResolverFactory& dns_resolver_factory = + Network::createDnsResolverFactoryFromTypedConfig(proto_config.typed_dns_resolver_config()); + auto& server_context = context.serverFactoryContext(); + dns_resolver_or_error = dns_resolver_factory.createDnsResolver( + server_context.mainThreadDispatcher(), server_context.api(), + proto_config.typed_dns_resolver_config()); + } else { + dns_resolver_or_error = context.dnsResolver(); + } + RETURN_IF_NOT_OK(dns_resolver_or_error.status()); + absl::StatusOr> cluster_or_error; + if (proto_config.all_addresses_in_single_endpoint()) { + cluster_or_error = LogicalDnsCluster::create(cluster, proto_config, context, + std::move(*dns_resolver_or_error)); + } else { + cluster_or_error = StrictDnsClusterImpl::create(cluster, proto_config, context, + std::move(*dns_resolver_or_error)); + } + + RETURN_IF_NOT_OK(cluster_or_error.status()); + return std::make_pair(std::shared_ptr(std::move(*cluster_or_error)), nullptr); +} + +/** + * Static registration for the dns cluster factory. @see RegisterFactory. + */ +REGISTER_FACTORY(DnsClusterFactory, Upstream::ClusterFactory); + +} // namespace Upstream +} // namespace Envoy diff --git a/source/extensions/clusters/dns/dns_cluster.h b/source/extensions/clusters/dns/dns_cluster.h new file mode 100644 index 000000000000..f198e09cef54 --- /dev/null +++ b/source/extensions/clusters/dns/dns_cluster.h @@ -0,0 +1,37 @@ +#pragma once + +#include "envoy/config/cluster/v3/cluster.pb.h" +#include "envoy/config/endpoint/v3/endpoint_components.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.validate.h" + +#include "source/common/upstream/cluster_factory_impl.h" +#include "source/common/upstream/upstream_impl.h" + +namespace Envoy { +namespace Upstream { + +class LogicalDnsClusterTest; + +/** + * Factory for DnsClusterImpl + */ + +class DnsClusterFactory : public Upstream::ConfigurableClusterFactoryBase< + envoy::extensions::clusters::dns::v3::DnsCluster> { +public: + DnsClusterFactory() : ConfigurableClusterFactoryBase("envoy.cluster.dns") {} + +private: + friend class LogicalDnsClusterTest; + absl::StatusOr< + std::pair> + createClusterWithConfig(const envoy::config::cluster::v3::Cluster& cluster, + const envoy::extensions::clusters::dns::v3::DnsCluster& proto_config, + Upstream::ClusterFactoryContext& context) override; +}; + +DECLARE_FACTORY(DnsClusterFactory); + +} // namespace Upstream +} // namespace Envoy diff --git a/source/extensions/clusters/logical_dns/BUILD b/source/extensions/clusters/logical_dns/BUILD index fe4b358b31a2..1ba2bd814c4a 100644 --- a/source/extensions/clusters/logical_dns/BUILD +++ b/source/extensions/clusters/logical_dns/BUILD @@ -21,13 +21,16 @@ envoy_cc_extension( "//source/common/config:utility_lib", "//source/common/network:address_lib", "//source/common/network:utility_lib", + "//source/common/network/dns_resolver:dns_factory_util_lib", "//source/common/protobuf", "//source/common/protobuf:utility_lib", "//source/common/upstream:cluster_factory_lib", "//source/common/upstream:upstream_includes", + "//source/extensions/clusters/common:dns_cluster_backcompat_lib", "//source/extensions/clusters/common:logical_host_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/clusters/dns/v3:pkg_cc_proto", ], ) diff --git a/source/extensions/clusters/logical_dns/logical_dns_cluster.cc b/source/extensions/clusters/logical_dns/logical_dns_cluster.cc index 5bb36332d355..dab8e011e05a 100644 --- a/source/extensions/clusters/logical_dns/logical_dns_cluster.cc +++ b/source/extensions/clusters/logical_dns/logical_dns_cluster.cc @@ -10,15 +10,18 @@ #include "envoy/config/cluster/v3/cluster.pb.h" #include "envoy/config/core/v3/address.pb.h" #include "envoy/config/endpoint/v3/endpoint.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.h" #include "envoy/stats/scope.h" #include "source/common/common/dns_utils.h" #include "source/common/common/fmt.h" #include "source/common/config/utility.h" #include "source/common/network/address_impl.h" +#include "source/common/network/dns_resolver/dns_factory_util.h" #include "source/common/network/utility.h" #include "source/common/protobuf/protobuf.h" #include "source/common/protobuf/utility.h" +#include "source/extensions/clusters/common/dns_cluster_backcompat.h" namespace Envoy { namespace Upstream { @@ -44,23 +47,58 @@ convertPriority(const envoy::config::endpoint::v3::ClusterLoadAssignment& load_a } } // namespace -LogicalDnsCluster::LogicalDnsCluster(const envoy::config::cluster::v3::Cluster& cluster, - ClusterFactoryContext& context, - Network::DnsResolverSharedPtr dns_resolver, - absl::Status& creation_status) +absl::StatusOr> +LogicalDnsCluster::create(const envoy::config::cluster::v3::Cluster& cluster, + const envoy::extensions::clusters::dns::v3::DnsCluster& dns_cluster, + ClusterFactoryContext& context, + Network::DnsResolverSharedPtr dns_resolver) { + const auto& load_assignment = cluster.load_assignment(); + const auto& locality_lb_endpoints = load_assignment.endpoints(); + if (locality_lb_endpoints.size() != 1 || locality_lb_endpoints[0].lb_endpoints().size() != 1) { + if (cluster.has_load_assignment()) { + return absl::InvalidArgumentError( + "LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint"); + } else { + return absl::InvalidArgumentError("LOGICAL_DNS clusters must have a single host"); + } + } + + const envoy::config::core::v3::SocketAddress& socket_address = + locality_lb_endpoints[0].lb_endpoints()[0].endpoint().address().socket_address(); + if (!socket_address.resolver_name().empty()) { + return absl::InvalidArgumentError( + "LOGICAL_DNS clusters must NOT have a custom resolver name set"); + } + + absl::Status creation_status = absl::OkStatus(); + std::unique_ptr ret; + + ret = std::unique_ptr(new LogicalDnsCluster( + cluster, dns_cluster, context, std::move(dns_resolver), creation_status)); + RETURN_IF_NOT_OK(creation_status); + return ret; +} + +LogicalDnsCluster::LogicalDnsCluster( + const envoy::config::cluster::v3::Cluster& cluster, + const envoy::extensions::clusters::dns::v3::DnsCluster& dns_cluster, + ClusterFactoryContext& context, Network::DnsResolverSharedPtr dns_resolver, + absl::Status& creation_status) : ClusterImplBase(cluster, context, creation_status), dns_resolver_(dns_resolver), - dns_refresh_rate_ms_( - std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(cluster, dns_refresh_rate, 5000))), - dns_jitter_ms_(std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(cluster, dns_jitter, 0))), - respect_dns_ttl_(cluster.respect_dns_ttl()), + dns_refresh_rate_ms_(std::chrono::milliseconds( + PROTOBUF_GET_MS_OR_DEFAULT(dns_cluster, dns_refresh_rate, 5000))), + dns_jitter_ms_( + std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(dns_cluster, dns_jitter, 0))), + respect_dns_ttl_(dns_cluster.respect_dns_ttl()), + dns_lookup_family_( + Envoy::DnsUtils::getDnsLookupFamilyFromEnum(dns_cluster.dns_lookup_family())), resolve_timer_(context.serverFactoryContext().mainThreadDispatcher().createTimer( [this]() -> void { startResolve(); })), local_info_(context.serverFactoryContext().localInfo()), load_assignment_(convertPriority(cluster.load_assignment())) { - failure_backoff_strategy_ = - Config::Utility::prepareDnsRefreshStrategy( - cluster, dns_refresh_rate_ms_.count(), - context.serverFactoryContext().api().randomGenerator()); + failure_backoff_strategy_ = Config::Utility::prepareDnsRefreshStrategy( + dns_cluster, dns_refresh_rate_ms_.count(), + context.serverFactoryContext().api().randomGenerator()); const envoy::config::core::v3::SocketAddress& socket_address = lbEndpoint().endpoint().address().socket_address(); @@ -75,7 +113,6 @@ LogicalDnsCluster::LogicalDnsCluster(const envoy::config::cluster::v3::Cluster& } else { hostname_ = lbEndpoint().endpoint().hostname(); } - dns_lookup_family_ = getDnsLookupFamilyFromCluster(cluster); } void LogicalDnsCluster::startPreInit() { @@ -182,31 +219,14 @@ LogicalDnsClusterFactory::createClusterImpl(const envoy::config::cluster::v3::Cl auto dns_resolver_or_error = selectDnsResolver(cluster, context); THROW_IF_NOT_OK_REF(dns_resolver_or_error.status()); - const auto& load_assignment = cluster.load_assignment(); - const auto& locality_lb_endpoints = load_assignment.endpoints(); - if (locality_lb_endpoints.size() != 1 || locality_lb_endpoints[0].lb_endpoints().size() != 1) { - if (cluster.has_load_assignment()) { - return absl::InvalidArgumentError( - "LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint"); - } else { - return absl::InvalidArgumentError("LOGICAL_DNS clusters must have a single host"); - } - } - - const envoy::config::core::v3::SocketAddress& socket_address = - locality_lb_endpoints[0].lb_endpoints()[0].endpoint().address().socket_address(); - if (!socket_address.resolver_name().empty()) { - return absl::InvalidArgumentError( - "LOGICAL_DNS clusters must NOT have a custom resolver name set"); - } + absl::StatusOr> cluster_or_error; + envoy::extensions::clusters::dns::v3::DnsCluster proto_config_legacy{}; + createDnsClusterFromLegacyFields(cluster, proto_config_legacy); + cluster_or_error = LogicalDnsCluster::create(cluster, proto_config_legacy, context, + std::move(*dns_resolver_or_error)); - absl::Status creation_status = absl::OkStatus(); - auto ret = std::make_pair( - std::shared_ptr(new LogicalDnsCluster( - cluster, context, std::move(dns_resolver_or_error.value()), creation_status)), - nullptr); - RETURN_IF_NOT_OK(creation_status); - return ret; + RETURN_IF_NOT_OK(cluster_or_error.status()); + return std::make_pair(std::shared_ptr(std::move(*cluster_or_error)), nullptr); } /** diff --git a/source/extensions/clusters/logical_dns/logical_dns_cluster.h b/source/extensions/clusters/logical_dns/logical_dns_cluster.h index 8522af08edd8..975a494ca94a 100644 --- a/source/extensions/clusters/logical_dns/logical_dns_cluster.h +++ b/source/extensions/clusters/logical_dns/logical_dns_cluster.h @@ -8,6 +8,8 @@ #include "envoy/config/cluster/v3/cluster.pb.h" #include "envoy/config/endpoint/v3/endpoint.pb.h" #include "envoy/config/endpoint/v3/endpoint_components.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.validate.h" #include "envoy/stats/scope.h" #include "source/common/common/empty_string.h" @@ -41,8 +43,14 @@ class LogicalDnsCluster : public ClusterImplBase { // Upstream::Cluster InitializePhase initializePhase() const override { return InitializePhase::Primary; } + static absl::StatusOr> + create(const envoy::config::cluster::v3::Cluster& cluster, + const envoy::extensions::clusters::dns::v3::DnsCluster& dns_cluster, + ClusterFactoryContext& context, Network::DnsResolverSharedPtr dns_resolver); + protected: LogicalDnsCluster(const envoy::config::cluster::v3::Cluster& cluster, + const envoy::extensions::clusters::dns::v3::DnsCluster& dns_cluster, ClusterFactoryContext& context, Network::DnsResolverSharedPtr dns_resolver, absl::Status& creation_status); diff --git a/source/extensions/clusters/strict_dns/BUILD b/source/extensions/clusters/strict_dns/BUILD index 72fdb583d55b..76e7b060aaa1 100644 --- a/source/extensions/clusters/strict_dns/BUILD +++ b/source/extensions/clusters/strict_dns/BUILD @@ -15,9 +15,13 @@ envoy_cc_extension( # prevously considered core code. visibility = ["//visibility:public"], deps = [ + "//source/common/common:dns_utils_lib", + "//source/common/network/dns_resolver:dns_factory_util_lib", "//source/common/upstream:cluster_factory_includes", "//source/common/upstream:upstream_includes", + "//source/extensions/clusters/common:dns_cluster_backcompat_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/clusters/dns/v3:pkg_cc_proto", ], ) diff --git a/source/extensions/clusters/strict_dns/strict_dns_cluster.cc b/source/extensions/clusters/strict_dns/strict_dns_cluster.cc index fdd72c5a37c7..b8fe1315516f 100644 --- a/source/extensions/clusters/strict_dns/strict_dns_cluster.cc +++ b/source/extensions/clusters/strict_dns/strict_dns_cluster.cc @@ -6,37 +6,45 @@ #include "envoy/config/cluster/v3/cluster.pb.h" #include "envoy/config/endpoint/v3/endpoint.pb.h" #include "envoy/config/endpoint/v3/endpoint_components.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.h" + +#include "source/common/common/dns_utils.h" +#include "source/common/network/dns_resolver/dns_factory_util.h" +#include "source/extensions/clusters/common/dns_cluster_backcompat.h" namespace Envoy { namespace Upstream { absl::StatusOr> StrictDnsClusterImpl::create(const envoy::config::cluster::v3::Cluster& cluster, + const envoy::extensions::clusters::dns::v3::DnsCluster& dns_cluster, ClusterFactoryContext& context, Network::DnsResolverSharedPtr dns_resolver) { absl::Status creation_status = absl::OkStatus(); - auto ret = std::unique_ptr( - new StrictDnsClusterImpl(cluster, context, dns_resolver, creation_status)); + auto ret = std::unique_ptr(new StrictDnsClusterImpl( + cluster, dns_cluster, context, std::move(dns_resolver), creation_status)); RETURN_IF_NOT_OK(creation_status); return ret; } -StrictDnsClusterImpl::StrictDnsClusterImpl(const envoy::config::cluster::v3::Cluster& cluster, - ClusterFactoryContext& context, - Network::DnsResolverSharedPtr dns_resolver, - absl::Status& creation_status) +StrictDnsClusterImpl::StrictDnsClusterImpl( + const envoy::config::cluster::v3::Cluster& cluster, + const envoy::extensions::clusters::dns::v3::DnsCluster& dns_cluster, + ClusterFactoryContext& context, Network::DnsResolverSharedPtr dns_resolver, + absl::Status& creation_status) : BaseDynamicClusterImpl(cluster, context, creation_status), load_assignment_(cluster.load_assignment()), local_info_(context.serverFactoryContext().localInfo()), dns_resolver_(dns_resolver), - dns_refresh_rate_ms_( - std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(cluster, dns_refresh_rate, 5000))), - dns_jitter_ms_(PROTOBUF_GET_MS_OR_DEFAULT(cluster, dns_jitter, 0)), - respect_dns_ttl_(cluster.respect_dns_ttl()) { - failure_backoff_strategy_ = - Config::Utility::prepareDnsRefreshStrategy( - cluster, dns_refresh_rate_ms_.count(), - context.serverFactoryContext().api().randomGenerator()); + dns_refresh_rate_ms_(std::chrono::milliseconds( + PROTOBUF_GET_MS_OR_DEFAULT(dns_cluster, dns_refresh_rate, 5000))), + dns_jitter_ms_(PROTOBUF_GET_MS_OR_DEFAULT(dns_cluster, dns_jitter, 0)), + respect_dns_ttl_(dns_cluster.respect_dns_ttl()), + dns_lookup_family_( + Envoy::DnsUtils::getDnsLookupFamilyFromEnum(dns_cluster.dns_lookup_family())) { + failure_backoff_strategy_ = Config::Utility::prepareDnsRefreshStrategy( + dns_cluster, dns_refresh_rate_ms_.count(), + context.serverFactoryContext().api().randomGenerator()); std::list resolve_targets; const auto& locality_lb_endpoints = load_assignment_.endpoints(); @@ -55,7 +63,6 @@ StrictDnsClusterImpl::StrictDnsClusterImpl(const envoy::config::cluster::v3::Clu } } resolve_targets_ = std::move(resolve_targets); - dns_lookup_family_ = getDnsLookupFamilyFromCluster(cluster); overprovisioning_factor_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT( load_assignment_.policy(), overprovisioning_factor, kDefaultOverProvisioningFactor); @@ -229,12 +236,16 @@ void StrictDnsClusterImpl::ResolveTarget::startResolve() { absl::StatusOr> StrictDnsClusterFactory::createClusterImpl(const envoy::config::cluster::v3::Cluster& cluster, - ClusterFactoryContext& context) { + Upstream::ClusterFactoryContext& context) { + absl::StatusOr> cluster_or_error; auto dns_resolver_or_error = selectDnsResolver(cluster, context); RETURN_IF_NOT_OK(dns_resolver_or_error.status()); - auto cluster_or_error = - StrictDnsClusterImpl::create(cluster, context, std::move(dns_resolver_or_error.value())); + envoy::extensions::clusters::dns::v3::DnsCluster proto_config_legacy{}; + createDnsClusterFromLegacyFields(cluster, proto_config_legacy); + cluster_or_error = StrictDnsClusterImpl::create(cluster, proto_config_legacy, context, + std::move(*dns_resolver_or_error)); + RETURN_IF_NOT_OK(cluster_or_error.status()); return std::make_pair(std::shared_ptr(std::move(*cluster_or_error)), nullptr); diff --git a/source/extensions/clusters/strict_dns/strict_dns_cluster.h b/source/extensions/clusters/strict_dns/strict_dns_cluster.h index 72b4070e5a85..0f15b399b794 100644 --- a/source/extensions/clusters/strict_dns/strict_dns_cluster.h +++ b/source/extensions/clusters/strict_dns/strict_dns_cluster.h @@ -2,6 +2,8 @@ #include "envoy/config/cluster/v3/cluster.pb.h" #include "envoy/config/endpoint/v3/endpoint_components.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.validate.h" #include "source/common/upstream/cluster_factory_impl.h" #include "source/common/upstream/upstream_impl.h" @@ -18,11 +20,13 @@ class StrictDnsClusterImpl : public BaseDynamicClusterImpl { // Upstream::Cluster InitializePhase initializePhase() const override { return InitializePhase::Primary; } static absl::StatusOr> - create(const envoy::config::cluster::v3::Cluster& cluster, ClusterFactoryContext& context, - Network::DnsResolverSharedPtr dns_resolver); + create(const envoy::config::cluster::v3::Cluster& cluster, + const envoy::extensions::clusters::dns::v3::DnsCluster& dns_cluster, + ClusterFactoryContext& context, Network::DnsResolverSharedPtr dns_resolver); protected: StrictDnsClusterImpl(const envoy::config::cluster::v3::Cluster& cluster, + const envoy::extensions::clusters::dns::v3::DnsCluster& dns_cluster, ClusterFactoryContext& context, Network::DnsResolverSharedPtr dns_resolver, absl::Status& creation_status); diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index 36ba8f0a7ba6..7bb6e24b1daf 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -19,6 +19,7 @@ EXTENSIONS = { # "envoy.clusters.aggregate": "//source/extensions/clusters/aggregate:cluster", + "envoy.clusters.dns": "//source/extensions/clusters/dns:dns_cluster_lib", "envoy.clusters.dynamic_forward_proxy": "//source/extensions/clusters/dynamic_forward_proxy:cluster", "envoy.clusters.eds": "//source/extensions/clusters/eds:eds_lib", "envoy.clusters.redis": "//source/extensions/clusters/redis:redis_cluster", diff --git a/source/extensions/extensions_metadata.yaml b/source/extensions/extensions_metadata.yaml index ef2f3b5e6a57..6a24b8aa0740 100644 --- a/source/extensions/extensions_metadata.yaml +++ b/source/extensions/extensions_metadata.yaml @@ -94,11 +94,15 @@ envoy.clusters.aggregate: - envoy.clusters security_posture: requires_trusted_downstream_and_upstream status: stable + type_urls: + - envoy.extensions.clusters.aggregate.v3.ClusterConfig envoy.clusters.dynamic_forward_proxy: categories: - envoy.clusters security_posture: robust_to_untrusted_downstream status: stable + type_urls: + - envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig envoy.clusters.static: categories: - envoy.clusters @@ -109,6 +113,13 @@ envoy.clusters.strict_dns: - envoy.clusters security_posture: robust_to_untrusted_downstream_and_upstream status: stable +envoy.clusters.dns: + categories: + - envoy.clusters + security_posture: robust_to_untrusted_downstream_and_upstream + status: stable + type_urls: + - envoy.extensions.clusters.dns.v3.DnsCluster envoy.clusters.original_dst: categories: - envoy.clusters @@ -124,6 +135,8 @@ envoy.clusters.redis: - envoy.clusters security_posture: requires_trusted_downstream_and_upstream status: stable + type_urls: + - envoy.extensions.clusters.redis.v3.RedisClusterConfig envoy.clusters.logical_dns: categories: - envoy.clusters diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index 2902ddce0c07..8f77860fbf06 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -460,6 +460,8 @@ envoy_cc_test( "//source/common/network:address_lib", "//source/common/network:resolver_lib", "//source/common/network:utility_lib", + "//source/extensions/clusters/dns:dns_cluster_lib", + "//source/extensions/clusters/eds:eds_lib", # TODO(mattklein123): Split this into 2 tests for each cluster. "//source/extensions/clusters/static:static_cluster_lib", "//source/extensions/clusters/strict_dns:strict_dns_cluster_lib", diff --git a/test/common/upstream/cluster_factory_impl_test.cc b/test/common/upstream/cluster_factory_impl_test.cc index d120617bc2dc..356adee0b0fe 100644 --- a/test/common/upstream/cluster_factory_impl_test.cc +++ b/test/common/upstream/cluster_factory_impl_test.cc @@ -168,7 +168,7 @@ TEST_F(TestStaticClusterImplTest, CreateWithTypedConfig) { cluster_type: name: envoy.clusters.custom_static typed_config: - "@type": type.googleapis.com/test.integration.clusters.CustomStaticConfig + "@type": type.googleapis.com/test.integration.clusters.CustomStaticConfig1 priority: 10 address: 127.0.0.1 port_value: 80 @@ -194,7 +194,7 @@ TEST_F(TestStaticClusterImplTest, CreateWithTypedConfig) { EXPECT_FALSE(cluster->info()->addedViaApi()); } -TEST_F(TestStaticClusterImplTest, UnsupportedClusterType) { +TEST_F(TestStaticClusterImplTest, UnsupportedClusterName) { const std::string yaml = R"EOF( name: staticcluster connect_timeout: 0.25s @@ -209,9 +209,6 @@ TEST_F(TestStaticClusterImplTest, UnsupportedClusterType) { port_value: 443 cluster_type: name: envoy.clusters.bad_cluster_name - typed_config: - "@type": type.googleapis.com/test.integration.clusters.CustomStaticConfig - priority: 10 )EOF"; // the factory is not registered, expect to fail const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml); @@ -224,6 +221,35 @@ TEST_F(TestStaticClusterImplTest, UnsupportedClusterType) { "'envoy.clusters.bad_cluster_name'"); } +TEST_F(TestStaticClusterImplTest, UnsupportedClusterType) { + std::string yaml = R"EOF( + name: staticcluster + connect_timeout: 0.25s + lb_policy: ROUND_ROBIN + load_assignment: + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: 10.0.0.1 + port_value: 443 + cluster_type: + name: envoy.clusters.logical_dns + typed_config: + "@type": type.googleapis.com/envoy.config.cluster.v3.Cluster + )EOF"; + // the factory is not registered, expect to fail + const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml); + auto create_result = + ClusterFactoryImplBase::create(cluster_config, server_context_, cm_, dns_resolver_fn_, + ssl_context_manager_, std::move(outlier_event_logger_), false); + EXPECT_FALSE(create_result.ok()); + EXPECT_EQ(create_result.status().message(), + "Didn't find a registered cluster factory implementation for type: " + "'envoy.config.cluster.v3.Cluster'"); +} + TEST_F(TestStaticClusterImplTest, HostnameWithoutDNS) { const std::string yaml = R"EOF( name: staticcluster @@ -241,7 +267,7 @@ TEST_F(TestStaticClusterImplTest, HostnameWithoutDNS) { address: 10.0.0.1 port_value: 443 cluster_type: - name: envoy.clusters.test_static + name: envoy.clusters.custom_static )EOF"; const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml); @@ -251,7 +277,7 @@ TEST_F(TestStaticClusterImplTest, HostnameWithoutDNS) { EXPECT_FALSE(create_result.ok()); EXPECT_EQ(create_result.status().message(), "Cannot use hostname for consistent hashing loadbalancing for cluster of type: " - "'envoy.clusters.test_static'"); + "'envoy.clusters.custom_static'"); } } // namespace diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index a977668b8ca7..d0afc6a2334f 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -24,7 +24,9 @@ #include "source/common/network/resolver_impl.h" #include "source/common/network/transport_socket_options_impl.h" #include "source/common/network/utility.h" +#include "source/common/protobuf/utility.h" #include "source/common/singleton/manager_impl.h" +#include "source/extensions/clusters/common/dns_cluster_backcompat.h" #include "source/extensions/clusters/static/static_cluster.h" #include "source/extensions/clusters/strict_dns/strict_dns_cluster.h" #include "source/extensions/load_balancing_policies/least_request/config.h" @@ -80,6 +82,24 @@ class UpstreamImplTestBase { return std::dynamic_pointer_cast(status_or_cluster->first); } + absl::StatusOr> + createStrictDnsCluster(const envoy::config::cluster::v3::Cluster& cluster_config, + ClusterFactoryContext& factory_context, + std::shared_ptr dns_resolver) { + envoy::extensions::clusters::dns::v3::DnsCluster dns_cluster{}; + + ClusterFactoryContextImpl::LazyCreateDnsResolver resolver_fn = [&]() { return dns_resolver; }; + auto status_or_cluster = ClusterFactoryImplBase::create( + cluster_config, factory_context.serverFactoryContext(), + factory_context.serverFactoryContext().clusterManager(), resolver_fn, + factory_context.sslContextManager(), nullptr, factory_context.addedViaApi()); + + if (!status_or_cluster.ok()) { + return status_or_cluster.status(); + } + return (std::dynamic_pointer_cast(status_or_cluster->first)); + } + NiceMock server_context_; Stats::TestUtil::TestStore& stats_ = server_context_.store_; NiceMock random_; @@ -90,6 +110,7 @@ class UpstreamImplTestBase { }; namespace { + std::list hostListToAddresses(const HostVector& hosts) { std::list addresses; for (const HostSharedPtr& host : hosts) { @@ -196,13 +217,11 @@ class StrictDnsParamTest : public testing::TestWithParam, server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); if (numerator <= 100) { - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver); EXPECT_EQ(drop_ratio, cluster->dropOverload().value()); } else { EXPECT_EQ( - StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver) - .status() - .message(), + createStrictDnsCluster(cluster_config, factory_context, dns_resolver).status().message(), fmt::format("load_balancing_policy.drop_overload_limit runtime key config {} is invalid. " "The valid range is 0~100", numerator)); @@ -245,7 +264,7 @@ TEST_P(StrictDnsParamTest, ImmediateResolve) { Envoy::Upstream::ClusterFactoryContextImpl factory_context( server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver); cluster->initialize([&]() -> absl::Status { initialized.ready(); @@ -277,7 +296,7 @@ TEST_P(StrictDnsParamTest, DropOverLoadConfigTestBasicMillion) { Envoy::Upstream::ClusterFactoryContextImpl factory_context( server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver); EXPECT_EQ(0.000035f, cluster->dropOverload().value()); EXPECT_EQ("test", cluster->dropCategory()); } @@ -304,7 +323,7 @@ TEST_P(StrictDnsParamTest, DropOverLoadConfigTestBasicTenThousand) { Envoy::Upstream::ClusterFactoryContextImpl factory_context( server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver); EXPECT_EQ(0.1f, cluster->dropOverload().value()); EXPECT_EQ("foo", cluster->dropCategory()); } @@ -332,10 +351,9 @@ TEST_P(StrictDnsParamTest, DropOverLoadConfigTestBadDenominator) { Envoy::Upstream::ClusterFactoryContextImpl factory_context( server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - EXPECT_EQ(StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver) - .status() - .message(), - "Cluster drop_overloads config denominator setting is invalid : 4. Valid range 0~2."); + EXPECT_EQ( + createStrictDnsCluster(cluster_config, factory_context, dns_resolver).status().message(), + "Cluster drop_overloads config denominator setting is invalid : 4. Valid range 0~2."); } TEST_P(StrictDnsParamTest, DropOverLoadConfigTestBadNumerator) { @@ -362,9 +380,7 @@ TEST_P(StrictDnsParamTest, DropOverLoadConfigTestBadNumerator) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); EXPECT_EQ( - StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver) - .status() - .message(), + createStrictDnsCluster(cluster_config, factory_context, dns_resolver).status().message(), "Cluster drop_overloads config is invalid. drop_ratio=2(Numerator 200 / Denominator 100). " "The valid range is 0~1."); } @@ -394,10 +410,9 @@ TEST_P(StrictDnsParamTest, DropOverLoadConfigTestMultipleCategory) { Envoy::Upstream::ClusterFactoryContextImpl factory_context( server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - EXPECT_EQ(StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver) - .status() - .message(), - "Cluster drop_overloads config has 2 categories. Envoy only support one."); + EXPECT_EQ( + createStrictDnsCluster(cluster_config, factory_context, dns_resolver).status().message(), + "Cluster drop_overloads config has 2 categories. Envoy only support one."); } // Drop overload runtime key configuration test @@ -441,7 +456,7 @@ TEST_F(StrictDnsClusterImplTest, ZeroHostsIsInializedImmediately) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); EXPECT_CALL(initialized, ready()); cluster->initialize([&]() -> absl::Status { initialized.ready(); @@ -477,7 +492,7 @@ TEST_F(StrictDnsClusterImplTest, ZeroHostsHealthChecker) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); std::shared_ptr health_checker(new MockHealthChecker()); EXPECT_CALL(*health_checker, start()); EXPECT_CALL(*health_checker, addHostCheckCompleteCb(_)); @@ -524,7 +539,7 @@ TEST_F(StrictDnsClusterImplTest, DontWaitForDNSOnInit) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); ReadyWatcher initialized; @@ -608,7 +623,7 @@ TEST_F(StrictDnsClusterImplTest, Basic) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); EXPECT_CALL(runtime_.snapshot_, getInteger("circuit_breakers.name.default.max_connections", 43)) .Times(AnyNumber()); @@ -777,7 +792,7 @@ TEST_F(StrictDnsClusterImplTest, HostRemovalActiveHealthSkipped) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); std::shared_ptr health_checker(new MockHealthChecker()); EXPECT_CALL(*health_checker, start()); @@ -837,7 +852,7 @@ TEST_F(StrictDnsClusterImplTest, HostRemovalAfterHcFail) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); std::shared_ptr health_checker(new MockHealthChecker()); EXPECT_CALL(*health_checker, start()); @@ -919,7 +934,7 @@ TEST_F(StrictDnsClusterImplTest, HostUpdateWithDisabledACEndpoint) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); std::shared_ptr health_checker(new MockHealthChecker()); EXPECT_CALL(*health_checker, start()); @@ -1036,7 +1051,7 @@ TEST_F(StrictDnsClusterImplTest, LoadAssignmentBasic) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); EXPECT_CALL(runtime_.snapshot_, getInteger("circuit_breakers.name.default.max_connections", 43)); EXPECT_EQ(43U, cluster->info()->resourceManager(ResourcePriority::Default).connections().max()); @@ -1280,7 +1295,7 @@ TEST_F(StrictDnsClusterImplTest, LoadAssignmentBasicMultiplePriorities) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); ReadyWatcher membership_updated; auto priority_update_cb = cluster->prioritySet().addPriorityUpdateCb( @@ -1399,7 +1414,7 @@ TEST_F(StrictDnsClusterImplTest, CustomResolverFails) { false); EXPECT_THROW_WITH_MESSAGE( - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_), + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_), EnvoyException, "STRICT_DNS clusters must NOT have a custom resolver name set"); } @@ -1431,7 +1446,7 @@ TEST_F(StrictDnsClusterImplTest, FailureRefreshRateBackoffResetsWhenSuccessHappe server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); cluster->initialize([] { return absl::OkStatus(); }); @@ -1453,6 +1468,130 @@ TEST_F(StrictDnsClusterImplTest, FailureRefreshRateBackoffResetsWhenSuccessHappe TestUtility::makeDnsResponse({})); } +TEST_F(StrictDnsClusterImplTest, ClusterTypeConfig) { + ResolverData resolver(*dns_resolver_, server_context_.dispatcher_); + + const std::string yaml = R"EOF( + name: name + connect_timeout: 0.25s + cluster_type: + name: envoy.cluster.strict_dns + dns_refresh_rate: 4s + dns_jitter: 0s + respect_dns_ttl: true + lb_policy: ROUND_ROBIN + load_assignment: + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: localhost1 + port_value: 11001 + )EOF"; + + envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml); + + Envoy::Upstream::ClusterFactoryContextImpl factory_context( + server_context_, server_context_.cluster_manager_, + [dns_resolver = this->dns_resolver_]() { return dns_resolver; }, ssl_context_manager_, + nullptr, false); + + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); + + cluster->initialize([] { return absl::OkStatus(); }); + + EXPECT_CALL(*resolver.timer_, enableTimer(_, _)); + ON_CALL(random_, random()).WillByDefault(Return(8000)); + resolver.dns_callback_( + Network::DnsResolver::ResolutionStatus::Completed, "", + TestUtility::makeDnsResponse({"192.168.1.1", "192.168.1.2"}, std::chrono::seconds(30))); +} + +TEST_F(StrictDnsClusterImplTest, ClusterTypeConfig2) { + ResolverData resolver(*dns_resolver_, server_context_.dispatcher_); + + const std::string yaml = R"EOF( + name: name + connect_timeout: 0.25s + cluster_type: + name: envoy.cluster.dns + typed_config: + "@type": type.googleapis.com/envoy.extensions.clusters.dns.v3.DnsCluster + dns_refresh_rate: 4s + dns_jitter: 0s + respect_dns_ttl: true + all_addresses_in_single_endpoint: false + lb_policy: ROUND_ROBIN + load_assignment: + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: localhost1 + port_value: 11001 + )EOF"; + + envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml); + + Envoy::Upstream::ClusterFactoryContextImpl factory_context( + server_context_, server_context_.cluster_manager_, + [dns_resolver = this->dns_resolver_]() { return dns_resolver; }, ssl_context_manager_, + nullptr, false); + + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); + + cluster->initialize([] { return absl::OkStatus(); }); + + EXPECT_CALL(*resolver.timer_, enableTimer(_, _)); + ON_CALL(random_, random()).WillByDefault(Return(8000)); + resolver.dns_callback_( + Network::DnsResolver::ResolutionStatus::Completed, "", + TestUtility::makeDnsResponse({"192.168.1.1", "192.168.1.2"}, std::chrono::seconds(30))); +} + +TEST_F(StrictDnsClusterImplTest, ClusterTypeConfigTypedDnsResolverConfig) { + NiceMock dns_resolver_factory; + Registry::InjectFactory registered_dns_factory(dns_resolver_factory); + EXPECT_CALL(dns_resolver_factory, createDnsResolver(_, _, _)).WillOnce(Return(dns_resolver_)); + const std::string yaml = R"EOF( + name: name + connect_timeout: 0.25s + cluster_type: + name: envoy.cluster.dns + typed_config: + "@type": type.googleapis.com/envoy.extensions.clusters.dns.v3.DnsCluster + dns_refresh_rate: 4s + dns_jitter: 0s + respect_dns_ttl: true + all_addresses_in_single_endpoint: false + typed_dns_resolver_config: + name: envoy.network.dns_resolver.cares + typed_config: + "@type": type.googleapis.com/envoy.extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig + lb_policy: ROUND_ROBIN + load_assignment: + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: localhost1 + port_value: 11001 + )EOF"; + + envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml); + + // No `dns_resolver_fn` so test segfaults if trying to use default Dns resolver rather than + // creating one from the function> + Envoy::Upstream::ClusterFactoryContextImpl factory_context( + server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, + false); + + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, nullptr); +} + TEST_F(StrictDnsClusterImplTest, TtlAsDnsRefreshRateNoJitter) { ResolverData resolver(*dns_resolver_, server_context_.dispatcher_); @@ -1479,7 +1618,7 @@ TEST_F(StrictDnsClusterImplTest, TtlAsDnsRefreshRateNoJitter) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); ReadyWatcher membership_updated; auto priority_update_cb = cluster->prioritySet().addPriorityUpdateCb( @@ -1530,7 +1669,7 @@ TEST_F(StrictDnsClusterImplTest, NegativeDnsJitter) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); EXPECT_THROW_WITH_MESSAGE( - auto x = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_), + auto x = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_), EnvoyException, "Invalid duration: Expected positive duration: seconds: -1\n"); } TEST_F(StrictDnsClusterImplTest, TtlAsDnsRefreshRateYesJitter) { @@ -1560,7 +1699,7 @@ TEST_F(StrictDnsClusterImplTest, TtlAsDnsRefreshRateYesJitter) { server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); cluster->initialize([] { return absl::OkStatus(); }); @@ -1600,7 +1739,7 @@ TEST_F(StrictDnsClusterImplTest, ExtremeJitter) { Envoy::Upstream::ClusterFactoryContextImpl factory_context( server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_); cluster->initialize([] { return absl::OkStatus(); }); EXPECT_CALL(*resolver.timer_, enableTimer(testing::Ge(std::chrono::milliseconds(1000)), _)); @@ -1663,7 +1802,7 @@ TEST_F(StrictDnsClusterImplTest, Http2UserDefinedSettingsParametersValidation) { false); EXPECT_THROW_WITH_REGEX( - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver_), + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver_), EnvoyException, R"(the \{hpack_table_size\} HTTP/2 SETTINGS parameter\(s\) can not be configured through)" " both"); @@ -3884,7 +4023,7 @@ TEST_F(ClusterImplTest, CloseConnectionsOnHostHealthFailure) { Envoy::Upstream::ClusterFactoryContextImpl factory_context( server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - auto cluster = *StrictDnsClusterImpl::create(cluster_config, factory_context, dns_resolver); + auto cluster = *createStrictDnsCluster(cluster_config, factory_context, dns_resolver); EXPECT_TRUE(cluster->info()->features() & ClusterInfo::Features::CLOSE_CONNECTIONS_ON_HOST_HEALTH_FAILURE); @@ -4083,20 +4222,21 @@ TEST(PrioritySet, MainPrioritySetTest) { EXPECT_EQ(nullptr, priority_set.mutableHostMapForTest().get()); } -class ClusterInfoImplTest : public testing::Test { +class ClusterInfoImplTest : public testing::Test, public UpstreamImplTestBase { public: ClusterInfoImplTest() { ON_CALL(server_context_, api()).WillByDefault(ReturnRef(*api_)); } - std::unique_ptr makeCluster(const std::string& yaml) { + std::shared_ptr makeCluster(const std::string& yaml) { cluster_config_ = parseClusterFromV3Yaml(yaml); Envoy::Upstream::ClusterFactoryContextImpl factory_context( - server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, - false); + server_context_, server_context_.cluster_manager_, [&]() { return dns_resolver_; }, + ssl_context_manager_, nullptr, false); - return THROW_OR_RETURN_VALUE( - StrictDnsClusterImpl::create(cluster_config_, factory_context, dns_resolver_), - std::unique_ptr); + StrictDnsClusterFactory factory{}; + auto status_or_cluster = factory.create(cluster_config_, factory_context); + THROW_IF_NOT_OK_REF(status_or_cluster.status()); + return std::dynamic_pointer_cast(status_or_cluster->first); } class RetryBudgetTestClusterInfo : public ClusterInfoImpl { @@ -4999,7 +5139,7 @@ TEST_F(ClusterInfoImplTest, ExtensionProtocolOptionsForFilterWithOptions) { // This vector is used to gather clusters with extension_protocol_options from the different // types of extension factories (network, http). - std::vector> clusters; + std::vector> clusters; { // Get the cluster with extension_protocol_options for a network filter factory. diff --git a/test/config/utility.cc b/test/config/utility.cc index c603215b36e8..4fecbf579fe6 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -27,6 +27,7 @@ #include "test/test_common/resources.h" #include "test/test_common/utility.h" +#include "absl/strings/match.h" #include "absl/strings/str_replace.h" #include "gtest/gtest.h" @@ -1021,7 +1022,8 @@ void ConfigHelper::setPorts(const std::vector& ports, bool override_po eds_hosts = true; } else if (cluster->type() == envoy::config::cluster::v3::Cluster::ORIGINAL_DST) { original_dst_cluster = true; - } else if (cluster->has_cluster_type()) { + } else if (cluster->has_cluster_type() && + !absl::StartsWith(cluster->cluster_type().name(), "envoy.cluster.")) { custom_cluster = true; } else { // Assign ports to statically defined load_assignment hosts. diff --git a/test/extensions/clusters/common/BUILD b/test/extensions/clusters/common/BUILD index e3b6361029af..82b2239190f6 100644 --- a/test/extensions/clusters/common/BUILD +++ b/test/extensions/clusters/common/BUILD @@ -8,6 +8,19 @@ licenses(["notice"]) # Apache 2 envoy_package() +envoy_cc_test( + name = "dns_cluster_backcompat_test", + srcs = ["dns_cluster_backcompat_test.cc"], + rbe_pool = "2core", + deps = [ + "//source/extensions/clusters/common:dns_cluster_backcompat_lib", + "//test/test_common:utility_lib", + "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/clusters/common/dns/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/clusters/dns/v3:pkg_cc_proto", + ], +) + envoy_cc_test( name = "logical_host_test", srcs = ["logical_host_test.cc"], @@ -26,10 +39,12 @@ envoy_cc_test( deps = [ "//source/common/config:utility_lib", "//source/extensions/clusters/common:logical_host_lib", + "//source/extensions/clusters/dns:dns_cluster_lib", "//source/extensions/clusters/logical_dns:logical_dns_cluster_lib", "//test/integration:http_integration_lib", "//test/mocks/network:network_mocks", "//test/test_common:registry_lib", "//test/test_common:threadsafe_singleton_injector_lib", + "@envoy_api//envoy/extensions/clusters/dns/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/clusters/common/dns_cluster_backcompat_test.cc b/test/extensions/clusters/common/dns_cluster_backcompat_test.cc new file mode 100644 index 000000000000..e86d2bf45443 --- /dev/null +++ b/test/extensions/clusters/common/dns_cluster_backcompat_test.cc @@ -0,0 +1,126 @@ +#include "envoy/config/cluster/v3/cluster.pb.h" +#include "envoy/extensions/clusters/common/dns/v3/dns.pb.h" +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.h" + +#include "source/extensions/clusters/common/dns_cluster_backcompat.h" + +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Upstream { + +class DnsClusterBackcompatUtilTest : public testing::Test {}; + +TEST_F(DnsClusterBackcompatUtilTest, Empty) { + envoy::config::cluster::v3::Cluster cluster{}; + envoy::extensions::clusters::dns::v3::DnsCluster dns_cluster{}; + createDnsClusterFromLegacyFields(cluster, dns_cluster); + EXPECT_FALSE(dns_cluster.has_dns_jitter()); + EXPECT_FALSE(dns_cluster.has_dns_refresh_rate()); + EXPECT_FALSE(dns_cluster.has_dns_failure_refresh_rate()); + EXPECT_FALSE(dns_cluster.respect_dns_ttl()); + EXPECT_TRUE(dns_cluster.dns_lookup_family() == + envoy::extensions::clusters::common::dns::v3::AUTO); + EXPECT_FALSE(dns_cluster.has_typed_dns_resolver_config()); +}; + +TEST_F(DnsClusterBackcompatUtilTest, EmptyButSetFailureRefreshRate) { + envoy::config::cluster::v3::Cluster cluster{}; + + cluster.mutable_dns_failure_refresh_rate(); + envoy::extensions::clusters::dns::v3::DnsCluster dns_cluster{}; + createDnsClusterFromLegacyFields(cluster, dns_cluster); + EXPECT_FALSE(dns_cluster.has_dns_jitter()); + EXPECT_FALSE(dns_cluster.has_dns_refresh_rate()); + EXPECT_TRUE(dns_cluster.has_dns_failure_refresh_rate()); + EXPECT_FALSE(dns_cluster.dns_failure_refresh_rate().has_base_interval()); + EXPECT_FALSE(dns_cluster.dns_failure_refresh_rate().has_max_interval()); + EXPECT_FALSE(dns_cluster.respect_dns_ttl()); + EXPECT_TRUE(dns_cluster.dns_lookup_family() == + envoy::extensions::clusters::common::dns::v3::AUTO); + EXPECT_FALSE(dns_cluster.has_typed_dns_resolver_config()); +}; + +TEST_F(DnsClusterBackcompatUtilTest, FullClusterConfig) { + envoy::config::cluster::v3::Cluster cluster = + TestUtility::parseYaml(R"EOF( + type: STRICT_DNS + dns_jitter: + seconds: 1 + nanos: 2 + dns_failure_refresh_rate: + base_interval: + seconds: 3 + nanos: 4 + max_interval: + seconds: 5 + nanos: 6 + dns_refresh_rate: + seconds: 7 + nanos: 8 + respect_dns_ttl: true + dns_lookup_family: V6_ONLY + )EOF"); + + envoy::extensions::clusters::dns::v3::DnsCluster dns_cluster{}; + createDnsClusterFromLegacyFields(cluster, dns_cluster); + EXPECT_EQ(dns_cluster.dns_jitter().seconds(), 1); + EXPECT_EQ(dns_cluster.dns_jitter().nanos(), 2); + EXPECT_EQ(dns_cluster.dns_failure_refresh_rate().base_interval().seconds(), 3); + EXPECT_EQ(dns_cluster.dns_failure_refresh_rate().base_interval().nanos(), 4); + EXPECT_EQ(dns_cluster.dns_failure_refresh_rate().max_interval().seconds(), 5); + EXPECT_EQ(dns_cluster.dns_failure_refresh_rate().max_interval().nanos(), 6); + EXPECT_EQ(dns_cluster.dns_refresh_rate().seconds(), 7); + EXPECT_EQ(dns_cluster.dns_refresh_rate().nanos(), 8); + EXPECT_TRUE(dns_cluster.respect_dns_ttl()); + EXPECT_TRUE(dns_cluster.dns_lookup_family() == + envoy::extensions::clusters::common::dns::v3::V6_ONLY); + EXPECT_FALSE(dns_cluster.has_typed_dns_resolver_config()); +}; + +TEST_F(DnsClusterBackcompatUtilTest, LookupFamilyTranslation) { + envoy::config::cluster::v3::Cluster cluster = + TestUtility::parseYaml(R"EOF( + type: STRICT_DNS + dns_lookup_family: V6_ONLY + )EOF"); + envoy::extensions::clusters::dns::v3::DnsCluster dns_cluster{}; + createDnsClusterFromLegacyFields(cluster, dns_cluster); + EXPECT_TRUE(dns_cluster.dns_lookup_family() == + envoy::extensions::clusters::common::dns::v3::V6_ONLY); + + cluster = TestUtility::parseYaml(R"EOF( + type: STRICT_DNS + dns_lookup_family: V4_ONLY + )EOF"); + createDnsClusterFromLegacyFields(cluster, dns_cluster); + EXPECT_TRUE(dns_cluster.dns_lookup_family() == + envoy::extensions::clusters::common::dns::v3::V4_ONLY); + + cluster = TestUtility::parseYaml(R"EOF( + type: STRICT_DNS + dns_lookup_family: AUTO + )EOF"); + createDnsClusterFromLegacyFields(cluster, dns_cluster); + EXPECT_TRUE(dns_cluster.dns_lookup_family() == + envoy::extensions::clusters::common::dns::v3::AUTO); + + cluster = TestUtility::parseYaml(R"EOF( + type: STRICT_DNS + dns_lookup_family: V4_PREFERRED + )EOF"); + createDnsClusterFromLegacyFields(cluster, dns_cluster); + EXPECT_TRUE(dns_cluster.dns_lookup_family() == + envoy::extensions::clusters::common::dns::v3::V4_PREFERRED); + + cluster = TestUtility::parseYaml(R"EOF( + type: STRICT_DNS + dns_lookup_family: ALL + )EOF"); + createDnsClusterFromLegacyFields(cluster, dns_cluster); + EXPECT_TRUE(dns_cluster.dns_lookup_family() == envoy::extensions::clusters::common::dns::v3::ALL); +} +} // namespace Upstream +} // namespace Envoy diff --git a/test/extensions/clusters/common/logical_host_integration_test.cc b/test/extensions/clusters/common/logical_host_integration_test.cc index 66beadcdd2cd..003c8a17fb3b 100644 --- a/test/extensions/clusters/common/logical_host_integration_test.cc +++ b/test/extensions/clusters/common/logical_host_integration_test.cc @@ -1,3 +1,5 @@ +#include "envoy/extensions/clusters/dns/v3/dns_cluster.pb.h" + #include "source/common/config/utility.h" #include "test/integration/http_integration.h" @@ -84,10 +86,13 @@ TEST_P(LogicalHostIntegrationTest, LogicalDNSRaceCrashTest) { config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void { RELEASE_ASSERT(bootstrap.mutable_static_resources()->clusters_size() == 1, ""); auto& cluster = *bootstrap.mutable_static_resources()->mutable_clusters(0); - cluster.set_type(envoy::config::cluster::v3::Cluster::LOGICAL_DNS); cluster.set_dns_lookup_family(envoy::config::cluster::v3::Cluster::ALL); + cluster.mutable_cluster_type()->set_name("envoy.cluster.logical_dns"); + envoy::extensions::clusters::dns::v3::DnsCluster dns_cluster{}; // Make the refresh rate fast to hit the R/W race. - cluster.mutable_dns_refresh_rate()->set_nanos(1000001); + dns_cluster.mutable_dns_refresh_rate()->set_nanos(1000001); + dns_cluster.set_all_addresses_in_single_endpoint(true); + cluster.mutable_cluster_type()->mutable_typed_config()->PackFrom(dns_cluster); }); config_helper_.addConfigModifier( [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& diff --git a/test/extensions/clusters/logical_dns/BUILD b/test/extensions/clusters/logical_dns/BUILD index ed36b316d45e..5c1608f9b9e8 100644 --- a/test/extensions/clusters/logical_dns/BUILD +++ b/test/extensions/clusters/logical_dns/BUILD @@ -17,6 +17,7 @@ envoy_cc_test( "//source/common/event:dispatcher_lib", "//source/common/network:utility_lib", "//source/common/upstream:upstream_lib", + "//source/extensions/clusters/dns:dns_cluster_lib", "//source/extensions/clusters/logical_dns:logical_dns_cluster_lib", "//source/extensions/load_balancing_policies/round_robin:config", "//source/extensions/transport_sockets/raw_buffer:config", diff --git a/test/extensions/clusters/logical_dns/logical_dns_cluster_test.cc b/test/extensions/clusters/logical_dns/logical_dns_cluster_test.cc index 6679425dd1c6..3fe6aa112d41 100644 --- a/test/extensions/clusters/logical_dns/logical_dns_cluster_test.cc +++ b/test/extensions/clusters/logical_dns/logical_dns_cluster_test.cc @@ -13,6 +13,8 @@ #include "source/common/network/utility.h" #include "source/common/singleton/manager_impl.h" +#include "source/extensions/clusters/common/dns_cluster_backcompat.h" +#include "source/extensions/clusters/dns/dns_cluster.h" #include "source/extensions/clusters/logical_dns/logical_dns_cluster.h" #include "source/server/transport_socket_config_impl.h" @@ -58,10 +60,27 @@ class LogicalDnsClusterTest : public Event::TestUsingSimulatedTime, public testi Envoy::Upstream::ClusterFactoryContextImpl factory_context( server_context_, server_context_.cluster_manager_, nullptr, ssl_context_manager_, nullptr, false); - absl::Status creation_status = absl::OkStatus(); - cluster_ = std::shared_ptr( - new LogicalDnsCluster(cluster_config, factory_context, dns_resolver_, creation_status)); - THROW_IF_NOT_OK(creation_status); + absl::StatusOr> status_or_cluster; + + envoy::extensions::clusters::dns::v3::DnsCluster dns_cluster{}; + if (cluster_config.has_cluster_type()) { + ProtobufTypes::MessagePtr dns_cluster_msg = + std::make_unique(); + ASSERT_TRUE(Config::Utility::translateOpaqueConfig( + cluster_config.cluster_type().typed_config(), + factory_context.messageValidationVisitor(), *dns_cluster_msg) + .ok()); + dns_cluster = + MessageUtil::downcastAndValidate( + *dns_cluster_msg, factory_context.messageValidationVisitor()); + } else { + createDnsClusterFromLegacyFields(cluster_config, dns_cluster); + } + + status_or_cluster = + LogicalDnsCluster::create(cluster_config, dns_cluster, factory_context, dns_resolver_); + THROW_IF_NOT_OK_REF(status_or_cluster.status()); + cluster_ = std::move(*status_or_cluster); priority_update_cb_ = cluster_->prioritySet().addPriorityUpdateCb( [&](uint32_t, const HostVector&, const HostVector&) { membership_updated_.ready(); @@ -75,15 +94,31 @@ class LogicalDnsClusterTest : public Event::TestUsingSimulatedTime, public testi absl::Status factorySetupFromV3Yaml(const std::string& yaml) { ON_CALL(server_context_, api()).WillByDefault(ReturnRef(*api_)); + resolve_timer_ = new Event::MockTimer(&server_context_.dispatcher_); NiceMock cm; envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml); ClusterFactoryContextImpl::LazyCreateDnsResolver resolver_fn = [&]() { return dns_resolver_; }; - Envoy::Upstream::ClusterFactoryContextImpl factory_context( - server_context_, server_context_.cluster_manager_, resolver_fn, ssl_context_manager_, - nullptr, false); - - LogicalDnsClusterFactory factory; - return factory.createClusterImpl(cluster_config, factory_context).status(); + auto status_or_cluster = ClusterFactoryImplBase::create( + cluster_config, server_context_, server_context_.cluster_manager_, resolver_fn, + ssl_context_manager_, nullptr, false); + if (status_or_cluster.ok()) { + cluster_ = std::dynamic_pointer_cast(status_or_cluster->first); + priority_update_cb_ = cluster_->prioritySet().addPriorityUpdateCb( + [&](uint32_t, const HostVector&, const HostVector&) { + membership_updated_.ready(); + return absl::OkStatus(); + }); + cluster_->initialize([&]() { + initialized_.ready(); + return absl::OkStatus(); + }); + } else { + // the Event::MockTimer constructor creates EXPECT_CALL for the dispatcher. + // If we want cluster creation to fail, there won't be a cluster to create the timer, + // so we need to clear the expectation manually. + server_context_.dispatcher_.createTimer([]() -> void {}); + } + return status_or_cluster.status(); } void expectResolve(Network::DnsLookupFamily dns_lookup_family, @@ -437,13 +472,17 @@ TEST_F(LogicalDnsClusterTest, BadConfig) { factorySetupFromV3Yaml(multiple_hosts_yaml).message(), "LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint"); - const std::string multiple_lb_endpoints_yaml = R"EOF( + const std::string multiple_hosts_cluster_type_yaml = R"EOF( name: name - type: LOGICAL_DNS - dns_refresh_rate: 4s + cluster_type: + name: envoy.cluster.strict_dns # (this is right, name shouldnt matter) + typed_config: + "@type": type.googleapis.com/envoy.extensions.clusters.dns.v3.DnsCluster + dns_refresh_rate: 4s + all_addresses_in_single_endpoint: true + dns_lookup_family: V4_ONLY connect_timeout: 0.25s lb_policy: ROUND_ROBIN - dns_lookup_family: V4_ONLY load_assignment: cluster_name: name endpoints: @@ -464,66 +503,268 @@ TEST_F(LogicalDnsClusterTest, BadConfig) { port_value: 8000 )EOF"; + EXPECT_EQ( + factorySetupFromV3Yaml(multiple_hosts_cluster_type_yaml).message(), + "LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint"); + + const std::string multiple_lb_endpoints_yaml = R"EOF( + name: name + type: LOGICAL_DNS + dns_refresh_rate: 4s + connect_timeout: 0.25s + lb_policy: ROUND_ROBIN + dns_lookup_family: V4_ONLY + load_assignment: + cluster_name: name + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: foo.bar.com + port_value: 443 + health_check_config: + port_value: 8000 + - endpoint: + address: + socket_address: + address: hello.world.com + port_value: 443 + health_check_config: + port_value: 8000 + )EOF"; + EXPECT_EQ( factorySetupFromV3Yaml(multiple_lb_endpoints_yaml).message(), "LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint"); + const std::string multiple_lb_endpoints_cluster_type_yaml = R"EOF( + name: name + cluster_type: + name: envoy.cluster.logical_dns + typed_config: + "@type": type.googleapis.com/envoy.extensions.clusters.dns.v3.DnsCluster + dns_refresh_rate: 4s + all_addresses_in_single_endpoint: true + connect_timeout: 0.25s + lb_policy: ROUND_ROBIN + dns_lookup_family: V4_ONLY + load_assignment: + cluster_name: name + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: foo.bar.com + port_value: 443 + health_check_config: + port_value: 8000 + - endpoint: + address: + socket_address: + address: hello.world.com + port_value: 443 + health_check_config: + port_value: 8000 + )EOF"; + + EXPECT_EQ( + factorySetupFromV3Yaml(multiple_lb_endpoints_cluster_type_yaml).message(), + "LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint"); + const std::string multiple_endpoints_yaml = R"EOF( + name: name + type: LOGICAL_DNS + dns_refresh_rate: 4s + connect_timeout: 0.25s + lb_policy: ROUND_ROBIN + dns_lookup_family: V4_ONLY + load_assignment: + cluster_name: name + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: foo.bar.com + port_value: 443 + health_check_config: + port_value: 8000 + + - lb_endpoints: + - endpoint: + address: + socket_address: + address: hello.world.com + port_value: 443 + health_check_config: + port_value: 8000 + )EOF"; + + EXPECT_EQ( + factorySetupFromV3Yaml(multiple_endpoints_yaml).message(), + "LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint"); + + const std::string multiple_endpoints_cluster_type_yaml = R"EOF( + name: name + cluster_type: + name: abc + typed_config: + "@type": type.googleapis.com/envoy.extensions.clusters.dns.v3.DnsCluster + dns_lookup_family: V4_ONLY + dns_refresh_rate: 4s + all_addresses_in_single_endpoint: true + connect_timeout: 0.25s + lb_policy: ROUND_ROBIN + load_assignment: + cluster_name: name + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: foo.bar.com + port_value: 443 + health_check_config: + port_value: 8000 + - lb_endpoints: + - endpoint: + address: + socket_address: + address: hello.world.com + port_value: 443 + health_check_config: + port_value: 8000 + )EOF"; + + EXPECT_EQ( + factorySetupFromV3Yaml(multiple_endpoints_cluster_type_yaml).message(), + "LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint"); + + const std::string custom_resolver_yaml = R"EOF( + name: name + type: LOGICAL_DNS + dns_refresh_rate: 4s + connect_timeout: 0.25s + lb_policy: ROUND_ROBIN + dns_lookup_family: V4_ONLY + load_assignment: + cluster_name: name + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: hello.world.com + port_value: 443 + resolver_name: customresolver + health_check_config: + port_value: 8000 + )EOF"; + + EXPECT_EQ(factorySetupFromV3Yaml(custom_resolver_yaml).message(), + "LOGICAL_DNS clusters must NOT have a custom resolver name set"); + + const std::string custom_resolver_cluster_type_yaml = R"EOF( + name: name + cluster_type: + name: abc + typed_config: + "@type": type.googleapis.com/envoy.extensions.clusters.dns.v3.DnsCluster + dns_lookup_family: V4_ONLY + dns_refresh_rate: 4s + all_addresses_in_single_endpoint: true + connect_timeout: 0.25s + lb_policy: ROUND_ROBIN + load_assignment: + cluster_name: name + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: hello.world.com + port_value: 443 + resolver_name: customresolver + health_check_config: + port_value: 8000 + )EOF"; + + EXPECT_EQ(factorySetupFromV3Yaml(custom_resolver_cluster_type_yaml).message(), + "LOGICAL_DNS clusters must NOT have a custom resolver name set"); +} + +// Test using both types of names in the cluster type. +TEST_F(LogicalDnsClusterTest, UseDnsExtension) { + const std::string config = R"EOF( name: name - type: LOGICAL_DNS - dns_refresh_rate: 4s + cluster_type: + name: arglegarble + typed_config: + "@type": type.googleapis.com/envoy.extensions.clusters.dns.v3.DnsCluster + dns_refresh_rate: 4s + dns_lookup_family: V4_ONLY + all_addresses_in_single_endpoint: true connect_timeout: 0.25s lb_policy: ROUND_ROBIN - dns_lookup_family: V4_ONLY + # Since the following expectResolve() requires Network::DnsLookupFamily::V4Only we need to set + # dns_lookup_family to V4_ONLY explicitly for v2 .yaml config. + wait_for_warm_on_init: false load_assignment: - cluster_name: name - endpoints: - - lb_endpoints: - - endpoint: - address: - socket_address: - address: foo.bar.com - port_value: 443 - health_check_config: - port_value: 8000 - - - lb_endpoints: - - endpoint: - address: - socket_address: - address: hello.world.com - port_value: 443 - health_check_config: - port_value: 8000 + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: foo.bar.com + port_value: 443 )EOF"; - EXPECT_EQ( - factorySetupFromV3Yaml(multiple_endpoints_yaml).message(), - "LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint"); + EXPECT_CALL(initialized_, ready()); + expectResolve(Network::DnsLookupFamily::V4Only, "foo.bar.com"); + ASSERT_TRUE(factorySetupFromV3Yaml(config).ok()); - const std::string custom_resolver_yaml = R"EOF( + EXPECT_CALL(membership_updated_, ready()); + EXPECT_CALL(*resolve_timer_, enableTimer(std::chrono::milliseconds(4000), _)); + + dns_callback_( + Network::DnsResolver::ResolutionStatus::Completed, "", + TestUtility::makeDnsResponse({"127.0.0.1", "127.0.0.2"}, std::chrono::seconds(3000))); +} + +TEST_F(LogicalDnsClusterTest, TypedConfigBackcompat) { + const std::string config = R"EOF( name: name - type: LOGICAL_DNS + cluster_type: + name: envoy.cluster.logical_dns dns_refresh_rate: 4s + dns_lookup_family: V4_ONLY connect_timeout: 0.25s lb_policy: ROUND_ROBIN - dns_lookup_family: V4_ONLY + # Since the following expectResolve() requires Network::DnsLookupFamily::V4Only we need to set + # dns_lookup_family to V4_ONLY explicitly for v2 .yaml config. + wait_for_warm_on_init: false load_assignment: - cluster_name: name - endpoints: - - lb_endpoints: - - endpoint: - address: - socket_address: - address: hello.world.com - port_value: 443 - resolver_name: customresolver - health_check_config: - port_value: 8000 + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: foo.bar.com + port_value: 443 )EOF"; - EXPECT_EQ(factorySetupFromV3Yaml(custom_resolver_yaml).message(), - "LOGICAL_DNS clusters must NOT have a custom resolver name set"); + EXPECT_CALL(initialized_, ready()); + expectResolve(Network::DnsLookupFamily::V4Only, "foo.bar.com"); + ASSERT_TRUE(factorySetupFromV3Yaml(config).ok()); + + EXPECT_CALL(membership_updated_, ready()); + EXPECT_CALL(*resolve_timer_, enableTimer(std::chrono::milliseconds(4000), _)); + + dns_callback_( + Network::DnsResolver::ResolutionStatus::Completed, "", + TestUtility::makeDnsResponse({"127.0.0.1", "127.0.0.2"}, std::chrono::seconds(3000))); } TEST_F(LogicalDnsClusterTest, Basic) { diff --git a/test/integration/clusters/cluster_factory_config.proto b/test/integration/clusters/cluster_factory_config.proto index 160e4ae5eb7d..a90ce520db99 100644 --- a/test/integration/clusters/cluster_factory_config.proto +++ b/test/integration/clusters/cluster_factory_config.proto @@ -2,7 +2,13 @@ syntax = "proto3"; package test.integration.clusters; -message CustomStaticConfig { +message CustomStaticConfig1 { + uint32 priority = 1; + string address = 2; + uint32 port_value = 3; +} + +message CustomStaticConfig2 { uint32 priority = 1; string address = 2; uint32 port_value = 3; diff --git a/test/integration/clusters/custom_static_cluster.h b/test/integration/clusters/custom_static_cluster.h index b540fba2b6e5..35655b7bf1eb 100644 --- a/test/integration/clusters/custom_static_cluster.h +++ b/test/integration/clusters/custom_static_cluster.h @@ -22,6 +22,8 @@ namespace Envoy { +template class CustomStaticClusterFactoryBase; + class CustomStaticCluster : public Upstream::ClusterImplBase { public: CustomStaticCluster(const envoy::config::cluster::v3::Cluster& cluster, @@ -47,39 +49,43 @@ class CustomStaticCluster : public Upstream::ClusterImplBase { const uint32_t port_; const Upstream::HostSharedPtr host_; - friend class CustomStaticClusterFactoryBase; + friend class CustomStaticClusterFactoryBase; + friend class CustomStaticClusterFactoryBase; }; -class CustomStaticClusterFactoryBase : public Upstream::ConfigurableClusterFactoryBase< - test::integration::clusters::CustomStaticConfig> { +template +class CustomStaticClusterFactoryBase + : public Upstream::ConfigurableClusterFactoryBase { protected: CustomStaticClusterFactoryBase(const std::string& name, bool create_lb) - : ConfigurableClusterFactoryBase(name), create_lb_(create_lb) {} + : Upstream::ConfigurableClusterFactoryBase(name), create_lb_(create_lb) {} private: absl::StatusOr< std::pair> createClusterWithConfig(const envoy::config::cluster::v3::Cluster& cluster, - const test::integration::clusters::CustomStaticConfig& proto_config, + const ConfigProto& proto_config, Upstream::ClusterFactoryContext& context) override { absl::Status creation_status = absl::OkStatus(); auto new_cluster = std::make_shared( cluster, context, proto_config.priority(), proto_config.address(), proto_config.port_value(), creation_status); - THROW_IF_NOT_OK(creation_status); + THROW_IF_NOT_OK_REF(creation_status); return std::make_pair(new_cluster, create_lb_ ? new_cluster->threadAwareLb() : nullptr); } const bool create_lb_; }; -class CustomStaticClusterFactoryNoLb : public CustomStaticClusterFactoryBase { +class CustomStaticClusterFactoryNoLb + : public CustomStaticClusterFactoryBase { public: CustomStaticClusterFactoryNoLb() - : CustomStaticClusterFactoryBase("envoy.clusters.custom_static", false) {} + : CustomStaticClusterFactoryBase("envoy.clusters.custom_static", false){}; }; -class CustomStaticClusterFactoryWithLb : public CustomStaticClusterFactoryBase { +class CustomStaticClusterFactoryWithLb + : public CustomStaticClusterFactoryBase { public: CustomStaticClusterFactoryWithLb() : CustomStaticClusterFactoryBase("envoy.clusters.custom_static_with_lb", true) {} diff --git a/test/integration/custom_cluster_integration_test.cc b/test/integration/custom_cluster_integration_test.cc index 0ba7910e1a2c..1ba5c02c5b07 100644 --- a/test/integration/custom_cluster_integration_test.cc +++ b/test/integration/custom_cluster_integration_test.cc @@ -33,11 +33,19 @@ class CustomClusterIntegrationTest : public testing::TestWithParamlocalAddress()->ip()->port()); - cluster_type.mutable_typed_config()->PackFrom(config); + if (!cluster_provided_lb_) { + test::integration::clusters::CustomStaticConfig1 config; + config.set_priority(10); + config.set_address(Network::Test::getLoopbackAddressString(ipVersion())); + config.set_port_value(fake_upstreams_[UpstreamIndex]->localAddress()->ip()->port()); + cluster_type.mutable_typed_config()->PackFrom(config); + } else { + test::integration::clusters::CustomStaticConfig2 config; + config.set_priority(10); + config.set_address(Network::Test::getLoopbackAddressString(ipVersion())); + config.set_port_value(fake_upstreams_[UpstreamIndex]->localAddress()->ip()->port()); + cluster_type.mutable_typed_config()->PackFrom(config); + } cluster_0->mutable_cluster_type()->CopyFrom(cluster_type); }); diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml index db31470d3ea2..375133f7e048 100644 --- a/tools/code_format/config.yaml +++ b/tools/code_format/config.yaml @@ -463,6 +463,7 @@ visibility_excludes: - source/extensions/clusters/static/ - source/extensions/clusters/original_dst/ - source/extensions/clusters/logical_dns/ +- source/extensions/clusters/dns/ - source/extensions/early_data/BUILD - source/extensions/filters/http/buffer/BUILD - source/extensions/filters/network/common/BUILD