Skip to content

Commit c85ed80

Browse files
NOS-6649..NOS-6656: fboss2 config/delete protocol bgp policy community-list
Adds `config protocol bgp policy community-list <name> [community <name>] [<attribute> <value> ...]` and `delete protocol bgp policy community-list <name>` — the second BGP policy object-type command family, beneath the `policy` grouping node alongside as-path-list. - community-list dispatcher with a two-level key (list name + inline community member name) writing bgp_policy.BgpPolicies.community_lists[] through the typed ConfigSession; boolean-operator maps to routing_policy.BooleanOperator, exact-match to the optional bool, and the member attributes (description/type/value) set the CommunityRefType union's inline Community arm, keyed by Community.name. - new boolAttr factory in the shared BgpCliAttrHandlers.h (generalizing the neighbor dispatcher's), reused by upcoming prefix-list. - fix a latent dangling-string_view in the shared enumAttr factory: the lambda captured a string_view over the caller's fmt::format temporary, so every invalid-enum-value rejection printed garbage after "expected". valueDesc is now taken (and captured) by value; the as-path-list and community-list rejection tests pin the full message. - unit tests (20) + commit-path integration tests mirroring ConfigBgpPolicyAsPathListTest (not yet run on a DUT). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 131988a commit c85ed80

17 files changed

Lines changed: 1242 additions & 27 deletions

