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

DeployService: auto upsert IAM Join Token #28537

Merged
merged 3 commits into from
Jul 6, 2023

Conversation

marcoandredinis
Copy link
Contributor

@marcoandredinis marcoandredinis commented Jun 30, 2023

When using the DeployService, the deployed services (database service only for now) will join the Teleport Cluster using the IAM Join Method.

In order to do so, we require an IAM Token that allows the AWS Account ID and ARN of the assumed-role.
Instead of asking the user to create it, we do it for them.

This PR creates or updates the IAM Join Token.

Token created before deploying the service:

ubuntu@ip-172-31-39-120 ~/pg> sudo $HOME/bin/tctl --config teleport.yaml get tokens
kind: token
metadata:
  id: 1688144017037290044
  name: discover-aws-oidc-iam-token
spec:
  allow:
  - aws_account: "278576220453"
    aws_arn: arn:aws:sts::278576220453:assumed-role/MarcoEC2Role/*
  join_method: iam
  roles:
  - Db
version: v2

@marcoandredinis marcoandredinis force-pushed the marco/deployservice_iam_token branch 2 times, most recently from 89567fd to cc3c2e3 Compare July 3, 2023 17:13
@marcoandredinis marcoandredinis marked this pull request as ready for review July 3, 2023 17:18
@github-actions github-actions bot requested review from jakule and tigrato July 3, 2023 17:18
@marcoandredinis marcoandredinis force-pushed the marco/deployservice_iam_token branch 4 times, most recently from 24eac22 to 7a8dc4d Compare July 5, 2023 13:47
@marcoandredinis
Copy link
Contributor Author

@jakule Can you please review this PR when you get a chance?
@tigrato Can you please take another look?

@marcoandredinis marcoandredinis added discover Issues related to Teleport Discover backport/branch/v13 labels Jul 6, 2023
When using the DeployService, the deployed services (database service
only for now) will join the Teleport Cluster using the IAM Join Method.

In order to do so, we require an IAM Token that allows the AWS Account
ID and ARN of the assumed-role.
Instead of asking the user to create it, we do it for them.

This PR creates or updates the IAM Join Token.
}

token, err := clt.GetToken(ctx, req.tokenName)
switch {
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO in this case switch makes the flow very weird. My proposal

if trace.IsNotFound(err) {
    token = ....
    err = nil
}

if err != nil {
    return trace.Wrap(err)
}

...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a bit complex because it has the fallthrough down below
I could switch to a plain if condition.

Given that you approve, I'll merge as is
But feel free to comment again and I'll try to simplify this part

@marcoandredinis marcoandredinis added this pull request to the merge queue Jul 6, 2023
Merged via the queue into master with commit 7ccb7ef Jul 6, 2023
29 checks passed
@marcoandredinis marcoandredinis deleted the marco/deployservice_iam_token branch July 6, 2023 19:50
@public-teleport-github-review-bot

@marcoandredinis See the table below for backport results.

Branch Result
branch/v13 Create PR

ravicious pushed a commit that referenced this pull request Jul 11, 2023
* DeployService: auto upsert IAM Join Token

When using the DeployService, the deployed services (database service
only for now) will join the Teleport Cluster using the IAM Join Method.

In order to do so, we require an IAM Token that allows the AWS Account
ID and ARN of the assumed-role.
Instead of asking the user to create it, we do it for them.

This PR creates or updates the IAM Join Token.

* AccountID is optional when calling DeployService

* dry code when upserting the token
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v13 discover Issues related to Teleport Discover size/lg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants