Skip to content

[C] Remove mqttv5 mock hub bypass and DPS connectionProfile override - #414

Merged
Ewerton Scaboro da Silva (ewertons) merged 2 commits into
mainfrom
c/remove-mqttv5-mock-and-profile-override
Oct 12, 2026
Merged

[C] Remove mqttv5 mock hub bypass and DPS connectionProfile override#414
Ewerton Scaboro da Silva (ewertons) merged 2 commits into
mainfrom
c/remove-mqttv5-mock-and-profile-override

Conversation

@ewertons

@ewertons Ewerton Scaboro da Silva (ewertons) commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

DPS now returns connectionProfile, and the C client reaches mqttv5 hubs through DPS with no override. Direct mqttv5 connects are already supported through opts.host / opts.port / opts.connection_profile.

Removed

  • Connection client: the AZ_IOT_HUB_MQTT_V5_MOCK_ENDPOINT / AZ_IOT_DEVICE_ID bypass (skipped DPS) and AZ_IOT_DPS_CONNECTION_PROFILE_OVERRIDE.
  • c/tests/manual: local mqttv5 hub mock scripts.
  • Samples: mock support; they provision through DPS only.
  • 8 unit tests covering the removed paths; related docs. check-layering.sh drops the exemption that only those scripts needed.

Unchanged

  • Absent/null connectionProfile still resolves to mqttv3.
  • az_iot_env_read() stays (coding-convention seam; no library caller remains).
  • Mosquitto-based adapter conformance tests are unrelated and stay.

Validation (Linux, gcc, linux-gcc-debug + samples)

  • Build: no warnings.
  • ctest: 49/49 passed (conformance/integration against a local Mosquitto).
  • check-banned-constructs, check-layering, check-version, check-log-components, code-style (clang-format 18.1.8): clean.

@ewertons Ewerton Scaboro da Silva (ewertons) added the enhancement New feature or request label Oct 11, 2026
Copilot AI balanced review requested due to automatic review settings October 11, 2026 08:03
@ewertons Ewerton Scaboro da Silva (ewertons) added the enhancement New feature or request label Oct 11, 2026
Copilot AI reviewed Oct 11, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

DPS rollout compatibility and legacy credential-ignore protection must be addressed before approval.

0 open findings

What changed in this PR

Removes development-only MQTTv5 bypasses so DPS exclusively determines the assigned connection profile.

Changes:

  • Removes environment-variable overrides and related unit tests.
  • Deletes local MQTTv5 mock tooling and sample support.
  • Updates documentation and layering checks.
File Description
c/​src/​core/​connection_client.c Removes mock and profile overrides.
c/​inc/​azure/​iot/​az_iot_connection_client.h Updates profile contract documentation.
c/​tests/​unit/​connection_dps_test.c Removes override-specific tests.
c/​tests/​manual/​.gitignore Removes generated-artifact ignores.
c/​tests/​manual/​New-TestEnv.ps1 Deletes mock environment setup.
c/​tests/​manual/​New-TestClient.ps1 Deletes mock client setup.
c/​tests/​manual/​Remove-TestEnv.ps1 Deletes environment cleanup.
c/​tests/​manual/​Remove-TestClient.ps1 Deletes client cleanup.
c/​tests/​manual/​Read-ServiceLogs.ps1 Deletes mock log helper.
c/​samples/​common/​sample_utils.c Removes mock configuration handling.
c/​samples/​common/​sample_utils.h Removes mock configuration fields.
c/​samples/​unified/​telemetry/​main.c Updates provisioning guidance.
c/​samples/​unified/​file_upload/​main.c Removes mock-specific rejection.
c/​samples/​unified/​connect_first/​main.c Always reads the connected profile.
c/​samples/​unified/​c2d_receiver/​main.c Relies on profile mismatch handling.
c/​eng/​check-layering.sh Removes the deleted tooling exemption.
c/​docs/​eng/​test-coverage.md Removes override test documentation.
c/​docs/​eng/​connection-c.md Removes override behavior documentation.
c/​CHANGELOG.md Records the breaking removal.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI balanced review requested due to automatic review settings October 11, 2026 08:10
@ewertons

