Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ BROWSE
.deps
*.pyc
SOURCE_VERSION
.cache
10 changes: 10 additions & 0 deletions source/common/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "datasource_lib",
srcs = ["datasource.cc"],
hdrs = ["datasource.h"],
deps = [
"//source/common/filesystem:filesystem_lib",
"@envoy_api//envoy/api/v2/core:base_cc",
],
)

envoy_cc_library(
name = "filter_json_lib",
srcs = ["filter_json.cc"],
Expand Down
33 changes: 33 additions & 0 deletions source/common/config/datasource.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#include "common/config/datasource.h"

#include "common/common/fmt.h"
#include "common/filesystem/filesystem_impl.h"

namespace Envoy {
namespace Config {

std::string DataSource::read(bool allow_empty) const {
switch (source_.specifier_case()) {
case envoy::api::v2::core::DataSource::kFilename:
return Filesystem::fileReadToEnd(source_.filename());
case envoy::api::v2::core::DataSource::kInlineBytes:
return source_.inline_bytes();
case envoy::api::v2::core::DataSource::kInlineString:
return source_.inline_string();
default:
if (!allow_empty) {
throw EnvoyException(
fmt::format("Unexpected DataSource::specifier_case(): {}", source_.specifier_case()));
}
return "";
}
}

std::string DataSource::getPath() const {
return source_.specifier_case() == envoy::api::v2::core::DataSource::kFilename
? source_.filename()
: "";
}

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

#include "envoy/api/v2/core/base.pb.h"

namespace Envoy {
namespace Config {

// envoy::api::v2::core::DataSource helper.
class DataSource {
public:
DataSource(const envoy::api::v2::core::DataSource& source) : source_(source) {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given how this class is used, the API is a bit weird: every use constructs a temporary, calls one method, then destructs it. Instead, should we just have two static functions here, that both take a DataSource& ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Greg, this feels weird. Is there any reason those are not just namespaced functions that take source as a parameter, similarly to those in Envoy::Filesystem?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was assuming the compiler would be smart enough to optimize any overhead here away, since the class is entirely const/immutable everywhere, but sure, I can make it just some namespace functions if you folks prefer this convention.


/**
* Read contents of the DataSource.
* @param allow_empty return an empty string if no DataSource case is specified.
* @return std::string with DataSource contents.
* @throw EnvoyException if no DataSource case is specified and !allow_empty.
*/
std::string read(bool allow_empty) const;

/**
* @return std::string path to DataSource if a filename, otherwise an empty string.
*/
std::string getPath() const;

private:
const envoy::api::v2::core::DataSource& source_;
};

} // namespace Config
} // namespace Envoy
1 change: 1 addition & 0 deletions source/common/grpc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ envoy_cc_library(
"//source/common/common:empty_string",
"//source/common/common:linked_object",
"//source/common/common:thread_lib",
"//source/common/config:datasource_lib",
"//source/common/tracing:http_tracer_lib",
],
)
Expand Down
1 change: 1 addition & 0 deletions source/common/grpc/async_client_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ GoogleAsyncClientFactoryImpl::GoogleAsyncClientFactoryImpl(
scope_(scope.createScope(fmt::format("grpc.{}.", config.stat_prefix()))), config_(config) {
#ifndef ENVOY_GOOGLE_GRPC
UNREFERENCED_PARAMETER(tls_);
UNREFERENCED_PARAMETER(google_tls_slot_);
UNREFERENCED_PARAMETER(scope_);
UNREFERENCED_PARAMETER(config_);
throw EnvoyException("Google C++ gRPC client is not linked");
Expand Down
1 change: 1 addition & 0 deletions source/common/grpc/google_async_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ GoogleAsyncClientImpl::GoogleAsyncClientImpl(
}

GoogleAsyncClientImpl::~GoogleAsyncClientImpl() {
ENVOY_LOG(debug, "Client teardown, resetting streams");
while (!active_streams_.empty()) {
active_streams_.front()->resetStream();
}
Expand Down
15 changes: 13 additions & 2 deletions source/common/grpc/google_async_client_site.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Site-specific customizations for Google gRPC client implementation.

#include "common/config/datasource.h"
#include "common/grpc/google_async_client_impl.h"

#include "grpc++/channel.h"
Expand All @@ -11,8 +12,18 @@ namespace Grpc {

std::shared_ptr<grpc::Channel>
GoogleAsyncClientImpl::createChannel(const envoy::api::v2::core::GrpcService::GoogleGrpc& config) {
// TODO(htuch): add support for SSL, OAuth2, GCP, etc. credentials.
std::shared_ptr<grpc::ChannelCredentials> creds = grpc::InsecureChannelCredentials();
// TODO(htuch): add support for OAuth2, GCP, etc. credentials.
std::shared_ptr<grpc::ChannelCredentials> creds;
if (config.has_ssl_credentials()) {
const grpc::SslCredentialsOptions ssl_creds = {
.pem_root_certs = Config::DataSource(config.ssl_credentials().root_certs()).read(true),
.pem_private_key = Config::DataSource(config.ssl_credentials().private_key()).read(true),
.pem_cert_chain = Config::DataSource(config.ssl_credentials().cert_chain()).read(true),
};
creds = grpc::SslCredentials(ssl_creds);
} else {
creds = grpc::InsecureChannelCredentials();
}
return CreateChannel(config.target_uri(), creds);
}

Expand Down
1 change: 1 addition & 0 deletions source/common/ssl/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ envoy_cc_library(
deps = [
"//include/envoy/ssl:context_config_interface",
"//source/common/common:assert_lib",
"//source/common/config:datasource_lib",
"//source/common/config:tls_context_json_lib",
"//source/common/json:json_loader_lib",
"//source/common/protobuf:utility_lib",
Expand Down
61 changes: 21 additions & 40 deletions source/common/ssl/context_config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
#include <string>

#include "common/common/assert.h"
#include "common/config/datasource.h"
#include "common/config/tls_context_json.h"
#include "common/filesystem/filesystem_impl.h"
#include "common/protobuf/utility.h"

#include "openssl/ssl.h"
Expand Down Expand Up @@ -41,22 +41,27 @@ ContextConfigImpl::ContextConfigImpl(const envoy::api::v2::auth::CommonTlsContex
RepeatedPtrUtil::join(config.tls_params().cipher_suites(), ":"), DEFAULT_CIPHER_SUITES)),
ecdh_curves_(StringUtil::nonEmptyStringOrDefault(
RepeatedPtrUtil::join(config.tls_params().ecdh_curves(), ":"), DEFAULT_ECDH_CURVES)),
ca_cert_(readDataSource(config.validation_context().trusted_ca(), true)),
ca_cert_path_(getDataSourcePath(config.validation_context().trusted_ca())),
certificate_revocation_list_(readDataSource(config.validation_context().crl(), true)),
certificate_revocation_list_path_(getDataSourcePath(config.validation_context().crl())),
cert_chain_(config.tls_certificates().empty()
? ""
: readDataSource(config.tls_certificates()[0].certificate_chain(), true)),
cert_chain_path_(config.tls_certificates().empty()
? ""
: getDataSourcePath(config.tls_certificates()[0].certificate_chain())),
ca_cert_(Config::DataSource(config.validation_context().trusted_ca()).read(true)),
ca_cert_path_(Config::DataSource(config.validation_context().trusted_ca()).getPath()),
certificate_revocation_list_(
Config::DataSource(config.validation_context().crl()).read(true)),
certificate_revocation_list_path_(
Config::DataSource(config.validation_context().crl()).getPath()),
cert_chain_(
config.tls_certificates().empty()
? ""
: Config::DataSource(config.tls_certificates()[0].certificate_chain()).read(true)),
cert_chain_path_(
config.tls_certificates().empty()
? ""
: Config::DataSource(config.tls_certificates()[0].certificate_chain()).getPath()),
private_key_(config.tls_certificates().empty()
? ""
: readDataSource(config.tls_certificates()[0].private_key(), true)),
private_key_path_(config.tls_certificates().empty()
? ""
: getDataSourcePath(config.tls_certificates()[0].private_key())),
: Config::DataSource(config.tls_certificates()[0].private_key()).read(true)),
private_key_path_(
config.tls_certificates().empty()
? ""
: Config::DataSource(config.tls_certificates()[0].private_key()).getPath()),
verify_subject_alt_name_list_(config.validation_context().verify_subject_alt_name().begin(),
config.validation_context().verify_subject_alt_name().end()),
verify_certificate_hash_(config.validation_context().verify_certificate_hash().empty()
Expand All @@ -74,30 +79,6 @@ ContextConfigImpl::ContextConfigImpl(const envoy::api::v2::auth::CommonTlsContex
}
}

const std::string ContextConfigImpl::readDataSource(const envoy::api::v2::core::DataSource& source,
bool allow_empty) {
switch (source.specifier_case()) {
case envoy::api::v2::core::DataSource::kFilename:
return Filesystem::fileReadToEnd(source.filename());
case envoy::api::v2::core::DataSource::kInlineBytes:
return source.inline_bytes();
case envoy::api::v2::core::DataSource::kInlineString:
return source.inline_string();
default:
if (!allow_empty) {
throw EnvoyException(
fmt::format("Unexpected DataSource::specifier_case(): {}", source.specifier_case()));
}
return "";
}
}

const std::string
ContextConfigImpl::getDataSourcePath(const envoy::api::v2::core::DataSource& source) {
return source.specifier_case() == envoy::api::v2::core::DataSource::kFilename ? source.filename()
: "";
}

unsigned ContextConfigImpl::tlsVersionFromProto(
const envoy::api::v2::auth::TlsParameters_TlsProtocol& version, unsigned default_version) {
switch (version) {
Expand Down Expand Up @@ -143,7 +124,7 @@ ServerContextConfigImpl::ServerContextConfigImpl(
switch (config.session_ticket_keys_type_case()) {
case envoy::api::v2::auth::DownstreamTlsContext::kSessionTicketKeys:
for (const auto& datasource : config.session_ticket_keys().keys()) {
validateAndAppendKey(ret, readDataSource(datasource, false));
validateAndAppendKey(ret, Config::DataSource(datasource).read(false));
}
break;
case envoy::api::v2::auth::DownstreamTlsContext::kSessionTicketKeysSdsSecretConfig:
Expand Down
4 changes: 0 additions & 4 deletions source/common/ssl/context_config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ class ContextConfigImpl : public virtual Ssl::ContextConfig {
protected:
ContextConfigImpl(const envoy::api::v2::auth::CommonTlsContext& config);

static const std::string readDataSource(const envoy::api::v2::core::DataSource& source,
bool allow_empty);
static const std::string getDataSourcePath(const envoy::api::v2::core::DataSource& source);

private:
static unsigned
tlsVersionFromProto(const envoy::api::v2::auth::TlsParameters_TlsProtocol& version,
Expand Down
4 changes: 4 additions & 0 deletions test/common/grpc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ envoy_cc_test_library(
envoy_cc_test(
name = "grpc_client_integration_test",
srcs = ["grpc_client_integration_test.cc"],
data = ["//test/config/integration/certs"],
deps = [
":grpc_client_integration_lib",
"//source/common/common:enum_to_int",
Expand All @@ -89,6 +90,9 @@ envoy_cc_test(
"//source/common/http/http2:conn_pool_lib",
"//source/common/network:connection_lib",
"//source/common/network:raw_buffer_socket_lib",
"//source/common/ssl:context_lib",
"//source/common/ssl:context_config_lib",
"//source/common/ssl:ssl_socket_lib",
"//source/common/stats:stats_lib",
"//test/integration:integration_lib",
"//test/mocks/grpc:grpc_mocks",
Expand Down
Loading