implemented code review suggestions

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go

@@ -5,6 +5,7 @@ package main
 
 import (
 	"encoding/json"
+	"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/k8sutils"
 	"os"
 	"time"
 
@@ -1132,7 +1133,7 @@ var (
 			Name:            "bar",
 			Namespace:       "foo",
 			ResourceVersion: "1",
-			Annotations:     map[string]string{Annotation_Description_Key: "repo desc"},
+			Annotations:     map[string]string{k8sutils.AnnotationDescriptionKey: "repo desc"},
 		},
 		Spec: sourcev1.HelmRepositorySpec{
 			URL:      "http://example.com",

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"encoding/gob"
 	"fmt"
+	"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/k8sutils"
 	"regexp"
 	"strings"
 	"time"
@@ -40,9 +41,6 @@ const (
 	// see docs at https://fluxcd.io/docs/components/source/ and
 	// https://fluxcd.io/docs/components/helm/api/
 	fluxHelmRepositories = "helmrepositories"
-
-	// description support
-	Annotation_Description_Key = "kubeapps.dev/description"
 )
 
 var (
@@ -348,7 +346,7 @@ func (s *Server) repoDetail(ctx context.Context, repoRef *corev1.PackageReposito
 			Plugin:     GetPluginDetail(),
 		},
 		Name:        repo.Name,
-		Description: getDescription(repo),
+		Description: k8sutils.GetDescription(&repo.ObjectMeta),
 		// flux repositories are now considered to be namespaced, to support the most common cases.
 		// see discussion at https://github.com/vmware-tanzu/kubeapps/issues/5542
 		NamespaceScoped: true,
@@ -401,7 +399,7 @@ func (s *Server) repoSummaries(ctx context.Context, ns string) ([]*corev1.Packag
 				Plugin:     GetPluginDetail(),
 			},
 			Name:        repo.Name,
-			Description: getDescription(&repo),
+			Description: k8sutils.GetDescription(&repo.ObjectMeta),
 			// flux repositories are now considered to be namespaced, to support the most common cases.
 			// see discussion at https://github.com/vmware-tanzu/kubeapps/issues/5542
 			NamespaceScoped: true,
@@ -436,7 +434,7 @@ func (s *Server) updateRepo(ctx context.Context, repoRef *corev1.PackageReposito
 	repo.Spec.URL = request.Url
 
 	// description now supported via annotation
-	setDescription(repo, request.Description)
+	k8sutils.SetDescription(&repo.ObjectMeta, request.Description)
 
 	if request.Interval != "" {
 		if duration, err := pkgutils.ToDuration(request.Interval); err != nil {
@@ -944,7 +942,7 @@ func newFluxHelmRepo(
 		fluxRepo.Spec.Type = sourcev1.HelmRepositoryTypeOCI
 	}
 	if desc != "" {
-		setDescription(fluxRepo, desc)
+		k8sutils.SetDescription(&fluxRepo.ObjectMeta, desc)
 	}
 	if secret != nil {
 		fluxRepo.Spec.SecretRef = &fluxmeta.LocalObjectReference{
@@ -959,19 +957,3 @@ func newFluxHelmRepo(
 	}
 	return fluxRepo, nil
 }
-
-// description
-func setDescription(fluxRepo *sourcev1.HelmRepository, description string) {
-	if description != "" {
-		if fluxRepo.Annotations == nil {
-			fluxRepo.Annotations = make(map[string]string)
-		}
-		fluxRepo.Annotations[Annotation_Description_Key] = description
-	} else {
-		delete(fluxRepo.Annotations, Annotation_Description_Key)
-	}
-}
-
-func getDescription(fluxRepo *sourcev1.HelmRepository) string {
-	return fluxRepo.Annotations[Annotation_Description_Key]
-}

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_auth.go

@@ -605,6 +605,7 @@ func decodeDockerAuth(dockerjson []byte) (*corev1.DockerCredentials, error) {
 	if err := json.Unmarshal(dockerjson, config); err != nil {
 		return nil, err
 	}
+	// note: by design, this method is used when the secret is kubeapps managed, hence we expect only one item in the map
 	for server, entry := range config.Auths {
 		docker := &corev1.DockerCredentials{
 			Server:   server,

cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_data_adapters.go

@@ -6,6 +6,7 @@ package main
 import (
 	"context"
 	"fmt"
+	"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/k8sutils"
 	"strings"
 
 	kappctrlv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1"
@@ -35,8 +36,6 @@ const (
 
 	Redacted = "REDACTED"
 
-	Annotation_Description_Key = "kubeapps.dev/description"
-
 	Annotation_ManagedBy_Key   = "kubeapps.dev/managed-by"
 	Annotation_ManagedBy_Value = "plugin:kapp-controller"
 
@@ -438,7 +437,7 @@ func (s *Server) buildPackageRepositorySummary(pkgRepository *packagingv1alpha1.
 			Identifier: pkgRepository.Name,
 		},
 		Name:            pkgRepository.Name,
-		Description:     getDescription(pkgRepository),
+		Description:     k8sutils.GetDescription(&pkgRepository.ObjectMeta),
 		NamespaceScoped: s.pluginConfig.globalPackagingNamespace != pkgRepository.Namespace,
 		RequiresAuth:    repositorySecretRef(pkgRepository) != nil,
 	}
@@ -490,7 +489,7 @@ func (s *Server) buildPackageRepository(pkgRepository *packagingv1alpha1.Package
 			Identifier: pkgRepository.Name,
 		},
 		Name:            pkgRepository.Name,
-		Description:     getDescription(pkgRepository),
+		Description:     k8sutils.GetDescription(&pkgRepository.ObjectMeta),
 		NamespaceScoped: s.pluginConfig.globalPackagingNamespace != pkgRepository.Namespace,
 	}
 
@@ -653,7 +652,7 @@ func (s *Server) buildPkgRepositoryCreate(request *corev1.AddPackageRepositoryRe
 	repository.Spec = s.buildPkgRepositorySpec(request.Type, request.Interval, request.Url, request.Auth, pkgSecret, details)
 
 	// description
-	setDescription(repository, request.Description)
+	k8sutils.SetDescription(&repository.ObjectMeta, request.Description)
 
 	return repository, nil
 }
@@ -684,7 +683,7 @@ func (s *Server) buildPkgRepositoryUpdate(request *corev1.UpdatePackageRepositor
 	repository.Spec = s.buildPkgRepositorySpec(rptype, request.Interval, request.Url, request.Auth, pkgSecret, details)
 
 	// description
-	setDescription(repository, request.Description)
+	k8sutils.SetDescription(&repository.ObjectMeta, request.Description)
 
 	return repository, nil
 }

cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_test.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/k8sutils"
 	authorizationv1 "k8s.io/api/authorization/v1"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	k8stesting "k8s.io/client-go/testing"
@@ -6812,7 +6813,7 @@ func TestAddPackageRepository(t *testing.T) {
 				return request
 			},
 			repositoryCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository {
-				repository.Annotations = map[string]string{Annotation_Description_Key: "repository description"}
+				repository.Annotations = map[string]string{k8sutils.AnnotationDescriptionKey: "repository description"}
 				return repository
 			},
 			expectedStatusCode: codes.OK,
@@ -7496,15 +7497,15 @@ func TestUpdatePackageRepository(t *testing.T) {
 		{
 			name: "update with new description",
 			initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository {
-				repository.Annotations = map[string]string{Annotation_Description_Key: "initial description"}
+				repository.Annotations = map[string]string{k8sutils.AnnotationDescriptionKey: "initial description"}
 				return repository
 			},
 			requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest {
 				request.Description = "updated description"
 				return request
 			},
 			repositoryCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository {
-				repository.Annotations = map[string]string{Annotation_Description_Key: "updated description"}
+				repository.Annotations = map[string]string{k8sutils.AnnotationDescriptionKey: "updated description"}
 				return repository
 			},
 			expectedStatusCode: codes.OK,
@@ -7513,7 +7514,7 @@ func TestUpdatePackageRepository(t *testing.T) {
 		{
 			name: "update remove description",
 			initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository {
-				repository.Annotations = map[string]string{Annotation_Description_Key: "initial description"}
+				repository.Annotations = map[string]string{k8sutils.AnnotationDescriptionKey: "initial description"}
 				return repository
 			},
 			requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest {
@@ -8440,7 +8441,7 @@ func TestGetPackageRepositoryDetail(t *testing.T) {
 		{
 			name: "check description",
 			repositoryCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository {
-				repository.Annotations = map[string]string{Annotation_Description_Key: "repository description"}
+				repository.Annotations = map[string]string{k8sutils.AnnotationDescriptionKey: "repository description"}
 				return repository
 			},
 			responseCustomizer: func(response *corev1.GetPackageRepositoryDetailResponse) *corev1.GetPackageRepositoryDetailResponse {
@@ -9161,7 +9162,7 @@ func TestGetPackageRepositorySummaries(t *testing.T) {
 			existingObjects: []k8sruntime.Object{
 				&packagingv1alpha1.PackageRepository{
 					TypeMeta:   defaultTypeMeta,
-					ObjectMeta: metav1.ObjectMeta{Name: "globalrepo", Namespace: demoGlobalPackagingNamespace, Annotations: map[string]string{Annotation_Description_Key: "repository summary description"}},
+					ObjectMeta: metav1.ObjectMeta{Name: "globalrepo", Namespace: demoGlobalPackagingNamespace, Annotations: map[string]string{k8sutils.AnnotationDescriptionKey: "repository summary description"}},
 					Spec: packagingv1alpha1.PackageRepositorySpec{
 						Fetch: &packagingv1alpha1.PackageRepositoryFetch{
 							ImgpkgBundle: &kappctrlv1alpha1.AppFetchImgpkgBundle{

cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_utils.go

@@ -490,22 +490,6 @@ func toPkgVersionSelection(version *kappcorev1.VersionSelection) *vendirversions
 	return pkgversion
 }
 
-// description
-func setDescription(pkgRepository *packagingv1alpha1.PackageRepository, description string) {
-	if description != "" {
-		if pkgRepository.Annotations == nil {
-			pkgRepository.Annotations = make(map[string]string)
-		}
-		pkgRepository.Annotations[Annotation_Description_Key] = description
-	} else {
-		delete(pkgRepository.Annotations, Annotation_Description_Key)
-	}
-}
-
-func getDescription(pkgRepository *packagingv1alpha1.PackageRepository) string {
-	return pkgRepository.Annotations[Annotation_Description_Key]
-}
-
 // secret state
 
 func repositorySecretRef(pkgRepository *packagingv1alpha1.PackageRepository) *kappctrlv1alpha1.AppFetchLocalRef {

cmd/kubeapps-apis/plugins/pkg/k8sutils/k8sutils.go

@@ -14,6 +14,11 @@ import (
 	"k8s.io/client-go/dynamic"
 )
 
+const (
+	// description as annotation
+	AnnotationDescriptionKey = "kubeapps.dev/description"
+)
+
 func WaitForResource(ctx context.Context, ri dynamic.ResourceInterface, name string, interval, timeout time.Duration) error {
 	err := wait.PollImmediateWithContext(ctx, interval, timeout, func(ctx context.Context) (bool, error) {
 		_, err := ri.Get(ctx, name, metav1.GetOptions{})
@@ -31,3 +36,19 @@ func WaitForResource(ctx context.Context, ri dynamic.ResourceInterface, name str
 	})
 	return err
 }
+
+// description
+func SetDescription(metadata *metav1.ObjectMeta, description string) {
+	if description != "" {
+		if metadata.Annotations == nil {
+			metadata.Annotations = make(map[string]string)
+		}
+		metadata.Annotations[AnnotationDescriptionKey] = description
+	} else {
+		delete(metadata.Annotations, AnnotationDescriptionKey)
+	}
+}
+
+func GetDescription(metadata *metav1.ObjectMeta) string {
+	return metadata.Annotations[AnnotationDescriptionKey]
+}

cmd/kubeapps-apis/plugins/pkg/k8sutils/k8sutils_test.go

@@ -96,3 +96,75 @@ func TestWaitForResource(t *testing.T) {
 		})
 	}
 }
+
+func TestSetDescription(t *testing.T) {
+	// with no prior annotations
+	{
+		metadata := metav1.ObjectMeta{
+			Name:      "foo",
+			Namespace: "bar",
+		}
+		SetDescription(&metadata, "description")
+
+		if metadata.Annotations[AnnotationDescriptionKey] != "description" {
+			t.Errorf("description was not set when annotations were empty")
+		}
+	}
+
+	// test with existing annotations
+	{
+		metadata := metav1.ObjectMeta{
+			Name:        "foo",
+			Namespace:   "bar",
+			Annotations: map[string]string{"hello": "world"},
+		}
+		SetDescription(&metadata, "description")
+
+		if metadata.Annotations[AnnotationDescriptionKey] != "description" {
+			t.Errorf("description was not set when annotations existed")
+		}
+		if metadata.Annotations["hello"] != "world" {
+			t.Errorf("existing annotation was removed")
+		}
+	}
+
+	// test unsetting annotations
+	{
+		metadata := metav1.ObjectMeta{
+			Name:        "foo",
+			Namespace:   "bar",
+			Annotations: map[string]string{AnnotationDescriptionKey: "description"},
+		}
+		SetDescription(&metadata, "")
+		if _, ok := metadata.Annotations[AnnotationDescriptionKey]; ok {
+			t.Errorf("the description annotation was not deleted as expeted when setting to empty string")
+		}
+	}
+}
+
+func TestGetDescription(t *testing.T) {
+	// with no annotations
+	{
+		metadata := metav1.ObjectMeta{
+			Name:      "foo",
+			Namespace: "bar",
+		}
+
+		if desc := GetDescription(&metadata); desc != "" {
+			t.Errorf("unexpected result calling get description with no annotations")
+		}
+	}
+
+	// test with existing annotations
+	{
+		metadata := metav1.ObjectMeta{
+			Name:        "foo",
+			Namespace:   "bar",
+			Annotations: map[string]string{"hello": "world", AnnotationDescriptionKey: "description"},
+		}
+
+		if desc := GetDescription(&metadata); desc != "description" {
+			t.Errorf("unable to get the description")
+		}
+	}
+}