PolicyManager: New Config provider.
This patch adds a new Config provider to the Policy Manager. This
provider reads a fixed set of config options from a file on /etc.
On dev mode, if the file is present on the stateful partition, it
will read it from there instead.
Currently, the only enabled setting consists on the OOBE enabled bit.
This setting states whether the board/device has an OOBE or not.
BUG=chromium:359674
TEST=Unit tests added and passing.
Change-Id: I86c64ab4c447409ef7aeccf0ed374906edf450e7
Reviewed-on: https://chromium-review.googlesource.com/195542
Tested-by: Alex Deymo <deymo@chromium.org>
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
diff --git a/policy_manager/config_provider.h b/policy_manager/config_provider.h
new file mode 100644
index 0000000..4741b33
--- /dev/null
+++ b/policy_manager/config_provider.h
@@ -0,0 +1,33 @@
+// Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_CONFIG_PROVIDER_H_
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_CONFIG_PROVIDER_H_
+
+#include <base/memory/scoped_ptr.h>
+
+#include "update_engine/policy_manager/provider.h"
+#include "update_engine/policy_manager/variable.h"
+
+namespace chromeos_policy_manager {
+
+// Provider for const system configurations. This provider reads the
+// configuration from a file on /etc.
+class ConfigProvider : public Provider {
+ public:
+ // Returns a variable stating whether the out of the box experience (OOBE) is
+ // enabled on this device. A value of false means that the device doesn't have
+ // an OOBE workflow.
+ virtual Variable<bool>* var_is_oobe_enabled() = 0;
+
+ protected:
+ ConfigProvider() {}
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(ConfigProvider);
+};
+
+} // namespace chromeos_policy_manager
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_CONFIG_PROVIDER_H_
diff --git a/policy_manager/fake_config_provider.h b/policy_manager/fake_config_provider.h
new file mode 100644
index 0000000..3a31eb3
--- /dev/null
+++ b/policy_manager/fake_config_provider.h
@@ -0,0 +1,34 @@
+// Copyright 2014 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_FAKE_CONFIG_PROVIDER_H_
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_FAKE_CONFIG_PROVIDER_H_
+
+#include "update_engine/policy_manager/config_provider.h"
+#include "update_engine/policy_manager/fake_variable.h"
+
+namespace chromeos_policy_manager {
+
+// Fake implementation of the ConfigProvider base class.
+class FakeConfigProvider : public ConfigProvider {
+ public:
+ FakeConfigProvider() {}
+
+ protected:
+ virtual bool DoInit() override { return true; }
+
+ virtual FakeVariable<bool>* var_is_oobe_enabled() override {
+ return &var_is_oobe_enabled_;
+ }
+
+ private:
+ FakeVariable<bool> var_is_oobe_enabled_{
+ "is_oobe_enabled", kVariableModeConst};
+
+ DISALLOW_COPY_AND_ASSIGN(FakeConfigProvider);
+};
+
+} // namespace chromeos_policy_manager
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_FAKE_CONFIG_PROVIDER_H_
diff --git a/policy_manager/fake_state.cc b/policy_manager/fake_state.cc
index 2af8de0..8bbf452 100644
--- a/policy_manager/fake_state.cc
+++ b/policy_manager/fake_state.cc
@@ -8,7 +8,8 @@
namespace chromeos_policy_manager {
-FakeState::FakeState() : State(new FakeDevicePolicyProvider(),
+FakeState::FakeState() : State(new FakeConfigProvider(),
+ new FakeDevicePolicyProvider(),
new FakeRandomProvider(),
new FakeShillProvider(),
new FakeSystemProvider(),
@@ -18,7 +19,8 @@
FakeState* FakeState::Construct() {
scoped_ptr<FakeState> fake_state(new FakeState());
- if (!(fake_state->device_policy_provider()->Init() &&
+ if (!(fake_state->config_provider()->Init() &&
+ fake_state->device_policy_provider()->Init() &&
fake_state->random_provider()->Init() &&
fake_state->shill_provider()->Init() &&
fake_state->system_provider()->Init() &&
diff --git a/policy_manager/fake_state.h b/policy_manager/fake_state.h
index edad4aa..4f9243b 100644
--- a/policy_manager/fake_state.h
+++ b/policy_manager/fake_state.h
@@ -5,6 +5,7 @@
#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_FAKE_STATE_H_
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_FAKE_STATE_H_
+#include "update_engine/policy_manager/fake_config_provider.h"
#include "update_engine/policy_manager/fake_device_policy_provider.h"
#include "update_engine/policy_manager/fake_random_provider.h"
#include "update_engine/policy_manager/fake_shill_provider.h"
@@ -25,10 +26,15 @@
virtual ~FakeState() {}
// Downcasted getters, to allow access to the fake instances during testing.
+ virtual FakeConfigProvider* config_provider() override {
+ return reinterpret_cast<FakeConfigProvider*>(State::config_provider());
+ }
+
virtual FakeDevicePolicyProvider* device_policy_provider() override {
return reinterpret_cast<FakeDevicePolicyProvider*>(
State::device_policy_provider());
}
+
virtual FakeRandomProvider* random_provider() override {
return reinterpret_cast<FakeRandomProvider*>(State::random_provider());
}
diff --git a/policy_manager/real_config_provider.cc b/policy_manager/real_config_provider.cc
new file mode 100644
index 0000000..d30db12
--- /dev/null
+++ b/policy_manager/real_config_provider.cc
@@ -0,0 +1,51 @@
+// Copyright 2014 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "update_engine/policy_manager/real_config_provider.h"
+
+#include <base/logging.h>
+
+#include "update_engine/constants.h"
+#include "update_engine/policy_manager/generic_variables.h"
+#include "update_engine/simple_key_value_store.h"
+#include "update_engine/utils.h"
+
+using chromeos_update_engine::KeyValueStore;
+using std::string;
+
+namespace {
+
+const char* kConfigFilePath = "/etc/policy_manager.conf";
+
+// Config options:
+const char* kConfigOptsIsOOBEEnabled = "is_oobe_enabled";
+
+} // namespace
+
+namespace chromeos_policy_manager {
+
+bool RealConfigProvider::DoInit() {
+ KeyValueStore store;
+
+ if (hardware_->IsNormalBootMode()) {
+ store.Load(root_prefix_ + kConfigFilePath);
+ } else {
+ if (store.Load(root_prefix_ + chromeos_update_engine::kStatefulPartition +
+ kConfigFilePath)) {
+ LOG(INFO) << "PolicyManager Config loaded from stateful partition.";
+ } else {
+ store.Load(root_prefix_ + kConfigFilePath);
+ }
+ }
+
+ bool is_oobe_enabled;
+ if (!store.GetBoolean(kConfigOptsIsOOBEEnabled, &is_oobe_enabled))
+ is_oobe_enabled = true; // Default value.
+ var_is_oobe_enabled_.reset(
+ new ConstCopyVariable<bool>(kConfigOptsIsOOBEEnabled, is_oobe_enabled));
+
+ return true;
+}
+
+} // namespace chromeos_policy_manager
diff --git a/policy_manager/real_config_provider.h b/policy_manager/real_config_provider.h
new file mode 100644
index 0000000..d79227f
--- /dev/null
+++ b/policy_manager/real_config_provider.h
@@ -0,0 +1,53 @@
+// Copyright 2014 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_REAL_CONFIG_PROVIDER_H_
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_REAL_CONFIG_PROVIDER_H_
+
+#include <string>
+
+#include <base/memory/scoped_ptr.h>
+
+#include "update_engine/hardware_interface.h"
+#include "update_engine/policy_manager/config_provider.h"
+#include "update_engine/policy_manager/generic_variables.h"
+
+namespace chromeos_policy_manager {
+
+// ConfigProvider concrete implementation.
+class RealConfigProvider : public ConfigProvider {
+ public:
+ explicit RealConfigProvider(
+ chromeos_update_engine::HardwareInterface* hardware)
+ : hardware_(hardware) {}
+
+ Variable<bool>* var_is_oobe_enabled() override {
+ return var_is_oobe_enabled_.get();
+ }
+
+ private:
+ friend class PmRealConfigProviderTest;
+
+ virtual bool DoInit() override;
+
+ // Used for testing. Sets the root prefix, which is by default "". Call this
+ // method before calling Init() in order to mock out the place where the files
+ // are being read from.
+ void SetRootPrefix(const std::string& prefix) {
+ root_prefix_ = prefix;
+ }
+
+ scoped_ptr<ConstCopyVariable<bool>> var_is_oobe_enabled_;
+
+ chromeos_update_engine::HardwareInterface* hardware_;
+
+ // Prefix to prepend to the file paths. Useful for testing.
+ std::string root_prefix_;
+
+ DISALLOW_COPY_AND_ASSIGN(RealConfigProvider);
+};
+
+} // namespace chromeos_policy_manager
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_REAL_CONFIG_PROVIDER_H_
diff --git a/policy_manager/real_config_provider_unittest.cc b/policy_manager/real_config_provider_unittest.cc
new file mode 100644
index 0000000..37b480e
--- /dev/null
+++ b/policy_manager/real_config_provider_unittest.cc
@@ -0,0 +1,98 @@
+// Copyright 2014 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "update_engine/policy_manager/real_config_provider.h"
+
+#include <base/file_util.h>
+#include <base/files/scoped_temp_dir.h>
+#include <base/memory/scoped_ptr.h>
+#include <gtest/gtest.h>
+
+#include "update_engine/constants.h"
+#include "update_engine/fake_hardware.h"
+#include "update_engine/policy_manager/pmtest_utils.h"
+#include "update_engine/test_utils.h"
+
+using base::TimeDelta;
+using chromeos_update_engine::WriteFileString;
+using std::string;
+
+namespace chromeos_policy_manager {
+
+class PmRealConfigProviderTest : public ::testing::Test {
+ protected:
+ virtual void SetUp() {
+ ASSERT_TRUE(root_dir_.CreateUniqueTempDir());
+ provider_.reset(new RealConfigProvider(&fake_hardware_));
+ provider_->SetRootPrefix(root_dir_.path().value());
+ }
+
+ // TODO(deymo): Merge this method into common unittest tools for PM.
+ // Calls GetValue and expects its result to be the passed one.
+ template<typename T>
+ void ExpectVariableValue(const T& expected, Variable<T>* variable) {
+ PMTEST_ASSERT_NOT_NULL(variable);
+ scoped_ptr<const T> value(variable->GetValue(default_timeout_, nullptr));
+ PMTEST_ASSERT_NOT_NULL(value.get()) << "Variable: " << variable->GetName();
+ EXPECT_EQ(expected, *value) << "Variable: " << variable->GetName();
+ }
+
+ void WriteStatefulConfig(const string& config) {
+ base::FilePath kFile(root_dir_.path().value()
+ + chromeos_update_engine::kStatefulPartition
+ + "/etc/policy_manager.conf");
+ ASSERT_TRUE(base::CreateDirectory(kFile.DirName()));
+ ASSERT_TRUE(WriteFileString(kFile.value(), config));
+ }
+
+ void WriteRootfsConfig(const string& config) {
+ base::FilePath kFile(root_dir_.path().value()
+ + "/etc/policy_manager.conf");
+ ASSERT_TRUE(base::CreateDirectory(kFile.DirName()));
+ ASSERT_TRUE(WriteFileString(kFile.value(), config));
+ }
+
+ scoped_ptr<RealConfigProvider> provider_;
+ chromeos_update_engine::FakeHardware fake_hardware_;
+ TimeDelta default_timeout_ = TimeDelta::FromSeconds(1);
+ base::ScopedTempDir root_dir_;
+};
+
+TEST_F(PmRealConfigProviderTest, InitTest) {
+ EXPECT_TRUE(provider_->Init());
+ PMTEST_EXPECT_NOT_NULL(provider_->var_is_oobe_enabled());
+}
+
+TEST_F(PmRealConfigProviderTest, NoFileFoundReturnsDefault) {
+ EXPECT_TRUE(provider_->Init());
+ ExpectVariableValue(true, provider_->var_is_oobe_enabled());
+}
+
+TEST_F(PmRealConfigProviderTest, DontReadStatefulInNormalMode) {
+ fake_hardware_.SetIsNormalBootMode(true);
+ WriteStatefulConfig("is_oobe_enabled=false");
+
+ EXPECT_TRUE(provider_->Init());
+ ExpectVariableValue(true, provider_->var_is_oobe_enabled());
+}
+
+TEST_F(PmRealConfigProviderTest, ReadStatefulInDevMode) {
+ fake_hardware_.SetIsNormalBootMode(false);
+ WriteRootfsConfig("is_oobe_enabled=true");
+ // Since the stateful is present, this should read that one.
+ WriteStatefulConfig("is_oobe_enabled=false");
+
+ EXPECT_TRUE(provider_->Init());
+ ExpectVariableValue(false, provider_->var_is_oobe_enabled());
+}
+
+TEST_F(PmRealConfigProviderTest, ReadRootfsIfStatefulNotFound) {
+ fake_hardware_.SetIsNormalBootMode(false);
+ WriteRootfsConfig("is_oobe_enabled=false");
+
+ EXPECT_TRUE(provider_->Init());
+ ExpectVariableValue(false, provider_->var_is_oobe_enabled());
+}
+
+} // namespace chromeos_policy_manager
diff --git a/policy_manager/state.h b/policy_manager/state.h
index b81a88e..23c2d6e 100644
--- a/policy_manager/state.h
+++ b/policy_manager/state.h
@@ -6,6 +6,7 @@
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_STATE_H_
#include "update_engine/policy_manager/device_policy_provider.h"
+#include "update_engine/policy_manager/config_provider.h"
#include "update_engine/policy_manager/random_provider.h"
#include "update_engine/policy_manager/shill_provider.h"
#include "update_engine/policy_manager/system_provider.h"
@@ -19,10 +20,14 @@
class State {
public:
virtual ~State() {}
- State(DevicePolicyProvider* device_policy_provider,
- RandomProvider* random_provider, ShillProvider* shill_provider,
- SystemProvider* system_provider, TimeProvider* time_provider,
+ State(ConfigProvider* config_provider,
+ DevicePolicyProvider* device_policy_provider,
+ RandomProvider* random_provider,
+ ShillProvider* shill_provider,
+ SystemProvider* system_provider,
+ TimeProvider* time_provider,
UpdaterProvider* updater_provider) :
+ config_provider_(config_provider),
device_policy_provider_(device_policy_provider),
random_provider_(random_provider),
shill_provider_(shill_provider),
@@ -31,6 +36,7 @@
updater_provider_(updater_provider) {}
// These methods return the given provider.
+ virtual ConfigProvider* config_provider() { return config_provider_.get(); }
virtual DevicePolicyProvider* device_policy_provider() {
return device_policy_provider_.get();
}
@@ -44,6 +50,7 @@
private:
// Instances of the providers.
+ scoped_ptr<ConfigProvider> config_provider_;
scoped_ptr<DevicePolicyProvider> device_policy_provider_;
scoped_ptr<RandomProvider> random_provider_;
scoped_ptr<ShillProvider> shill_provider_;
diff --git a/policy_manager/state_factory.cc b/policy_manager/state_factory.cc
index b5bfb7f..5866a12 100644
--- a/policy_manager/state_factory.cc
+++ b/policy_manager/state_factory.cc
@@ -8,6 +8,7 @@
#include "update_engine/clock_interface.h"
#include "update_engine/policy_manager/real_device_policy_provider.h"
+#include "update_engine/policy_manager/real_config_provider.h"
#include "update_engine/policy_manager/real_random_provider.h"
#include "update_engine/policy_manager/real_shill_provider.h"
#include "update_engine/policy_manager/real_system_provider.h"
@@ -20,6 +21,8 @@
chromeos_update_engine::DBusWrapperInterface* dbus,
chromeos_update_engine::SystemState* system_state) {
chromeos_update_engine::ClockInterface* const clock = system_state->clock();
+ scoped_ptr<RealConfigProvider> config_provider(new RealConfigProvider(
+ system_state->hardware()));
scoped_ptr<RealDevicePolicyProvider> device_policy_provider(
new RealDevicePolicyProvider(policy_provider));
scoped_ptr<RealRandomProvider> random_provider(new RealRandomProvider());
@@ -30,7 +33,8 @@
scoped_ptr<RealUpdaterProvider> updater_provider(
new RealUpdaterProvider(system_state));
- if (!(device_policy_provider->Init() &&
+ if (!(config_provider->Init() &&
+ device_policy_provider->Init() &&
random_provider->Init() &&
shill_provider->Init() &&
system_provider->Init() &&
@@ -40,7 +44,8 @@
return NULL;
}
- return new State(device_policy_provider.release(),
+ return new State(config_provider.release(),
+ device_policy_provider.release(),
random_provider.release(),
shill_provider.release(),
system_provider.release(),
diff --git a/policy_manager/variable.h b/policy_manager/variable.h
index cd13be1..e855e03 100644
--- a/policy_manager/variable.h
+++ b/policy_manager/variable.h
@@ -174,6 +174,7 @@
// directly from the variable.
friend class EvaluationContext;
+ friend class PmRealConfigProviderTest;
friend class PmRealDevicePolicyProviderTest;
FRIEND_TEST(PmRealDevicePolicyProviderTest,
NonExistentDevicePolicyEmptyVariables);