From d46b70e941aff4b640acc1a642636160bbc223ca Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Mon, 19 Mar 2018 15:21:50 -0400 Subject: [PATCH 1/4] Add a User-Agent header to metadata API requests; allow overriding via config. --- src/Makefile | 13 ++++++++----- src/api_server.cc | 8 ++++++-- src/configuration.cc | 20 ++++++++++++++++++++ src/configuration.h | 5 +++++ 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/Makefile b/src/Makefile index c2e27294..9ab5fd2a 100644 --- a/src/Makefile +++ b/src/Makefile @@ -1,3 +1,7 @@ +PKG_VERSION=0.0.17 +PKG_RELEASE=1 +DOCKER_VERSION=0.2 + LIBDIR=../lib CPP_NETLIB_DIR=$(LIBDIR)/cpp-netlib CPP_NETLIB_LIBDIR=$(CPP_NETLIB_DIR)/libs/network/src @@ -9,10 +13,12 @@ SUBMODULE_DIRS=$(CPP_NETLIB_DIR) $(YAML_CPP_DIR) SED_I=/usr/bin/env sed -i CMAKE=cmake -CXXFLAGS=\ - -std=c++11 -g -Wno-write-strings -Wno-deprecated \ +CPPFLAGS=\ + -DAGENT_VERSION='$(PKG_VERSION)-$(PKG_RELEASE)' \ -DENABLE_DOCKER_METADATA -DENABLE_KUBERNETES_METADATA \ -I$(CPP_NETLIB_DIR) -I$(NETWORK_URI_DIR)/include -I$(YAML_CPP_DIR)/include +CXXFLAGS=\ + -std=c++11 -g -Wno-write-strings -Wno-deprecated LDFLAGS=-L$(CPP_NETLIB_LIBDIR) -L$(NETWORK_URI_LIBDIR) -L$(YAML_CPP_LIBDIR) LDLIBS=\ -lcppnetlib-client-connections -lcppnetlib-server-parsers -lnetwork-uri \ @@ -77,11 +83,8 @@ install: metadatad export DISTRO PKG_NAME=stackdriver-metadata -PKG_VERSION=0.0.17 -PKG_RELEASE=1 PKG_MAINTAINER=Stackdriver Agents -DOCKER_VERSION=0.2 DOCKER_IMAGE=us.gcr.io/container-monitoring-storage/stackdriver-metadata-agent DOCKER_TAG=$(DOCKER_VERSION)-$(PKG_VERSION)-$(PKG_RELEASE) diff --git a/src/api_server.cc b/src/api_server.cc index a0051026..c7b29dd3 100644 --- a/src/api_server.cc +++ b/src/api_server.cc @@ -192,6 +192,7 @@ namespace { void SendMetadataRequest(std::vector&& entries, const std::string& endpoint, const std::string& auth_header, + const std::string& user_agent, bool verbose_logging) throw (boost::system::system_error) { json::value update_metadata_request = json::object({ @@ -200,12 +201,14 @@ void SendMetadataRequest(std::vector&& entries, if (verbose_logging) { LOG(INFO) << "About to send request: POST " << endpoint + << " User-Agent: " << user_agent << " " << *update_metadata_request; } http::client client; http::client::request request(endpoint); std::string request_body = update_metadata_request->ToString(); + request << boost::network::header("User-Agent", user_agent); request << boost::network::header("Content-Length", std::to_string(request_body.size())); request << boost::network::header("Content-Type", "application/json"); @@ -247,6 +250,7 @@ void MetadataReporter::SendMetadata( format::Substitute(agent_->config_.MetadataIngestionEndpointFormat(), {{"project_id", project_id}}); const std::string auth_header = auth_.GetAuthHeaderValue(); + const std::string user_agent = agent_->config_.MetadataReporterUserAgent(); const json::value empty_request = json::object({ {"entries", json::array({})}, @@ -282,7 +286,7 @@ void MetadataReporter::SendMetadata( continue; } if (total_size + size > limit_bytes) { - SendMetadataRequest(std::move(entries), endpoint, auth_header, + SendMetadataRequest(std::move(entries), endpoint, auth_header, user_agent, agent_->config_.VerboseLogging()); entries.clear(); total_size = empty_size; @@ -291,7 +295,7 @@ void MetadataReporter::SendMetadata( total_size += size; } if (!entries.empty()) { - SendMetadataRequest(std::move(entries), endpoint, auth_header, + SendMetadataRequest(std::move(entries), endpoint, auth_header, user_agent, agent_->config_.VerboseLogging()); } } diff --git a/src/configuration.cc b/src/configuration.cc index 796b6020..563f1a72 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -26,6 +26,13 @@ namespace google { namespace { +#ifndef AGENT_VERSION +#define AGENT_VERSION 0.0 +#endif + +#define STRINGIFY_H(x) #x +#define STRINGIFY(x) STRINGIFY_H(x) + constexpr const char kConfigFileFlag[] = "config-file"; constexpr const char kDefaultProjectId[] = ""; @@ -35,6 +42,8 @@ constexpr const int kMetadataApiDefaultPort = 8000; constexpr const char kMetadataApiDefaultResourceTypeSeparator[] = "."; constexpr const int kMetadataReporterDefaultIntervalSeconds = 60; constexpr const int kMetadataReporterDefaultPurgeDeleted = false; +constexpr const char kMetadataReporterDefaultUserAgent[] = + "metadata-agent/" STRINGIFY(AGENT_VERSION); constexpr const char kMetadataIngestionDefaultEndpointFormat[] = "https://stackdriver.googleapis.com/v1beta2/projects/{{project_id}}" "/resourceMetadata:batchUpdate"; @@ -60,6 +69,8 @@ constexpr const bool kKubernetesDefaultUseWatch = true; constexpr const char kDefaultInstanceId[] = ""; constexpr const char kDefaultInstanceZone[] = ""; +#undef STRINGIFY +#undef STRINGIFY_H } MetadataAgentConfiguration::MetadataAgentConfiguration() @@ -74,6 +85,8 @@ MetadataAgentConfiguration::MetadataAgentConfiguration() kMetadataReporterDefaultIntervalSeconds), metadata_reporter_purge_deleted_( kMetadataReporterDefaultPurgeDeleted), + metadata_reporter_user_agent_( + kMetadataReporterDefaultUserAgent), metadata_ingestion_endpoint_format_( kMetadataIngestionDefaultEndpointFormat), metadata_ingestion_request_size_limit_bytes_( @@ -124,6 +137,10 @@ int MetadataAgentConfiguration::ParseArguments(int ac, char** av) { boost::program_options::notify(flags); if (flags.count("help")) { + std::cout << flags_desc << std::endl; + return 0; + } + if (flags.count("version")) { std::cout << flags_desc << std::endl; return 1; } @@ -159,6 +176,9 @@ void MetadataAgentConfiguration::ParseConfiguration(std::istream& input) { metadata_reporter_purge_deleted_ = config["MetadataReporterPurgeDeleted"].as( kMetadataReporterDefaultPurgeDeleted); + metadata_reporter_user_agent_ = + config["MetadataReporterUserAgent"].as( + kMetadataReporterDefaultUserAgent); metadata_ingestion_endpoint_format_ = config["MetadataIngestionEndpointFormat"].as( kMetadataIngestionDefaultEndpointFormat); diff --git a/src/configuration.h b/src/configuration.h index cc4dbb94..06b20714 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -61,6 +61,10 @@ class MetadataAgentConfiguration { std::lock_guard lock(mutex_); return metadata_reporter_purge_deleted_; } + const std::string& MetadataReporterUserAgent() const { + std::lock_guard lock(mutex_); + return metadata_reporter_user_agent_; + } const std::string& MetadataIngestionEndpointFormat() const { std::lock_guard lock(mutex_); return metadata_ingestion_endpoint_format_; @@ -153,6 +157,7 @@ class MetadataAgentConfiguration { std::string metadata_api_resource_type_separator_; int metadata_reporter_interval_seconds_; bool metadata_reporter_purge_deleted_; + std::string metadata_reporter_user_agent_; std::string metadata_ingestion_endpoint_format_; int metadata_ingestion_request_size_limit_bytes_; std::string metadata_ingestion_raw_content_version_; From b934da30ff651b7bf7e930871853188678229278 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Mon, 19 Mar 2018 15:41:37 -0400 Subject: [PATCH 2/4] Update unit test. --- test/Makefile | 5 ++++- test/configuration_unittest.cc | 11 ++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/test/Makefile b/test/Makefile index 84460837..0ce40f18 100644 --- a/test/Makefile +++ b/test/Makefile @@ -5,6 +5,7 @@ GTEST_SOURCEDIR=$(GTEST_DIR)/src GTEST_HEADERS=$(GTEST_DIR)/include/gtest/*.h \ $(GTEST_DIR)/include/gtest/internal/*.h GTEST_SRCS_=$(GTEST_SOURCEDIR)/*.cc $(GTEST_SOURCEDIR)/*.h $(GTEST_HEADERS) +GMOCK_DIR=$(LIBDIR)/googletest/googlemock # TODO: Factor out the common variables. CPP_NETLIB_DIR=$(LIBDIR)/cpp-netlib @@ -13,7 +14,9 @@ YAML_CPP_LIBDIR=$(YAML_CPP_DIR) YAML_CPP_LIBS=$(YAML_CPP_LIBDIR)/libyaml-cpp.a -CPPFLAGS+=-isystem $(GTEST_DIR)/include -I$(YAML_CPP_DIR)/include +CPPFLAGS+= \ + -isystem $(GTEST_DIR)/include -I$(GMOCK_DIR)/include \ + -I$(YAML_CPP_DIR)/include CXXFLAGS=-std=c++11 -g -pthread LDLIBS=-lpthread -lboost_program_options -lyaml-cpp LDFLAGS=-L$(YAML_CPP_LIBDIR) diff --git a/test/configuration_unittest.cc b/test/configuration_unittest.cc index 3432abdf..825e2d56 100644 --- a/test/configuration_unittest.cc +++ b/test/configuration_unittest.cc @@ -1,5 +1,6 @@ #include "../src/configuration.h" #include "gtest/gtest.h" +#include "gmock/gmock.h" namespace google { @@ -18,6 +19,8 @@ class MetadataAgentConfigurationTest : public ::testing::Test { EXPECT_EQ(".", config.MetadataApiResourceTypeSeparator()); EXPECT_EQ(60, config.MetadataReporterIntervalSeconds()); EXPECT_EQ(false, config.MetadataReporterPurgeDeleted()); + EXPECT_THAT(config.MetadataReporterUserAgent(), + ::testing::StartsWith("metadata-agent/")); EXPECT_EQ("https://stackdriver.googleapis.com/" "v1beta2/projects/{{project_id}}/resourceMetadata:batchUpdate", config.MetadataIngestionEndpointFormat()); @@ -58,18 +61,20 @@ TEST_F(MetadataAgentConfigurationTest, PopulatedConfig) { ParseConfiguration( "ProjectId: TestProjectId\n" "MetadataApiNumThreads: 13\n" - "MetadataReporterPurgeDeleted: true" + "MetadataReporterPurgeDeleted: true\n" + "MetadataReporterUserAgent: \"foobar/foobaz\"\n" ); EXPECT_EQ("TestProjectId", config.ProjectId()); EXPECT_EQ(13, config.MetadataApiNumThreads()); EXPECT_EQ(true, config.MetadataReporterPurgeDeleted()); + EXPECT_EQ("foobar/foobaz", config.MetadataReporterUserAgent()); } TEST_F(MetadataAgentConfigurationTest, CommentSkipped) { ParseConfiguration( "ProjectId: TestProjectId\n" "#MetadataApiNumThreads: 13\n" - "MetadataReporterPurgeDeleted: true" + "MetadataReporterPurgeDeleted: true\n" ); EXPECT_EQ(3, config.MetadataApiNumThreads()); } @@ -79,7 +84,7 @@ TEST_F(MetadataAgentConfigurationTest, BlankLine) { "ProjectId: TestProjectId\n" "\n" "\n" - "MetadataReporterPurgeDeleted: true" + "MetadataReporterPurgeDeleted: true\n" ); EXPECT_EQ("TestProjectId", config.ProjectId()); EXPECT_EQ(true, config.MetadataReporterPurgeDeleted()); From 6b41852b5bac11a47bcb4557b29b40b70cf823d9 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Mon, 19 Mar 2018 20:00:22 -0400 Subject: [PATCH 3/4] Reference GCC CPP docs for stringification. --- src/configuration.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/configuration.cc b/src/configuration.cc index 563f1a72..2a2b72ee 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -30,6 +30,7 @@ namespace { #define AGENT_VERSION 0.0 #endif +// https://gcc.gnu.org/onlinedocs/gcc-7.3.0/cpp/Stringizing.html #define STRINGIFY_H(x) #x #define STRINGIFY(x) STRINGIFY_H(x) From 16b7ae4f09c642d5ed8105c28750f0e3b0c5ab40 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Mon, 19 Mar 2018 22:07:35 -0400 Subject: [PATCH 4/4] Print version correctly. Handle argument parse errors. Fix ParseArguments return value bug. --- src/configuration.cc | 46 ++++++++++++++++++++++++++------------------ src/configuration.h | 5 +++++ src/metadatad.cc | 2 +- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/configuration.cc b/src/configuration.cc index 2a2b72ee..872b26cc 100644 --- a/src/configuration.cc +++ b/src/configuration.cc @@ -23,9 +23,6 @@ #include -namespace google { - -namespace { #ifndef AGENT_VERSION #define AGENT_VERSION 0.0 #endif @@ -34,6 +31,9 @@ namespace { #define STRINGIFY_H(x) #x #define STRINGIFY(x) STRINGIFY_H(x) +namespace google { + +namespace { constexpr const char kConfigFileFlag[] = "config-file"; constexpr const char kDefaultProjectId[] = ""; @@ -69,9 +69,6 @@ constexpr const char kKubernetesDefaultNodeName[] = ""; constexpr const bool kKubernetesDefaultUseWatch = true; constexpr const char kDefaultInstanceId[] = ""; constexpr const char kDefaultInstanceZone[] = ""; - -#undef STRINGIFY -#undef STRINGIFY_H } MetadataAgentConfiguration::MetadataAgentConfiguration() @@ -117,6 +114,7 @@ int MetadataAgentConfiguration::ParseArguments(int ac, char** av) { boost::program_options::options_description flags_desc; flags_desc.add_options() ("help,h", "Print help message") + ("version,V", "Print the agent version") ("verbose,v", boost::program_options::bool_switch(&verbose_logging_), "Enable verbose logging") ; @@ -132,22 +130,29 @@ int MetadataAgentConfiguration::ParseArguments(int ac, char** av) { boost::program_options::positional_options_description positional_desc; positional_desc.add(kConfigFileFlag, 1); boost::program_options::variables_map flags; - boost::program_options::store( - boost::program_options::command_line_parser(ac, av) - .options(all_desc).positional(positional_desc).run(), flags); - boost::program_options::notify(flags); - - if (flags.count("help")) { - std::cout << flags_desc << std::endl; + try { + boost::program_options::store( + boost::program_options::command_line_parser(ac, av) + .options(all_desc).positional(positional_desc).run(), flags); + boost::program_options::notify(flags); + + if (flags.count("help")) { + std::cout << flags_desc << std::endl; + return -1; + } + if (flags.count("version")) { + std::cout << "Stackdriver Metadata Agent v" << STRINGIFY(AGENT_VERSION) + << std::endl; + return -1; + } + + ParseConfigFile(config_file); return 0; - } - if (flags.count("version")) { - std::cout << flags_desc << std::endl; + } catch (const boost::program_options::error& arg_error) { + std::cerr << arg_error.what() << std::endl; + std::cerr << flags_desc << std::endl; return 1; } - - ParseConfigFile(config_file); - return 0; } void MetadataAgentConfiguration::ParseConfigFile(const std::string& filename) { @@ -232,3 +237,6 @@ void MetadataAgentConfiguration::ParseConfiguration(std::istream& input) { } } // google + +#undef STRINGIFY +#undef STRINGIFY_H diff --git a/src/configuration.h b/src/configuration.h index 06b20714..cb533a02 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -24,6 +24,11 @@ namespace google { class MetadataAgentConfiguration { public: MetadataAgentConfiguration(); + // Parse the command line. + // A zero return value means that parsing succeeded and the program should + // proceed. A positive return value means that parsing failed. A negative + // value means that parsing succeeded, but all of the arguments were handled + // within the function and the program should exit with a success exit code. int ParseArguments(int ac, char** av); // Shared configuration. diff --git a/src/metadatad.cc b/src/metadatad.cc index 57d78ac6..8aea1bec 100644 --- a/src/metadatad.cc +++ b/src/metadatad.cc @@ -28,7 +28,7 @@ int main(int ac, char** av) { google::MetadataAgentConfiguration config; int parse_result = config.ParseArguments(ac, av); if (parse_result) { - return parse_result; + return parse_result < 0 ? 0 : parse_result; } google::MetadataAgent server(config);