diff --git a/pkg/catalog/endpoint.go b/pkg/catalog/endpoint.go index ca1c9efd07..2de428d3c9 100644 --- a/pkg/catalog/endpoint.go +++ b/pkg/catalog/endpoint.go @@ -38,15 +38,24 @@ func (mc *MeshCatalog) GetResolvableServiceEndpoints(svc service.MeshService) ([ return endpoints, nil } -// ListEndpointsForServiceIdentity returns a list of endpoints that belongs to an upstream service accounts -// 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) { +// ListAllowedUpstreamEndpointsForService returns the list of endpoints over which the downstream client identity +// is allowed access the upstream service +func (mc *MeshCatalog) ListAllowedUpstreamEndpointsForService(downstreamIdentity identity.ServiceIdentity, upstreamSvc service.MeshService) ([]endpoint.Endpoint, error) { outboundEndpoints, err := mc.listEndpointsForService(upstreamSvc) if err != nil { log.Error().Err(err).Msgf("Error looking up endpoints for upstream service %s", upstreamSvc) return nil, err } + + if mc.configurator.IsPermissiveTrafficPolicyMode() { + return outboundEndpoints, nil + } + + // In SMI mode, the endpoints for an upstream service must be filtered based on the service account + // associated with the endpoint. Only endpoints associated with authorized service accounts as referenced + // in SMI TrafficTarget resources should be returned. + // + // The following code filters the upstream service's endpoints for this purpose. outboundEndpointsSet := make(map[string][]endpoint.Endpoint) for _, ep := range outboundEndpoints { ipStr := ep.IP.String() @@ -60,7 +69,7 @@ func (mc *MeshCatalog) ListEndpointsForServiceIdentity(downstreamIdentity identi } // allowedEndpoints comprises of only those endpoints from outboundEndpoints that matches the endpoints from listEndpointsForServiceIdentity - // i.e. only those interseting endpoints are taken into cosideration + // i.e. only those intersecting endpoints are taken into cosideration var allowedEndpoints []endpoint.Endpoint for _, destSvcIdentity := range destSvcIdentities { for _, ep := range mc.listEndpointsForServiceIdentity(destSvcIdentity) { diff --git a/pkg/catalog/endpoint_test.go b/pkg/catalog/endpoint_test.go index b2f8560111..d11906ae0b 100644 --- a/pkg/catalog/endpoint_test.go +++ b/pkg/catalog/endpoint_test.go @@ -54,7 +54,7 @@ var _ = Describe("Test catalog functions", func() { }) -func TestListEndpointsForServiceIdentity(t *testing.T) { +func TestListAllowedUpstreamEndpointsForService(t *testing.T) { assert := tassert.New(t) testCases := []struct { @@ -65,6 +65,7 @@ func TestListEndpointsForServiceIdentity(t *testing.T) { services []service.MeshService outboundServices map[identity.ServiceIdentity][]service.MeshService outboundServiceEndpoints map[service.MeshService][]endpoint.Endpoint + permissiveMode bool expectedEndpoints []endpoint.Endpoint }{ { @@ -81,6 +82,7 @@ func TestListEndpointsForServiceIdentity(t *testing.T) { outboundServiceEndpoints: map[service.MeshService][]endpoint.Endpoint{ tests.BookstoreV1Service: {tests.Endpoint}, }, + permissiveMode: false, expectedEndpoints: []endpoint.Endpoint{tests.Endpoint}, }, { @@ -102,6 +104,7 @@ func TestListEndpointsForServiceIdentity(t *testing.T) { Port: endpoint.Port(tests.ServicePort), }}, }, + permissiveMode: false, expectedEndpoints: []endpoint.Endpoint{}, }, { @@ -124,11 +127,40 @@ func TestListEndpointsForServiceIdentity(t *testing.T) { Port: endpoint.Port(tests.ServicePort), }}, }, + permissiveMode: false, expectedEndpoints: []endpoint.Endpoint{{ IP: net.ParseIP("9.9.9.9"), Port: endpoint.Port(tests.ServicePort), }}, }, + { + name: `Permissive mode should return all endpoints for a service without filtering them`, + proxyIdentity: tests.BookbuyerServiceIdentity, + upstreamSvc: tests.BookstoreV2Service, + outboundServiceEndpoints: map[service.MeshService][]endpoint.Endpoint{ + tests.BookstoreV2Service: { + { + IP: net.ParseIP("1.1.1.1"), + Port: 80, + }, + { + IP: net.ParseIP("2.2.2.2"), + Port: 80, + }, + }, + }, + permissiveMode: true, + expectedEndpoints: []endpoint.Endpoint{ + { + IP: net.ParseIP("1.1.1.1"), + Port: 80, + }, + { + IP: net.ParseIP("2.2.2.2"), + Port: 80, + }, + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -147,9 +179,22 @@ func TestListEndpointsForServiceIdentity(t *testing.T) { meshSpec: mockMeshSpec, endpointsProviders: []endpoint.Provider{mockEndpointProvider}, serviceProviders: []service.Provider{mockServiceProvider}, + configurator: mockConfigurator, + } + + mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(tc.permissiveMode).AnyTimes() + + for svc, endpoints := range tc.outboundServiceEndpoints { + mockEndpointProvider.EXPECT().ListEndpointsForService(svc).Return(endpoints).AnyTimes() + } + + if tc.permissiveMode { + actual, err := mc.ListAllowedUpstreamEndpointsForService(tc.proxyIdentity, tc.upstreamSvc) + assert.Nil(err) + assert.ElementsMatch(actual, tc.expectedEndpoints) + return } - mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes() mockMeshSpec.EXPECT().ListTrafficTargets().Return(tc.trafficTargets).AnyTimes() mockEndpointProvider.EXPECT().GetID().Return("fake").AnyTimes() @@ -162,10 +207,6 @@ func TestListEndpointsForServiceIdentity(t *testing.T) { mockServiceProvider.EXPECT().GetServicesForServiceIdentity(sa).Return(services, nil).AnyTimes() } - for svc, endpoints := range tc.outboundServiceEndpoints { - mockEndpointProvider.EXPECT().ListEndpointsForService(svc).Return(endpoints).AnyTimes() - } - var pods []*v1.Pod for serviceIdentity, services := range tc.outboundServices { // TODO(draychev): use ServiceIdentity in the rest of the tests [https://github.com/openservicemesh/osm/issues/2218] @@ -197,7 +238,7 @@ func TestListEndpointsForServiceIdentity(t *testing.T) { } } - actual, err := mc.ListEndpointsForServiceIdentity(tc.proxyIdentity, tc.upstreamSvc) + actual, err := mc.ListAllowedUpstreamEndpointsForService(tc.proxyIdentity, tc.upstreamSvc) assert.Nil(err) assert.ElementsMatch(actual, tc.expectedEndpoints) }) diff --git a/pkg/catalog/mock_catalog_generated.go b/pkg/catalog/mock_catalog_generated.go index e4594e034f..d955933020 100644 --- a/pkg/catalog/mock_catalog_generated.go +++ b/pkg/catalog/mock_catalog_generated.go @@ -156,19 +156,19 @@ func (mr *MockMeshCatalogerMockRecorder) GetWeightedClustersForUpstream(arg0 int return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWeightedClustersForUpstream", reflect.TypeOf((*MockMeshCataloger)(nil).GetWeightedClustersForUpstream), arg0) } -// ListEndpointsForServiceIdentity mocks base method -func (m *MockMeshCataloger) ListEndpointsForServiceIdentity(arg0 identity.ServiceIdentity, arg1 service.MeshService) ([]endpoint.Endpoint, error) { +// ListAllowedUpstreamEndpointsForService mocks base method +func (m *MockMeshCataloger) ListAllowedUpstreamEndpointsForService(arg0 identity.ServiceIdentity, arg1 service.MeshService) ([]endpoint.Endpoint, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListEndpointsForServiceIdentity", arg0, arg1) + ret := m.ctrl.Call(m, "ListAllowedUpstreamEndpointsForService", arg0, arg1) ret0, _ := ret[0].([]endpoint.Endpoint) ret1, _ := ret[1].(error) return ret0, ret1 } -// ListEndpointsForServiceIdentity indicates an expected call of ListEndpointsForServiceIdentity -func (mr *MockMeshCatalogerMockRecorder) ListEndpointsForServiceIdentity(arg0, arg1 interface{}) *gomock.Call { +// ListAllowedUpstreamEndpointsForService indicates an expected call of ListAllowedUpstreamEndpointsForService +func (mr *MockMeshCatalogerMockRecorder) ListAllowedUpstreamEndpointsForService(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListEndpointsForServiceIdentity", reflect.TypeOf((*MockMeshCataloger)(nil).ListEndpointsForServiceIdentity), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListAllowedUpstreamEndpointsForService", reflect.TypeOf((*MockMeshCataloger)(nil).ListAllowedUpstreamEndpointsForService), arg0, arg1) } // ListInboundServiceIdentities mocks base method diff --git a/pkg/catalog/types.go b/pkg/catalog/types.go index cd9991e51c..b242bde3d7 100644 --- a/pkg/catalog/types.go +++ b/pkg/catalog/types.go @@ -64,8 +64,9 @@ type MeshCataloger interface { // ListServiceIdentitiesForService lists the service identities associated with the given service ListServiceIdentitiesForService(service.MeshService) ([]identity.ServiceIdentity, error) - // ListEndpointsForServiceIdentity returns the list of endpoints backing a service and its allowed service identities - ListEndpointsForServiceIdentity(identity.ServiceIdentity, service.MeshService) ([]endpoint.Endpoint, error) + // ListAllowedUpstreamEndpointsForService returns the list of endpoints over which the downstream client identity + // is allowed access the upstream service + ListAllowedUpstreamEndpointsForService(identity.ServiceIdentity, service.MeshService) ([]endpoint.Endpoint, error) // GetResolvableServiceEndpoints returns the resolvable set of endpoint over which a service is accessible using its FQDN. // These are the endpoint destinations we'd expect client applications sends the traffic towards to, when attempting to diff --git a/pkg/envoy/eds/response.go b/pkg/envoy/eds/response.go index 66a18ea481..d32553c8ee 100644 --- a/pkg/envoy/eds/response.go +++ b/pkg/envoy/eds/response.go @@ -51,7 +51,7 @@ func fulfillEDSRequest(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, re log.Error().Err(err).Msgf("Error retrieving MeshService from Cluster %s", cluster) continue } - endpoints, err := meshCatalog.ListEndpointsForServiceIdentity(proxyIdentity, meshSvc) + endpoints, err := meshCatalog.ListAllowedUpstreamEndpointsForService(proxyIdentity, meshSvc) if err != nil { log.Error().Err(err).Msgf("Failed listing allowed endpoints for service %s, for proxy identity %s", meshSvc, proxyIdentity) continue @@ -103,7 +103,7 @@ func getEndpointsForProxy(meshCatalog catalog.MeshCataloger, proxyIdentity ident allowedServicesEndpoints := make(map[service.MeshService][]endpoint.Endpoint) for _, dstSvc := range meshCatalog.ListOutboundServicesForIdentity(proxyIdentity) { - endpoints, err := meshCatalog.ListEndpointsForServiceIdentity(proxyIdentity, dstSvc) + endpoints, err := meshCatalog.ListAllowedUpstreamEndpointsForService(proxyIdentity, dstSvc) if err != nil { log.Error().Err(err).Msgf("Failed listing allowed endpoints for service %s for proxy identity %s", dstSvc, proxyIdentity) continue