diff --git a/cmake/CliFboss2.cmake b/cmake/CliFboss2.cmake index b7dcfabcd1219..7550836221954 100644 --- a/cmake/CliFboss2.cmake +++ b/cmake/CliFboss2.cmake @@ -974,6 +974,9 @@ add_library(fboss2_config_lib fboss/cli/fboss2/commands/config/protocol/bgp/policy/as-path-list/BgpAsPathListCliUtils.h fboss/cli/fboss2/commands/config/protocol/bgp/policy/as-path-list/CmdConfigProtocolBgpPolicyAsPathList.cpp fboss/cli/fboss2/commands/config/protocol/bgp/policy/as-path-list/CmdConfigProtocolBgpPolicyAsPathList.h + fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/BgpCommunityListCliUtils.h + fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.cpp + fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.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 @@ -1114,6 +1117,8 @@ add_library(fboss2_config_lib fboss/cli/fboss2/commands/delete/protocol/bgp/policy/CmdDeleteProtocolBgpPolicy.h fboss/cli/fboss2/commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.cpp fboss/cli/fboss2/commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.h + fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.cpp + fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.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 diff --git a/cmake/CliFboss2TestConfig.cmake b/cmake/CliFboss2TestConfig.cmake index 375d53ecc1c93..f9226bda5a1be 100644 --- a/cmake/CliFboss2TestConfig.cmake +++ b/cmake/CliFboss2TestConfig.cmake @@ -15,6 +15,7 @@ add_executable(fboss2_cmd_config_test fboss/cli/fboss2/test/config/CmdConfigBgpNeighborTest.cpp fboss/cli/fboss2/test/config/CmdConfigBgpPeerGroupTest.cpp fboss/cli/fboss2/test/config/CmdConfigBgpPolicyAsPathListTest.cpp + fboss/cli/fboss2/test/config/CmdConfigBgpPolicyCommunityListTest.cpp fboss/cli/fboss2/test/config/CmdConfigCoppTest.cpp fboss/cli/fboss2/test/config/CmdConfigDataPlaneTrafficPolicyTest.cpp fboss/cli/fboss2/test/config/CmdConfigDhcpTest.cpp @@ -53,6 +54,7 @@ add_executable(fboss2_cmd_config_test fboss/cli/fboss2/test/config/CmdDeleteBgpNeighborTest.cpp fboss/cli/fboss2/test/config/CmdDeleteBgpPeerGroupTest.cpp fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyAsPathListTest.cpp + fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyCommunityListTest.cpp fboss/cli/fboss2/test/config/CmdDeleteConfigInterfaceTest.cpp fboss/cli/fboss2/test/config/CmdDeleteCoppQueueTest.cpp fboss/cli/fboss2/test/config/CmdDeleteCoppReasonTest.cpp diff --git a/cmake/CliFboss2TestIntegrationTest.cmake b/cmake/CliFboss2TestIntegrationTest.cmake index b011edd5a7230..7bfff504c46a8 100644 --- a/cmake/CliFboss2TestIntegrationTest.cmake +++ b/cmake/CliFboss2TestIntegrationTest.cmake @@ -19,6 +19,7 @@ add_executable(fboss2_integration_test fboss/cli/fboss2/test/integration_test/ConfigBgpNeighborTest.cpp fboss/cli/fboss2/test/integration_test/ConfigBgpPeerGroupTest.cpp fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyAsPathListTest.cpp + fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyCommunityListTest.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 diff --git a/fboss/cli/fboss2/BUCK b/fboss/cli/fboss2/BUCK index 6d5b26c79fa69..6bf9e425317a9 100644 --- a/fboss/cli/fboss2/BUCK +++ b/fboss/cli/fboss2/BUCK @@ -1184,6 +1184,7 @@ cpp_library( "commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.cpp", "commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.cpp", "commands/config/protocol/bgp/policy/as-path-list/CmdConfigProtocolBgpPolicyAsPathList.cpp", + "commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.cpp", "commands/config/protocol/static/CmdConfigProtocolStatic.cpp", "commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.cpp", "commands/config/ptp/CmdConfigPtp.cpp", @@ -1253,6 +1254,7 @@ cpp_library( "commands/delete/protocol/bgp/peer-group/CmdDeleteProtocolBgpPeerGroup.cpp", "commands/delete/protocol/bgp/policy/CmdDeleteProtocolBgpPolicy.cpp", "commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.cpp", + "commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.cpp", "commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp", "commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.cpp", "commands/delete/qos/CmdDeleteQos.cpp", @@ -1330,6 +1332,8 @@ cpp_library( "commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.h", "commands/config/protocol/bgp/policy/as-path-list/BgpAsPathListCliUtils.h", "commands/config/protocol/bgp/policy/as-path-list/CmdConfigProtocolBgpPolicyAsPathList.h", + "commands/config/protocol/bgp/policy/community-list/BgpCommunityListCliUtils.h", + "commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h", "commands/config/protocol/static/CmdConfigProtocolStatic.h", "commands/config/protocol/static/route/StaticRouteUtils.h", "commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h", @@ -1400,6 +1404,7 @@ cpp_library( "commands/delete/protocol/bgp/peer-group/CmdDeleteProtocolBgpPeerGroup.h", "commands/delete/protocol/bgp/policy/CmdDeleteProtocolBgpPolicy.h", "commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.h", + "commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h", "commands/delete/protocol/static/CmdDeleteProtocolStatic.h", "commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h", "commands/delete/qos/CmdDeleteQos.h", diff --git a/fboss/cli/fboss2/CmdListConfig.cpp b/fboss/cli/fboss2/CmdListConfig.cpp index efe8dc24e2feb..5d3fcd0a64bd2 100644 --- a/fboss/cli/fboss2/CmdListConfig.cpp +++ b/fboss/cli/fboss2/CmdListConfig.cpp @@ -44,6 +44,7 @@ #include "fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.h" #include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.h" #include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/as-path-list/CmdConfigProtocolBgpPolicyAsPathList.h" +#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.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" @@ -110,6 +111,7 @@ #include "fboss/cli/fboss2/commands/delete/protocol/bgp/peer-group/CmdDeleteProtocolBgpPeerGroup.h" #include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/CmdDeleteProtocolBgpPolicy.h" #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/static/CmdDeleteProtocolStatic.h" #include "fboss/cli/fboss2/commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h" #include "fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.h" @@ -423,15 +425,26 @@ const CommandTree& kConfigCommandTree() { commandHandler, argRegistrar, {{ - "as-path-list", - "Configure BGP AS-path list: " - "[ ...] (description, " - "regex, boolean-operator)", - commandHandler< - CmdConfigProtocolBgpPolicyAsPathList>, - argRegistrar< - CmdConfigProtocolBgpPolicyAsPathListTraits>, - }}, + "as-path-list", + "Configure BGP AS-path list: " + "[ ...] (description, " + "regex, boolean-operator)", + commandHandler< + CmdConfigProtocolBgpPolicyAsPathList>, + argRegistrar< + CmdConfigProtocolBgpPolicyAsPathListTraits>, + }, + { + "community-list", + "Configure BGP community-list: " + "[ ...] " + "(boolean-operator, community, description, " + "exact-match)", + commandHandler< + CmdConfigProtocolBgpPolicyCommunityList>, + argRegistrar< + CmdConfigProtocolBgpPolicyCommunityListTraits>, + }}, }, }, }, @@ -781,13 +794,21 @@ const CommandTree& kConfigCommandTree() { commandHandler, argTypeHandler, {{ - "as-path-list", - "Delete a BGP AS-path list, or one of its regexes: " - " [regex ]", - commandHandler, - argRegistrar< - CmdDeleteProtocolBgpPolicyAsPathListTraits>, - }}, + "as-path-list", + "Delete a BGP AS-path list, or one of its regexes: " + " [regex ]", + commandHandler, + argRegistrar< + CmdDeleteProtocolBgpPolicyAsPathListTraits>, + }, + { + "community-list", + "Delete a BGP community-list: ", + commandHandler< + CmdDeleteProtocolBgpPolicyCommunityList>, + argRegistrar< + CmdDeleteProtocolBgpPolicyCommunityListTraits>, + }}, }}, }, { diff --git a/fboss/cli/fboss2/commands/config/protocol/bgp/BgpCliValueParsers.h b/fboss/cli/fboss2/commands/config/protocol/bgp/BgpCliValueParsers.h index 6e80deec368b4..081c770e970ac 100644 --- a/fboss/cli/fboss2/commands/config/protocol/bgp/BgpCliValueParsers.h +++ b/fboss/cli/fboss2/commands/config/protocol/bgp/BgpCliValueParsers.h @@ -10,13 +10,18 @@ #pragma once +#include #include +#include #include #include #include #include +#include #include +#include #include +#include /** * Value parsing helpers shared by the BGP config dispatchers @@ -93,4 +98,58 @@ inline std::optional parseAsn4Byte(const std::string& value) { return static_cast(*parsed); } +// ---- constructor-time token parsing ---------------------------------------- +// Unlike the value parsers above, this runs while constructing a command's +// ObjectArgType, where throwing std::invalid_argument IS the framework's +// error channel — the message is surfaced to the user as the parse error. + +// The ` [ ]` prefix, for the *delete* +// command of a policy list whose members are keyed objects (prefix-list +// entries): the list name, the optional member selected by `keyword`, and +// where any remaining tokens begin. +// +// Only delete parses this shape by hand. The config side reaches the same +// grammar through CLI11 subcommand nesting instead — the member keyword is a +// registered subcommand there, so each level gets its own already-split token +// vector and never calls this helper. Lists whose members are a flat string +// list in the daemon (as-path-list `regex`, community-list `community`) have +// no member level at all: their optional ` ` selector is a +// plain attribute-style pair, parsed inline by their own delete command. +struct ListMemberSelector { + std::string listName; + std::optional memberName; + // Index of the first token after the parsed prefix (== tokens.size() when + // nothing follows). The delete dispatcher rejects any tail. + size_t restStart; +}; + +// Parse the prefix. `objectName` is the list flavor for messages (e.g. +// "community-list"), `memberKeyword` selects the nested member (e.g. +// `community`), and `usage` is the whole-command usage line thrown when no +// tokens were given. A second token other than `memberKeyword` is left to +// the caller (restStart == 1), which delete rejects as an unexpected token. +inline ListMemberSelector parseListMemberSelector( + const std::vector& tokens, + std::string_view objectName, + std::string_view memberKeyword, + std::string_view usage) { + if (tokens.empty()) { + throw std::invalid_argument(std::string(usage)); + } + if (tokens[0].empty()) { + throw std::invalid_argument( + fmt::format("Error: {} name must not be empty", objectName)); + } + ListMemberSelector selector{tokens[0], std::nullopt, 1}; + if (tokens.size() > 1 && tokens[1] == memberKeyword) { + if (tokens.size() < 3 || tokens[2].empty()) { + throw std::invalid_argument( + fmt::format("Error: `{}` requires a ", memberKeyword)); + } + selector.memberName = tokens[2]; + selector.restStart = 3; + } + return selector; +} + } // namespace facebook::fboss::bgpcli diff --git a/fboss/cli/fboss2/commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.h b/fboss/cli/fboss2/commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.h index 4830fe77f5545..6d0115fb5aa0f 100644 --- a/fboss/cli/fboss2/commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.h +++ b/fboss/cli/fboss2/commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.h @@ -10,15 +10,18 @@ #pragma once +#include +#include #include "fboss/cli/fboss2/CmdHandler.h" #include "fboss/cli/fboss2/commands/config/protocol/bgp/CmdConfigProtocolBgp.h" +#include "fboss/cli/fboss2/utils/HostInfo.h" namespace facebook::fboss { // Grouping node for `config protocol bgp policy ...`. Holds no -// state of its own; the per-object-type dispatchers (as-path-list, and later -// community-list/prefix-list/routing-policy) are its subcommands. Mirrors -// CmdConfigProtocolBgp. +// state of its own; the per-object-type dispatchers (as-path-list, +// community-list, and later prefix-list/routing-policy) are its subcommands. +// Mirrors CmdConfigProtocolBgp. struct CmdConfigProtocolBgpPolicyTraits : public WriteCommandTraits { using ParentCmd = CmdConfigProtocolBgp; using ObjectArgType = std::monostate; diff --git a/fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/BgpCommunityListCliUtils.h b/fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/BgpCommunityListCliUtils.h new file mode 100644 index 0000000000000..b7299a901beed --- /dev/null +++ b/fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/BgpCommunityListCliUtils.h @@ -0,0 +1,58 @@ +/* + * 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 +#include +#include +#include "configerator/structs/neteng/bgp_policy/thrift/gen-cpp2/bgp_policy_types.h" + +/** + * Lookup/create helpers for the community-list CLI family, shared between the + * list-level dispatcher (CmdConfigProtocolBgpPolicyCommunityList) and its + * delete counterpart. A CommunityList is keyed by name. Its values live in the + * flat `communities` string list, which is the field bgpd matches against; the + * structured members[] are never read by bgpd and have no CLI. + */ +namespace facebook::fboss::bgpcli { + +inline bool communityListExists( + const bgp::thrift::BgpConfig& cfg, + const std::string& name) { + if (!cfg.policies().has_value()) { + return false; + } + const auto& lists = *cfg.policies()->community_lists(); + return std::any_of(lists.begin(), lists.end(), [&](const auto& list) { + return *list.name() == name; + }); +} + +// Find the community-list keyed by name, creating it if absent. Setting an +// attribute on a not-yet-created list implicitly creates it, so command +// ordering stays forgiving; a bare `community-list ` creates one +// explicitly. CommunityList's only key field is `name`. +inline bgp::bgp_policy::CommunityList& findOrCreateCommunityList( + bgp::thrift::BgpConfig& cfg, + const std::string& name) { + auto& lists = *cfg.policies().ensure().community_lists(); + for (auto& list : lists) { + if (*list.name() == name) { + return list; + } + } + lists.emplace_back(); + auto& list = lists.back(); + list.name() = name; + return list; +} + +} // namespace facebook::fboss::bgpcli diff --git a/fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.cpp b/fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.cpp new file mode 100644 index 0000000000000..ceba3c4c5dcb2 --- /dev/null +++ b/fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.cpp @@ -0,0 +1,252 @@ +/* + * 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/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h" + +#include "fboss/cli/fboss2/CmdHandler.cpp" + +#include // NOLINT(misc-include-cleaner) +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "configerator/structs/neteng/bgp_policy/thrift/gen-cpp2/bgp_policy_types.h" +#include "configerator/structs/neteng/bgp_policy/thrift/gen-cpp2/routing_policy_types.h" +#include "fboss/cli/fboss2/commands/config/protocol/bgp/BgpCliAttrHandlers.h" +#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/BgpCommunityListCliUtils.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. Values are flat +// `community` attributes because bgpd stores them as a flat string list; +// nested entries are reserved for families where the daemon stores objects +// (prefix-list, routing-policy). +constexpr std::string_view kBooleanOperator = "boolean-operator"; +constexpr std::string_view kCommunity = "community"; +constexpr std::string_view kDescription = "description"; +constexpr std::string_view kExactMatch = "exact-match"; + +// boolean-operator values (routing_policy.BooleanOperator names). +constexpr std::string_view kBooleanOperatorAnd = "AND"; +constexpr std::string_view kBooleanOperatorOr = "OR"; + +using BooleanOperator = bgp::routing_policy::BooleanOperator; +using CommunityList = bgp::bgp_policy::CommunityList; +using bgpcli::AttrHandler; +using bgpcli::boolAttr; +using bgpcli::enumAttr; +using bgpcli::err; +using bgpcli::joinedStringAttr; +using bgpcli::ok; +using bgpcli::Result; +using bgpcli::Tokens; + +// ---- value lookups ---------------------------------------------------------- + +// NOT is deliberately not offered: bgpd treats every operator other than OR +// as "all must match", so NOT would silently behave as AND. +std::optional lookupBooleanOperator(const std::string& s) { + if (s == kBooleanOperatorAnd) { + return BooleanOperator::AND; + } + if (s == kBooleanOperatorOr) { + return BooleanOperator::OR; + } + return std::nullopt; +} + +// ---- list-level setters ----------------------------------------------------- +// Each writes one already-parsed, already-validated value. Parsing and message +// text belong to the shared factories in BgpCliAttrHandlers.h. + +void setListBooleanOperator(CommunityList& list, BooleanOperator op) { + list.boolean_operator() = op; +} + +void setListDescription(CommunityList& list, const std::string& description) { + list.description() = description; +} + +void setListExactMatch(CommunityList& list, bool exactMatch) { + list.exact_match() = exactMatch; +} + +// Hand-written because no factory covers an accumulating, validated set: +// each `community` appends one value to communities (the field bgpd matches +// against). bgpd accepts either a literal `:` or a regex, and +// compiles anything that is not a literal as a regex at load, so compile it +// here too and keep a pattern bgpd would reject out of the session. +// Idempotent on repeats. +Result community(CommunityList& list, const Tokens& values) { + if (values.size() != 1 || values[0].empty()) { + return err( + fmt::format( + "Error: {} requires (one token; a literal such as " + "65000:100 or a regex)", + kCommunity)); + } + const auto& value = values[0]; + if (std::any_of(value.begin(), value.end(), [](unsigned char c) { + return std::isspace(c) != 0; + })) { + return err( + fmt::format( + "Error: {} '{}' must not contain whitespace; repeat the command " + "once per community", + kCommunity, + value)); + } + try { + boost::regex compiled(value); + } catch (const boost::regex_error& e) { + return err( + fmt::format( + "Error: Malformed {} '{}': {}", kCommunity, value, e.what())); + } + auto& communities = list.communities().ensure(); + if (std::find(communities.begin(), communities.end(), value) != + communities.end()) { + return ok(fmt::format("{} {} already present", kCommunity, value)); + } + communities.push_back(value); + return ok(fmt::format("Successfully added {} {}", kCommunity, value)); +} + +// ---- list-level attribute registry ------------------------------------------ +// One line per documented attribute: its dispatch key, its value shape, and +// the setter that stores it. +const std::map, std::less<>>& +listAttrHandlers() { + static const std::string kBooleanOperatorValues = + fmt::format("{}|{}", kBooleanOperatorAnd, kBooleanOperatorOr); + static const std::map, std::less<>> + kHandlers = { + {std::string(kBooleanOperator), + enumAttr( + kBooleanOperator, + kBooleanOperatorValues, + lookupBooleanOperator, + setListBooleanOperator)}, + {std::string(kCommunity), community}, + {std::string(kDescription), + joinedStringAttr(kDescription, setListDescription)}, + {std::string(kExactMatch), + boolAttr(kExactMatch, setListExactMatch)}, + }; + return kHandlers; +} + +std::string validAttrList() { + std::string out; + for (const auto& [name, _] : listAttrHandlers()) { + if (!out.empty()) { + out += ", "; + } + out += name; + } + return out; +} + +} // 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 BgpAsPathListConfig). +BgpCommunityListConfig::BgpCommunityListConfig(std::vector v) + : utils::BaseObjectArgType(v) { + if (v.empty()) { + throw std::invalid_argument( + "Error: community-list is required, optionally followed by an " + " "); + } + if (v[0].empty()) { + throw std::invalid_argument("Error: community-list name must not be empty"); + } + listName_ = v[0]; + + if (v.size() == 1) { + return; // bare `community-list `: create the list + } + + attr_ = v[1]; + values_.assign(v.begin() + 2, v.end()); + + if (listAttrHandlers().find(attr_) == listAttrHandlers().end()) { + throw std::invalid_argument( + fmt::format( + "Error: unknown community-list attribute '{}'. Valid attributes: " + "{}", + attr_, + validAttrList())); + } +} + +CmdConfigProtocolBgpPolicyCommunityListTraits::RetType +CmdConfigProtocolBgpPolicyCommunityList::queryClient( + const HostInfo& /* hostInfo */, + const ObjectArgType& args) { + auto& session = ConfigSession::getInstance(); + auto& cfg = session.getBgpConfig(); + const bool listCreated = !bgpcli::communityListExists(cfg, args.listName()); + auto& list = bgpcli::findOrCreateCommunityList(cfg, args.listName()); + + Result result = args.attr().empty() + ? ok(listCreated + ? fmt::format( + "Successfully created BGP community-list {}", + args.listName()) + : fmt::format( + "BGP community-list {} already exists", args.listName())) + // The attribute is guaranteed valid: BgpCommunityListConfig's + // constructor rejects an unknown attribute before we get here. + : listAttrHandlers().find(args.attr())->second(list, args.values()); + + if (result.ok) { + if (!args.attr().empty()) { + result.message += fmt::format(" for community-list {}", args.listName()); + } + session.saveBgpConfig(); + result.message += + fmt::format("\nConfig saved to: {}", session.getBgpSessionConfigPath()); + } else if (listCreated) { + // Drop the phantom list so a rejected value is not visible to later + // lookups in the same process. + cfg.policies()->community_lists()->pop_back(); + } + return result.message; +} + +void CmdConfigProtocolBgpPolicyCommunityList::printOutput( + const RetType& output) { + std::cout << output << std::endl; +} + +// Explicit template instantiation +template void CmdHandler< + CmdConfigProtocolBgpPolicyCommunityList, + CmdConfigProtocolBgpPolicyCommunityListTraits>::run(); + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h b/fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h new file mode 100644 index 0000000000000..4a61d2e195d25 --- /dev/null +++ b/fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h @@ -0,0 +1,88 @@ +/* + * 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 +#include +#include "CLI/App.hpp" +#include "fboss/cli/fboss2/CmdHandler.h" +#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.h" +#include "fboss/cli/fboss2/utils/CmdUtilsCommon.h" +#include "fboss/cli/fboss2/utils/HostInfo.h" + +namespace facebook::fboss { + +// Parsed `community-list [ ...]`, validated at +// construction. A CommunityList is keyed by . Its values are a flat +// string list in the daemon, so they are flat `community` attributes here +// (one per invocation), not nested entries. Mirrors BgpAsPathListConfig. +// +// Grammar (from the FBOSS proposed syntax): +// community-list (create/select list) +// community-list boolean-operator (list attribute) +// community-list community (append to communities, +// the field bgpd matches +// on) +// community-list description (list attribute) +// community-list exact-match (list attribute) +class BgpCommunityListConfig : public utils::BaseObjectArgType { + public: + // NOLINTNEXTLINE(google-explicit-constructor) + /* implicit */ BgpCommunityListConfig(std::vector v); + const std::string& listName() const { + return listName_; + } + const std::string& attr() const { + return attr_; + } + const std::vector& values() const { + return values_; + } + const static utils::ObjectArgTypeId id = + utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_MESSAGE; + + private: + std::string listName_; + std::string attr_; // matched dispatch key ("" = bare create) + std::vector values_; +}; + +// Single handler for the whole `config protocol bgp policy community-list` +// family; dispatch and per-attribute parsing live in the .cpp so a new tunable +// is a one-entry change. Mirrors CmdConfigProtocolBgpPolicyAsPathList. +struct CmdConfigProtocolBgpPolicyCommunityListTraits + : public WriteCommandTraits { + using ParentCmd = CmdConfigProtocolBgpPolicy; + static void addCliArg(CLI::App& cmd, std::vector& args) { + // Stops CLI11 from classifying attribute tokens as subcommands once the + // list name is consumed. See CmdConfigProtocolBgpNeighborTraits. + cmd.positionals_at_end(); + cmd.add_option("args", args, " [ ...]"); + } + using ObjectArgType = BgpCommunityListConfig; + using RetType = std::string; +}; + +class CmdConfigProtocolBgpPolicyCommunityList + : public CmdHandler< + CmdConfigProtocolBgpPolicyCommunityList, + CmdConfigProtocolBgpPolicyCommunityListTraits> { + public: + using ObjectArgType = + CmdConfigProtocolBgpPolicyCommunityListTraits::ObjectArgType; + using RetType = CmdConfigProtocolBgpPolicyCommunityListTraits::RetType; + + RetType queryClient(const HostInfo& hostInfo, const ObjectArgType& args); + + void printOutput(const RetType& output); +}; + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.cpp b/fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.cpp new file mode 100644 index 0000000000000..92322498ceec4 --- /dev/null +++ b/fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.cpp @@ -0,0 +1,138 @@ +/* + * 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/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h" + +#include "fboss/cli/fboss2/CmdHandler.cpp" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#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 { +constexpr std::string_view kCommunityKeyword = "community"; +} // namespace + +// Parse + validate at construction so queryClient stays a thin dispatch. +BgpCommunityListRef::BgpCommunityListRef(std::vector v) + : utils::BaseObjectArgType(v) { + if (v.empty()) { + throw std::invalid_argument( + "Error: delete protocol bgp policy community-list requires , " + "optionally followed by `community `"); + } + if (v[0].empty()) { + throw std::invalid_argument("Error: community-list name must not be empty"); + } + listName_ = v[0]; + if (v.size() == 1) { + return; + } + if (v[1] != kCommunityKeyword) { + throw std::invalid_argument( + fmt::format( + "Error: unexpected token '{}'. Usage: delete protocol bgp policy " + "community-list [community ]", + v[1])); + } + if (v.size() < 3 || v[2].empty()) { + throw std::invalid_argument("Error: `community` requires a "); + } + if (v.size() > 3) { + throw std::invalid_argument( + fmt::format( + "Error: unexpected token '{}' after community ", v[3])); + } + hasCommunity_ = true; + community_ = v[2]; +} + +CmdDeleteProtocolBgpPolicyCommunityListTraits::RetType +CmdDeleteProtocolBgpPolicyCommunityList::queryClient( + const HostInfo& /* hostInfo */, + const ObjectArgType& args) { + auto& session = ConfigSession::getInstance(); + auto& cfg = session.getBgpConfig(); + // Delete mirrors add: an absent target is already the requested end state, + // so this is a success with a warning. Nothing is saved, so a typo'd delete + // can't stage an unrelated session change. + auto absent = fmt::format( + "Warning: BGP community-list {} does not exist; nothing to delete", + args.listName()); + if (!cfg.policies().has_value()) { + return absent; + } + auto& lists = *cfg.policies()->community_lists(); + auto it = std::find_if(lists.begin(), lists.end(), [&](const auto& list) { + return *list.name() == args.listName(); + }); + if (it == lists.end()) { + return absent; + } + if (args.hasCommunity()) { + // Remove one value from communities; the list itself stays. + auto& list = *it; + if (list.communities().has_value()) { + auto& communities = *list.communities(); + auto valueIt = + std::find(communities.begin(), communities.end(), args.community()); + if (valueIt != communities.end()) { + communities.erase(valueIt); + if (communities.empty()) { + // Leave the list looking like one that never had values, so a + // later `community ` add starts from the same shape. + list.communities().reset(); + } + session.saveBgpConfig(); + return fmt::format( + "Successfully deleted BGP community-list {} community {}\n" + "Config saved to: {}", + args.listName(), + args.community(), + session.getBgpSessionConfigPath()); + } + } + return fmt::format( + "Warning: BGP community-list {} has no community {}; nothing to " + "delete", + args.listName(), + args.community()); + } + lists.erase(it); + session.saveBgpConfig(); + return fmt::format( + "Successfully deleted BGP community-list {}\nConfig saved to: {}", + args.listName(), + session.getBgpSessionConfigPath()); +} + +void CmdDeleteProtocolBgpPolicyCommunityList::printOutput( + const RetType& output) { + std::cout << output << std::endl; +} + +// Explicit template instantiation +template void CmdHandler< + CmdDeleteProtocolBgpPolicyCommunityList, + CmdDeleteProtocolBgpPolicyCommunityListTraits>::run(); + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h b/fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h new file mode 100644 index 0000000000000..bf95887a04a25 --- /dev/null +++ b/fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h @@ -0,0 +1,74 @@ +/* + * 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 +#include +#include "CLI/App.hpp" +#include "fboss/cli/fboss2/CmdHandler.h" +#include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/CmdDeleteProtocolBgpPolicy.h" +#include "fboss/cli/fboss2/utils/CmdUtilsCommon.h" +#include "fboss/cli/fboss2/utils/HostInfo.h" + +namespace facebook::fboss { + +// Parsed `delete protocol bgp policy community-list +// [community ]`, validated at construction. The name is the +// list's identity, matched exactly the same way the config command stores +// it; the optional `community` selector removes one communities value +// instead of the whole list. Mirrors BgpAsPathListRef. +class BgpCommunityListRef : public utils::BaseObjectArgType { + public: + // NOLINTNEXTLINE(google-explicit-constructor) + /* implicit */ BgpCommunityListRef(std::vector v); + const std::string& listName() const { + return listName_; + } + bool hasCommunity() const { + return hasCommunity_; + } + const std::string& community() const { + return community_; + } + const static utils::ObjectArgTypeId id = + utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_MESSAGE; + + private: + std::string listName_; + bool hasCommunity_{false}; + std::string community_; +}; + +struct CmdDeleteProtocolBgpPolicyCommunityListTraits + : public WriteCommandTraits { + using ParentCmd = CmdDeleteProtocolBgpPolicy; + static void addCliArg(CLI::App& cmd, std::vector& args) { + cmd.add_option("args", args, " [community ]"); + } + using ObjectArgType = BgpCommunityListRef; + using RetType = std::string; +}; + +class CmdDeleteProtocolBgpPolicyCommunityList + : public CmdHandler< + CmdDeleteProtocolBgpPolicyCommunityList, + CmdDeleteProtocolBgpPolicyCommunityListTraits> { + public: + using ObjectArgType = + CmdDeleteProtocolBgpPolicyCommunityListTraits::ObjectArgType; + using RetType = CmdDeleteProtocolBgpPolicyCommunityListTraits::RetType; + + RetType queryClient(const HostInfo& hostInfo, const ObjectArgType& args); + + void printOutput(const RetType& output); +}; + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/test/config/BUCK b/fboss/cli/fboss2/test/config/BUCK index f41a1e3e4bb4a..2299fdcd28117 100644 --- a/fboss/cli/fboss2/test/config/BUCK +++ b/fboss/cli/fboss2/test/config/BUCK @@ -16,6 +16,7 @@ cpp_unittest( "CmdConfigBgpNeighborTest.cpp", "CmdConfigBgpPeerGroupTest.cpp", "CmdConfigBgpPolicyAsPathListTest.cpp", + "CmdConfigBgpPolicyCommunityListTest.cpp", "CmdConfigCoppTest.cpp", "CmdConfigDataPlaneTrafficPolicyTest.cpp", "CmdConfigDhcpTest.cpp", @@ -54,6 +55,7 @@ cpp_unittest( "CmdDeleteBgpNeighborTest.cpp", "CmdDeleteBgpPeerGroupTest.cpp", "CmdDeleteBgpPolicyAsPathListTest.cpp", + "CmdDeleteBgpPolicyCommunityListTest.cpp", "CmdDeleteConfigInterfaceTest.cpp", "CmdDeleteCoppQueueTest.cpp", "CmdDeleteCoppReasonTest.cpp", diff --git a/fboss/cli/fboss2/test/config/CmdConfigBgpPolicyCommunityListTest.cpp b/fboss/cli/fboss2/test/config/CmdConfigBgpPolicyCommunityListTest.cpp new file mode 100644 index 0000000000000..4991950c51d1e --- /dev/null +++ b/fboss/cli/fboss2/test/config/CmdConfigBgpPolicyCommunityListTest.cpp @@ -0,0 +1,261 @@ +// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary. + +#include +#include +#include + +#include +#include +#include +#include + +#include "configerator/structs/neteng/bgp_policy/thrift/gen-cpp2/bgp_policy_types.h" +#include "configerator/structs/neteng/bgp_policy/thrift/gen-cpp2/routing_policy_types.h" +#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.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; +using facebook::bgp::routing_policy::BooleanOperator; + +namespace facebook::fboss { + +// The community-list dispatcher only touches the BGP side of ConfigSession, +// which seeds from thrift schema defaults when neither a staged session nor a +// system bgpcpp.conf exists — so no seed agent config is needed (mirrors +// CmdConfigBgpPolicyAsPathListTest). +class CmdConfigBgpPolicyCommunityListTestFixture : public CmdConfigTestBase { + public: + CmdConfigBgpPolicyCommunityListTestFixture() + : CmdConfigTestBase("bgp_community_list_test_%%%%-%%%%-%%%%", "") {} + + void SetUp() override { + CmdConfigTestBase::SetUp(); + setupTestableConfigSession(); + } + + std::string run(const std::vector& tokens) { + CmdConfigProtocolBgpPolicyCommunityList cmd; + HostInfo hostInfo("testhost"); + return cmd.queryClient(hostInfo, BgpCommunityListConfig(tokens)); + } + + const std::vector& lists() { + return *ConfigSession::getInstance() + .getBgpConfig() + .policies() + .ensure() + .community_lists(); + } + + bool sessionFileExists() { + return std::filesystem::exists( + ConfigSession::getInstance().getBgpSessionConfigPath()); + } +}; + +// ============================================================================== +// BgpCommunityListConfig (arg) validation +// ============================================================================== + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, argValidation) { + // Bare create list. + auto bareList = BgpCommunityListConfig({"CL100"}); + EXPECT_EQ(bareList.listName(), "CL100"); + EXPECT_TRUE(bareList.attr().empty()); + + // List-level attribute. + auto listAttr = + BgpCommunityListConfig({"CL100", "description", "my", "list"}); + EXPECT_EQ(listAttr.attr(), "description"); + EXPECT_EQ(listAttr.values(), std::vector({"my", "list"})); + + // Invalid: empty, empty name, unknown attribute. + EXPECT_THROW(BgpCommunityListConfig({}), std::invalid_argument); + EXPECT_THROW(BgpCommunityListConfig({""}), std::invalid_argument); + EXPECT_THROW( + BgpCommunityListConfig({"CL100", "no-such-attr", "1"}), + std::invalid_argument); + // value was the removed community subcommand's attribute; bgpd never read + // the members it wrote, so it must not be accepted anywhere. + EXPECT_THROW( + BgpCommunityListConfig({"CL100", "value", "65000:100"}), + std::invalid_argument); + + // List-level community and boolean-operator are recognized. + EXPECT_EQ( + BgpCommunityListConfig({"CL100", "community", "65000:100"}).attr(), + "community"); + EXPECT_EQ( + BgpCommunityListConfig({"CL100", "boolean-operator", "AND"}).attr(), + "boolean-operator"); +} + +// ============================================================================== +// List-level handlers +// ============================================================================== + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, bareCreateList) { + auto result = run({"CL100"}); + EXPECT_THAT( + result, HasSubstr("Successfully created BGP community-list CL100")); + ASSERT_EQ(lists().size(), 1); + EXPECT_EQ(*lists()[0].name(), "CL100"); + EXPECT_FALSE(lists()[0].communities().has_value()); + EXPECT_TRUE(sessionFileExists()); + + run({"CL200"}); + ASSERT_EQ(lists().size(), 2); + EXPECT_EQ(*lists()[1].name(), "CL200"); +} + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, setListDescription) { + auto result = run({"CL100", "description", "spine", "communities"}); + EXPECT_THAT(result, HasSubstr("Successfully set description")); + ASSERT_EQ(lists().size(), 1); + // Multi-token description is re-joined. + EXPECT_EQ(*lists()[0].description(), "spine communities"); +} + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, setBooleanOperator) { + auto result = run({"CL100", "boolean-operator", "AND"}); + EXPECT_THAT(result, HasSubstr("Successfully set boolean-operator to: AND")); + EXPECT_EQ(*lists()[0].boolean_operator(), BooleanOperator::AND); + + run({"CL100", "boolean-operator", "OR"}); + EXPECT_EQ(*lists()[0].boolean_operator(), BooleanOperator::OR); +} + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, booleanOperatorNotRejected) { + auto result = run({"CL100", "boolean-operator", "NOT"}); + EXPECT_THAT(result, HasSubstr("Invalid boolean-operator value 'NOT'")); + EXPECT_THAT(result, HasSubstr("AND|OR")); + EXPECT_TRUE(lists().empty()); + EXPECT_FALSE(sessionFileExists()); +} + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, booleanOperatorDefaultIsOr) { + run({"CL100"}); + EXPECT_EQ(*lists()[0].boolean_operator(), BooleanOperator::OR); +} + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, setExactMatch) { + auto result = run({"CL100", "exact-match", "true"}); + EXPECT_THAT(result, HasSubstr("Successfully enabled exact-match")); + ASSERT_TRUE(lists()[0].exact_match().has_value()); + EXPECT_TRUE(*lists()[0].exact_match()); + + auto disabled = run({"CL100", "exact-match", "false"}); + EXPECT_THAT(disabled, HasSubstr("Successfully disabled exact-match")); + EXPECT_FALSE(*lists()[0].exact_match()); +} + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, namedListsAreDistinct) { + run({"CL100", "description", "one"}); + run({"CL200", "description", "two"}); + ASSERT_EQ(lists().size(), 2); + // Re-referencing an existing list by name updates it, not appends. + run({"CL100", "description", "one-updated"}); + ASSERT_EQ(lists().size(), 2); + EXPECT_EQ(*lists()[0].description(), "one-updated"); +} + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, reReferenceReportsExisting) { + run({"CL100"}); + // A second bare reference must not claim to have created it again. + EXPECT_THAT(run({"CL100"}), HasSubstr("already exists")); + ASSERT_EQ(lists().size(), 1); +} + +// ============================================================================== +// Reject paths — the error is surfaced and nothing is persisted +// ============================================================================== + +TEST_F( + CmdConfigBgpPolicyCommunityListTestFixture, + invalidBooleanOperatorRejected) { + auto result = run({"CL100", "boolean-operator", "XOR"}); + // The full message matters: the accepted-values suffix once printed garbage + // (a string_view capture dangling over a fmt::format temporary). + EXPECT_THAT( + result, + HasSubstr("Invalid boolean-operator value 'XOR'; expected AND|OR")); + // The rejected value must not leave a phantom list behind. + EXPECT_TRUE(lists().empty()); + EXPECT_FALSE(sessionFileExists()) + << "session file should not exist after rejected input"; +} + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, invalidExactMatchRejected) { + auto result = run({"CL100", "exact-match", "maybe"}); + EXPECT_THAT(result, HasSubstr("Invalid")); + EXPECT_TRUE(lists().empty()); + EXPECT_FALSE(sessionFileExists()) + << "session file should not exist after rejected input"; +} + +// ============================================================================== +// community — communities, the field bgpd matches against +// ============================================================================== + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, communityAppends) { + auto result = run({"CL100", "community", "65000:100"}); + EXPECT_THAT(result, HasSubstr("Successfully added community 65000:100")); + EXPECT_THAT(result, HasSubstr("for community-list CL100")); + run({"CL100", "community", "65000:200"}); + ASSERT_EQ(lists().size(), 1); + // Regression guard for the inert-config bug: the value must land in the + // flat `communities` list bgpd reads, never in the dead `members`. + ASSERT_TRUE(lists()[0].communities().has_value()); + EXPECT_EQ( + *lists()[0].communities(), + std::vector({"65000:100", "65000:200"})); + EXPECT_FALSE(lists()[0].members().has_value()); + EXPECT_TRUE(sessionFileExists()); +} + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, communityAcceptsRegex) { + // bgpd compiles a non-literal value as a regex, so one is accepted here. + auto result = run({"CL100", "community", "^65000:.*$"}); + EXPECT_THAT(result, HasSubstr("Successfully added community ^65000:.*$")); + EXPECT_EQ( + *lists()[0].communities(), std::vector({"^65000:.*$"})); +} + +TEST_F( + CmdConfigBgpPolicyCommunityListTestFixture, + communityAddAgainIsIdempotent) { + run({"CL100", "community", "65000:100"}); + auto result = run({"CL100", "community", "65000:100"}); + EXPECT_THAT(result, HasSubstr("already present")); + ASSERT_EQ(lists().size(), 1); + EXPECT_EQ(lists()[0].communities()->size(), 1); +} + +TEST_F(CmdConfigBgpPolicyCommunityListTestFixture, malformedCommunityRejected) { + // Neither a literal nor a compilable regex: bgpd would refuse it at load. + auto result = run({"CL100", "community", "65000:("}); + EXPECT_THAT(result, HasSubstr("Error: Malformed community")); + // A rejected first attribute leaves no phantom list and nothing on disk. + EXPECT_TRUE(lists().empty()); + EXPECT_FALSE(sessionFileExists()) + << "session file should not exist after rejected input"; +} + +TEST_F( + CmdConfigBgpPolicyCommunityListTestFixture, + communityWithWhitespaceRejected) { + run({"CL100"}); + auto result = run({"CL100", "community", "65000:100 65000:200"}); + EXPECT_THAT(result, HasSubstr("must not contain whitespace")); + ASSERT_EQ(lists().size(), 1); + EXPECT_FALSE(lists()[0].communities().has_value()); + + EXPECT_THAT(run({"CL100", "community"}), HasSubstr("requires ")); + EXPECT_THAT( + run({"CL100", "community", "65000:100", "65000:200"}), + HasSubstr("requires ")); +} + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyCommunityListTest.cpp b/fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyCommunityListTest.cpp new file mode 100644 index 0000000000000..a627453c5a230 --- /dev/null +++ b/fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyCommunityListTest.cpp @@ -0,0 +1,249 @@ +// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary. + +#include +#include +#include + +#include +#include +#include +#include + +#include "configerator/structs/neteng/bgp_policy/thrift/gen-cpp2/bgp_policy_types.h" +#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h" +#include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.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 { + +// Deleting only touches the BGP side of ConfigSession, which seeds from thrift +// schema defaults when neither a staged session nor a system bgpcpp.conf +// exists — so no seed agent config is needed (mirrors +// CmdDeleteBgpPolicyAsPathListTest). +class CmdDeleteBgpPolicyCommunityListTestFixture : public CmdConfigTestBase { + public: + CmdDeleteBgpPolicyCommunityListTestFixture() + : CmdConfigTestBase("bgp_community_list_delete_test_%%%%-%%%%-%%%%", "") { + } + + void SetUp() override { + CmdConfigTestBase::SetUp(); + setupTestableConfigSession(); + } + + std::string configure(const std::vector& tokens) { + CmdConfigProtocolBgpPolicyCommunityList cmd; + HostInfo hostInfo("testhost"); + return cmd.queryClient(hostInfo, BgpCommunityListConfig(tokens)); + } + + std::string del(const std::vector& tokens) { + CmdDeleteProtocolBgpPolicyCommunityList cmd; + HostInfo hostInfo("testhost"); + return cmd.queryClient(hostInfo, BgpCommunityListRef(tokens)); + } + + const std::vector& lists() { + return *ConfigSession::getInstance() + .getBgpConfig() + .policies() + .ensure() + .community_lists(); + } + + bool sessionFileExists() { + return std::filesystem::exists( + ConfigSession::getInstance().getBgpSessionConfigPath()); + } +}; + +// ============================================================================== +// BgpCommunityListRef (arg) validation +// ============================================================================== + +TEST_F(CmdDeleteBgpPolicyCommunityListTestFixture, argValidation) { + auto listOnly = BgpCommunityListRef({"CL100"}); + EXPECT_EQ(listOnly.listName(), "CL100"); + EXPECT_FALSE(listOnly.hasCommunity()); + + // Optional `community ` selector. + auto withValue = BgpCommunityListRef({"CL100", "community", "65000:100"}); + EXPECT_EQ(withValue.listName(), "CL100"); + EXPECT_TRUE(withValue.hasCommunity()); + EXPECT_EQ(withValue.community(), "65000:100"); + + // Invalid: empty, empty name, extra tokens, incomplete or trailing selector. + EXPECT_THROW(BgpCommunityListRef({}), std::invalid_argument); + EXPECT_THROW(BgpCommunityListRef({""}), std::invalid_argument); + EXPECT_THROW(BgpCommunityListRef({"CL100", "CL200"}), std::invalid_argument); + EXPECT_THROW( + BgpCommunityListRef({"CL100", "community"}), std::invalid_argument); + EXPECT_THROW( + BgpCommunityListRef({"CL100", "community", ""}), std::invalid_argument); + EXPECT_THROW( + BgpCommunityListRef({"CL100", "community", "65000:100", "extra"}), + std::invalid_argument); +} + +// ============================================================================== +// queryClient +// ============================================================================== + +TEST_F(CmdDeleteBgpPolicyCommunityListTestFixture, deleteExistingList) { + configure({"CL100", "community", "65000:100"}); + configure({"CL200", "description", "keep"}); + ASSERT_EQ(lists().size(), 2); + + auto result = del({"CL100"}); + EXPECT_THAT( + result, HasSubstr("Successfully deleted BGP community-list CL100")); + // The other list survives untouched. + ASSERT_EQ(lists().size(), 1); + EXPECT_EQ(*lists()[0].name(), "CL200"); + EXPECT_EQ(*lists()[0].description(), "keep"); + EXPECT_TRUE(sessionFileExists()); +} + +// Delete mirrors add: an absent target is a success with a warning, never an +// error, so a replayed script stays idempotent. +TEST_F(CmdDeleteBgpPolicyCommunityListTestFixture, deleteUnknownListWarns) { + auto result = del({"NO-SUCH-LIST"}); + EXPECT_THAT( + result, + HasSubstr( + "Warning: BGP community-list NO-SUCH-LIST does not exist; nothing " + "to delete")); + EXPECT_THAT(result, Not(HasSubstr("Error:"))); + // Nothing changed, so nothing is staged. + EXPECT_FALSE(sessionFileExists()) + << "session file should not exist after a no-op delete"; +} + +TEST_F(CmdDeleteBgpPolicyCommunityListTestFixture, deleteTwiceIsIdempotent) { + configure({"CL100", "description", "delete-me"}); + EXPECT_THAT(del({"CL100"}), HasSubstr("Successfully deleted")); + EXPECT_TRUE(lists().empty()); + + auto again = del({"CL100"}); + EXPECT_THAT( + again, + HasSubstr( + "Warning: BGP community-list CL100 does not exist; nothing to " + "delete")); + EXPECT_THAT(again, Not(HasSubstr("Error:"))); + EXPECT_TRUE(lists().empty()); +} + +TEST_F( + CmdDeleteBgpPolicyCommunityListTestFixture, + deleteUnknownLeavesOthersIntact) { + configure({"CL100", "description", "one"}); + auto result = del({"CL200"}); + EXPECT_THAT(result, HasSubstr("does not exist")); + ASSERT_EQ(lists().size(), 1); + EXPECT_EQ(*lists()[0].name(), "CL100"); +} + +// ============================================================================== +// community selector — remove one value, keep the list +// ============================================================================== + +TEST_F(CmdDeleteBgpPolicyCommunityListTestFixture, deleteOneCommunity) { + configure({"CL100", "community", "65000:100"}); + configure({"CL100", "community", "65000:200"}); + + auto result = del({"CL100", "community", "65000:100"}); + EXPECT_THAT( + result, + HasSubstr( + "Successfully deleted BGP community-list CL100 community " + "65000:100")); + // The list and its other value survive. + ASSERT_EQ(lists().size(), 1); + EXPECT_EQ(*lists()[0].communities(), std::vector({"65000:200"})); + EXPECT_TRUE(sessionFileExists()); + + // Deleting the last value clears communities but keeps the list. + del({"CL100", "community", "65000:200"}); + ASSERT_EQ(lists().size(), 1); + EXPECT_EQ(*lists()[0].name(), "CL100"); + EXPECT_FALSE(lists()[0].communities().has_value()); +} + +TEST_F( + CmdDeleteBgpPolicyCommunityListTestFixture, + deleteUnknownCommunityWarns) { + configure({"CL100", "community", "65000:100"}); + // Remove the session file created by configure so its absence afterwards + // proves the no-op delete did not persist anything new. + ASSERT_TRUE(sessionFileExists()); + std::filesystem::remove( + ConfigSession::getInstance().getBgpSessionConfigPath()); + + auto result = del({"CL100", "community", "65000:999"}); + EXPECT_THAT( + result, + HasSubstr( + "Warning: BGP community-list CL100 has no community 65000:999; " + "nothing to delete")); + EXPECT_THAT(result, Not(HasSubstr("Error:"))); + EXPECT_FALSE(sessionFileExists()) + << "no-op delete must not persist a session file"; + // The existing value is untouched. + ASSERT_EQ(lists().size(), 1); + EXPECT_EQ(*lists()[0].communities(), std::vector({"65000:100"})); +} + +TEST_F( + CmdDeleteBgpPolicyCommunityListTestFixture, + deleteCommunityTwiceIsIdempotent) { + configure({"CL100", "community", "65000:100"}); + EXPECT_THAT( + del({"CL100", "community", "65000:100"}), HasSubstr("Successfully")); + EXPECT_FALSE(lists()[0].communities().has_value()); + + auto again = del({"CL100", "community", "65000:100"}); + EXPECT_THAT( + again, + HasSubstr( + "Warning: BGP community-list CL100 has no community 65000:100; " + "nothing to delete")); + EXPECT_THAT(again, Not(HasSubstr("Error:"))); + ASSERT_EQ(lists().size(), 1); + EXPECT_FALSE(lists()[0].communities().has_value()); +} + +TEST_F( + CmdDeleteBgpPolicyCommunityListTestFixture, + deleteCommunityFromValuelessListWarns) { + configure({"CL100", "description", "no", "values"}); + + auto result = del({"CL100", "community", "65000:100"}); + EXPECT_THAT( + result, + HasSubstr( + "Warning: BGP community-list CL100 has no community 65000:100; " + "nothing to delete")); + ASSERT_EQ(lists().size(), 1); + EXPECT_FALSE(lists()[0].communities().has_value()); +} + +TEST_F( + CmdDeleteBgpPolicyCommunityListTestFixture, + deleteCommunityFromUnknownListWarns) { + auto result = del({"NO-SUCH-LIST", "community", "65000:100"}); + EXPECT_THAT( + result, + HasSubstr( + "Warning: BGP community-list NO-SUCH-LIST does not exist; nothing " + "to delete")); + EXPECT_THAT(result, Not(HasSubstr("Error:"))); + EXPECT_FALSE(sessionFileExists()) + << "session file should not exist after a no-op delete"; +} + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/test/integration_test/BUCK b/fboss/cli/fboss2/test/integration_test/BUCK index 474b742608a43..a588c56107a77 100644 --- a/fboss/cli/fboss2/test/integration_test/BUCK +++ b/fboss/cli/fboss2/test/integration_test/BUCK @@ -26,6 +26,7 @@ cpp_binary( "ConfigBgpNeighborTest.cpp", "ConfigBgpPeerGroupTest.cpp", "ConfigBgpPolicyAsPathListTest.cpp", + "ConfigBgpPolicyCommunityListTest.cpp", "ConfigBgpSessionTest.cpp", "ConfigConcurrentSessionsTest.cpp", "ConfigCoppTest.cpp", diff --git a/fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyCommunityListTest.cpp b/fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyCommunityListTest.cpp new file mode 100644 index 0000000000000..e9d863dc7d2d9 --- /dev/null +++ b/fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyCommunityListTest.cpp @@ -0,0 +1,170 @@ +// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary. + +/** + * End-to-end tests for `fboss2-dev config protocol bgp policy community-list + * [ ...]`. + * + * Scope: the whole community-list, including its flat `community` values (the + * communities field bgpd matches on). Every test + * stages the change AND commits it, then asserts the value landed at the + * correct thrift field path inside the matching .policies.community_lists[] + * entry of bgpd's running config (via getRunningConfig RPC) — which also + * confirms bgpd accepts and adopts a `.policies` blob at all. Stage-only + * behavior (attribute parsing, validation, rejection) is covered by the unit + * tests; an integration test that never commits exercises no daemon. + * + * Requirements: + * - The fboss2-dev binary under test (config subcommand tree). + * - HOME is set (the session file lives under $HOME/.fboss2). + * - bgpd is installed/active (commit restarts it). + */ + +#include +#include +#include +#include + +#include "fboss/cli/fboss2/test/integration_test/ConfigBgpTestBase.h" +#include "folly/json/dynamic.h" +#include "gmock/gmock.h" + +using namespace facebook::fboss; +using ::testing::HasSubstr; +using ::testing::Not; + +namespace { +// Test-only community-list name, unlikely to collide with a real list in the +// device's running BGP config. +const std::string kList = "FBOSS2-TEST-COMMUNITY"; +} // namespace + +class ConfigBgpPolicyCommunityListTest : public ConfigBgpTestBase { + protected: + // Stage `config protocol bgp policy community-list ` WITHOUT + // clearing the staged session, so attributes can accumulate across + // invocations. Returns the staged session JSON. + folly::dynamic stageCommunityList(const std::vector& tokens) { + std::vector args = { + "config", "protocol", "bgp", "policy", "community-list"}; + args.insert(args.end(), tokens.begin(), tokens.end()); + auto result = runCli(args); + EXPECT_EQ(result.exitCode, 0) + << "stdout=" << result.stdout << " stderr=" << result.stderr; + EXPECT_THAT(result.stdout, Not(HasSubstr("Error:"))); + return readBgpSessionConfig(); + } + + // The .policies.community_lists[] entry named `name`, or nullptr. + static const folly::dynamic* findList( + const folly::dynamic& config, + const std::string& name) { + if (config.count("policies") == 0 || + config["policies"].count("community_lists") == 0) { + return nullptr; + } + for (const auto& list : config["policies"]["community_lists"]) { + if (list.count("name") && list["name"].asString() == name) { + return &list; + } + } + return nullptr; + } +}; + +TEST_F(ConfigBgpPolicyCommunityListTest, SetListAttributesAndCommit) { + discardSession(); + clearBgpSession(); + stageCommunityList({kList, "description", "test", "spine", "communities"}); + stageCommunityList({kList, "boolean-operator", "AND"}); + stageCommunityList({kList, "exact-match", "true"}); + commitAndGetSha(); + ASSERT_TRUE(waitForBgpDaemonActive()) + << "bgpd did not return active after commit; state=" + << bgpDaemonActiveState(); + // Two layers: the promoted file proves what the commit wrote; the daemon's + // getRunningConfig RPC proves bgpd parsed and adopted the .policies blob. + auto config = readSystemBgpConfig(); + const auto* list = findList(config, kList); + ASSERT_NE(list, nullptr) << "committed config has no community-list " + << kList; + EXPECT_EQ((*list)["description"].asString(), "test spine communities"); + + auto running = readRunningBgpConfigViaRpc(); + const auto* runningList = findList(running, kList); + ASSERT_NE(runningList, nullptr) + << "bgpd's running config has no community-list " << kList; + EXPECT_EQ((*runningList)["description"].asString(), "test spine communities"); + // Enums ride the SimpleJSON wire format as integers: + // routing_policy.BooleanOperator.AND = 1. + EXPECT_EQ((*runningList)["boolean_operator"].asInt(), 1); + EXPECT_TRUE((*runningList)["exact_match"].asBool()); +} + +TEST_F(ConfigBgpPolicyCommunityListTest, SetCommunityAndOperatorAndCommit) { + discardSession(); + clearBgpSession(); + stageCommunityList({kList, "community", "65000:100"}); + stageCommunityList({kList, "community", "65000:200"}); + stageCommunityList({kList, "boolean-operator", "AND"}); + commitAndGetSha(); + ASSERT_TRUE(waitForBgpDaemonActive()) + << "bgpd did not return active after commit; state=" + << bgpDaemonActiveState(); + // bgpd matches on the flat `communities` list + boolean_operator (never on + // members[]); the running config proves it parsed and adopted both. + auto running = readRunningBgpConfigViaRpc(); + const auto* list = findList(running, kList); + ASSERT_NE(list, nullptr) << "bgpd's running config has no community-list " + << kList; + ASSERT_TRUE(list->count("communities")); + std::vector communities; + for (const auto& c : (*list)["communities"]) { + communities.push_back(c.asString()); + } + EXPECT_EQ(communities, std::vector({"65000:100", "65000:200"})); + ASSERT_TRUE(list->count("boolean_operator")); + // routing_policy.BooleanOperator.AND == 1 + EXPECT_EQ((*list)["boolean_operator"].asInt(), 1); +} + +TEST_F(ConfigBgpPolicyCommunityListTest, MalformedCommunityRejected) { + clearBgpSession(); + auto result = runCli( + {"config", + "protocol", + "bgp", + "policy", + "community-list", + kList, + "community", + "65000:("}); + EXPECT_THAT(result.stdout, HasSubstr("Malformed community")); + EXPECT_FALSE(std::filesystem::exists(bgpSessionPath())) + << "session file should not exist after rejected input"; +} + +TEST_F(ConfigBgpPolicyCommunityListTest, DeleteListAndCommit) { + // Land a community-list in the system config, then delete it through a + // second commit and verify it is gone from bgpd's running config. + discardSession(); + clearBgpSession(); + stageCommunityList({kList, "description", "delete-me"}); + commitAndGetSha(); + ASSERT_TRUE(waitForBgpDaemonActive()) + << "bgpd did not return active after commit; state=" + << bgpDaemonActiveState(); + ASSERT_NE(findList(readRunningBgpConfigViaRpc(), kList), nullptr) + << "setup commit did not land the community-list in bgpd's running " + "config"; + + clearBgpSession(); + auto result = + runCli({"delete", "protocol", "bgp", "policy", "community-list", kList}); + EXPECT_THAT(result.stdout, HasSubstr("Successfully deleted")); + commitAndGetSha(); + ASSERT_TRUE(waitForBgpDaemonActive()) + << "bgpd did not return active after delete commit; state=" + << bgpDaemonActiveState(); + EXPECT_EQ(findList(readRunningBgpConfigViaRpc(), kList), nullptr) + << "deleted community-list still present in bgpd's running config"; +} diff --git a/fboss/cli/fboss2/tools/bgp_json_to_cli.py b/fboss/cli/fboss2/tools/bgp_json_to_cli.py index 71ed0f5d8f607..30fc6e9c0a472 100644 --- a/fboss/cli/fboss2/tools/bgp_json_to_cli.py +++ b/fboss/cli/fboss2/tools/bgp_json_to_cli.py @@ -488,6 +488,61 @@ def generate_as_path_list_commands(as_path_list: dict[str, Any]) -> list[str]: return commands +def generate_community_list_commands(community_list: dict[str, Any]) -> list[str]: + """Generate `config protocol bgp policy community-list` commands for one list. + + bgpd matches on `communities` (one `community` line each), `boolean_operator` + (emitted only when it differs from the OR default) and `exact_match`. + `community_list_names` and the `members` entries are never read by bgpd and + have no CLI spelling here, so they surface as warnings rather than vanishing. + """ + name = community_list.get("name", "") + if not name: + return [] + + prefix = f"config protocol bgp policy community-list {escape_shell_arg(name)}" + commands = [] + if community_list.get("description"): + commands.append( + f"{prefix} description {escape_shell_arg(community_list['description'])}" + ) + for community in community_list.get("communities") or []: + commands.append(f"{prefix} community {escape_shell_arg(community)}") + if "boolean_operator" in community_list: + operator = _boolean_operator_name(community_list["boolean_operator"]) + if operator == "NOT": + commands.append( + _warning( + f"community-list {name}: boolean_operator NOT is not " + "accepted by the CLI (bgpd treats it as AND); not emitted" + ) + ) + elif operator != "OR": + commands.append(f"{prefix} boolean-operator {escape_shell_arg(operator)}") + if "exact_match" in community_list: + commands.append( + f"{prefix} exact-match {_shell_bool(community_list['exact_match'])}" + ) + if community_list.get("community_list_names"): + commands.append( + _warning( + f"community-list {name}: community_list_names is not read by " + "bgpd and has no CLI equivalent; not emitted" + ) + ) + if community_list.get("members"): + commands.append( + _warning( + f"community-list {name}: members entries are not read by bgpd " + "(bgpd matches communities); not emitted" + ) + ) + if not commands: + # Nothing to set: still recreate the (empty) community-list by name. + commands.append(prefix) + return commands + + def generate_policy_commands(config: dict[str, Any]) -> list[str]: """Generate the `config protocol bgp policy ...` commands. @@ -499,6 +554,8 @@ def generate_policy_commands(config: dict[str, Any]) -> list[str]: commands = [] for as_path_list in policies.get("aspath_lists", []): commands.extend(generate_as_path_list_commands(as_path_list)) + for community_list in policies.get("community_lists", []): + commands.extend(generate_community_list_commands(community_list)) return commands diff --git a/fboss/cli/fboss2/tools/tests/test_bgp_json_to_cli.py b/fboss/cli/fboss2/tools/tests/test_bgp_json_to_cli.py index 0ced31be80c67..c1c240e7f9e6b 100644 --- a/fboss/cli/fboss2/tools/tests/test_bgp_json_to_cli.py +++ b/fboss/cli/fboss2/tools/tests/test_bgp_json_to_cli.py @@ -23,6 +23,7 @@ format_bandwidth, generate_as_path_list_commands, generate_commands, + generate_community_list_commands, generate_exec_commands, generate_global_commands, generate_peer_commands, @@ -1081,6 +1082,108 @@ def test_no_policies_key(self) -> None: self.assertEqual(generate_policy_commands({}), []) +class GenerateCommunityListCommandsTest(unittest.TestCase): + """Tests for generate_community_list_commands (community-list grammar).""" + + PREFIX = "config protocol bgp policy community-list CL" + + def test_empty_name_returns_empty(self) -> None: + self.assertEqual(generate_community_list_commands({"exact_match": True}), []) + + def test_bare_list_is_recreated(self) -> None: + self.assertEqual( + generate_community_list_commands({"name": "CL"}), [self.PREFIX] + ) + + def test_scalar_attributes(self) -> None: + commands = generate_community_list_commands( + { + "name": "CL", + "description": "transit communities", + "boolean_operator": 1, + "exact_match": True, + } + ) + self.assertEqual( + commands, + [ + f"{self.PREFIX} description 'transit communities'", + f"{self.PREFIX} boolean-operator AND", + f"{self.PREFIX} exact-match true", + ], + ) + + def test_community_per_value(self) -> None: + """Each communities value is one quoted `community` line, in order.""" + commands = generate_community_list_commands( + {"name": "CL", "communities": ["65000:100", "^65000:.*$"]} + ) + self.assertEqual( + commands, + [ + f"{self.PREFIX} community {escape_shell_arg('65000:100')}", + f"{self.PREFIX} community {escape_shell_arg('^65000:.*$')}", + ], + ) + + def test_injection_in_community_neutralized(self) -> None: + commands = generate_community_list_commands( + {"name": "CL", "communities": ["$(reboot)"]} + ) + self.assertEqual( + commands, [f"{self.PREFIX} community {escape_shell_arg('$(reboot)')}"] + ) + + def test_boolean_operator_or_omitted_not_warns(self) -> None: + self.assertEqual( + generate_community_list_commands({"name": "CL", "boolean_operator": "OR"}), + [self.PREFIX], + ) + commands = generate_community_list_commands( + {"name": "CL", "boolean_operator": 3} + ) + self.assertEqual(len(commands), 1) + self.assertTrue(commands[0].startswith("# WARNING:"), commands[0]) + self.assertIn("NOT", commands[0]) + + def test_exact_match_false_preserved(self) -> None: + self.assertEqual( + generate_community_list_commands({"name": "CL", "exact_match": False}), + [f"{self.PREFIX} exact-match false"], + ) + + def test_dead_fields_warn(self) -> None: + """Fields bgpd never reads surface as warnings, not silently dropped.""" + commands = generate_community_list_commands( + { + "name": "CL", + "community_list_names": ["X"], + "members": [{"community": {"name": "CM1", "value": "65000:1"}}], + } + ) + self.assertEqual(len(commands), 2) + self.assertIn("community_list_names", commands[0]) + self.assertIn("members entries", commands[1]) + for c in commands: + self.assertTrue(c.startswith("# WARNING:"), c) + + def test_policy_block_lists_after_as_path_lists(self) -> None: + config = { + "policies": { + "aspath_lists": [{"name": "A"}], + "community_lists": [{"name": "CL"}], + } + } + commands = generate_policy_commands(config) + self.assertEqual( + commands, + [ + "config protocol bgp policy as-path-list A", + "config protocol bgp policy community-list CL", + ], + ) + + class GenerateScriptCommandsTest(unittest.TestCase): """Tests for generate_exec_commands."""