From e233de64c51bcfa16eabab7b1c854063f43d7719 Mon Sep 17 00:00:00 2001
From: Igor Peshansky <igorpeshansky@github.com>
Date: Tue, 3 Apr 2018 00:35:43 -0400
Subject: [PATCH 1/5] Add a unit test for credentials file handling.

---
 src/environment.h               |   2 +
 test/Makefile                   |   7 ++-
 test/environment_unittest.cc    | 102 ++++++++++++++++++++++++++++++++
 test/health_checker_unittest.cc |   1 -
 4 files changed, 109 insertions(+), 3 deletions(-)
 create mode 100644 test/environment_unittest.cc

diff --git a/src/environment.h b/src/environment.h
index 7f5d7206..aa356b61 100644
--- a/src/environment.h
+++ b/src/environment.h
@@ -44,6 +44,8 @@ class Environment {
   }
 
  private:
+  friend class EnvironmentTest;
+
   void ReadApplicationDefaultCredentials() const;
 
   const Configuration& config_;
diff --git a/test/Makefile b/test/Makefile
index 51bf89d3..7cdee451 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -42,6 +42,7 @@ TEST_OBJS=$(TEST_SOURCES:$(TEST_DIR)/%.cc=%.o)
 TESTS=\
       base64_unittest \
       configuration_unittest \
+      environment_unittest \
       format_unittest \
       health_checker_unittest \
       instance_unittest \
@@ -100,12 +101,14 @@ $(TESTS): $(GTEST_LIB) $(CPP_NETLIB_LIBS) $(YAML_CPP_LIBS)
 # All unittest objects depend on GTEST_LIB.
 $(TESTS:%=%.o): $(GTEST_LIB)
 
-format_unittest: format_unittest.o $(SRC_DIR)/format.o
-	$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
 base64_unittest: base64_unittest.o $(SRC_DIR)/base64.o
 	$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
 configuration_unittest: configuration_unittest.o $(SRC_DIR)/configuration.o
 	$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
+environment_unittest: environment_unittest.o $(SRC_DIR)/environment.o $(SRC_DIR)/configuration.o $(SRC_DIR)/format.o $(SRC_DIR)/json.o $(SRC_DIR)/logging.o $(SRC_DIR)/time.o
+	$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
+format_unittest: format_unittest.o $(SRC_DIR)/format.o
+	$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
 health_checker_unittest: health_checker_unittest.o $(SRC_DIR)/health_checker.o $(SRC_DIR)/configuration.o
 	$(CXX) $(LDFLAGS) $^ $(LDLIBS) -lboost_filesystem -lboost_system -o $@
 instance_unittest: instance_unittest.o $(SRC_DIR)/instance.o $(SRC_DIR)/configuration.o $(SRC_DIR)/environment.o $(SRC_DIR)/format.o $(SRC_DIR)/json.o $(SRC_DIR)/logging.o $(SRC_DIR)/resource.o $(SRC_DIR)/store.o $(SRC_DIR)/time.o
