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.

Sign up for GitHub

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

Jump to bottom

Allow passing an optional ARN when health checking an AWSOIDC integration #46935

Merged
merged 3 commits into from
Oct 28, 2024
Merged

Allow passing an optional ARN when health checking an AWSOIDC integration #46935

merged 3 commits into from
Oct 28, 2024

Conversation

kimlisa
Copy link
Contributor

@kimlisa kimlisa commented Sep 26, 2024 •
edited by marcoandredinis
Loading

This new addition allows us to health check aws oidc integrations before a user creates or edits the integration.

Demo:

curl --data '{"arn":"arn:aws:iam::123456789012:role/MyRole"}' '.../integrations/aws-oidc/my-integration/ping'

{
  "accountId": "123456789012",
  "arn": "arn:aws:sts::123456789012:assumed-role/MyRole/123",
  "userId": "AAA:17290"
}

@GavinFrazar
Copy link
Contributor

Is the check to see if the role ARN exists (or if it's configured to trust the OIDC token) before creating the integration?

@marcoandredinis
Copy link
Contributor

I'll convert it into draft because I want to polish some parts and add tests.

@marcoandredinis marcoandredinis marked this pull request as draft September 27, 2024 17:35
@marcoandredinis
Copy link
Contributor

Is the check to see if the role ARN exists (or if it's configured to trust the OIDC token) before creating the integration?

It checks if it is configured to trust the OIDC token.
So, it will fail when the role does not exist or when the IAM Role's trust relationship does not allow this particular Identity Provider's.

@marcoandredinis marcoandredinis self-assigned this Sep 30, 2024
@marcoandredinis marcoandredinis marked this pull request as ready for review October 15, 2024 14:27
@marcoandredinis marcoandredinis removed their request for review October 15, 2024 14:27
@marcoandredinis marcoandredinis added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v16 labels Oct 16, 2024
GavinFrazar
GavinFrazar approved these changes Oct 21, 2024
View reviewed changes
lib/web/ui/integration.go Outdated Show resolved Hide resolved
@marcoandredinis
Copy link
Contributor

@fspmarshall @camscale Can you please take a look?

camscale
camscale approved these changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@camscale camscale left a comment

Choose a reason for hiding this comment

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

Sorry I didn't review this sooner. When I see a PR with people explicitly chosen to review it, I kind of ignore the bot assignment assuming those others have better context for the review.

api/proto/teleport/integration/v1/awsoidc_service.proto Outdated Show resolved Hide resolved
lib/integrations/awsoidc/clients_test.go Outdated Show resolved Hide resolved
@marcoandredinis
AWS OIDC Ping: allow custom ARN
ccf94fb 
This PR changes the Ping method to accept a custom ARN.

This is meant to be used by WebUI to do a health check for the
integration:
- when creating
- when editing
- when selecting during Discover flows

If the Ping method receives an ARN, it will use that value instead of
using the one stored in the backend.
@marcoandredinis
rename arn to roleArn
d04f832
@marcoandredinis
rename arn to role_arn
32070ae
@marcoandredinis
Copy link
Contributor

@kimlisa After changing some things, would you mind taking a look?
Given that you created the PR, you can't actually approve it but let me know if it looks good and I'll ask for a G1 bot review.

@kimlisa
Copy link
Contributor Author

kimlisa commented Oct 28, 2024

approved

r0mant
r0mant approved these changes Oct 28, 2024
View reviewed changes
Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

Bot.

@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from fspmarshall October 28, 2024 17:08
@marcoandredinis marcoandredinis added this pull request to the merge queue Oct 28, 2024
Merged via the queue into master with commit 02354f8 Oct 28, 2024
42 checks passed
@marcoandredinis marcoandredinis deleted the lisa/marco/arn branch October 28, 2024 17:42
@public-teleport-github-review-bot
Copy link

@kimlisa See the table below for backport results.

Branch Result
branch/v16 Failed
branch/v17 Create PR

@marcoandredinis marcoandredinis mentioned this pull request Oct 28, 2024
Merged
marcoandredinis added a commit that referenced this pull request Oct 28, 2024
@kimlisa @marcoandredinis
Allow passing an optional ARN when health checking an AWSOIDC integra…
953dae4 
…tion (#46935)

* AWS OIDC Ping: allow custom ARN

This PR changes the Ping method to accept a custom ARN.

This is meant to be used by WebUI to do a health check for the
integration:
- when creating
- when editing
- when selecting during Discover flows

If the Ping method receives an ARN, it will use that value instead of
using the one stored in the backend.

* rename arn to roleArn

* rename arn to role_arn

---------

Co-authored-by: Marco Dinis <[email protected]>
@marcoandredinis marcoandredinis mentioned this pull request Oct 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcoandredinis marcoandredinis marcoandredinis left review comments

@camscale camscale camscale approved these changes

@GavinFrazar GavinFrazar GavinFrazar approved these changes

@r0mant r0mant r0mant approved these changes

Assignees

@marcoandredinis marcoandredinis

Labels
backport/branch/v16 backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry size/sm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kimlisa @GavinFrazar @marcoandredinis @r0mant @camscale