[Nexthop][fboss2-dev] fboss2 config/delete protocol bgp policy routing-policy commands

cmake/CliFboss2.cmake

@@ -984,6 +984,8 @@ add_library(fboss2_config_lib
   fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.h
   fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.cpp
   fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.h
+  fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.cpp
+  fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.h
   fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.cpp
   fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.h
   fboss/cli/fboss2/commands/config/ptp/transparent_clock/CmdConfigPtpTransparentClock.cpp
@@ -1128,6 +1130,8 @@ add_library(fboss2_config_lib
   fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h
   fboss/cli/fboss2/commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.cpp
   fboss/cli/fboss2/commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.h
+  fboss/cli/fboss2/commands/delete/protocol/bgp/policy/routing-policy/CmdDeleteProtocolBgpPolicyRoutingPolicy.cpp
+  fboss/cli/fboss2/commands/delete/protocol/bgp/policy/routing-policy/CmdDeleteProtocolBgpPolicyRoutingPolicy.h
   fboss/cli/fboss2/commands/delete/protocol/CmdDeleteProtocol.h
   fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp
   fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.h

cmake/CliFboss2TestConfig.cmake

@@ -18,6 +18,7 @@ add_executable(fboss2_cmd_config_test
   fboss/cli/fboss2/test/config/CmdConfigBgpPolicyCommunityListTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigBgpPolicyPrefixListTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigBgpPolicyPrefixListEntryTest.cpp
+  fboss/cli/fboss2/test/config/CmdConfigBgpPolicyRoutingPolicyTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigCoppTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigDataPlaneTrafficPolicyTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigDhcpTest.cpp
@@ -58,6 +59,7 @@ add_executable(fboss2_cmd_config_test
   fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyAsPathListTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyCommunityListTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyPrefixListTest.cpp
+  fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyRoutingPolicyTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteConfigInterfaceTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteCoppQueueTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteCoppReasonTest.cpp

cmake/CliFboss2TestIntegrationTest.cmake

@@ -22,6 +22,7 @@ add_executable(fboss2_integration_test
   fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyCommunityListTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyPrefixListTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyPrefixListEntryTest.cpp
+  fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyRoutingPolicyTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigBgpSessionTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigConcurrentSessionsTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigHostnameTest.cpp

fboss/cli/fboss2/BUCK

@@ -1218,6 +1218,7 @@ cpp_library(
         "commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.cpp",
         "commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.cpp",
         "commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.cpp",
+        "commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.cpp",
         "commands/config/protocol/static/CmdConfigProtocolStatic.cpp",
         "commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.cpp",
         "commands/config/ptp/CmdConfigPtp.cpp",
@@ -1289,6 +1290,7 @@ cpp_library(
         "commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.cpp",
         "commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.cpp",
         "commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.cpp",
+        "commands/delete/protocol/bgp/policy/routing-policy/CmdDeleteProtocolBgpPolicyRoutingPolicy.cpp",
         "commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp",
         "commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.cpp",
         "commands/delete/qos/CmdDeleteQos.cpp",
@@ -1370,6 +1372,7 @@ cpp_library(
         "commands/config/protocol/bgp/policy/prefix-list/BgpPrefixListCliUtils.h",
         "commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.h",
         "commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.h",
+        "commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.h",
         "commands/config/protocol/static/CmdConfigProtocolStatic.h",
         "commands/config/protocol/static/route/StaticRouteUtils.h",
         "commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h",
@@ -1442,6 +1445,7 @@ cpp_library(
         "commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.h",
         "commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h",
         "commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.h",
+        "commands/delete/protocol/bgp/policy/routing-policy/CmdDeleteProtocolBgpPolicyRoutingPolicy.h",
         "commands/delete/protocol/static/CmdDeleteProtocolStatic.h",
         "commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h",
         "commands/delete/qos/CmdDeleteQos.h",

fboss/cli/fboss2/CmdListConfig.cpp

@@ -47,6 +47,7 @@
 #include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h"
 #include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.h"
 #include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.h"
+#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.h"
 #include "fboss/cli/fboss2/commands/config/protocol/static/CmdConfigProtocolStatic.h"
 #include "fboss/cli/fboss2/commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h"
 #include "fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.h"
@@ -115,6 +116,7 @@
 #include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.h"
 #include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h"
 #include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.h"
+#include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/routing-policy/CmdDeleteProtocolBgpPolicyRoutingPolicy.h"
 #include "fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.h"
 #include "fboss/cli/fboss2/commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h"
 #include "fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.h"
@@ -470,6 +472,16 @@ const CommandTree& kConfigCommandTree() {
                                    argRegistrar<
                                        CmdConfigProtocolBgpPolicyPrefixListEntryTraits>,
                                }},
+                           },
+                           {
+                               "routing-policy",
+                               "Configure BGP routing-policy: <name> "
+                               "[<attribute> <value> ...] "
+                               "(description)",
+                               commandHandler<
+                                   CmdConfigProtocolBgpPolicyRoutingPolicy>,
+                               argRegistrar<
+                                   CmdConfigProtocolBgpPolicyRoutingPolicyTraits>,
                            }},
                       },
                   },
@@ -842,6 +854,14 @@ const CommandTree& kConfigCommandTree() {
                          commandHandler<CmdDeleteProtocolBgpPolicyPrefixList>,
                          argRegistrar<
                              CmdDeleteProtocolBgpPolicyPrefixListTraits>,
+                     },
+                     {
+                         "routing-policy",
+                         "Delete a BGP routing-policy: <name>",
+                         commandHandler<
+                             CmdDeleteProtocolBgpPolicyRoutingPolicy>,
+                         argRegistrar<
+                             CmdDeleteProtocolBgpPolicyRoutingPolicyTraits>,
                      }},
                 }},
            },

