Skip to content
This repository was archived by the owner on Nov 5, 2024. It is now read-only.

AWS IAM auth SA name support#1453

Merged
tzununbekov merged 2 commits into
mainfrom
aws-sa-name
Jul 4, 2023
Merged

AWS IAM auth SA name support#1453
tzununbekov merged 2 commits into
mainfrom
aws-sa-name

Conversation

@tzununbekov

@tzununbekov tzununbekov commented Jun 20, 2023

Copy link
Copy Markdown
Member

Initial proposal: #1309

Context

Per Amazon's design, users must create one IAM Role and one Trust Relationship per Service Account (hence, per source instance). This is quite a tedious process, and for some people it is just fine to use a shared serviceAccount for selected sources.

The original PR was not merged because the solution there was based on the k8s object annotations which is not a common approach for this kind of modification in our components.
An additional requirement: provided service account may already exist; in this case, we should not reconcile it to avoid conflicts.

Proposed change

This PR introduces the following AWS Auth spec update (for all AWS components):

  • Deprecate spec.auth.iamRole parameter - all creation requests for AWS components with this attribute in their spec will be rejected by the webhook (and CRD requirements). Existing objects will continue their work,
  • Introduce a new spec.auth.iam object with roleArn and serviceAccount attributes to replace and extend the deprecated iamRole parameter. roleArn expects the value that was previously set in iamRole parameter, and serviceAccount is the Kubernetes service account name to create or re-use for the adapter deployment. At least one of the attributes must be present in the spec.auth.iam object.

Example

AWS Auth spec before:

apiVersion: sources.triggermesh.io/v1alpha1
kind: <AWS source>
metadata:
  name: foo
spec:
  arn: <resource ARN>

  auth:
    iamRole: arn:aws:iam::123456789012:role/dev-role

-service account with a preconfigured name will be created for the source by the controller.

Some options after this PR:

apiVersion: sources.triggermesh.io/v1alpha1
kind: <AWS source>
metadata:
  name: foo
spec:
  arn: <resource ARN>

  auth:
    iam:
      roleArn: arn:aws:iam::123456789012:role/dev-role

-spec format update with no behavior change - SA with the preconfigured name will be created by the controller.

apiVersion: sources.triggermesh.io/v1alpha1
kind: <AWS source>
metadata:
  name: foo
spec:
  arn: <resource ARN>

  auth:
    iam:
      roleArn: arn:aws:iam::123456789012:role/dev-role
      serviceAccount: aws-source-sa

-service account aws-source-sa will be created by the controller. If a service account with this name already exists, the controller checks the labels, and, if the label managed-by has the value triggermesh-controller, SA's owners list is updated with the current object. If the label managed-by does not exist or it has a value different from triggermesh-controller, reconciliation is skipped. Then this SA is assigned to the adapter deployment.
SA's annotation "eks.amazonaws.com/role-arn" can be overwritten by the controller only if SA is managed by the triggermesh controller and there is only one owner to it, otherwise, annotation is ignored to avoid reconciliation conflicts.

apiVersion: sources.triggermesh.io/v1alpha1
kind: <AWS source>
metadata:
  name: foo
spec:
  arn: <resource ARN>

  auth:
    iam:
      serviceAccount: aws-source-sa

-service account aws-source-sa will be created if needed and assigned to the receive adapter deployment. If the service account did not exist before, it will be created without the IAM role annotation (spec.auth.iam.roleArn is not defined) and the source will fail to authenticate against the AWS API.


Bonus 1:
Since this PR updates common reconciliation logic for Kubernetes service accounts, this also affects the GCP components. Custom KSA name support added in #1372 can now be utilized as shared and externally created service account support in GCP sources and targets.

Bonus 2:
This PR introduces a new Make target - codegen-cleanup. In dev environments where the triggermesh project is located outside the GOPATH, make codegen could result in a github.com directory created in the project's root with all the generated files in it. While I failed to find the proper configuration fix for the k8s code generators, make codegen-cleanup can fix the resulting directory tree.

@tzununbekov tzununbekov self-assigned this Jun 20, 2023
@tzununbekov tzununbekov force-pushed the aws-sa-name branch 2 times, most recently from 7d4083c to 1ff4515 Compare June 22, 2023 14:13
@tzununbekov tzununbekov requested a review from a team June 24, 2023 08:16
@tzununbekov tzununbekov marked this pull request as ready for review June 24, 2023 08:17

@FranBarrera FranBarrera left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 (I haven't tested it manually)

@jmcx

jmcx commented Jul 2, 2023

Copy link
Copy Markdown
Contributor

For docs, I'm assuming for now that this has no impact on the spec.auth.credentials.assumeIamRole parameter which remains in the same place. Let me know if that isn't right!

@jmcx

jmcx commented Jul 2, 2023

Copy link
Copy Markdown
Contributor

@tzununbekov your Bonus 1 comment makes it sound like this feature request would be addressed by this PR: #1463

How ever, the comment from the user there indicates that this might only work for sources and not targets? Any thoughts? Perhaps there is still a TODO for GCP targets.

@jmcx

jmcx commented Jul 2, 2023

Copy link
Copy Markdown
Contributor

docs PR triggermesh/docs#394

@tzununbekov tzununbekov merged commit 628e5cb into main Jul 4, 2023
@tzununbekov tzununbekov deleted the aws-sa-name branch July 4, 2023 13:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants