diff --git a/pkg/catalog/egress_test.go b/pkg/catalog/egress_test.go index f02defec7a..762f9f09e8 100644 --- a/pkg/catalog/egress_test.go +++ b/pkg/catalog/egress_test.go @@ -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) { diff --git a/pkg/catalog/outbound_traffic_policies_test.go b/pkg/catalog/outbound_traffic_policies_test.go index 9ec6e9f08a..de37903156 100644 --- a/pkg/catalog/outbound_traffic_policies_test.go +++ b/pkg/catalog/outbound_traffic_policies_test.go @@ -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, }, @@ -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{ diff --git a/pkg/catalog/traffictarget_test.go b/pkg/catalog/traffictarget_test.go index 7c6b360b3a..9f5d9df2db 100644 --- a/pkg/catalog/traffictarget_test.go +++ b/pkg/catalog/traffictarget_test.go @@ -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{ { @@ -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{ { @@ -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{ { @@ -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{ { @@ -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{ { diff --git a/pkg/envoy/lds/inmesh_test.go b/pkg/envoy/lds/inmesh_test.go index 32107f6afd..9aa33b3abd 100644 --- a/pkg/envoy/lds/inmesh_test.go +++ b/pkg/envoy/lds/inmesh_test.go @@ -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, }, @@ -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, }, diff --git a/pkg/envoy/lds/rbac.go b/pkg/envoy/lds/rbac.go index 86a079dbda..03386fad82 100644 --- a/pkg/envoy/lds/rbac.go +++ b/pkg/envoy/lds/rbac.go @@ -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) if err != nil { log.Error().Err(err).Msgf("Error listing allowed inbound traffic targets for proxy identity %s", proxyIdentity) return nil, err diff --git a/pkg/envoy/lds/rbac_test.go b/pkg/envoy/lds/rbac_test.go index 46aa7a48d8..0af7f7635b 100644 --- a/pkg/envoy/lds/rbac_test.go +++ b/pkg/envoy/lds/rbac_test.go @@ -31,11 +31,23 @@ func TestBuildRBACPolicyFromTrafficTarget(t *testing.T) { // Test 1 name: "traffic target without TCP routes", trafficTarget: 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, }, @@ -74,11 +86,23 @@ func TestBuildRBACPolicyFromTrafficTarget(t *testing.T) { // Test 2 name: "traffic target with TCP routes", trafficTarget: 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: []trafficpolicy.TCPRouteMatch{ { @@ -173,11 +197,23 @@ func TestBuildInboundRBACPolicies(t *testing.T) { name: "traffic target without TCP routes", 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, }, @@ -193,18 +229,38 @@ func TestBuildInboundRBACPolicies(t *testing.T) { name: "traffic target with TCP routes", 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", + }, }, }, { - 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-4.ns-2.cluster.local"), + { + ServiceAccount: "sa-4", + Namespace: "ns-2", + ClusterDomain: "cluster.local", + }, }, }, }, @@ -259,11 +315,23 @@ func TestBuildRBACFilter(t *testing.T) { name: "traffic target without TCP routes", 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, }, @@ -277,18 +345,38 @@ func TestBuildRBACFilter(t *testing.T) { name: "traffic target with TCP routes", 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", + }, }, }, { - 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-4.ns-2.cluster.local"), + { + ServiceAccount: "sa-4", + Namespace: "ns-2", + ClusterDomain: "cluster.local", + }, }, }, }, diff --git a/pkg/envoy/proxy_test.go b/pkg/envoy/proxy_test.go index 78f22b47c1..cb1a89e970 100644 --- a/pkg/envoy/proxy_test.go +++ b/pkg/envoy/proxy_test.go @@ -394,9 +394,13 @@ var _ = Describe("Test XDS certificate tooling", func() { Expect(err).ToNot(HaveOccurred()) expected := &certificateCommonNameMeta{ - ProxyUUID: proxyUUID, - ProxyKind: KindSidecar, - ServiceIdentity: identity.ServiceIdentity(fmt.Sprintf("%s.%s.%s", serviceAccount, testNamespace, identity.ClusterLocalTrustDomain)), + ProxyUUID: proxyUUID, + ProxyKind: KindSidecar, + ServiceIdentity: identity.ServiceIdentity{ + ServiceAccount: serviceAccount, + Namespace: testNamespace, + ClusterDomain: identity.ClusterLocalTrustDomain, + }, } Expect(cnMeta).To(Equal(expected)) }) @@ -418,9 +422,13 @@ var _ = Describe("Test XDS certificate tooling", func() { actualMeta, err := getCertificateCommonNameMeta(cn) expectedMeta := certificateCommonNameMeta{ - ProxyUUID: proxyUUID, - ProxyKind: KindSidecar, - ServiceIdentity: identity.ServiceIdentity(fmt.Sprintf("%s.%s.%s", serviceAccount, namespace, identity.ClusterLocalTrustDomain)), + ProxyUUID: proxyUUID, + ProxyKind: KindSidecar, + ServiceIdentity: identity.ServiceIdentity{ + ServiceAccount: serviceAccount, + Namespace: namespace, + ClusterDomain: identity.ClusterLocalTrustDomain, + }, } Expect(err).ToNot(HaveOccurred()) Expect(actualMeta).To(Equal(&expectedMeta)) @@ -432,13 +440,16 @@ var _ = Describe("Test XDS certificate tooling", func() { cn := certificate.CommonName(fmt.Sprintf("%s.sidecar.sa-name.sa-namespace.cluster.local", uuid.New())) proxyIdentity, err := GetServiceIdentityFromProxyCertificate(cn) Expect(err).ToNot(HaveOccurred()) - Expect(proxyIdentity).To(Equal(identity.ServiceIdentity("sa-name.sa-namespace.cluster.local"))) + Expect(proxyIdentity).To(Equal(identity.ServiceIdentity{ + ServiceAccount: "sa-name", + Namespace: "sa-namespace", + ClusterDomain: "cluster.local"})) }) It("should correctly error when the XDS certificate CN is invalid", func() { proxyIdentity, err := GetServiceIdentityFromProxyCertificate(certificate.CommonName("invalid")) Expect(err).To(HaveOccurred()) - Expect(proxyIdentity).To(Equal(identity.ServiceIdentity(""))) + Expect(proxyIdentity).To(Equal(identity.ServiceIdentity{})) }) }) }) diff --git a/pkg/envoy/sds/response.go b/pkg/envoy/sds/response.go index 1b5870fad7..dda6c3d7b4 100644 --- a/pkg/envoy/sds/response.go +++ b/pkg/envoy/sds/response.go @@ -42,7 +42,7 @@ func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, request log.Info().Msgf("Creating SDS response for request for resources %v for proxy %s", requestedCerts, proxy.String()) // 1. Issue a service certificate for this proxy - cert, err := certManager.IssueCertificate(certificate.CommonName(s.serviceIdentity), cfg.GetServiceCertValidityPeriod()) + cert, err := certManager.IssueCertificate(certificate.CommonName(s.serviceIdentity.String()), cfg.GetServiceCertValidityPeriod()) if err != nil { log.Error().Err(err).Msgf("Error issuing a certificate for proxy %s", proxy.String()) return nil, err diff --git a/pkg/envoy/secrets/secrets_test.go b/pkg/envoy/secrets/secrets_test.go index 986b92e1d8..1aa518bce3 100644 --- a/pkg/envoy/secrets/secrets_test.go +++ b/pkg/envoy/secrets/secrets_test.go @@ -260,11 +260,19 @@ func TestGetSecretNameForIdentity(t *testing.T) { expected string }{ { - si: identity.ServiceIdentity("foo.bar.cluster.local"), + si: identity.ServiceIdentity{ + ServiceAccount: "foo", + Namespace: "bar", + ClusterDomain: "cluster.local", + }, expected: "bar/foo", }, { - si: identity.ServiceIdentity("foo.baz.cluster.local"), + si: identity.ServiceIdentity{ + ServiceAccount: "foo", + Namespace: "baz", + ClusterDomain: "cluster.local", + }, expected: "baz/foo", }, } diff --git a/pkg/envoy/xdsutil.go b/pkg/envoy/xdsutil.go index b4590a9c2c..6f116da9be 100644 --- a/pkg/envoy/xdsutil.go +++ b/pkg/envoy/xdsutil.go @@ -324,7 +324,7 @@ func getCertificateCommonNameMeta(cn certificate.CommonName) (*certificateCommon return &certificateCommonNameMeta{ ProxyUUID: proxyUUID, ProxyKind: ProxyKind(chunks[1]), - ServiceIdentity: identity.ServiceIdentity(chunks[2]), + ServiceIdentity: identity.NewServiceIdentityFromString(chunks[2]), }, nil } diff --git a/pkg/identity/kubernetes.go b/pkg/identity/kubernetes.go index 390131cf8d..1e004f2e7c 100644 --- a/pkg/identity/kubernetes.go +++ b/pkg/identity/kubernetes.go @@ -1,9 +1,5 @@ package identity -import ( - "strings" -) - const ( // ClusterLocalTrustDomain is the trust domain for the local kubernetes cluster ClusterLocalTrustDomain = "cluster.local" @@ -13,6 +9,5 @@ const ( // GetKubernetesServiceIdentity returns the ServiceIdentity based on Kubernetes ServiceAccount and a trust domain func GetKubernetesServiceIdentity(svcAccount K8sServiceAccount, trustDomain string) ServiceIdentity { - si := strings.Join([]string{svcAccount.Name, svcAccount.Namespace, trustDomain}, identityDelimiter) - return ServiceIdentity(si) + return ServiceIdentity{svcAccount.Name, svcAccount.Namespace, trustDomain} } diff --git a/pkg/identity/kubernetes_test.go b/pkg/identity/kubernetes_test.go index e4048f5739..3e951fcabf 100644 --- a/pkg/identity/kubernetes_test.go +++ b/pkg/identity/kubernetes_test.go @@ -18,12 +18,12 @@ func TestGetKubernetesServiceIdentity(t *testing.T) { { K8sServiceAccount{Name: "foo", Namespace: "bar"}, "cluster.local", - ServiceIdentity("foo.bar.cluster.local"), + ServiceIdentity{"foo", "bar", "cluster.local"}, }, { K8sServiceAccount{Name: "foo", Namespace: "bar"}, "cluster.baz", - ServiceIdentity("foo.bar.cluster.baz"), + ServiceIdentity{"foo", "bar", "cluster.baz"}, }, } @@ -34,5 +34,75 @@ func TestGetKubernetesServiceIdentity(t *testing.T) { }) } - assert.Equal(ServiceIdentity("foo").String(), "foo") + assert.Equal(ServiceIdentity{"foo", "", ""}.String(), "foo") +} + +func TestServiceIdentityToString(t *testing.T) { + assert := tassert.New(t) + + testCases := []struct { + identity ServiceIdentity + expectedString string + }{ + { + ServiceIdentity{ + ServiceAccount: "sa", + }, + "sa", + }, + { + ServiceIdentity{ + ServiceAccount: "sa", + Namespace: "ns", + }, + "sa.ns", + }, + { + ServiceIdentity{ + ServiceAccount: "sa", + Namespace: "ns", + ClusterDomain: "a.n.y", + }, + "sa.ns.a.n.y", + }, + } + + for _, tc := range testCases { + assert.Equal(tc.identity.String(), tc.expectedString) + } +} + +func TestStringToServiceIdentity(t *testing.T) { + assert := tassert.New(t) + + testCases := []struct { + identityString string + expectedIdentity ServiceIdentity + }{ + { + "sa", + ServiceIdentity{ + ServiceAccount: "sa", + }, + }, + { + "sa.ns", + ServiceIdentity{ + ServiceAccount: "sa", + Namespace: "ns", + }, + }, + { + "sa.ns.a.n.y", + ServiceIdentity{ + ServiceAccount: "sa", + Namespace: "ns", + ClusterDomain: "a.n.y", + }, + }, + } + + for _, tc := range testCases { + assert.Equal(NewServiceIdentityFromString(tc.identityString), tc.expectedIdentity) + } } diff --git a/pkg/identity/types.go b/pkg/identity/types.go index 19fa59ca99..66df1d2d22 100644 --- a/pkg/identity/types.go +++ b/pkg/identity/types.go @@ -13,11 +13,45 @@ const ( // ServiceIdentity is the type used to represent the identity for a service // For Kubernetes services this string will be in the format: ..cluster.local -type ServiceIdentity string +type ServiceIdentity struct { + ServiceAccount string + Namespace string + ClusterDomain string +} + +// NewServiceIdentityFromString creates a ServiceIdentity from a given string. +// The string will be splitted using "." into at most 3 chunks. The first two chunks do not contain ".". They will be the ServiceAccount and Namespace of the identity respectively. The chunk after the second "." will be the ClusterDomain. If any required chunk is not found, the corresponding field of the identity will be an empty string. +func NewServiceIdentityFromString(identityStr string) ServiceIdentity { + id := ServiceIdentity{} + chunks := strings.SplitN(identityStr, identityDelimiter, 3) + + if len(chunks) > 0 { + id.ServiceAccount = chunks[0] + } + if len(chunks) > 1 { + id.Namespace = chunks[1] + } + if len(chunks) > 2 { + id.ClusterDomain = chunks[2] + } + + return id +} // String returns the ServiceIdentity as a string func (si ServiceIdentity) String() string { - return string(si) + result := si.ServiceAccount + if si.Namespace == "" { + return result + } + result += fmt.Sprintf(".%s", si.Namespace) + + if si.ClusterDomain == "" { + return result + } + result += fmt.Sprintf(".%s", si.ClusterDomain) + + return result } // ToK8sServiceAccount converts a ServiceIdentity to a K8sServiceAccount to help with transition from K8sServiceAccount to ServiceIdentity @@ -52,5 +86,5 @@ func (sa K8sServiceAccount) IsEmpty() bool { // ToServiceIdentity converts K8sServiceAccount to the newer ServiceIdentity // TODO(draychev): ToServiceIdentity is used in many places to ease with transition from K8sServiceAccount to ServiceIdentity and should be removed (not everywhere) - [https://github.com/openservicemesh/osm/issues/2218] func (sa K8sServiceAccount) ToServiceIdentity() ServiceIdentity { - return ServiceIdentity(fmt.Sprintf("%s.%s.%s", sa.Name, sa.Namespace, ClusterLocalTrustDomain)) + return ServiceIdentity{sa.Name, sa.Namespace, ClusterLocalTrustDomain} } diff --git a/pkg/identity/types_test.go b/pkg/identity/types_test.go index 1f842acb94..b1a8c17c91 100644 --- a/pkg/identity/types_test.go +++ b/pkg/identity/types_test.go @@ -34,12 +34,12 @@ var _ = Describe("Test pkg/service functions", func() { Namespace: "ns", Name: "name", }.ToServiceIdentity() - expected := ServiceIdentity("name.ns.cluster.local") + expected := ServiceIdentity{"name", "ns", "cluster.local"} Expect(actual).To(Equal(expected)) }) It("implements ServiceIdentity{}.ToK8sServiceAccount() correctly", func() { - actual := ServiceIdentity("name.ns.cluster.local").ToK8sServiceAccount() + actual := ServiceIdentity{"name", "ns", "cluster.local"}.ToK8sServiceAccount() expected := K8sServiceAccount{ Namespace: "ns", Name: "name",