[Nexthop][fboss2-dev] Add config admin-distance CLI command

cmake/CliFboss2.cmake

@@ -762,6 +762,8 @@ add_library(fboss2_config_lib
   fboss/cli/fboss2/commands/config/CmdConfigAppliedInfo.cpp
   fboss/cli/fboss2/commands/config/CmdConfigReload.h
   fboss/cli/fboss2/commands/config/CmdConfigReload.cpp
+  fboss/cli/fboss2/commands/config/admin_distance/CmdConfigAdminDistance.cpp
+  fboss/cli/fboss2/commands/config/admin_distance/CmdConfigAdminDistance.h
   fboss/cli/fboss2/commands/config/arp/CmdConfigArp.cpp
   fboss/cli/fboss2/commands/config/arp/CmdConfigArp.h
   fboss/cli/fboss2/commands/config/interface/CmdConfigInterface.cpp

cmake/CliFboss2TestConfig.cmake

@@ -4,6 +4,7 @@
 add_executable(fboss2_cmd_config_test
   fboss/util/oss/TestMain.cpp
   fboss/cli/fboss2/oss/config/CmdListImpl.cpp
+  fboss/cli/fboss2/test/config/CmdConfigAdminDistanceTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigAppliedInfoTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigArpTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigHistoryTest.cpp

cmake/CliFboss2TestIntegrationTest.cmake

@@ -8,6 +8,7 @@
 add_executable(fboss2_integration_test
   fboss/cli/fboss2/oss/config/CmdListImpl.cpp
   fboss/cli/fboss2/test/integration_test/Fboss2IntegrationTest.cpp
+  fboss/cli/fboss2/test/integration_test/ConfigAdminDistanceTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigArpTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigConcurrentSessionsTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigInterfaceDescriptionTest.cpp

fboss/cli/fboss2/BUCK

@@ -1031,6 +1031,7 @@ cpp_library(
         "CmdListConfig.cpp",
         "commands/config/CmdConfigAppliedInfo.cpp",
         "commands/config/CmdConfigReload.cpp",
+        "commands/config/admin_distance/CmdConfigAdminDistance.cpp",
         "commands/config/arp/CmdConfigArp.cpp",
         "commands/config/history/CmdConfigHistory.cpp",
         "commands/config/interface/CmdConfigInterface.cpp",
@@ -1140,6 +1141,7 @@ cpp_library(
     headers = [
         "commands/config/CmdConfigAppliedInfo.h",
         "commands/config/CmdConfigReload.h",
+        "commands/config/admin_distance/CmdConfigAdminDistance.h",
         "commands/config/arp/CmdConfigArp.h",
         "commands/config/history/CmdConfigHistory.h",
         "commands/config/interface/CmdConfigInterface.h",

fboss/cli/fboss2/CmdListConfig.cpp

@@ -10,9 +10,9 @@
 
 #include <algorithm>
 #include "fboss/cli/fboss2/CmdList.h"
-
 #include "fboss/cli/fboss2/commands/config/CmdConfigAppliedInfo.h"
 #include "fboss/cli/fboss2/commands/config/CmdConfigReload.h"
+#include "fboss/cli/fboss2/commands/config/admin_distance/CmdConfigAdminDistance.h"
 #include "fboss/cli/fboss2/commands/config/arp/CmdConfigArp.h"
 #include "fboss/cli/fboss2/commands/config/history/CmdConfigHistory.h"
 #include "fboss/cli/fboss2/commands/config/interface/CmdConfigInterface.h"
@@ -116,6 +116,12 @@ namespace facebook::fboss {
 
 const CommandTree& kConfigCommandTree() {
   static CommandTree root = {
+      {"config",
+       "admin-distance",
+       "Set administrative distance for a routing client",
+       commandHandler<CmdConfigAdminDistance>,
+       argRegistrar<CmdConfigAdminDistanceTraits>},
+
       {"config",
        "applied-info",
        "Show config applied information",

fboss/cli/fboss2/commands/config/admin_distance/CmdConfigAdminDistance.cpp

@@ -0,0 +1,104 @@
+/*
+ *  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/admin_distance/CmdConfigAdminDistance.h"
+
+#include "fboss/cli/fboss2/CmdHandler.cpp"
+
+#include <fmt/format.h>
+#include <folly/Conv.h>
+#include <iostream>
+#include "fboss/cli/fboss2/session/ConfigSession.h"
+
+namespace facebook::fboss {
+
+namespace {
+constexpr int32_t kMaxAdminDistance = 255;
+} // namespace
+
+AdminDistanceArg::AdminDistanceArg(std::vector<std::string> v) {
+  if (v.size() != 2) {
+    throw std::invalid_argument(
+        fmt::format(
+            "Expected exactly two arguments: <client-id> <distance>, got {}",
+            v.size()));
+  }
+
+  try {
+    clientId_ = folly::to<int32_t>(v[0]);
+  } catch (const folly::ConversionError&) {
+    throw std::invalid_argument(
+        fmt::format("Invalid client-id '{}': must be an integer", v[0]));
+  }
+  if (clientId_ < 0) {
+    throw std::invalid_argument(
+        fmt::format("client-id must be a non-negative integer, got {}", v[0]));
+  }
+
+  try {
+    distance_ = folly::to<int32_t>(v[1]);
+  } catch (const folly::ConversionError&) {
+    throw std::invalid_argument(
+        fmt::format("Invalid distance '{}': must be an integer", v[1]));
+  }
+  if (distance_ < 0 || distance_ > kMaxAdminDistance) {
+    throw std::invalid_argument(
+        fmt::format(
+            "distance must be in the range [0, {}], got {}",
+            kMaxAdminDistance,
+            v[1]));
+  }
+
+  data_ = std::move(v);
+}
+
+CmdConfigAdminDistanceTraits::RetType CmdConfigAdminDistance::queryClient(
+    const HostInfo& /* hostInfo */,
+    const ObjectArgType& arg) {
+  auto& session = ConfigSession::getInstance();
+  auto& config = session.getAgentConfig();
+  auto& swConfig = *config.sw();
+
+  int32_t clientId = arg.getClientId();
+  int32_t distance = arg.getDistance();
+
+  // Read-modify-write a single map entry, preserving all other clientId
+  // mappings already present in the config.
+  auto& adminDistanceMap = *swConfig.clientIdToAdminDistance();
+  auto it = adminDistanceMap.find(clientId);
+  if (it != adminDistanceMap.end()) {
+    if (it->second == distance) {
+      return fmt::format(
+          "Admin distance for client-id {} is already set to {}",
+          clientId,
+          distance);
+    }
+    it->second = distance;
+  } else {
+    adminDistanceMap.emplace(clientId, distance);
+  }
+
+  session.saveConfig(cli::ServiceType::AGENT, cli::ConfigActionLevel::HITLESS);
+
+  return fmt::format(
+      "Successfully set admin distance for client-id {} to {}",
+      clientId,
+      distance);
+}
+
+void CmdConfigAdminDistance::printOutput(const RetType& output) {
+  std::cout << output << "\n";
+}
+
+// Explicit template instantiation
+template void
+CmdHandler<CmdConfigAdminDistance, CmdConfigAdminDistanceTraits>::run();
+
+} // namespace facebook::fboss

fboss/cli/fboss2/commands/config/admin_distance/CmdConfigAdminDistance.h

@@ -0,0 +1,62 @@
+/*
+ *  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"
+
+namespace facebook::fboss {
+
+// Parses the two positional arguments of
+//   config admin-distance <client-id> <distance>
+// where <client-id> is a routing ClientID (e.g. 0=BGP, 1=static, 786=local-BGP)
+// and <distance> is the administrative distance to associate with it.
+class AdminDistanceArg : public utils::BaseObjectArgType<std::string> {
+ public:
+  /* implicit */ AdminDistanceArg( // NOLINT(google-explicit-constructor)
+      std::vector<std::string> v);
+
+  int32_t getClientId() const {
+    return clientId_;
+  }
+
+  int32_t getDistance() const {
+    return distance_;
+  }
+
+ private:
+  int32_t clientId_{0};
+  int32_t distance_{0};
+};
+
+struct CmdConfigAdminDistanceTraits : public WriteCommandTraits {
+  static void addCliArg(CLI::App& cmd, std::vector<std::string>& args) {
+    cmd.add_option(
+        "client_distance",
+        args,
+        "<client-id> <distance> - admin distance for a routing client "
+        "(client-id: 0=BGP, 1=static, 786=local-BGP; distance: 0-255)");
+  }
+  using ObjectArgType = AdminDistanceArg;
+  using RetType = std::string;
+};
+
+class CmdConfigAdminDistance
+    : public CmdHandler<CmdConfigAdminDistance, CmdConfigAdminDistanceTraits> {
+ public:
+  using ObjectArgType = CmdConfigAdminDistanceTraits::ObjectArgType;
+  using RetType = CmdConfigAdminDistanceTraits::RetType;
+
+  RetType queryClient(const HostInfo& hostInfo, const ObjectArgType& arg);
+
+  void printOutput(const RetType& output);
+};
+
+} // namespace facebook::fboss

fboss/cli/fboss2/test/config/BUCK

@@ -6,6 +6,7 @@ cpp_unittest(
     name = "cmd_config_test",
     srcs = [
         "BgpConfigSessionTest.cpp",
+        "CmdConfigAdminDistanceTest.cpp",
         "CmdConfigAppliedInfoTest.cpp",
         "CmdConfigArpTest.cpp",
         "CmdConfigHistoryTest.cpp",

fboss/cli/fboss2/test/config/CmdConfigAdminDistanceTest.cpp

@@ -0,0 +1,142 @@
+/*
+ *  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 <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "fboss/cli/fboss2/commands/config/admin_distance/CmdConfigAdminDistance.h"
+#include "fboss/cli/fboss2/session/ConfigSession.h"
+#include "fboss/cli/fboss2/test/config/CmdConfigTestBase.h"
+
+using namespace ::testing;
+
+namespace facebook::fboss {
+
+// Seed mirrors the production clientIdToAdminDistance map shipped on Meta
+// devices: every routing client is mapped (BGP=20, static=1, interface/
+// linklocal/remote=0, local-BGP/OpenR=10, internal=255). The CLI mutates a
+// single entry, so the test asserts the other entries survive untouched.
+class CmdConfigAdminDistanceTestFixture : public CmdConfigTestBase {
+ public:
+  CmdConfigAdminDistanceTestFixture()
+      : CmdConfigTestBase(
+            "fboss_admin_distance_test_%%%%-%%%%-%%%%-%%%%",
+            R"({
+  "sw": {
+    "clientIdToAdminDistance": {
+      "0": 20,
+      "1": 1,
+      "2": 0,
+      "3": 0,
+      "4": 0,
+      "700": 255,
+      "786": 10
+    }
+  }
+})") {}
+
+ protected:
+  const std::string setPrefix_ = "config admin-distance";
+};
+
+// ==============================================================================
+// AdminDistanceArg Validation Tests
+// ==============================================================================
+
+TEST_F(CmdConfigAdminDistanceTestFixture, argValidation) {
+  // Valid values
+  EXPECT_EQ(AdminDistanceArg({"0", "20"}).getClientId(), 0);
+  EXPECT_EQ(AdminDistanceArg({"0", "20"}).getDistance(), 20);
+  EXPECT_EQ(AdminDistanceArg({"786", "10"}).getClientId(), 786);
+  EXPECT_EQ(AdminDistanceArg({"1", "0"}).getDistance(), 0);
+  EXPECT_EQ(AdminDistanceArg({"700", "255"}).getDistance(), 255);
+
+  // Invalid: empty
+  EXPECT_THROW(AdminDistanceArg({}), std::invalid_argument);
+
+  // Invalid: wrong arity
+  EXPECT_THROW(AdminDistanceArg({"0"}), std::invalid_argument);
+  EXPECT_THROW(AdminDistanceArg({"0", "20", "30"}), std::invalid_argument);
+
+  // Invalid: non-integer
+  EXPECT_THROW(AdminDistanceArg({"bgp", "20"}), std::invalid_argument);
+  EXPECT_THROW(AdminDistanceArg({"0", "twenty"}), std::invalid_argument);
+  EXPECT_THROW(AdminDistanceArg({"0", "20.5"}), std::invalid_argument);
+
+  // Invalid: negative client-id
+  EXPECT_THROW(AdminDistanceArg({"-1", "20"}), std::invalid_argument);
+
+  // Invalid: distance out of [0, 255]
+  EXPECT_THROW(AdminDistanceArg({"0", "-1"}), std::invalid_argument);
+  EXPECT_THROW(AdminDistanceArg({"0", "256"}), std::invalid_argument);
+}
+
+// ==============================================================================
+// Set Command Tests
+// ==============================================================================
+
+TEST_F(CmdConfigAdminDistanceTestFixture, updateExistingClient) {
+  setupTestableConfigSession(setPrefix_, "0 30");
+  CmdConfigAdminDistance cmd;
+  HostInfo hostInfo("testhost");
+  AdminDistanceArg arg({"0", "30"});
+
+  auto result = cmd.queryClient(hostInfo, arg);
+  EXPECT_THAT(result, HasSubstr("30"));
+
+  auto& config = ConfigSession::getInstance().getAgentConfig();
+  const auto& map = *config.sw()->clientIdToAdminDistance();
+  EXPECT_EQ(map.at(0), 30);
+
+  // Other entries must survive untouched.
+  EXPECT_EQ(map.at(1), 1);
+  EXPECT_EQ(map.at(2), 0);
+  EXPECT_EQ(map.at(3), 0);
+  EXPECT_EQ(map.at(4), 0);
+  EXPECT_EQ(map.at(700), 255);
+  EXPECT_EQ(map.at(786), 10);
+  EXPECT_EQ(map.size(), 7);
+}
+
+TEST_F(CmdConfigAdminDistanceTestFixture, addNewClient) {
+  setupTestableConfigSession(setPrefix_, "800 2");
+  CmdConfigAdminDistance cmd;
+  HostInfo hostInfo("testhost");
+  AdminDistanceArg arg({"800", "2"});
+
+  auto result = cmd.queryClient(hostInfo, arg);
+  EXPECT_THAT(result, HasSubstr("800"));
+
+  auto& config = ConfigSession::getInstance().getAgentConfig();
+  const auto& map = *config.sw()->clientIdToAdminDistance();
+  EXPECT_EQ(map.at(800), 2);
+  // All original entries preserved; new key added.
+  EXPECT_EQ(map.at(0), 20);
+  EXPECT_EQ(map.at(1), 1);
+  EXPECT_EQ(map.at(2), 0);
+  EXPECT_EQ(map.at(3), 0);
+  EXPECT_EQ(map.at(4), 0);
+  EXPECT_EQ(map.at(700), 255);
+  EXPECT_EQ(map.at(786), 10);
+  EXPECT_EQ(map.size(), 8);
+}
+
+TEST_F(CmdConfigAdminDistanceTestFixture, alreadySet) {
+  // Seed has client-id 786 -> 10; setting to 10 is a no-op.
+  setupTestableConfigSession(setPrefix_, "786 10");
+  CmdConfigAdminDistance cmd;
+  HostInfo hostInfo("testhost");
+  AdminDistanceArg arg({"786", "10"});
+
+  auto result = cmd.queryClient(hostInfo, arg);
+  EXPECT_THAT(result, HasSubstr("already"));
+}
+
+} // namespace facebook::fboss

fboss/cli/fboss2/test/integration_test/BUCK

@@ -16,6 +16,7 @@ oncall("fboss_oss")
 cpp_binary(
     name = "fboss2_integration_test",
     srcs = [
+        "ConfigAdminDistanceTest.cpp",
         "ConfigArpTest.cpp",
         "ConfigConcurrentSessionsTest.cpp",
         "ConfigInterfaceDescriptionTest.cpp",