Configure flux-specific RBAC for kubeapps-apis #3494

chart/kubeapps/templates/kubeappsapis/deployment.yaml

@@ -29,9 +29,7 @@ spec:
         {{- end }}
     spec:
       {{- include "kubeapps.imagePullSecrets" . | indent 6 }}
-       {{- if .Values.kubeappsapis.unsafeUseDemoSA }}
       serviceAccountName: {{ template "kubeapps.kubeappsapis.fullname" . }}
-      {{- end }}
       {{- if .Values.kubeappsapis.hostAliases }}
       hostAliases: {{- include "common.tplvalues.render" (dict "value" .Values.kubeappsapis.hostAliases "context" $) | nindent 8 }}
       {{- end }}

chart/kubeapps/templates/kubeappsapis/rbac.yaml

@@ -1,10 +1,10 @@
 {{- if .Values.featureFlags.kubeappsAPIsServer }}
 {{- if .Values.rbac.create -}}
+{{- if .Values.kubeappsapis.unsafeUseDemoSA }}
 apiVersion: {{ include "common.capabilities.rbac.apiVersion" . }}
 kind: ClusterRole
 metadata:
   name: "kubeapps:controller:kubeapps-apis-dev-{{ .Release.Namespace }}"
-  namespace: {{ .Release.Namespace | quote }}
   labels: {{- include "common.labels.standard" . | nindent 4 }}
     app.kubernetes.io/component: kubeappsapis
     {{- if .Values.commonLabels }}
@@ -17,29 +17,19 @@ rules:
   - apiGroups:
       - "packageinstalls.packaging.carvel.dev"
       - "packagerepositories.packaging.carvel.dev"
-  # TODO (gfichtenholt) possibly surround this with if "fluxv2 plugin is enabled"
-      - "source.toolkit.fluxcd.io"
-      - "helm.toolkit.fluxcd.io"
     resources: ['*']
     verbs: ['*']
   # So that our dev user is seen as having access to a namespace.
   # We'll need to add rbac for our dev user to install later as well.
   - apiGroups: [""]
     resources: ["secrets"]
     verbs: ["get", "list"]
-  # needed by fluxv2 plug-in to check whether flux CRDs have been installed
-  # TODO (gfichtenholt) possibly surround this with "if fluxv2 plugin is enabled"
-  - apiGroups: ["apiextensions.k8s.io"] 
-    resources: ["customresourcedefinitions"]
-    verbs: ["get", "list"]
-{{- if .Values.kubeappsapis.unsafeUseDemoSA }}
 # Dev-only ClusterRoleBinding to the ServiceAccount
 ---
 apiVersion: {{ include "common.capabilities.rbac.apiVersion" . }}
 kind: ClusterRoleBinding
 metadata:
-  name: "kubeapps:controller:kubeapps-apis-dev-{{ .Release.Namespace }}"
-  namespace: {{ .Release.Namespace | quote }}
+  name: "kubeapps:controller:kubeapps-apis-dev"
   labels: {{- include "common.labels.standard" . | nindent 4 }}
     app.kubernetes.io/component: kubeappsapis
     {{- if .Values.commonLabels }}

chart/kubeapps/templates/kubeappsapis/rbac_fluxv2.yaml