diff --git a/test/environment_unittest.cc b/test/environment_unittest.cc
new file mode 100644
index 00000000..0b3dc0b9
--- /dev/null
+++ b/test/environment_unittest.cc
@@ -0,0 +1,102 @@
+#include "../src/environment.h"
+#include "gtest/gtest.h"
+
+#include <fstream>
+#include <sstream>
+#include <boost/filesystem.hpp>
+
+namespace google {
+
+class EnvironmentTest : public ::testing::Test {
+ protected:
+  static void ReadApplicationDefaultCredentials(const Environment& environment) {
+    environment.ReadApplicationDefaultCredentials();
+  }
+};
+
+namespace {
+// A file with a given name in a temporary (unique) directory.
+boost::filesystem::path TempPath(const std::string& filename) {
+  boost::filesystem::path path = boost::filesystem::temp_directory_path();
+  path.append(boost::filesystem::unique_path().native());
+  path.append(filename);
+  return path;
+}
+
+// Creates a file for the lifetime of the object and removes it after.
+class TemporaryFile {
+ public:
+  TemporaryFile(const std::string& filename, const std::string& contents)
+      : path_(TempPath(filename)) {
+    boost::filesystem::create_directories(path_.parent_path());
+    SetContents(contents);
+  }
+  ~TemporaryFile() {
+    boost::filesystem::remove_all(path_.parent_path());
+  }
+  void SetContents(const std::string& contents) const {
+    std::ofstream file(path_.native());
+    file << contents << std::flush;
+  }
+  const std::string& FullPath() const { return path_.native(); }
+ private:
+  boost::filesystem::path path_;
+};
+}  // namespace
+
+TEST(TemporaryFile, Basic) {
+  std::string path;
+  {
+    TemporaryFile f("foo", "bar");
+    path = f.FullPath();
+    EXPECT_TRUE(boost::filesystem::exists(path));
+    std::string contents;
+    {
+      std::ifstream in(path);
+      in >> contents;
+    }
+    EXPECT_EQ("bar", contents);
+    f.SetContents("xyz");
+    {
+      std::ifstream in(path);
+      in >> contents;
+    }
+    EXPECT_EQ("xyz", contents);
+  }
+  EXPECT_FALSE(boost::filesystem::exists(path));
+}
+
+TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsSucceeds) {
+  TemporaryFile credentials_file(
+    std::string(test_info_->name()) + "_creds.json",
+    "{\"client_email\":\"foo@bar.com\",\"private_key\":\"12345\"}");
+  std::string cfg;
+  Configuration config(std::istringstream(
+      "CredentialsFile: '" + credentials_file.FullPath() + "'\n"
+  ));
+  Environment environment(config);
+  EXPECT_NO_THROW(ReadApplicationDefaultCredentials(environment));
+  EXPECT_EQ("foo@bar.com", environment.CredentialsClientEmail());
+  EXPECT_EQ("12345", environment.CredentialsPrivateKey());
+  &Configuration::CredentialsFile;  // TODO: without this the test crashes.
+}
+
+TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsCaches) {
+  TemporaryFile credentials_file(
+    std::string(test_info_->name()) + "_creds.json",
+    "{\"client_email\":\"foo@bar.com\",\"private_key\":\"12345\"}");
+  Configuration config(std::istringstream(
+      "CredentialsFile: '" + credentials_file.FullPath() + "'\n"
+  ));
+  Environment environment(config);
+  EXPECT_NO_THROW(ReadApplicationDefaultCredentials(environment));
+  credentials_file.SetContents(
+      "{\"client_email\":\"x@y.com\",\"private_key\":\"98765\"}"
+  );
+  EXPECT_EQ("foo@bar.com", environment.CredentialsClientEmail());
+  credentials_file.SetContents(
+      "{\"client_email\":\"a@b.com\",\"private_key\":\"09876\"}"
+  );
+  EXPECT_EQ("12345", environment.CredentialsPrivateKey());
+}
+}  // namespace google
diff --git a/test/health_checker_unittest.cc b/test/health_checker_unittest.cc
index a6df3b1e..d1d83bd8 100644
--- a/test/health_checker_unittest.cc
+++ b/test/health_checker_unittest.cc
@@ -1,6 +1,5 @@
 #include "../src/health_checker.h"
 #include "gtest/gtest.h"
-#include <stdio.h>
 #include <sstream>
 #include <boost/filesystem.hpp>
 

From ff39c988ba232625917063a42ed9ef36aab2956c Mon Sep 17 00:00:00 2001
From: Igor Peshansky <igorpeshansky@github.com>
Date: Tue, 3 Apr 2018 01:43:29 -0400
Subject: [PATCH 2/5] Fix valgrind issues.

---
 src/json.cc                  |  2 +-
 test/environment_unittest.cc | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/json.cc b/src/json.cc
