Skip to content

Accept "0" as a valid port value to support UDS#242

Merged
sarah-witt merged 2 commits into
jenkinsci:masterfrom
sa-spag:valid-port
Dec 17, 2021
Merged

Accept "0" as a valid port value to support UDS#242
sarah-witt merged 2 commits into
jenkinsci:masterfrom
sa-spag:valid-port

Conversation

@sa-spag

@sa-spag sa-spag commented Sep 1, 2021

Copy link
Copy Markdown
Contributor

What does this PR do?

It makes port "0" valid, as the java-dogstatsd-client accepts it to support Unix Domain Socket (UDS).

Description of the Change

Any string that can be parsed as a positive integer is considered a valid port.

Alternate Designs

Further improvements could test for NumberUtils.createInteger(targetPort) <= 65535, unless there are other special values I am not aware of. Also, the current validation does not test for the hostname to correspond to a socket path when the port is set to 0.

Possible Drawbacks

None.

Verification Process

None.

Additional Notes

None.

Release Notes

None.

Review checklist (to be filled by reviewers)

  • Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • PR should not have do-not-merge/ label attached.
  • If Applicable, issue must have kind/ and severity/ labels attached at least.

java-dogstatsd-client 2.6 and later versions support Unix Domain Socket (UDS). To use this connection method, one must set the port to "0".
@github-actions

github-actions Bot commented Oct 2, 2021

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had activity in the last 30 days.
Note that the issue will not be automatically closed, but this notification will remind us to investigate why there's been inactivity.

@github-actions github-actions Bot added the stale Stale - Bot reminder label Oct 2, 2021
@Xyaren

Xyaren commented Oct 4, 2021

Copy link
Copy Markdown

I think this is still a valid requirement :)

@sarah-witt sarah-witt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry for the delay in review, this looks good to me!

@sarah-witt
sarah-witt merged commit 04e000f into jenkinsci:master Dec 17, 2021
@sa-spag
sa-spag deleted the valid-port branch December 18, 2021 20:06
@sarah-witt sarah-witt added changelog/Fixed Fixed features results into a bug fix version bump and removed stale Stale - Bot reminder labels Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/Fixed Fixed features results into a bug fix version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants