Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create an IAM Service Account role without creating/annotating the service account #3042

Closed
Legion2 opened this issue Jan 7, 2021 · 15 comments · Fixed by #3122
Closed

Create an IAM Service Account role without creating/annotating the service account #3042

Legion2 opened this issue Jan 7, 2021 · 15 comments · Fixed by #3122
Assignees
Labels
area/aws-iam kind/feature New feature or request

Comments

@Legion2
Copy link
Contributor

Legion2 commented Jan 7, 2021

What happened?
I used the IAM role name feature to give the iam role generated by eksctl a predictable name. Then I used the IAM role name in a GitOps setup and attached it with the eks.amazonaws.com/role-arn annotation to a service account which is provided by a helm chart.
The service account is generated be the helm chart and only the annotation and the IAM role is provided by me. I don't know the name of the service account generated by the helm chart.

I don't know if this is a bug or feature request but from the description of the PR (#2863) that introduced the feature:

For instance, when following a git-ops workflow it might be necessary to include the serviceaccount annotations for the IAM role ARNs in the manifest repo.

I thought that this is the intended use of the IAM role name feature.

What you expected to happen?
I created a IAM role using the following cluster config.
The config generates two things: the IAM role and a Service Account.
I don't need the service account, I only want to use the IAM role and attach it to the service account generated by the helm chart.

...
iam:
  withOIDC: true
  serviceAccounts:
    - metadata:
        name: some-name
        namespace: default
      roleName: role-for-helm-chart
      attachPolicy: ...

The helm chart is managed the helm.fluxcd.io/v1 HelmRelease crd.
In the values section I added the following:

    serviceAccount:
      annotations:
        eks.amazonaws.com/role-arn: arn:aws:iam::<redacted>:role/role-for-helm-chart

How to reproduce it?

  1. Create a IAM role with a defined name using the eksctl
  2. Attach the eks.amazonaws.com/role-arn annotation to a service account different than the one created by eksctl
  3. see error WebIdentityErr: failed to retrieve credentials caused by: AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity in the application that uses the service account

Anything else we need to know?
#2863 (comment)

Versions
Please paste in the output of these commands:

