Issue5542 - Unable to edit or delete newly created flux package repos…

…itory (#5664)

### Description of the change

This change completes the changes previously made in an earlier change
regarding flux repository edit/delete buttons enablement issues.
The main switch is to change the scope of a flux repository from global
to namespaced. A flux repository behaves more similar to a namespaced
repository than a global repository, making its handling easier in the
UI.

The change also includes a fix where carvel global repositories were not
included, even if the user has list/view access to them.

Finally, the create/update repository dialog in the UI has been updated
to reflect that the default scope of a flux repository is namespaced,
and not global, and that it cannot be changed.

### Benefits

Users that have namespace-only access can now properly manage the flux
repositories they created.

### Applicable issues

- fixes #5542

### Additional information

This change does not include a fix for the page and table buttons when
the "show in all namespaces" toggle is on.
Users can use the workaround to toggle off the switch. For this reason,
a proper fix is best delayed to when the repository page is revamped.

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

@@ -232,8 +232,10 @@ func (s *Server) newRepo(ctx context.Context, request *corev1.AddPackageReposito
 		return nil, status.Errorf(codes.InvalidArgument, "no request Name provided")
 	}
 
-	if request.GetNamespaceScoped() {
-		return nil, status.Errorf(codes.Unimplemented, "namespaced-scoped repositories are not supported")
+	// 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
+	if !request.GetNamespaceScoped() {
+		return nil, status.Errorf(codes.Unimplemented, "global-scoped repositories are not supported")
 	}
 
 	typ := request.GetType()
@@ -377,8 +379,10 @@ func (s *Server) repoDetail(ctx context.Context, repoRef *corev1.PackageReposito
 		},
 		Name: repo.Name,
 		// TODO (gfichtenholt) Flux HelmRepository CR doesn't have a designated field for description
-		Description:     "",
-		NamespaceScoped: false,
+		Description: "",
+		// 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,
 		Type:            typ,
 		Url:             repo.Spec.URL,
 		Interval:        pkgutils.FromDuration(&repo.Spec.Interval),
@@ -429,8 +433,10 @@ func (s *Server) repoSummaries(ctx context.Context, ns string) ([]*corev1.Packag
 			},
 			Name: repo.Name,
 			// TODO (gfichtenholt) Flux HelmRepository CR doesn't have a designated field for description
-			Description:     "",
-			NamespaceScoped: false,
+			Description: "",
+			// 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,
 			Type:            typ,
 			Url:             repo.Spec.URL,
 			Status:          repoStatus(repo),

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

@@ -7,7 +7,6 @@ import (
 	"context"
 	packagingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1"
 	"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/resources"
-	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	corev1 "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/core/packages/v1alpha1"
@@ -149,17 +148,33 @@ func (s *Server) GetPackageRepositorySummaries(ctx context.Context, request *cor
 	// trace logging
 	log.InfoS("+kapp-controller GetPackageRepositories", "cluster", cluster, "namespace", namespace)
 
-	// retrieve the list of installed packages
-	pkgRepositories, err := s.getPkgRepositories(ctx, cluster, namespace)
-	if err != nil {
-		if errors.IsForbidden(err) && namespace == "" {
+	// retrieve the list of repositories
+	var pkgRepositories []*packagingv1alpha1.PackageRepository
+	if namespace == "" {
+		// find globally, either via cluster access or by enumerating through namespaces
+		if repos, err := s.getPkgRepositories(ctx, cluster, ""); err == nil {
+			pkgRepositories = append(pkgRepositories, repos...)
+		} else {
 			log.Warningf("+kapp-controller unable to list package repositories at the cluster scope in '%s' due to [%v]", cluster, err)
-			pkgRepositories, err = s.getAccessiblePackageRepositories(ctx, cluster)
-			if err != nil {
+			if repos, err = s.getAccessiblePackageRepositories(ctx, cluster); err == nil {
+				pkgRepositories = append(pkgRepositories, repos...)
+			} else {
 				return nil, err
 			}
+		}
+	} else {
+		// include namespace specific  repositories
+		if repos, err := s.getPkgRepositories(ctx, cluster, namespace); err == nil {
+			pkgRepositories = append(pkgRepositories, repos...)
 		} else {
-			return nil, statuserror.FromK8sError("get", "PackageRepository", "", err)
+			return nil, err
+		}
+
+		// try to also include global repositories
+		if namespace != s.pluginConfig.globalPackagingNamespace {
+			if repos, err := s.getPkgRepositories(ctx, cluster, s.pluginConfig.globalPackagingNamespace); err == nil {
+				pkgRepositories = append(pkgRepositories, repos...)
+			}
 		}
 	}
 
@@ -323,6 +338,7 @@ func (s *Server) DeletePackageRepository(ctx context.Context, request *corev1.De
 	return response, nil
 }
 
+// GetPackageRepositoryPermissions provides permissions available to manage package repository by the 'kapp_controller' plugin
 func (s *Server) GetPackageRepositoryPermissions(ctx context.Context, request *corev1.GetPackageRepositoryPermissionsRequest) (*corev1.GetPackageRepositoryPermissionsResponse, error) {
 	log.Infof("+kapp-controller GetPackageRepositoryPermissions [%v]", request)
 

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

@@ -358,15 +358,33 @@ func (s *Server) getAccessiblePackageRepositories(ctx context.Context, cluster s
 		return nil, err
 	}
 	namespaceList = resources.FilterActiveNamespaces(namespaceList)
+
 	var accessibleRepos []*packagingv1alpha1.PackageRepository
 	for _, ns := range namespaceList {
-		nsRepos, err := s.getPkgRepositories(ctx, cluster, ns.Name)
-		if err != nil {
+		if nsRepos, err := s.getPkgRepositories(ctx, cluster, ns.Name); err != nil {
 			log.Warningf("++kapp-controller could not list PackageRepository in namespace %s", ns.Name)
 			// Continue. Error in a single namespace should not block the whole list
+		} else {
+			accessibleRepos = append(accessibleRepos, nsRepos...)
 		}
-		accessibleRepos = append(accessibleRepos, nsRepos...)
 	}
+
+	// in general, the user will not have admin level access to the global namespace, so checking explicitly
+	var hasglobalns bool
+	for _, ns := range namespaceList {
+		if ns.Name == s.pluginConfig.globalPackagingNamespace {
+			hasglobalns = true
+			break
+		}
+	}
+	if !hasglobalns {
+		if nsRepos, err := s.getPkgRepositories(ctx, cluster, s.pluginConfig.globalPackagingNamespace); err != nil {
+			log.Warningf("++kapp-controller could not list PackageRepository in global namespace")
+		} else {
+			accessibleRepos = append(accessibleRepos, nsRepos...)
+		}
+	}
+
 	return accessibleRepos, nil
 }
 

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

@@ -9310,8 +9310,10 @@ func TestGetPackageRepositorySummariesFiltering(t *testing.T) {
 			request: &corev1.GetPackageRepositorySummariesRequest{
 				Context: &corev1.Context{Namespace: "default"},
 			},
-			existingObjects:  repositories,
-			expectedResponse: []metav1.ObjectMeta{},
+			existingObjects: repositories,
+			expectedResponse: []metav1.ObjectMeta{
+				{Name: "globalrepo", Namespace: demoGlobalPackagingNamespace},
+			},
 		},
 		{
 			name: "returns repositories from given namespace",
@@ -9320,6 +9322,7 @@ func TestGetPackageRepositorySummariesFiltering(t *testing.T) {
 			},
 			existingObjects: repositories,
 			expectedResponse: []metav1.ObjectMeta{
+				{Name: "globalrepo", Namespace: demoGlobalPackagingNamespace},
 				{Name: "nsrepo", Namespace: "privatens"},
 			},
 		},

dashboard/src/components/Config/PkgRepoList/PkgRepoForm.tsx

@@ -532,10 +532,8 @@ export function PkgRepoForm(props: IPkgRepoFormProps) {
           setType(RepositoryStorageTypes.PACKAGE_REPOSITORY_STORAGE_HELM);
         }
 
-        // update the isNampespaced field based on the plugin
-        setIsNamespaceScoped(
-          !isGlobalNamespace(namespace, PluginNames.PACKAGES_FLUX, currentNsConfig),
-        );
+        // flux is always namespace scoped
+        setIsNamespaceScoped(true);
         break;
       }
       case PluginNames.PACKAGES_KAPP:
@@ -849,7 +847,8 @@ export function PkgRepoForm(props: IPkgRepoFormProps) {
                           onChange={handleRepoScopeChange}
                           disabled={
                             !!repo.name ||
-                            isGlobalNamespace(namespace, plugin?.name, currentNsConfig)
+                            isGlobalNamespace(namespace, plugin?.name, currentNsConfig) ||
+                            plugin?.name === PluginNames.PACKAGES_FLUX
                           }
                           required={true}
                         />
@@ -870,7 +869,8 @@ export function PkgRepoForm(props: IPkgRepoFormProps) {
                           onChange={handleRepoScopeChange}
                           disabled={
                             !!repo.name ||
-                            isGlobalNamespace(namespace, plugin?.name, currentNsConfig)
+                            isGlobalNamespace(namespace, plugin?.name, currentNsConfig) ||
+                            plugin?.name === PluginNames.PACKAGES_FLUX
                           }
                           required={true}
                         />

dashboard/src/shared/utils.test.ts

@@ -328,8 +328,8 @@ it("isGlobalNamespace", () => {
   } as IConfig;
   expect(isGlobalNamespace("helm-global", PluginNames.PACKAGES_HELM, kubeappsConfig)).toBe(true);
   expect(isGlobalNamespace("helm-global", PluginNames.PACKAGES_KAPP, kubeappsConfig)).toBe(false);
-  expect(isGlobalNamespace("helm-global", PluginNames.PACKAGES_FLUX, kubeappsConfig)).toBe(true);
+  expect(isGlobalNamespace("helm-global", PluginNames.PACKAGES_FLUX, kubeappsConfig)).toBe(false);
   expect(isGlobalNamespace("carvel-global", PluginNames.PACKAGES_HELM, kubeappsConfig)).toBe(false);
   expect(isGlobalNamespace("carvel-global", PluginNames.PACKAGES_KAPP, kubeappsConfig)).toBe(true);
-  expect(isGlobalNamespace("carvel-global", PluginNames.PACKAGES_FLUX, kubeappsConfig)).toBe(true);
+  expect(isGlobalNamespace("carvel-global", PluginNames.PACKAGES_FLUX, kubeappsConfig)).toBe(false);
 });

dashboard/src/shared/utils.ts

@@ -314,9 +314,9 @@ export function isGlobalNamespace(namespace: string, pluginName: string, kubeapp
       return namespace === kubeappsConfig.helmGlobalNamespace;
     case PluginNames.PACKAGES_KAPP:
       return namespace === kubeappsConfig.carvelGlobalNamespace;
-    // Currently, Flux doesn't namespaced repos, so it will always be global
+    // Currently, Flux doesn't support global repositories
     case PluginNames.PACKAGES_FLUX:
-      return true;
+      return false;
     default:
       return false;
   }
@@ -332,7 +332,7 @@ export function getGlobalNamespaceOrNamespace(
       return kubeappsConfig.helmGlobalNamespace;
     case PluginNames.PACKAGES_KAPP:
       return kubeappsConfig.carvelGlobalNamespace;
-    // Currently, Flux doesn't namespaced repos, so the given ns will be indeed global
+    // Currently, Flux doesn't support global repositories, so returning the namespace so we have a value
     case PluginNames.PACKAGES_FLUX:
       return namespace;
     default: