flux plugin to use targetNamespace for HelmRelease CRD per #3640 (#3662)

* step 1

* step 2

* step 3

* step 4

* Michael's feedback #1

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

@@ -312,6 +312,23 @@ func kubeDeleteServiceAccount(t *testing.T, name, namespace string) error {
 	return nil
 }
 
+func kubeCreateNamespace(t *testing.T, namespace string) error {
+	t.Logf("+kubeCreateNamespace(%s)", namespace)
+	if typedClient, err := kubeGetTypedClient(); err != nil {
+		return err
+	} else if typedClient.CoreV1().Namespaces().Create(
+		context.TODO(),
+		&kubecorev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: namespace,
+			},
+		},
+		metav1.CreateOptions{}); err != nil {
+		return err
+	}
+	return nil
+}
+
 func kubeDeleteNamespace(t *testing.T, namespace string) error {
 	t.Logf("+kubeDeleteNamespace(%s)", namespace)
 	if typedClient, err := kubeGetTypedClient(); err != nil {

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

@@ -16,7 +16,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"os"
 	"strings"
 	"time"
 
@@ -77,14 +76,6 @@ func (s *Server) listReleasesInCluster(ctx context.Context, namespace string) (*
 	}
 }
 
-func (s *Server) getReleaseInCluster(ctx context.Context, name types.NamespacedName) (*unstructured.Unstructured, error) {
-	releasesIfc, err := s.getReleasesResourceInterface(ctx, name.Namespace)
-	if err != nil {
-		return nil, err
-	}
-	return releasesIfc.Get(ctx, name.Name, metav1.GetOptions{})
-}
-
 func (s *Server) paginatedInstalledPkgSummaries(ctx context.Context, namespace string, pageSize int32, pageOffset int) ([]*corev1.InstalledPackageSummary, error) {
 	releasesFromCluster, err := s.listReleasesInCluster(ctx, namespace)
 	if err != nil {
@@ -212,14 +203,18 @@ func (s *Server) installedPkgSummaryFromRelease(unstructuredRelease map[string]i
 }
 
 func (s *Server) installedPackageDetail(ctx context.Context, name types.NamespacedName) (*corev1.InstalledPackageDetail, error) {
-	unstructuredRelease, err := s.getReleaseInCluster(ctx, name)
+	releasesIfc, err := s.getReleasesResourceInterface(ctx, name.Namespace)
+	if err != nil {
+		return nil, err
+	}
+	unstructuredRelease, err := releasesIfc.Get(ctx, name.Name, metav1.GetOptions{})
 	if err != nil {
 		if errors.IsNotFound(err) {
 			return nil, status.Errorf(codes.NotFound, "%q", err)
 		} else if errors.IsForbidden(err) || errors.IsUnauthorized(err) {
 			return nil, status.Errorf(codes.Unauthenticated, "unable to get release due to %v", err)
 		} else {
-			return nil, status.Errorf(codes.Internal, "%q", err)
+			return nil, status.Errorf(codes.Internal, "unable to get release due to %v", err)
 		}
 	}
 
@@ -331,12 +326,9 @@ func (s *Server) helmReleaseFromUnstructured(ctx context.Context, name types.Nam
 }
 
 func (s *Server) newRelease(ctx context.Context, packageRef *corev1.AvailablePackageReference, targetName types.NamespacedName, versionRef *corev1.VersionReference, reconcile *corev1.ReconciliationOptions, valuesString string) (*corev1.InstalledPackageReference, error) {
-	// HACK: just for now assume HelmRelease CRD will live in the kubeapps namespace
-	kubeappsNamespace := os.Getenv("POD_NAMESPACE")
-	if kubeappsNamespace == "" {
-		return nil, status.Errorf(codes.FailedPrecondition, "POD_NAMESPACE not specified")
-	}
-	resourceIfc, err := s.getReleasesResourceInterface(ctx, kubeappsNamespace)
+	// per https://github.com/kubeapps/kubeapps/pull/3640#issuecomment-949315105
+	// the helm release CR to also be created in the target namespace (where the helm release itself is currently created)
+	resourceIfc, err := s.getReleasesResourceInterface(ctx, targetName.Namespace)
 	if err != nil {
 		return nil, err
 	}
@@ -367,16 +359,18 @@ func (s *Server) newRelease(ctx context.Context, packageRef *corev1.AvailablePac
 		}
 	}
 
-	fluxHelmRelease, err := newFluxHelmRelease(chart, kubeappsNamespace, targetName, versionRef, reconcile, values)
+	fluxHelmRelease, err := newFluxHelmRelease(chart, targetName, versionRef, reconcile, values)
 	if err != nil {
 		return nil, err
 	}
 	newRelease, err := resourceIfc.Create(ctx, fluxHelmRelease, metav1.CreateOptions{})
 	if err != nil {
 		if errors.IsForbidden(err) || errors.IsUnauthorized(err) {
+			// TODO (gfichtenholt) I think in some cases we should be returning codes.PermissionDenied instead,
+			// but that has to be done consistently accross all plug-in operations, not just here
 			return nil, status.Errorf(codes.Unauthenticated, "Unable to create release due to %v", err)
 		} else {
-			return nil, err
+			return nil, status.Errorf(codes.Internal, "Unable to create release due to %v", err)
 		}
 	}
 
@@ -402,9 +396,9 @@ func (s *Server) updateRelease(ctx context.Context, packageRef *corev1.Installed
 		if errors.IsNotFound(err) {
 			return nil, status.Errorf(codes.NotFound, "%q", err)
 		} else if errors.IsForbidden(err) || errors.IsUnauthorized(err) {
-			return nil, status.Errorf(codes.Unauthenticated, "Unable to ase due to %v", err)
+			return nil, status.Errorf(codes.Unauthenticated, "Unable to get release due to %v", err)
 		} else {
-			return nil, status.Errorf(codes.Internal, "%q", err)
+			return nil, status.Errorf(codes.Internal, "Unable to get release due to %v", err)
 		}
 	}
 
@@ -467,7 +461,7 @@ func (s *Server) updateRelease(ctx context.Context, packageRef *corev1.Installed
 		if errors.IsForbidden(err) || errors.IsUnauthorized(err) {
 			return nil, status.Errorf(codes.Unauthenticated, "Unable to update release due to %v", err)
 		} else {
-			return nil, err
+			return nil, status.Errorf(codes.Internal, "Unable to update release due to %v", err)
 		}
 	}
 
@@ -494,7 +488,7 @@ func (s *Server) deleteRelease(ctx context.Context, packageRef *corev1.Installed
 		} else if errors.IsForbidden(err) || errors.IsUnauthorized(err) {
 			return status.Errorf(codes.Unauthenticated, "Unable to delete release due to %v", err)
 		} else {
-			return status.Errorf(codes.Internal, "%q", err)
+			return status.Errorf(codes.Internal, "Unable to delete release due to %v", err)
 		}
 	}
 	return nil
@@ -616,16 +610,17 @@ func installedPackageAvailablePackageRefFromUnstructured(unstructuredRelease map
 
 // Potentially, there are 3 different namespaces that can be specified here
 // 1. spec.chart.spec.sourceRef.namespace, where HelmRepository CRD object referenced exists
-// 2. metadata.namespace, where this HelmRelease CRD will exist
+// 2. metadata.namespace, where this HelmRelease CRD will exist, same as (3) below
+//    per https://github.com/kubeapps/kubeapps/pull/3640#issuecomment-949315105
 // 3. spec.targetNamespace, where flux will install any artifacts from the release
-func newFluxHelmRelease(chart *models.Chart, releaseNamespace string, targetName types.NamespacedName, versionRef *corev1.VersionReference, reconcile *corev1.ReconciliationOptions, values map[string]interface{}) (*unstructured.Unstructured, error) {
+func newFluxHelmRelease(chart *models.Chart, targetName types.NamespacedName, versionRef *corev1.VersionReference, reconcile *corev1.ReconciliationOptions, values map[string]interface{}) (*unstructured.Unstructured, error) {
 	unstructuredRel := unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"apiVersion": fmt.Sprintf("%s/%s", fluxHelmReleaseGroup, fluxHelmReleaseVersion),
 			"kind":       fluxHelmRelease,
 			"metadata": map[string]interface{}{
 				"name":      targetName.Name,
-				"namespace": releaseNamespace,
+				"namespace": targetName.Namespace,
 			},
 			"spec": map[string]interface{}{
 				"chart": map[string]interface{}{
@@ -638,9 +633,6 @@ func newFluxHelmRelease(chart *models.Chart, releaseNamespace string, targetName
 						},
 					},
 				},
-				"install": map[string]interface{}{
-					"createNamespace": true,
-				},
 				"targetNamespace": targetName.Namespace,
 			},
 		},