index 1ad7b660..2876544b 100644
--- a/src/json.cc
+++ b/src/json.cc
@@ -513,7 +513,7 @@ Parser::~Parser() = default;
 
 std::size_t Parser::ParseStream(std::istream& stream) throw(Exception) {
   const int kMax = 65536;
-  unsigned char data[kMax];
+  unsigned char data[kMax] = {0};
   size_t total_bytes_consumed = 0;
   yajl_handle handle = state_->handle();
 
diff --git a/test/environment_unittest.cc b/test/environment_unittest.cc
index 0b3dc0b9..b3fdf70e 100644
--- a/test/environment_unittest.cc
+++ b/test/environment_unittest.cc
@@ -38,27 +38,27 @@ class TemporaryFile {
     std::ofstream file(path_.native());
     file << contents << std::flush;
   }
-  const std::string& FullPath() const { return path_.native(); }
+  const boost::filesystem::path& FullPath() const { return path_; }
  private:
   boost::filesystem::path path_;
 };
 }  // namespace
 
 TEST(TemporaryFile, Basic) {
-  std::string path;
+  boost::filesystem::path path;
   {
     TemporaryFile f("foo", "bar");
     path = f.FullPath();
     EXPECT_TRUE(boost::filesystem::exists(path));
     std::string contents;
     {
-      std::ifstream in(path);
+      std::ifstream in(path.native());
       in >> contents;
     }
     EXPECT_EQ("bar", contents);
     f.SetContents("xyz");
     {
-      std::ifstream in(path);
+      std::ifstream in(path.native());
       in >> contents;
     }
     EXPECT_EQ("xyz", contents);
@@ -72,13 +72,13 @@ TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsSucceeds) {
     "{\"client_email\":\"foo@bar.com\",\"private_key\":\"12345\"}");
   std::string cfg;
   Configuration config(std::istringstream(
-      "CredentialsFile: '" + credentials_file.FullPath() + "'\n"
+      "CredentialsFile: '" + credentials_file.FullPath().native() + "'\n"
   ));
   Environment environment(config);
   EXPECT_NO_THROW(ReadApplicationDefaultCredentials(environment));
   EXPECT_EQ("foo@bar.com", environment.CredentialsClientEmail());
   EXPECT_EQ("12345", environment.CredentialsPrivateKey());
-  &Configuration::CredentialsFile;  // TODO: without this the test crashes.
+  //&Configuration::CredentialsFile;  // TODO: without this the test crashes.
 }
 
 TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsCaches) {
@@ -86,7 +86,7 @@ TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsCaches) {
     std::string(test_info_->name()) + "_creds.json",
     "{\"client_email\":\"foo@bar.com\",\"private_key\":\"12345\"}");
   Configuration config(std::istringstream(
-      "CredentialsFile: '" + credentials_file.FullPath() + "'\n"
+      "CredentialsFile: '" + credentials_file.FullPath().native() + "'\n"
   ));
   Environment environment(config);
   EXPECT_NO_THROW(ReadApplicationDefaultCredentials(environment));

From 0ce5f42e181ada6e31086651bdf0953993f41782 Mon Sep 17 00:00:00 2001
From: Igor Peshansky <igorpeshansky@github.com>
Date: Tue, 3 Apr 2018 21:12:51 -0400
Subject: [PATCH 3/5] Instruct the compiler to not inline config accessors.

---
 src/Makefile                 |  5 ++--
 src/configuration.h          | 58 ++++++++++++++++++------------------
 test/Makefile                | 11 +++++++
 test/environment_unittest.cc |  1 -
 4 files changed, 43 insertions(+), 32 deletions(-)

diff --git a/src/Makefile b/src/Makefile
index 81dc663f..3336281e 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -19,6 +19,7 @@ CMAKE=cmake
 CPPFLAGS=\
     -DAGENT_VERSION='$(PKG_VERSION)-$(PKG_RELEASE)' \
     -DENABLE_DOCKER_METADATA -DENABLE_KUBERNETES_METADATA \
+    -DNOOPT= \
     -I$(CPP_NETLIB_DIR) -I$(NETWORK_URI_DIR)/include -I$(YAML_CPP_DIR)/include
 CXXFLAGS=\
     -std=c++11 -g -pthread -Wno-write-strings -Wno-deprecated
@@ -31,8 +32,8 @@ SED_EXTRA=-e 's/-Wall/-Wall -Wno-deprecated/'
 
 UNAME_S=$(shell uname -s)
 ifeq ($(UNAME_S),Darwin)
-  CXXFLAGS+= -Wno-deprecated-declarations -Wno-c++14-extensions \
-      -I/usr/local/include
+  CPPFLAGS+= -UNOOPT -DNOOPT='[[clang::optnone]]' -I/usr/local/include
+  CXXFLAGS+= -Wno-deprecated-declarations -Wno-c++14-extensions
   LDFLAGS+= -L/usr/local/lib
   SED_I+= ''
   SED_EXTRA+= -e \