@@ -0,0 +1,47 @@
+{{- if .Values.featureFlags.kubeappsAPIsServer }}
+{{- if has "fluxv2" .Values.kubeappsapis.enabledPlugins }}
+{{- if .Values.rbac.create -}}
+apiVersion: {{ include "common.capabilities.rbac.apiVersion" . }}
+kind: ClusterRole
+metadata:
+  name: "kubeapps:controller:kubeapps-apis-fluxv2-plugin"
+  labels: {{- include "common.labels.standard" . | nindent 4 }}
+    app.kubernetes.io/component: kubeappsapis
+    {{- if .Values.commonLabels }}
+    {{- include "common.tplvalues.render" ( dict "value" .Values.commonLabels "context" . ) | nindent 4 }}
+    {{- end }}
+  {{- if .Values.commonAnnotations }}
+  annotations: {{- include "common.tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }}
+  {{- end }}
+rules:
+  - apiGroups: ["source.toolkit.fluxcd.io"]
+    resources: ["helmrepositories"]
+    verbs: ["get", "list", "watch"]
+  # needed by fluxv2 plug-in to check whether flux CRDs have been installed
+  - apiGroups: ["apiextensions.k8s.io"] 
+    resources: ["customresourcedefinitions"]
+    verbs: ["get", "list"]
+---
+apiVersion: {{ include "common.capabilities.rbac.apiVersion" . }}
+kind: ClusterRoleBinding
+metadata:
+  name: "kubeapps:controller:kubeapps-apis-fluxv2-plugin"
+  labels: {{- include "common.labels.standard" . | nindent 4 }}
+    app.kubernetes.io/component: kubeappsapis
+    {{- if .Values.commonLabels }}
+    {{- include "common.tplvalues.render" ( dict "value" .Values.commonLabels "context" . ) | nindent 4 }}
+    {{- end }}
+  {{- if .Values.commonAnnotations }}
+  annotations: {{- include "common.tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }}
+  {{- end }}
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: "kubeapps:controller:kubeapps-apis-fluxv2-plugin"
+subjects:
+  - kind: ServiceAccount
+    name: {{ template "kubeapps.kubeappsapis.fullname" . }}
+    namespace: {{ .Release.Namespace }}
+{{- end }}
+{{- end }}
+{{- end }}

chart/kubeapps/templates/kubeappsapis/serviceaccount.yaml

@@ -1,5 +1,4 @@
 {{- if .Values.featureFlags.kubeappsAPIsServer }}
-  {{- if .Values.kubeappsapis.unsafeUseDemoSA }}
 apiVersion: v1
 kind: ServiceAccount
 metadata:
@@ -13,5 +12,4 @@ metadata:
   {{- if .Values.commonAnnotations }}
   annotations: {{- include "common.tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }}
   {{- end }}
-  {{- end }}
 {{- end }}

chart/kubeapps/templates/kubeops/rbac.yaml

@@ -56,7 +56,6 @@ apiVersion: {{ include "common.capabilities.rbac.apiVersion" . }}
 kind: ClusterRole
 metadata:
   name: "kubeapps:controller:kubeops-ns-discovery-{{ .Release.Namespace }}"
-  namespace: {{ .Release.Namespace | quote }}
   labels: {{- include "common.labels.standard" . | nindent 4 }}
     app.kubernetes.io/component: kubeops
     {{- if .Values.commonLabels }}
@@ -77,7 +76,6 @@ apiVersion: {{ include "common.capabilities.rbac.apiVersion" . }}
 kind: ClusterRoleBinding
 metadata:
   name: "kubeapps:controller:kubeops-ns-discovery-{{ .Release.Namespace }}"
-  namespace: {{ .Release.Namespace | quote }}
   labels: {{- include "common.labels.standard" . | nindent 4 }}
     app.kubernetes.io/component: kubeops
     {{- if .Values.commonLabels }}
@@ -100,7 +98,6 @@ apiVersion: {{ include "common.capabilities.rbac.apiVersion" . }}
 kind: ClusterRole
 metadata:
   name: "kubeapps:controller:kubeops-operators-{{ .Release.Namespace }}"
-  namespace: {{ .Release.Namespace | quote }}
   labels: {{- include "common.labels.standard" . | nindent 4 }}
     app.kubernetes.io/component: kubeops
     {{- if .Values.commonLabels }}
@@ -121,7 +118,6 @@ apiVersion: {{ include "common.capabilities.rbac.apiVersion" . }}
 kind: ClusterRoleBinding
 metadata:
   name: "kubeapps:controller:kubeops-operators-{{ .Release.Namespace }}"
-  namespace: {{ .Release.Namespace | quote }}
   labels: {{- include "common.labels.standard" . | nindent 4 }}
     app.kubernetes.io/component: kubeops
     {{- if .Values.commonLabels }}

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

@@ -176,14 +176,14 @@ func (c NamespacedResourceWatcherCache) isGvrValid() error {
 	}
 
 	name := fmt.Sprintf("%s.%s", c.config.gvr.Resource, c.config.gvr.Group)
-	crd, err := apiExt.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, name, metav1.GetOptions{})
-	if err != nil {
+	if crd, err := apiExt.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, name, metav1.GetOptions{}); err != nil {
 		return err
-	}
-	for _, condition := range crd.Status.Conditions {
-		if condition.Type == apiextv1.Established &&
-			condition.Status == apiextv1.ConditionTrue {
-			return nil
+	} else {
+		for _, condition := range crd.Status.Conditions {
+			if condition.Type == apiextv1.Established &&
+				condition.Status == apiextv1.ConditionTrue {
+				return nil
+			}
 		}
 	}
 	return status.Errorf(codes.FailedPrecondition, "CRD [%s] is not valid", c.config.gvr)
@@ -246,12 +246,10 @@ func (c NamespacedResourceWatcherCache) resync() (string, error) {
 		return "", status.Errorf(codes.FailedPrecondition, "unable to get client due to: %v", err)
 	}
 
-	// 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 code runs in the background, i.e. not in a context of any specific user request.
+	// As such, it requires RBAC to be set up properly during install to be able to list specified GVR
+	// (e.g. flux CRDs). For further details, see https://github.com/kubeapps/kubeapps/pull/3551 and
+	// see helm chart templates/kubeappsapis/rbac_fluxv2.yaml
 
 	// Notice, we are not setting resourceVersion in ListOptions, which means
 	// per https://kubernetes.io/docs/reference/using-api/api-concepts/

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

@@ -107,14 +107,17 @@ func (s *Server) getChartTarball(ctx context.Context, repoName string, chartName
 	// this model toward this proposal
 	// https://github.com/fluxcd/flux2/blob/1c5a25313561771d585c4192d7f330b45753cd99/docs/proposals/secure-impersonation.md
 	// So we may not necessarily want to follow what flux does today
-	unstructuredChart := newFluxHelmChart(chartName, repoName, chartVersion)
+	unstructuredChart, err := newFluxHelmChart(chartName, repoName, chartVersion)
+	if err != nil {
+		return "", err, nil
+	}
 
 	resourceIfc, err := s.getChartsResourceInterface(ctx, namespace)
 	if err != nil {
 		return "", err, nil
 	}
 
-	newChart, err := resourceIfc.Create(ctx, &unstructuredChart, metav1.CreateOptions{})
+	newChart, err := resourceIfc.Create(ctx, unstructuredChart, metav1.CreateOptions{})
 	if err != nil {
 		log.Errorf("Error creating chart: %v\n%v", err, unstructuredChart)
 		return "", err, nil
@@ -459,7 +462,7 @@ func filterAndPaginateCharts(filters *corev1.FilterOptions, pageSize int32, page
 	return summaries, nil
 }
 
-func newFluxHelmChart(chartName, repoName, version string) unstructured.Unstructured {
+func newFluxHelmChart(chartName, repoName, version string) (*unstructured.Unstructured, error) {
 	unstructuredChart := unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"apiVersion": fmt.Sprintf("%s/%s", fluxGroup, fluxVersion),
@@ -478,9 +481,11 @@ func newFluxHelmChart(chartName, repoName, version string) unstructured.Unstruct
 		},
 	}
 	if version != "" {
-		unstructured.SetNestedField(unstructuredChart.Object, version, "spec", "version")
+		if err := unstructured.SetNestedField(unstructuredChart.Object, version, "spec", "version"); err != nil {
+			return nil, err
+		}
 	}
-	return unstructuredChart
+	return &unstructuredChart, nil
 }
 
 func availablePackageDetailFromTarball(chartID, tarUrl string) (*corev1.AvailablePackageDetail, error) {

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

@@ -26,6 +26,7 @@ import (
 	plugins "github.com/kubeapps/kubeapps/cmd/kubeapps-apis/gen/core/plugins/v1alpha1"
 	fluxplugin "github.com/kubeapps/kubeapps/cmd/kubeapps-apis/gen/plugins/fluxv2/packages/v1alpha1"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/metadata"
 	kubecorev1 "k8s.io/api/core/v1"
 	kuberbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -224,22 +225,56 @@ func kubeGetPodNames(t *testing.T, namespace string) (names []string, err error)
 	}
 }
 
-// will create a service account with cluster-admin privs
-func kubeCreateServiceAccount(t *testing.T, name, namespace string) error {
+// will create a service account with cluster-admin privs and return the associated
+// Bearer token (base64-encoded)
+func kubeCreateAdminServiceAccount(t *testing.T, name, namespace string) (string, error) {
 	t.Logf("+kubeCreateServiceAccount(%s,%s)", name, namespace)
-	if typedClient, err := kubeGetTypedClient(); err != nil {
-		return err
-	} else if _, err = typedClient.CoreV1().ServiceAccounts(namespace).Create(
+	typedClient, err := kubeGetTypedClient()
+	if err != nil {
+		return "", err
+	}
+	_, err = typedClient.CoreV1().ServiceAccounts(namespace).Create(
 		context.TODO(),
 		&kubecorev1.ServiceAccount{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      name,
 				Namespace: namespace,
 			},
 		},
-		metav1.CreateOptions{}); err != nil {
-		return err
-	} else if _, err = typedClient.RbacV1().ClusterRoleBindings().Create(
+		metav1.CreateOptions{})
+	if err != nil {
+		return "", err
+	}
+
+	secretName := ""
+	for i := 0; i < 10; i++ {
+		svcAccount, err := typedClient.CoreV1().ServiceAccounts(namespace).Get(context.TODO(), name, metav1.GetOptions{})
+		if err != nil {
+			return "", err
+		}
+		if len(svcAccount.Secrets) >= 1 && svcAccount.Secrets[0].Name != "" {
+			secretName = svcAccount.Secrets[0].Name
+			break
+		}
+		t.Logf("Waiting 1s for service account [%s] secret...", name)
+		time.Sleep(1 * time.Second)
+	}
+	if secretName == "" {
+		return "", fmt.Errorf("Service account [%s] has no secrets", name)
+	}
+
+	secret, err := typedClient.CoreV1().Secrets(namespace).Get(
+		context.TODO(),
+		secretName,
+		metav1.GetOptions{})
+	if err != nil {
+		return "", err
+	}
+	token := secret.Data["token"]
+	if token == nil {
+		return "", err
+	}
+	_, err = typedClient.RbacV1().ClusterRoleBindings().Create(
 		context.TODO(),
 		&kuberbacv1.ClusterRoleBinding{
 			ObjectMeta: metav1.ObjectMeta{
@@ -257,10 +292,11 @@ func kubeCreateServiceAccount(t *testing.T, name, namespace string) error {
 				Name: "cluster-admin",
 			},
 		},
-		metav1.CreateOptions{}); err != nil {
-		return err
+		metav1.CreateOptions{})
+	if err != nil {
+		return "", err
 	}
-	return nil
+	return string(token), nil
 }
 
 func kubeDeleteServiceAccount(t *testing.T, name, namespace string) error {
@@ -359,6 +395,21 @@ func randSeq(n int) string {
 	return string(b)
 }
 
+func newGrpcContext(t *testing.T, name string) context.Context {
+	token, err := kubeCreateAdminServiceAccount(t, name, "default")
+	if err != nil {
+		t.Fatalf("%+v", err)
+	}
+	t.Cleanup(func() {
+		if err := kubeDeleteServiceAccount(t, name, "default"); err != nil {
+			t.Logf("Failed to delete service account due to [%v]", err)
+		}
+	})
+	return metadata.NewOutgoingContext(
+		context.TODO(),
+		metadata.Pairs("Authorization", "Bearer "+token))
+}
+
 // global vars
 var (
 	dynamicClient dynamic.Interface