-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor dns cluster api #36353
Merged
Merged
Refactor dns cluster api #36353
Changes from 42 commits
Commits
Show all changes
55 commits
Select commit
Hold shift + click to select a range
9048a69
Move cluster dns config and validate in strict dns clusters
Stevenjin8 3917c63
Roll back incompatible API changes
Stevenjin8 dab5712
Merge branch 'main' into refactor/dns-api
Stevenjin8 fd255d0
Use cluster_type as extension point
Stevenjin8 c944d7e
Revert unecessary changes
Stevenjin8 8ed51ae
Format
Stevenjin8 8915b81
Fix logical dns tests
Stevenjin8 2464e2c
format
Stevenjin8 821bb34
Remove extra changes
Stevenjin8 1d54cff
extra change
Stevenjin8 58dbf1f
Doc fixes in dns cluster proto
Stevenjin8 9784406
more docs
Stevenjin8 7309f8d
More logical dns tests
Stevenjin8 69f9cec
CI please work
Stevenjin8 5420879
fix tests?
Stevenjin8 a9a5a5c
Fix integration test (not really)
Stevenjin8 6da56ce
Merge branch 'main' into refactor/dns-api
Stevenjin8 16f9cc1
Use Absl StartsWith
Stevenjin8 d4a0058
Remove FIXME
Stevenjin8 9586603
Some typos
Stevenjin8 8a5b9f6
Merge branch 'main' into refactor/dns-api
Stevenjin8 676a8ba
Merge branch 'main' into refactor/dns-api
Stevenjin8 bdc8883
Merge branch 'main' into refactor/dns-api
Stevenjin8 64a6fbd
Add generic factory
Stevenjin8 52647e3
wip
Stevenjin8 9fe2bb2
Tests
Stevenjin8 3dc3fb1
Merge branch 'main' into refactor/dns-api
Stevenjin8 1c319dc
lint
Stevenjin8 2e7d6e5
Change visibility
Stevenjin8 c22edc1
lint
Stevenjin8 31b9024
extension registration
Stevenjin8 c348f70
Lints and delete extra diffs
Stevenjin8 cea08c9
more lint and tests
Stevenjin8 cb2d8fb
proto docs
Stevenjin8 199bbb1
fix tests
Stevenjin8 c0607d3
Mikhail review
Stevenjin8 05fd078
rst error
Stevenjin8 684ca7a
rst error and add more coverage
Stevenjin8 6c36802
docs
Stevenjin8 0463313
docs
Stevenjin8 39545a4
Update extensions status
Stevenjin8 727cf77
release notes
Stevenjin8 5232706
Merge branch 'main' into refactor/dns-api
Stevenjin8 df92eef
Register cluster extensions by type
Stevenjin8 82ae4ce
Better coverage
Stevenjin8 e8bd59a
Typo
Stevenjin8 752a805
Merge branch 'main' into refactor/dns-api
Stevenjin8 bd0ae97
Better docs
Stevenjin8 1ffdf41
Merge branch 'main' into refactor/dns-api
Stevenjin8 aeb3302
second merge
Stevenjin8 0b816c6
third merge
Stevenjin8 2a7832f
update logical dns api
Stevenjin8 9b7927c
forgot one!
Stevenjin8 3078d7c
last one please
Stevenjin8 e31a09b
API docs clarification
Stevenjin8 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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"], | ||
| ) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 { | ||
Stevenjin8 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| UNSPECIFIED = 0; | ||
| AUTO = 1; | ||
| V4_ONLY = 2; | ||
| V6_ONLY = 3; | ||
| V4_PREFERRED = 4; | ||
| ALL = 5; | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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", | ||
| ], | ||
| ) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| 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 strict and logical DNS clusters. | ||
| // [#extension: envoy.clusters.dns] | ||
|
|
||
| // [#next-free-field: 10] | ||
| message DnsCluster { | ||
Stevenjin8 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| enum DnsDiscoveryType { | ||
| LOGICAL = 0; | ||
| STRICT = 1; | ||
| } | ||
|
|
||
| message RefreshRate { | ||
| // Specifies the base interval between refreshes. This parameter is required and must be greater | ||
| // than zero and less than | ||
| // :ref:`max_interval <envoy_v3_api_field_extensions.clusters.dns.v3.DnsCluster.RefreshRate.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 <envoy_v3_api_field_extensions.clusters.dns.v3.DnsCluster.RefreshRate.base_interval>` if set. The default | ||
| // is 10 times the :ref:`base_interval <envoy_v3_api_field_extensions.clusters.dns.v3.DnsCluster.RefreshRate.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}}]; | ||
Stevenjin8 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // 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<envoy_v3_api_msg_extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig>` | ||
| // can be packed into this ``typed_dns_resolver_config``. This configuration replaces the | ||
| // :ref:`Cluster.dns_resolver_config<envoy_v3_api_field_config.cluster.v3.Cluster.typed_dns_resolver_config>` | ||
| // configuration which replaces :ref:`Cluster.dns_resolver_config<envoy_v3_api_field_config.cluster.v3.Cluster.dns_resolution_config>`. | ||
| // During the transition period when ``DnsCluster.typed_dns_resolver_config``, ``Cluster.dns_resolver_config``, and | ||
| // ``Cluster.typed_dns_resolver_config`` exists, Envoy will prefer, ``DnsCluster.typed_dns_resolver_config`` over | ||
| // ``Cluster.typed_dns_resolver_config`` over ``cluster.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<envoy_v3_api_enum_value_extensions.clusters.common.dns.v3.DnsLookupFamily.AUTO>`. | ||
| common.dns.v3.DnsLookupFamily dns_lookup_family = 8; | ||
|
|
||
| // Required. Whether to do logical or strict dns resolution. | ||
| // This field is only considered when the ``name`` field of the ``TypedConfig`` is ``envoy.cluster.dns``. | ||
| DnsDiscoveryType dns_discovery_type = 9; | ||
Stevenjin8 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,4 +5,4 @@ Cluster | |
| :glob: | ||
| :maxdepth: 2 | ||
|
|
||
| ../../extensions/clusters/*/v3/* | ||
| ../../extensions/clusters/**/v3/* | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
59 changes: 59 additions & 0 deletions
59
source/extensions/clusters/common/dns_cluster_backcompat.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these fields still need to be used for other cluster types, like redis? If so, we probably can't mark them as deprecated yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both redis and dynamic forward proxies define their own refresh rates
envoy/api/envoy/extensions/clusters/redis/v3/redis_cluster.proto
Lines 58 to 85 in 983545f
envoy/api/envoy/extensions/common/dynamic_forward_proxy/v3/dns_cache.proto
Line 65 in 983545f
Couldn't find any other ones.