[Nexthop][fboss2-dev] Add fboss2-dev config hostname CLI

cmake/CliFboss2.cmake

@@ -791,6 +791,8 @@ add_library(fboss2_config_lib
   fboss/cli/fboss2/commands/config/switch/CmdConfigSwitch.h
   fboss/cli/fboss2/commands/config/switch/admin_distance/CmdConfigAdminDistance.cpp
   fboss/cli/fboss2/commands/config/switch/admin_distance/CmdConfigAdminDistance.h
+  fboss/cli/fboss2/commands/config/switch/hostname/CmdConfigHostname.cpp
+  fboss/cli/fboss2/commands/config/switch/hostname/CmdConfigHostname.h
   fboss/cli/fboss2/commands/config/arp/CmdConfigArp.cpp
   fboss/cli/fboss2/commands/config/arp/CmdConfigArp.h
   fboss/cli/fboss2/commands/config/copp/CmdConfigCopp.cpp

cmake/CliFboss2TestConfig.cmake

@@ -9,6 +9,7 @@ add_executable(fboss2_cmd_config_test
   fboss/cli/fboss2/test/config/CmdConfigArpTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigCoppTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigDhcpTest.cpp
+  fboss/cli/fboss2/test/config/CmdConfigHostnameTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigHistoryTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigInterfaceIpv6NdpTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigInterfaceSwitchportAccessVlanTest.cpp

cmake/CliFboss2TestIntegrationTest.cmake

@@ -11,6 +11,7 @@ add_executable(fboss2_integration_test
   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/ConfigHostnameTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigCoppTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigDhcpSourceOverrideTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigInterfaceDescriptionTest.cpp

fboss/cli/fboss2/BUCK

@@ -1141,6 +1141,7 @@ cpp_library(
         "commands/config/session/CmdConfigSessionRebase.cpp",
         "commands/config/switch/CmdConfigSwitch.cpp",
         "commands/config/switch/admin_distance/CmdConfigAdminDistance.cpp",
+        "commands/config/switch/hostname/CmdConfigHostname.cpp",
         "commands/config/tunnel/CmdConfigTunnel.cpp",
         "commands/config/tunnel/ip_in_ip/CmdConfigTunnelIpInIp.cpp",
         "commands/config/tunnel/ip_in_ip/TunnelIpInIpConfigUtils.cpp",
@@ -1262,6 +1263,7 @@ cpp_library(
         "commands/config/session/CmdConfigSessionRebase.h",
         "commands/config/switch/CmdConfigSwitch.h",
         "commands/config/switch/admin_distance/CmdConfigAdminDistance.h",
+        "commands/config/switch/hostname/CmdConfigHostname.h",
         "commands/config/tunnel/CmdConfigTunnel.h",
         "commands/config/tunnel/ip_in_ip/CmdConfigTunnelIpInIp.h",
         "commands/config/tunnel/ip_in_ip/TunnelIpInIpConfigUtils.h",

fboss/cli/fboss2/CmdListConfig.cpp

@@ -111,6 +111,7 @@
 #include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionRebase.h"
 #include "fboss/cli/fboss2/commands/config/switch/CmdConfigSwitch.h"
 #include "fboss/cli/fboss2/commands/config/switch/admin_distance/CmdConfigAdminDistance.h"
+#include "fboss/cli/fboss2/commands/config/switch/hostname/CmdConfigHostname.h"
 #include "fboss/cli/fboss2/commands/config/tunnel/CmdConfigTunnel.h"
 #include "fboss/cli/fboss2/commands/config/tunnel/ip_in_ip/CmdConfigTunnelIpInIp.h"
 #include "fboss/cli/fboss2/commands/config/tunnel/ip_in_ip/decap/CmdConfigTunnelIpInIpDecap.h"
@@ -145,11 +146,17 @@ const CommandTree& kConfigCommandTree() {
           commandHandler<CmdConfigSwitch>,
           argRegistrar<CmdConfigSwitchTraits>,
           {{
-              "admin-distance",
-              "Set administrative distance for a routing client",
-              commandHandler<CmdConfigAdminDistance>,
-              argRegistrar<CmdConfigAdminDistanceTraits>,
-          }},
+               "admin-distance",
+               "Set administrative distance for a routing client",
+               commandHandler<CmdConfigAdminDistance>,
+               argRegistrar<CmdConfigAdminDistanceTraits>,
+           },
+           {
+               "hostname",
+               "Set switch hostname",
+               commandHandler<CmdConfigHostname>,
+               argRegistrar<CmdConfigHostnameTraits>,
+           }},
       },
 
       {"config",

fboss/cli/fboss2/commands/config/switch/CmdConfigSwitch.cpp

@@ -18,7 +18,7 @@ namespace facebook::fboss {
 
 CmdConfigSwitchTraits::RetType CmdConfigSwitch::queryClient(
     const HostInfo& /* hostInfo */) {
-  return "Switch configuration commands. Use subcommands: admin-distance";
+  return "Switch configuration commands. Use subcommands: admin-distance, hostname";
 }
 
 void CmdConfigSwitch::printOutput(const RetType& output) {

fboss/cli/fboss2/commands/config/switch/hostname/CmdConfigHostname.cpp

@@ -0,0 +1,87 @@
+/*
+ *  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/switch/hostname/CmdConfigHostname.h"
+
+#include "fboss/cli/fboss2/CmdHandler.cpp" // NOLINT(facebook-unused-include-check)
+
+#include <fmt/format.h>
+#include <re2/re2.h>
+#include <iostream>
+#include <stdexcept>
+#include "fboss/cli/fboss2/gen-cpp2/cli_metadata_types.h"
+#include "fboss/cli/fboss2/session/ConfigSession.h"
+#include "fboss/cli/fboss2/utils/HostInfo.h"
+
+namespace facebook::fboss {
+
+namespace {
+// RFC 952/1123: dot-separated labels, each 1-63 chars of [a-zA-Z0-9-] with no
+// leading/trailing hyphen. Total length (checked separately) max 253 chars.
+bool isValidHostname(const std::string& hostname) {
+  static const re2::RE2 kHostnamePattern(
+      "^[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?"
+      "(\\.[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$");
+  return hostname.length() <= 253 &&
+      re2::RE2::FullMatch(hostname, kHostnamePattern);
+}
+} // namespace
+
+HostnameConfigArgs::HostnameConfigArgs(std::vector<std::string> v) {
+  if (v.empty()) {
+    throw std::invalid_argument("Hostname is required");
+  }
+  if (v.size() != 1) {
+    throw std::invalid_argument(
+        fmt::format("Expected exactly one hostname, got {}", v.size()));
+  }
+  if (v[0].empty()) {
+    throw std::invalid_argument("Hostname must not be empty");
+  }
+  if (!isValidHostname(v[0])) {
+    throw std::invalid_argument(
+        fmt::format(
+            "Invalid hostname '{}': must be RFC 952/1123 compliant (alphanumeric + "
+            "hyphens, max 63 chars per label, total max 253 chars)",
+            v[0]));
+  }
+  data_ = std::move(v);
+}
+
+CmdConfigHostnameTraits::RetType CmdConfigHostname::queryClient(
+    const HostInfo& /* hostInfo */,
+    const ObjectArgType& args) {
+  auto& session = ConfigSession::getInstance();
+  auto& config = session.getAgentConfig();
+  auto& swConfig = *config.sw();
+
+  const auto& newName = args.getName();
+
+  if (swConfig.hostname().has_value() && *swConfig.hostname() == newName) {
+    return fmt::format("Hostname is already set to '{}'", newName);
+  }
+
+  swConfig.hostname() = newName;
+
+  // hostname is applied hitlessly via ThriftConfigApplier::updateSwitchSettings
+  // (newSwitchSettings->setHostname); no SAI ChangeProhibited guard exists.
+  session.saveConfig(cli::ServiceType::AGENT, cli::ConfigActionLevel::HITLESS);
+
+  return fmt::format("Successfully set hostname to '{}'", newName);
+}
+
+void CmdConfigHostname::printOutput(const RetType& logMsg) {
+  std::cout << logMsg << std::endl;
+}
+
+// Explicit template instantiation
+template void CmdHandler<CmdConfigHostname, CmdConfigHostnameTraits>::run();
+
+} // namespace facebook::fboss

fboss/cli/fboss2/commands/config/switch/hostname/CmdConfigHostname.h

@@ -0,0 +1,54 @@
+/*
+ *  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 <string>
+#include <vector>
+
+#include "fboss/cli/fboss2/CmdHandler.h"
+#include "fboss/cli/fboss2/commands/config/switch/CmdConfigSwitch.h"
+
+namespace facebook::fboss {
+
+// Args: <name> — a non-empty string hostname (RFC 952/1123 compliant)
+class HostnameConfigArgs : public utils::BaseObjectArgType<std::string> {
+ public:
+  /* implicit */ HostnameConfigArgs( // NOLINT(google-explicit-constructor)
+      std::vector<std::string> v);
+
+  const std::string& getName() const {
+    return data_[0];
+  }
+};
+
+struct CmdConfigHostnameTraits : public WriteCommandTraits {
+  using ParentCmd = CmdConfigSwitch;
+  static void addCliArg(CLI::App& cmd, std::vector<std::string>& args) {
+    cmd.add_option("hostname", args, "Switch hostname")
+        ->required()
+        ->expected(1);
+  }
+  using ObjectArgType = HostnameConfigArgs;
+  using RetType = std::string;
+};
+
+class CmdConfigHostname
+    : public CmdHandler<CmdConfigHostname, CmdConfigHostnameTraits> {
+ public:
+  using ObjectArgType = CmdConfigHostnameTraits::ObjectArgType;
+  using RetType = CmdConfigHostnameTraits::RetType;
+
+  RetType queryClient(const HostInfo& hostInfo, const ObjectArgType& args);
+
+  void printOutput(const RetType& logMsg);
+};
+
+} // namespace facebook::fboss

fboss/cli/fboss2/test/config/BUCK

@@ -13,6 +13,7 @@ cpp_unittest(
         "CmdConfigCoppTest.cpp",
         "CmdConfigDhcpTest.cpp",
         "CmdConfigHistoryTest.cpp",
+        "CmdConfigHostnameTest.cpp",
         "CmdConfigInterfaceIpv6NdpTest.cpp",
         "CmdConfigInterfaceSwitchportAccessVlanTest.cpp",
         "CmdConfigInterfaceSwitchportTrunkAllowedVlanTest.cpp",

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

@@ -0,0 +1,150 @@
+/*
+ *  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 <stdexcept>
+#include <string>
+
+#include "fboss/cli/fboss2/commands/config/switch/hostname/CmdConfigHostname.h"
+#include "fboss/cli/fboss2/session/ConfigSession.h"
+#include "fboss/cli/fboss2/test/config/CmdConfigTestBase.h"
+#include "fboss/cli/fboss2/utils/HostInfo.h"
+
+using namespace ::testing;
+
+namespace facebook::fboss {
+
+class CmdConfigHostnameTestFixture : public CmdConfigTestBase {
+ public:
+  CmdConfigHostnameTestFixture()
+      : CmdConfigTestBase(
+            "fboss_hostname_config_test_%%%%-%%%%-%%%%-%%%%",
+            R"({
+  "sw": {
+    "hostname": "rsw001.p001.m002.qzr1.example.com"
+  }
+})") {}
+
+ protected:
+  const std::string cmdPrefix_ = "config hostname";
+};
+
+class CmdConfigHostnameAbsentTestFixture : public CmdConfigTestBase {
+ public:
+  CmdConfigHostnameAbsentTestFixture()
+      : CmdConfigTestBase(
+            "fboss_hostname_absent_config_test_%%%%-%%%%-%%%%-%%%%",
+            R"({
+  "sw": {}
+})") {}
+
+ protected:
+  const std::string cmdPrefix_ = "config hostname";
+};
+
+// ==============================================================================
+// HostnameConfigArgs validation
+// ==============================================================================
+
+TEST_F(CmdConfigHostnameTestFixture, argValidation_valid) {
+  // Valid: simple hostname
+  HostnameConfigArgs a({"myswitch"});
+  EXPECT_EQ(a.getName(), "myswitch");
+}
+
+TEST_F(CmdConfigHostnameTestFixture, argValidation_fqdn) {
+  // Valid: FQDN
+  HostnameConfigArgs a({"rsw001.p001.m002.qzr1.example.com"});
+  EXPECT_EQ(a.getName(), "rsw001.p001.m002.qzr1.example.com");
+}
+
+TEST_F(CmdConfigHostnameTestFixture, argValidation_empty) {
+  EXPECT_THROW(HostnameConfigArgs({}), std::invalid_argument);
+}
+
+TEST_F(CmdConfigHostnameTestFixture, argValidation_emptyString) {
+  EXPECT_THROW(HostnameConfigArgs({""}), std::invalid_argument);
+}
+
+TEST_F(CmdConfigHostnameTestFixture, argValidation_tooManyArgs) {
+  EXPECT_THROW(
+      HostnameConfigArgs({"myswitch", "extra"}), std::invalid_argument);
+}
+
+TEST_F(CmdConfigHostnameTestFixture, argValidation_invalidFormat) {
+  // Invalid: contains underscore (not RFC 952)
+  EXPECT_THROW(HostnameConfigArgs({"my_switch"}), std::invalid_argument);
+
+  // Invalid: leading hyphen
+  EXPECT_THROW(HostnameConfigArgs({"-invalid"}), std::invalid_argument);
+
+  // Invalid: trailing hyphen
+  EXPECT_THROW(HostnameConfigArgs({"invalid-"}), std::invalid_argument);
+
+  // Invalid: label too long (> 63 chars)
+  std::string longLabel(65, 'a');
+  EXPECT_THROW(HostnameConfigArgs({longLabel}), std::invalid_argument);
+
+  // Invalid: total length > 253
+  std::string tooLong(260, 'a');
+  EXPECT_THROW(HostnameConfigArgs({tooLong}), std::invalid_argument);
+}
+
+// ==============================================================================
+// queryClient tests
+// ==============================================================================
+
+TEST_F(CmdConfigHostnameTestFixture, setNewHostname) {
+  setupTestableConfigSession(cmdPrefix_, "newswitch.example.com");
+  CmdConfigHostname cmd;
+  HostInfo hostInfo("testhost");
+  HostnameConfigArgs arg({"newswitch.example.com"});
+
+  auto result = cmd.queryClient(hostInfo, arg);
+  EXPECT_THAT(result, HasSubstr("newswitch.example.com"));
+  EXPECT_THAT(result, HasSubstr("Successfully"));
+
+  auto& hostname =
+      *ConfigSession::getInstance().getAgentConfig().sw()->hostname();
+  EXPECT_EQ(hostname, "newswitch.example.com");
+}
+
+TEST_F(CmdConfigHostnameTestFixture, alreadySet) {
+  // Seed config already has this hostname
+  setupTestableConfigSession(cmdPrefix_, "rsw001.p001.m002.qzr1.example.com");
+  CmdConfigHostname cmd;
+  HostInfo hostInfo("testhost");
+  HostnameConfigArgs arg({"rsw001.p001.m002.qzr1.example.com"});
+
+  auto result = cmd.queryClient(hostInfo, arg);
+  EXPECT_THAT(result, HasSubstr("already"));
+}
+
+// ==============================================================================
+// Absent hostname fixture and tests
+// ==============================================================================
+
+TEST_F(CmdConfigHostnameAbsentTestFixture, setNewHostnameWhenAbsent) {
+  setupTestableConfigSession("config hostname", "newswitch.example.com");
+  CmdConfigHostname cmd;
+  HostInfo hostInfo("testhost");
+  HostnameConfigArgs arg({"newswitch.example.com"});
+
+  auto result = cmd.queryClient(hostInfo, arg);
+  EXPECT_THAT(result, HasSubstr("Successfully"));
+  EXPECT_THAT(result, HasSubstr("newswitch.example.com"));
+
+  auto& hostname =
+      *ConfigSession::getInstance().getAgentConfig().sw()->hostname();
+  EXPECT_EQ(hostname, "newswitch.example.com");
+}
+
+} // namespace facebook::fboss

fboss/cli/fboss2/test/integration_test/BUCK

@@ -21,6 +21,7 @@ cpp_binary(
         "ConfigConcurrentSessionsTest.cpp",
         "ConfigCoppTest.cpp",
         "ConfigDhcpSourceOverrideTest.cpp",
+        "ConfigHostnameTest.cpp",
         "ConfigInterfaceDescriptionTest.cpp",
         "ConfigInterfaceFlowControlTest.cpp",
         "ConfigInterfaceIpv6NdpTest.cpp",