Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

ref: method renaming for cataloger endpoint methods #3744

Merged
merged 1 commit into from
Jul 8, 2021
Merged

ref: method renaming for cataloger endpoint methods #3744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions DESIGN.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,11 +302,11 @@ type MeshCataloger interface {
// GetSMISpec returns the SMI spec
GetSMISpec() smi.MeshSpec

// ListAllowedInboundServiceIdentities lists the downstream service identities that can connect to the given service account
ListAllowedInboundServiceIdentities(service.K8sServiceAccount) ([]service.K8sServiceAccount, error)
// ListInboundServiceIdentities lists the downstream service identities that can connect to the given service account
ListInboundServiceIdentities(service.K8sServiceAccount) ([]service.K8sServiceAccount, error)

// ListAllowedOutboundServiceIdentities lists the upstream service identities the given service account can connect to
ListAllowedOutboundServiceIdentities(service.K8sServiceAccount) ([]service.K8sServiceAccount, error)
// ListOutboundServiceIdentities lists the upstream service identities the given service account can connect to
ListOutboundServiceIdentities(service.K8sServiceAccount) ([]service.K8sServiceAccount, error)

// ListServiceIdentitiesForService lists the service identities associated with the given service
ListServiceIdentitiesForService(service.MeshService) ([]service.K8sServiceAccount, error)
Expand Down
10 changes: 3 additions & 7 deletions pkg/catalog/endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// 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

