Skip to content

Commit

Permalink
Revert "Resolver: switching to statusor (envoyproxy#31312)"
Browse files Browse the repository at this point in the history
This reverts commit 7a1a883.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
  • Loading branch information
alyssawilk committed Jan 4, 2024
1 parent b61026f commit 1cb0c62
Show file tree
Hide file tree
Showing 19 changed files with 81 additions and 132 deletions.
4 changes: 2 additions & 2 deletions envoy/network/resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ class Resolver : public Config::UntypedFactory {
/**
* Resolve a custom address string and port to an Address::Instance.
* @param socket_address supplies the socket address to resolve.
* @return InstanceConstSharedPtr appropriate Address::Instance or an error status.
* @return InstanceConstSharedPtr appropriate Address::Instance.
*/
virtual absl::StatusOr<InstanceConstSharedPtr>
virtual InstanceConstSharedPtr
resolve(const envoy::config::core::v3::SocketAddress& socket_address) PURE;

std::string category() const override { return "envoy.resolvers"; }
Expand Down
12 changes: 3 additions & 9 deletions source/common/listener_manager/listener_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,7 @@ std::string listenerStatsScope(const envoy::config::listener::v3::Listener& conf
if (config.has_internal_listener()) {
return absl::StrCat("envoy_internal_", config.name());
}
auto address_or_error = Network::Address::resolveProtoAddress(config.address());
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
return address_or_error.value()->asString();
return Network::Address::resolveProtoAddress(config.address())->asString();
}
} // namespace