fboss/cli/fboss2/commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.h

@@ -20,7 +20,7 @@ namespace facebook::fboss {
 
 // Grouping node for `config protocol bgp policy <object-type> ...`. Holds no
 // state of its own; the per-object-type dispatchers (as-path-list,
-// community-list, prefix-list, and later routing-policy) are its subcommands.
+// community-list, prefix-list, routing-policy) are its subcommands.
 // Mirrors CmdConfigProtocolBgp.
 struct CmdConfigProtocolBgpPolicyTraits : public WriteCommandTraits {
   using ParentCmd = CmdConfigProtocolBgp;

fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.cpp

@@ -0,0 +1,195 @@
+/*
+ *  Copyright (c) Meta Platforms, Inc. and affiliates.
+ *  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/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.h"
+
+#include "fboss/cli/fboss2/CmdHandler.cpp"
+
+#include <fmt/core.h>
+#include <neteng/fboss/bgp/public_tld/configerator/structs/neteng/fboss/bgp/gen-cpp2/bgp_config_types.h>
+#include <functional>
+#include <iostream>
+#include <map>
+#include <ostream>
+#include <stdexcept>
+#include <string>
+#include <string_view>
+#include <vector>
+#include "configerator/structs/neteng/bgp_policy/thrift/gen-cpp2/bgp_policy_types.h"
+#include "fboss/cli/fboss2/commands/config/protocol/bgp/BgpCliAttrHandlers.h"
+#include "fboss/cli/fboss2/commands/config/protocol/bgp/BgpCliValueParsers.h"
+#include "fboss/cli/fboss2/session/ConfigSession.h"
+#include "fboss/cli/fboss2/utils/CmdUtilsCommon.h"
+#include "fboss/cli/fboss2/utils/HostInfo.h"
+#include "fmt/format.h"
+
+namespace facebook::fboss {
+
+namespace {
+
+// The attribute names, exactly as documented. Kept here so the valid-attribute
+// set and the handler table stay in sync.
+constexpr std::string_view kObjectName = "routing-policy";
+constexpr std::string_view kDescription = "description";
+
+using BgpPolicyStatement = bgp::bgp_policy::BgpPolicyStatement;
+using bgpcli::AttrHandler;
+using bgpcli::joinedStringAttr;
+using bgpcli::ok;
+using bgpcli::Result;
+
+// ---- policy-level setters ---------------------------------------------------
+// Each writes one already-parsed, already-validated value. Parsing and message
+// text belong to the shared factories in BgpCliAttrHandlers.h.
+
+void setDescription(
+    BgpPolicyStatement& policy,
+    const std::string& description) {
+  policy.description() = description;
+}
+
+// ---- policy-level attribute registry ---------------------------------------
+// One line per documented attribute: its dispatch key, its value shape, and
+// the setter that stores it.
+const std::map<std::string, AttrHandler<BgpPolicyStatement>, std::less<>>&
+policyAttrHandlers() {
+  static const std::
+      map<std::string, AttrHandler<BgpPolicyStatement>, std::less<>>
+          kHandlers = {
+              {std::string(kDescription),
+               joinedStringAttr<BgpPolicyStatement>(
+                   kDescription, setDescription)},
+          };
+  return kHandlers;
+}
+
+std::string validAttrList() {
+  std::string out;
+  for (const auto& [name, _] : policyAttrHandlers()) {
+    if (!out.empty()) {
+      out += ", ";
+    }
+    out += name;
+  }
+  return out;
+}
+
+// Find the routing-policy statement keyed by name, creating it if absent.
+// Setting an attribute on a not-yet-created policy implicitly creates it, so
+// command ordering stays forgiving; a bare `routing-policy <name>` creates one
+// explicitly. BgpPolicyStatement's only key field is `name`.
+BgpPolicyStatement& findOrCreatePolicy(
+    bgp::thrift::BgpConfig& cfg,
+    const std::string& name) {
+  auto& policies = *cfg.policies().ensure().bgp_policy_statements();
+  for (auto& policy : policies) {
+    if (*policy.name() == name) {
+      return policy;
+    }
+  }
+  policies.emplace_back();
+  auto& policy = policies.back();
+  policy.name() = name;
+  return policy;
+}
+
+bool policyExists(const bgp::thrift::BgpConfig& cfg, const std::string& name) {
+  if (!cfg.policies().has_value()) {
+    return false;
+  }
+  for (const auto& policy : *cfg.policies()->bgp_policy_statements()) {
+    if (*policy.name() == name) {
+      return true;
+    }
+  }
+  return false;
+}
+
+} // namespace
+
+// Parse + validate at construction so queryClient stays a thin dispatch.
+// Throwing std::invalid_argument is how the framework surfaces arg parse
+// errors (same mechanism as BgpCommunityListConfig).
+BgpRoutingPolicyConfig::BgpRoutingPolicyConfig(std::vector<std::string> v)
+    : utils::BaseObjectArgType<std::string>(v) {
+  if (v.empty()) {
+    throw std::invalid_argument(
+        "Error: routing-policy <name> is required, optionally followed by "
+        "an <attribute> <value>");
+  }
+  if (v[0].empty()) {
+    throw std::invalid_argument(
+        fmt::format("Error: {} name must not be empty", kObjectName));
+  }
+  policyName_ = v[0];
+  if (v.size() == 1) {
+    return; // bare `routing-policy <name>`: create it
+  }
+
+  attr_ = v[1];
+  values_.assign(v.begin() + 2, v.end());
+
+  if (policyAttrHandlers().find(attr_) == policyAttrHandlers().end()) {
+    throw std::invalid_argument(
+        fmt::format(
+            "Error: unknown routing-policy attribute '{}'. Valid "
+            "attributes: {}",
+            attr_,
+            validAttrList()));
+  }
+}
+
+CmdConfigProtocolBgpPolicyRoutingPolicyTraits::RetType
+CmdConfigProtocolBgpPolicyRoutingPolicy::queryClient(
+    const HostInfo& /* hostInfo */,
+    const ObjectArgType& args) {
+  auto& session = ConfigSession::getInstance();
+  auto& cfg = session.getBgpConfig();
+  const bool policyCreated = !policyExists(cfg, args.policyName());
+  auto& policy = findOrCreatePolicy(cfg, args.policyName());
+
+  Result result = args.attr().empty()
+      ? ok(policyCreated
+               ? fmt::format(
+                     "Successfully created BGP routing-policy {}",
+                     args.policyName())
+               : fmt::format(
+                     "BGP routing-policy {} already exists", args.policyName()))
+      // The attribute is guaranteed valid: BgpRoutingPolicyConfig's
+      // constructor rejects an unknown attribute before we get here.
+      : policyAttrHandlers().find(args.attr())->second(policy, args.values());
+
+  if (result.ok) {
+    if (!args.attr().empty()) {
+      result.message +=
+          fmt::format(" for routing-policy {}", args.policyName());
+    }
+    session.saveBgpConfig();
+    result.message +=
+        fmt::format("\nConfig saved to: {}", session.getBgpSessionConfigPath());
+  } else if (policyCreated) {
+    // Drop the phantom policy so a rejected value is not visible to later
+    // lookups in the same process.
+    cfg.policies()->bgp_policy_statements()->pop_back();
+  }
+  return result.message;
+}
+
+void CmdConfigProtocolBgpPolicyRoutingPolicy::printOutput(
+    const RetType& output) {
+  std::cout << output << std::endl;
+}
+
+// Explicit template instantiation
+template void CmdHandler<
+    CmdConfigProtocolBgpPolicyRoutingPolicy,
+    CmdConfigProtocolBgpPolicyRoutingPolicyTraits>::run();
+
+} // namespace facebook::fboss