// from the given downstream identity's perspective
// Note: ServiceIdentity must be in the format "name.namespace" [https://github.com/openservicemesh/osm/issues/3188]
func (mc *MeshCatalog) ListEndpointsForServiceIdentity(downstreamIdentity identity.ServiceIdentity, upstreamSvc service.MeshService) ([]endpoint.Endpoint, error) {
outboundEndpoints, err := mc.listEndpointsForService(upstreamSvc)
Expand All @@ -52,9 +52,8 @@ func (mc *MeshCatalog) ListEndpointsForServiceIdentity(downstreamIdentity identi
ipStr := ep.IP.String()
outboundEndpointsSet[ipStr] = append(outboundEndpointsSet[ipStr], ep)
}
log.Info().Msgf("outbound endpoints: %v", outboundEndpointsSet)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added by mistake initially. It was used for debugging.


destSvcIdentities, err := mc.ListAllowedOutboundServiceIdentities(downstreamIdentity)
destSvcIdentities, err := mc.ListOutboundServiceIdentities(downstreamIdentity)
if err != nil {
log.Error().Err(err).Msgf("Error looking up outbound service accounts for downstream identity %s", downstreamIdentity)
return nil, err
Expand All @@ -64,14 +63,11 @@ func (mc *MeshCatalog) ListEndpointsForServiceIdentity(downstreamIdentity identi
// i.e. only those interseting endpoints are taken into cosideration
var allowedEndpoints []endpoint.Endpoint
for _, destSvcIdentity := range destSvcIdentities {
log.Info().Msgf("ups svc endpoints: %v, %v", destSvcIdentity, mc.listEndpointsForServiceIdentity(destSvcIdentity))

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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Current implementation is correct. Thus it's no longer a TODO

allowedEndpoints = append(allowedEndpoints, outboundEndpointsSet[epIPStr]...)
}
}
Expand Down
84 changes: 42 additions & 42 deletions pkg/catalog/mock_catalog_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions pkg/catalog/outbound_traffic_policies.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ func (mc *MeshCatalog) listOutboundTrafficPoliciesForTrafficSplits(sourceNamespa
return outboundPoliciesFromSplits
}

// ListAllowedOutboundServicesForIdentity list the services the given service account is allowed to initiate outbound connections to
// ListOutboundServicesForIdentity list the services the given service account is allowed to initiate outbound connections to
// Note: ServiceIdentity must be in the format "name.namespace" [https://github.com/openservicemesh/osm/issues/3188]
func (mc *MeshCatalog) ListAllowedOutboundServicesForIdentity(serviceIdentity identity.ServiceIdentity) []service.MeshService {
func (mc *MeshCatalog) ListOutboundServicesForIdentity(serviceIdentity identity.ServiceIdentity) []service.MeshService {
ident := serviceIdentity.ToK8sServiceAccount()
if mc.isOSMGateway(serviceIdentity) {
var services []service.MeshService
Expand Down Expand Up @@ -312,7 +312,7 @@ func (mc *MeshCatalog) GetWeightedClustersForUpstream(upstream service.MeshServi
// ListMeshServicesForIdentity returns a list of services the service with the
// given identity can communicate with, including apex TrafficSplit services.
func (mc *MeshCatalog) ListMeshServicesForIdentity(identity identity.ServiceIdentity) []service.MeshService {
upstreamServices := mc.ListAllowedOutboundServicesForIdentity(identity)
upstreamServices := mc.ListOutboundServicesForIdentity(identity)
if len(upstreamServices) == 0 {
log.Debug().Msgf("Proxy with identity %s does not have any allowed upstream services", identity)
return nil
Expand Down
4 changes: 2 additions & 2 deletions pkg/catalog/outbound_traffic_policies_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ func TestListOutboundTrafficPoliciesForTrafficSplits(t *testing.T) {
}
}

func TestListAllowedOutboundServicesForIdentity(t *testing.T) {
func TestListOutboundServicesForIdentity(t *testing.T) {
assert := tassert.New(t)

testCases := []struct {
Expand Down Expand Up @@ -796,7 +796,7 @@ func TestListAllowedOutboundServicesForIdentity(t *testing.T) {
mc := newFakeMeshCatalogForRoutes(t, testParams{
permissiveMode: tc.permissiveMode,
})
actualList := mc.ListAllowedOutboundServicesForIdentity(tc.svcIdentity)
actualList := mc.ListOutboundServicesForIdentity(tc.svcIdentity)
assert.ElementsMatch(actualList, tc.expectedList)
})
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/catalog/traffictarget.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ const (
httpRouteGroupKind = "HTTPRouteGroup"
)

// ListAllowedInboundServiceIdentities lists the downstream service identities that can connect to the given upstream service account
// ListInboundServiceIdentities lists the downstream service identities that are allowed to connect to the given service identity
// Note: ServiceIdentity must be in the format "name.namespace" [https://github.com/openservicemesh/osm/issues/3188]
func (mc *MeshCatalog) ListAllowedInboundServiceIdentities(upstream identity.ServiceIdentity) ([]identity.ServiceIdentity, error) {
func (mc *MeshCatalog) ListInboundServiceIdentities(upstream identity.ServiceIdentity) ([]identity.ServiceIdentity, error) {
return mc.getAllowedDirectionalServiceAccounts(upstream, inbound)
}

// ListAllowedOutboundServiceIdentities lists the upstream service identities the given downstream service account can connect to
// ListOutboundServiceIdentities lists the upstream service identities the given service identity are allowed to connect to
// Note: ServiceIdentity must be in the format "name.namespace" [https://github.com/openservicemesh/osm/issues/3188]
func (mc *MeshCatalog) ListAllowedOutboundServiceIdentities(downstream identity.ServiceIdentity) ([]identity.ServiceIdentity, error) {
func (mc *MeshCatalog) ListOutboundServiceIdentities(downstream identity.ServiceIdentity) ([]identity.ServiceIdentity, error) {
return mc.getAllowedDirectionalServiceAccounts(downstream, outbound)
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/catalog/traffictarget_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"github.com/openservicemesh/osm/pkg/trafficpolicy"
)

func TestListAllowedInboundServiceIdentities(t *testing.T) {
func TestListInboundServiceIdentities(t *testing.T) {
assert := tassert.New(t)
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
Expand Down Expand Up @@ -189,14 +189,14 @@ func TestListAllowedInboundServiceIdentities(t *testing.T) {
// Mock TrafficTargets returned by MeshSpec, should return all TrafficTargets relevant for this test
mockMeshSpec.EXPECT().ListTrafficTargets().Return(tc.trafficTargets).Times(1)

actual, err := meshCatalog.ListAllowedInboundServiceIdentities(tc.serviceIdentity)
actual, err := meshCatalog.ListInboundServiceIdentities(tc.serviceIdentity)
assert.Equal(err != nil, tc.expectError)
assert.ElementsMatch(actual, tc.expectedInboundServiceIdentities)
})
}
}

func TestListAllowedOutboundServiceIdentities(t *testing.T) {
func TestListOutboundServiceIdentities(t *testing.T) {
assert := tassert.New(t)
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
Expand Down Expand Up @@ -372,7 +372,7 @@ func TestListAllowedOutboundServiceIdentities(t *testing.T) {
// Mock TrafficTargets returned by MeshSpec, should return all TrafficTargets relevant for this test
mockMeshSpec.EXPECT().ListTrafficTargets().Return(tc.trafficTargets).Times(1)

actual, err := meshCatalog.ListAllowedOutboundServiceIdentities(tc.serviceIdentity)
actual, err := meshCatalog.ListOutboundServiceIdentities(tc.serviceIdentity)
assert.Equal(err != nil, tc.expectError)
assert.ElementsMatch(actual, tc.expectedOutboundServiceIdentities)
})
Expand Down
12 changes: 6 additions & 6 deletions pkg/catalog/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ type MeshCataloger interface {
// ListOutboundTrafficPolicies returns all outbound traffic policies related to the given service identity
ListOutboundTrafficPolicies(identity.ServiceIdentity) []*trafficpolicy.OutboundTrafficPolicy

// ListAllowedOutboundServicesForIdentity list the services the given service identity is allowed to initiate outbound connections to
ListAllowedOutboundServicesForIdentity(identity.ServiceIdentity) []service.MeshService
// ListOutboundServicesForIdentity list the services the given service identity is allowed to initiate outbound connections to
ListOutboundServicesForIdentity(identity.ServiceIdentity) []service.MeshService

// ListAllowedInboundServiceIdentities lists the downstream service identities that can connect to the given service identity
ListAllowedInboundServiceIdentities(identity.ServiceIdentity) ([]identity.ServiceIdentity, error)
// ListInboundServiceIdentities lists the downstream service identities that are allowed to connect to the given service identity
ListInboundServiceIdentities(identity.ServiceIdentity) ([]identity.ServiceIdentity, error)

// ListAllowedOutboundServiceIdentities lists the upstream service identities the given service identity can connect to
ListAllowedOutboundServiceIdentities(identity.ServiceIdentity) ([]identity.ServiceIdentity, error)
// ListOutboundServiceIdentities lists the upstream service identities the given service identity are allowed to connect to
ListOutboundServiceIdentities(identity.ServiceIdentity) ([]identity.ServiceIdentity, error)

// ListServiceIdentitiesForService lists the service identities associated with the given service
ListServiceIdentitiesForService(service.MeshService) ([]identity.ServiceIdentity, error)
Expand Down
2 changes: 1 addition & 1 deletion pkg/envoy/ads/secrets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func makeRequestForAllSecrets(proxy *envoy.Proxy, meshCatalog catalog.MeshCatalo

// Create an SDS validation cert corresponding to each upstream service that this proxy can connect to.
// Each cert is used to validate the certificate presented by the corresponding upstream service.
upstreamServices := meshCatalog.ListAllowedOutboundServicesForIdentity(proxyIdentity)
upstreamServices := meshCatalog.ListOutboundServicesForIdentity(proxyIdentity)
for _, upstream := range upstreamServices {
upstreamRootCertResource := secrets.SDSCert{
Name: upstream.NameWithoutCluster(),
Expand Down
2 changes: 1 addition & 1 deletion pkg/envoy/ads/secrets_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func TestMakeRequestForAllSecrets(t *testing.T) {
t.Run(fmt.Sprintf("Testing test case %d: %s", i, tc.name), func(t *testing.T) {
assert := tassert.New(t)

mockCatalog.EXPECT().ListAllowedOutboundServicesForIdentity(tc.proxyIdentity).Return(tc.allowedOutboundServices).Times(1)
mockCatalog.EXPECT().ListOutboundServicesForIdentity(tc.proxyIdentity).Return(tc.allowedOutboundServices).Times(1)

actual := makeRequestForAllSecrets(testProxy, mockCatalog)

Expand Down
4 changes: 2 additions & 2 deletions pkg/envoy/cds/response.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_d

if proxy.Kind() == envoy.KindGateway {
// Build remote clusters based on allowed outbound services
for _, dstService := range meshCatalog.ListAllowedOutboundServicesForIdentity(proxyIdentity) {
for _, dstService := range meshCatalog.ListOutboundServicesForIdentity(proxyIdentity) {
cluster, err := getUpstreamServiceCluster(proxyIdentity, dstService, cfg)
if err != nil {
log.Error().Err(err).Msgf("Failed to construct service cluster for service %s for proxy with XDS Certificate SerialNumber=%s on Pod with UID=%s",
Expand All @@ -39,7 +39,7 @@ func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_d
}

// Build remote clusters based on allowed outbound services
for _, dstService := range meshCatalog.ListAllowedOutboundServicesForIdentity(proxyIdentity) {
for _, dstService := range meshCatalog.ListOutboundServicesForIdentity(proxyIdentity) {
opts := []clusterOption{withTLS}
if cfg.IsPermissiveTrafficPolicyMode() {
opts = append(opts, permissive)
Expand Down
Loading