cmake/CliFboss2.cmake

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -887,6 +887,9 @@ add_library(fboss2_config_lib
887887
fboss/cli/fboss2/commands/config/protocol/bgp/policy/as-path-list/CmdConfigProtocolBgpPolicyAsPathList.h
888888
fboss/cli/fboss2/commands/config/protocol/bgp/policy/as-path-list/entry/CmdConfigProtocolBgpPolicyAsPathListEntry.cpp
889889
fboss/cli/fboss2/commands/config/protocol/bgp/policy/as-path-list/entry/CmdConfigProtocolBgpPolicyAsPathListEntry.h
890+
fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/BgpCommunityListCliUtils.h
891+
fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.cpp
892+
fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h
890893
fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.cpp
891894
fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.h
892895
fboss/cli/fboss2/commands/config/ptp/transparent_clock/CmdConfigPtpTransparentClock.cpp
@@ -989,6 +992,8 @@ add_library(fboss2_config_lib
989992
fboss/cli/fboss2/commands/delete/protocol/bgp/policy/CmdDeleteProtocolBgpPolicy.h
990993
fboss/cli/fboss2/commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.cpp
991994
fboss/cli/fboss2/commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.h
995+
fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.cpp
996+
fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h
992997
fboss/cli/fboss2/commands/delete/protocol/CmdDeleteProtocol.h
993998
fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp
994999
fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.h

cmake/CliFboss2TestConfig.cmake

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ add_executable(fboss2_cmd_config_test
1111
fboss/cli/fboss2/test/config/CmdConfigIcmpV4UnavailableSrcAddrTest.cpp
1212
fboss/cli/fboss2/test/config/CmdConfigBgpNeighborTest.cpp
1313
fboss/cli/fboss2/test/config/CmdConfigBgpPeerGroupTest.cpp
14+
fboss/cli/fboss2/test/config/CmdConfigBgpPolicyAsPathListTest.cpp
15+
fboss/cli/fboss2/test/config/CmdConfigBgpPolicyCommunityListTest.cpp
1416
fboss/cli/fboss2/test/config/CmdConfigCoppTest.cpp
1517
fboss/cli/fboss2/test/config/CmdConfigDhcpTest.cpp
1618
fboss/cli/fboss2/test/config/CmdConfigHostnameTest.cpp
@@ -42,6 +44,8 @@ add_executable(fboss2_cmd_config_test
4244
fboss/cli/fboss2/test/config/CmdDeleteArpTest.cpp
4345
fboss/cli/fboss2/test/config/CmdDeleteBgpNeighborTest.cpp
4446
fboss/cli/fboss2/test/config/CmdDeleteBgpPeerGroupTest.cpp
47+
fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyAsPathListTest.cpp
48+
fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyCommunityListTest.cpp
4549
fboss/cli/fboss2/test/config/CmdDeleteConfigInterfaceTest.cpp
4650
fboss/cli/fboss2/test/config/CmdDeleteDhcpTest.cpp
4751
fboss/cli/fboss2/test/config/CmdDeleteInterfaceIpv6NdpTest.cpp

cmake/CliFboss2TestIntegrationTest.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ add_executable(fboss2_integration_test
1414
fboss/cli/fboss2/test/integration_test/ConfigBgpGlobalTest.cpp
1515
fboss/cli/fboss2/test/integration_test/ConfigBgpNeighborTest.cpp
1616
fboss/cli/fboss2/test/integration_test/ConfigBgpPeerGroupTest.cpp
17+
fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyCommunityListTest.cpp
1718
fboss/cli/fboss2/test/integration_test/ConfigBgpSessionTest.cpp
1819
fboss/cli/fboss2/test/integration_test/ConfigConcurrentSessionsTest.cpp
1920
fboss/cli/fboss2/test/integration_test/ConfigHostnameTest.cpp

fboss/cli/fboss2/BUCK

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,6 +1103,7 @@ cpp_library(
11031103
"commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.cpp",
11041104
"commands/config/protocol/bgp/policy/as-path-list/CmdConfigProtocolBgpPolicyAsPathList.cpp",
11051105
"commands/config/protocol/bgp/policy/as-path-list/entry/CmdConfigProtocolBgpPolicyAsPathListEntry.cpp",
1106+
"commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.cpp",
11061107
"commands/config/protocol/static/CmdConfigProtocolStatic.cpp",
11071108
"commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.cpp",
11081109
"commands/config/ptp/CmdConfigPtp.cpp",
@@ -1154,6 +1155,7 @@ cpp_library(
11541155
"commands/delete/protocol/bgp/peer-group/CmdDeleteProtocolBgpPeerGroup.cpp",
11551156
"commands/delete/protocol/bgp/policy/CmdDeleteProtocolBgpPolicy.cpp",
11561157
"commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.cpp",
1158+
"commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.cpp",
11571159
"commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp",
11581160
"commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.cpp",
11591161
"commands/delete/qos/CmdDeleteQos.cpp",
@@ -1216,6 +1218,8 @@ cpp_library(
12161218
"commands/config/protocol/bgp/policy/as-path-list/BgpAsPathListCliUtils.h",
12171219
"commands/config/protocol/bgp/policy/as-path-list/CmdConfigProtocolBgpPolicyAsPathList.h",
12181220
"commands/config/protocol/bgp/policy/as-path-list/entry/CmdConfigProtocolBgpPolicyAsPathListEntry.h",
1221+
"commands/config/protocol/bgp/policy/community-list/BgpCommunityListCliUtils.h",
1222+
"commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h",
12191223
"commands/config/protocol/static/CmdConfigProtocolStatic.h",
12201224
"commands/config/protocol/static/route/StaticRouteUtils.h",
12211225
"commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h",
@@ -1268,6 +1272,7 @@ cpp_library(
12681272
"commands/delete/protocol/bgp/peer-group/CmdDeleteProtocolBgpPeerGroup.h",
12691273
"commands/delete/protocol/bgp/policy/CmdDeleteProtocolBgpPolicy.h",
12701274
"commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.h",
1275+
"commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h",
12711276
"commands/delete/protocol/static/CmdDeleteProtocolStatic.h",
12721277
"commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h",
12731278
"commands/delete/qos/CmdDeleteQos.h",

fboss/cli/fboss2/CmdListConfig.cpp

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.h"
4444
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/as-path-list/CmdConfigProtocolBgpPolicyAsPathList.h"
4545
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/as-path-list/entry/CmdConfigProtocolBgpPolicyAsPathListEntry.h"
46+
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h"
4647
#include "fboss/cli/fboss2/commands/config/protocol/static/CmdConfigProtocolStatic.h"
4748
#include "fboss/cli/fboss2/commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h"
4849
#include "fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.h"
@@ -92,6 +93,7 @@
9293
#include "fboss/cli/fboss2/commands/delete/protocol/bgp/peer-group/CmdDeleteProtocolBgpPeerGroup.h"
9394
#include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/CmdDeleteProtocolBgpPolicy.h"
9495
#include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.h"
96+
#include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h"
9597
#include "fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.h"
9698
#include "fboss/cli/fboss2/commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h"
9799
#include "fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.h"
@@ -389,24 +391,34 @@ const CommandTree& kConfigCommandTree() {
389391
commandHandler<CmdConfigProtocolBgpPolicy>,
390392
argRegistrar<CmdConfigProtocolBgpPolicyTraits>,
391393
{{
392-
"as-path-list",
393-
"Configure BGP AS-path list: <name> "
394-
"[<attribute> <value> ...] (description)",
395-
commandHandler<
396-
CmdConfigProtocolBgpPolicyAsPathList>,
397-
argRegistrar<
398-
CmdConfigProtocolBgpPolicyAsPathListTraits>,
399-
{{
400-
"entry",
401-
"Configure an AS-path list entry: "
402-
"<seq-num> [<attribute> <value> ...] "
403-
"(asn-regexp, description, match-logic)",
404-
commandHandler<
405-
CmdConfigProtocolBgpPolicyAsPathListEntry>,
406-
argRegistrar<
407-
CmdConfigProtocolBgpPolicyAsPathListEntryTraits>,
408-
}},
409-
}},
394+
"as-path-list",
395+
"Configure BGP AS-path list: <name> "
396+
"[<attribute> <value> ...] (description)",
397+
commandHandler<
398+
CmdConfigProtocolBgpPolicyAsPathList>,
399+
argRegistrar<
400+
CmdConfigProtocolBgpPolicyAsPathListTraits>,
401+
{{
402+
"entry",
403+
"Configure an AS-path list entry: "
404+
"<seq-num> [<attribute> <value> ...] "
405+
"(asn-regexp, description, match-logic)",
406+
commandHandler<
407+
CmdConfigProtocolBgpPolicyAsPathListEntry>,
408+
argRegistrar<
409+
CmdConfigProtocolBgpPolicyAsPathListEntryTraits>,
410+
}},
411+
},
412+
{
413+
"community-list",
414+
"Configure BGP community-list: <name> "
415+
"[<attribute> <value> ...] "
416+
"(boolean-operator, description, exact-match)",
417+
commandHandler<
418+
CmdConfigProtocolBgpPolicyCommunityList>,
419+
argRegistrar<
420+
CmdConfigProtocolBgpPolicyCommunityListTraits>,
421+
}},
410422
},
411423
},
412424
},
@@ -711,12 +723,20 @@ const CommandTree& kConfigCommandTree() {
711723
commandHandler<CmdDeleteProtocolBgpPolicy>,
712724
argTypeHandler<CmdDeleteProtocolBgpPolicyTraits>,
713725
{{
714-
"as-path-list",
715-
"Delete a BGP AS-path list: <name>",
716-
commandHandler<CmdDeleteProtocolBgpPolicyAsPathList>,
717-
argRegistrar<
718-
CmdDeleteProtocolBgpPolicyAsPathListTraits>,
719-
}},
726+
"as-path-list",
727+
"Delete a BGP AS-path list: <name>",
728+
commandHandler<CmdDeleteProtocolBgpPolicyAsPathList>,
729+
argRegistrar<
730+
CmdDeleteProtocolBgpPolicyAsPathListTraits>,
731+
},
732+
{
733+
"community-list",
734+
"Delete a BGP community-list: <name>",
735+
commandHandler<
736+
CmdDeleteProtocolBgpPolicyCommunityList>,
737+
argRegistrar<
738+
CmdDeleteProtocolBgpPolicyCommunityListTraits>,
739+
}},
720740
}},
721741
},
722742
{

fboss/cli/fboss2/commands/config/protocol/bgp/BgpCliValueParsers.h

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,18 @@
1010

1111
#pragma once
1212

13+
#include <fmt/format.h>
1314
#include <folly/Conv.h>
15+
#include <cstddef>
1416
#include <cstdint>
1517
#include <exception>
1618
#include <limits>
1719
#include <optional>
20+
#include <stdexcept>
1821
#include <string>
22+
#include <string_view>
1923
#include <utility>
24+
#include <vector>
2025

2126
/**
2227
* Value parsing helpers shared by the BGP config dispatchers
@@ -93,4 +98,51 @@ inline std::optional<int64_t> parseAsn4Byte(const std::string& value) {
9398
return static_cast<int64_t>(*parsed);
9499
}
95100

101+
// ---- constructor-time token parsing ----------------------------------------
102+
// Unlike the value parsers above, this runs while constructing a command's
103+
// ObjectArgType, where throwing std::invalid_argument IS the framework's
104+
// error channel — the message is surfaced to the user as the parse error.
105+
106+
// The `<list-name> [<keyword> <member-name>]` prefix shared by the policy
107+
// list commands (config and delete): the list name, the optional named
108+
// member selected by `keyword`, and where any remaining tokens begin.
109+
struct ListMemberSelector {
110+
std::string listName;
111+
std::optional<std::string> memberName;
112+
// Index of the first token after the parsed prefix (== tokens.size() when
113+
// nothing follows). The config dispatcher reads an <attribute> <value>...
114+
// tail from here; the delete dispatcher rejects any tail.
115+
size_t restStart;
116+
};
117+
118+
// Parse the shared prefix. `objectName` is the list flavor for messages
119+
// (e.g. "community-list"), `memberKeyword` selects the nested member (e.g.
120+
// `community`), and `usage` is the whole-command usage line thrown when no
121+
// tokens were given. A second token other than `memberKeyword` is left to
122+
// the caller (restStart == 1): the config grammar treats it as a list-level
123+
// attribute, delete as an unexpected token.
124+
inline ListMemberSelector parseListMemberSelector(
125+
const std::vector<std::string>& tokens,
126+
std::string_view objectName,
127+
std::string_view memberKeyword,
128+
std::string_view usage) {
129+
if (tokens.empty()) {
130+
throw std::invalid_argument(std::string(usage));
131+
}
132+
if (tokens[0].empty()) {
133+
throw std::invalid_argument(
134+
fmt::format("Error: {} name must not be empty", objectName));
135+
}
136+
ListMemberSelector selector{tokens[0], std::nullopt, 1};
137+
if (tokens.size() > 1 && tokens[1] == memberKeyword) {
138+
if (tokens.size() < 3 || tokens[2].empty()) {
139+
throw std::invalid_argument(
140+
fmt::format("Error: `{}` requires a <name>", memberKeyword));
141+
}
142+
selector.memberName = tokens[2];
143+
selector.restStart = 3;
144+
}
145+
return selector;
146+
}
147+
96148
} // namespace facebook::fboss::bgpcli

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,18 @@
1010

1111
#pragma once
1212

13+
#include <string>
14+
#include <variant>
1315
#include "fboss/cli/fboss2/CmdHandler.h"
1416
#include "fboss/cli/fboss2/commands/config/protocol/bgp/CmdConfigProtocolBgp.h"
17+
#include "fboss/cli/fboss2/utils/HostInfo.h"
1518

1619
namespace facebook::fboss {
1720

1821
// Grouping node for `config protocol bgp policy <object-type> ...`. Holds no
19-
// state of its own; the per-object-type dispatchers (as-path-list, and later
20-
// community-list/prefix-list/routing-policy) are its subcommands. Mirrors
21-
// CmdConfigProtocolBgp.
22+
// state of its own; the per-object-type dispatchers (as-path-list,
23+
// community-list, and later prefix-list/routing-policy) are its subcommands.
24+
// Mirrors CmdConfigProtocolBgp.
2225
struct CmdConfigProtocolBgpPolicyTraits : public WriteCommandTraits {
2326
using ParentCmd = CmdConfigProtocolBgp;
2427
using ObjectArgType = std::monostate;
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
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+
#pragma once
12+
13+
#include <neteng/fboss/bgp/public_tld/configerator/structs/neteng/fboss/bgp/gen-cpp2/bgp_config_types.h>
14+
#include <algorithm>
15+
#include <string>
16+
#include "configerator/structs/neteng/bgp_policy/thrift/gen-cpp2/bgp_policy_types.h"
17+
18+
/**
19+
* Lookup/create helpers for the community-list CLI family, shared between the
20+
* list-level dispatcher (CmdConfigProtocolBgpPolicyCommunityList), the
21+
* community subcommand (CmdConfigProtocolBgpPolicyCommunityListCommunity), and
22+
* the delete counterparts. A CommunityList is keyed by name; the inline
23+
* Community members it holds are keyed by their own name.
24+
*/
25+
namespace facebook::fboss::bgpcli {
26+
27+
inline bool communityListExists(
28+
const bgp::thrift::BgpConfig& cfg,
29+
const std::string& name) {
30+
if (!cfg.policies().has_value()) {
31+
return false;
32+
}
33+
const auto& lists = *cfg.policies()->community_lists();
34+
return std::any_of(lists.begin(), lists.end(), [&](const auto& list) {
35+
return *list.name() == name;
36+
});
37+
}
38+
39+
// Find the community-list keyed by name, creating it if absent. Setting an
40+
// attribute on a not-yet-created list implicitly creates it, so command
41+
// ordering stays forgiving; a bare `community-list <name>` creates one
42+
// explicitly. CommunityList's only key field is `name`.
43+
inline bgp::bgp_policy::CommunityList& findOrCreateCommunityList(
44+
bgp::thrift::BgpConfig& cfg,
45+
const std::string& name) {
46+
auto& lists = *cfg.policies().ensure().community_lists();
47+
for (auto& list : lists) {
48+
if (*list.name() == name) {
49+
return list;
50+
}
51+
}
52+
lists.emplace_back();
53+
auto& list = lists.back();
54+
list.name() = name;
55+
return list;
56+
}
57+
58+
inline bool communityMemberExists(
59+
const bgp::bgp_policy::CommunityList& list,
60+
const std::string& name) {
61+
if (!list.members().has_value()) {
62+
return false;
63+
}
64+
const auto& members = *list.members();
65+
return std::any_of(members.begin(), members.end(), [&](const auto& member) {
66+
return member.community_ref().has_value() &&
67+
*member.community_ref()->name() == name;
68+
});
69+
}
70+
71+
// Find the inline Community member keyed by name within a list, creating it
72+
// if absent. The CLI always defines members inline (the CommunityRefType
73+
// union's `community` arm); the member's name is its identity.
74+
inline bgp::bgp_policy::Community& findOrCreateCommunityMember(
75+
bgp::bgp_policy::CommunityList& list,
76+
const std::string& name) {
77+
auto& members = list.members().ensure();
78+
for (auto& member : members) {
79+
if (member.community_ref().has_value() &&
80+
*member.community_ref()->name() == name) {
81+
return *member.community_ref();
82+
}
83+
}
84+
members.emplace_back();
85+
auto& community = members.back().set_community();
86+
community.name() = name;
87+
return community;
88+
}
89+
90+
} // namespace facebook::fboss::bgpcli

0 commit comments

Comments
 (0)