Expand Down Expand Up @@ -310,9 +308,7 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config,
} else {
// All the addresses should be same socket type, so get the first address's socket type is
// enough.
auto address_or_error = Network::Address::resolveProtoAddress(config.address());
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
auto address = std::move(address_or_error.value());
auto address = Network::Address::resolveProtoAddress(config.address());
checkIpv4CompatAddress(address, config.address());
addresses_.emplace_back(address);
address_opts_list.emplace_back(std::ref(config.socket_options()));
Expand All @@ -325,10 +321,8 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config,
"support same socket type for all the addresses.",
name_));
}
auto addresses_or_error =
auto additional_address =
Network::Address::resolveProtoAddress(config.additional_addresses(i).address());
THROW_IF_STATUS_NOT_OK(addresses_or_error, throw);
auto additional_address = std::move(addresses_or_error.value());
checkIpv4CompatAddress(address, config.additional_addresses(i).address());
addresses_.emplace_back(additional_address);
if (config.additional_addresses(i).has_socket_options()) {
Expand Down
4 changes: 1 addition & 3 deletions source/common/listener_manager/listener_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -468,9 +468,7 @@ ListenerManagerImpl::addOrUpdateListener(const envoy::config::listener::v3::List
name));
}
if (!api_listener_ && !added_via_api) {
auto listener_or_error = HttpApiListener::create(config, server_, config.name());
THROW_IF_STATUS_NOT_OK(listener_or_error, throw);
api_listener_ = std::move(listener_or_error.value());
api_listener_ = std::make_unique<HttpApiListener>(config, server_, config.name());
return true;
} else {
ENVOY_LOG(warn, "listener {} can not be added because currently only one ApiListener is "
Expand Down
15 changes: 6 additions & 9 deletions source/common/network/resolver_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace Address {
class IpResolver : public Resolver {

public:
absl::StatusOr<InstanceConstSharedPtr>
InstanceConstSharedPtr
resolve(const envoy::config::core::v3::SocketAddress& socket_address) override {
switch (socket_address.port_specifier_case()) {
case envoy::config::core::v3::SocketAddress::PortSpecifierCase::kPortValue:
Expand All @@ -31,8 +31,8 @@ class IpResolver : public Resolver {
case envoy::config::core::v3::SocketAddress::PortSpecifierCase::kNamedPort:
break;
}
return absl::InvalidArgumentError(fmt::format("IP resolver can't handle port specifier type {}",
socket_address.port_specifier_case()));
throwEnvoyExceptionOrPanic(fmt::format("IP resolver can't handle port specifier type {}",
socket_address.port_specifier_case()));
}

std::string name() const override { return Config::AddressResolverNames::get().IP; }
Expand All @@ -43,8 +43,7 @@ class IpResolver : public Resolver {
*/
REGISTER_FACTORY(IpResolver, Resolver);

absl::StatusOr<InstanceConstSharedPtr>
resolveProtoAddress(const envoy::config::core::v3::Address& address) {
InstanceConstSharedPtr resolveProtoAddress(const envoy::config::core::v3::Address& address) {
switch (address.address_case()) {
case envoy::config::core::v3::Address::AddressCase::ADDRESS_NOT_SET:
throwEnvoyExceptionOrPanic("Address must be set: " + address.DebugString());
Expand All @@ -67,7 +66,7 @@ resolveProtoAddress(const envoy::config::core::v3::Address& address) {
throwEnvoyExceptionOrPanic("Failed to resolve address:" + address.DebugString());
}

absl::StatusOr<InstanceConstSharedPtr>
InstanceConstSharedPtr
resolveProtoSocketAddress(const envoy::config::core::v3::SocketAddress& socket_address) {
Resolver* resolver = nullptr;
const std::string& resolver_name = socket_address.resolver_name();
Expand All @@ -80,9 +79,7 @@ resolveProtoSocketAddress(const envoy::config::core::v3::SocketAddress& socket_a
if (resolver == nullptr) {
throwEnvoyExceptionOrPanic(fmt::format("Unknown address resolver: {}", resolver_name));
}
auto instance_or_error = resolver->resolve(socket_address);
THROW_IF_STATUS_NOT_OK(instance_or_error, throw);
return std::move(instance_or_error.value());
return resolver->resolve(socket_address);
}

} // namespace Address
Expand Down
8 changes: 4 additions & 4 deletions source/common/network/resolver_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@ namespace Address {
/**
* Create an Instance from a envoy::config::core::v3::Address.
* @param address supplies the address proto to resolve.
* @return pointer to the Instance or an error status
* @return pointer to the Instance.
*/
absl::StatusOr<Address::InstanceConstSharedPtr>
Address::InstanceConstSharedPtr
resolveProtoAddress(const envoy::config::core::v3::Address& address);

/**
* Create an Instance from a envoy::config::core::v3::SocketAddress.
* @param address supplies the socket address proto to resolve.
* @return pointer to the Instance or an error status.
* @return pointer to the Instance.
*/
absl::StatusOr<Address::InstanceConstSharedPtr>
Address::InstanceConstSharedPtr
resolveProtoSocketAddress(const envoy::config::core::v3::SocketAddress& address);

DECLARE_FACTORY(IpResolver);
Expand Down
18 changes: 7 additions & 11 deletions source/common/upstream/health_discovery_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -371,11 +371,10 @@ HdsCluster::HdsCluster(Server::Configuration::ServerFactoryContext& server_conte
for (const auto& host : locality_endpoints.lb_endpoints()) {
const LocalityEndpointTuple endpoint_key = {locality_endpoints.locality(), host};
// Initialize an endpoint host object.
auto address_or_error = Network::Address::resolveProtoAddress(host.endpoint().address());
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
HostSharedPtr endpoint = std::make_shared<HostImpl>(
info_, "", std::move(address_or_error.value()), nullptr, 1, locality_endpoints.locality(),
host.endpoint().health_check_config(), 0, envoy::config::core::v3::UNKNOWN, time_source_);
info_, "", Network::Address::resolveProtoAddress(host.endpoint().address()), nullptr, 1,
locality_endpoints.locality(), host.endpoint().health_check_config(), 0,
envoy::config::core::v3::UNKNOWN, time_source_);
// Add this host/endpoint pointer to our flat list of endpoints for health checking.
hosts_->push_back(endpoint);
// Add this host/endpoint pointer to our structured list by locality so results can be
Expand Down Expand Up @@ -484,13 +483,10 @@ void HdsCluster::updateHosts(
host = host_pair->second;
} else {
// We do not have this endpoint saved, so create a new one.
auto address_or_error =
Network::Address::resolveProtoAddress(endpoint.endpoint().address());
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
host = std::make_shared<HostImpl>(info_, "", std::move(address_or_error.value()), nullptr,
1, endpoints.locality(),
endpoint.endpoint().health_check_config(), 0,
envoy::config::core::v3::UNKNOWN, time_source_);
host = std::make_shared<HostImpl>(
info_, "", Network::Address::resolveProtoAddress(endpoint.endpoint().address()),
nullptr, 1, endpoints.locality(), endpoint.endpoint().health_check_config(), 0,
envoy::config::core::v3::UNKNOWN, time_source_);

// Set the initial health status as in HdsCluster::initialize.
host->healthFlagSet(Host::HealthFlag::FAILED_ACTIVE_HC);
Expand Down
30 changes: 8 additions & 22 deletions source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -227,14 +227,10 @@ parseBindConfig(::Envoy::OptRef<const envoy::config::core::v3::BindConfig> bind_
std::vector<::Envoy::Upstream::UpstreamLocalAddress> upstream_local_addresses;
if (bind_config.has_value()) {
UpstreamLocalAddress upstream_local_address;
upstream_local_address.address_ = nullptr;
if (bind_config->has_source_address()) {

auto address_or_error =
::Envoy::Network::Address::resolveProtoSocketAddress(bind_config->source_address());
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
upstream_local_address.address_ = address_or_error.value();
}
upstream_local_address.address_ =
bind_config->has_source_address()
? ::Envoy::Network::Address::resolveProtoSocketAddress(bind_config->source_address())
: nullptr;
upstream_local_address.socket_options_ = std::make_shared<Network::ConnectionSocket::Options>();

::Envoy::Network::Socket::appendOptions(upstream_local_address.socket_options_,
Expand All @@ -246,10 +242,8 @@ parseBindConfig(::Envoy::OptRef<const envoy::config::core::v3::BindConfig> bind_

for (const auto& extra_source_address : bind_config->extra_source_addresses()) {
UpstreamLocalAddress extra_upstream_local_address;
auto address_or_error =
extra_upstream_local_address.address_ =
::Envoy::Network::Address::resolveProtoSocketAddress(extra_source_address.address());
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
extra_upstream_local_address.address_ = address_or_error.value();

extra_upstream_local_address.socket_options_ =
std::make_shared<::Envoy::Network::ConnectionSocket::Options>();
Expand All @@ -270,10 +264,8 @@ parseBindConfig(::Envoy::OptRef<const envoy::config::core::v3::BindConfig> bind_

for (const auto& additional_source_address : bind_config->additional_source_addresses()) {
UpstreamLocalAddress additional_upstream_local_address;
auto address_or_error =
additional_upstream_local_address.address_ =
::Envoy::Network::Address::resolveProtoSocketAddress(additional_source_address);
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
additional_upstream_local_address.address_ = address_or_error.value();
additional_upstream_local_address.socket_options_ =
std::make_shared<::Envoy::Network::ConnectionSocket::Options>();
::Envoy::Network::Socket::appendOptions(additional_upstream_local_address.socket_options_,
Expand Down Expand Up @@ -1749,11 +1741,7 @@ void ClusterImplBase::reloadHealthyHostsHelper(const HostSharedPtr&) {

const Network::Address::InstanceConstSharedPtr
ClusterImplBase::resolveProtoAddress(const envoy::config::core::v3::Address& address) {
TRY_ASSERT_MAIN_THREAD {
auto address_or_error = Network::Address::resolveProtoAddress(address);
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
return address_or_error.value();
}
TRY_ASSERT_MAIN_THREAD { return Network::Address::resolveProtoAddress(address); }
END_TRY
CATCH(EnvoyException & e, {
if (info_->type() == envoy::config::cluster::v3::Cluster::STATIC ||
Expand Down Expand Up @@ -2421,9 +2409,7 @@ Network::Address::InstanceConstSharedPtr resolveHealthCheckAddress(
Network::Address::InstanceConstSharedPtr health_check_address;
const auto& port_value = health_check_config.port_value();
if (health_check_config.has_address()) {
auto address_or_error = Network::Address::resolveProtoAddress(health_check_config.address());
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
auto address = address_or_error.value();
auto address = Network::Address::resolveProtoAddress(health_check_config.address());
health_check_address =
port_value == 0 ? address : Network::Utility::getAddressWithPort(*address, port_value);
} else {
Expand Down
4 changes: 1 addition & 3 deletions source/extensions/network/dns_resolver/cares/dns_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -555,9 +555,7 @@ class CaresDnsResolverFactory : public DnsResolverFactory,
const auto& resolver_addrs = cares.resolvers();
resolvers.reserve(resolver_addrs.size());
for (const auto& resolver_addr : resolver_addrs) {
auto address_or_error = Network::Address::resolveProtoAddress(resolver_addr);
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
resolvers.push_back(std::move(address_or_error.value()));
resolvers.push_back(Network::Address::resolveProtoAddress(resolver_addr));
}
}
return std::make_shared<Network::DnsResolverImpl>(cares, dispatcher, resolvers,
Expand Down
5 changes: 2 additions & 3 deletions source/extensions/stat_sinks/dog_statsd/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ DogStatsdSinkFactory::createStatsSink(const Protobuf::Message& config,
const auto& sink_config =
MessageUtil::downcastAndValidate<const envoy::config::metrics::v3::DogStatsdSink&>(
config, server.messageValidationContext().staticValidationVisitor());
auto address_or_error = Network::Address::resolveProtoAddress(sink_config.address());
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
Network::Address::InstanceConstSharedPtr address = address_or_error.value();
Network::Address::InstanceConstSharedPtr address =
Network::Address::resolveProtoAddress(sink_config.address());
ENVOY_LOG(debug, "dog_statsd UDP ip address: {}", address->asString());
absl::optional<uint64_t> max_bytes;
if (sink_config.has_max_bytes_per_datagram()) {
Expand Down
5 changes: 2 additions & 3 deletions source/extensions/stat_sinks/graphite_statsd/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ GraphiteStatsdSinkFactory::createStatsSink(const Protobuf::Message& config,
switch (statsd_sink.statsd_specifier_case()) {
case envoy::extensions::stat_sinks::graphite_statsd::v3::GraphiteStatsdSink::StatsdSpecifierCase::
kAddress: {
auto address_or_error = Network::Address::resolveProtoAddress(statsd_sink.address());
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
Network::Address::InstanceConstSharedPtr address = address_or_error.value();
Network::Address::InstanceConstSharedPtr address =
Network::Address::resolveProtoAddress(statsd_sink.address());
ENVOY_LOG(debug, "statsd UDP ip address: {}", address->asString());
absl::optional<uint64_t> max_bytes;
if (statsd_sink.has_max_bytes_per_datagram()) {
Expand Down
5 changes: 2 additions & 3 deletions source/extensions/stat_sinks/statsd/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ StatsdSinkFactory::createStatsSink(const Protobuf::Message& config,
config, server.messageValidationContext().staticValidationVisitor());
switch (statsd_sink.statsd_specifier_case()) {
case envoy::config::metrics::v3::StatsdSink::StatsdSpecifierCase::kAddress: {
auto address_or_error = Network::Address::resolveProtoAddress(statsd_sink.address());
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
Network::Address::InstanceConstSharedPtr address = address_or_error.value();
Network::Address::InstanceConstSharedPtr address =
Network::Address::resolveProtoAddress(statsd_sink.address());
ENVOY_LOG(debug, "statsd UDP ip address: {}", address->asString());
return std::make_unique<Common::Statsd::UdpStatsdSink>(server.threadLocal(), std::move(address),
false, statsd_sink.prefix());
Expand Down
20 changes: 5 additions & 15 deletions source/server/api_listener_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
namespace Envoy {
namespace Server {

ApiListenerImplBase::ApiListenerImplBase(Network::Address::InstanceConstSharedPtr&& address,
const envoy::config::listener::v3::Listener& config,
ApiListenerImplBase::ApiListenerImplBase(const envoy::config::listener::v3::Listener& config,
Server::Instance& server, const std::string& name)
: config_(config), name_(name), address_(std::move(address)),
: config_(config), name_(name),
address_(Network::Address::resolveProtoAddress(config.address())),
factory_context_(server, *this, server.stats().createScope(""),
server.stats().createScope(fmt::format("listener.api.{}.", name_)),
std::make_shared<ListenerInfoImpl>(config)) {}
Expand All @@ -41,19 +41,9 @@ HttpApiListener::ApiListenerWrapper::newStreamHandle(Http::ResponseEncoder& resp
return http_connection_manager_->newStreamHandle(response_encoder, is_internally_created);
}

absl::StatusOr<std::unique_ptr<HttpApiListener>>
HttpApiListener::create(const envoy::config::listener::v3::Listener& config,
Server::Instance& server, const std::string& name) {
auto address_or_error = Network::Address::resolveProtoAddress(config.address());
RETURN_IF_STATUS_NOT_OK(address_or_error);
return std::unique_ptr<HttpApiListener>(
new HttpApiListener(std::move(address_or_error.value()), config, server, name));
}

HttpApiListener::HttpApiListener(Network::Address::InstanceConstSharedPtr&& address,
const envoy::config::listener::v3::Listener& config,
HttpApiListener::HttpApiListener(const envoy::config::listener::v3::Listener& config,
Server::Instance& server, const std::string& name)
: ApiListenerImplBase(std::move(address), config, server, name) {
: ApiListenerImplBase(config, server, name) {
if (config.api_listener().api_listener().type_url() ==
absl::StrCat(
"type.googleapis.com/",
Expand Down
13 changes: 4 additions & 9 deletions source/server/api_listener_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ class ApiListenerImplBase : public ApiListener,
}

protected:
ApiListenerImplBase(Network::Address::InstanceConstSharedPtr&& address,
const envoy::config::listener::v3::Listener& config, Server::Instance& server,
ApiListenerImplBase(const envoy::config::listener::v3::Listener& config, Server::Instance& server,
const std::string& name);

// Synthetic class that acts as a stub Network::ReadFilterCallbacks.
Expand Down Expand Up @@ -222,18 +221,14 @@ class HttpApiListener : public ApiListenerImplBase {
Http::ApiListenerPtr http_connection_manager_;
};

HttpApiListener(const envoy::config::listener::v3::Listener& config, Server::Instance& server,
const std::string& name);

// ApiListener
ApiListener::Type type() const override { return ApiListener::Type::HttpApiListener; }
Http::ApiListenerPtr createHttpApiListener(Event::Dispatcher& dispatcher) override;
static absl::StatusOr<std::unique_ptr<HttpApiListener>>
create(const envoy::config::listener::v3::Listener& config, Server::Instance& server,
const std::string& name);

private:
HttpApiListener(Network::Address::InstanceConstSharedPtr&& address,
const envoy::config::listener::v3::Listener& config, Server::Instance& server,
const std::string& name);

// Need to store the factory due to the shared_ptrs that need to be kept alive: date provider,
// route config manager, scoped route config manager.
std::function<Http::ApiListenerPtr(Network::ReadFilterCallbacks&)>
Expand Down
4 changes: 1 addition & 3 deletions source/server/configuration_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,7 @@ InitialImpl::InitialImpl(const envoy::config::bootstrap::v3::Bootstrap& bootstra
admin_.profile_path_ =
admin.profile_path().empty() ? "/var/log/envoy/envoy.prof" : admin.profile_path();
if (admin.has_address()) {
auto address_or_error = Network::Address::resolveProtoAddress(admin.address());
THROW_IF_STATUS_NOT_OK(address_or_error, throw);
admin_.address_ = std::move(address_or_error.value());
admin_.address_ = Network::Address::resolveProtoAddress(admin.address());
}
admin_.socket_options_ = std::make_shared<std::vector<Network::Socket::OptionConstSharedPtr>>();
if (!admin.socket_options().empty()) {
Expand Down
Loading

0 comments on commit 1cb0c62

Please sign in to comment.