From e4f6d94ec532488a1baae392550d8b20b9472f00 Mon Sep 17 00:00:00 2001 From: Enrique Estrada Date: Tue, 12 Aug 2025 13:42:21 -0600 Subject: [PATCH 01/16] issue244 solved --- aci-preupgrade-validation-script.py | 58 ++++++- docs/docs/validations.md | 15 +- .../global_pg_fvAEPg.json | 40 +++++ .../global_pg_l3extInstP.json | 3 + .../global_vzBrCP_pos.json | 24 +++ .../test_pg_and_shared_svc_contract_check.py | 147 ++++++++++++++++++ 6 files changed, 285 insertions(+), 2 deletions(-) create mode 100644 tests/pg_and_shared_svc_contract_check/global_pg_fvAEPg.json create mode 100644 tests/pg_and_shared_svc_contract_check/global_pg_l3extInstP.json create mode 100644 tests/pg_and_shared_svc_contract_check/global_vzBrCP_pos.json create mode 100644 tests/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index edfee703..99d3dbce 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -5297,6 +5297,62 @@ def apic_database_size_check(cversion, **kwargs): result = FAIL_UF return Result(result=result, headers=headers, data=data, recommended_action=recommended_action, doc_url=doc_url) +@check_wrapper(check_title='Shared Services Providers with Preferred Group enabled') +def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): + result= PASS + headers = ["Shared Service Contract", "Provider in Preferred Group", "PcTag"] + data = [] + recommended_action = 'an EPG in a Contract Preferred Group can consume a shared service contract, but cannot be a provider for a shared service contract.' + doc_url = 'https://datacenter.github.io/ACI-Pre-Upgrade-Validation-Script/validations/#preferred_group_shared_service_provider' + + if not tversion: + return Result(result=MANUAL, msg=TVER_MISSING) + # Configuration becomes faulted after 4.2-6d and 5.1-1h + if tversion.older_than("4.2(6d)"): + return Result(result=NA) + elif tversion.older_than("5.1(1h)"): + return Result(result=NA) + shrd_contracts_api = 'vzBrCP.json' + shrd_contracts_api += '?query-target-filter=and(eq(vzBrCP.scope,"global"))' + shrd_contracts = icurl('class', shrd_contracts_api) + if not shrd_contracts: + return Result(result=NA) + list_of_shrd_contracts =[] + for shrd_contract in shrd_contracts: + list_of_shrd_contracts.append(shrd_contract["vzBrCP"]["attributes"]["dn"]) + # Because of CSCwb32627 # Configuration only becomes faulted for extEPGs after 6.0(1g), normal epgs permitted. + if tversion.older_than("6.0(1g)"): + glbl_epgs_api = 'fvAEPg.json' + glbl_epgs_api += '?query-target-filter=and(le(fvAEPg.pcTag,"16385"),ge(fvAEPg.pcTag,"16"),eq(fvAEPg.prefGrMemb,"include"))' + glbl_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' + glbl_epgs = icurl('class', glbl_epgs_api) + if glbl_epgs: + for glbl_epg in glbl_epgs: + for prov_contract in glbl_epg["fvAEPg"]["children"]: + if prov_contract["fvRsProv"]["attributes"]["tDn"] in list_of_shrd_contracts: + contract = prov_contract["fvRsProv"]["attributes"]["tDn"] + pctag = glbl_epg["fvAEPg"]["attributes"]["pcTag"] + provider = glbl_epg["fvAEPg"]["attributes"]["dn"] + data.append([contract, provider, pctag]) + # Regardless of version, check extEPGs + glbl_ext_epgs_api = 'l3extInstP.json' + glbl_ext_epgs_api += '?query-target-filter=and(le(l3extInstP.pcTag,"16385"),ge(l3extInstP.pcTag,"16"),eq(l3extInstP.prefGrMemb,"include"))' + glbl_ext_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' + glbl_ext_epgs = icurl('class', glbl_ext_epgs_api) + if glbl_ext_epgs: + for glbl_ext_epg in glbl_ext_epgs: + for prov_ext_contract in glbl_ext_epg["l3extInstP"]["children"]: + if prov_ext_contract["fvRsProv"]["attributes"]["tDn"] in list_of_shrd_contracts: + contract = prov_ext_contract["fvRsProv"]["attributes"]["tDn"] + pctag = glbl_ext_epg["l3extInstP"]["attributes"]["pcTag"] + provider = glbl_ext_epg["l3extInstP"]["attributes"]["dn"] + data.append([contract, provider, pctag]) + + if data: + result = FAIL_O + + return Result(result=result, headers=headers, data=data, recommended_action=recommended_action, doc_url=doc_url) + # ---- Script Execution ---- def parse_args(args): @@ -5425,6 +5481,7 @@ def get_checks(api_only, debug_function): aes_encryption_check, service_bd_forceful_routing_check, ave_eol_check, + pg_and_shared_svc_contract_check, # Bugs ep_announce_check, @@ -5452,7 +5509,6 @@ def get_checks(api_only, debug_function): standby_sup_sync_check, stale_pcons_ra_mo_check, isis_database_byte_check, - ] conn_checks = [ # General diff --git a/docs/docs/validations.md b/docs/docs/validations.md index e46a8815..bf730e88 100644 --- a/docs/docs/validations.md +++ b/docs/docs/validations.md @@ -131,7 +131,7 @@ Items | Faults | This Script [Global AES Encryption][c21] | :white_check_mark: | :white_check_mark: 6.1(2) | :no_entry_sign: [Service Graph BD Forceful Routing][c22] | :white_check_mark: | :no_entry_sign: | :no_entry_sign: [AVE End-of-life][c23] | :white_check_mark: | :no_entry_sign: | :no_entry_sign: - +[Preferred Group Shared Service Provider][c23] | :white_check_mark: | :no_entry_sign: | :no_entry_sign: [c1]: #vpc-paired-leaf-switches [c2]: #overlapping-vlan-pool @@ -156,6 +156,7 @@ Items | Faults | This Script [c21]: #global-aes-encryption [c22]: #service-graph-bd-forceful-routing [c23]: #ave-end-of-life +[c24]: #preferred_group_shared_service_provider ### Defect Condition Checks @@ -2221,6 +2222,16 @@ As outlined in the [End-of-Sale and End-of-Life Announcement for Cisco Applicati If planning an upgrade to 6.0+, review the [Cisco ACI Virtual Edge Migration Guide][56] and complete a domain migration prior to performing the upgrade. +### Preferred Group Shared Service Provider + +As detailed in the[ACI Policy Model][59] Starting in 4.2(6d) and later, or 5.1(1h) and later an EPG in a Contract Preferred Group can consume a shared service contract, but cannot be a provider for a shared service contract with an L3Out EPG as consumer. + +Due to the behavior change contracts will no longer be pushed post-upgrade causing a significant datacenter outage. + +The configuration will be faulted: e.g. F0467 Configuration failed for uni/tn-common/brc-shared/contract/epgCont-1/instp-ext-epg due to Invalid Contract Configuration, debug message: invalid-contract-config: Shared service provider cannot be in a Preferred Group. + + +Starting version 6.0(1g), this validation is relaxed for normal EPGs but is still not allowed for External EPGs. ## Defect Check Details @@ -2648,3 +2659,5 @@ Do not upgrade to any affected ACI software release if this check fails. [56]: https://www.cisco.com/c/en/us/td/docs/dcn/whitepapers/cisco-aci-virtual-edge-migration.html [57]: https://bst.cloudapps.cisco.com/bugsearch/bug/CSCwp22212 [58]: https://bst.cloudapps.cisco.com/bugsearch/bug/CSCwp15375 +[59]: https://www.cisco.com/c/en/us/td/docs/switches/datacenter/aci/apic/sw/5-x/aci-fundamentals/cisco-aci-fundamentals-50x/m_policy-model.html#concept_tds_vcc_fy + diff --git a/tests/pg_and_shared_svc_contract_check/global_pg_fvAEPg.json b/tests/pg_and_shared_svc_contract_check/global_pg_fvAEPg.json new file mode 100644 index 00000000..a7af4538 --- /dev/null +++ b/tests/pg_and_shared_svc_contract_check/global_pg_fvAEPg.json @@ -0,0 +1,40 @@ +[ +{"fvAEPg": +{"attributes": +{"annotation":"","childAction":"","configIssues":"","configSt":"applied","descr":"","dn":"uni/tn-common/ap-apptest/epg-epg1","exceptionTag":"","extMngdBy":"","floodOnEncap":"disabled","fwdCtrl":"","hasMcastSource":"no","isAttrBasedEPg":"no","isSharedSrvMsiteEPg":"no","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-08-07T18:05:49.910+00:00","monPolDn":"uni/tn-common/monepg-default","name":"epg1","nameAlias":"","pcEnfPref":"unenforced","pcTag":"5555","pcTagAllocSrc":"idmanager","prefGrMemb":"include","prio":"unspecified","scope":"2261001","shutdown":"no","status":"","triggerSt":"triggerable","txId":"1729382256913953811","uid":"15374","userdom":":all:"},"children":[ +{"fvRsProv": +{"attributes": +{"accessPrivilege":"USER","annotation":"","childAction":"","ctrctUpd":"ctrct","extMngdBy":"","forceResolve":"yes","intent":"install","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-08-07T18:05:49.910+00:00","monPolDn":"uni/tn-common/monepg-default","prio":"unspecified","rType":"mo","rn":"rsprov-AD_C","state":"formed","stateQual":"none","status":"","tCl":"vzBrCP","tContextDn":"","tDn":"uni/tn-common/brc-AD_C","tRn":"brc-AD_C","tType":"name","tnVzBrCPName":"AD_C","triggerSt":"triggerable","uid":"15374","updateCollection":"no","userdom":":all:"}}}]}}, +{"fvAEPg": +{"attributes": +{"annotation":"orchestrator:msc-shadow:no","childAction":"","configIssues":"","configSt":"applied","descr":"","dn":"uni/tn-AJ-PROD/ap-PTC_AP1/epg-PTC_EPG4","exceptionTag":"","extMngdBy":"","floodOnEncap":"disabled","fwdCtrl":"","hasMcastSource":"no","isAttrBasedEPg":"no","isSharedSrvMsiteEPg":"yes","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-08-06T23:29:28.934+00:00","monPolDn":"uni/tn-common/monepg-default","name":"PTC_EPG4","nameAlias":"","pcEnfPref":"unenforced","pcTag":"44","pcTagAllocSrc":"idmanager","prefGrMemb":"include","prio":"unspecified","scope":"2555906","shutdown":"no","status":"","triggerSt":"triggerable","txId":"7493989779944511091","uid":"0","userdom":"all"},"children":[ +{"fvRsProv": +{"attributes": +{"accessPrivilege":"USER","annotation":"orchestrator:msc-shadow:no","childAction":"","ctrctUpd":"ctrct","extMngdBy":"","forceResolve":"yes","intent":"install","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-08-06T23:29:28.934+00:00","monPolDn":"uni/tn-common/monepg-default","prio":"unspecified","rType":"mo","rn":"rsprov-common_contract","state":"formed","stateQual":"none","status":"","tCl":"vzBrCP","tContextDn":"","tDn":"uni/tn-common/brc-common_contract","tRn":"brc-common_contract","tType":"name","tnVzBrCPName":"common_contract","triggerSt":"triggerable","uid":"0","updateCollection":"no","userdom":"all"}}}]}}, +{"fvAEPg": +{"attributes": +{"annotation":"","childAction":"","configIssues":"","configSt":"applied","descr":"","dn":"uni/tn-vavictor_TenantA/ap-eApp_AP/epg-App_EPG","exceptionTag":"","extMngdBy":"","floodOnEncap":"disabled","fwdCtrl":"","hasMcastSource":"no","isAttrBasedEPg":"no","isSharedSrvMsiteEPg":"no","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-07-20T00:45:30.670+00:00","monPolDn":"uni/tn-common/monepg-default","name":"App_EPG","nameAlias":"","pcEnfPref":"unenforced","pcTag":"10932","pcTagAllocSrc":"idmanager","prefGrMemb":"include","prio":"level3","scope":"2818049","shutdown":"no","status":"","triggerSt":"triggerable","txId":"15564440312192537861","uid":"15374","userdom":":all:"},"children":[ +{"fvRsProv": +{"attributes": +{"accessPrivilege":"USER","annotation":"","childAction":"","ctrctUpd":"ctrct","extMngdBy":"","forceResolve":"yes","intent":"install","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-07-20T00:45:30.670+00:00","monPolDn":"uni/tn-common/monepg-default","prio":"unspecified","rType":"mo","rn":"rsprov-BackEnd_Ctrt","state":"formed","stateQual":"none","status":"","tCl":"vzBrCP","tContextDn":"","tDn":"uni/tn-vavictor_TenantA/brc-BackEnd_Ctrt","tRn":"brc-BackEnd_Ctrt","tType":"name","tnVzBrCPName":"BackEnd_Ctrt","triggerSt":"triggerable","uid":"13299","updateCollection":"no","userdom":":"}}}, +{"fvRsProv": +{"attributes": +{"accessPrivilege":"USER","annotation":"","childAction":"","ctrctUpd":"ctrct","extMngdBy":"","forceResolve":"yes","intent":"install","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-07-20T00:45:30.670+00:00","monPolDn":"uni/tn-common/monepg-default","prio":"unspecified","rType":"mo","rn":"rsprov-L3Out_Ctrt","state":"formed","stateQual":"none","status":"","tCl":"vzBrCP","tContextDn":"","tDn":"uni/tn-vavictor_TenantA/brc-L3Out_Ctrt","tRn":"brc-L3Out_Ctrt","tType":"name","tnVzBrCPName":"L3Out_Ctrt","triggerSt":"triggerable","uid":"15374","updateCollection":"no","userdom":":all:"}}}, +{"fvRsProv": +{"attributes": +{"accessPrivilege":"USER","annotation":"","childAction":"","ctrctUpd":"ctrct","extMngdBy":"","forceResolve":"yes","intent":"install","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-07-20T00:45:30.670+00:00","monPolDn":"uni/tn-common/monepg-default","prio":"unspecified","rType":"mo","rn":"rsprov-FrontEnd_Ctrt","state":"formed","stateQual":"none","status":"","tCl":"vzBrCP","tContextDn":"","tDn":"uni/tn-vavictor_TenantA/brc-FrontEnd_Ctrt","tRn":"brc-FrontEnd_Ctrt","tType":"name","tnVzBrCPName":"FrontEnd_Ctrt","triggerSt":"triggerable","uid":"15374","updateCollection":"no","userdom":":all:"}}}, +{"fvRsProv": +{"attributes": +{"accessPrivilege":"USER","annotation":"","childAction":"","ctrctUpd":"ctrct","extMngdBy":"","forceResolve":"yes","intent":"install","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-07-20T00:45:30.670+00:00","monPolDn":"uni/tn-common/monepg-default","prio":"unspecified","rType":"mo","rn":"rsprov-vavictor_from_Scan","state":"formed","stateQual":"none","status":"","tCl":"vzBrCP","tContextDn":"","tDn":"uni/tn-common/brc-vavictor_from_Scan","tRn":"brc-vavictor_from_Scan","tType":"name","tnVzBrCPName":"vavictor_from_Scan","triggerSt":"triggerable","uid":"15374","updateCollection":"no","userdom":":all:"}}}]}}, +{"fvAEPg": +{"attributes": +{"annotation":"","childAction":"","configIssues":"","configSt":"applied","descr":"","dn":"uni/tn-vavictor_TenantA/ap-eApp_AP/epg-Web_EPG","exceptionTag":"","extMngdBy":"","floodOnEncap":"disabled","fwdCtrl":"","hasMcastSource":"no","isAttrBasedEPg":"no","isSharedSrvMsiteEPg":"no","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-04-20T20:22:26.475+00:00","monPolDn":"uni/tn-common/monepg-default","name":"Web_EPG","nameAlias":"","pcEnfPref":"unenforced","pcTag":"5476","pcTagAllocSrc":"idmanager","prefGrMemb":"include","prio":"level3","scope":"2818049","shutdown":"no","status":"","triggerSt":"triggerable","txId":"15564440312192537861","uid":"15374","userdom":":all:"},"children":[ +{"fvRsProv": +{"attributes": +{"accessPrivilege":"USER","annotation":"","childAction":"","ctrctUpd":"ctrct","extMngdBy":"","forceResolve":"yes","intent":"install","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-04-20T20:22:26.475+00:00","monPolDn":"uni/tn-common/monepg-default","prio":"unspecified","rType":"mo","rn":"rsprov-EPG_2_uSeg_Ctrt","state":"formed","stateQual":"none","status":"","tCl":"vzBrCP","tContextDn":"","tDn":"uni/tn-vavictor_TenantA/brc-EPG_2_uSeg_Ctrt","tRn":"brc-EPG_2_uSeg_Ctrt","tType":"name","tnVzBrCPName":"EPG_2_uSeg_Ctrt","triggerSt":"triggerable","uid":"15374","updateCollection":"no","userdom":":all:"}}}, +{"fvRsProv": +{"attributes": +{"accessPrivilege":"USER","annotation":"","childAction":"","ctrctUpd":"ctrct","extMngdBy":"","forceResolve":"yes","intent":"install","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-04-20T20:22:26.475+00:00","monPolDn":"uni/tn-common/monepg-default","prio":"unspecified","rType":"mo","rn":"rsprov-L3Out_Ctrt","state":"formed","stateQual":"none","status":"","tCl":"vzBrCP","tContextDn":"","tDn":"uni/tn-vavictor_TenantA/brc-L3Out_Ctrt","tRn":"brc-L3Out_Ctrt","tType":"name","tnVzBrCPName":"L3Out_Ctrt","triggerSt":"triggerable","uid":"15374","updateCollection":"no","userdom":":all:"}}}, +{"fvRsProv": +{"attributes": +{"accessPrivilege":"USER","annotation":"","childAction":"","ctrctUpd":"ctrct","extMngdBy":"","forceResolve":"yes","intent":"install","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-04-20T21:41:40.628+00:00","monPolDn":"uni/tn-common/monepg-default","prio":"unspecified","rType":"mo","rn":"rsprov-vavictor_from_Scan","state":"formed","stateQual":"none","status":"","tCl":"vzBrCP","tContextDn":"","tDn":"uni/tn-common/brc-vavictor_from_Scan","tRn":"brc-vavictor_from_Scan","tType":"name","tnVzBrCPName":"vavictor_from_Scan","triggerSt":"triggerable","uid":"15374","updateCollection":"no","userdom":":all:"}}}]}}] \ No newline at end of file diff --git a/tests/pg_and_shared_svc_contract_check/global_pg_l3extInstP.json b/tests/pg_and_shared_svc_contract_check/global_pg_l3extInstP.json new file mode 100644 index 00000000..98ffc9f3 --- /dev/null +++ b/tests/pg_and_shared_svc_contract_check/global_pg_l3extInstP.json @@ -0,0 +1,3 @@ +[ +{"l3extInstP":{"attributes":{"annotation":"","childAction":"","configIssues":"","configSt":"applied","descr":"","dn":"uni/tn-common/out-test-L3Out/instP-testExtEPG","exceptionTag":"","extMngdBy":"","floodOnEncap":"disabled","isSharedSrvMsiteEPg":"no","lcOwn":"local","matchT":"AtleastOne","mcast":"no","modTs":"2025-08-11T18:14:54.961+00:00","monPolDn":"uni/tn-common/monepg-default","name":"testExtEPG","nameAlias":"","pcEnfPref":"unenforced","pcTag":"10953","pcTagAllocSrc":"idmanager","prefGrMemb":"include","prio":"unspecified","scope":"2490368","status":"","targetDscp":"unspecified","triggerSt":"triggerable","txId":"1729382256913953508","uid":"15374","userdom":":all:"}, +"children":[{"fvRsProv":{"attributes":{"accessPrivilege":"USER","annotation":"","childAction":"","ctrctUpd":"ctrct","extMngdBy":"","forceResolve":"yes","intent":"install","lcOwn":"local","matchT":"AtleastOne","modTs":"2025-08-11T18:14:54.961+00:00","monPolDn":"uni/tn-common/monepg-default","prio":"unspecified","rType":"mo","rn":"rsprov-AD_C","state":"formed","stateQual":"none","status":"","tCl":"vzBrCP","tContextDn":"","tDn":"uni/tn-common/brc-AD_C","tRn":"brc-AD_C","tType":"name","tnVzBrCPName":"AD_C","triggerSt":"triggerable","uid":"15374","updateCollection":"no","userdom":":all:"}}}]}}] \ No newline at end of file diff --git a/tests/pg_and_shared_svc_contract_check/global_vzBrCP_pos.json b/tests/pg_and_shared_svc_contract_check/global_vzBrCP_pos.json new file mode 100644 index 00000000..ea548bf7 --- /dev/null +++ b/tests/pg_and_shared_svc_contract_check/global_vzBrCP_pos.json @@ -0,0 +1,24 @@ +[ + {"vzBrCP":{ + "attributes":{ + "accessPrivilege":"USER", + "annotation":"", + "childAction":"", + "configIssues":"", + "descr":"", + "dn":"uni/tn-common/brc-AD_C", + "name":"AD_C", + "scope":"global" + } + } + }, + {"vzBrCP":{ + "attributes":{ + "accessPrivilege":"USER", + "dn":"uni/tn-laumende/brc-test", + "name":"test", + "scope":"global" + } + } + } + ] \ No newline at end of file diff --git a/tests/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py b/tests/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py new file mode 100644 index 00000000..e271870d --- /dev/null +++ b/tests/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py @@ -0,0 +1,147 @@ +import os +import pytest +import logging +import importlib +from helpers.utils import read_data + +script = importlib.import_module("aci-preupgrade-validation-script") + +log = logging.getLogger(__name__) +dir = os.path.dirname(os.path.abspath(__file__)) + + +# icurl queries +# shared contracts +shrd_contracts_api = 'vzBrCP.json' +shrd_contracts_api += '?query-target-filter=and(eq(vzBrCP.scope,"global"))' + +# global epgs ( 16 <= pgtag <= 16385) with Preferred group enabled with provided contracts + +glbl_epgs_api = 'fvAEPg.json' +glbl_epgs_api += '?query-target-filter=and(le(fvAEPg.pcTag,"16385"),ge(fvAEPg.pcTag,"16"),eq(fvAEPg.prefGrMemb,"include"))' +glbl_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' + +# global external Epgs ( 16 <= pgtag <= 16385) with Preferred group enabled with provided contracts + +glbl_ext_epgs_api = 'l3extInstP.json' +glbl_ext_epgs_api += '?query-target-filter=and(le(l3extInstP.pcTag,"16385"),ge(l3extInstP.pcTag,"16"),eq(l3extInstP.prefGrMemb,"include"))' +glbl_ext_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' + + +@pytest.mark.parametrize( + "icurl_outputs, cversion, tversion, expected_result", + [ + + # MANUAL cases + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + }, + "4.2(4a)", None, + script.MANUAL, + ), + # NA cases + # Target version is lower than 4.2(6d), Result = NA + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + }, + "4.2(1a)", "4.2(6c)", + script.NA, + ), + # Target version is lower than 5.1(1h), Result = NA + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + }, + "4.2(1a)", "5.1(1g)", + script.NA, + ), + # There are no global contracts, Result = NA + ( + { + shrd_contracts_api: [], + glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + }, + "4.2(1a)", "6.1(1g)", + script.NA, + ), + # FAIL_O Cases + # Target version is older than 6.0(1g), Result = FAIL_O + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + }, + "4.2(1a)", "6.0(1f)", + script.FAIL_O, + ), + # Target version is newer than 6.0(1g), both global_pg EPGs and extEPGs , Result = FAIL_O + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + }, + "4.2(1a)", "6.0(1g)", + script.FAIL_O, + ), + # Target version is newer than 6.0(1g), no EPGS, only global_pg extEPGs , Result = FAIL_O + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [], + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + }, + "4.2(1a)", "6.0(1g)", + script.FAIL_O, + ), + # PASS Cases + # Target version is older than 6.0(1g), no global_pg EPGs or extEPGs , Result = PASS + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [], + glbl_ext_epgs_api: [] + }, + "4.2(1a)", "6.0(1f)", + script.PASS, + ), + # Target version is newer than 6.0(1g), no global_pg EPGs or extEPGs , Result = PASS + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [], + glbl_ext_epgs_api: [] + }, + "4.2(1a)", "6.0(1h)", + script.PASS, + ), + # Target version is newer than 6.0(1g), only global_pg EPGs , no global_pg extEPGs , Result = PASS + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), + glbl_ext_epgs_api: [] + }, + "4.2(1a)", "6.0(1h)", + script.PASS, + ), + ] +) +def test_logic(mock_icurl, cversion, tversion, expected_result): + result = script.pg_and_shared_svc_contract_check( + 1, + 1, + script.AciVersion(cversion), + script.AciVersion(tversion) if tversion else None + ) + assert result == expected_result From 04fef1969939d83070431082c75b9dfb88275519 Mon Sep 17 00:00:00 2001 From: GM Date: Fri, 27 Feb 2026 11:08:57 -0500 Subject: [PATCH 02/16] Add arg to define thread limit - to throttle concurrent API calls when required (#355) * add `--max-threads` arg * fix bad descriptor errs/race conditions * update pytests --- .gitignore | 1 + aci-preupgrade-validation-script.py | 39 ++++++++++++++++++++++++----- tests/test_CheckManager.py | 7 ++++-- tests/test_ThreadManager.py | 38 +++++++++++++++++++++++----- tests/test_parse_args.py | 13 ++++++++++ 5 files changed, 84 insertions(+), 14 deletions(-) diff --git a/.gitignore b/.gitignore index 21e24437..b8086d32 100644 --- a/.gitignore +++ b/.gitignore @@ -50,6 +50,7 @@ coverage.xml .hypothesis/ .pytest_cache/ cover/ +preupgrade_validator*.tgz # Translations *.mo diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index 4b83f4c7..bb01c456 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -1023,6 +1023,7 @@ def __init__( common_kwargs, monitor_interval=0.5, # sec monitor_timeout=600, # sec + max_threads=None, callback_on_monitoring=None, callback_on_start_failure=None, callback_on_timeout=None, @@ -1030,6 +1031,9 @@ def __init__( self.funcs = funcs self.threads = None self.common_kwargs = common_kwargs + # Semaphore to cap the number of concurrently running check threads. + # None means unlimited. + self.semaphore = threading.Semaphore(max_threads) if max_threads and max_threads > 0 else None # Not using `thread.join(timeout)` because it waits for each thread sequentially, # which means the program may wait for "timeout * num of threads" at worst case. @@ -1053,7 +1057,7 @@ def start(self): raise RuntimeError("Threading on going. Cannot start again.") self.threads = [ - self._generate_thread(target=func, kwargs=self.common_kwargs) + self._generate_thread(target=func, kwargs=self.common_kwargs, use_semaphore=True) for func in self.funcs ] @@ -1080,9 +1084,19 @@ def join(self): def is_timeout(self): return self.timeout_event.is_set() - def _generate_thread(self, target, args=(), kwargs=None): + def _generate_thread(self, target, args=(), kwargs=None, use_semaphore=False): if kwargs is None: kwargs = {} + if use_semaphore and self.semaphore is not None: + semaphore = self.semaphore + original_target = target + def _wrapped_target(*a, **kw): + try: + original_target(*a, **kw) + finally: + semaphore.release() + _wrapped_target.__name__ = target.__name__ + target = _wrapped_target thread = CustomThread( target=target, name=target.__name__, args=args, kwargs=kwargs ) @@ -1102,11 +1116,16 @@ def _start_thread(self, thread): thread_started = False while not self.is_timeout(): try: + if self.semaphore is not None: + log.info("({}) Waiting for an available thread slot.".format(thread.name)) + self.semaphore.acquire() log.info("({}) Starting thread.".format(thread.name)) thread.start() thread_started = True break except RuntimeError as e: + if self.semaphore is not None: + self.semaphore.release() if str(e) != "can't start new thread": log.error("({}) Unexpected error to start a thread.".format(thread.name), exc_info=True) break @@ -1121,6 +1140,8 @@ def _start_thread(self, thread): time_elapsed += queue_interval continue except Exception: + if self.semaphore is not None: + self.semaphore.release() log.error("({}) Unexpected error to start a thread.".format(thread.name), exc_info=True) break @@ -1493,11 +1514,14 @@ def get_row(widths, values, spad=" ", lpad=""): def prints(objects, sep=' ', end='\n'): with open(RESULT_FILE, 'a') as f: - print(objects, sep=sep, end=end, file=sys.stdout) + try: + print(objects, sep=sep, end=end, file=sys.stdout) + sys.stdout.flush() + except OSError: + pass if end == "\r": end = "\n" # easier to read with \n in a log file print(objects, sep=sep, end=end, file=f) - sys.stdout.flush() f.flush() @@ -6039,6 +6063,7 @@ def parse_args(args): parser.add_argument("-v", "--version", action="store_true", help="Only show the script version, then end.") parser.add_argument("--total-checks", action="store_true", help="Only show the total number of checks, then end.") parser.add_argument("--timeout", action="store", nargs="?", type=int, const=-1, default=DEFAULT_TIMEOUT, help="Show default script timeout (sec) or overwrite it when a number is provided (e.g. --timeout 1200).") + parser.add_argument("--max-threads", action="store", type=int, default=None, help="Maximum number of check threads to run concurrently. Defaults to unlimited.") parsed_args = parser.parse_args(args) return parsed_args @@ -6209,11 +6234,12 @@ class CheckManager: apic_ca_cert_validation, ] - def __init__(self, api_only=False, debug_function="", timeout=600, monitor_interval=0.5): + def __init__(self, api_only=False, debug_function="", timeout=600, monitor_interval=0.5, max_threads=None): self.api_only = api_only self.debug_function = debug_function self.monitor_interval = monitor_interval # sec self.monitor_timeout = timeout # sec + self.max_threads = max_threads self.timeout_event = None self.check_funcs = self.get_check_funcs() @@ -6284,6 +6310,7 @@ def run_checks(self, common_data): common_kwargs=dict({"finalize_check": self.finalize_check}, **common_data), monitor_interval=self.monitor_interval, monitor_timeout=self.monitor_timeout, + max_threads=self.max_threads, callback_on_monitoring=print_progress, callback_on_start_failure=self.finalize_check_on_thread_failure, callback_on_timeout=self.finalize_check_on_thread_timeout, @@ -6303,7 +6330,7 @@ def main(_args=None): print("Timeout(sec): {}".format(DEFAULT_TIMEOUT)) return - cm = CheckManager(args.api_only, args.debug_function, args.timeout) + cm = CheckManager(args.api_only, args.debug_function, args.timeout, max_threads=args.max_threads) if args.total_checks: print("Total Number of Checks: {}".format(cm.total_checks)) diff --git a/tests/test_CheckManager.py b/tests/test_CheckManager.py index e5c7e794..fe4a7eea 100644 --- a/tests/test_CheckManager.py +++ b/tests/test_CheckManager.py @@ -41,7 +41,7 @@ def mock_generate_thread(monkeypatch, request): def thread_start_with_exception(timeout=5.0): raise exception - def _mock_generate_thread(self, target, args=(), kwargs=None): + def _mock_generate_thread(self, target, args=(), kwargs=None, use_semaphore=False): if kwargs is None: kwargs = {} thread = script.CustomThread(target=target, name=target.__name__, args=args, kwargs=kwargs) @@ -85,7 +85,10 @@ def test_initialize_checks(self, caplog, cm): assert cm.get_check_result("fake_10_check") is None # Check number of initialized checks in result files - result_files = os.listdir(script.JSON_DIR) + result_files = [ + f for f in os.listdir(script.JSON_DIR) + if f.replace(".json", "") in cm.check_ids + ] assert len(result_files) == cm.total_checks # Check the filename of result files and their `ruleStatus` diff --git a/tests/test_ThreadManager.py b/tests/test_ThreadManager.py index 4b02f3bb..65e3a44b 100644 --- a/tests/test_ThreadManager.py +++ b/tests/test_ThreadManager.py @@ -9,41 +9,67 @@ def task1(data=""): - time.sleep(2.5) + time.sleep(2) if not global_timeout: print("Thread task1: Finishing with data {}".format(data)) def task2(data=""): - time.sleep(0.5) + time.sleep(2.5) if not global_timeout: print("Thread task2: Finishing with data {}".format(data)) def task3(data=""): - time.sleep(0.2) + time.sleep(1) if not global_timeout: print("Thread task3: Finishing with data {}".format(data)) +def task4(data=""): + time.sleep(5) + if not global_timeout: + print("Thread task4: Finishing with data {}".format(data)) + + +def task5(data=""): + time.sleep(5) + if not global_timeout: + print("Thread task5: Finishing with data {}".format(data)) + + def test_ThreadManager(capsys): global global_timeout tm = script.ThreadManager( - funcs=[task1, task2, task3], + funcs=[task1, task2, task3, task4, task5], common_kwargs={"data": "common_data"}, monitor_timeout=1, + max_threads=2, callback_on_timeout=lambda x: print("Timeout. Abort {}".format(x)) ) tm.start() tm.join() + # Join each task thread to ensure any in-progress prints complete before + # capsys.readouterr() is called. Without this there is a race where a + # thread passes the `if not global_timeout` check and then tries to print + # after pytest has already torn down the captured stdout fd, causing + # OSError: [Errno 9] Bad file descriptor. + for thread in tm.threads: + try: + thread.join(timeout=1.5) + except RuntimeError: + pass # thread was never started + if tm.is_timeout(): global_timeout = True expected_output = """\ -Thread task3: Finishing with data common_data -Thread task2: Finishing with data common_data Timeout. Abort task1 +Timeout. Abort task2 +Thread task1: Finishing with data common_data +Thread task2: Finishing with data common_data +Thread task3: Finishing with data common_data """ captured = capsys.readouterr() assert captured.out == expected_output diff --git a/tests/test_parse_args.py b/tests/test_parse_args.py index a460da9d..ac8ec5d2 100644 --- a/tests/test_parse_args.py +++ b/tests/test_parse_args.py @@ -22,6 +22,7 @@ def test_no_args(): assert args.no_cleanup is False assert args.version is False assert args.total_checks is False + assert args.max_threads is None @pytest.mark.parametrize( @@ -127,3 +128,15 @@ def test_version(args, expected_result): def test_total_checks(args, expected_result): args = script.parse_args(args) assert args.total_checks == expected_result + + +@pytest.mark.parametrize( + "args, expected_result", + [ + ([], None), + (["--max-threads", "4"], 4), + ], +) +def test_max_threads(args, expected_result): + args = script.parse_args(args) + assert args.max_threads == expected_result From 233359a6c9783b5b2f7c670e27572592045fe34a Mon Sep 17 00:00:00 2001 From: GM Date: Fri, 27 Feb 2026 11:11:20 -0500 Subject: [PATCH 03/16] 353 the script incorrectly detects vpc and port channel interfaces as cscwh68103 invalid fabricpathep targets (#357) * specific testing for known failure conditions of cscwh68103 as to not catch valid scenarios --- aci-preupgrade-validation-script.py | 9 ++++++++- .../fabricPathEP_target_check/infraRsHPathAtt_neg.json | 8 ++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index bb01c456..67fe1932 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -4738,6 +4738,7 @@ def fabricPathEp_target_check(**kwargs): fex_a = groups.get("fexA") fex_b = groups.get("fexB") path = groups.get("path") + print(path) # CHECK FEX ID(s) of extpath(s) is 101 or greater if fex_a: @@ -4772,7 +4773,13 @@ def fabricPathEp_target_check(**kwargs): elif int(third) > 16: data.append([dn, "eth port {} is invalid (1-16 expected) for breakout ports".format(third)]) else: - data.append([dn, "PathEp 'eth' syntax is invalid"]) + # CHECK eth1//0 malform scenario (double slashes) + if "//" in path: + data.append([dn, "PathEp 'eth' syntax is invalid"]) + # CHECK Ethx/y malform scenario (should not be caps) + elif path.startswith("Eth"): + data.append([dn, "PathEp 'eth' should be lowercase 'eth'"]) + else: data.append([dn, "target is not a valid fabricPathEp DN"]) diff --git a/tests/checks/fabricPathEP_target_check/infraRsHPathAtt_neg.json b/tests/checks/fabricPathEP_target_check/infraRsHPathAtt_neg.json index cf19d8af..3c0ed122 100644 --- a/tests/checks/fabricPathEP_target_check/infraRsHPathAtt_neg.json +++ b/tests/checks/fabricPathEP_target_check/infraRsHPathAtt_neg.json @@ -14,5 +14,13 @@ "tDn": "topology/pod-1/paths-101/pathep-[eth1/1]" } } + }, { + "infraRsHPathAtt": { + "attributes": { + "dn": "uni/infra/hpaths-__ui_xxx_201-202_Eth49-50/rsHPathAtt-[topology/pod-1/paths-201/pathep-[xxx_201-202_Eth49-50]]", + "tCl": "fabricPathEp", + "tDn": "topology/pod-1/paths-201/pathep-[xxx_201-202_Eth49-50]" + } + } } ] \ No newline at end of file From 6d7d4d46c43cc7d429e91a71637aad8b18b7496c Mon Sep 17 00:00:00 2001 From: GM Date: Fri, 27 Feb 2026 13:32:56 -0500 Subject: [PATCH 04/16] Update aci-preupgrade-validation-script.py mark version --- aci-preupgrade-validation-script.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index 67fe1932..e7a73ce6 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -38,7 +38,7 @@ import os import re -SCRIPT_VERSION = "v4.0.1" +SCRIPT_VERSION = "v4.1.0-dev" DEFAULT_TIMEOUT = 600 # sec # result constants DONE = 'DONE' From e4defbfd28919d59fb799d1ebb8be93b91810962 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Fri, 27 Feb 2026 13:45:22 -0500 Subject: [PATCH 05/16] print cleanup --- aci-preupgrade-validation-script.py | 1 - 1 file changed, 1 deletion(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index e7a73ce6..6c218b13 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -4738,7 +4738,6 @@ def fabricPathEp_target_check(**kwargs): fex_a = groups.get("fexA") fex_b = groups.get("fexB") path = groups.get("path") - print(path) # CHECK FEX ID(s) of extpath(s) is 101 or greater if fex_a: From 080308c179f49d573d0bad1d47e7ca8bed4ca93f Mon Sep 17 00:00:00 2001 From: Gabriel Date: Fri, 27 Feb 2026 14:47:27 -0500 Subject: [PATCH 06/16] move test into checks --- .../pg_and_shared_svc_contract_check/global_pg_fvAEPg.json | 0 .../pg_and_shared_svc_contract_check/global_pg_l3extInstP.json | 0 .../pg_and_shared_svc_contract_check/global_vzBrCP_pos.json | 0 .../test_pg_and_shared_svc_contract_check.py | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename tests/{ => checks}/pg_and_shared_svc_contract_check/global_pg_fvAEPg.json (100%) rename tests/{ => checks}/pg_and_shared_svc_contract_check/global_pg_l3extInstP.json (100%) rename tests/{ => checks}/pg_and_shared_svc_contract_check/global_vzBrCP_pos.json (100%) rename tests/{ => checks}/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py (100%) diff --git a/tests/pg_and_shared_svc_contract_check/global_pg_fvAEPg.json b/tests/checks/pg_and_shared_svc_contract_check/global_pg_fvAEPg.json similarity index 100% rename from tests/pg_and_shared_svc_contract_check/global_pg_fvAEPg.json rename to tests/checks/pg_and_shared_svc_contract_check/global_pg_fvAEPg.json diff --git a/tests/pg_and_shared_svc_contract_check/global_pg_l3extInstP.json b/tests/checks/pg_and_shared_svc_contract_check/global_pg_l3extInstP.json similarity index 100% rename from tests/pg_and_shared_svc_contract_check/global_pg_l3extInstP.json rename to tests/checks/pg_and_shared_svc_contract_check/global_pg_l3extInstP.json diff --git a/tests/pg_and_shared_svc_contract_check/global_vzBrCP_pos.json b/tests/checks/pg_and_shared_svc_contract_check/global_vzBrCP_pos.json similarity index 100% rename from tests/pg_and_shared_svc_contract_check/global_vzBrCP_pos.json rename to tests/checks/pg_and_shared_svc_contract_check/global_vzBrCP_pos.json diff --git a/tests/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py similarity index 100% rename from tests/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py rename to tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py From 3e53f03bf5eb9c4d45799a0058a7de7c1486fa13 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Fri, 27 Feb 2026 15:00:22 -0500 Subject: [PATCH 07/16] fix pytest --- .../test_pg_and_shared_svc_contract_check.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py index e271870d..d363caea 100644 --- a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py +++ b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py @@ -9,6 +9,7 @@ log = logging.getLogger(__name__) dir = os.path.dirname(os.path.abspath(__file__)) +test_function = "pg_and_shared_svc_contract_check" # icurl queries # shared contracts @@ -137,11 +138,9 @@ ), ] ) -def test_logic(mock_icurl, cversion, tversion, expected_result): - result = script.pg_and_shared_svc_contract_check( - 1, - 1, - script.AciVersion(cversion), - script.AciVersion(tversion) if tversion else None +def test_logic(run_check, mock_icurl, cversion, tversion, expected_result): + result = run_check( + cversion=script.AciVersion(cversion), + tversion=script.AciVersion(tversion) if tversion else None ) - assert result == expected_result + assert result.result == expected_result From 6f55a21a75252f8643790217caa462a280aa2e0c Mon Sep 17 00:00:00 2001 From: jeestr4d <168469245+jeestr4d@users.noreply.github.com> Date: Fri, 27 Feb 2026 15:15:07 -0600 Subject: [PATCH 08/16] Update aci-preupgrade-validation-script.py commit --- aci-preupgrade-validation-script.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index 52624a85..fd5c3087 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -6079,7 +6079,7 @@ def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): list_of_shrd_contracts =[] for shrd_contract in shrd_contracts: list_of_shrd_contracts.append(shrd_contract["vzBrCP"]["attributes"]["dn"]) - # Because of CSCwb32627 # Configuration only becomes faulted for extEPGs after 6.0(1g), normal epgs permitted. + # Configuration only becomes faulted for extEPGs after 6.0(1g), normal epgs permitted. if tversion.older_than("6.0(1g)"): glbl_epgs_api = 'fvAEPg.json' glbl_epgs_api += '?query-target-filter=and(le(fvAEPg.pcTag,"16385"),ge(fvAEPg.pcTag,"16"),eq(fvAEPg.prefGrMemb,"include"))' From 8cde91676eef3f4e09633c5691dbddb2a83fc7c9 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Wed, 9 Sep 2026 15:08:38 -0400 Subject: [PATCH 09/16] fix: Check shared service risk for all 4.2+ targets (#244) Remove unsupported F0467 version thresholds so the check also covers releases with the silent forwarding risk. Add regression coverage across the source-verified 4.2 through 6.0 boundaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- aci-preupgrade-validation-script.py | 6 +-- .../test_pg_and_shared_svc_contract_check.py | 39 +++++++++++++++++-- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index fd5c3087..5f5a444c 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -6066,10 +6066,8 @@ def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): if not tversion: return Result(result=MANUAL, msg=TVER_MISSING) - # Configuration becomes faulted after 4.2-6d and 5.1-1h - if tversion.older_than("4.2(6d)"): - return Result(result=NA) - elif tversion.older_than("5.1(1h)"): + # Only releases 4.2 and later are in scope for this validation. + if tversion.older_than("4.2(1a)"): return Result(result=NA) shrd_contracts_api = 'vzBrCP.json' shrd_contracts_api += '?query-target-filter=and(eq(vzBrCP.scope,"global"))' diff --git a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py index d363caea..0e54125c 100644 --- a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py +++ b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py @@ -44,17 +44,18 @@ script.MANUAL, ), # NA cases - # Target version is lower than 4.2(6d), Result = NA + # Target version is lower than 4.2, Result = NA ( { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") }, - "4.2(1a)", "4.2(6c)", + "4.2(1a)", "4.1(2a)", script.NA, ), - # Target version is lower than 5.1(1h), Result = NA + # Target version predates Preferred Group-specific F0467 enforcement, + # but the forwarding risk is still present. ( { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), @@ -62,7 +63,7 @@ glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") }, "4.2(1a)", "5.1(1g)", - script.NA, + script.FAIL_O, ), # There are no global contracts, Result = NA ( @@ -144,3 +145,33 @@ def test_logic(run_check, mock_icurl, cversion, tversion, expected_result): tversion=script.AciVersion(tversion) if tversion else None ) assert result.result == expected_result + + +@pytest.mark.parametrize( + "icurl_outputs", + [ + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + } + ] +) +@pytest.mark.parametrize( + "tversion", + [ + "4.2(5n)", + "4.2(6d)", + "5.1(1h)", + "5.1(3e)", + "5.2(1g)", + "5.2(8i)", + "6.0(1g)", + ] +) +def test_all_4_2_and_newer_targets_are_checked(run_check, mock_icurl, tversion): + result = run_check( + cversion=script.AciVersion("4.2(1a)"), + tversion=script.AciVersion(tversion) + ) + assert result.result not in (script.NA, script.ERROR) From 45c265a519bdd5dbc062499eeb29cd54db46f7c8 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Wed, 9 Sep 2026 15:12:31 -0400 Subject: [PATCH 10/16] fix: Handle preferred-group objects without children (#244) Treat omitted or empty APIC subtree children as no provider relations so one childless object cannot abort the validation and hide other affected objects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- aci-preupgrade-validation-script.py | 4 +- .../test_pg_and_shared_svc_contract_check.py | 47 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index 5f5a444c..ca7ae6ea 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -6085,7 +6085,7 @@ def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): glbl_epgs = icurl('class', glbl_epgs_api) if glbl_epgs: for glbl_epg in glbl_epgs: - for prov_contract in glbl_epg["fvAEPg"]["children"]: + for prov_contract in glbl_epg["fvAEPg"].get("children") or []: if prov_contract["fvRsProv"]["attributes"]["tDn"] in list_of_shrd_contracts: contract = prov_contract["fvRsProv"]["attributes"]["tDn"] pctag = glbl_epg["fvAEPg"]["attributes"]["pcTag"] @@ -6098,7 +6098,7 @@ def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): glbl_ext_epgs = icurl('class', glbl_ext_epgs_api) if glbl_ext_epgs: for glbl_ext_epg in glbl_ext_epgs: - for prov_ext_contract in glbl_ext_epg["l3extInstP"]["children"]: + for prov_ext_contract in glbl_ext_epg["l3extInstP"].get("children") or []: if prov_ext_contract["fvRsProv"]["attributes"]["tDn"] in list_of_shrd_contracts: contract = prov_ext_contract["fvRsProv"]["attributes"]["tDn"] pctag = glbl_ext_epg["l3extInstP"]["attributes"]["pcTag"] diff --git a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py index 0e54125c..c3edb287 100644 --- a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py +++ b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py @@ -28,6 +28,23 @@ glbl_ext_epgs_api += '?query-target-filter=and(le(l3extInstP.pcTag,"16385"),ge(l3extInstP.pcTag,"16"),eq(l3extInstP.prefGrMemb,"include"))' glbl_ext_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' +childless_fvAEPg = { + "fvAEPg": { + "attributes": { + "dn": "uni/tn-test/ap-test/epg-no-provider", + "pcTag": "100" + } + } +} +childless_l3extInstP = { + "l3extInstP": { + "attributes": { + "dn": "uni/tn-test/out-test/instP-no-provider", + "pcTag": "101" + } + } +} + @pytest.mark.parametrize( "icurl_outputs, cversion, tversion, expected_result", @@ -106,6 +123,26 @@ "4.2(1a)", "6.0(1g)", script.FAIL_O, ), + # A childless fvAEPg does not hide a later affected fvAEPg. + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [childless_fvAEPg] + read_data(dir, "global_pg_fvAEPg.json"), + glbl_ext_epgs_api: [] + }, + "4.2(1a)", "5.2(8i)", + script.FAIL_O, + ), + # A childless l3extInstP does not hide a later affected l3extInstP. + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [], + glbl_ext_epgs_api: [childless_l3extInstP] + read_data(dir, "global_pg_l3extInstP.json") + }, + "4.2(1a)", "6.0(1g)", + script.FAIL_O, + ), # PASS Cases # Target version is older than 6.0(1g), no global_pg EPGs or extEPGs , Result = PASS ( @@ -137,6 +174,16 @@ "4.2(1a)", "6.0(1h)", script.PASS, ), + # Preferred-group objects without provider children are not affected. + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [childless_fvAEPg], + glbl_ext_epgs_api: [childless_l3extInstP] + }, + "4.2(1a)", "5.2(8i)", + script.PASS, + ), ] ) def test_logic(run_check, mock_icurl, cversion, tversion, expected_result): From 928427ebdf7814a8802075c855cf0d546eea4dc3 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Wed, 9 Sep 2026 15:20:05 -0400 Subject: [PATCH 11/16] fix: Correlate preferred providers with L3Out consumers (#244) Preserve broad pre-6.0 detection, but on 6.0(1g) and later only report shared-service providers with a cross-VRF L3Out consumer. Include the triggering consumer in result data and cover false-positive cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- aci-preupgrade-validation-script.py | 75 ++++++---- .../test_pg_and_shared_svc_contract_check.py | 131 +++++++++++++++++- 2 files changed, 174 insertions(+), 32 deletions(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index ca7ae6ea..5a8210f1 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -6059,7 +6059,7 @@ def apic_downgrade_compat_warning_check(cversion, tversion, **kwargs): @check_wrapper(check_title='Shared Services Providers with Preferred Group enabled') def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): result= PASS - headers = ["Shared Service Contract", "Provider in Preferred Group", "PcTag"] + headers = ["Shared Service Contract", "Provider in Preferred Group", "PcTag", "Affected Consumer"] data = [] recommended_action = 'an EPG in a Contract Preferred Group can consume a shared service contract, but cannot be a provider for a shared service contract.' doc_url = 'https://datacenter.github.io/ACI-Pre-Upgrade-Validation-Script/validations/#preferred_group_shared_service_provider' @@ -6074,36 +6074,59 @@ def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): shrd_contracts = icurl('class', shrd_contracts_api) if not shrd_contracts: return Result(result=NA) - list_of_shrd_contracts =[] + list_of_shrd_contracts = set() for shrd_contract in shrd_contracts: - list_of_shrd_contracts.append(shrd_contract["vzBrCP"]["attributes"]["dn"]) - # Configuration only becomes faulted for extEPGs after 6.0(1g), normal epgs permitted. - if tversion.older_than("6.0(1g)"): - glbl_epgs_api = 'fvAEPg.json' - glbl_epgs_api += '?query-target-filter=and(le(fvAEPg.pcTag,"16385"),ge(fvAEPg.pcTag,"16"),eq(fvAEPg.prefGrMemb,"include"))' - glbl_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' - glbl_epgs = icurl('class', glbl_epgs_api) - if glbl_epgs: - for glbl_epg in glbl_epgs: - for prov_contract in glbl_epg["fvAEPg"].get("children") or []: - if prov_contract["fvRsProv"]["attributes"]["tDn"] in list_of_shrd_contracts: - contract = prov_contract["fvRsProv"]["attributes"]["tDn"] - pctag = glbl_epg["fvAEPg"]["attributes"]["pcTag"] - provider = glbl_epg["fvAEPg"]["attributes"]["dn"] - data.append([contract, provider, pctag]) - # Regardless of version, check extEPGs + list_of_shrd_contracts.add(shrd_contract["vzBrCP"]["attributes"]["dn"]) + + broad_provider_check = tversion.older_than("6.0(1g)") + l3out_consumers_by_contract = {} + if not broad_provider_check: + l3out_consumers_api = 'l3extInstP.json' + l3out_consumers_api += '?rsp-subtree=children&rsp-subtree-class=fvRsCons' + l3out_consumers = icurl('class', l3out_consumers_api) + for l3out_consumer in l3out_consumers: + consumer_attributes = l3out_consumer["l3extInstP"]["attributes"] + for cons_contract in l3out_consumer["l3extInstP"].get("children") or []: + contract = cons_contract["fvRsCons"]["attributes"]["tDn"] + if contract in list_of_shrd_contracts: + l3out_consumers_by_contract.setdefault(contract, []).append( + (consumer_attributes["dn"], consumer_attributes["scope"]) + ) + + glbl_epgs_api = 'fvAEPg.json' + glbl_epgs_api += '?query-target-filter=and(le(fvAEPg.pcTag,"16385"),ge(fvAEPg.pcTag,"16"),eq(fvAEPg.prefGrMemb,"include"))' + glbl_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' + glbl_epgs = icurl('class', glbl_epgs_api) + glbl_ext_epgs_api = 'l3extInstP.json' glbl_ext_epgs_api += '?query-target-filter=and(le(l3extInstP.pcTag,"16385"),ge(l3extInstP.pcTag,"16"),eq(l3extInstP.prefGrMemb,"include"))' glbl_ext_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' glbl_ext_epgs = icurl('class', glbl_ext_epgs_api) - if glbl_ext_epgs: - for glbl_ext_epg in glbl_ext_epgs: - for prov_ext_contract in glbl_ext_epg["l3extInstP"].get("children") or []: - if prov_ext_contract["fvRsProv"]["attributes"]["tDn"] in list_of_shrd_contracts: - contract = prov_ext_contract["fvRsProv"]["attributes"]["tDn"] - pctag = glbl_ext_epg["l3extInstP"]["attributes"]["pcTag"] - provider = glbl_ext_epg["l3extInstP"]["attributes"]["dn"] - data.append([contract, provider, pctag]) + + for provider_class, providers in (("fvAEPg", glbl_epgs), ("l3extInstP", glbl_ext_epgs)): + for provider_mo in providers: + provider = provider_mo[provider_class] + provider_attributes = provider["attributes"] + for prov_contract in provider.get("children") or []: + contract = prov_contract["fvRsProv"]["attributes"]["tDn"] + if contract not in list_of_shrd_contracts: + continue + if broad_provider_check: + data.append([ + contract, + provider_attributes["dn"], + provider_attributes["pcTag"], + "Any" + ]) + continue + for consumer_dn, consumer_scope in l3out_consumers_by_contract.get(contract, []): + if consumer_scope != provider_attributes["scope"]: + data.append([ + contract, + provider_attributes["dn"], + provider_attributes["pcTag"], + consumer_dn + ]) if data: result = FAIL_O diff --git a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py index c3edb287..1dcc743b 100644 --- a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py +++ b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py @@ -28,6 +28,9 @@ glbl_ext_epgs_api += '?query-target-filter=and(le(l3extInstP.pcTag,"16385"),ge(l3extInstP.pcTag,"16"),eq(l3extInstP.prefGrMemb,"include"))' glbl_ext_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' +l3out_consumers_api = 'l3extInstP.json' +l3out_consumers_api += '?rsp-subtree=children&rsp-subtree-class=fvRsCons' + childless_fvAEPg = { "fvAEPg": { "attributes": { @@ -44,6 +47,57 @@ } } } +different_vrf_l3out_consumer = { + "l3extInstP": { + "attributes": { + "dn": "uni/tn-consumer/out-consumer/instP-different-vrf", + "scope": "999" + }, + "children": [ + { + "fvRsCons": { + "attributes": { + "tDn": "uni/tn-common/brc-AD_C" + } + } + } + ] + } +} +same_vrf_l3out_consumer = { + "l3extInstP": { + "attributes": { + "dn": "uni/tn-consumer/out-consumer/instP-same-vrf", + "scope": "2261001" + }, + "children": [ + { + "fvRsCons": { + "attributes": { + "tDn": "uni/tn-common/brc-AD_C" + } + } + } + ] + } +} +unrelated_l3out_consumer = { + "l3extInstP": { + "attributes": { + "dn": "uni/tn-consumer/out-consumer/instP-unrelated", + "scope": "999" + }, + "children": [ + { + "fvRsCons": { + "attributes": { + "tDn": "uni/tn-common/brc-unrelated" + } + } + } + ] + } +} @pytest.mark.parametrize( @@ -108,7 +162,8 @@ { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), - glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), + l3out_consumers_api: [different_vrf_l3out_consumer] }, "4.2(1a)", "6.0(1g)", script.FAIL_O, @@ -118,7 +173,8 @@ { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [], - glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), + l3out_consumers_api: [different_vrf_l3out_consumer] }, "4.2(1a)", "6.0(1g)", script.FAIL_O, @@ -138,7 +194,8 @@ { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [], - glbl_ext_epgs_api: [childless_l3extInstP] + read_data(dir, "global_pg_l3extInstP.json") + glbl_ext_epgs_api: [childless_l3extInstP] + read_data(dir, "global_pg_l3extInstP.json"), + l3out_consumers_api: [different_vrf_l3out_consumer] }, "4.2(1a)", "6.0(1g)", script.FAIL_O, @@ -159,7 +216,8 @@ { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [], - glbl_ext_epgs_api: [] + glbl_ext_epgs_api: [], + l3out_consumers_api: [] }, "4.2(1a)", "6.0(1h)", script.PASS, @@ -169,7 +227,8 @@ { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), - glbl_ext_epgs_api: [] + glbl_ext_epgs_api: [], + l3out_consumers_api: [] }, "4.2(1a)", "6.0(1h)", script.PASS, @@ -184,6 +243,39 @@ "4.2(1a)", "5.2(8i)", script.PASS, ), + # No L3Out consumer means an ordinary EPG consumer cannot trigger this check. + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), + glbl_ext_epgs_api: [], + l3out_consumers_api: [childless_l3extInstP] + }, + "4.2(1a)", "6.0(1g)", + script.PASS, + ), + # A same-VRF L3Out consumer does not trigger the cross-VRF restriction. + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [read_data(dir, "global_pg_fvAEPg.json")[0]], + glbl_ext_epgs_api: [], + l3out_consumers_api: [same_vrf_l3out_consumer] + }, + "4.2(1a)", "6.0(1g)", + script.PASS, + ), + # An unrelated L3Out consumer does not affect the provider. + ( + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), + glbl_ext_epgs_api: [], + l3out_consumers_api: [unrelated_l3out_consumer] + }, + "4.2(1a)", "6.0(1g)", + script.PASS, + ), ] ) def test_logic(run_check, mock_icurl, cversion, tversion, expected_result): @@ -200,7 +292,8 @@ def test_logic(run_check, mock_icurl, cversion, tversion, expected_result): { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), - glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), + l3out_consumers_api: [different_vrf_l3out_consumer] } ] ) @@ -222,3 +315,29 @@ def test_all_4_2_and_newer_targets_are_checked(run_check, mock_icurl, tversion): tversion=script.AciVersion(tversion) ) assert result.result not in (script.NA, script.ERROR) + + +@pytest.mark.parametrize( + "icurl_outputs", + [ + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [read_data(dir, "global_pg_fvAEPg.json")[0]], + glbl_ext_epgs_api: [], + l3out_consumers_api: [different_vrf_l3out_consumer] + } + ] +) +def test_reports_correlated_l3out_consumer(run_check, mock_icurl): + result = run_check( + cversion=script.AciVersion("5.2(8i)"), + tversion=script.AciVersion("6.0(1g)") + ) + + assert result.result == script.FAIL_O + assert result.data == [[ + "uni/tn-common/brc-AD_C", + "uni/tn-common/ap-apptest/epg-epg1", + "5555", + "uni/tn-consumer/out-consumer/instP-different-vrf" + ]] From caba12b3b97eba3409bb4010431e51844ac1e71e Mon Sep 17 00:00:00 2001 From: Gabriel Date: Wed, 9 Sep 2026 15:31:25 -0400 Subject: [PATCH 12/16] fix: Add actionable preferred-group remediation (#244) Tell operators how to remove the unsupported relationship and verify policy deployment across the F0467 and F4684 enforcement paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- aci-preupgrade-validation-script.py | 8 +++++++- .../test_pg_and_shared_svc_contract_check.py | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index 5a8210f1..e97649d7 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -6061,7 +6061,13 @@ def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): result= PASS headers = ["Shared Service Contract", "Provider in Preferred Group", "PcTag", "Affected Consumer"] data = [] - recommended_action = 'an EPG in a Contract Preferred Group can consume a shared service contract, but cannot be a provider for a shared service contract.' + recommended_action = ( + "Before upgrading, remove each listed provider from the Preferred Group, " + "stop it from providing the listed shared-service contract, or remove the " + "unsupported L3Out/vzAny consumer relationship. Re-deploy the policy and " + "confirm the contract and Preferred Group configuration deploy successfully. " + "On releases that enforce this restriction, verify that F0467 or F4684 clears." + ) doc_url = 'https://datacenter.github.io/ACI-Pre-Upgrade-Validation-Script/validations/#preferred_group_shared_service_provider' if not tversion: diff --git a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py index 1dcc743b..6a478cc9 100644 --- a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py +++ b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py @@ -341,3 +341,7 @@ def test_reports_correlated_l3out_consumer(run_check, mock_icurl): "5555", "uni/tn-consumer/out-consumer/instP-different-vrf" ]] + assert "remove each listed provider from the Preferred Group" in result.recommended_action + assert "stop it from providing the listed shared-service contract" in result.recommended_action + assert "remove the unsupported L3Out/vzAny consumer relationship" in result.recommended_action + assert "F0467 or F4684" in result.recommended_action From 1c9a4a59d9b2ab9bf6fbb06d62eb51feb58abb57 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Wed, 9 Sep 2026 15:34:40 -0400 Subject: [PATCH 13/16] fix: Include tenant-scoped shared contracts (#244) Treat tenant-scoped contracts as shared-service candidates and verify both broad pre-6.0 behavior and cross-VRF L3Out correlation on 6.0(1g) and later. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- aci-preupgrade-validation-script.py | 2 +- .../test_pg_and_shared_svc_contract_check.py | 82 ++++++++++++++++++- 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index e97649d7..066ab5bb 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -6076,7 +6076,7 @@ def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): if tversion.older_than("4.2(1a)"): return Result(result=NA) shrd_contracts_api = 'vzBrCP.json' - shrd_contracts_api += '?query-target-filter=and(eq(vzBrCP.scope,"global"))' + shrd_contracts_api += '?query-target-filter=or(eq(vzBrCP.scope,"global"),eq(vzBrCP.scope,"tenant"))' shrd_contracts = icurl('class', shrd_contracts_api) if not shrd_contracts: return Result(result=NA) diff --git a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py index 6a478cc9..e62e91fe 100644 --- a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py +++ b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py @@ -14,7 +14,7 @@ # icurl queries # shared contracts shrd_contracts_api = 'vzBrCP.json' -shrd_contracts_api += '?query-target-filter=and(eq(vzBrCP.scope,"global"))' +shrd_contracts_api += '?query-target-filter=or(eq(vzBrCP.scope,"global"),eq(vzBrCP.scope,"tenant"))' # global epgs ( 16 <= pgtag <= 16385) with Preferred group enabled with provided contracts @@ -98,6 +98,50 @@ ] } } +tenant_contract = { + "vzBrCP": { + "attributes": { + "dn": "uni/tn-test/brc-tenant-shared", + "name": "tenant-shared", + "scope": "tenant" + } + } +} +tenant_provider = { + "fvAEPg": { + "attributes": { + "dn": "uni/tn-test/ap-provider/epg-provider", + "pcTag": "102", + "scope": "1000" + }, + "children": [ + { + "fvRsProv": { + "attributes": { + "tDn": "uni/tn-test/brc-tenant-shared" + } + } + } + ] + } +} +tenant_l3out_consumer = { + "l3extInstP": { + "attributes": { + "dn": "uni/tn-test/out-consumer/instP-consumer", + "scope": "2000" + }, + "children": [ + { + "fvRsCons": { + "attributes": { + "tDn": "uni/tn-test/brc-tenant-shared" + } + } + } + ] + } +} @pytest.mark.parametrize( @@ -157,6 +201,16 @@ "4.2(1a)", "6.0(1f)", script.FAIL_O, ), + # Tenant-scope contracts carry the same broad forwarding risk through 5.2. + ( + { + shrd_contracts_api: [tenant_contract], + glbl_epgs_api: [tenant_provider], + glbl_ext_epgs_api: [] + }, + "4.2(1a)", "5.2(8i)", + script.FAIL_O, + ), # Target version is newer than 6.0(1g), both global_pg EPGs and extEPGs , Result = FAIL_O ( { @@ -345,3 +399,29 @@ def test_reports_correlated_l3out_consumer(run_check, mock_icurl): assert "stop it from providing the listed shared-service contract" in result.recommended_action assert "remove the unsupported L3Out/vzAny consumer relationship" in result.recommended_action assert "F0467 or F4684" in result.recommended_action + + +@pytest.mark.parametrize( + "icurl_outputs", + [ + { + shrd_contracts_api: [tenant_contract], + glbl_epgs_api: [tenant_provider], + glbl_ext_epgs_api: [], + l3out_consumers_api: [tenant_l3out_consumer] + } + ] +) +def test_reports_tenant_scope_contract_across_vrfs(run_check, mock_icurl): + result = run_check( + cversion=script.AciVersion("5.2(8i)"), + tversion=script.AciVersion("6.0(1g)") + ) + + assert result.result == script.FAIL_O + assert result.data == [[ + "uni/tn-test/brc-tenant-shared", + "uni/tn-test/ap-provider/epg-provider", + "102", + "uni/tn-test/out-consumer/instP-consumer" + ]] From 9a60ea06f93165e22683749319248e71f5fbd184 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Wed, 9 Sep 2026 15:43:33 -0400 Subject: [PATCH 14/16] fix: Correlate preferred providers with vzAny consumers (#244) Use the APIC reverse vzAny consumer relation on 6.0(1g) and later, report the triggering consumer, reject malformed relation data explicitly, and avoid consumer inventory queries when no candidate provider exists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- aci-preupgrade-validation-script.py | 76 ++++++----- .../test_pg_and_shared_svc_contract_check.py | 118 ++++++++++++++++-- 2 files changed, 153 insertions(+), 41 deletions(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index 066ab5bb..df940db6 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -6084,21 +6084,6 @@ def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): for shrd_contract in shrd_contracts: list_of_shrd_contracts.add(shrd_contract["vzBrCP"]["attributes"]["dn"]) - broad_provider_check = tversion.older_than("6.0(1g)") - l3out_consumers_by_contract = {} - if not broad_provider_check: - l3out_consumers_api = 'l3extInstP.json' - l3out_consumers_api += '?rsp-subtree=children&rsp-subtree-class=fvRsCons' - l3out_consumers = icurl('class', l3out_consumers_api) - for l3out_consumer in l3out_consumers: - consumer_attributes = l3out_consumer["l3extInstP"]["attributes"] - for cons_contract in l3out_consumer["l3extInstP"].get("children") or []: - contract = cons_contract["fvRsCons"]["attributes"]["tDn"] - if contract in list_of_shrd_contracts: - l3out_consumers_by_contract.setdefault(contract, []).append( - (consumer_attributes["dn"], consumer_attributes["scope"]) - ) - glbl_epgs_api = 'fvAEPg.json' glbl_epgs_api += '?query-target-filter=and(le(fvAEPg.pcTag,"16385"),ge(fvAEPg.pcTag,"16"),eq(fvAEPg.prefGrMemb,"include"))' glbl_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' @@ -6109,30 +6094,65 @@ def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): glbl_ext_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' glbl_ext_epgs = icurl('class', glbl_ext_epgs_api) + shared_service_providers = [] for provider_class, providers in (("fvAEPg", glbl_epgs), ("l3extInstP", glbl_ext_epgs)): for provider_mo in providers: provider = provider_mo[provider_class] provider_attributes = provider["attributes"] for prov_contract in provider.get("children") or []: contract = prov_contract["fvRsProv"]["attributes"]["tDn"] - if contract not in list_of_shrd_contracts: - continue - if broad_provider_check: + if contract in list_of_shrd_contracts: + shared_service_providers.append((contract, provider_attributes)) + + broad_provider_check = tversion.older_than("6.0(1g)") + if broad_provider_check: + for contract, provider_attributes in shared_service_providers: + data.append([ + contract, + provider_attributes["dn"], + provider_attributes["pcTag"], + "Any" + ]) + elif shared_service_providers: + provider_contracts = set(provider[0] for provider in shared_service_providers) + restricted_consumers_by_contract = {} + + l3out_consumers_api = 'l3extInstP.json' + l3out_consumers_api += '?rsp-subtree=children&rsp-subtree-class=fvRsCons' + l3out_consumers = icurl('class', l3out_consumers_api) + for l3out_consumer in l3out_consumers: + consumer_attributes = l3out_consumer["l3extInstP"]["attributes"] + for cons_contract in l3out_consumer["l3extInstP"].get("children") or []: + contract = cons_contract["fvRsCons"]["attributes"]["tDn"] + if contract in provider_contracts: + restricted_consumers_by_contract.setdefault(contract, []).append( + (consumer_attributes["dn"], consumer_attributes["scope"], "l3extInstP") + ) + + vzany_consumers = icurl('class', 'vzRtAnyToCons.json') + for vzany_consumer in vzany_consumers: + consumer_attributes = vzany_consumer["vzRtAnyToCons"]["attributes"] + relation_dn = consumer_attributes["dn"] + contract, separator, _ = relation_dn.rpartition("/rtanyToCons-") + if not separator: + return Result( + result=ERROR, + msg="Failed to get contract DN from vzRtAnyToCons DN: {}".format(relation_dn) + ) + if contract in provider_contracts: + restricted_consumers_by_contract.setdefault(contract, []).append( + (consumer_attributes["tDn"], None, "vzAny") + ) + + for contract, provider_attributes in shared_service_providers: + for consumer_dn, consumer_scope, consumer_class in restricted_consumers_by_contract.get(contract, []): + if consumer_class == "vzAny" or consumer_scope != provider_attributes["scope"]: data.append([ contract, provider_attributes["dn"], provider_attributes["pcTag"], - "Any" + consumer_dn ]) - continue - for consumer_dn, consumer_scope in l3out_consumers_by_contract.get(contract, []): - if consumer_scope != provider_attributes["scope"]: - data.append([ - contract, - provider_attributes["dn"], - provider_attributes["pcTag"], - consumer_dn - ]) if data: result = FAIL_O diff --git a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py index e62e91fe..e4924ad8 100644 --- a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py +++ b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py @@ -30,6 +30,7 @@ l3out_consumers_api = 'l3extInstP.json' l3out_consumers_api += '?rsp-subtree=children&rsp-subtree-class=fvRsCons' +vzany_consumers_api = 'vzRtAnyToCons.json' childless_fvAEPg = { "fvAEPg": { @@ -142,6 +143,36 @@ ] } } +tenant_vzany_consumer = { + "vzRtAnyToCons": { + "attributes": { + "dn": ( + "uni/tn-test/brc-tenant-shared/" + "rtanyToCons-[uni/tn-test/ctx-consumer/any]" + ), + "tDn": "uni/tn-test/ctx-consumer/any" + } + } +} +unrelated_vzany_consumer = { + "vzRtAnyToCons": { + "attributes": { + "dn": ( + "uni/tn-test/brc-unrelated/" + "rtanyToCons-[uni/tn-test/ctx-consumer/any]" + ), + "tDn": "uni/tn-test/ctx-consumer/any" + } + } +} +malformed_vzany_consumer = { + "vzRtAnyToCons": { + "attributes": { + "dn": "uni/tn-test/brc-tenant-shared/bad-relation", + "tDn": "uni/tn-test/ctx-consumer/any" + } + } +} @pytest.mark.parametrize( @@ -217,7 +248,8 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), - l3out_consumers_api: [different_vrf_l3out_consumer] + l3out_consumers_api: [different_vrf_l3out_consumer], + vzany_consumers_api: [] }, "4.2(1a)", "6.0(1g)", script.FAIL_O, @@ -228,7 +260,8 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [], glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), - l3out_consumers_api: [different_vrf_l3out_consumer] + l3out_consumers_api: [different_vrf_l3out_consumer], + vzany_consumers_api: [] }, "4.2(1a)", "6.0(1g)", script.FAIL_O, @@ -249,7 +282,8 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [], glbl_ext_epgs_api: [childless_l3extInstP] + read_data(dir, "global_pg_l3extInstP.json"), - l3out_consumers_api: [different_vrf_l3out_consumer] + l3out_consumers_api: [different_vrf_l3out_consumer], + vzany_consumers_api: [] }, "4.2(1a)", "6.0(1g)", script.FAIL_O, @@ -270,8 +304,7 @@ { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [], - glbl_ext_epgs_api: [], - l3out_consumers_api: [] + glbl_ext_epgs_api: [] }, "4.2(1a)", "6.0(1h)", script.PASS, @@ -282,7 +315,8 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), glbl_ext_epgs_api: [], - l3out_consumers_api: [] + l3out_consumers_api: [], + vzany_consumers_api: [] }, "4.2(1a)", "6.0(1h)", script.PASS, @@ -303,7 +337,8 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), glbl_ext_epgs_api: [], - l3out_consumers_api: [childless_l3extInstP] + l3out_consumers_api: [childless_l3extInstP], + vzany_consumers_api: [] }, "4.2(1a)", "6.0(1g)", script.PASS, @@ -314,18 +349,20 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [read_data(dir, "global_pg_fvAEPg.json")[0]], glbl_ext_epgs_api: [], - l3out_consumers_api: [same_vrf_l3out_consumer] + l3out_consumers_api: [same_vrf_l3out_consumer], + vzany_consumers_api: [] }, "4.2(1a)", "6.0(1g)", script.PASS, ), - # An unrelated L3Out consumer does not affect the provider. + # Unrelated L3Out and vzAny consumers do not affect the provider. ( { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), glbl_ext_epgs_api: [], - l3out_consumers_api: [unrelated_l3out_consumer] + l3out_consumers_api: [unrelated_l3out_consumer], + vzany_consumers_api: [unrelated_vzany_consumer] }, "4.2(1a)", "6.0(1g)", script.PASS, @@ -347,7 +384,8 @@ def test_logic(run_check, mock_icurl, cversion, tversion, expected_result): shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), - l3out_consumers_api: [different_vrf_l3out_consumer] + l3out_consumers_api: [different_vrf_l3out_consumer], + vzany_consumers_api: [] } ] ) @@ -378,7 +416,8 @@ def test_all_4_2_and_newer_targets_are_checked(run_check, mock_icurl, tversion): shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [read_data(dir, "global_pg_fvAEPg.json")[0]], glbl_ext_epgs_api: [], - l3out_consumers_api: [different_vrf_l3out_consumer] + l3out_consumers_api: [different_vrf_l3out_consumer], + vzany_consumers_api: [] } ] ) @@ -408,7 +447,8 @@ def test_reports_correlated_l3out_consumer(run_check, mock_icurl): shrd_contracts_api: [tenant_contract], glbl_epgs_api: [tenant_provider], glbl_ext_epgs_api: [], - l3out_consumers_api: [tenant_l3out_consumer] + l3out_consumers_api: [tenant_l3out_consumer], + vzany_consumers_api: [] } ] ) @@ -425,3 +465,55 @@ def test_reports_tenant_scope_contract_across_vrfs(run_check, mock_icurl): "102", "uni/tn-test/out-consumer/instP-consumer" ]] + + +@pytest.mark.parametrize( + "icurl_outputs", + [ + { + shrd_contracts_api: [tenant_contract], + glbl_epgs_api: [tenant_provider], + glbl_ext_epgs_api: [], + l3out_consumers_api: [], + vzany_consumers_api: [tenant_vzany_consumer] + } + ] +) +def test_reports_correlated_vzany_consumer(run_check, mock_icurl): + result = run_check( + cversion=script.AciVersion("5.2(8i)"), + tversion=script.AciVersion("6.0(1g)") + ) + + assert result.result == script.FAIL_O + assert result.data == [[ + "uni/tn-test/brc-tenant-shared", + "uni/tn-test/ap-provider/epg-provider", + "102", + "uni/tn-test/ctx-consumer/any" + ]] + + +@pytest.mark.parametrize( + "icurl_outputs", + [ + { + shrd_contracts_api: [tenant_contract], + glbl_epgs_api: [tenant_provider], + glbl_ext_epgs_api: [], + l3out_consumers_api: [], + vzany_consumers_api: [malformed_vzany_consumer] + } + ] +) +def test_rejects_malformed_vzany_reverse_relation(run_check, mock_icurl): + result = run_check( + cversion=script.AciVersion("5.2(8i)"), + tversion=script.AciVersion("6.0(1g)") + ) + + assert result.result == script.ERROR + assert result.msg == ( + "Failed to get contract DN from vzRtAnyToCons DN: " + "uni/tn-test/brc-tenant-shared/bad-relation" + ) From e8f5b7d4c46cf1014992d55b9090d9e9f754f88b Mon Sep 17 00:00:00 2001 From: Gabriel Date: Wed, 9 Sep 2026 15:47:51 -0400 Subject: [PATCH 15/16] fix: Correct preferred-group validation anchor (#244) Use the kebab-case MkDocs anchor in both the check result and validation index, with regression coverage for the generated URL. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- aci-preupgrade-validation-script.py | 2 +- docs/docs/validations.md | 3 +-- .../test_pg_and_shared_svc_contract_check.py | 1 + 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index df940db6..bb7402ed 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -6068,7 +6068,7 @@ def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): "confirm the contract and Preferred Group configuration deploy successfully. " "On releases that enforce this restriction, verify that F0467 or F4684 clears." ) - doc_url = 'https://datacenter.github.io/ACI-Pre-Upgrade-Validation-Script/validations/#preferred_group_shared_service_provider' + doc_url = 'https://datacenter.github.io/ACI-Pre-Upgrade-Validation-Script/validations/#preferred-group-shared-service-provider' if not tversion: return Result(result=MANUAL, msg=TVER_MISSING) diff --git a/docs/docs/validations.md b/docs/docs/validations.md index 12e2442b..efcc2da1 100644 --- a/docs/docs/validations.md +++ b/docs/docs/validations.md @@ -160,7 +160,7 @@ Items | Faults | This Script [c22]: #service-graph-bd-forceful-routing [c23]: #ave-end-of-life [c24]: #shared-service-with-vzany-consumer -[c25]: #preferred_group_shared_service_provider +[c25]: #preferred-group-shared-service-provider ### Defect Condition Checks @@ -2722,4 +2722,3 @@ If any instances of `configpushShardCont` are flagged by this script, Cisco TAC [60]: https://www.cisco.com/c/en/us/solutions/collateral/data-center-virtualization/application-centric-infrastructure/white-paper-c11-743951.html#Inter [61]: https://www.cisco.com/c/en/us/solutions/collateral/data-center-virtualization/application-centric-infrastructure/white-paper-c11-743951.html#EnablePolicyCompression [62]: https://www.cisco.com/c/en/us/td/docs/switches/datacenter/aci/apic/sw/5-x/aci-fundamentals/cisco-aci-fundamentals-50x/m_policy-model.html#concept_tds_vcc_fy - diff --git a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py index e4924ad8..b8e4c886 100644 --- a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py +++ b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py @@ -438,6 +438,7 @@ def test_reports_correlated_l3out_consumer(run_check, mock_icurl): assert "stop it from providing the listed shared-service contract" in result.recommended_action assert "remove the unsupported L3Out/vzAny consumer relationship" in result.recommended_action assert "F0467 or F4684" in result.recommended_action + assert result.doc_url.endswith("/#preferred-group-shared-service-provider") @pytest.mark.parametrize( From 1a0e546f724b3f97317f60d1677411b967b82749 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Thu, 10 Sep 2026 11:35:54 -0400 Subject: [PATCH 16/16] fix: Correlate preferred providers with derived consumers (#244) Use materialized vzFromEPg-to-vzToEPg relationships and authoritative context definitions to identify affected cross-VRF shared services. Preserve the pre-6.0 broad behavior while restricting 6.0+ findings to L3Out and vzAny consumers, and exclude reserved pcTag 16. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- aci-preupgrade-validation-script.py | 177 ++++-- docs/docs/validations.md | 10 +- .../test_pg_and_shared_svc_contract_check.py | 527 +++++++++++++----- 3 files changed, 527 insertions(+), 187 deletions(-) diff --git a/aci-preupgrade-validation-script.py b/aci-preupgrade-validation-script.py index 390a9981..1b5c90e3 100644 --- a/aci-preupgrade-validation-script.py +++ b/aci-preupgrade-validation-script.py @@ -6421,17 +6421,18 @@ def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): shrd_contracts = icurl('class', shrd_contracts_api) if not shrd_contracts: return Result(result=NA) - list_of_shrd_contracts = set() + shared_contract_scopes = {} for shrd_contract in shrd_contracts: - list_of_shrd_contracts.add(shrd_contract["vzBrCP"]["attributes"]["dn"]) + contract_attributes = shrd_contract["vzBrCP"]["attributes"] + shared_contract_scopes[contract_attributes["dn"]] = contract_attributes["scope"] glbl_epgs_api = 'fvAEPg.json' - glbl_epgs_api += '?query-target-filter=and(le(fvAEPg.pcTag,"16385"),ge(fvAEPg.pcTag,"16"),eq(fvAEPg.prefGrMemb,"include"))' + glbl_epgs_api += '?query-target-filter=and(le(fvAEPg.pcTag,"16385"),ge(fvAEPg.pcTag,"17"),eq(fvAEPg.prefGrMemb,"include"))' glbl_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' glbl_epgs = icurl('class', glbl_epgs_api) glbl_ext_epgs_api = 'l3extInstP.json' - glbl_ext_epgs_api += '?query-target-filter=and(le(l3extInstP.pcTag,"16385"),ge(l3extInstP.pcTag,"16"),eq(l3extInstP.prefGrMemb,"include"))' + glbl_ext_epgs_api += '?query-target-filter=and(le(l3extInstP.pcTag,"16385"),ge(l3extInstP.pcTag,"17"),eq(l3extInstP.prefGrMemb,"include"))' glbl_ext_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' glbl_ext_epgs = icurl('class', glbl_ext_epgs_api) @@ -6442,58 +6443,134 @@ def pg_and_shared_svc_contract_check(cversion, tversion, **kwargs): provider_attributes = provider["attributes"] for prov_contract in provider.get("children") or []: contract = prov_contract["fvRsProv"]["attributes"]["tDn"] - if contract in list_of_shrd_contracts: + if contract in shared_contract_scopes: shared_service_providers.append((contract, provider_attributes)) - broad_provider_check = tversion.older_than("6.0(1g)") - if broad_provider_check: + if shared_service_providers: + providers_by_dn = {} for contract, provider_attributes in shared_service_providers: - data.append([ - contract, - provider_attributes["dn"], - provider_attributes["pcTag"], - "Any" - ]) - elif shared_service_providers: - provider_contracts = set(provider[0] for provider in shared_service_providers) - restricted_consumers_by_contract = {} - - l3out_consumers_api = 'l3extInstP.json' - l3out_consumers_api += '?rsp-subtree=children&rsp-subtree-class=fvRsCons' - l3out_consumers = icurl('class', l3out_consumers_api) - for l3out_consumer in l3out_consumers: - consumer_attributes = l3out_consumer["l3extInstP"]["attributes"] - for cons_contract in l3out_consumer["l3extInstP"].get("children") or []: - contract = cons_contract["fvRsCons"]["attributes"]["tDn"] - if contract in provider_contracts: - restricted_consumers_by_contract.setdefault(contract, []).append( - (consumer_attributes["dn"], consumer_attributes["scope"], "l3extInstP") - ) + providers_by_dn.setdefault(provider_attributes["dn"], []).append( + (contract, provider_attributes) + ) - vzany_consumers = icurl('class', 'vzRtAnyToCons.json') - for vzany_consumer in vzany_consumers: - consumer_attributes = vzany_consumer["vzRtAnyToCons"]["attributes"] - relation_dn = consumer_attributes["dn"] - contract, separator, _ = relation_dn.rpartition("/rtanyToCons-") - if not separator: - return Result( - result=ERROR, - msg="Failed to get contract DN from vzRtAnyToCons DN: {}".format(relation_dn) - ) - if contract in provider_contracts: - restricted_consumers_by_contract.setdefault(contract, []).append( - (consumer_attributes["tDn"], None, "vzAny") + ctx_defs = icurl('class', 'fvCtxDef.json') + ctx_def_by_scope = {} + for ctx_def in ctx_defs: + ctx_attributes = ctx_def["fvCtxDef"]["attributes"] + ctx_def_by_scope[ctx_attributes["scope"]] = ctx_attributes["dn"] + + provider_relationships_api = 'vzFromEPg.json' + provider_relationships_api += '?query-target-filter=and(eq(vzFromEPg.membType,"prov"),' + provider_relationships_api += 'le(vzFromEPg.pcTag,"16385"),ge(vzFromEPg.pcTag,"17"))' + provider_relationships_api += '&rsp-subtree=children&rsp-subtree-class=vzToEPg' + provider_relationships = icurl('class', provider_relationships_api) + cross_context_relationships = [] + relationship_errors = [] + + def tenant_dn(dn): + dn_parts = dn.split("/", 2) + if len(dn_parts) >= 2 and dn_parts[0] == "uni" and dn_parts[1].startswith("tn-"): + return "/".join(dn_parts[:2]) + return None + + for provider_relationship in provider_relationships: + from_epg = provider_relationship["vzFromEPg"] + from_attributes = from_epg["attributes"] + provider_dn = from_attributes["epgDn"] + provider_contracts = providers_by_dn.get(provider_dn, []) + if not provider_contracts: + continue + + matching_provider_contracts = [ + provider_contract + for provider_contract in provider_contracts + if from_attributes["dn"].startswith( + "cdef-[{}]/".format(provider_contract[0]) ) + ] + if not matching_provider_contracts: + continue - for contract, provider_attributes in shared_service_providers: - for consumer_dn, consumer_scope, consumer_class in restricted_consumers_by_contract.get(contract, []): - if consumer_class == "vzAny" or consumer_scope != provider_attributes["scope"]: - data.append([ - contract, - provider_attributes["dn"], - provider_attributes["pcTag"], - consumer_dn - ]) + to_epgs = from_epg.get("children") or [] + if not to_epgs: + continue + + provider_ctx_def_dn = ctx_def_by_scope.get(from_attributes["scopeId"]) + if not provider_ctx_def_dn: + relationship_errors.append([ + from_attributes["dn"], + "No fvCtxDef found for scopeId {}".format(from_attributes["scopeId"]) + ]) + continue + + for contract, provider_attributes in matching_provider_contracts: + for to_epg_mo in to_epgs: + if "vzToEPg" not in to_epg_mo: + continue + consumer_attributes = to_epg_mo["vzToEPg"]["attributes"] + consumer_dn = consumer_attributes["epgDn"] + consumer_ctx_def_dn = consumer_attributes.get("ctxDefDn") + if not consumer_ctx_def_dn: + relationship_errors.append([ + consumer_attributes["dn"], + "vzToEPg.ctxDefDn is empty" + ]) + continue + + if shared_contract_scopes[contract] == "tenant": + contract_tenant = tenant_dn(contract) + if ( + contract_tenant != tenant_dn(provider_dn) + or contract_tenant != tenant_dn(consumer_dn) + ): + continue + + if provider_ctx_def_dn != consumer_ctx_def_dn: + cross_context_relationships.append( + (contract, provider_attributes, consumer_dn) + ) + + broad_provider_check = tversion.older_than("6.0(1g)") + if broad_provider_check: + affected_relationships = cross_context_relationships + elif cross_context_relationships: + affected_relationships = [ + relationship + for relationship in cross_context_relationships + if "/instP-" in relationship[2] + or relationship[2].endswith("/any") + ] + else: + affected_relationships = [] + + reported_relationships = set() + for contract, provider_attributes, consumer_dn in affected_relationships: + result_row = ( + contract, + provider_attributes["dn"], + provider_attributes["pcTag"], + consumer_dn + ) + if result_row not in reported_relationships: + reported_relationships.add(result_row) + data.append(list(result_row)) + + if relationship_errors: + return Result( + result=ERROR, + msg="Unable to resolve context for one or more derived contract relationships", + headers=["Derived Relationship", "Context Resolution Error"], + data=relationship_errors, + unformatted_headers=headers, + unformatted_data=data, + recommended_action=( + "Retry the check. Review any confirmed affected relationships " + "shown in the failure details. If context resolution continues " + "to fail, contact Cisco TAC with the listed derived relationship " + "DNs." + ), + doc_url=doc_url + ) if data: result = FAIL_O diff --git a/docs/docs/validations.md b/docs/docs/validations.md index 71917129..ead94e1f 100644 --- a/docs/docs/validations.md +++ b/docs/docs/validations.md @@ -2376,13 +2376,15 @@ See [Enable Policy Compression in Cisco ACI Contract Guide][61] for details abou ### Preferred Group Shared Service Provider -ACI 4.2 and later configurations where a Preferred Group member provides a tenant- or global-scope shared-service contract can cause traffic loss or contract rejection. +ACI 4.2 and later configurations can be affected by CSCvm63145 and CSCvv51121 when a Preferred Group member provides a tenant- or global-scope shared-service contract to a consumer in another VRF. -Before 6.0(1g), this configuration is subject to broad provider-side behavior. Depending on the release, the forwarding risk can be silent or the contract can be rejected with F0467 and `invalid-contract-config: Shared service provider cannot be in a Preferred Group`. +The script reports only materialized, cross-VRF provider-to-consumer relationships represented by `vzFromEPg` and `vzToEPg`. A configured provider without such a relationship is not reported. Tenant-scope contracts are considered only when the contract, provider, and consumer belong to the same tenant. Shared/global pcTags `17` through `16385` are treated as fabric-wide identities; VRF separation is determined independently from the context-definition DNs. -Starting with 6.0(1g), ordinary EPG-to-EPG shared service is allowed. The unsupported condition remains when the Preferred Group provider is paired with an L3Out consumer in another VRF or with a `vzAny` consumer. Starting with 6.1(3f), this condition may be reported through F4684. +Before 6.0(1g), any consumer class in a materialized cross-VRF relationship can be affected. Depending on the release, the forwarding risk can be silent or the contract can be rejected with F0467 and `invalid-contract-config: Shared service provider cannot be in a Preferred Group`. -Before upgrading, remove the provider from the Preferred Group, stop it from providing the shared-service contract, or remove the unsupported L3Out/`vzAny` consumer relationship. See the [ACI Policy Model][78] for additional background. +Starting with 6.0(1g), ordinary EPG-to-EPG shared service is allowed. The unsupported condition remains only when the Preferred Group provider has a materialized relationship with an L3Out or `vzAny` consumer in another VRF. Same-VRF L3Out and `vzAny` relationships are not reported. Starting with 6.1(3f), this condition may be reported through F4684. + +Before upgrading, use the provider and consumer DNs shown in the result to remove the provider from the Preferred Group, stop it from providing the shared-service contract, or remove the unsupported relationship. See the [ACI Policy Model][78] for additional background. ## Defect Check Details diff --git a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py index b8e4c886..ae93129b 100644 --- a/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py +++ b/tests/checks/pg_and_shared_svc_contract_check/test_pg_and_shared_svc_contract_check.py @@ -16,21 +16,23 @@ shrd_contracts_api = 'vzBrCP.json' shrd_contracts_api += '?query-target-filter=or(eq(vzBrCP.scope,"global"),eq(vzBrCP.scope,"tenant"))' -# global epgs ( 16 <= pgtag <= 16385) with Preferred group enabled with provided contracts +# global epgs (17 <= pcTag <= 16385) with Preferred Group enabled and provided contracts glbl_epgs_api = 'fvAEPg.json' -glbl_epgs_api += '?query-target-filter=and(le(fvAEPg.pcTag,"16385"),ge(fvAEPg.pcTag,"16"),eq(fvAEPg.prefGrMemb,"include"))' +glbl_epgs_api += '?query-target-filter=and(le(fvAEPg.pcTag,"16385"),ge(fvAEPg.pcTag,"17"),eq(fvAEPg.prefGrMemb,"include"))' glbl_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' -# global external Epgs ( 16 <= pgtag <= 16385) with Preferred group enabled with provided contracts +# global external EPGs (17 <= pcTag <= 16385) with Preferred Group enabled and provided contracts glbl_ext_epgs_api = 'l3extInstP.json' -glbl_ext_epgs_api += '?query-target-filter=and(le(l3extInstP.pcTag,"16385"),ge(l3extInstP.pcTag,"16"),eq(l3extInstP.prefGrMemb,"include"))' +glbl_ext_epgs_api += '?query-target-filter=and(le(l3extInstP.pcTag,"16385"),ge(l3extInstP.pcTag,"17"),eq(l3extInstP.prefGrMemb,"include"))' glbl_ext_epgs_api += '&rsp-subtree=children&rsp-subtree-class=fvRsProv' -l3out_consumers_api = 'l3extInstP.json' -l3out_consumers_api += '?rsp-subtree=children&rsp-subtree-class=fvRsCons' -vzany_consumers_api = 'vzRtAnyToCons.json' +ctx_defs_api = 'fvCtxDef.json' +provider_relationships_api = 'vzFromEPg.json' +provider_relationships_api += '?query-target-filter=and(eq(vzFromEPg.membType,"prov"),' +provider_relationships_api += 'le(vzFromEPg.pcTag,"16385"),ge(vzFromEPg.pcTag,"17"))' +provider_relationships_api += '&rsp-subtree=children&rsp-subtree-class=vzToEPg' childless_fvAEPg = { "fvAEPg": { @@ -48,57 +50,108 @@ } } } -different_vrf_l3out_consumer = { - "l3extInstP": { - "attributes": { - "dn": "uni/tn-consumer/out-consumer/instP-different-vrf", - "scope": "999" - }, - "children": [ - { - "fvRsCons": { - "attributes": { - "tDn": "uni/tn-common/brc-AD_C" - } - } - } - ] - } -} -same_vrf_l3out_consumer = { - "l3extInstP": { - "attributes": { - "dn": "uni/tn-consumer/out-consumer/instP-same-vrf", - "scope": "2261001" - }, - "children": [ - { - "fvRsCons": { - "attributes": { - "tDn": "uni/tn-common/brc-AD_C" - } - } +provider_dn = "uni/tn-common/ap-apptest/epg-epg1" +provider_scope = "2261001" +provider_ctx_def_dn = "uni/ctx-[uni/tn-common/ctx-provider]" +external_provider_dn = "uni/tn-common/out-test-L3Out/instP-testExtEPG" +external_provider_scope = "2490368" +external_provider_ctx_def_dn = "uni/ctx-[uni/tn-common/ctx-external-provider]" +ordinary_consumer_dn = "uni/tn-consumer/ap-app/epg-consumer" +different_vrf_l3out_consumer_dn = "uni/tn-consumer/out-consumer/instP-different-vrf" +same_vrf_l3out_consumer_dn = "uni/tn-consumer/out-consumer/instP-same-vrf" +different_ctx_def_dn = "uni/ctx-[uni/tn-consumer/ctx-consumer]" + + +def ctx_def(scope, ctx_def_dn): + return { + "fvCtxDef": { + "attributes": { + "dn": ctx_def_dn, + "scope": scope } - ] + } } -} -unrelated_l3out_consumer = { - "l3extInstP": { - "attributes": { - "dn": "uni/tn-consumer/out-consumer/instP-unrelated", - "scope": "999" - }, - "children": [ - { - "fvRsCons": { - "attributes": { - "tDn": "uni/tn-common/brc-unrelated" + + +def provider_relationship( + contract, + provider, + provider_scope_id, + consumer, + consumer_ctx_def_dn, + consumer_scope_id="999" +): + return { + "vzFromEPg": { + "attributes": { + "dn": "cdef-[{}]/epgCont-[{}]/fr-[provider]".format( + contract, + provider + ), + "epgDn": provider, + "membType": "prov", + "scopeId": provider_scope_id + }, + "children": [ + { + "vzToEPg": { + "attributes": { + "ctxDefDn": consumer_ctx_def_dn, + "dn": ( + "cdef-[{}]/epgCont-[{}]/fr-[provider]/" + "to-[{}]" + ).format(contract, provider, consumer), + "epgDn": consumer, + "scopeId": consumer_scope_id + } } } - } - ] + ] + } } -} + + +provider_ctx_defs = [ + ctx_def(provider_scope, provider_ctx_def_dn), + ctx_def(external_provider_scope, external_provider_ctx_def_dn) +] +cross_context_ordinary_relationship = provider_relationship( + "uni/tn-common/brc-AD_C", + provider_dn, + provider_scope, + ordinary_consumer_dn, + different_ctx_def_dn +) +same_context_ordinary_relationship = provider_relationship( + "uni/tn-common/brc-AD_C", + provider_dn, + provider_scope, + ordinary_consumer_dn, + provider_ctx_def_dn, + provider_scope +) +cross_context_l3out_relationship = provider_relationship( + "uni/tn-common/brc-AD_C", + provider_dn, + provider_scope, + different_vrf_l3out_consumer_dn, + different_ctx_def_dn +) +same_context_l3out_relationship = provider_relationship( + "uni/tn-common/brc-AD_C", + provider_dn, + provider_scope, + same_vrf_l3out_consumer_dn, + provider_ctx_def_dn, + provider_scope +) +external_provider_l3out_relationship = provider_relationship( + "uni/tn-common/brc-AD_C", + external_provider_dn, + external_provider_scope, + different_vrf_l3out_consumer_dn, + different_ctx_def_dn +) tenant_contract = { "vzBrCP": { "attributes": { @@ -126,53 +179,38 @@ ] } } -tenant_l3out_consumer = { - "l3extInstP": { - "attributes": { - "dn": "uni/tn-test/out-consumer/instP-consumer", - "scope": "2000" - }, - "children": [ - { - "fvRsCons": { - "attributes": { - "tDn": "uni/tn-test/brc-tenant-shared" - } - } - } - ] - } -} -tenant_vzany_consumer = { - "vzRtAnyToCons": { - "attributes": { - "dn": ( - "uni/tn-test/brc-tenant-shared/" - "rtanyToCons-[uni/tn-test/ctx-consumer/any]" - ), - "tDn": "uni/tn-test/ctx-consumer/any" - } - } -} -unrelated_vzany_consumer = { - "vzRtAnyToCons": { - "attributes": { - "dn": ( - "uni/tn-test/brc-unrelated/" - "rtanyToCons-[uni/tn-test/ctx-consumer/any]" - ), - "tDn": "uni/tn-test/ctx-consumer/any" - } - } -} -malformed_vzany_consumer = { - "vzRtAnyToCons": { - "attributes": { - "dn": "uni/tn-test/brc-tenant-shared/bad-relation", - "tDn": "uni/tn-test/ctx-consumer/any" - } - } -} +tenant_provider_ctx_def_dn = "uni/ctx-[uni/tn-test/ctx-provider]" +tenant_consumer_ctx_def_dn = "uni/ctx-[uni/tn-test/ctx-consumer]" +tenant_l3out_consumer_dn = "uni/tn-test/out-consumer/instP-consumer" +tenant_vzany_consumer_dn = "uni/tn-test/ctx-consumer/any" +tenant_mismatch_consumer_dn = "uni/tn-other/out-consumer/instP-consumer" +tenant_ctx_defs = [ + ctx_def("1000", tenant_provider_ctx_def_dn) +] +tenant_l3out_relationship = provider_relationship( + "uni/tn-test/brc-tenant-shared", + "uni/tn-test/ap-provider/epg-provider", + "1000", + tenant_l3out_consumer_dn, + tenant_consumer_ctx_def_dn, + "2000" +) +tenant_vzany_relationship = provider_relationship( + "uni/tn-test/brc-tenant-shared", + "uni/tn-test/ap-provider/epg-provider", + "1000", + tenant_vzany_consumer_dn, + tenant_consumer_ctx_def_dn, + "2000" +) +tenant_mismatch_relationship = provider_relationship( + "uni/tn-test/brc-tenant-shared", + "uni/tn-test/ap-provider/epg-provider", + "1000", + tenant_mismatch_consumer_dn, + "uni/ctx-[uni/tn-other/ctx-consumer]", + "3000" +) @pytest.mark.parametrize( @@ -184,7 +222,9 @@ { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), - glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [cross_context_ordinary_relationship] }, "4.2(4a)", None, script.MANUAL, @@ -195,7 +235,9 @@ { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), - glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [cross_context_ordinary_relationship] }, "4.2(1a)", "4.1(2a)", script.NA, @@ -206,7 +248,9 @@ { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), - glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [cross_context_ordinary_relationship] }, "4.2(1a)", "5.1(1g)", script.FAIL_O, @@ -227,7 +271,9 @@ { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), - glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json") + glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [cross_context_ordinary_relationship] }, "4.2(1a)", "6.0(1f)", script.FAIL_O, @@ -237,7 +283,9 @@ { shrd_contracts_api: [tenant_contract], glbl_epgs_api: [tenant_provider], - glbl_ext_epgs_api: [] + glbl_ext_epgs_api: [], + ctx_defs_api: tenant_ctx_defs, + provider_relationships_api: [tenant_l3out_relationship] }, "4.2(1a)", "5.2(8i)", script.FAIL_O, @@ -248,8 +296,8 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), - l3out_consumers_api: [different_vrf_l3out_consumer], - vzany_consumers_api: [] + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [cross_context_l3out_relationship] }, "4.2(1a)", "6.0(1g)", script.FAIL_O, @@ -260,8 +308,8 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [], glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), - l3out_consumers_api: [different_vrf_l3out_consumer], - vzany_consumers_api: [] + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [external_provider_l3out_relationship] }, "4.2(1a)", "6.0(1g)", script.FAIL_O, @@ -271,7 +319,9 @@ { shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [childless_fvAEPg] + read_data(dir, "global_pg_fvAEPg.json"), - glbl_ext_epgs_api: [] + glbl_ext_epgs_api: [], + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [cross_context_ordinary_relationship] }, "4.2(1a)", "5.2(8i)", script.FAIL_O, @@ -282,8 +332,8 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [], glbl_ext_epgs_api: [childless_l3extInstP] + read_data(dir, "global_pg_l3extInstP.json"), - l3out_consumers_api: [different_vrf_l3out_consumer], - vzany_consumers_api: [] + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [external_provider_l3out_relationship] }, "4.2(1a)", "6.0(1g)", script.FAIL_O, @@ -315,8 +365,8 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), glbl_ext_epgs_api: [], - l3out_consumers_api: [], - vzany_consumers_api: [] + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [] }, "4.2(1a)", "6.0(1h)", script.PASS, @@ -337,8 +387,8 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), glbl_ext_epgs_api: [], - l3out_consumers_api: [childless_l3extInstP], - vzany_consumers_api: [] + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [cross_context_ordinary_relationship] }, "4.2(1a)", "6.0(1g)", script.PASS, @@ -349,8 +399,8 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [read_data(dir, "global_pg_fvAEPg.json")[0]], glbl_ext_epgs_api: [], - l3out_consumers_api: [same_vrf_l3out_consumer], - vzany_consumers_api: [] + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [same_context_l3out_relationship] }, "4.2(1a)", "6.0(1g)", script.PASS, @@ -361,8 +411,8 @@ shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), glbl_ext_epgs_api: [], - l3out_consumers_api: [unrelated_l3out_consumer], - vzany_consumers_api: [unrelated_vzany_consumer] + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [] }, "4.2(1a)", "6.0(1g)", script.PASS, @@ -377,6 +427,32 @@ def test_logic(run_check, mock_icurl, cversion, tversion, expected_result): assert result.result == expected_result +def test_provider_queries_exclude_reserved_and_local_pctags(run_check, monkeypatch): + queries = [] + + def recording_icurl(apitype, query, page=0, page_size=100000): + queries.append(query) + if query == shrd_contracts_api: + return [tenant_contract] + return [] + + monkeypatch.setattr(script, "icurl", recording_icurl) + + result = run_check( + cversion=script.AciVersion("5.2(8i)"), + tversion=script.AciVersion("5.2(8i)") + ) + + assert result.result == script.PASS + assert queries == [shrd_contracts_api, glbl_epgs_api, glbl_ext_epgs_api] + assert 'ge(fvAEPg.pcTag,"17")' in glbl_epgs_api + assert 'le(fvAEPg.pcTag,"16385")' in glbl_epgs_api + assert 'ge(l3extInstP.pcTag,"17")' in glbl_ext_epgs_api + assert 'le(l3extInstP.pcTag,"16385")' in glbl_ext_epgs_api + assert 'ge(vzFromEPg.pcTag,"17")' in provider_relationships_api + assert 'le(vzFromEPg.pcTag,"16385")' in provider_relationships_api + + @pytest.mark.parametrize( "icurl_outputs", [ @@ -384,8 +460,8 @@ def test_logic(run_check, mock_icurl, cversion, tversion, expected_result): shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: read_data(dir, "global_pg_fvAEPg.json"), glbl_ext_epgs_api: read_data(dir, "global_pg_l3extInstP.json"), - l3out_consumers_api: [different_vrf_l3out_consumer], - vzany_consumers_api: [] + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [cross_context_l3out_relationship] } ] ) @@ -416,8 +492,8 @@ def test_all_4_2_and_newer_targets_are_checked(run_check, mock_icurl, tversion): shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), glbl_epgs_api: [read_data(dir, "global_pg_fvAEPg.json")[0]], glbl_ext_epgs_api: [], - l3out_consumers_api: [different_vrf_l3out_consumer], - vzany_consumers_api: [] + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [cross_context_l3out_relationship] } ] ) @@ -448,8 +524,8 @@ def test_reports_correlated_l3out_consumer(run_check, mock_icurl): shrd_contracts_api: [tenant_contract], glbl_epgs_api: [tenant_provider], glbl_ext_epgs_api: [], - l3out_consumers_api: [tenant_l3out_consumer], - vzany_consumers_api: [] + ctx_defs_api: tenant_ctx_defs, + provider_relationships_api: [tenant_l3out_relationship] } ] ) @@ -475,8 +551,8 @@ def test_reports_tenant_scope_contract_across_vrfs(run_check, mock_icurl): shrd_contracts_api: [tenant_contract], glbl_epgs_api: [tenant_provider], glbl_ext_epgs_api: [], - l3out_consumers_api: [], - vzany_consumers_api: [tenant_vzany_consumer] + ctx_defs_api: tenant_ctx_defs, + provider_relationships_api: [tenant_vzany_relationship] } ] ) @@ -502,12 +578,148 @@ def test_reports_correlated_vzany_consumer(run_check, mock_icurl): shrd_contracts_api: [tenant_contract], glbl_epgs_api: [tenant_provider], glbl_ext_epgs_api: [], - l3out_consumers_api: [], - vzany_consumers_api: [malformed_vzany_consumer] + ctx_defs_api: tenant_ctx_defs, + provider_relationships_api: [provider_relationship( + "uni/tn-test/brc-tenant-shared", + "uni/tn-test/ap-provider/epg-provider", + "1000", + tenant_vzany_consumer_dn, + tenant_provider_ctx_def_dn, + "1000" + )] } ] ) -def test_rejects_malformed_vzany_reverse_relation(run_check, mock_icurl): +def test_same_context_vzany_consumer_is_not_reported(run_check, mock_icurl): + result = run_check( + cversion=script.AciVersion("5.2(8i)"), + tversion=script.AciVersion("6.0(1g)") + ) + + assert result.result == script.PASS + + +@pytest.mark.parametrize( + "icurl_outputs", + [ + { + shrd_contracts_api: [tenant_contract], + glbl_epgs_api: [tenant_provider], + glbl_ext_epgs_api: [], + ctx_defs_api: tenant_ctx_defs, + provider_relationships_api: [tenant_mismatch_relationship] + } + ] +) +def test_tenant_scope_relationship_requires_matching_tenant(run_check, mock_icurl): + result = run_check( + cversion=script.AciVersion("5.2(8i)"), + tversion=script.AciVersion("6.0(1g)") + ) + + assert result.result == script.PASS + + +@pytest.mark.parametrize( + "icurl_outputs", + [ + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [read_data(dir, "global_pg_fvAEPg.json")[0]], + glbl_ext_epgs_api: [], + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [] + } + ] +) +def test_provider_without_materialized_relationship_is_not_reported( + run_check, + mock_icurl +): + result = run_check( + cversion=script.AciVersion("5.2(8i)"), + tversion=script.AciVersion("5.2(8i)") + ) + + assert result.result == script.PASS + + +@pytest.mark.parametrize( + "icurl_outputs", + [ + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [read_data(dir, "global_pg_fvAEPg.json")[0]], + glbl_ext_epgs_api: [], + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [same_context_ordinary_relationship] + } + ] +) +def test_same_context_relationship_is_not_reported_before_6_0( + run_check, + mock_icurl +): + result = run_check( + cversion=script.AciVersion("5.2(8i)"), + tversion=script.AciVersion("5.2(8i)") + ) + + assert result.result == script.PASS + + +@pytest.mark.parametrize( + "icurl_outputs", + [ + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [read_data(dir, "global_pg_fvAEPg.json")[0]], + glbl_ext_epgs_api: [], + ctx_defs_api: [], + provider_relationships_api: [cross_context_l3out_relationship] + } + ] +) +def test_missing_provider_context_is_an_error(run_check, mock_icurl): + result = run_check( + cversion=script.AciVersion("5.2(8i)"), + tversion=script.AciVersion("6.0(1g)") + ) + + assert result.result == script.ERROR + assert result.msg == ( + "Unable to resolve context for one or more derived contract relationships" + ) + assert result.data == [[ + ( + "cdef-[uni/tn-common/brc-AD_C]/" + "epgCont-[uni/tn-common/ap-apptest/epg-epg1]/fr-[provider]" + ), + "No fvCtxDef found for scopeId 2261001" + ]] + assert "Retry the check" in result.recommended_action + assert "contact Cisco TAC" in result.recommended_action + + +@pytest.mark.parametrize( + "icurl_outputs", + [ + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [read_data(dir, "global_pg_fvAEPg.json")[0]], + glbl_ext_epgs_api: [], + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [provider_relationship( + "uni/tn-common/brc-AD_C", + provider_dn, + provider_scope, + different_vrf_l3out_consumer_dn, + "" + )] + } + ] +) +def test_missing_consumer_context_is_an_error(run_check, mock_icurl): result = run_check( cversion=script.AciVersion("5.2(8i)"), tversion=script.AciVersion("6.0(1g)") @@ -515,6 +727,55 @@ def test_rejects_malformed_vzany_reverse_relation(run_check, mock_icurl): assert result.result == script.ERROR assert result.msg == ( - "Failed to get contract DN from vzRtAnyToCons DN: " - "uni/tn-test/brc-tenant-shared/bad-relation" + "Unable to resolve context for one or more derived contract relationships" + ) + assert result.data == [[ + ( + "cdef-[uni/tn-common/brc-AD_C]/" + "epgCont-[uni/tn-common/ap-apptest/epg-epg1]/fr-[provider]/" + "to-[uni/tn-consumer/out-consumer/instP-different-vrf]" + ), + "vzToEPg.ctxDefDn is empty" + ]] + assert "Retry the check" in result.recommended_action + assert "contact Cisco TAC" in result.recommended_action + + +@pytest.mark.parametrize( + "icurl_outputs", + [ + { + shrd_contracts_api: read_data(dir, "global_vzBrCP_pos.json"), + glbl_epgs_api: [read_data(dir, "global_pg_fvAEPg.json")[0]], + glbl_ext_epgs_api: [], + ctx_defs_api: provider_ctx_defs, + provider_relationships_api: [ + cross_context_l3out_relationship, + provider_relationship( + "uni/tn-common/brc-AD_C", + provider_dn, + "missing-scope", + ordinary_consumer_dn, + different_ctx_def_dn + ) + ] + } + ] +) +def test_context_error_preserves_confirmed_affected_relationship( + run_check, + mock_icurl +): + result = run_check( + cversion=script.AciVersion("5.2(8i)"), + tversion=script.AciVersion("6.0(1g)") ) + + assert result.result == script.ERROR + assert result.data[0][1] == "No fvCtxDef found for scopeId missing-scope" + assert result.unformatted_data == [[ + "uni/tn-common/brc-AD_C", + provider_dn, + "5555", + different_vrf_l3out_consumer_dn + ]]