Conversation
|
@vadimeisenbergibm: Adding do-not-merge/release-note-label-needed because the release note process has not been followed. DetailsOne of the following labels is required "release-note", "release-note-action-required", or "release-note-none". Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these OWNERS Files:You can indicate your approval by writing |
| if err := ValidatePort(int(port.GetNumber())); err != nil { | ||
| return err | ||
| } | ||
|
|
There was a problem hiding this comment.
You need a conversion function here, to convert the Port spec into our internal Port struct in the service model. That would allow you to reuse code in proxy/envoy/
| portNumber := int(port.GetNumber()) | ||
| modelPort := &model.Port{Name: fmt.Sprintf("external-%v-%d", protocol, portNumber), | ||
| Port: portNumber, Protocol: protocol} | ||
| httpConfig := httpConfigs.EnsurePort(portNumber) |
There was a problem hiding this comment.
See my earlier comment. We have a Port struct in the service model. Just convert the Port (proto) to that struct. That would eliminate these changes. you could even hoist the port name construction (external-x-y) into that conversion function..
@ijsnellf thoughts? I leave it to you guys
|
closing as per discussion in istio/api |
What this PR does / why we need it:
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Release note: