Skip to content

Commit

Permalink
api: remove hardcoded type urls part.1 (#10479)
Browse files Browse the repository at this point in the history
 #10209. There are hardcoded type URLs in resources.h. In this PR, I removed all of the dependencies of those. Now, codes that depend on that are only test code. In the future, I will remove that from test codes and completely destroy that.

Risk Level: Low
Testing: N/A

Signed-off-by: shikugawa <rei@tetrate.io>
  • Loading branch information
Shikugawa authored Apr 3, 2020
1 parent 991f97c commit 1323ff2
Show file tree
Hide file tree
Showing 30 changed files with 154 additions and 79 deletions.
11 changes: 1 addition & 10 deletions include/envoy/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ envoy_cc_library(
hdrs = ["subscription.h"],
deps = [
"//include/envoy/stats:stats_macros",
"//source/common/config:api_type_oracle_lib",
"//source/common/protobuf",
"@envoy_api//envoy/service/discovery/v3:pkg_cc_proto",
],
Expand All @@ -75,13 +76,3 @@ envoy_cc_library(
"//source/common/protobuf",
],
)

envoy_cc_library(
name = "discovery_service_base_interface",
hdrs = ["discovery_service_base.h"],
deps = [
":subscription_interface",
"//source/common/config:api_type_oracle_lib",
"//source/common/protobuf",
],
)
28 changes: 0 additions & 28 deletions include/envoy/config/discovery_service_base.h

This file was deleted.

19 changes: 19 additions & 0 deletions source/common/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,16 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "resource_name_lib",
hdrs = ["resource_name.h"],
deps = [
":api_type_oracle_lib",
"//source/common/common:assert_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
],
)

envoy_cc_library(
name = "resources_lib",
hdrs = ["resources.h"],
Expand Down Expand Up @@ -361,6 +371,15 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "subscription_base_interface",
hdrs = ["subscription_base.h"],
deps = [
":resource_name_lib",
"//include/envoy/config:subscription_interface",
],
)

envoy_cc_library(
name = "well_known_names",
srcs = ["well_known_names.cc"],
Expand Down
2 changes: 1 addition & 1 deletion source/common/config/grpc_mux_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,4 +244,4 @@ void GrpcMuxImpl::drainRequests() {
}

} // namespace Config
} // namespace Envoy
} // namespace Envoy
61 changes: 61 additions & 0 deletions source/common/config/resource_name.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#pragma once

#include <string>
#include <vector>

#include "envoy/config/core/v3/config_source.pb.h"

#include "common/common/assert.h"
#include "common/config/api_type_oracle.h"

namespace Envoy {
namespace Config {

/**
* Get resource name from api type and version.
*/
template <typename Current>
std::string getResourceName(envoy::config::core::v3::ApiVersion resource_api_version) {
switch (resource_api_version) {
case envoy::config::core::v3::ApiVersion::AUTO:
case envoy::config::core::v3::ApiVersion::V2:
return ApiTypeOracle::getEarlierVersionMessageTypeName(Current().GetDescriptor()->full_name())
.value();
case envoy::config::core::v3::ApiVersion::V3:
return Current().GetDescriptor()->full_name();
default:
NOT_REACHED_GCOVR_EXCL_LINE;
}
}

/**
* Get type url from api type and version.
*/
template <typename Current>
std::string getTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version) {
return "type.googleapis.com/" + getResourceName<Current>(resource_api_version);
}

/**
* get all version resource names.
*/
template <typename Current> std::vector<std::string> getAllVersionResourceNames() {
return std::vector<std::string>{
Current().GetDescriptor()->full_name(),
ApiTypeOracle::getEarlierVersionMessageTypeName(Current().GetDescriptor()->full_name())
.value()};
}

/**
* get all version type urls.
*/
template <typename Current> std::vector<std::string> getAllVersionTypeUrls() {
auto resource_names = getAllVersionResourceNames<Current>();
for (auto&& resource_name : resource_names) {
resource_name = "type.googleapis.com/" + resource_name;
}
return resource_names;
}

} // namespace Config
} // namespace Envoy
24 changes: 24 additions & 0 deletions source/common/config/subscription_base.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#pragma once

