Skip to content

Commit bcb0566

Browse files
[JCM] Fix DNS-SD Port Overrides, Port Randomization Collisions, and Storage Conflicts in Joint Fabrics JCM Python Tests (project-chip#72327)
* feat: skip DNS-SD port overwrite in combined server/commissioner * feat: use port randomization in Joint Fabric Python tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add more logic to random port selection * Restyle * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * feat: consolidate storage and add robust teardown cleanup in JCM Python tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix: preserve preventDnssdPortOverwrite on reinit and resolve Python test feedback * fix: resolve CA manager flakiness, memory leaks, and inconsistent teardowns - Implement a strict fail-fast IPv6 port check in get_random_port to avoid hangs. - Ensure consistent None assignments in TC_JFADMIN_2_1.py teardown. - Proactively resolve CA manager and persistent storage resource leaks in TC_JFADMIN_1_1.py and TC_JFDS_2_1.py by promoting them to instance variables and ensuring explicit shutdown in teardown_class. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add comment explaining preventDnssdPortOverwrite in CommissionerMain * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent b578401 commit bcb0566

14 files changed

Lines changed: 263 additions & 159 deletions

File tree

examples/platform/linux/CommissionerMain.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,10 @@ CHIP_ERROR InitCommissioner(uint16_t commissionerPort, uint16_t udcListenPort, F
134134
factoryParams.fabricIndependentStorage = &gServerStorage;
135135
factoryParams.fabricTable = &Server::GetInstance().GetFabricTable();
136136
factoryParams.sessionKeystore = &gSessionKeystore;
137+
factoryParams.enableServerInteractions = true;
138+
// Since CommissionerMain is used in combined server/commissioner applications,
139+
// we must prevent the commissioner from overwriting the server's DNS-SD port.
140+
factoryParams.preventDnssdPortOverwrite = true;
137141
// We're running alongside an existing Server, so use the existing DataModelProvider
138142
// without changing the underlying persistent storage delegate.
139143
factoryParams.dataModelProvider = chip::app::CodegenDataModelProviderInstance(nullptr);

src/controller/CHIPDeviceControllerFactory.cpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ CHIP_ERROR DeviceControllerFactory::Init(FactoryInitParams params)
6969
mCertificateValidityPolicy = params.certificateValidityPolicy;
7070
mSessionResumptionStorage = params.sessionResumptionStorage;
7171
mEnableServerInteractions = params.enableServerInteractions;
72+
mPreventDnssdPortOverwrite = params.preventDnssdPortOverwrite;
7273
mDataModelProvider = params.dataModelProvider;
7374

7475
// Initialize the system state. Note that it is left in a somewhat
@@ -96,6 +97,7 @@ CHIP_ERROR DeviceControllerFactory::ReinitSystemStateIfNecessary()
9697
params.interfaceId = mInterfaceId;
9798
params.fabricIndependentStorage = mFabricIndependentStorage;
9899
params.enableServerInteractions = mEnableServerInteractions;
100+
params.preventDnssdPortOverwrite = mPreventDnssdPortOverwrite;
99101
params.groupDataProvider = mSystemState->GetGroupDataProvider();
100102
params.sessionKeystore = mSystemState->GetSessionKeystore();
101103
params.fabricTable = mSystemState->Fabrics();
@@ -288,15 +290,18 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
288290
stateParams.exchangeMgr, stateParams.sessionMgr, stateParams.fabricTable, sessionResumptionStorage,
289291
stateParams.certificateValidityPolicy, stateParams.groupDataProvider));
290292

291-
// Our IPv6 transport is at index 0.
292-
app::DnssdServer::Instance().SetSecuredIPv6Port(
293-
stateParams.transportMgr->GetTransport().GetImplAtIndex<0>().GetBoundPort());
293+
if (!params.preventDnssdPortOverwrite)
294+
{
295+
// Our IPv6 transport is at index 0.
296+
app::DnssdServer::Instance().SetSecuredIPv6Port(
297+
stateParams.transportMgr->GetTransport().GetImplAtIndex<0>().GetBoundPort());
294298

295299
#if INET_CONFIG_ENABLE_IPV4
296-
// If enabled, our IPv4 transport is at index 1.
297-
app::DnssdServer::Instance().SetSecuredIPv4Port(
298-
stateParams.transportMgr->GetTransport().GetImplAtIndex<1>().GetBoundPort());
300+
// If enabled, our IPv4 transport is at index 1.
301+
app::DnssdServer::Instance().SetSecuredIPv4Port(
302+
stateParams.transportMgr->GetTransport().GetImplAtIndex<1>().GetBoundPort());
299303
#endif // INET_CONFIG_ENABLE_IPV4
304+
}
300305