Ewerton Scaboro da Silva (ewertons) commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

Re: Copilot review summary.

  • Ignore rules for local credentials: applied in 836a5e8. c/.gitignore still ignores tests/manual/.certs/ (generated private keys), .env-mqttv5/ and .client-env.ps1, so existing checkouts cannot commit them.
  • DPS rollout compatibility: not changed. The override applied only when connectionProfile was absent or null. The rollout failure actually observed was an explicit "classic", which the override could not correct. Absent or null still resolves to mqttv3 by contract. A direct mqttv5 connect remains available through opts.host/opts.port/opts.connection_profile. The removal is recorded as breaking in CHANGELOG.md.

Copilot AI reviewed Oct 11, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Removal depends on an unverified rollout of a future-dated DPS preview API across supported endpoints.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Medium severity Retain guarded MQTT fallback until connectionProfile rollout is confirmed

c/​src/​core/​connection_client.c:2891

This makes an absent profile unconditionally select MQTTv3 even though the only API that carries connectionProfile is the pinned 2026-11-02-preview version (connection_client.c:114-127), dated after this PR. Public DPS documentation currently does not establish that this preview and field are deployed across every supported global/regional/private endpoint. Removing the bridge before that rollout is verified can route an MQTTv5 assignment through MQTTv3. Please retain a guarded fallback until service coverage is confirmed, or gate this removal on that deployment.

🧠 Review effort: Balanced

@ewertons

Ewerton Scaboro da Silva (ewertons) commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

Re: review of 836a5e8 (DPS preview rollout): not changed. The 2026-11-02-preview pin comes from #254, not this PR. Where DPS leaves connectionProfile out, the device still resolves to mqttv3 per the contract. The removed override only covered that case for local development; it did not handle an explicit wrong value. Whether to keep a fallback until the API is GA is a maintainer decision.

@ewertons
[C] Remove mqttv5 mock hub bypass and DPS connectionProfile override
dd635f3 
DPS now returns connectionProfile, and the SDK connects to mqttv5 hubs
through DPS without an override. Direct mqttv5 connects are already
supported through opts.host/opts.port/opts.connection_profile.

- Remove AZ_IOT_HUB_MQTT_V5_MOCK_ENDPOINT / AZ_IOT_DEVICE_ID bypass from
  the connection client, and AZ_IOT_DPS_CONNECTION_PROFILE_OVERRIDE.
- Remove the local mqttv5 hub mock scripts (c/tests/manual).
- Remove mock support from samples; they provision through DPS.
- Remove the related unit tests and docs.
@ewertons
[C] Keep ignoring files left by the removed mock scripts
6a2d78c 
c/tests/manual/.certs/ holds private keys generated by the deleted
scripts; keep it, .env-mqttv5/ and .client-env.ps1 ignored so existing
checkouts cannot commit them.
Copilot AI balanced review requested due to automatic review settings October 11, 2026 16:06
@ewertons
Ewerton Scaboro da Silva (ewertons) force-pushed the c/remove-mqttv5-mock-and-profile-override branch from 836a5e8 to 6a2d78c Compare October 11, 2026 16:06
Copilot AI reviewed Oct 11, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Removing the fallback depends on an unverified rollout of a future-dated DPS API across all supported endpoint classes.

0 open findings

🧠 Review effort: Balanced

@ewertons
Ewerton Scaboro da Silva (ewertons) merged commit c964613 into main Oct 12, 2026
57 checks passed
@ewertons
Ewerton Scaboro da Silva (ewertons) deleted the c/remove-mqttv5-mock-and-profile-override branch October 12, 2026 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@maximsemenov80 Maxim Semenov (maximsemenov80) Awaiting requested review from maximsemenov80 maximsemenov80 is a code owner
@avishekpant Avishek (avishekpant) Awaiting requested review from avishekpant avishekpant is a code owner

Assignees

@ewertons Ewerton Scaboro da Silva (ewertons)

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ewertons