Skip to content

Commit cb867d4

Browse files
NOS-6671, NOS-6689: fboss2 config/delete protocol bgp policy routing-policy
Adds `config protocol bgp policy routing-policy <name> [<attribute> <value> ...]` and `delete protocol bgp policy routing-policy <name>` — the fourth BGP policy object-type command family, beneath the `policy` grouping node alongside as-path-list, community-list, and prefix-list. - routing-policy dispatcher keyed by policy name writing bgp_policy.BgpPolicies.bgp_policy_statements[] (BgpPolicyStatement) through the typed ConfigSession. Policy level: description (joined multi-token string, NOS-6671); bare create (NOS-6689). The seq-num-keyed `term <seq-num>` level and its match/action attributes land as follow-ups. - delete removes the whole policy statement by name; extra tokens are rejected. - unit tests (11) + commit-path integration tests mirroring ConfigBgpPolicyCommunityListTest, verifying the description lands in bgpd's running config and that bgpd adopts a policy statement with no terms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0b0313e commit cb867d4

15 files changed

Lines changed: 873 additions & 1 deletion

cmake/CliFboss2.cmake

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,8 @@ add_library(fboss2_config_lib
897897
fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.h
898898
fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.cpp
899899
fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.h
900+
fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.cpp
901+
fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.h
900902
fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.cpp
901903
fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.h
902904
fboss/cli/fboss2/commands/config/ptp/transparent_clock/CmdConfigPtpTransparentClock.cpp
@@ -1003,6 +1005,8 @@ add_library(fboss2_config_lib
10031005
fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h
10041006
fboss/cli/fboss2/commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.cpp
10051007
fboss/cli/fboss2/commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.h
1008+
fboss/cli/fboss2/commands/delete/protocol/bgp/policy/routing-policy/CmdDeleteProtocolBgpPolicyRoutingPolicy.cpp
1009+
fboss/cli/fboss2/commands/delete/protocol/bgp/policy/routing-policy/CmdDeleteProtocolBgpPolicyRoutingPolicy.h
10061010
fboss/cli/fboss2/commands/delete/protocol/CmdDeleteProtocol.h
10071011
fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp
10081012
fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.h

cmake/CliFboss2TestConfig.cmake

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ add_executable(fboss2_cmd_config_test
1414
fboss/cli/fboss2/test/config/CmdConfigBgpPolicyAsPathListTest.cpp
1515
fboss/cli/fboss2/test/config/CmdConfigBgpPolicyCommunityListTest.cpp
1616
fboss/cli/fboss2/test/config/CmdConfigBgpPolicyPrefixListTest.cpp
17+
fboss/cli/fboss2/test/config/CmdConfigBgpPolicyRoutingPolicyTest.cpp
1718
fboss/cli/fboss2/test/config/CmdConfigCoppTest.cpp
1819
fboss/cli/fboss2/test/config/CmdConfigDhcpTest.cpp
1920
fboss/cli/fboss2/test/config/CmdConfigHostnameTest.cpp
@@ -48,6 +49,7 @@ add_executable(fboss2_cmd_config_test
4849
fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyAsPathListTest.cpp
4950
fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyCommunityListTest.cpp
5051
fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyPrefixListTest.cpp
52+
fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyRoutingPolicyTest.cpp
5153
fboss/cli/fboss2/test/config/CmdDeleteConfigInterfaceTest.cpp
5254
fboss/cli/fboss2/test/config/CmdDeleteDhcpTest.cpp
5355
fboss/cli/fboss2/test/config/CmdDeleteInterfaceIpv6NdpTest.cpp

cmake/CliFboss2TestIntegrationTest.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ add_executable(fboss2_integration_test
1616
fboss/cli/fboss2/test/integration_test/ConfigBgpPeerGroupTest.cpp
1717
fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyCommunityListTest.cpp
1818
fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyPrefixListTest.cpp
19+
fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyRoutingPolicyTest.cpp
1920
fboss/cli/fboss2/test/integration_test/ConfigBgpSessionTest.cpp
2021
fboss/cli/fboss2/test/integration_test/ConfigConcurrentSessionsTest.cpp
2122
fboss/cli/fboss2/test/integration_test/ConfigHostnameTest.cpp

fboss/cli/fboss2/BUCK

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,7 @@ cpp_library(
11071107
"commands/config/protocol/bgp/policy/community-list/community/CmdConfigProtocolBgpPolicyCommunityListCommunity.cpp",
11081108
"commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.cpp",
11091109
"commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.cpp",
1110+
"commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.cpp",
11101111
"commands/config/protocol/static/CmdConfigProtocolStatic.cpp",
11111112
"commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.cpp",
11121113
"commands/config/ptp/CmdConfigPtp.cpp",
@@ -1160,6 +1161,7 @@ cpp_library(
11601161
"commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.cpp",
11611162
"commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.cpp",
11621163
"commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.cpp",
1164+
"commands/delete/protocol/bgp/policy/routing-policy/CmdDeleteProtocolBgpPolicyRoutingPolicy.cpp",
11631165
"commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp",
11641166
"commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.cpp",
11651167
"commands/delete/qos/CmdDeleteQos.cpp",
@@ -1228,6 +1230,7 @@ cpp_library(
12281230
"commands/config/protocol/bgp/policy/prefix-list/BgpPrefixListCliUtils.h",
12291231
"commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.h",
12301232
"commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.h",
1233+
"commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.h",
12311234
"commands/config/protocol/static/CmdConfigProtocolStatic.h",
12321235
"commands/config/protocol/static/route/StaticRouteUtils.h",
12331236
"commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h",
@@ -1282,6 +1285,7 @@ cpp_library(
12821285
"commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.h",
12831286
"commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h",
12841287
"commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.h",
1288+
"commands/delete/protocol/bgp/policy/routing-policy/CmdDeleteProtocolBgpPolicyRoutingPolicy.h",
12851289
"commands/delete/protocol/static/CmdDeleteProtocolStatic.h",
12861290
"commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h",
12871291
"commands/delete/qos/CmdDeleteQos.h",

fboss/cli/fboss2/CmdListConfig.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/community/CmdConfigProtocolBgpPolicyCommunityListCommunity.h"
4848
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.h"
4949
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.h"
50+
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.h"
5051
#include "fboss/cli/fboss2/commands/config/protocol/static/CmdConfigProtocolStatic.h"
5152
#include "fboss/cli/fboss2/commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h"
5253
#include "fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.h"
@@ -98,6 +99,7 @@
9899
#include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.h"
99100
#include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h"
100101
#include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.h"
102+
#include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/routing-policy/CmdDeleteProtocolBgpPolicyRoutingPolicy.h"
101103
#include "fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.h"
102104
#include "fboss/cli/fboss2/commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h"
103105
#include "fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.h"
@@ -455,6 +457,16 @@ const CommandTree& kConfigCommandTree() {
455457
argRegistrar<
456458
CmdConfigProtocolBgpPolicyPrefixListEntryTraits>,
457459
}},
460+
},
461+
{
462+
"routing-policy",
463+
"Configure BGP routing-policy: <name> "
464+
"[<attribute> <value> ...] "
465+
"(description)",
466+
commandHandler<
467+
CmdConfigProtocolBgpPolicyRoutingPolicy>,
468+
argRegistrar<
469+
CmdConfigProtocolBgpPolicyRoutingPolicyTraits>,
458470
}},
459471
},
460472
},
@@ -781,6 +793,14 @@ const CommandTree& kConfigCommandTree() {
781793
commandHandler<CmdDeleteProtocolBgpPolicyPrefixList>,
782794
argRegistrar<
783795
CmdDeleteProtocolBgpPolicyPrefixListTraits>,
796+
},
797+
{
798+
"routing-policy",
799+
"Delete a BGP routing-policy: <name>",
800+
commandHandler<
801+
CmdDeleteProtocolBgpPolicyRoutingPolicy>,
802+
argRegistrar<
803+
CmdDeleteProtocolBgpPolicyRoutingPolicyTraits>,
784804
}},
785805
}},
786806
},

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace facebook::fboss {
2020

2121
// Grouping node for `config protocol bgp policy <object-type> ...`. Holds no
2222
// state of its own; the per-object-type dispatchers (as-path-list,
23-
// community-list, prefix-list, and later routing-policy) are its subcommands.
23+
// community-list, prefix-list, routing-policy) are its subcommands.
2424
// Mirrors CmdConfigProtocolBgp.
2525
struct CmdConfigProtocolBgpPolicyTraits : public WriteCommandTraits {
2626
using ParentCmd = CmdConfigProtocolBgp;
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
/*
2+
* Copyright (c) 2004-present, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*
9+
*/
10+
11+
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.h"
12+
13+
#include "fboss/cli/fboss2/CmdHandler.cpp"
14+
15+
#include <fmt/core.h>
16+
#include <neteng/fboss/bgp/public_tld/configerator/structs/neteng/fboss/bgp/gen-cpp2/bgp_config_types.h>
17+
#include <functional>
18+
#include <iostream>
19+
#include <map>
20+
#include <ostream>
21+
#include <stdexcept>
22+
#include <string>
23+
#include <string_view>
24+
#include <vector>
25+
#include "configerator/structs/neteng/bgp_policy/thrift/gen-cpp2/bgp_policy_types.h"
26+
#include "fboss/cli/fboss2/commands/config/protocol/bgp/BgpCliAttrHandlers.h"
27+
#include "fboss/cli/fboss2/commands/config/protocol/bgp/BgpCliValueParsers.h"
28+
#include "fboss/cli/fboss2/session/ConfigSession.h"
29+
#include "fboss/cli/fboss2/utils/CmdUtilsCommon.h"
30+
#include "fboss/cli/fboss2/utils/HostInfo.h"
31+
#include "fmt/format.h"
32+
33+
namespace facebook::fboss {
34+
35+
namespace {
36+
37+
// The attribute names, exactly as documented. Kept here so the valid-attribute
38+
// set and the handler table stay in sync.
39+
constexpr std::string_view kObjectName = "routing-policy";
40+
constexpr std::string_view kDescription = "description";
41+
42+
using BgpPolicyStatement = bgp::bgp_policy::BgpPolicyStatement;
43+
using bgpcli::AttrHandler;
44+
using bgpcli::joinedStringAttr;
45+
using bgpcli::ok;
46+
using bgpcli::Result;
47+
48+
// ---- policy-level setters ---------------------------------------------------
49+
// Each writes one already-parsed, already-validated value. Parsing and message
50+
// text belong to the shared factories in BgpCliAttrHandlers.h.
51+
52+
void setDescription(
53+
BgpPolicyStatement& policy,
54+
const std::string& description) {
55+
policy.description() = description;
56+
}
57+
58+
// ---- policy-level attribute registry ---------------------------------------
59+
// One line per documented attribute: its dispatch key, its value shape, and
60+
// the setter that stores it.
61+
const std::map<std::string, AttrHandler<BgpPolicyStatement>, std::less<>>&
62+
policyAttrHandlers() {
63+
static const std::
64+
map<std::string, AttrHandler<BgpPolicyStatement>, std::less<>>
65+
kHandlers = {
66+
{std::string(kDescription),
67+
joinedStringAttr<BgpPolicyStatement>(
68+
kDescription, setDescription)},
69+
};
70+
return kHandlers;
71+
}
72+
73+
std::string validAttrList() {
74+
std::string out;
75+
for (const auto& [name, _] : policyAttrHandlers()) {
76+
if (!out.empty()) {
77+
out += ", ";
78+
}
79+
out += name;
80+
}
81+
return out;
82+
}
83+
84+
// Find the routing-policy statement keyed by name, creating it if absent.
85+
// Setting an attribute on a not-yet-created policy implicitly creates it, so
86+
// command ordering stays forgiving; a bare `routing-policy <name>` creates one
87+
// explicitly. BgpPolicyStatement's only key field is `name`.
88+
BgpPolicyStatement& findOrCreatePolicy(
89+
bgp::thrift::BgpConfig& cfg,
90+
const std::string& name) {
91+
auto& policies = *cfg.policies().ensure().bgp_policy_statements();
92+
for (auto& policy : policies) {
93+
if (*policy.name() == name) {
94+
return policy;
95+
}
96+
}
97+
policies.emplace_back();
98+
auto& policy = policies.back();
99+
policy.name() = name;
100+
return policy;
101+
}
102+
103+
bool policyExists(const bgp::thrift::BgpConfig& cfg, const std::string& name) {
104+
if (!cfg.policies().has_value()) {
105+
return false;
106+
}
107+
for (const auto& policy : *cfg.policies()->bgp_policy_statements()) {
108+
if (*policy.name() == name) {
109+
return true;
110+
}
111+
}
112+
return false;
113+
}
114+
115+
} // namespace
116+
117+
// Parse + validate at construction so queryClient stays a thin dispatch.
118+
// Throwing std::invalid_argument is how the framework surfaces arg parse
119+
// errors (same mechanism as BgpCommunityListConfig).
120+
BgpRoutingPolicyConfig::BgpRoutingPolicyConfig(std::vector<std::string> v)
121+
: utils::BaseObjectArgType<std::string>(v) {
122+
if (v.empty()) {
123+
throw std::invalid_argument(
124+
"Error: routing-policy <name> is required, optionally followed by "
125+
"an <attribute> <value>");
126+
}
127+
if (v[0].empty()) {
128+
throw std::invalid_argument(
129+
fmt::format("Error: {} name must not be empty", kObjectName));
130+
}
131+
policyName_ = v[0];
132+
if (v.size() == 1) {
133+
return; // bare `routing-policy <name>`: create it
134+
}
135+
136+
attr_ = v[1];
137+
values_.assign(v.begin() + 2, v.end());
138+
139+
if (policyAttrHandlers().find(attr_) == policyAttrHandlers().end()) {
140+
throw std::invalid_argument(
141+
fmt::format(
142+
"Error: unknown routing-policy attribute '{}'. Valid "
143+
"attributes: {}",
144+
attr_,
145+
validAttrList()));
146+
}
147+
}
148+
149+
CmdConfigProtocolBgpPolicyRoutingPolicyTraits::RetType
150+
CmdConfigProtocolBgpPolicyRoutingPolicy::queryClient(
151+
const HostInfo& /* hostInfo */,
152+
const ObjectArgType& args) {
153+
auto& session = ConfigSession::getInstance();
154+
auto& cfg = session.getBgpConfig();
155+
const bool policyCreated = !policyExists(cfg, args.policyName());
156+
auto& policy = findOrCreatePolicy(cfg, args.policyName());
157+
158+
Result result = args.attr().empty()
159+
? ok(policyCreated
160+
? fmt::format(
161+
"Successfully created BGP routing-policy {}",
162+
args.policyName())
163+
: fmt::format(
164+
"BGP routing-policy {} already exists", args.policyName()))
165+
// The attribute is guaranteed valid: BgpRoutingPolicyConfig's
166+
// constructor rejects an unknown attribute before we get here.
167+
: policyAttrHandlers().find(args.attr())->second(policy, args.values());
168+
169+
if (result.ok) {
170+
if (!args.attr().empty()) {
171+
result.message +=
172+
fmt::format(" for routing-policy {}", args.policyName());
173+
}
174+
session.saveBgpConfig();
175+
result.message +=
176+
fmt::format("\nConfig saved to: {}", session.getBgpSessionConfigPath());
177+
} else if (policyCreated) {
178+
// Drop the phantom policy so a rejected value is not visible to later
179+
// lookups in the same process.
180+
cfg.policies()->bgp_policy_statements()->pop_back();
181+
}
182+
return result.message;
183+
}
184+
185+
void CmdConfigProtocolBgpPolicyRoutingPolicy::printOutput(
186+
const RetType& output) {
187+
std::cout << output << std::endl;
188+
}
189+
190+
// Explicit template instantiation
191+
template void CmdHandler<
192+
CmdConfigProtocolBgpPolicyRoutingPolicy,
193+
CmdConfigProtocolBgpPolicyRoutingPolicyTraits>::run();
194+
195+
} // namespace facebook::fboss

0 commit comments

Comments
 (0)