fix: dedupe exposed ports with normalized protocols - #2103
Conversation
|
|
|
Welcome @immanuwell! |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: immanuwell The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
c7e0b0d to
e539056
Compare
|
@HaraldNordgren just rebaised this PR and now also all CI checks should be green |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: HaraldNordgren, immanuwell The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
cdrage
left a comment
There was a problem hiding this comment.
So sorry been a delay in reviews!
Can you please add a unit test as well?
These are typically added to this folder: https://github.com/kubernetes/kompose/tree/main/script/test/fixtures
Fixes a tiny duplicate-port edge case
Repro before this patch:
portscomes from compose-go astcp, whileexposedefaults toTCP, so the dedupe key missed and emitted80/TCPtwice. not great.This normalizes protocols before checking the key, so
80/TCPis kept once and8080/TCPstill gets added.Related history: #1070 / #1223 fixed another duplicate-port path; this is the ports+expose version.
Tested:
go test ./...