From 269e8c7a3e7d97cb931933a7477c62ee23f85b08 Mon Sep 17 00:00:00 2001 From: Icarus Wu Date: Fri, 20 Sep 2024 14:28:13 +0800 Subject: [PATCH 1/4] fix(api-server): skip display-name label for service insight Signed-off-by: Icarus Wu --- pkg/api-server/service_insight_endpoints.go | 45 +++++++++++++++++---- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/pkg/api-server/service_insight_endpoints.go b/pkg/api-server/service_insight_endpoints.go index 06f85192f999..40e627c44f9f 100644 --- a/pkg/api-server/service_insight_endpoints.go +++ b/pkg/api-server/service_insight_endpoints.go @@ -2,6 +2,7 @@ package api_server import ( "fmt" + "maps" "sort" "strconv" "strings" @@ -55,6 +56,10 @@ func (s *serviceInsightEndpoints) findResource(request *restful.Request, respons res := out.(*rest_unversioned.Resource) res.Meta.Name = service res.Spec = stat + + mapperFn := removeDisplayNameLabel() + mapperFn(res) + if err := response.WriteAsJson(res); err != nil { core.Log.Error(err, "Could not write the response") } @@ -99,13 +104,17 @@ func (s *serviceInsightEndpoints) listResources(request *restful.Request, respon } } - items := s.expandInsights(serviceInsightList, nameContains, func(service *v1alpha1.ServiceInsight_Service) bool { - if len(filterMap) == 0 { - return true - } - _, exists := filterMap[service.ServiceType] - return exists - }) + items := s.expandInsights(serviceInsightList, nameContains, + func(service *v1alpha1.ServiceInsight_Service) bool { + if len(filterMap) == 0 { + return true + } + _, exists := filterMap[service.ServiceType] + return exists + }, + removeDisplayNameLabel(), + ) + restList := rest.ResourceList{ Total: uint32(len(items)), Items: items, @@ -135,7 +144,12 @@ func (s *serviceInsightEndpoints) fillStaticInfo(name string, stat *v1alpha1.Ser // 2) Mesh+Name is a key on Universal, but not on Kubernetes, so if there are two services of the same name in different Meshes we would have problems with naming. // From the API perspective it's better to provide ServiceInsight per Service, not per Mesh. // For this reason, this method expand the one ServiceInsight resource for the mesh to resource per service -func (s *serviceInsightEndpoints) expandInsights(serviceInsightList *mesh.ServiceInsightResourceList, nameContains string, filterFn func(service *v1alpha1.ServiceInsight_Service) bool) []rest.Resource { +func (s *serviceInsightEndpoints) expandInsights( + serviceInsightList *mesh.ServiceInsightResourceList, + nameContains string, + filterFn func(service *v1alpha1.ServiceInsight_Service) bool, + mapperFn unversionedResourceMapper, +) []rest.Resource { restItems := []rest.Resource{} // Needs to be set to avoid returning nil and have the api return [] for _, insight := range serviceInsightList.Items { for serviceName, service := range insight.Spec.Services { @@ -145,6 +159,7 @@ func (s *serviceInsightEndpoints) expandInsights(serviceInsightList *mesh.Servic res := out.(*rest_unversioned.Resource) res.Meta.Name = serviceName res.Spec = service + mapperFn(res) restItems = append(restItems, out) } } @@ -196,3 +211,17 @@ func (s *serviceInsightEndpoints) paginateResources(request *restful.Request, re restList.Next = nextLink(request, nextOffset) return nil } + +type unversionedResourceMapper func(resource *rest_unversioned.Resource) + +// Since the value of label "kuma.io/display-name" is same with the ServiceInsight resource name, +// in which it looks weird for the API to each service. Ref: https://github.com/kumahq/kuma/issues/9729 +func removeDisplayNameLabel() unversionedResourceMapper { + return func(resource *rest_unversioned.Resource) { + tmpMeta := resource.GetMeta() + maps.DeleteFunc(tmpMeta.Labels, func(key string, val string) bool { + return key == v1alpha1.DisplayName + }) + resource.Meta = tmpMeta + } +} From 09e07aef2e6d1ce84fdf2aee32a2efe37146b067 Mon Sep 17 00:00:00 2001 From: Icarus Wu Date: Thu, 24 Oct 2024 11:07:09 +0800 Subject: [PATCH 2/4] remove mapperFn to make codes simpler Signed-off-by: Icarus Wu --- pkg/api-server/service_insight_endpoints.go | 47 +++++++-------------- 1 file changed, 16 insertions(+), 31 deletions(-) diff --git a/pkg/api-server/service_insight_endpoints.go b/pkg/api-server/service_insight_endpoints.go index 40e627c44f9f..50627d4afc63 100644 --- a/pkg/api-server/service_insight_endpoints.go +++ b/pkg/api-server/service_insight_endpoints.go @@ -56,10 +56,7 @@ func (s *serviceInsightEndpoints) findResource(request *restful.Request, respons res := out.(*rest_unversioned.Resource) res.Meta.Name = service res.Spec = stat - - mapperFn := removeDisplayNameLabel() - mapperFn(res) - + removeDisplayNameLabel(res) if err := response.WriteAsJson(res); err != nil { core.Log.Error(err, "Could not write the response") } @@ -104,16 +101,13 @@ func (s *serviceInsightEndpoints) listResources(request *restful.Request, respon } } - items := s.expandInsights(serviceInsightList, nameContains, - func(service *v1alpha1.ServiceInsight_Service) bool { - if len(filterMap) == 0 { - return true - } - _, exists := filterMap[service.ServiceType] - return exists - }, - removeDisplayNameLabel(), - ) + items := s.expandInsights(serviceInsightList, nameContains, func(service *v1alpha1.ServiceInsight_Service) bool { + if len(filterMap) == 0 { + return true + } + _, exists := filterMap[service.ServiceType] + return exists + }) restList := rest.ResourceList{ Total: uint32(len(items)), @@ -144,12 +138,7 @@ func (s *serviceInsightEndpoints) fillStaticInfo(name string, stat *v1alpha1.Ser // 2) Mesh+Name is a key on Universal, but not on Kubernetes, so if there are two services of the same name in different Meshes we would have problems with naming. // From the API perspective it's better to provide ServiceInsight per Service, not per Mesh. // For this reason, this method expand the one ServiceInsight resource for the mesh to resource per service -func (s *serviceInsightEndpoints) expandInsights( - serviceInsightList *mesh.ServiceInsightResourceList, - nameContains string, - filterFn func(service *v1alpha1.ServiceInsight_Service) bool, - mapperFn unversionedResourceMapper, -) []rest.Resource { +func (s *serviceInsightEndpoints) expandInsights(serviceInsightList *mesh.ServiceInsightResourceList, nameContains string, filterFn func(service *v1alpha1.ServiceInsight_Service) bool) []rest.Resource { restItems := []rest.Resource{} // Needs to be set to avoid returning nil and have the api return [] for _, insight := range serviceInsightList.Items { for serviceName, service := range insight.Spec.Services { @@ -159,7 +148,7 @@ func (s *serviceInsightEndpoints) expandInsights( res := out.(*rest_unversioned.Resource) res.Meta.Name = serviceName res.Spec = service - mapperFn(res) + removeDisplayNameLabel(res) restItems = append(restItems, out) } } @@ -212,16 +201,12 @@ func (s *serviceInsightEndpoints) paginateResources(request *restful.Request, re return nil } -type unversionedResourceMapper func(resource *rest_unversioned.Resource) - // Since the value of label "kuma.io/display-name" is same with the ServiceInsight resource name, // in which it looks weird for the API to each service. Ref: https://github.com/kumahq/kuma/issues/9729 -func removeDisplayNameLabel() unversionedResourceMapper { - return func(resource *rest_unversioned.Resource) { - tmpMeta := resource.GetMeta() - maps.DeleteFunc(tmpMeta.Labels, func(key string, val string) bool { - return key == v1alpha1.DisplayName - }) - resource.Meta = tmpMeta - } +func removeDisplayNameLabel(resource *rest_unversioned.Resource) { + tmpMeta := resource.GetMeta() + maps.DeleteFunc(tmpMeta.Labels, func(key string, val string) bool { + return key == v1alpha1.DisplayName + }) + resource.Meta = tmpMeta } From cb254a92666873bdda6de8541df92791e3ec13a0 Mon Sep 17 00:00:00 2001 From: Icarus Wu Date: Thu, 24 Oct 2024 22:43:49 +0800 Subject: [PATCH 3/4] supplement unit test Signed-off-by: Icarus Wu --- pkg/api-server/service_insight_endpoints.go | 1 - pkg/api-server/testdata/service-insights/get.input.yaml | 2 ++ pkg/api-server/testdata/service-insights/list-simple.input.yaml | 2 ++ pkg/api-server/testdata/service-insights/pagination.input.yaml | 2 ++ 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/api-server/service_insight_endpoints.go b/pkg/api-server/service_insight_endpoints.go index 50627d4afc63..31910a985f76 100644 --- a/pkg/api-server/service_insight_endpoints.go +++ b/pkg/api-server/service_insight_endpoints.go @@ -108,7 +108,6 @@ func (s *serviceInsightEndpoints) listResources(request *restful.Request, respon _, exists := filterMap[service.ServiceType] return exists }) - restList := rest.ResourceList{ Total: uint32(len(items)), Items: items, diff --git a/pkg/api-server/testdata/service-insights/get.input.yaml b/pkg/api-server/testdata/service-insights/get.input.yaml index a0af3a335df6..ecdcba9e2d13 100644 --- a/pkg/api-server/testdata/service-insights/get.input.yaml +++ b/pkg/api-server/testdata/service-insights/get.input.yaml @@ -5,6 +5,8 @@ name: mesh-1 type: ServiceInsight mesh: mesh-1 name: all-services-mesh-1 +labels: + kuma.io/display-name: all-services-mesh-1 # refer to https://github.com/kumahq/kuma/issues/9729 services: backend: status: partially_degraded diff --git a/pkg/api-server/testdata/service-insights/list-simple.input.yaml b/pkg/api-server/testdata/service-insights/list-simple.input.yaml index 49b07373c32c..6d331b03620a 100644 --- a/pkg/api-server/testdata/service-insights/list-simple.input.yaml +++ b/pkg/api-server/testdata/service-insights/list-simple.input.yaml @@ -5,6 +5,8 @@ name: mesh-1 type: ServiceInsight mesh: mesh-1 name: all-services-mesh-1 +labels: + kuma.io/display-name: all-services-mesh-1 # refer to https://github.com/kumahq/kuma/issues/9729 services: frontend: status: partially_degraded diff --git a/pkg/api-server/testdata/service-insights/pagination.input.yaml b/pkg/api-server/testdata/service-insights/pagination.input.yaml index d121bbb64c03..7fd35d61e8d4 100644 --- a/pkg/api-server/testdata/service-insights/pagination.input.yaml +++ b/pkg/api-server/testdata/service-insights/pagination.input.yaml @@ -6,6 +6,8 @@ name: mesh-1 type: ServiceInsight mesh: mesh-1 name: all-services-mesh-1 +labels: + kuma.io/display-name: all-services-mesh-1 # refer to https://github.com/kumahq/kuma/issues/9729 services: frontend: status: partially_degraded From 62446e5d1231aa1f682207ba354ba08c3a0bb61a Mon Sep 17 00:00:00 2001 From: Icarus Wu Date: Fri, 25 Oct 2024 10:03:47 +0800 Subject: [PATCH 4/4] modify comment Signed-off-by: Icarus Wu --- pkg/api-server/testdata/service-insights/get.input.yaml | 2 +- pkg/api-server/testdata/service-insights/list-simple.input.yaml | 2 +- pkg/api-server/testdata/service-insights/pagination.input.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/api-server/testdata/service-insights/get.input.yaml b/pkg/api-server/testdata/service-insights/get.input.yaml index ecdcba9e2d13..6e2f64c40e99 100644 --- a/pkg/api-server/testdata/service-insights/get.input.yaml +++ b/pkg/api-server/testdata/service-insights/get.input.yaml @@ -6,7 +6,7 @@ type: ServiceInsight mesh: mesh-1 name: all-services-mesh-1 labels: - kuma.io/display-name: all-services-mesh-1 # refer to https://github.com/kumahq/kuma/issues/9729 + kuma.io/display-name: all-services-mesh-1 # add display name manually to test if it's removed in the response services: backend: status: partially_degraded diff --git a/pkg/api-server/testdata/service-insights/list-simple.input.yaml b/pkg/api-server/testdata/service-insights/list-simple.input.yaml index 6d331b03620a..d90bb5f6b13d 100644 --- a/pkg/api-server/testdata/service-insights/list-simple.input.yaml +++ b/pkg/api-server/testdata/service-insights/list-simple.input.yaml @@ -6,7 +6,7 @@ type: ServiceInsight mesh: mesh-1 name: all-services-mesh-1 labels: - kuma.io/display-name: all-services-mesh-1 # refer to https://github.com/kumahq/kuma/issues/9729 + kuma.io/display-name: all-services-mesh-1 # add display name manually to test if it's removed in the response services: frontend: status: partially_degraded diff --git a/pkg/api-server/testdata/service-insights/pagination.input.yaml b/pkg/api-server/testdata/service-insights/pagination.input.yaml index 7fd35d61e8d4..8e34db140350 100644 --- a/pkg/api-server/testdata/service-insights/pagination.input.yaml +++ b/pkg/api-server/testdata/service-insights/pagination.input.yaml @@ -7,7 +7,7 @@ type: ServiceInsight mesh: mesh-1 name: all-services-mesh-1 labels: - kuma.io/display-name: all-services-mesh-1 # refer to https://github.com/kumahq/kuma/issues/9729 + kuma.io/display-name: all-services-mesh-1 # add display name manually to test if it's removed in the response services: frontend: status: partially_degraded