diff --git a/cmake/CliFboss2.cmake b/cmake/CliFboss2.cmake index d0891c04e7737..6a5872c93f967 100644 --- a/cmake/CliFboss2.cmake +++ b/cmake/CliFboss2.cmake @@ -577,6 +577,8 @@ add_library(fboss2_config_lib fboss/cli/fboss2/commands/config/CmdConfigReload.cpp fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.h fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.cpp + fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h + fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.cpp fboss/cli/fboss2/session/ConfigSession.h fboss/cli/fboss2/session/ConfigSession.cpp fboss/cli/fboss2/CmdListConfig.cpp diff --git a/cmake/CliFboss2Test.cmake b/cmake/CliFboss2Test.cmake index 3d4391cb82dee..e50f1dd335b8e 100644 --- a/cmake/CliFboss2Test.cmake +++ b/cmake/CliFboss2Test.cmake @@ -5,6 +5,7 @@ add_executable(fboss2_cmd_test fboss/cli/fboss2/test/TestMain.cpp fboss/cli/fboss2/test/CmdConfigAppliedInfoTest.cpp fboss/cli/fboss2/test/CmdConfigReloadTest.cpp + fboss/cli/fboss2/test/CmdConfigSessionDiffTest.cpp fboss/cli/fboss2/test/CmdConfigSessionTest.cpp fboss/cli/fboss2/test/CmdSetPortStateTest.cpp fboss/cli/fboss2/test/CmdShowAclTest.cpp diff --git a/fboss/cli/fboss2/BUCK b/fboss/cli/fboss2/BUCK index 592c7bc87e675..eb56fe91079a5 100644 --- a/fboss/cli/fboss2/BUCK +++ b/fboss/cli/fboss2/BUCK @@ -772,12 +772,14 @@ cpp_library( "commands/config/CmdConfigAppliedInfo.cpp", "commands/config/CmdConfigReload.cpp", "commands/config/session/CmdConfigSessionCommit.cpp", + "commands/config/session/CmdConfigSessionDiff.cpp", "session/ConfigSession.cpp", ], headers = [ "commands/config/CmdConfigAppliedInfo.h", "commands/config/CmdConfigReload.h", "commands/config/session/CmdConfigSessionCommit.h", + "commands/config/session/CmdConfigSessionDiff.h", "session/ConfigSession.h", ], exported_deps = [ diff --git a/fboss/cli/fboss2/CmdHandlerImplConfig.cpp b/fboss/cli/fboss2/CmdHandlerImplConfig.cpp index d4217e43fa286..6c93601afdc7a 100644 --- a/fboss/cli/fboss2/CmdHandlerImplConfig.cpp +++ b/fboss/cli/fboss2/CmdHandlerImplConfig.cpp @@ -13,6 +13,7 @@ #include "fboss/cli/fboss2/commands/config/CmdConfigAppliedInfo.h" #include "fboss/cli/fboss2/commands/config/CmdConfigReload.h" #include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.h" +#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h" namespace facebook::fboss { @@ -21,5 +22,7 @@ CmdHandler::run(); template void CmdHandler::run(); template void CmdHandler::run(); +template void +CmdHandler::run(); } // namespace facebook::fboss diff --git a/fboss/cli/fboss2/CmdListConfig.cpp b/fboss/cli/fboss2/CmdListConfig.cpp index e59256deb5fbc..231cef7c5990c 100644 --- a/fboss/cli/fboss2/CmdListConfig.cpp +++ b/fboss/cli/fboss2/CmdListConfig.cpp @@ -14,6 +14,7 @@ #include "fboss/cli/fboss2/commands/config/CmdConfigAppliedInfo.h" #include "fboss/cli/fboss2/commands/config/CmdConfigReload.h" #include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.h" +#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h" namespace facebook::fboss { @@ -30,11 +31,17 @@ const CommandTree& kConfigCommandTree() { "session", "Manage config session", {{ - "commit", - "Commit the current config session", - commandHandler, - argTypeHandler, - }}, + "commit", + "Commit the current config session", + commandHandler, + argTypeHandler, + }, + { + "diff", + "Show diff between configs (session vs live, session vs revision, or revision vs revision)", + commandHandler, + argTypeHandler, + }}, }, {"config", diff --git a/fboss/cli/fboss2/CmdSubcommands.cpp b/fboss/cli/fboss2/CmdSubcommands.cpp index 87485bcd711c5..199f4b00a6c6b 100644 --- a/fboss/cli/fboss2/CmdSubcommands.cpp +++ b/fboss/cli/fboss2/CmdSubcommands.cpp @@ -219,6 +219,10 @@ CLI::App* CmdSubcommands::addCommand( case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_FAN_PWM: subCmd->add_option("pwm", args, "Fan PWM (0..100) or 'disable'"); break; + case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_REVISION_LIST: + subCmd->add_option( + "revisions", args, "Revision(s) in the form 'rN' or 'current'"); + break; case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_UNINITIALIZE: case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_NONE: break; diff --git a/fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.cpp b/fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.cpp new file mode 100644 index 0000000000000..efb0fb447f8aa --- /dev/null +++ b/fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.cpp @@ -0,0 +1,149 @@ +/* + * Copyright (c) 2004-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + */ + +#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h" +#include "fboss/cli/fboss2/session/ConfigSession.h" + +#include +#include + +namespace fs = std::filesystem; + +namespace facebook::fboss { + +namespace { + +// Helper function to resolve a revision specifier to a file path +// Note: Revision format validation is done in RevisionList constructor +std::string resolveRevisionPath( + const std::string& revision, + const std::string& cliConfigDir, + const std::string& systemConfigPath) { + if (revision == "current") { + return systemConfigPath; + } + + // Build the path (revision is already validated to be in "rN" format) + std::string revisionPath = cliConfigDir + "/agent-" + revision + ".conf"; + + // Check if the file exists + if (!fs::exists(revisionPath)) { + throw std::invalid_argument( + "Revision " + revision + " does not exist at " + revisionPath); + } + + return revisionPath; +} + +// Helper function to execute diff and return the result +std::string executeDiff( + const std::string& path1, + const std::string& path2, + const std::string& label1, + const std::string& label2) { + try { + folly::Subprocess proc( + std::vector{ + "/usr/bin/diff", + "-u", + "--label", + label1, + "--label", + label2, + path1, + path2}, + folly::Subprocess::Options().pipeStdout().pipeStderr()); + + auto result = proc.communicate(); + int returnCode = proc.wait().exitStatus(); + + // diff returns 0 if files are identical, 1 if different, 2 on error + if (returnCode == 0) { + return "No differences between " + label1 + " and " + label2 + "."; + } else if (returnCode == 1) { + // Files differ - return the diff output + return result.first; + } else { + // Error occurred + throw std::runtime_error("diff command failed: " + result.second); + } + } catch (const std::exception& ex) { + throw std::runtime_error( + "Failed to execute diff command: " + std::string(ex.what())); + } +} + +} // namespace + +CmdConfigSessionDiffTraits::RetType CmdConfigSessionDiff::queryClient( + const HostInfo& /* hostInfo */, + const utils::RevisionList& revisions) { + auto& session = ConfigSession::getInstance(); + + std::string systemConfigPath = session.getSystemConfigPath(); + std::string sessionConfigPath = session.getSessionConfigPath(); + std::string cliConfigDir = session.getCliConfigDir(); + + // Mode 1: No arguments - diff session vs current live config + if (revisions.empty()) { + if (!session.sessionExists()) { + return "No config session exists. Make a config change first."; + } + + return executeDiff( + systemConfigPath, + sessionConfigPath, + "current live config", + "session config"); + } + + // Mode 2: One argument - diff session vs specified revision + if (revisions.size() == 1) { + if (!session.sessionExists()) { + return "No config session exists. Make a config change first."; + } + + std::string revisionPath = + resolveRevisionPath(revisions[0], cliConfigDir, systemConfigPath); + std::string label = + revisions[0] == "current" ? "current live config" : revisions[0]; + + return executeDiff( + revisionPath, sessionConfigPath, label, "session config"); + } + + // Mode 3: Two arguments - diff between two revisions + if (revisions.size() == 2) { + std::string path1 = + resolveRevisionPath(revisions[0], cliConfigDir, systemConfigPath); + std::string path2 = + resolveRevisionPath(revisions[1], cliConfigDir, systemConfigPath); + + std::string label1 = + revisions[0] == "current" ? "current live config" : revisions[0]; + std::string label2 = + revisions[1] == "current" ? "current live config" : revisions[1]; + + return executeDiff(path1, path2, label1, label2); + } + + // More than 2 arguments is an error + throw std::invalid_argument( + "Too many arguments. Expected 0, 1, or 2 revision specifiers."); +} + +void CmdConfigSessionDiff::printOutput(const RetType& diffOutput) { + std::cout << diffOutput; + if (!diffOutput.empty() && diffOutput.back() != '\n') { + std::cout << std::endl; + } +} + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h b/fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h new file mode 100644 index 0000000000000..10655c6485cbd --- /dev/null +++ b/fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2004-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + */ + +#pragma once + +#include "fboss/cli/fboss2/CmdHandler.h" +#include "fboss/cli/fboss2/utils/CmdClientUtils.h" +#include "fboss/cli/fboss2/utils/CmdUtils.h" + +namespace facebook::fboss { + +struct CmdConfigSessionDiffTraits : public WriteCommandTraits { + static constexpr utils::ObjectArgTypeId ObjectArgTypeId = + utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_REVISION_LIST; + using ObjectArgType = utils::RevisionList; + using RetType = std::string; +}; + +class CmdConfigSessionDiff + : public CmdHandler { + public: + using ObjectArgType = CmdConfigSessionDiffTraits::ObjectArgType; + using RetType = CmdConfigSessionDiffTraits::RetType; + + RetType queryClient( + const HostInfo& hostInfo, + const utils::RevisionList& revisions); + + void printOutput(const RetType& diffOutput); +}; + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/session/ConfigSession.cpp b/fboss/cli/fboss2/session/ConfigSession.cpp index ecf11bb06a86b..e9232a33b216f 100644 --- a/fboss/cli/fboss2/session/ConfigSession.cpp +++ b/fboss/cli/fboss2/session/ConfigSession.cpp @@ -197,10 +197,24 @@ ConfigSession::ConfigSession( initializeSession(); } -ConfigSession& ConfigSession::getInstance() { - static ConfigSession instance; +namespace { +std::unique_ptr& getInstancePtr() { + static std::unique_ptr instance; return instance; } +} // namespace + +ConfigSession& ConfigSession::getInstance() { + auto& instance = getInstancePtr(); + if (!instance) { + instance = std::make_unique(); + } + return *instance; +} + +void ConfigSession::setInstance(std::unique_ptr newInstance) { + getInstancePtr() = std::move(newInstance); +} std::string ConfigSession::getSessionConfigPath() const { return sessionConfigPath_; @@ -210,6 +224,10 @@ std::string ConfigSession::getSystemConfigPath() const { return systemConfigPath_; } +std::string ConfigSession::getCliConfigDir() const { + return cliConfigDir_; +} + bool ConfigSession::sessionExists() const { return fs::exists(sessionConfigPath_); } diff --git a/fboss/cli/fboss2/session/ConfigSession.h b/fboss/cli/fboss2/session/ConfigSession.h index b07e9962c1702..5f962dad5c874 100644 --- a/fboss/cli/fboss2/session/ConfigSession.h +++ b/fboss/cli/fboss2/session/ConfigSession.h @@ -93,6 +93,9 @@ class ConfigSession { // Get the path to the system config file (/etc/coop/agent.conf) std::string getSystemConfigPath() const; + // Get the path to the CLI config directory (/etc/coop/cli) + std::string getCliConfigDir() const; + // Atomically commit the session to /etc/coop/cli/agent-rN.conf, // update the symlink /etc/coop/agent.conf to point to it, and reload config. // Returns the revision number that was committed if the commit was @@ -124,6 +127,9 @@ class ConfigSession { const std::string& systemConfigPath, const std::string& cliConfigDir); + // Set the singleton instance (for testing only) + static void setInstance(std::unique_ptr instance); + private: std::string sessionConfigPath_; std::string systemConfigPath_; diff --git a/fboss/cli/fboss2/test/BUCK b/fboss/cli/fboss2/test/BUCK index f15b5acd5fe8b..797d2da0b80c8 100644 --- a/fboss/cli/fboss2/test/BUCK +++ b/fboss/cli/fboss2/test/BUCK @@ -57,6 +57,7 @@ cpp_unittest( srcs = [ "CmdConfigAppliedInfoTest.cpp", "CmdConfigReloadTest.cpp", + "CmdConfigSessionDiffTest.cpp", "CmdConfigSessionTest.cpp", "CmdGetPcapTest.cpp", "CmdSetPortStateTest.cpp", diff --git a/fboss/cli/fboss2/test/CmdConfigSessionDiffTest.cpp b/fboss/cli/fboss2/test/CmdConfigSessionDiffTest.cpp new file mode 100644 index 0000000000000..8488e18815854 --- /dev/null +++ b/fboss/cli/fboss2/test/CmdConfigSessionDiffTest.cpp @@ -0,0 +1,376 @@ +// (c) Facebook, Inc. and its affiliates. Confidential and proprietary. + +#include +#include +#include +#include + +#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h" +#include "fboss/cli/fboss2/session/ConfigSession.h" +#include "fboss/cli/fboss2/test/CmdHandlerTestBase.h" +#include "fboss/cli/fboss2/test/TestableConfigSession.h" +#include "fboss/cli/fboss2/utils/CmdUtils.h" +#include "fboss/cli/fboss2/utils/PortMap.h" + +#include +#include + +namespace fs = std::filesystem; + +using namespace ::testing; + +namespace facebook::fboss { + +class CmdConfigSessionDiffTestFixture : public CmdHandlerTestBase { + public: + fs::path testHomeDir_; + fs::path testEtcDir_; + fs::path systemConfigPath_; + fs::path sessionConfigPath_; + fs::path cliConfigDir_; + + void SetUp() override { + CmdHandlerTestBase::SetUp(); + + // Create unique test directories for each test to avoid conflicts when + // running tests in parallel + auto tempBase = fs::temp_directory_path(); + auto uniquePath = boost::filesystem::unique_path( + "fboss2_config_session_diff_test_%%%%-%%%%-%%%%-%%%%"); + testHomeDir_ = tempBase / (uniquePath.string() + "_home"); + testEtcDir_ = tempBase / (uniquePath.string() + "_etc"); + + // Clean up any previous test artifacts (shouldn't exist with unique names) + std::error_code ec; + fs::remove_all(testHomeDir_, ec); + fs::remove_all(testEtcDir_, ec); + + // Create fresh test directories + fs::create_directories(testHomeDir_); + fs::create_directories(testEtcDir_); + + // Set up paths + systemConfigPath_ = testEtcDir_ / "agent.conf"; + sessionConfigPath_ = testHomeDir_ / ".fboss2" / "agent.conf"; + cliConfigDir_ = testEtcDir_ / "coop" / "cli"; + + // Create CLI config directory + fs::create_directories(cliConfigDir_); + + // Create a default system config + createTestConfig( + systemConfigPath_, + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 2, + "speed": 100000 + } + ] + } +})"); + } + + void TearDown() override { + // Reset the singleton to ensure tests don't interfere with each other + TestableConfigSession::setInstance(nullptr); + // Clean up test directories (use error_code to avoid exceptions) + std::error_code ec; + fs::remove_all(testHomeDir_, ec); + fs::remove_all(testEtcDir_, ec); + CmdHandlerTestBase::TearDown(); + } + + void createTestConfig(const fs::path& path, const std::string& content) { + fs::create_directories(path.parent_path()); + std::ofstream file(path); + file << content; + file.close(); + } + + std::string readFile(const fs::path& path) { + std::ifstream file(path); + std::stringstream buffer; + buffer << file.rdbuf(); + return buffer.str(); + } + + void initializeTestSession() { + // Ensure system config exists before initializing session + // (ConfigSession constructor calls initializeSession which will copy it) + if (!fs::exists(systemConfigPath_)) { + createTestConfig( + systemConfigPath_, + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 2, + "speed": 100000 + } + ] + } +})"); + } + + // Ensure the parent directory of session config exists + // (initializeSession will try to copy system config to session config) + fs::create_directories(sessionConfigPath_.parent_path()); + + // Initialize ConfigSession singleton with test paths + // The constructor will automatically call initializeSession() + TestableConfigSession::setInstance( + std::make_unique( + sessionConfigPath_.string(), + systemConfigPath_.string(), + cliConfigDir_.string())); + } +}; + +TEST_F(CmdConfigSessionDiffTestFixture, diffNoSession) { + // Initialize the session (which creates the session config file) + initializeTestSession(); + + // Then delete the session config file to simulate "no session" case + fs::remove(sessionConfigPath_); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList emptyRevisions(std::vector{}); + + auto result = cmd.queryClient(localhost(), emptyRevisions); + + EXPECT_EQ(result, "No config session exists. Make a config change first."); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffIdenticalConfigs) { + initializeTestSession(); + + // initializeTestSession() already creates the session config by copying + // the system config, so we don't need to do it again + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList emptyRevisions(std::vector{}); + + auto result = cmd.queryClient(localhost(), emptyRevisions); + + EXPECT_NE( + result.find( + "No differences between current live config and session config"), + std::string::npos); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffDifferentConfigs) { + initializeTestSession(); + + // Create session directory and modify the config + fs::create_directories(sessionConfigPath_.parent_path()); + createTestConfig( + sessionConfigPath_, + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "description": "Modified port", + "state": 2, + "speed": 100000 + } + ] + } +})"); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList emptyRevisions(std::vector{}); + + auto result = cmd.queryClient(localhost(), emptyRevisions); + + // Should show a diff with the added "description" field + EXPECT_NE(result.find("description"), std::string::npos); + EXPECT_NE(result.find("Modified port"), std::string::npos); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffSessionVsRevision) { + initializeTestSession(); + + // Create a revision file + createTestConfig( + cliConfigDir_ / "agent-r1.conf", + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 1 + } + ] + } +})"); + + // Create session with different content + fs::create_directories(sessionConfigPath_.parent_path()); + createTestConfig( + sessionConfigPath_, + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 2 + } + ] + } +})"); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList revisions(std::vector{"r1"}); + + auto result = cmd.queryClient(localhost(), revisions); + + // Should show a diff between r1 and session (state changed from 1 to 2) + EXPECT_NE(result.find("- \"state\": 1"), std::string::npos); + EXPECT_NE(result.find("+ \"state\": 2"), std::string::npos); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffTwoRevisions) { + initializeTestSession(); + + // Create two different revision files + createTestConfig( + cliConfigDir_ / "agent-r1.conf", + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 2 + } + ] + } +})"); + + createTestConfig( + cliConfigDir_ / "agent-r2.conf", + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "description": "Added description", + "state": 2 + } + ] + } +})"); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList revisions(std::vector{"r1", "r2"}); + + auto result = cmd.queryClient(localhost(), revisions); + + // Should show the added "description" field + EXPECT_NE(result.find("description"), std::string::npos); + EXPECT_NE(result.find("Added description"), std::string::npos); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffWithCurrentKeyword) { + initializeTestSession(); + + // Create a revision file + createTestConfig( + cliConfigDir_ / "agent-r1.conf", + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 1 + } + ] + } +})"); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList revisions(std::vector{"r1", "current"}); + + auto result = cmd.queryClient(localhost(), revisions); + + // Should show a diff between r1 and current system config (state changed from + // 1 to 2, speed added) + EXPECT_NE(result.find("- \"state\": 1"), std::string::npos); + EXPECT_NE(result.find("+ \"state\": 2"), std::string::npos); + EXPECT_NE(result.find("+ \"speed\": 100000"), std::string::npos); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffNonexistentRevision) { + initializeTestSession(); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList revisions(std::vector{"r999"}); + + // Should throw an exception for nonexistent revision + EXPECT_THROW(cmd.queryClient(localhost(), revisions), std::invalid_argument); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffTooManyArguments) { + initializeTestSession(); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList revisions(std::vector{"r1", "r2", "r3"}); + + // Should throw an exception for too many arguments + EXPECT_THROW(cmd.queryClient(localhost(), revisions), std::invalid_argument); +} + +TEST_F(CmdConfigSessionDiffTestFixture, printOutput) { + auto cmd = CmdConfigSessionDiff(); + std::string diffOutput = + "--- a/config\n+++ b/config\n@@ -1 +1 @@\n-old\n+new"; + + // Redirect cout to capture output + std::stringstream buffer; + std::streambuf* old = std::cout.rdbuf(buffer.rdbuf()); + + cmd.printOutput(diffOutput); + + // Restore cout + std::cout.rdbuf(old); + + std::string output = buffer.str(); + + EXPECT_NE(output.find("--- a/config"), std::string::npos); + EXPECT_NE(output.find("+++ b/config"), std::string::npos); + EXPECT_NE(output.find("-old"), std::string::npos); + EXPECT_NE(output.find("+new"), std::string::npos); +} + +TEST_F(CmdConfigSessionDiffTestFixture, printOutputNoDifferences) { + auto cmd = CmdConfigSessionDiff(); + std::string noDiffMessage = + "No differences between current live config and session config."; + + // Redirect cout to capture output + std::stringstream buffer; + std::streambuf* old = std::cout.rdbuf(buffer.rdbuf()); + + cmd.printOutput(noDiffMessage); + + // Restore cout + std::cout.rdbuf(old); + + std::string output = buffer.str(); + + EXPECT_NE(output.find("No differences"), std::string::npos); +} + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/test/TestableConfigSession.h b/fboss/cli/fboss2/test/TestableConfigSession.h index 4838656d3db37..1f2179dcf6c10 100644 --- a/fboss/cli/fboss2/test/TestableConfigSession.h +++ b/fboss/cli/fboss2/test/TestableConfigSession.h @@ -23,6 +23,9 @@ class TestableConfigSession : public ConfigSession { const std::string& systemConfigPath, const std::string& cliConfigDir) : ConfigSession(sessionConfigPath, systemConfigPath, cliConfigDir) {} + + // Expose protected setInstance() for testing + using ConfigSession::setInstance; }; } // namespace facebook::fboss diff --git a/fboss/cli/fboss2/utils/CmdUtils.cpp b/fboss/cli/fboss2/utils/CmdUtils.cpp index fba8013585a84..3a3b43ec16096 100644 --- a/fboss/cli/fboss2/utils/CmdUtils.cpp +++ b/fboss/cli/fboss2/utils/CmdUtils.cpp @@ -473,4 +473,41 @@ getUncachedSwitchReachabilityInfo( return reachabilityMatrix; } +RevisionList::RevisionList(std::vector v) { + // Validate each revision specifier + for (const auto& revision : v) { + if (revision == "current") { + // "current" is always valid + data_.push_back(revision); + continue; + } + + // Must be in the form "rN" where N is a positive integer + if (revision.empty() || revision[0] != 'r') { + throw std::invalid_argument( + "Invalid revision specifier: '" + revision + + "'. Expected 'rN' or 'current'"); + } + + // Extract the number part after 'r' + std::string revNum = revision.substr(1); + if (revNum.empty()) { + throw std::invalid_argument( + "Invalid revision specifier: '" + revision + + "'. Expected 'rN' or 'current'"); + } + + // Validate that it's all digits + for (char c : revNum) { + if (!std::isdigit(c)) { + throw std::invalid_argument( + "Invalid revision number: '" + revision + + "'. Expected 'rN' or 'current'"); + } + } + + data_.push_back(revision); + } +} + } // namespace facebook::fboss::utils diff --git a/fboss/cli/fboss2/utils/CmdUtils.h b/fboss/cli/fboss2/utils/CmdUtils.h index 03ce47b283bf9..fed01fabc6855 100644 --- a/fboss/cli/fboss2/utils/CmdUtils.h +++ b/fboss/cli/fboss2/utils/CmdUtils.h @@ -417,6 +417,15 @@ class MirrorList : public BaseObjectArgType { ObjectArgTypeId::OBJECT_ARG_TYPE_ID_MIRROR_LIST; }; +class RevisionList : public BaseObjectArgType { + public: + /* implicit */ RevisionList() : BaseObjectArgType() {} + /* implicit */ RevisionList(std::vector v); + + const static ObjectArgTypeId id = + ObjectArgTypeId::OBJECT_ARG_TYPE_ID_REVISION_LIST; +}; + // Called after CLI11 is initlized but before parsing, for any final // initialization steps void postAppInit(int argc, char* argv[], CLI::App& app); diff --git a/fboss/cli/fboss2/utils/CmdUtilsCommon.h b/fboss/cli/fboss2/utils/CmdUtilsCommon.h index 5987c6bd72e81..340b77dde69ee 100644 --- a/fboss/cli/fboss2/utils/CmdUtilsCommon.h +++ b/fboss/cli/fboss2/utils/CmdUtilsCommon.h @@ -59,6 +59,7 @@ enum class ObjectArgTypeId : uint8_t { OBJECT_ARG_TYPE_ID_MIRROR_LIST, OBJECT_ARG_TYPE_LINK_DIRECTION, OBJECT_ARG_TYPE_FAN_PWM, + OBJECT_ARG_TYPE_ID_REVISION_LIST, }; template