ansible/helm - apply the service account (#4653)

**Description of the change:**
- create and bind to a non-default service account
- follow up : kubernetes-sigs/kubebuilder#2070 and #4626

**Motivation for the change:**

- fix: Scorecard docs instructions are wrong now for Ansible/Helm
- rfe: apply the same go behaviour of Go default config for Ansible/Helm
- Reduce the complexities for we address kubernetes-sigs/kubebuilder#2015
- Keep Ansible/Helm/Go aligned (Align Helm/Ansible plugins with the changes made for Golang ( go/v3 ))
Closes: #4644

changelog/fragments/service_account.yaml

@@ -0,0 +1,86 @@
+# entries is a list of entries to include in
+# release notes and/or the migration guide
+entries:
+    - description: >
+          (ansible/v1, helm/v1) Create and bind to a non-default service account ([kubebuilder#2070](https://github.com/kubernetes-sigs/kubebuilder/pull/2070))
+      kind: "addition"
+      breaking: false
+
+        # NOTE: ONLY USE `pull_request_override` WHEN ADDING THIS
+        # FILE FOR A PREVIOUSLY MERGED PULL_REQUEST!
+        #
+        # The generator auto-detects the PR number from the commit
+        # message in which this file was originally added.
+        #
+        # What is the pull request number (without the "#")?
+        # pull_request_override: 0
+
+
+        # Migration can be defined to automatically add a section to
+        # the migration guide. This is required for breaking changes.
+      migration:
+          header: (ansible/v1, helm/v1) Add a `system:controller-manager` ServiceAccount to your project.
+          body: >
+              A non-default ServiceAccount `controller-manager` is scaffolded on `operator-sdk init`,
+              to improve security for operators installed in shared namespaces. To add this ServiceAccount
+              to your project, do the following:
+              ```sh
+              # Create the ServiceAccount.
+              cat <<EOF > config/rbac/service_account.yaml
+              apiVersion: v1
+              kind: ServiceAccount
+              metadata:
+                name: controller-manager
+                namespace: system
+              EOF
+              # Add it to the list of RBAC resources.
+              echo "- service_account.yaml" >> config/rbac/kustomization.yaml
+              # Update all RoleBinding and ClusterRoleBinding subjects that reference the operator's ServiceAccount.
+              find config/rbac -name *_binding.yaml -exec sed -i -E 's/  name: default/  name: controller-manager/g' {} \;
+              # Add the ServiceAccount name to the manager Deployment's spec.template.spec.serviceAccountName.
+              sed -i -E 's/([ ]+)(terminationGracePeriodSeconds:)/\1serviceAccountName: controller-manager\n\1\2/g' config/manager/manager.yaml
+              ```
+              The changes should look like:
+              ```diff
+              # config/manager/manager.yaml
+                         requests:
+                           cpu: 100m
+                           memory: 20Mi
+              +      serviceAccountName: controller-manager
+                     terminationGracePeriodSeconds: 10
+              # config/rbac/auth_proxy_role_binding.yaml
+                 name: proxy-role
+               subjects:
+               - kind: ServiceAccount
+              -  name: default
+              +  name: controller-manager
+                 namespace: system
+              # config/rbac/kustomization.yaml
+               resources:
+              +- service_account.yaml
+               - role.yaml
+               - role_binding.yaml
+               - leader_election_role.yaml
+              # config/rbac/leader_election_role_binding.yaml
+                 name: leader-election-role
+               subjects:
+               - kind: ServiceAccount
+              -  name: default
+              +  name: controller-manager
+                 namespace: system
+              # config/rbac/role_binding.yaml
+                 name: manager-role
+               subjects:
+               - kind: ServiceAccount
+              -  name: default
+              +  name: controller-manager
+                 namespace: system
+              # config/rbac/service_account.yaml
+              +apiVersion: v1
+              +kind: ServiceAccount
+              +metadata:
+              +  name: controller-manager
+              +  namespace: system
+              ```
+
+

internal/plugins/ansible/v1/scaffolds/init.go

@@ -96,7 +96,7 @@ func (s *initScaffolder) Scaffold() error {
 		&rbac.LeaderElectionRoleBinding{},
 		&rbac.ManagerRole{},
 		&rbac.RoleBinding{},
-
+		&rbac.ServiceAccount{},
 		&prometheus.Kustomization{},
 		&prometheus.ServiceMonitor{},
 

internal/plugins/ansible/v1/scaffolds/internal/templates/config/manager/manager.go

@@ -94,5 +94,6 @@ spec:
               port: 6789
             initialDelaySeconds: 5
             periodSeconds: 10
+      serviceAccountName: controller-manager
       terminationGracePeriodSeconds: 10
 `

internal/plugins/ansible/v1/scaffolds/internal/templates/config/rbac/auth_proxy_role_binding.go

@@ -50,6 +50,6 @@ roleRef:
   name: proxy-role
 subjects:
 - kind: ServiceAccount
-  name: default
+  name: controller-manager
   namespace: system
 `

internal/plugins/ansible/v1/scaffolds/internal/templates/config/rbac/kustomization.go

@@ -43,6 +43,12 @@ func (f *Kustomization) SetTemplateDefaults() error {
 }
 
 const kustomizeRBACTemplate = `resources:
+# All RBAC will be applied under this service account in
+# the deployment namespace. You may comment out this resource
+# if your manager will use a service account that exists at
+# runtime. Be sure to update RoleBinding and ClusterRoleBinding
+# subjects if changing service account names.
+- service_account.yaml
 - role.yaml
 - role_binding.yaml
 - leader_election_role.yaml

internal/plugins/ansible/v1/scaffolds/internal/templates/config/rbac/leader_election_role_binding.go

@@ -50,6 +50,6 @@ roleRef:
   name: leader-election-role
 subjects:
 - kind: ServiceAccount
-  name: default
+  name: controller-manager
   namespace: system
 `

internal/plugins/ansible/v1/scaffolds/internal/templates/config/rbac/role_binding.go

@@ -52,6 +52,6 @@ roleRef:
   name: manager-role
 subjects:
 - kind: ServiceAccount
-  name: default
+  name: controller-manager
   namespace: system
 `

internal/plugins/ansible/v1/scaffolds/internal/templates/config/rbac/service_account.go

@@ -0,0 +1,48 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package rbac
+
+import (
+	"path/filepath"
+
+	"sigs.k8s.io/kubebuilder/v3/pkg/model/file"
+)
+
+var _ file.Template = &ServiceAccount{}
+
+// ServiceAccount scaffolds a file that defines the service account the manager is deployed in.
+type ServiceAccount struct {
+	file.TemplateMixin
+}
+
+// SetTemplateDefaults implements file.Template
+func (f *ServiceAccount) SetTemplateDefaults() error {
+	if f.Path == "" {
+		f.Path = filepath.Join("config", "rbac", "service_account.yaml")
+	}
+
+	f.TemplateBody = serviceAccountTemplate
+
+	return nil
+}
+
+const serviceAccountTemplate = `apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: controller-manager
+  namespace: system
+`

internal/plugins/helm/v1/chartutil/testdata/test-chart/templates/_helpers.tpl

@@ -55,9 +55,5 @@ app.kubernetes.io/instance: {{ .Release.Name }}
 Create the name of the service account to use
 */}}
 {{- define "test-chart.serviceAccountName" -}}
-{{- if .Values.serviceAccount.create -}}
-    {{ default (include "test-chart.fullname" .) .Values.serviceAccount.name }}
-{{- else -}}
-    {{ default "default" .Values.serviceAccount.name }}
-{{- end -}}
-{{- end -}}
+{{ default "controller-manager" .Values.serviceAccount.name }}
+{{- end -}}

internal/plugins/helm/v1/scaffolds/init.go

@@ -96,6 +96,7 @@ func (s *initScaffolder) Scaffold() error {
 		&rbac.LeaderElectionRoleBinding{},
 		&rbac.ManagerRole{},
 		&rbac.ManagerRoleBinding{},
+		&rbac.ServiceAccount{},
 		&manager.Kustomization{},
 		&manager.Manager{Image: imageName},
 		&prometheus.Kustomization{},

internal/plugins/helm/v1/scaffolds/internal/templates/config/manager/manager.go

@@ -98,5 +98,6 @@ spec:
           requests:
             cpu: 100m
             memory: 60Mi
+      serviceAccountName: controller-manager
       terminationGracePeriodSeconds: 10
 `

internal/plugins/helm/v1/scaffolds/internal/templates/config/rbac/auth_proxy_role_binding.go

@@ -50,6 +50,6 @@ roleRef:
   name: proxy-role
 subjects:
 - kind: ServiceAccount
-  name: default
+  name: controller-manager
   namespace: system
 `

internal/plugins/helm/v1/scaffolds/internal/templates/config/rbac/kustomization.go

@@ -43,6 +43,12 @@ func (f *Kustomization) SetTemplateDefaults() error {
 }
 
 const kustomizeRBACTemplate = `resources:
+# All RBAC will be applied under this service account in
+# the deployment namespace. You may comment out this resource
+# if your manager will use a service account that exists at
+# runtime. Be sure to update RoleBinding and ClusterRoleBinding
+# subjects if changing service account names.
+- service_account.yaml
 - role.yaml
 - role_binding.yaml
 - leader_election_role.yaml

internal/plugins/helm/v1/scaffolds/internal/templates/config/rbac/leader_election_role_binding.go

@@ -50,6 +50,6 @@ roleRef:
   name: leader-election-role
 subjects:
 - kind: ServiceAccount
-  name: default
+  name: controller-manager
   namespace: system
 `

internal/plugins/helm/v1/scaffolds/internal/templates/config/rbac/manager_role_binding.go

@@ -50,6 +50,6 @@ roleRef:
   name: manager-role
 subjects:
 - kind: ServiceAccount
-  name: default
+  name: controller-manager
   namespace: system
 `

internal/plugins/helm/v1/scaffolds/internal/templates/config/rbac/service_account.go

@@ -0,0 +1,48 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package rbac
+
+import (
+	"path/filepath"
+
+	"sigs.k8s.io/kubebuilder/v3/pkg/model/file"
+)
+
+var _ file.Template = &ServiceAccount{}
+
+// ServiceAccount scaffolds a file that defines the service account the manager is deployed in.
+type ServiceAccount struct {
+	file.TemplateMixin
+}
+
+// SetTemplateDefaults implements file.Template
+func (f *ServiceAccount) SetTemplateDefaults() error {
+	if f.Path == "" {
+		f.Path = filepath.Join("config", "rbac", "service_account.yaml")
+	}
+
+	f.TemplateBody = serviceAccountTemplate
+
+	return nil
+}
+
+const serviceAccountTemplate = `apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: controller-manager
+  namespace: system
+`

test/e2e/ansible/cluster_test.go

@@ -245,49 +245,50 @@ var _ = Describe("Running ansible projects", func() {
 			Eventually(verifyMemcachedPatch, time.Minute, time.Second).Should(Succeed())
 
 			By("granting permissions to access the metrics and read the token")
-			_, err = tc.Kubectl.Command(
-				"create",
-				"clusterrolebinding", metricsClusterRoleBindingName,
+			_, err = tc.Kubectl.Command("create", "clusterrolebinding", metricsClusterRoleBindingName,
 				fmt.Sprintf("--clusterrole=%s-metrics-reader", tc.ProjectName),
-				fmt.Sprintf("--serviceaccount=%s:default", tc.Kubectl.Namespace))
+				fmt.Sprintf("--serviceaccount=%s:%s", tc.Kubectl.Namespace, tc.Kubectl.ServiceAccount))
 			Expect(err).NotTo(HaveOccurred())
 
-			By("getting the token")
-			b64Token, err := tc.Kubectl.Get(
-				true,
-				"secrets",
-				"-o=jsonpath={.items[0].data.token}")
+			By("reading the metrics token")
+			// Filter token query by service account in case more than one exists in a namespace.
+			query := fmt.Sprintf(`{.items[?(@.metadata.annotations.kubernetes\.io/service-account\.name=="%s")].data.token}`,
+				tc.Kubectl.ServiceAccount,
+			)
+			b64Token, err := tc.Kubectl.Get(true, "secrets", "-o=jsonpath="+query)
 			Expect(err).NotTo(HaveOccurred())
 			token, err := base64.StdEncoding.DecodeString(strings.TrimSpace(b64Token))
 			Expect(err).NotTo(HaveOccurred())
-			Expect(token).NotTo(HaveLen(0))
+			Expect(len(token)).To(BeNumerically(">", 0))
 
-			By("creating a pod with curl image")
-			// todo: the flag --generator=run-pod/v1 is deprecated, however, shows that besides
+			By("creating a curl pod")
+			// TODO: the flag --generator=run-pod/v1 is deprecated, however, shows that besides
 			// it should not make any difference and work locally successfully when the flag is removed
-			// CI has been failing and the curl pod is not found when the flag is not used
+			// the test will fail and the curl pod is not found when the flag is not used
 			cmdOpts := []string{
-				"run", "--generator=run-pod/v1", "curl", "--image=curlimages/curl:7.68.0", "--restart=OnFailure", "--",
+				"run", "--generator=run-pod/v1", "curl", "--image=curlimages/curl:7.68.0", "--restart=OnFailure",
+				"--serviceaccount", tc.Kubectl.ServiceAccount, "--",
 				"curl", "-v", "-k", "-H", fmt.Sprintf(`Authorization: Bearer %s`, token),
-				fmt.Sprintf("https://%s-controller-manager-metrics-service.%s.svc:8443/metrics",
-					tc.ProjectName, tc.Kubectl.Namespace),
+				fmt.Sprintf("https://%s-controller-manager-metrics-service.%s.svc:8443/metrics", tc.ProjectName, tc.Kubectl.Namespace),
 			}
 			_, err = tc.Kubectl.CommandInNamespace(cmdOpts...)
 			Expect(err).NotTo(HaveOccurred())
 
-			By("validating the curl pod running as expected")
+			By("validating that the curl pod is running as expected")
 			verifyCurlUp := func() error {
 				// Validate pod status
 				status, err := tc.Kubectl.Get(
 					true,
 					"pods", "curl", "-o", "jsonpath={.status.phase}")
-				Expect(err).NotTo(HaveOccurred())
+				if err != nil {
+					return err
+				}
 				if status != "Completed" && status != "Succeeded" {
 					return fmt.Errorf("curl pod in %s status", status)
 				}
 				return nil
 			}
-			Eventually(verifyCurlUp, 4*time.Minute, time.Second).Should(Succeed())
+			Eventually(verifyCurlUp, 2*time.Minute, time.Second).Should(Succeed())
 
 			By("checking metrics endpoint serving as expected")
 			getCurlLogs := func() string {

test/e2e/ansible/suite_test.go

@@ -56,6 +56,7 @@ var _ = BeforeSuite(func() {
 	tc.Kind = "Memcached"
 	tc.ProjectName = "memcached-operator"
 	tc.Kubectl.Namespace = fmt.Sprintf("%s-system", tc.ProjectName)
+	tc.Kubectl.ServiceAccount = fmt.Sprintf("%s-controller-manager", tc.ProjectName)
 
 	By("copying sample to a temporary e2e directory")
 	Expect(exec.Command("cp", "-r", "../../../testdata/ansible/memcached-operator", tc.Dir).Run()).To(Succeed())

test/e2e/go/cluster_test.go

@@ -154,7 +154,9 @@ var _ = Describe("operator-sdk", func() {
 				status, err := tc.Kubectl.Get(
 					true,
 					"pods", "curl", "-o", "jsonpath={.status.phase}")
-				ExpectWithOffset(1, err).NotTo(HaveOccurred())
+				if err != nil {
+					return err
+				}
 				if status != "Completed" && status != "Succeeded" {
 					return fmt.Errorf("curl pod in %s status", status)
 				}