ref: method renaming for cataloger endpoint methods #3744
Conversation
| for _, ep := range mc.listEndpointsForServiceIdentity(destSvcIdentity) { | ||
| epIPStr := ep.IP.String() | ||
| // check if endpoint IP is allowed | ||
| if _, ok := outboundEndpointsSet[epIPStr]; ok { | ||
| // add all allowed endpoints on the pod to result list | ||
| // TODO(allenlsy): only allow endpoint with matching port |
There was a problem hiding this comment.
Current implementation is correct. Thus it's no longer a TODO
There was a problem hiding this comment.
The reason the functions have Allowed in the name is because it filters the endpoints based on the policies, where some endpoints are disallowed and the rest allowed. There are 2 options to resolve any ambiguity:
- Keep the function names as before
- Rename the function names, and update all comments to make it explicit that only a subset of endpoints corresponding to the service identity will be returned by these functions
|
(Removed) |
You mean keeping the previous function names or the changes in this PR? |
|
(Removed) |
allenlsy
force-pushed
the
cataloger-list-endpoints-ref
branch
from
715a29e to
67af702
Compare
| @@ -52,7 +52,6 @@ func (mc *MeshCatalog) ListEndpointsForServiceIdentity(downstreamIdentity identi | |||
| ipStr := ep.IP.String() | |||
| outboundEndpointsSet[ipStr] = append(outboundEndpointsSet[ipStr], ep) | |||
| } | |||
| log.Info().Msgf("outbound endpoints: %v", outboundEndpointsSet) | |||
There was a problem hiding this comment.
This was added by mistake initially. It was used for debugging.
| @@ -38,8 +38,8 @@ func (mc *MeshCatalog) GetResolvableServiceEndpoints(svc service.MeshService) ([ | |||
| return endpoints, nil | |||
| } | |||
|
|
|||
| // ListEndpointsForServiceIdentity returns only those endpoints for a allowed outbound service accounts | |||
| // for the given downstream identity | |||
| // ListEndpointsForServiceIdentity returns a list of endpoints that belongs to an upstream service accounts | |||
There was a problem hiding this comment.
| // ListEndpointsForServiceIdentity returns a list of endpoints that belongs to an upstream service accounts | |
| // ListEndpointsForServiceIdentity returns a list of endpoints that belongs to an upstream service account |
Codecov Report
@@ Coverage Diff @@
## main #3744 +/- ##
==========================================
+ Coverage 69.54% 69.55% +0.01%
==========================================
Files 185 185
Lines 9035 9033 -2
==========================================
Hits 6283 6283
+ Misses 2703 2701 -2
Partials 49 49
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
allenlsy
force-pushed
the
cataloger-list-endpoints-ref
branch
from
67af702 to
807eed9
Compare
Signed-off-by: Allen Leigh <[email protected]>
allenlsy
force-pushed
the
cataloger-list-endpoints-ref
branch
from
807eed9 to
cdae897
Compare
|
The final choice is to go with dropping the |
Signed-off-by: Allen Leigh [email protected]
Description:
This PR aims to resolve issue #3190 .
It mainly drops the word
allowedfrom cataloger endpoints methods.Affected area:
Please answer the following questions with yes/no.
No
No