Repository: mesos Updated Branches: refs/heads/master 85b09e22b -> 38dbadc94
Changed secret field in `Credential` from `bytes` to `string` When decoding the JSON credential file into the `Credential` protobuf object, the `secret` which is in `bytes` is mapped into base64 string automatically by protobuf from JSON. This leads to unintended behavior, forcing users to encode in base64 their secret when wanting to pass a JSON file to the --credentials flag. Review: https://reviews.apache.org/r/39098 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/38dbadc9 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/38dbadc9 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/38dbadc9 Branch: refs/heads/master Commit: 38dbadc944f56e24725fba102bbd2db76cb31228 Parents: 85b09e2 Author: Isabel Jimenez <[email protected]> Authored: Tue Oct 13 10:47:24 2015 +0200 Committer: Michael Park <[email protected]> Committed: Tue Oct 13 10:55:41 2015 +0200 ---------------------------------------------------------------------- include/mesos/mesos.proto | 2 +- src/examples/java/TestExceptionFramework.java | 2 +- src/examples/java/TestFramework.java | 2 +- src/tests/credentials_tests.cpp | 72 +++++++++++++++++++--- 4 files changed, 66 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/38dbadc9/include/mesos/mesos.proto ---------------------------------------------------------------------- diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto index 2f774d1..f2ea4fc 100644 --- a/include/mesos/mesos.proto +++ b/include/mesos/mesos.proto @@ -1226,7 +1226,7 @@ message Parameters { */ message Credential { required string principal = 1; - optional bytes secret = 2; + optional string secret = 2; } http://git-wip-us.apache.org/repos/asf/mesos/blob/38dbadc9/src/examples/java/TestExceptionFramework.java ---------------------------------------------------------------------- diff --git a/src/examples/java/TestExceptionFramework.java b/src/examples/java/TestExceptionFramework.java index 78720b0..12bf603 100644 --- a/src/examples/java/TestExceptionFramework.java +++ b/src/examples/java/TestExceptionFramework.java @@ -100,7 +100,7 @@ public class TestExceptionFramework { Credential credential = Credential.newBuilder() .setPrincipal(System.getenv("DEFAULT_PRINCIPAL")) - .setSecret(ByteString.copyFrom(System.getenv("DEFAULT_SECRET").getBytes())) + .setSecret(System.getenv("DEFAULT_SECRET")) .build(); frameworkBuilder.setPrincipal(System.getenv("DEFAULT_PRINCIPAL")); http://git-wip-us.apache.org/repos/asf/mesos/blob/38dbadc9/src/examples/java/TestFramework.java ---------------------------------------------------------------------- diff --git a/src/examples/java/TestFramework.java b/src/examples/java/TestFramework.java index aad94c0..55c6ee6 100644 --- a/src/examples/java/TestFramework.java +++ b/src/examples/java/TestFramework.java @@ -246,7 +246,7 @@ public class TestFramework { .setPrincipal(System.getenv("DEFAULT_PRINCIPAL")); if (System.getenv("DEFAULT_SECRET") != null) { - credentialBuilder.setSecret(ByteString.copyFrom(System.getenv("DEFAULT_SECRET").getBytes())); + credentialBuilder.setSecret(System.getenv("DEFAULT_SECRET")); } frameworkBuilder.setPrincipal(System.getenv("DEFAULT_PRINCIPAL")); http://git-wip-us.apache.org/repos/asf/mesos/blob/38dbadc9/src/tests/credentials_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/credentials_tests.cpp b/src/tests/credentials_tests.cpp index ced27c4..9d9de81 100644 --- a/src/tests/credentials_tests.cpp +++ b/src/tests/credentials_tests.cpp @@ -52,13 +52,13 @@ class CredentialsTest : public MesosTest {}; // granted registration by the master. TEST_F(CredentialsTest, AuthenticatedSlave) { - Try<PID<Master> > master = StartMaster(); + Try<PID<Master>> master = StartMaster(); ASSERT_SOME(master); Future<SlaveRegisteredMessage> slaveRegisteredMessage = FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _); - Try<PID<Slave> > slave = StartSlave(); + Try<PID<Slave>> slave = StartSlave(); ASSERT_SOME(slave); AWAIT_READY(slaveRegisteredMessage); @@ -73,9 +73,7 @@ TEST_F(CredentialsTest, AuthenticatedSlave) // backwards compatibility. TEST_F(CredentialsTest, AuthenticatedSlaveText) { - master::Flags flags = CreateMasterFlags(); - - const string& path = path::join(os::getcwd(), "credentials"); + string path = path::join(os::getcwd(), "credentials"); Try<int> fd = os::open( path, @@ -84,24 +82,80 @@ TEST_F(CredentialsTest, AuthenticatedSlaveText) CHECK_SOME(fd); - const std::string& credentials = + std::string credentials = DEFAULT_CREDENTIAL.principal() + " " + DEFAULT_CREDENTIAL.secret(); + CHECK_SOME(os::write(fd.get(), credentials)) - << "Failed to write credentials to '" << path << "'"; + << "Failed to write credentials to '" << path << "'"; + CHECK_SOME(os::close(fd.get())); map<string, Option<string>> values{{"credentials", Some("file://" + path)}}; - flags.load(values, true); + master::Flags masterFlags = CreateMasterFlags(); + masterFlags.load(values, true); - Try<PID<Master>> master = StartMaster(flags); + Try<PID<Master>> master = StartMaster(masterFlags); ASSERT_SOME(master); Future<SlaveRegisteredMessage> slaveRegisteredMessage = FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _); slave::Flags slaveFlags = CreateSlaveFlags(); + slaveFlags.load(values, true); + + Try<PID<Slave>> slave = StartSlave(slaveFlags); + ASSERT_SOME(slave); + + AWAIT_READY(slaveRegisteredMessage); + ASSERT_NE("", slaveRegisteredMessage.get().slave_id().value()); + + Shutdown(); +} + + +// Using JSON base file for authentication without +// protobuf tools assistance. +TEST_F(CredentialsTest, AuthenticatedSlaveJSON) +{ + string path = path::join(os::getcwd(), "credentials"); + + Try<int> fd = os::open( + path, + O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, + S_IRUSR | S_IWUSR | S_IRGRP); + + CHECK_SOME(fd); + + // This unit tests our capacity to process JSON credentials without + // using our protobuf tools. + JSON::Object credential; + credential.values["principal"] = DEFAULT_CREDENTIAL.principal(); + credential.values["secret"] = DEFAULT_CREDENTIAL.secret(); + + JSON::Array array; + array.values.push_back(credential); + + JSON::Object credentials; + credentials.values["credentials"] = array; + + CHECK_SOME(os::write(fd.get(), stringify(credentials))) + << "Failed to write credentials to '" << path << "'"; + + CHECK_SOME(os::close(fd.get())); + map<string, Option<string>> values{{"credentials", Some("file://" + path)}}; + + master::Flags masterFlags = CreateMasterFlags(); + masterFlags.load(values, true); + + Try<PID<Master>> master = StartMaster(masterFlags); + ASSERT_SOME(master); + + Future<SlaveRegisteredMessage> slaveRegisteredMessage = + FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _); + + slave::Flags slaveFlags = CreateSlaveFlags(); slaveFlags.load(values, true); Try<PID<Slave>> slave = StartSlave(slaveFlags);