301306
if (params.interfaceId)
302307
{

src/controller/CHIPDeviceControllerFactory.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,12 @@ struct FactoryInitParams
161161
//
162162
bool enableServerInteractions = false;
163163

164+
// Controls whether the controller factory initialization is prevented from
165+
// overwriting the global DnssdServer's secured port. When running in a
166+
// combined server/controller app where the server's port is primary, this
167+
// must be set to true to keep DNS-SD advertising the server's port.
168+
bool preventDnssdPortOverwrite = false;
169+
164170
/* The port used for operational communication to listen for and send messages over UDP/TCP.
165171
* The default value of `0` will pick any available port. */
166172
uint16_t listenPort = 0;
@@ -315,6 +321,7 @@ class DeviceControllerFactory
315321
SessionResumptionStorage * mSessionResumptionStorage = nullptr;
316322
app::DataModel::Provider * mDataModelProvider = nullptr;
317323
bool mEnableServerInteractions = false;
324+
bool mPreventDnssdPortOverwrite = false;
318325
};
319326

320327
} // namespace Controller

src/python_testing/TC_JFADMIN_1_1.py

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ async def setup_class(self):
6262
self.fabric_a_ctrl = None
6363
self.fabric_a_admin = None
6464
self.storage_fabric_a = self.user_params.get("fabric_a_storage", None)
65+
self.fabric_a_persistent_storage = None
66+
self.cert_authority_manager_a = None
67+
self.dev_ctrl_eco_a = None
6568

6669
self.jfc_server_app = self.user_params.get("jfc_server_app", None)
6770
if not self.jfc_server_app:
@@ -88,6 +91,16 @@ def teardown_class(self):
8891
if self.fabric_a_ctrl is not None:
8992
self.fabric_a_ctrl.terminate()
9093

94+
if self.dev_ctrl_eco_a is not None:
95+
self.dev_ctrl_eco_a.Shutdown()
96+
self.dev_ctrl_eco_a = None
97+
if self.cert_authority_manager_a is not None:
98+
self.cert_authority_manager_a.Shutdown()
99+
self.cert_authority_manager_a = None
100+
if self.fabric_a_persistent_storage is not None:
101+
self.fabric_a_persistent_storage.Shutdown()
102+
self.fabric_a_persistent_storage = None
103+
91104
super().teardown_class()
92105

93106
def steps_TC_JFADMIN_1_1(self) -> list[TestStep]:
@@ -115,12 +128,12 @@ async def test_TC_JFADMIN_1_1(self):
115128
dut_rpc_server_ip = "127.0.0.1"
116129
jfadmin_fabric_a_passcode = random.randint(110220011, 110220999)
117130
jfadmin_fabric_a_discriminator = random.randint(0, 4095)
118-
dut_rpc_server_port = "33033"
131+
dut_rpc_server_port = str(self.get_random_port())
119132
# Start Fabric A JF-Administrator App
120133
self.fabric_a_admin = AppServerSubprocess(
121134
self.jfa_server_app,
122135
storage_dir=self.storage_fabric_a,
123-
port=random.randint(5001, 5999),
136+
port=self.get_random_port(),
124137
discriminator=jfadmin_fabric_a_discriminator,
125138
passcode=jfadmin_fabric_a_passcode,
126139
extra_args=["--capabilities", "0x04", "--rpc-server-port", dut_rpc_server_port])
@@ -182,20 +195,20 @@ async def test_TC_JFADMIN_1_1(self):
182195
self.ecoACATs = base64.b64decode(jfcStorage.get("Default", "CommissionerCATs"))[::-1].hex().strip('0')
183196