diff --git a/src/configuration.h b/src/configuration.h
index c275b080..b4037bdd 100644
--- a/src/configuration.h
+++ b/src/configuration.h
@@ -31,126 +31,126 @@ class Configuration {
   Configuration(std::istream&& input) : Configuration(input) {}
 
   // Shared configuration.
-  const std::string& ProjectId() const {
+  NOOPT const std::string& ProjectId() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return project_id_;
   }
-  const std::string& CredentialsFile() const {
+  NOOPT const std::string& CredentialsFile() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return credentials_file_;
   }
-  bool VerboseLogging() const {
+  NOOPT bool VerboseLogging() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return verbose_logging_;
   }
   // Metadata API server configuration options.
-  int MetadataApiNumThreads() const {
+  NOOPT int MetadataApiNumThreads() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_api_num_threads_;
   }
-  int MetadataApiPort() const {
+  NOOPT int MetadataApiPort() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_api_port_;
   }
-  const std::string& MetadataApiResourceTypeSeparator() const {
+  NOOPT const std::string& MetadataApiResourceTypeSeparator() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_api_resource_type_separator_;
   }
   // Metadata reporter options.
-  int MetadataReporterIntervalSeconds() const {
+  NOOPT int MetadataReporterIntervalSeconds() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_reporter_interval_seconds_;
   }
-  bool MetadataReporterPurgeDeleted() const {
+  NOOPT bool MetadataReporterPurgeDeleted() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_reporter_purge_deleted_;
   }
-  const std::string& MetadataReporterUserAgent() const {
+  NOOPT const std::string& MetadataReporterUserAgent() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_reporter_user_agent_;
   }
-  const std::string& MetadataIngestionEndpointFormat() const {
+  NOOPT const std::string& MetadataIngestionEndpointFormat() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_ingestion_endpoint_format_;
   }
-  int MetadataIngestionRequestSizeLimitBytes() const {
+  NOOPT int MetadataIngestionRequestSizeLimitBytes() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_ingestion_request_size_limit_bytes_;
   }
-  const std::string& MetadataIngestionRawContentVersion() const {
+  NOOPT const std::string& MetadataIngestionRawContentVersion() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_ingestion_raw_content_version_;
   }
   // Instance metadata updater options.
-  int InstanceUpdaterIntervalSeconds() const {
+  NOOPT int InstanceUpdaterIntervalSeconds() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return instance_updater_interval_seconds_;
   }
-  const std::string& InstanceResourceType() const {
+  NOOPT const std::string& InstanceResourceType() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return instance_resource_type_;
   }
   // Docker metadata updater options.
-  int DockerUpdaterIntervalSeconds() const {
+  NOOPT int DockerUpdaterIntervalSeconds() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return docker_updater_interval_seconds_;
   }
-  const std::string& DockerEndpointHost() const {
+  NOOPT const std::string& DockerEndpointHost() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return docker_endpoint_host_;
   }
-  const std::string& DockerApiVersion() const {
+  NOOPT const std::string& DockerApiVersion() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return docker_api_version_;
   }
-  const std::string& DockerContainerFilter() const {
+  NOOPT const std::string& DockerContainerFilter() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return docker_container_filter_;
   }
   // GKE metadata updater options.
-  int KubernetesUpdaterIntervalSeconds() const {
+  NOOPT int KubernetesUpdaterIntervalSeconds() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_updater_interval_seconds_;
   }
-  const std::string& KubernetesEndpointHost() const {
+  NOOPT const std::string& KubernetesEndpointHost() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_endpoint_host_;
   }
-  const std::string& KubernetesPodLabelSelector() const {
+  NOOPT const std::string& KubernetesPodLabelSelector() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_pod_label_selector_;
   }
-  const std::string& KubernetesClusterName() const {
+  NOOPT const std::string& KubernetesClusterName() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_cluster_name_;
   }
-  const std::string& KubernetesClusterLocation() const {
+  NOOPT const std::string& KubernetesClusterLocation() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_cluster_location_;
   }
-  const std::string& KubernetesNodeName() const {
+  NOOPT const std::string& KubernetesNodeName() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_node_name_;
   }
-  bool KubernetesUseWatch() const {
+  NOOPT bool KubernetesUseWatch() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_use_watch_;
   }
-  bool KubernetesClusterLevelMetadata() const {
+  NOOPT bool KubernetesClusterLevelMetadata() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_cluster_level_metadata_;
   }
   // Common metadata updater options.
-  const std::string& InstanceId() const {
+  NOOPT const std::string& InstanceId() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return instance_id_;
   }
-  const std::string& InstanceZone() const {
+  NOOPT const std::string& InstanceZone() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return instance_zone_;
   }
 
-  const std::string& HealthCheckFile() const {
+  NOOPT const std::string& HealthCheckFile() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return health_check_file_;
   }
diff --git a/test/Makefile b/test/Makefile
index 7cdee451..2e3ab673 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -23,6 +23,7 @@ YAML_CPP_LIBS=$(YAML_CPP_LIBDIR)/libyaml-cpp.a
 
 CPPFLAGS+= \
     -DENABLE_KUBERNETES_METADATA \
+    -DNOOPT= \
     -isystem $(GTEST_DIR)/include -I$(GMOCK_DIR)/include \
     -I$(YAML_CPP_DIR)/include
 CXXFLAGS=\
@@ -33,6 +34,16 @@ LDLIBS=\
     -lboost_program_options -lboost_system -lboost_thread -lboost_filesystem \
     -lpthread -lyajl -lssl -lcrypto -lyaml-cpp
 
+UNAME_S=$(shell uname -s)
+ifeq ($(UNAME_S),Darwin)
+  CPPFLAGS+= -UNOOPT -DNOOPT='[[clang::optnone]]' -I/usr/local/include
+  CXXFLAGS+= -Wno-deprecated-declarations -Wno-c++14-extensions
+  LDFLAGS+= -L/usr/local/lib
+  SED_I+= ''
+  SED_EXTRA+= -e \
+      's/-Wall/-Wall -Wno-deprecated-declarations -Wno-unused-local-typedef/'
+endif
+
 # Where to find code under test.
 SRC_DIR=../src
 
diff --git a/test/environment_unittest.cc b/test/environment_unittest.cc
index b3fdf70e..1ae7de24 100644
--- a/test/environment_unittest.cc
+++ b/test/environment_unittest.cc
@@ -78,7 +78,6 @@ TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsSucceeds) {
   EXPECT_NO_THROW(ReadApplicationDefaultCredentials(environment));
   EXPECT_EQ("foo@bar.com", environment.CredentialsClientEmail());
   EXPECT_EQ("12345", environment.CredentialsPrivateKey());
-  //&Configuration::CredentialsFile;  // TODO: without this the test crashes.
 }
 
 TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsCaches) {

From 49a413899ad47998fe50768e38fb0e95f387f5f8 Mon Sep 17 00:00:00 2001
From: Igor Peshansky <igorpeshansky@github.com>
Date: Wed, 4 Apr 2018 23:37:25 -0400
Subject: [PATCH 4/5] Make some test values clearer.

---
 test/environment_unittest.cc | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/test/environment_unittest.cc b/test/environment_unittest.cc
index 1ae7de24..09ab5655 100644
--- a/test/environment_unittest.cc
+++ b/test/environment_unittest.cc
@@ -69,33 +69,33 @@ TEST(TemporaryFile, Basic) {
 TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsSucceeds) {
   TemporaryFile credentials_file(
     std::string(test_info_->name()) + "_creds.json",
-    "{\"client_email\":\"foo@bar.com\",\"private_key\":\"12345\"}");
+    "{\"client_email\":\"user@example.com\",\"private_key\":\"some_key\"}");
   std::string cfg;
   Configuration config(std::istringstream(
       "CredentialsFile: '" + credentials_file.FullPath().native() + "'\n"
   ));
   Environment environment(config);
   EXPECT_NO_THROW(ReadApplicationDefaultCredentials(environment));
-  EXPECT_EQ("foo@bar.com", environment.CredentialsClientEmail());
-  EXPECT_EQ("12345", environment.CredentialsPrivateKey());
+  EXPECT_EQ("user@example.com", environment.CredentialsClientEmail());
+  EXPECT_EQ("some_key", environment.CredentialsPrivateKey());
 }
 
 TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsCaches) {
   TemporaryFile credentials_file(
     std::string(test_info_->name()) + "_creds.json",
-    "{\"client_email\":\"foo@bar.com\",\"private_key\":\"12345\"}");
+    "{\"client_email\":\"user@example.com\",\"private_key\":\"some_key\"}");
   Configuration config(std::istringstream(
       "CredentialsFile: '" + credentials_file.FullPath().native() + "'\n"
   ));
   Environment environment(config);
   EXPECT_NO_THROW(ReadApplicationDefaultCredentials(environment));
   credentials_file.SetContents(
-      "{\"client_email\":\"x@y.com\",\"private_key\":\"98765\"}"
+      "{\"client_email\":\"changed@example.com\",\"private_key\":\"12345\"}"
   );
