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

[HOLD] chore: convert ServiceIdentity to struct #3732

Closed
wants to merge 1 commit into from
Closed

[HOLD] chore: convert ServiceIdentity to struct #3732

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
6 changes: 5 additions & 1 deletion pkg/catalog/egress_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,11 @@ func TestGetEgressTrafficPolicy(t *testing.T) {
},
}

testSourceIdentity := identity.ServiceIdentity("foo.bar.cluster.local")
testSourceIdentity := identity.ServiceIdentity{
ServiceAccount: "foo",
Namespace: "bar",
ClusterDomain: "cluster.local",
}

for i, tc := range testCases {
t.Run(fmt.Sprintf("Running test case %d: %s", i, tc.name), func(t *testing.T) {
Expand Down
22 changes: 17 additions & 5 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 @@ -784,8 +784,12 @@ func TestListAllowedOutboundServicesForIdentity(t *testing.T) {
permissiveMode: true,
},
{
name: "gateway",
svcIdentity: "gateway.osm-system.cluster.local",
name: "gateway",
svcIdentity: identity.ServiceIdentity{
ServiceAccount: "gateway",
Namespace: "osm-system",
ClusterDomain: "cluster.local",
},
expectedList: []service.MeshService{tests.BookstoreV1Service, tests.BookstoreV2Service, tests.BookstoreApexService, tests.BookbuyerService},
permissiveMode: true,
},
Expand Down Expand Up @@ -1450,13 +1454,21 @@ func TestListMeshServicesForIdentity(t *testing.T) {
expected []service.MeshService
}{
{
name: "no allowed outbound services",
id: "foo.bar",
name: "no allowed outbound services",
id: identity.ServiceIdentity{
ServiceAccount: "foo",
Namespace: "bar",
ClusterDomain: "",
},
expected: nil,
},
{
name: "some allowed service",
id: "my-src-ns.my-src-name",
id: identity.ServiceIdentity{
ServiceAccount: "my-src-ns",
Namespace: "my-src-name",
ClusterDomain: "",
},
services: []*corev1.Service{
{
ObjectMeta: v1.ObjectMeta{
Expand Down
70 changes: 55 additions & 15 deletions pkg/catalog/traffictarget_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,10 +516,18 @@ func TestListInboundTrafficTargetsWithRoutes(t *testing.T) {

expectedTrafficTargets: []trafficpolicy.TrafficTargetWithRoutes{
{
Name: "ns-1/test-1",
Destination: identity.ServiceIdentity("sa-1.ns-1.cluster.local"),
Name: "ns-1/test-1",
Destination: identity.ServiceIdentity{
ServiceAccount: "sa-1",
Namespace: "ns-1",
ClusterDomain: "cluster.local",
},
Sources: []identity.ServiceIdentity{
identity.ServiceIdentity("sa-2.ns-2.cluster.local"),
{
ServiceAccount: "sa-2",
Namespace: "ns-2",
ClusterDomain: "cluster.local",
},
},
TCPRouteMatches: []trafficpolicy.TCPRouteMatch{
{
Expand Down Expand Up @@ -603,10 +611,18 @@ func TestListInboundTrafficTargetsWithRoutes(t *testing.T) {

expectedTrafficTargets: []trafficpolicy.TrafficTargetWithRoutes{
{
Name: "ns-1/test-1",
Destination: identity.ServiceIdentity("sa-1.ns-1.cluster.local"),
Name: "ns-1/test-1",
Destination: identity.ServiceIdentity{
ServiceAccount: "sa-1",
Namespace: "ns-1",
ClusterDomain: "cluster.local",
},
Sources: []identity.ServiceIdentity{
identity.ServiceIdentity("sa-2.ns-2.cluster.local"),
{
ServiceAccount: "sa-2",
Namespace: "ns-2",
ClusterDomain: "cluster.local",
},
},
TCPRouteMatches: []trafficpolicy.TCPRouteMatch{
{
Expand Down Expand Up @@ -751,10 +767,18 @@ func TestListInboundTrafficTargetsWithRoutes(t *testing.T) {

expectedTrafficTargets: []trafficpolicy.TrafficTargetWithRoutes{
{
Name: "ns-1/test-1",
Destination: identity.ServiceIdentity("sa-1.ns-1.cluster.local"),
Name: "ns-1/test-1",
Destination: identity.ServiceIdentity{
ServiceAccount: "sa-1",
Namespace: "ns-1",
ClusterDomain: "cluster.local",
},
Sources: []identity.ServiceIdentity{
identity.ServiceIdentity("sa-2.ns-2.cluster.local"),
{
ServiceAccount: "sa-2",
Namespace: "ns-2",
ClusterDomain: "cluster.local",
},
},
TCPRouteMatches: []trafficpolicy.TCPRouteMatch{
{
Expand All @@ -768,10 +792,18 @@ func TestListInboundTrafficTargetsWithRoutes(t *testing.T) {
},
},
{
Name: "ns-1/test-2",
Destination: identity.ServiceIdentity("sa-1.ns-1.cluster.local"),
Name: "ns-1/test-2",
Destination: identity.ServiceIdentity{
ServiceAccount: "sa-1",
Namespace: "ns-1",
ClusterDomain: "cluster.local",
},
Sources: []identity.ServiceIdentity{
identity.ServiceIdentity("sa-3.ns-3.cluster.local"),
{
ServiceAccount: "sa-3",
Namespace: "ns-3",
ClusterDomain: "cluster.local",
},
},
TCPRouteMatches: []trafficpolicy.TCPRouteMatch{
{
Expand Down Expand Up @@ -838,10 +870,18 @@ func TestListInboundTrafficTargetsWithRoutes(t *testing.T) {

expectedTrafficTargets: []trafficpolicy.TrafficTargetWithRoutes{
{
Name: "ns-1/test-1",
Destination: identity.ServiceIdentity("sa-1.ns-1.cluster.local"),
Name: "ns-1/test-1",
Destination: identity.ServiceIdentity{
ServiceAccount: "sa-1",
Namespace: "ns-1",
ClusterDomain: "cluster.local",
},
Sources: []identity.ServiceIdentity{
identity.ServiceIdentity("sa-2.ns-2.cluster.local"),
{
ServiceAccount: "sa-2",
Namespace: "ns-2",
ClusterDomain: "cluster.local",
},
},
TCPRouteMatches: []trafficpolicy.TCPRouteMatch{
{
Expand Down
40 changes: 32 additions & 8 deletions pkg/envoy/lds/inmesh_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,11 +262,23 @@ func TestGetInboundMeshHTTPFilterChain(t *testing.T) {

trafficTargets := []trafficpolicy.TrafficTargetWithRoutes{
{
Name: "ns-1/test-1",
Destination: identity.ServiceIdentity("sa-1.ns-1.cluster.local"),
Name: "ns-1/test-1",
Destination: identity.ServiceIdentity{
ServiceAccount: "sa-1",
Namespace: "ns-1",
ClusterDomain: "cluster.local",
},
Sources: []identity.ServiceIdentity{
identity.ServiceIdentity("sa-2.ns-2.cluster.local"),
identity.ServiceIdentity("sa-3.ns-3.cluster.local"),
{
ServiceAccount: "sa-2",
Namespace: "ns-2",
ClusterDomain: "cluster.local",
},
{
ServiceAccount: "sa-3",
Namespace: "ns-3",
ClusterDomain: "cluster.local",
},
},
TCPRouteMatches: nil,
},
Expand Down Expand Up @@ -360,11 +372,23 @@ func TestGetInboundMeshTCPFilterChain(t *testing.T) {

trafficTargets := []trafficpolicy.TrafficTargetWithRoutes{
{
Name: "ns-1/test-1",
Destination: identity.ServiceIdentity("sa-1.ns-1.cluster.local"),
Name: "ns-1/test-1",
Destination: identity.ServiceIdentity{
ServiceAccount: "sa-1",
Namespace: "ns-1",
ClusterDomain: "cluster.local",
},
Sources: []identity.ServiceIdentity{
identity.ServiceIdentity("sa-2.ns-2.cluster.local"),
identity.ServiceIdentity("sa-3.ns-3.cluster.local"),
{
ServiceAccount: "sa-2",
Namespace: "ns-2",
ClusterDomain: "cluster.local",
},
{
ServiceAccount: "sa-3",
Namespace: "ns-3",
ClusterDomain: "cluster.local",
},
},
TCPRouteMatches: nil,
},
Expand Down
9 changes: 7 additions & 2 deletions pkg/envoy/lds/rbac.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,13 @@ func (lb *listenerBuilder) buildRBACFilter() (*xds_listener.Filter, error) {

// buildInboundRBACPolicies builds the RBAC policies based on allowed principals
func (lb *listenerBuilder) buildInboundRBACPolicies() (*xds_network_rbac.RBAC, error) {
proxyIdentity := identity.ServiceIdentity(lb.serviceIdentity.String())
trafficTargets, err := lb.meshCatalog.ListInboundTrafficTargetsWithRoutes(lb.serviceIdentity)
serviceIdentity := lb.serviceIdentity
proxyIdentity := identity.ServiceIdentity{
ServiceAccount: serviceIdentity.ServiceAccount,
Namespace: serviceIdentity.Namespace,
ClusterDomain: serviceIdentity.ClusterDomain,
}
trafficTargets, err := lb.meshCatalog.ListInboundTrafficTargetsWithRoutes(serviceIdentity)
Comment on lines +42 to +48
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
serviceIdentity := lb.serviceIdentity
proxyIdentity := identity.ServiceIdentity{
ServiceAccount: serviceIdentity.ServiceAccount,
Namespace: serviceIdentity.Namespace,
ClusterDomain: serviceIdentity.ClusterDomain,
}
trafficTargets, err := lb.meshCatalog.ListInboundTrafficTargetsWithRoutes(serviceIdentity)
trafficTargets, err := lb.meshCatalog.ListInboundTrafficTargetsWithRoutes(lb.serviceIdentity)

if err != nil {
log.Error().Err(err).Msgf("Error listing allowed inbound traffic targets for proxy identity %s", proxyIdentity)
Copy link
Member

Choose a reason for hiding this comment

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

s/proxyIdentity/lb.serviceIdentity

return nil, err
Expand Down
Loading