#include "envoy/config/subscription.h"

#include "common/config/resource_name.h"

namespace Envoy {
namespace Config {

template <typename Current> struct SubscriptionBase : public Config::SubscriptionCallbacks {
public:
SubscriptionBase(const envoy::config::core::v3::ApiVersion api_version)
: api_version_(api_version) {}

std::string getResourceName() const {
return Envoy::Config::getResourceName<Current>(api_version_);
}

private:
const envoy::config::core::v3::ApiVersion api_version_;
};

} // namespace Config
} // namespace Envoy
1 change: 0 additions & 1 deletion source/common/config/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "common/common/hex.h"
#include "common/common/utility.h"
#include "common/config/api_type_oracle.h"
#include "common/config/resources.h"
#include "common/config/version_converter.h"
#include "common/config/well_known_names.h"
#include "common/protobuf/protobuf.h"
Expand Down
6 changes: 3 additions & 3 deletions source/common/router/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ envoy_cc_library(
hdrs = ["vhds.h"],
deps = [
":config_lib",
"//include/envoy/config:discovery_service_base_interface",
"//include/envoy/config:subscription_interface",
"//include/envoy/http:codes_interface",
"//include/envoy/local_info:local_info_interface",
Expand All @@ -135,6 +134,7 @@ envoy_cc_library(
"//source/common/common:assert_lib",
"//source/common/common:minimal_logger_lib",
"//source/common/config:api_version_lib",
"//source/common/config:subscription_base_interface",
"//source/common/config:utility_lib",
"//source/common/init:target_lib",
"//source/common/protobuf:utility_lib",
Expand All @@ -152,7 +152,6 @@ envoy_cc_library(
hdrs = ["rds_impl.h"],
deps = [
":config_lib",
"//include/envoy/config:discovery_service_base_interface",
"//include/envoy/config:subscription_interface",
"//include/envoy/http:codes_interface",
"//include/envoy/local_info:local_info_interface",
Expand All @@ -167,6 +166,7 @@ envoy_cc_library(
"//source/common/common:cleanup_lib",
"//source/common/common:minimal_logger_lib",
"//source/common/config:api_version_lib",
"//source/common/config:subscription_base_interface",
"//source/common/config:subscription_factory_lib",
"//source/common/config:utility_lib",
"//source/common/config:version_converter_lib",
Expand Down Expand Up @@ -210,7 +210,6 @@ envoy_cc_library(
":rds_lib",
":scoped_config_lib",
"//include/envoy/config:config_provider_interface",
"//include/envoy/config:discovery_service_base_interface",
"//include/envoy/config:subscription_interface",
"//include/envoy/router:route_config_provider_manager_interface",
"//include/envoy/stats:stats_interface",
Expand All @@ -219,6 +218,7 @@ envoy_cc_library(
"//source/common/common:minimal_logger_lib",
"//source/common/config:api_version_lib",
"//source/common/config:config_provider_lib",
"//source/common/config:subscription_base_interface",
"//source/common/config:version_converter_lib",
"//source/common/init:manager_lib",
"//source/common/init:watcher_lib",
Expand Down
6 changes: 4 additions & 2 deletions source/common/router/rds_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription(
const uint64_t manager_identifier, Server::Configuration::ServerFactoryContext& factory_context,
const std::string& stat_prefix,
Envoy::Router::RouteConfigProviderManagerImpl& route_config_provider_manager)
: route_config_name_(rds.route_config_name()), factory_context_(factory_context),
: Envoy::Config::SubscriptionBase<envoy::config::route::v3::RouteConfiguration>(
rds.config_source().resource_api_version()),
route_config_name_(rds.route_config_name()), factory_context_(factory_context),
validator_(factory_context.messageValidationContext().dynamicValidationVisitor()),
parent_init_target_(fmt::format("RdsRouteConfigSubscription init {}", route_config_name_),
[this]() { local_init_manager_.initialize(local_init_watcher_); }),
Expand All @@ -82,7 +84,7 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription(
stat_prefix_(stat_prefix), stats_({ALL_RDS_STATS(POOL_COUNTER(*scope_))}),
route_config_provider_manager_(route_config_provider_manager),
manager_identifier_(manager_identifier) {
const auto resource_name = getResourceName(rds.config_source().resource_api_version());
const auto resource_name = getResourceName();
subscription_ =
factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource(
rds.config_source(), Grpc::Common::typeUrl(resource_name), *scope_, *this);
Expand Down
1 change: 0 additions & 1 deletion source/common/router/rds_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
#include "common/common/callback_impl.h"
#include "common/common/cleanup.h"
#include "common/common/logger.h"
#include "common/config/resources.h"
#include "common/init/manager_impl.h"
#include "common/init/target_impl.h"
#include "common/init/watcher_impl.h"
Expand Down
4 changes: 3 additions & 1 deletion source/common/router/scoped_rds.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,15 @@ ScopedRdsConfigSubscription::ScopedRdsConfigSubscription(
ScopedRoutesConfigProviderManager& config_provider_manager)
: DeltaConfigSubscriptionInstance("SRDS", manager_identifier, config_provider_manager,
factory_context),
Envoy::Config::SubscriptionBase<envoy::config::route::v3::ScopedRouteConfiguration>(
rds_config_source.resource_api_version()),
factory_context_(factory_context), name_(name), scope_key_builder_(scope_key_builder),
scope_(factory_context.scope().createScope(stat_prefix + "scoped_rds." + name + ".")),
stats_({ALL_SCOPED_RDS_STATS(POOL_COUNTER(*scope_))}),
rds_config_source_(std::move(rds_config_source)),
validation_visitor_(factory_context.messageValidationContext().dynamicValidationVisitor()),
stat_prefix_(stat_prefix), route_config_provider_manager_(route_config_provider_manager) {
const auto resource_name = getResourceName(rds_config_source_.resource_api_version());
const auto resource_name = getResourceName();
subscription_ =
factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource(
scoped_rds.scoped_rds_config_source(), Grpc::Common::typeUrl(resource_name), *scope_,
Expand Down
2 changes: 1 addition & 1 deletion source/common/router/scoped_rds.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "envoy/common/callback.h"
#include "envoy/config/core/v3/config_source.pb.h"
#include "envoy/config/discovery_service_base.h"
#include "envoy/config/route/v3/scoped_route.pb.h"
#include "envoy/config/subscription.h"
#include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h"
Expand All @@ -13,6 +12,7 @@
#include "envoy/stats/scope.h"

#include "common/config/config_provider_impl.h"
#include "common/config/subscription_base.h"
#include "common/init/manager_impl.h"
#include "common/router/rds_impl.h"
#include "common/router/scoped_config_impl.h"
Expand Down
5 changes: 3 additions & 2 deletions source/common/router/vhds.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info,
const std::string& stat_prefix,
std::unordered_set<RouteConfigProvider*>& route_config_providers,
envoy::config::core::v3::ApiVersion resource_api_version)
: config_update_info_(config_update_info),
: Envoy::Config::SubscriptionBase<envoy::config::route::v3::VirtualHost>(resource_api_version),
config_update_info_(config_update_info),
scope_(factory_context.scope().createScope(stat_prefix + "vhds." +
config_update_info_->routeConfigName() + ".")),
stats_({ALL_VHDS_STATS(POOL_COUNTER(*scope_))}),
Expand All @@ -40,7 +41,7 @@ VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info,
if (config_source != envoy::config::core::v3::ApiConfigSource::DELTA_GRPC) {
throw EnvoyException("vhds: only 'DELTA_GRPC' is supported as an api_type.");
}
const auto resource_name = getResourceName(resource_api_version);
const auto resource_name = getResourceName();
subscription_ =
factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource(
config_update_info_->routeConfiguration().vhds().config_source(),
Expand Down
2 changes: 1 addition & 1 deletion source/common/router/vhds.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <unordered_set>

#include "envoy/config/core/v3/config_source.pb.h"
#include "envoy/config/discovery_service_base.h"
#include "envoy/config/route/v3/route_components.pb.h"
#include "envoy/config/subscription.h"
#include "envoy/http/codes.h"
Expand All @@ -21,6 +20,7 @@
#include "envoy/thread_local/thread_local.h"

#include "common/common/logger.h"
#include "common/config/subscription_base.h"
#include "common/init/target_impl.h"
#include "common/protobuf/utility.h"

Expand Down
2 changes: 1 addition & 1 deletion source/common/runtime/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ envoy_cc_library(
external_deps = ["ssl"],
deps = [
":runtime_features_lib",
"//include/envoy/config:discovery_service_base_interface",
"//include/envoy/config:subscription_interface",
"//include/envoy/event:dispatcher_interface",
"//include/envoy/init:manager_interface",
Expand All @@ -48,6 +47,7 @@ envoy_cc_library(
"//source/common/common:thread_lib",
"//source/common/common:utility_lib",
"//source/common/config:api_version_lib",
"//source/common/config:subscription_base_interface",
"//source/common/filesystem:directory_lib",
"//source/common/grpc:common_lib",
"//source/common/init:target_lib",
Expand Down
6 changes: 4 additions & 2 deletions source/common/runtime/runtime_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,9 @@ void LoaderImpl::initialize(Upstream::ClusterManager& cm) { cm_ = &cm; }
RtdsSubscription::RtdsSubscription(
LoaderImpl& parent, const envoy::config::bootstrap::v3::RuntimeLayer::RtdsLayer& rtds_layer,
Stats::Store& store, ProtobufMessage::ValidationVisitor& validation_visitor)
: parent_(parent), config_source_(rtds_layer.rtds_config()), store_(store),
: Envoy::Config::SubscriptionBase<envoy::service::runtime::v3::Runtime>(
rtds_layer.rtds_config().resource_api_version()),
parent_(parent), config_source_(rtds_layer.rtds_config()), store_(store),
resource_name_(rtds_layer.name()),
init_target_("RTDS " + resource_name_, [this]() { start(); }),
validation_visitor_(validation_visitor) {}
Expand Down Expand Up @@ -583,7 +585,7 @@ void RtdsSubscription::start() {
// We have to delay the subscription creation until init-time, since the
// cluster manager resources are not available in the constructor when
// instantiated in the server instance.
const auto resource_name = getResourceName(config_source_.resource_api_version());
const auto resource_name = getResourceName();
subscription_ = parent_.cm_->subscriptionFactory().subscriptionFromConfigSource(
config_source_, Grpc::Common::typeUrl(resource_name), store_, *this);
subscription_->start({resource_name_});
Expand Down
2 changes: 1 addition & 1 deletion source/common/runtime/runtime_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "envoy/common/exception.h"
#include "envoy/config/bootstrap/v3/bootstrap.pb.h"
#include "envoy/config/core/v3/config_source.pb.h"
#include "envoy/config/discovery_service_base.h"
#include "envoy/config/subscription.h"
#include "envoy/init/manager.h"
#include "envoy/runtime/runtime.h"
Expand All @@ -24,6 +23,7 @@
#include "common/common/assert.h"
#include "common/common/logger.h"
#include "common/common/thread.h"
#include "common/config/subscription_base.h"
#include "common/init/target_impl.h"
#include "common/singleton/threadsafe_singleton.h"

Expand Down
Loading

0 comments on commit 1323ff2

Please sign in to comment.