Skip to content

Conversation

@iko1
Copy link
Contributor

@iko1 iko1 commented Dec 29, 2025

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Motivation and Context

Fixes: #992

Port validation previously rejected valid configurations when the same port number was used for different protocols (TCP and UDP). For example:

-p 1024:1024/udp -p 1024:1024/tcp

Although this is a valid and common use case, the validation logic treated it as a conflict.

To fix this, I updated the validation key to include the protocol name. The validation now checks for overlapping port numbers only within the same protocol, rather than across all protocols.

This change enables binding the same port number for both TCP and UDP, aligning the validation behavior with real-world networking requirements.

Testing

  • Tested locally
  • Added/updated tests
  • Added/updated docs

@iko1 iko1 force-pushed the fix/handle-overlapping-ports-different-protocols branch from 627dc96 to cdf10b1 Compare December 29, 2025 16:04
@Test
func testPublishPortsSamePortDifferentProtocols() throws {
let result = try Parser.publishPorts([
"8080:8080/tcp",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a test case where the protocol isn't chosen so we can verify the default (tcp)? e.g.

"8081:8081",
"8081:8081/udp"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

@dcantah
Copy link
Contributor

dcantah commented Jan 1, 2026

@iko1 Can you fix the build issues?

@iko1 iko1 requested a review from dcantah January 1, 2026 21:23
@iko1
Copy link
Contributor Author

iko1 commented Jan 2, 2026

@iko1 Can you fix the build issues?

I don't think the current build issue is related to my change.

@iko1 iko1 force-pushed the fix/handle-overlapping-ports-different-protocols branch from 5a8d8ce to d085d67 Compare January 2, 2026 16:00
@jglogan
Copy link
Contributor

jglogan commented Jan 2, 2026

@iko1 it's definitely not you. There's some weirdness in the OSS header checks due to the new year, and to the CI builds merging PRs against main before the header checks.

Copy link
Contributor

@jglogan jglogan left a comment

Choose a reason for hiding this comment

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

lgtm, thank you for the fix!

@jglogan
Copy link
Contributor

jglogan commented Jan 3, 2026

@iko1 it looks like you might need to push this again such that all commits have verified signatures

@iko1 iko1 force-pushed the fix/handle-overlapping-ports-different-protocols branch 2 times, most recently from d085d67 to 4d3c396 Compare January 3, 2026 15:50
@iko1 iko1 force-pushed the fix/handle-overlapping-ports-different-protocols branch from 4d3c396 to 5112162 Compare January 3, 2026 15:52
@iko1
Copy link
Contributor Author

iko1 commented Jan 3, 2026

@iko1 it looks like you might need to push this again such that all commits have verified signatures

@jglogan Fixed, all commits in this PR are now re-signed.

@jglogan jglogan merged commit df368b7 into apple:main Jan 4, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Cannot publish same port with both TCP and UDP

3 participants