-  EXPECT_EQ("foo@bar.com", environment.CredentialsClientEmail());
+  EXPECT_EQ("user@example.com", environment.CredentialsClientEmail());
   credentials_file.SetContents(
-      "{\"client_email\":\"a@b.com\",\"private_key\":\"09876\"}"
+      "{\"client_email\":\"extra@example.com\",\"private_key\":\"09876\"}"
   );
-  EXPECT_EQ("12345", environment.CredentialsPrivateKey());
+  EXPECT_EQ("some_key", environment.CredentialsPrivateKey());
 }
 }  // namespace google

From da531f77954fa9994c2006d1d67bb89e87bdbcf0 Mon Sep 17 00:00:00 2001
From: Igor Peshansky <igorpeshansky@github.com>
Date: Tue, 10 Apr 2018 01:54:46 -0400
Subject: [PATCH 5/5] Remove superfluous directives.

---
 src/Makefile        |  3 +--
 src/configuration.h | 58 ++++++++++++++++++++++-----------------------
 test/Makefile       |  6 +----
 3 files changed, 31 insertions(+), 36 deletions(-)

diff --git a/src/Makefile b/src/Makefile
index 3336281e..8360733a 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -19,7 +19,6 @@ CMAKE=cmake
 CPPFLAGS=\
     -DAGENT_VERSION='$(PKG_VERSION)-$(PKG_RELEASE)' \
     -DENABLE_DOCKER_METADATA -DENABLE_KUBERNETES_METADATA \
