[xds ssa] Remove environment variable protection for stateful affinity (#34435)

pull/34454/head
Eugene Ostroukhov 2 years ago committed by GitHub
parent 05f14d002d
commit 2f78fffa37
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 4
      CMakeLists.txt
  2. 8
      build_autogenerated.yaml
  3. 46
      src/core/ext/xds/xds_cluster.cc
  4. 3
      src/core/ext/xds/xds_cluster.h
  5. 7
      src/core/ext/xds/xds_endpoint.cc
  6. 5
      src/core/ext/xds/xds_http_filters.cc
  7. 2
      test/core/xds/BUILD
  8. 27
      test/core/xds/xds_cluster_resource_type_test.cc
  9. 49
      test/core/xds/xds_endpoint_resource_type_test.cc
  10. 13
      test/core/xds/xds_http_filters_test.cc
  11. 3
      test/cpp/end2end/xds/BUILD
  12. 15
      test/cpp/end2end/xds/xds_override_host_end2end_test.cc
  13. 2
      tools/run_tests/xds_k8s_test_driver/kubernetes-manifests/client-secure.deployment.yaml
  14. 2
      tools/run_tests/xds_k8s_test_driver/kubernetes-manifests/client.deployment.yaml
  15. 2
      tools/run_tests/xds_k8s_test_driver/kubernetes-manifests/server-secure.deployment.yaml
  16. 2
      tools/run_tests/xds_k8s_test_driver/kubernetes-manifests/server.deployment.yaml

4
CMakeLists.txt generated

@ -28489,10 +28489,6 @@ if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX)
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/ads.grpc.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/ads.pb.h
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/ads.grpc.pb.h
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/aggregate_cluster.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/aggregate_cluster.grpc.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/aggregate_cluster.pb.h
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/aggregate_cluster.grpc.pb.h
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/base.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/base.grpc.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/base.pb.h

@ -17457,8 +17457,7 @@ targets:
gtest: true
build: test
language: c++
headers:
- test/core/util/scoped_env_var.h
headers: []
src:
- src/proto/grpc/testing/xds/v3/address.proto
- src/proto/grpc/testing/xds/v3/aggregate_cluster.proto
@ -17804,8 +17803,7 @@ targets:
gtest: true
build: test
language: c++
headers:
- test/core/util/scoped_env_var.h
headers: []
src:
- src/proto/grpc/testing/xds/v3/address.proto
- src/proto/grpc/testing/xds/v3/base.proto
@ -18159,7 +18157,6 @@ targets:
build: test
language: c++
headers:
- test/core/util/scoped_env_var.h
- test/cpp/end2end/counted_service.h
- test/cpp/end2end/test_service_impl.h
- test/cpp/end2end/xds/xds_end2end_test_lib.h
@ -18172,7 +18169,6 @@ targets:
- src/proto/grpc/testing/simple_messages.proto
- src/proto/grpc/testing/xds/v3/address.proto
- src/proto/grpc/testing/xds/v3/ads.proto
- src/proto/grpc/testing/xds/v3/aggregate_cluster.proto
- src/proto/grpc/testing/xds/v3/base.proto
- src/proto/grpc/testing/xds/v3/cluster.proto
- src/proto/grpc/testing/xds/v3/config_source.proto

