ECK Helm chart does not use the serviceAccount.create property during the installation

[ismarslomic]

Bug Report

What did you do?
I tried installing ECK Operator with Helm as restricted installation following the official documentation. I tried configuring ECK Operator/Helm to use existing ServiceAccount (created manually) and not create new one during the installation by setting the properties serviceAccount.create:false and serviceAccount.name:"elastic-operator" as described in the values.yaml.

What did you expect to see?
That existing ServiceAccount is being used and that Helm does not try to create one during the installation.

What did you see instead? Under which circumstances?
When running helm install I see that it tries to create new ServiceAccount, but finds out that the resource already exists with the same name and returns validation errors for labels and annotations not being set as expected.

After investigation the ECK Helm chart templates it seems that the serviceAccount.create property has not been used in any if-statements to check if ServiceAccount creation shall be performed or not

Environment

• ECK version: 1.3.0
• Kubernetes information: Cloud, EKS, v1.17

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.3", GitCommit:"1e11e4a2108024935ecfcb2912226cedeafd99df", GitTreeState:"clean", BuildDate:"2020-10-14T12:50:19Z", GoVersion:"go1.15.2", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"17+", GitVersion:"v1.17.12-eks-7684af", GitCommit:"7684af4ac41370dd109ac13817023cb8063e3d45", GitTreeState:"clean", BuildDate:"2020-10-20T22:57:40Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"linux/amd64"}

• values.yaml:

installCRDs: false
managedNamespaces: ["dev"]
createClusterScopedResources: false
webhook:
  enabled: false
config:
  validateStorageClass: false

serviceAccount:
  create: false
  annotations: {}
  name: "elastic-operator"

• Logs:

Error: rendered manifests contain a resource that already exists. Unable to continue with install: ServiceAccount "elastic-operator" in namespace "dev" exists and cannot be imported into the current release: invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "eck-operator"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "dev"

[ismarslomic]

Thanks @charith-elastic ! Will your commit also avoid creating role and rolebinding for existing ServiceAccount?

[charith-elastic]

No. Any particular reason why you want to do that?

[ismarslomic]

For two reasons:

1. semantics - setting the serviceAccount.create to false means in my opionion that you leave account management (including role, rolebinding and related resources) to the user. Otherwise this property should be renamed to more specific meaning

2. we have large global Kubernetes cluster where each team (20+) has their dedicated namespaces with almost all permissions but RBAC policy management is left to the Kubernetes admin. So my idea is that Kubernetes admin creates the SA, Role, Rolebinding ++ and that this setup is used by the Operator.

Another solution might be to extend theteam permissions with ECK specific RBAC permission, so we can leave SA creation to ECK Helm, but in that case we need a list of what these permissions are. Do you know if this is documented somewhere, or place in the code where I can take a look?

[charith-elastic]

The intention of serviceAccount.create is to allow you to reuse an existing account that might have additional role bindings attached to it. We don't support skipping the creation of the operator role binding because it is fundamental to the correct operation of the operator. We assume that the user has full access to the namespace they are installing the operator to -- including the ability to create role bindings. Your situation is a special case that complicates the chart quite a bit so we need to think about that a bit more to see if it is something we want to support in a future version.

The permissions required are not explicitly documented but you can inspect the all-in-one.yaml file or the Helm source to see what they are.