-    -DNOOPT= \
     -I$(CPP_NETLIB_DIR) -I$(NETWORK_URI_DIR)/include -I$(YAML_CPP_DIR)/include
 CXXFLAGS=\
     -std=c++11 -g -pthread -Wno-write-strings -Wno-deprecated
@@ -32,7 +31,7 @@ SED_EXTRA=-e 's/-Wall/-Wall -Wno-deprecated/'
 
 UNAME_S=$(shell uname -s)
 ifeq ($(UNAME_S),Darwin)
-  CPPFLAGS+= -UNOOPT -DNOOPT='[[clang::optnone]]' -I/usr/local/include
+  CPPFLAGS+= -I/usr/local/include
   CXXFLAGS+= -Wno-deprecated-declarations -Wno-c++14-extensions
   LDFLAGS+= -L/usr/local/lib
   SED_I+= ''
diff --git a/src/configuration.h b/src/configuration.h
index b4037bdd..c275b080 100644
--- a/src/configuration.h
+++ b/src/configuration.h
@@ -31,126 +31,126 @@ class Configuration {
   Configuration(std::istream&& input) : Configuration(input) {}
 
   // Shared configuration.
-  NOOPT const std::string& ProjectId() const {
+  const std::string& ProjectId() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return project_id_;
   }
-  NOOPT const std::string& CredentialsFile() const {
+  const std::string& CredentialsFile() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return credentials_file_;
   }
-  NOOPT bool VerboseLogging() const {
+  bool VerboseLogging() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return verbose_logging_;
   }
   // Metadata API server configuration options.
-  NOOPT int MetadataApiNumThreads() const {
+  int MetadataApiNumThreads() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_api_num_threads_;
   }
-  NOOPT int MetadataApiPort() const {
+  int MetadataApiPort() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_api_port_;
   }
-  NOOPT const std::string& MetadataApiResourceTypeSeparator() const {
+  const std::string& MetadataApiResourceTypeSeparator() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_api_resource_type_separator_;
   }
   // Metadata reporter options.
-  NOOPT int MetadataReporterIntervalSeconds() const {
+  int MetadataReporterIntervalSeconds() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_reporter_interval_seconds_;
   }
-  NOOPT bool MetadataReporterPurgeDeleted() const {
+  bool MetadataReporterPurgeDeleted() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_reporter_purge_deleted_;
   }
-  NOOPT const std::string& MetadataReporterUserAgent() const {
+  const std::string& MetadataReporterUserAgent() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_reporter_user_agent_;
   }
-  NOOPT const std::string& MetadataIngestionEndpointFormat() const {
+  const std::string& MetadataIngestionEndpointFormat() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_ingestion_endpoint_format_;
   }
-  NOOPT int MetadataIngestionRequestSizeLimitBytes() const {
+  int MetadataIngestionRequestSizeLimitBytes() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_ingestion_request_size_limit_bytes_;
   }
-  NOOPT const std::string& MetadataIngestionRawContentVersion() const {
+  const std::string& MetadataIngestionRawContentVersion() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return metadata_ingestion_raw_content_version_;
   }
   // Instance metadata updater options.
-  NOOPT int InstanceUpdaterIntervalSeconds() const {
+  int InstanceUpdaterIntervalSeconds() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return instance_updater_interval_seconds_;
   }
-  NOOPT const std::string& InstanceResourceType() const {
+  const std::string& InstanceResourceType() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return instance_resource_type_;
   }
   // Docker metadata updater options.
-  NOOPT int DockerUpdaterIntervalSeconds() const {
+  int DockerUpdaterIntervalSeconds() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return docker_updater_interval_seconds_;
   }
-  NOOPT const std::string& DockerEndpointHost() const {
+  const std::string& DockerEndpointHost() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return docker_endpoint_host_;
   }
-  NOOPT const std::string& DockerApiVersion() const {
+  const std::string& DockerApiVersion() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return docker_api_version_;
   }
-  NOOPT const std::string& DockerContainerFilter() const {
+  const std::string& DockerContainerFilter() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return docker_container_filter_;
   }
   // GKE metadata updater options.
-  NOOPT int KubernetesUpdaterIntervalSeconds() const {
+  int KubernetesUpdaterIntervalSeconds() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_updater_interval_seconds_;
   }
-  NOOPT const std::string& KubernetesEndpointHost() const {
+  const std::string& KubernetesEndpointHost() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_endpoint_host_;
   }
-  NOOPT const std::string& KubernetesPodLabelSelector() const {
+  const std::string& KubernetesPodLabelSelector() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_pod_label_selector_;
   }
-  NOOPT const std::string& KubernetesClusterName() const {
+  const std::string& KubernetesClusterName() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_cluster_name_;
   }
-  NOOPT const std::string& KubernetesClusterLocation() const {
+  const std::string& KubernetesClusterLocation() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_cluster_location_;
   }
-  NOOPT const std::string& KubernetesNodeName() const {
+  const std::string& KubernetesNodeName() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_node_name_;
   }
-  NOOPT bool KubernetesUseWatch() const {
+  bool KubernetesUseWatch() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_use_watch_;
   }
-  NOOPT bool KubernetesClusterLevelMetadata() const {
+  bool KubernetesClusterLevelMetadata() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return kubernetes_cluster_level_metadata_;
   }
   // Common metadata updater options.
-  NOOPT const std::string& InstanceId() const {
+  const std::string& InstanceId() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return instance_id_;
   }
-  NOOPT const std::string& InstanceZone() const {
+  const std::string& InstanceZone() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return instance_zone_;
   }
 
-  NOOPT const std::string& HealthCheckFile() const {
+  const std::string& HealthCheckFile() const {
     std::lock_guard<std::mutex> lock(mutex_);
     return health_check_file_;
   }
diff --git a/test/Makefile b/test/Makefile
index 2e3ab673..e46b57ab 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -23,7 +23,6 @@ YAML_CPP_LIBS=$(YAML_CPP_LIBDIR)/libyaml-cpp.a
 
 CPPFLAGS+= \
     -DENABLE_KUBERNETES_METADATA \
-    -DNOOPT= \
     -isystem $(GTEST_DIR)/include -I$(GMOCK_DIR)/include \
     -I$(YAML_CPP_DIR)/include
 CXXFLAGS=\
@@ -36,12 +35,9 @@ LDLIBS=\
 
 UNAME_S=$(shell uname -s)
 ifeq ($(UNAME_S),Darwin)
-  CPPFLAGS+= -UNOOPT -DNOOPT='[[clang::optnone]]' -I/usr/local/include
+  CPPFLAGS+= -I/usr/local/include
   CXXFLAGS+= -Wno-deprecated-declarations -Wno-c++14-extensions
   LDFLAGS+= -L/usr/local/lib
-  SED_I+= ''
-  SED_EXTRA+= -e \
-      's/-Wall/-Wall -Wno-deprecated-declarations -Wno-unused-local-typedef/'
 endif
 
 # Where to find code under test.