@ -20,6 +20,7 @@
#include <stddef.h>
#include <algorithm>
#include <memory>
#include <utility>
@ -58,8 +59,6 @@
#include "src/core/ext/xds/xds_resource_type.h"
#include "src/core/lib/config/core_configuration.h"
#include "src/core/lib/debug/trace.h"
#include "src/core/lib/gpr/string.h"
#include "src/core/lib/gprpp/env.h"
#include "src/core/lib/gprpp/host_port.h"
#include "src/core/lib/gprpp/match.h"
#include "src/core/lib/gprpp/ref_counted_ptr.h"
@ -71,15 +70,6 @@
namespace grpc_core {
// TODO(eostroukhov): Remove once this feature is no longer experimental.
bool XdsOverrideHostEnabled() {
auto value = GetEnv("GRPC_EXPERIMENTAL_XDS_ENABLE_OVERRIDE_HOST");
if (!value.has_value()) return false;
bool parsed_value;
bool parse_succeeded = gpr_parse_bool_value(value->c_str(), &parsed_value);
return parse_succeeded && parsed_value;
}
//
// XdsClusterResource
//
@ -633,24 +623,22 @@ absl::StatusOr<std::shared_ptr<const XdsClusterResource>> CdsResourceParse(
cds_update->outlier_detection = outlier_detection_update;
}
// Validate override host status.
if (XdsOverrideHostEnabled()) {
const auto* common_lb_config =
envoy_config_cluster_v3_Cluster_common_lb_config(cluster);
if (common_lb_config != nullptr) {
ValidationErrors::ScopedField field(&errors, ".common_lb_config");
const auto* override_host_status =
envoy_config_cluster_v3_Cluster_CommonLbConfig_override_host_status(
common_lb_config);
if (override_host_status != nullptr) {
ValidationErrors::ScopedField field(&errors, ".override_host_status");
size_t size;
const int32_t* statuses = envoy_config_core_v3_HealthStatusSet_statuses(
override_host_status, &size);
for (size_t i = 0; i < size; ++i) {
auto status = XdsHealthStatus::FromUpb(statuses[i]);
if (status.has_value()) {
cds_update->override_host_statuses.insert(*status);
}
const auto* common_lb_config =
envoy_config_cluster_v3_Cluster_common_lb_config(cluster);
if (common_lb_config != nullptr) {
ValidationErrors::ScopedField field(&errors, ".common_lb_config");
const auto* override_host_status =
envoy_config_cluster_v3_Cluster_CommonLbConfig_override_host_status(
common_lb_config);
if (override_host_status != nullptr) {
ValidationErrors::ScopedField field(&errors, ".override_host_status");
size_t size;
const int32_t* statuses = envoy_config_core_v3_HealthStatusSet_statuses(
override_host_status, &size);
for (size_t i = 0; i < size; ++i) {
auto status = XdsHealthStatus::FromUpb(statuses[i]);
if (status.has_value()) {
cds_update->override_host_statuses.insert(*status);
}
}
}

@ -21,7 +21,6 @@
#include <stdint.h>
#include <algorithm>
#include <set>
#include <string>
#include <vector>
@ -46,8 +45,6 @@
namespace grpc_core {
bool XdsOverrideHostEnabled();
struct XdsClusterResource : public XdsResourceType::ResourceData {
struct Eds {
// If empty, defaults to the cluster name.

@ -34,7 +34,6 @@
#include "absl/types/optional.h"
#include "envoy/config/core/v3/address.upb.h"
#include "envoy/config/core/v3/base.upb.h"
#include "envoy/config/core/v3/health_check.upb.h"
#include "envoy/config/endpoint/v3/endpoint.upb.h"
#include "envoy/config/endpoint/v3/endpoint.upbdefs.h"
#include "envoy/config/endpoint/v3/endpoint_components.upb.h"
@ -45,7 +44,6 @@
#include <grpc/support/log.h>
#include "src/core/ext/xds/upb_utils.h"
#include "src/core/ext/xds/xds_cluster.h"
#include "src/core/ext/xds/xds_health_status.h"
#include "src/core/ext/xds/xds_resource_type.h"
#include "src/core/lib/address_utils/parse_address.h"
@ -158,11 +156,6 @@ absl::optional<ServerAddress> ServerAddressParse(
// health_status
const int32_t health_status =
envoy_config_endpoint_v3_LbEndpoint_health_status(lb_endpoint);
if (!XdsOverrideHostEnabled() &&
health_status != envoy_config_core_v3_UNKNOWN &&
health_status != envoy_config_core_v3_HEALTHY) {
return absl::nullopt;
}
auto status = XdsHealthStatus::FromUpb(health_status);
if (!status.has_value()) return absl::nullopt;
// load_balancing_weight

@ -29,7 +29,6 @@
#include <grpc/support/log.h>
#include "src/core/ext/xds/xds_cluster.h"
#include "src/core/ext/xds/xds_http_fault_filter.h"
#include "src/core/ext/xds/xds_http_rbac_filter.h"
#include "src/core/ext/xds/xds_http_stateful_session_filter.h"
@ -88,9 +87,7 @@ XdsHttpFilterRegistry::XdsHttpFilterRegistry(bool register_builtins) {
RegisterFilter(std::make_unique<XdsHttpRouterFilter>());
RegisterFilter(std::make_unique<XdsHttpFaultFilter>());
RegisterFilter(std::make_unique<XdsHttpRbacFilter>());
if (XdsOverrideHostEnabled()) {
RegisterFilter(std::make_unique<XdsHttpStatefulSessionFilter>());
}
RegisterFilter(std::make_unique<XdsHttpStatefulSessionFilter>());
}
}

@ -311,7 +311,6 @@ grpc_cc_test(
"//src/proto/grpc/testing/xds/v3:typed_struct_proto",
"//src/proto/grpc/testing/xds/v3:wrr_locality_proto",
"//test/core/util:grpc_test_util",
"//test/core/util:scoped_env_var",
],
)
@ -329,6 +328,5 @@ grpc_cc_test(
"//src/core:grpc_xds_client",
"//src/proto/grpc/testing/xds/v3:endpoint_proto",
"//test/core/util:grpc_test_util",
"//test/core/util:scoped_env_var",
],
)

@ -63,7 +63,6 @@
#include "src/proto/grpc/testing/xds/v3/tls.pb.h"
#include "src/proto/grpc/testing/xds/v3/typed_struct.pb.h"
#include "src/proto/grpc/testing/xds/v3/wrr_locality.pb.h"
#include "test/core/util/scoped_env_var.h"
#include "test/core/util/test_config.h"
using envoy::config::cluster::v3::Cluster;
@ -1328,33 +1327,7 @@ TEST_F(OutlierDetectionTest, InvalidValues) {
using HostOverrideStatusTest = XdsClusterTest;
TEST_F(HostOverrideStatusTest, IgnoredWhenNotEnabled) {
Cluster cluster;
cluster.set_name("foo");
cluster.set_type(cluster.EDS);
cluster.mutable_eds_cluster_config()->mutable_eds_config()->mutable_self();
auto* status_set =
cluster.mutable_common_lb_config()->mutable_override_host_status();
status_set->add_statuses(envoy::config::core::v3::UNKNOWN);
status_set->add_statuses(envoy::config::core::v3::HEALTHY);
status_set->add_statuses(envoy::config::core::v3::DRAINING);
status_set->add_statuses(envoy::config::core::v3::UNHEALTHY);
std::string serialized_resource;
ASSERT_TRUE(cluster.SerializeToString(&serialized_resource));
auto* resource_type = XdsClusterResourceType::Get();
auto decode_result =
resource_type->Decode(decode_context_, serialized_resource);
ASSERT_TRUE(decode_result.resource.ok()) << decode_result.resource.status();
ASSERT_TRUE(decode_result.name.has_value());
EXPECT_EQ(*decode_result.name, "foo");
auto& resource =
static_cast<const XdsClusterResource&>(**decode_result.resource);
EXPECT_THAT(resource.override_host_statuses, ::testing::ElementsAre());
}
TEST_F(HostOverrideStatusTest, PassesOnRelevantHealthStatuses) {
ScopedExperimentalEnvVar env_var(
"GRPC_EXPERIMENTAL_XDS_ENABLE_OVERRIDE_HOST");
Cluster cluster;
cluster.set_name("foo");
cluster.set_type(cluster.EDS);

@ -54,7 +54,6 @@
#include "src/proto/grpc/testing/xds/v3/endpoint.pb.h"
#include "src/proto/grpc/testing/xds/v3/health_check.pb.h"
#include "src/proto/grpc/testing/xds/v3/percent.pb.h"
#include "test/core/util/scoped_env_var.h"
#include "test/core/util/test_config.h"
using envoy::config::endpoint::v3::ClusterLoadAssignment;
@ -914,55 +913,7 @@ TEST_F(XdsEndpointTest, DropPercentageInvalidDenominator) {
<< decode_result.resource.status();
}
TEST_F(XdsEndpointTest, IgnoresEndpointsInUnsupportedStates) {
ClusterLoadAssignment cla;
cla.set_cluster_name("foo");
auto* locality = cla.add_endpoints();
locality->mutable_load_balancing_weight()->set_value(1);
auto* locality_name = locality->mutable_locality();
locality_name->set_region("myregion");
locality_name->set_zone("myzone");
locality_name->set_sub_zone("mysubzone");
auto* endpoint = locality->add_lb_endpoints();
auto* socket_address =
endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address();
socket_address->set_address("127.0.0.1");
socket_address->set_port_value(443);
endpoint = locality->add_lb_endpoints();
endpoint->set_health_status(envoy::config::core::v3::HealthStatus::DRAINING);
socket_address =
endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address();
socket_address->set_address("127.0.0.1");
socket_address->set_port_value(444);
std::string serialized_resource;
ASSERT_TRUE(cla.SerializeToString(&serialized_resource));
auto* resource_type = XdsEndpointResourceType::Get();
auto decode_result =
resource_type->Decode(decode_context_, serialized_resource);
ASSERT_TRUE(decode_result.resource.ok()) << decode_result.resource.status();
ASSERT_TRUE(decode_result.name.has_value());
EXPECT_EQ(*decode_result.name, "foo");
auto& resource =
static_cast<const XdsEndpointResource&>(**decode_result.resource);
ASSERT_EQ(resource.priorities.size(), 1);
const auto& priority = resource.priorities[0];
ASSERT_EQ(priority.localities.size(), 1);
const auto& p = *priority.localities.begin();
ASSERT_EQ(p.first, p.second.name.get());
EXPECT_EQ(p.first->region(), "myregion");
EXPECT_EQ(p.first->zone(), "myzone");
EXPECT_EQ(p.first->sub_zone(), "mysubzone");
EXPECT_EQ(p.second.lb_weight, 1);
ASSERT_EQ(p.second.endpoints.size(), 1);
const auto& address = p.second.endpoints.front();
auto addr = grpc_sockaddr_to_string(&address.address(), /*normalize=*/false);
ASSERT_TRUE(addr.ok()) << addr.status();
EXPECT_EQ(*addr, "127.0.0.1:443");
}
TEST_F(XdsEndpointTest, EndpointHealthStatus) {
ScopedExperimentalEnvVar env_var(
"GRPC_EXPERIMENTAL_XDS_ENABLE_OVERRIDE_HOST");
ClusterLoadAssignment cla;
cla.set_cluster_name("foo");
auto* locality = cla.add_endpoints();

@ -49,7 +49,6 @@
#include "src/core/ext/xds/xds_bootstrap_grpc.h"
#include "src/core/ext/xds/xds_client.h"
#include "src/core/lib/gprpp/crash.h"
#include "src/core/lib/gprpp/env.h"
#include "src/core/lib/gprpp/ref_counted_ptr.h"
#include "src/core/lib/iomgr/error.h"
#include "src/core/lib/json/json_writer.h"
@ -1103,27 +1102,15 @@ TEST_P(XdsRbacFilterConfigTest, InvalidPermissionAndPrincipal) {
// StatefulSession filter tests
//
using XdsStatefulSessionFilterDisabledTest = XdsHttpFilterTest;
TEST_F(XdsStatefulSessionFilterDisabledTest, FilterNotRegistered) {
XdsExtension extension = MakeXdsExtension(StatefulSession());
EXPECT_EQ(GetFilter(extension.type), nullptr);
}
class XdsStatefulSessionFilterTest : public XdsHttpFilterTest {
protected:
void SetUp() override {
SetEnv("GRPC_EXPERIMENTAL_XDS_ENABLE_OVERRIDE_HOST", "true");
registry_ = XdsHttpFilterRegistry();
XdsExtension extension = MakeXdsExtension(StatefulSession());
filter_ = GetFilter(extension.type);
GPR_ASSERT(filter_ != nullptr);
}
void TearDown() override {
UnsetEnv("GRPC_EXPERIMENTAL_XDS_ENABLE_OVERRIDE_HOST");
}
const XdsHttpFilterImpl* filter_;
};

@ -440,12 +440,9 @@ grpc_cc_test(
"//:gpr",
"//:grpc",
"//:grpc++",
"//src/proto/grpc/testing/xds/v3:aggregate_cluster_proto",
"//src/proto/grpc/testing/xds/v3:router_proto",
"//src/proto/grpc/testing/xds/v3:stateful_session_cookie_proto",
"//src/proto/grpc/testing/xds/v3:stateful_session_proto",
"//test/core/util:grpc_test_util",
"//test/core/util:scoped_env_var",
],
)

@ -12,9 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
#include <chrono>
#include <string>
#include <thread>
#include <vector>
#include <gmock/gmock.h>
@ -24,17 +22,10 @@
#include "absl/strings/str_join.h"
#include "absl/strings/str_split.h"
#include "src/core/ext/filters/client_channel/backup_poller.h"
#include "src/core/lib/config/config_vars.h"
#include "src/core/lib/gprpp/match.h"
#include "src/core/lib/gprpp/time.h"
#include "src/proto/grpc/testing/xds/v3/aggregate_cluster.grpc.pb.h"
#include "src/proto/grpc/testing/xds/v3/cluster.grpc.pb.h"
#include "src/proto/grpc/testing/xds/v3/outlier_detection.grpc.pb.h"
#include "src/proto/grpc/testing/xds/v3/router.grpc.pb.h"
#include "src/proto/grpc/testing/xds/v3/stateful_session.grpc.pb.h"
#include "src/proto/grpc/testing/xds/v3/stateful_session.pb.h"
#include "src/proto/grpc/testing/xds/v3/stateful_session_cookie.pb.h"
#include "test/core/util/scoped_env_var.h"
#include "test/cpp/end2end/xds/xds_end2end_test_lib.h"
namespace grpc {
@ -189,7 +180,7 @@ class OverrideHostTest : public XdsEnd2endTest {
}
RouteConfiguration BuildRouteConfigurationWithWeightedClusters(
const std::map<absl::string_view, uint32_t> clusters) {
const std::map<absl::string_view, uint32_t>& clusters) {
RouteConfiguration new_route_config = default_route_config_;
auto* route1 = new_route_config.mutable_virtual_hosts(0)->mutable_routes(0);
for (const auto& cluster : clusters) {
@ -623,8 +614,6 @@ TEST_P(OverrideHostTest, TTLSetsMaxAge) {
} // namespace grpc
int main(int argc, char** argv) {
grpc_core::testing::ScopedExperimentalEnvVar env_var(
"GRPC_EXPERIMENTAL_XDS_ENABLE_OVERRIDE_HOST");
grpc::testing::TestEnvironment env(&argc, argv);
::testing::InitGoogleTest(&argc, argv);
// Make the backup poller poll very frequently in order to pick up

@ -51,8 +51,6 @@ spec:
value: "true"
- name: GRPC_XDS_EXPERIMENTAL_V3_SUPPORT
value: "true"
- name: GRPC_EXPERIMENTAL_XDS_ENABLE_OVERRIDE_HOST
value: "true"
volumeMounts:
- mountPath: /tmp/grpc-xds/
name: grpc-td-conf

@ -55,8 +55,6 @@ spec:
value: "true"
- name: GRPC_EXPERIMENTAL_XDS_CUSTOM_LB_CONFIG
value: "true"
- name: GRPC_EXPERIMENTAL_XDS_ENABLE_OVERRIDE_HOST
value: "true"
volumeMounts:
- mountPath: /tmp/grpc-xds/
name: grpc-td-conf

@ -54,8 +54,6 @@ spec:
value: "true"
- name: GRPC_XDS_EXPERIMENTAL_RBAC
value: "true"
- name: GRPC_EXPERIMENTAL_XDS_ENABLE_OVERRIDE_HOST
value: "true"
volumeMounts:
- mountPath: /tmp/grpc-xds/
name: grpc-td-conf

@ -47,8 +47,6 @@ spec:
value: "/tmp/grpc-xds/td-grpc-bootstrap.json"
- name: GRPC_XDS_EXPERIMENTAL_V3_SUPPORT
value: "true"
- name: GRPC_EXPERIMENTAL_XDS_ENABLE_OVERRIDE_HOST
value: "true"
volumeMounts:
- mountPath: /tmp/grpc-xds/
name: grpc-td-conf

Loading…
Cancel
Save