Michael's feedback #1

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

@@ -247,11 +247,15 @@ func (c NamespacedResourceWatcherCache) resync() (string, error) {
 	}
 
 	// TODO: (gfichtenholt) RBAC check whether I list and watch specified GVR?
+	// Currently you'll need to run with the unsafe dev-only service account, since the plugin sets
+	// up background jobs that are running outside of requests from the user (ie. we're not using the
+	// users' token for those). Longer term, the plan is to create a separate RBAC yaml specific to
+	// the plugin that will need to be applied for using the plugin (nice and explicit),
+	// granting additional RBAC privs to the service account used by kubeapps-apis
 
-	// this will list resources from all namespaces.
 	// Notice, we are not setting resourceVersion in ListOptions, which means
 	// per https://kubernetes.io/docs/reference/using-api/api-concepts/
-	// For get and list, the semantics of resource version unset are to get the most recent
+	// For Get() and List(), the semantics of resource version unset are to get the most recent
 	// version
 	listItems, err := dynamicClient.Resource(c.config.gvr).Namespace(apiv1.NamespaceAll).List(ctx, metav1.ListOptions{})
 	if err != nil {

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

@@ -28,6 +28,7 @@ import (
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage/driver"
 	apiv1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -214,7 +215,7 @@ func (s *Server) installedPackageDetail(ctx context.Context, name types.Namespac
 		return nil, status.Errorf(codes.NotFound, "Unable to find Helm release %q due to: %v", name, err)
 	}
 
-	log.Infof("installedPackageDetail:\n[%s]", prettyPrintMap(unstructuredRelease.Object))
+	log.V(4).Infof("installedPackageDetail:\n[%s]", prettyPrintMap(unstructuredRelease.Object))
 
 	obj := unstructuredRelease.Object
 	var pkgVersionRef *corev1.VersionReference
@@ -255,11 +256,16 @@ func (s *Server) installedPackageDetail(ctx context.Context, name types.Namespac
 	// to invoke helm layer yet
 	if err == nil && release != nil {
 		// a couple of fields currrently only available via helm API
-		appVersion = release.Chart.AppVersion()
+		if release.Chart != nil {
+			appVersion = release.Chart.AppVersion()
+		}
 		if release.Info != nil {
 			postInstallNotes = release.Info.Notes
 		}
+	} else if err != nil && !errors.IsNotFound(err) {
+		log.Warningf("Failed to get helm release due to %v", err)
 	}
+
 	return &corev1.InstalledPackageDetail{
 		InstalledPackageRef: &corev1.InstalledPackageReference{
 			Context: &corev1.Context{

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

@@ -18,6 +18,7 @@ import (
 	"math/rand"
 	"os"
 	"os/exec"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -41,6 +42,7 @@ import (
 )
 
 // This is an integration test: it tests the full integration of flux plugin with flux back-end
+// To run these tests, enable ENABLE_FLUX_INTEGRATION_TESTS variable
 // pre-requisites for these tests to run:
 // 1) kind cluster with flux deployed
 // 2) kubeapps apis apiserver service running with fluxv2 plug-in enabled, port forwarded to 8080, e.g.
@@ -49,9 +51,10 @@ import (
 //    at this point.
 // 3) run ./kind-cluster-setup.sh once prior to these tests
 
-// if (1) or (2) of the above pre-requisites is not satisfied, the tests are simply skipped
-
 const (
+	// EnvvarFluxIntegrationTests enables tests that run against a local kind cluster
+	envVarFluxIntegrationTests = "ENABLE_FLUX_INTEGRATION_TESTS"
+
 	// the only repo this test uses so far. Enough for this test. This is local copy of what was on
 	// "https://stefanprodan.github.io/podinfo" on Sept 10 2021.
 	// If we want other repos, we'll have add directories and tinker with ./Dockerfile and NGINX conf.
@@ -60,6 +63,8 @@ const (
 )
 
 func TestKindClusterCreateInstalledPackage(t *testing.T) {
+	fluxPluginClient := checkEnv(t)
+
 	testCases := []struct {
 		testName          string
 		repoUrl           string
@@ -97,22 +102,13 @@ func TestKindClusterCreateInstalledPackage(t *testing.T) {
 		},
 	}
 
-	if up, err := isLocalKindClusterUp(t); err != nil || !up {
-		t.Skipf("skipping tests because due to failure to find local kind cluster: [%v]", err)
-	}
-	var fluxPluginClient fluxplugin.FluxV2PackagesServiceClient
-	var err error
-	if fluxPluginClient, err = getFluxPluginClient(t); err != nil {
-		t.Skipf("skipping tests due to failure to get fluxv2 plugin: [%v]", err)
-	}
-
 	rand.Seed(time.Now().UnixNano())
 
 	for _, tc := range testCases {
 		t.Run(tc.testName, func(t *testing.T) {
 			availablePackageRef := tc.request.AvailablePackageRef
 			idParts := strings.Split(availablePackageRef.Identifier, "/")
-			err = kubeCreateHelmRepository(t, idParts[0], tc.repoUrl, availablePackageRef.Context.Namespace)
+			err := kubeCreateHelmRepository(t, idParts[0], tc.repoUrl, availablePackageRef.Context.Namespace)
 			if err != nil {
 				t.Fatalf("%+v", err)
 			}
@@ -239,6 +235,33 @@ func TestKindClusterCreateInstalledPackage(t *testing.T) {
 	}
 }
 
+func checkEnv(t *testing.T) fluxplugin.FluxV2PackagesServiceClient {
+	enableEnvVar := os.Getenv(envVarFluxIntegrationTests)
+	runTests := false
+	if enableEnvVar != "" {
+		var err error
+		runTests, err = strconv.ParseBool(enableEnvVar)
+		if err != nil {
+			t.Fatalf("%+v", err)
+		}
+	}
+
+	if !runTests {
+		t.Skipf("skipping flux plugin integration tests as %q not set to be true", envVarFluxIntegrationTests)
+	} else {
+		if up, err := isLocalKindClusterUp(t); err != nil || !up {
+			t.Fatalf("Failed to find local kind cluster due to: [%v]", err)
+		}
+		var fluxPluginClient fluxplugin.FluxV2PackagesServiceClient
+		var err error
+		if fluxPluginClient, err = getFluxPluginClient(t); err != nil {
+			t.Fatalf("Failed to get fluxv2 plugin due to: [%v]", err)
+		}
+		return fluxPluginClient
+	}
+	return nil
+}
+
 func isLocalKindClusterUp(t *testing.T) (up bool, err error) {
 	t.Logf("+isLocalKindClusterUp")
 	cmd := exec.Command("kind", "get", "clusters")

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/testdata/Dockerfile

@@ -1,2 +1,5 @@
+# this Dockerfile is for building a docker image for a pod that can be deployed into a k8s cluster
+# that serves some of the content of test-data (index.yaml, charts .tgz)
 FROM nginx
-COPY ./podinfo-index.yaml /usr/share/nginx/html/index.yaml
+COPY ./podinfo-index.yaml /usr/share/nginx/html/index.yaml
+COPY ./podinfo-6.0.0.tgz /usr/share/nginx/html/

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/testdata/kind-cluster-setup.sh

@@ -6,7 +6,7 @@ set -o errexit
 set -o nounset
 set -o pipefail
 
-TAG=0.0.3
+TAG=0.0.5
 
 function deploy {
   docker build -t kubeapps/fluxv2plugin-testdata:$TAG .
@@ -25,13 +25,13 @@ function portforward {
   kubectl -n default port-forward svc/fluxv2plugin-testdata-svc 8081:80 --context kind-kubeapps 
 }
 
-function exec {
-  kubectl exec --stdin --tty fluxv2plugin-testdata-app-74766cf559-695qg -- /bin/bash
+function shell {
+  kubectl exec --stdin --tty fluxv2plugin-testdata-app-74766cf559-695qg -- /bin/bash --context kind-kubeapps 
 }
 
 if [ $# -lt 1 ]
 then
-  echo "Usage : $0 deploy|undeploy"
+  echo "Usage : $0 deploy|undeploy|portforward|shell"
   exit
 fi
 
@@ -42,7 +42,7 @@ undeploy) undeploy
     ;;
 portforward) portforward
     ;;
-exec) exec
+shell) shell
     ;;
 *) echo "Invalid option"
    ;;

cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/testdata/podinfo-index.yaml

@@ -15,5 +15,5 @@ entries:
     sources:
     - https://github.com/stefanprodan/podinfo
     urls:
-    - https://stefanprodan.github.io/podinfo/podinfo-6.0.0.tgz
+    - http://fluxv2plugin-testdata-svc.default.svc.cluster.local:80/podinfo-6.0.0.tgz
     version: 6.0.0