184197
# Creating a Controller for Ecosystem A
185-
_fabric_a_persistent_storage = VolatileTemporaryPersistentStorage(
198+
self.fabric_a_persistent_storage = VolatileTemporaryPersistentStorage(
186199
self.ecoACtrlStorage['repl-config'], self.ecoACtrlStorage['sdk-config'])
187-
_certAuthorityManagerA = CertificateAuthority.CertificateAuthorityManager(
200+
self.cert_authority_manager_a = CertificateAuthority.CertificateAuthorityManager(
188201
chipStack=self.matter_stack._chip_stack,
189-
persistentStorage=_fabric_a_persistent_storage)
190-
_certAuthorityManagerA.LoadAuthoritiesFromStorage()
191-
devCtrlEcoA = _certAuthorityManagerA.activeCaList[0].adminList[0].NewController(
202+
persistentStorage=self.fabric_a_persistent_storage)
203+
self.cert_authority_manager_a.LoadAuthoritiesFromStorage()
204+
self.dev_ctrl_eco_a = self.cert_authority_manager_a.activeCaList[0].adminList[0].NewController(
192205
nodeId=101,
193206
paaTrustStorePath=str(self.matter_test_config.paa_trust_store_path),
194207
catTags=[int(self.ecoACATs, 16)])
195208

196209
if self.pics_guard(self.check_pics("JFADMIN.S.A0000")):
197210
self.step("2")
198-
response = await devCtrlEcoA.ReadAttribute(
211+
response = await self.dev_ctrl_eco_a.ReadAttribute(
199212
nodeId=1, attributes=[(1, Clusters.JointFabricAdministrator.Attributes.AdministratorFabricIndex)],
200213
returnClusterObject=True)
201214
attributeAdminFabricIndex = response[1][Clusters.JointFabricAdministrator].administratorFabricIndex
@@ -204,7 +217,7 @@ async def test_TC_JFADMIN_1_1(self):
204217

205218
# Step 3 is under same PICS guard as Step 2 becasue it relies on attributeAdminFabricIndex
206219
self.step("3")
207-
response = await devCtrlEcoA.ReadAttribute(
220+
response = await self.dev_ctrl_eco_a.ReadAttribute(
208221
nodeId=1, attributes=[(0, Clusters.OperationalCredentials.Attributes.Fabrics)],
209222
returnClusterObject=True)
210223
fabricid_found = False
@@ -217,9 +230,6 @@ async def test_TC_JFADMIN_1_1(self):
217230
asserts.assert_true(fabricid_found,
218231
"No FabricDescriptorStruct with fabricIndex = AdministratorFabricIndex found in Operational Cluster Fabrics attribute on EP0")
219232

220-
# Shutdown the Python Controllers started at the beginning of this script
221-
devCtrlEcoA.Shutdown()
222-
223233

224234
if __name__ == "__main__":
225235
default_matter_test_main()

src/python_testing/TC_JFADMIN_1_2.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,15 @@ async def setup_class(self):
8383
if self.is_pics_sdk_ci_only:
8484
self.admin_passcode = random.randint(20202021, 20202099)
8585
self.admin_discriminator = random.randint(0, 4095)
86+
rpc_port = self.get_random_port()
8687
# Start JF-Administrator App
8788
self.jf_admin = AppServerSubprocess(
8889
self.jfa_server_app,
8990
storage_dir=self.fabric_storage,
90-
port=random.randint(5001, 5999),
91+
port=self.get_random_port(),
9192
discriminator=self.admin_discriminator,
9293
passcode=self.admin_passcode,
93-
extra_args=["--capabilities", "0x04", "--rpc-server-port", "33033"])
94+
extra_args=["--capabilities", "0x04", "--rpc-server-port", str(rpc_port)])
9495
self.jf_admin.start(
9596
expected_output="Server initialization complete",
9697
timeout=10)

src/python_testing/TC_JFADMIN_1_4.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,12 +192,12 @@ async def setup_class(self):
192192
self.jfadmin_fabric_a_passcode = random.randint(20202021, 20202099)
193193
self.jfadmin_fabric_a_discriminator = random.randint(0, 4095)
194194
dut_rpc_server_ip = "127.0.0.1"
195-
dut_rpc_server_port = "33033"
195+
dut_rpc_server_port = str(self.get_random_port())
196196
self.fabric_a_admin = JFAdministratorSubprocess(
197197
self.jfa_server_app,
198198
prefix="JFA-A",
199199
storage_dir=self.storage_fabric_a,
200-
port=random.randint(5001, 5999),
200+
port=self.get_random_port(),
201201
discriminator=self.jfadmin_fabric_a_discriminator,
202202
passcode=self.jfadmin_fabric_a_passcode,
203203
extra_args=["--capabilities", "0x04", "--rpc-server-port", dut_rpc_server_port, "--min_commissioning_timeout", f"{self.ojcw_timeout_baseline}"])

0 commit comments

Comments
 (0)