$ eksctl version
0.36.0-rc.0
$ 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:"windows/amd64"}
Server Version: version.Info{Major:"1", Minor:"18+", GitVersion:"v1.18.9-eks-d1db3c", GitCommit:"d1db3c46e55f95d6a7d3e5578689371318f95ff9", GitTreeState:"clean", BuildDate:"2020-10-20T22:18:07Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"linux/amd64"}
@aclevername
Copy link
Contributor

Thanks for opening the issue @Legion2

You are correct that this will not work. When you create the IAMServiceAccount the role is setup to only work with the specified serviceaccount. This is an intentional security feature, otherwise any service account could use the role created.
https://github.com/weaveworks/eksctl/blame/5145b139fad61106beee3201ae62b6fb2be96abe/pkg/iam/oidc/api.go#L148-L156

I don't know the name of the service account generated by the helm chart.

This is an interesting use case, you're essentially using the create iamserviceaccount simply to create a role, ignoring that it also creates a serviceaccount and applies the annotation. We could add support for removing this security condition via a flag, but I'm not sure we want to advise people to do this as its quite insecure.

@Legion2
Copy link
Contributor Author

Legion2 commented Jan 7, 2021

On solution would be, that eksctl can be configured to only create the IAM role, which is coupled to a specific service account namespace and name, and not manage that service account (create/delete it). Because the service account is managed using the git-ops approach.

I don't know the name of the service account generated by the helm chart.

This can be overcome by looking into the helm chart template, which is not optimal but then required for security reasons.

@aclevername
Copy link
Contributor

On solution would be, that eksctl can be configured to only create the IAM role, which is coupled to a specific service account namespace and name, and not manage that service account (create/delete it). Because the service account is managed using the git-ops approach.

I don't know the name of the service account generated by the helm chart.

This can be overcome by looking into the helm chart template, which is not optimal but then required for security reasons.

If the service account already exists, you can use the --override-existing-serviceaccounts flag to prevent it from creating a service account. In this scenario the command creates the role for a given serviceaccount/namespace & applys the annotation, which is close to what your describing? But this can only be done if the serviceaccount exists, so you can't do this beforehand

@Legion2
Copy link
Contributor Author

Legion2 commented Jan 7, 2021

When using git-ops you simply define manifest files which are applied to the cluster from a repository. You do not want to think about using different cli command to manage the state of the cluster. For git-ops it should not matter if the service account already exists or not. You just want to define that a service account has an iam role assigned.

@aclevername
Copy link
Contributor

When using git-ops you simply define manifest files which are applied to the cluster from a repository. You do not want to think about using different cli command to manage the state of the cluster. For git-ops it should not matter if the service account already exists or not. You just want to define that a service account has an iam role assigned.

I see where your coming from @Legion2

On solution would be, that eksctl can be configured to only create the IAM role, which is coupled to a specific service account namespace and name, and not manage that service account (create/delete it). Because the service account is managed using the git-ops approach.

A flag/field that introduces this behaviour sound reasonable to me. Would you be willing to work on a PR to introduce this functionality 😄 ?

@Legion2
Copy link
Contributor Author

Legion2 commented Jan 7, 2021

Would you be willing to work on a PR to introduce this functionality 😄 ?

I will try. It's my first time implementing something in go. If I have any problems I will let you know.

@aclevername
Copy link
Contributor

Would you be willing to work on a PR to introduce this functionality 😄 ?

I will try. It's my first time implementing something in go. If I have any problems I will let you know.

Fantastic 🎉 Happy to help

@aclevername aclevername added area/aws-iam kind/feature New feature or request and removed kind/bug labels Jan 7, 2021
@aclevername aclevername changed the title Use IAM role with name on any service account Create an IAM Service Account role without creating/annotating the service account Jan 7, 2021
@Legion2
Copy link
Contributor Author

Legion2 commented Jan 7, 2021

We need to talk about the naming. How should the new option of the ClusterIAMServiceAccount be called: RoleOnly, WithoutServiceAccount, CreateServiceAccount, ManageServiceAccount, NoServiceAccount...?

Also should we only allow this option in combination with RoleName?

When deleting the ClusterIAMServiceAccount, currently the service account is also delete (even when it was not created). It uses the alpha.eksctl.io/iamserviceaccount-name tag on the role in the cloud formation stack. When the service account is not managed by eksctl is should also not be deleted. Should we add a new tag to the cfs when the service account should not be deleted?

@aclevername
Copy link
Contributor

We need to talk about the naming. How should the new option of the ClusterIAMServiceAccount be called: RoleOnly, WithoutServiceAccount, CreateServiceAccount, ManageServiceAccount, NoServiceAccount...?

So we need a flag for the cli and a field in the config that represent the intent that: "we are not going to create and/or annotate a service account during creation of this iamserviceaccount". I think RoleOnly/--role-only is reasonable

Also should we only allow this option in combination with RoleName?

I think this functionality is best used when RoleName is specified, but I don't think we necessarily have to restrict it so that both are set.

When deleting the ClusterIAMServiceAccount, currently the service account is also delete (even when it was not created). It uses the alpha.eksctl.io/iamserviceaccount-name tag on the role in the cloud formation stack. When the service account is not managed by eksctl is should also not be deleted. Should we add a new tag to the cfs when the service account should not be deleted?

I think adding the a new tag might be more complexity that needed. How about using the same field/flag RoleOnly/--role-only during deletion?

@Legion2
Copy link
Contributor Author

Legion2 commented Jan 8, 2021

When deleting an iam service account using --only-missing and a config file, we don't have the RoleOnly information, because the iam service account is deleted from the config file before eksctl is called. I think that's because the alpha.eksctl.io/iamserviceaccount-name tag is added to the cfs.

@cPu1
Copy link
Collaborator

cPu1 commented Jan 8, 2021

@Legion2 not all features in eksctl are designed with GitOps in mind, and IRSA is one of them. The IAM role created by eksctl is not only tied to the ServiceAccount (via the role-arn annotation), but also the cluster because it has a trust relationship with the cluster's OIDC provider, which is different for each cluster. Trying to use the same role name across clusters will not work, so if you have a SA annotated with the role ARN created by eksctl in a Git repo managed by GitOps, it will not work on a different cluster.

The IRSA feature is designed to work as a whole, so disabling parts of it can be unintuitive to users (e.g., why does it allow skipping the SA creation but not the role creation?), and deletions and updates can get complicated as you've discovered. I'm leaning towards not adding this feature.

As to your original issue:

The service account is generated be the helm chart and only the annotation and the IAM role is provided by me. I don't know the name of the service account generated by the helm chart.

Helm charts usually allow configuring resource names, so your chart may already support specifying serviceAccount.name.

@aclevername
Copy link
Contributor

@Legion2 not all features in eksctl are designed with GitOps in mind, and IRSA is one of them. The IAM role created by eksctl is not only tied to the ServiceAccount (via the role-arn annotation), but also the cluster because it has a trust relationship with the cluster's OIDC provider, which is different for each cluster. Trying to use the same role name across clusters will not work, so if you have a SA annotated with the role ARN created by eksctl in a Git repo managed by GitOps, it will not work on a different cluster.

I think I took @Legion2 feature request slightly different. I was imaging a setup in which:

  • I've created my cluster using eksctl
  • I want to manage all k8s resources inside the cluster using a gitops approach
  • I want to utilise IAMServiceAccounts

The option to skip service account creation/annotation in combination with the existing roleName feature would enable the above flow.

The IRSA feature is designed to work as a whole, so disabling parts of it can be unintuitive to users (e.g., why does it allow skipping the SA creation but not the role creation?)

I don't think this is a reason not to implement this feature. The default behaviour will remain the same, so the command is still intuitive.

deletions and updates can get complicated as you've discovered.

I think there are some workarounds that could reduce the complexity. For example before deleting a serviceaccount we could check to see who is the manager of the annotation by inspecting the managedFields. If we created the annotation we can delete the SA, otherwise we don't. for example:

apiVersion: v1
kind: ServiceAccount
metadata:
  annotations:
    eks.amazonaws.com/role-arn: arn:aws:iam::<redacted>:role/eksctl-jk-console-addon-iamserviceaccount
  creationTimestamp: "2021-01-07T12:20:10Z"
  managedFields:
  - apiVersion: v1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .: {}
          f:eks.amazonaws.com/role-arn: {}
    manager: eksctl
    operation: Update
    time: "2021-01-07T12:20:10Z"

@Legion2
Copy link
Contributor Author

Legion2 commented Jan 8, 2021

Helm charts usually allow configuring resource names, so your chart may already support specifying serviceAccount.name.

@cPu1 yes, I tried that, but when the service account is created by eksctl, the helm chart can not be deployed because the service account already exists:
dry-run upgrade for comparison failed: rendered manifests contain a new resource that already exists. Unable to continue with update: existing resource conflict: namespace: monitoring, name: grafana-aws-cloudwatch, existing_kind: /v1, Kind=ServiceAccount, new_kind: /v1, Kind=ServiceAccount
Yes we can disable that the helm chart creates the service account, but this does not work in with helm charts that customize the service account. I also don't like the execution order dependency.

The problem is that both (eksctl and helm) claim to be the manager of the service account. It should be clearly defined who is the owner of the service account and can create and delete it.

@aclevername the managedFields for me contains the following manager: eksctl.exe because I use windows. So the manager attribute is not a reliable option to detect who created the annotation.

@Legion2
Copy link
Contributor Author

Legion2 commented Jan 9, 2021

After rethinking the whole situation and considering that IRSA is not build for GitOps, I think there is no simple way to make it fully GitOps compatible. However, the IRSA feature is very important and should be usable with GitOps.

Therefore, the relevant documentation should be updated to reflect how IRSA can be used in a GitOps setup. The pattern would be, to first deploy the service account with the GitOps approach and then use eksctl to create the IAM role and attach it to the service account. The documentation should state that IRSA is not built for GitOps and therefore the annotation is lost when the service account is recreated in the cluster.

We should add an option to the cli (--only-annotations) eksctl create iamserviceaccount -f ... --only-annotations which only recreate the role arn annotation on the existing service accounts. It should not create new roles or service accounts. The option should only include already existing iamserviceaccounts instead of excluding them. Also the command should be idempotent.

@Legion2
Copy link
Contributor Author

Legion2 commented Jan 20, 2021

I found #3037 which conceptually covers the GitOps use case and reconciliation much better that it would be possible here. So instead of trying to implement the GitOps use case here, it should be done with the apply command. However, the need for an onlyRole option is still there even with an apply command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/aws-iam kind/feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants