tofu-controller icon indicating copy to clipboard operation
tofu-controller copied to clipboard

Helm chart: using runner ServiceAccount created by eksctl with imagePullSecrets

Open artem-nefedov opened this issue 1 year ago • 5 comments

eksctl has built-in support for EKS IRSA, where it can create both IAM role and corresponding ServiceAccount in cluster based on defined manifest. With this approach, it makes sense to not create by helm ServiceAccount at all, so you would specify something like this in values:

runner:
  serviceAccount:
    create: false
    name: tf-runner

The problem arises if you also need to use private image registry that requires authentication, so you also need imagePullSecrets. Currently, the chart adds imagePullSecrets to ServiceAccount instead of adding them to a Pod, which makes it hard to use this setup (eksctl doesn't support adding imagePullSecrets to ServiceAccount, so you'll have to perform extra step to patch SA it created).

For reference, every other chart that we use which also require IAM role to operate would always add imagePullSecrets to Pod.

I understand that this change is not as simple as just changing helm chart, because "tf-runner" doesn't exist there as a manifest, but instead generated by the controller at runtime, but I still hope you think about this, especially considering eksctl is also a Weave product.

artem-nefedov avatar Aug 16 '22 11:08 artem-nefedov

Another approach here could be to use annotations to associate the runner service account with your IRSA IAM Role:

runner:
  serviceAccount:
    annotations:
      eks.amazonaws.com/role-arn: [aws-arn-id]

Nalum avatar Aug 16 '22 12:08 Nalum

@Nalum I know. But this approach doesn't work if you want to use eksctl to create IAM roles, because eksctl will always also create a ServiceAccount in this case. And if you try to install helm over existing ServiceAccount without disabling its creation, it will complain about the conflict. You can't just use a different SA name, because eksctl creates trust policy for that specific name in that specific namespace.

artem-nefedov avatar Aug 16 '22 12:08 artem-nefedov

Ah right, sorry I forgot that the IRSA Role needs to have the name of the Service Account set. You could pass the flag --role-only in the eksctl command to not create the serviceaccount and allow the helm chart to do that side of it.

https://eksctl.io/usage/iamserviceaccounts/#how-it-works

By default the service account will be created or updated to include the role annotation, this can be disabled using the flag --role-only.

Edit: Just to note, I'm not saying this won't be done, I think it's probably worth doing but just trying to help you get around the issue.

Nalum avatar Aug 16 '22 13:08 Nalum

@Nalum Thanks for pointing out --role-only flag. There are 2 problems with it:

  1. We use declarative approach, where every IAM ServiceAccount is declared in a single eksctl yaml manifest. And for all other IAM ServiceAccounts listed there, we actually need to have ServiceAccount created. This option doesn't support telling it exactly what SA should be created and what shouldn't, so having a single declarative manifest will no longer work.
  2. This option is only supported by eksctl create iamserviceaccount command. However, we instead create everything in the cluster (including IAM roles/SAs) from a single manifest using a single eksctl create cluster command, which doesn't support it.

artem-nefedov avatar Aug 16 '22 13:08 artem-nefedov

It seems to be a must-fix feature. Let me take a look at it.

chanwit avatar Aug 16 '22 17:08 chanwit

After looking at eksctl schema, I found that ClusterIAMServiceAccount actually supports roleOnly field, so you can define it directly in the yaml manifest. Combined with specifying static role name there, which can then be used in annotation, this is probably good enough as a solution. You may close this ticket if you wish.

artem-nefedov avatar Aug 23 '22 09:08 artem-nefedov