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

Migrate eks discovery to aws sdk v2 #50603

Merged
merged 2 commits into from
Jan 10, 2025
Merged

Migrate eks discovery to aws sdk v2 #50603

merged 2 commits into from
Jan 10, 2025

Conversation

creack
Copy link
Member

@creack creack commented Dec 28, 2024

Closes #49129

Removes all usage of sdk v1 for EKS.

@creack creack changed the title Migrate eks discovery to aws sdk v2 [wip] Migrate eks discovery to aws sdk v2 Dec 28, 2024
@creack creack force-pushed the creack/eks-sdk-v2 branch 4 times, most recently from 2c56eb2 to 8de9fdc Compare December 30, 2024 21:55
@creack creack force-pushed the creack/eks-sdk-v2 branch from 7174944 to d968a0e Compare January 5, 2025 18:35
@creack creack marked this pull request as ready for review January 5, 2025 18:41
@github-actions github-actions bot added database-access Database access related issues and PRs discovery kubernetes-access labels Jan 5, 2025
@github-actions github-actions bot requested a review from greedy52 January 5, 2025 18:41
@github-actions github-actions bot requested a review from tigrato January 5, 2025 18:41
@creack creack added the no-changelog Indicates that a PR does not require a changelog entry label Jan 5, 2025
@creack creack changed the title [wip] Migrate eks discovery to aws sdk v2 Migrate eks discovery to aws sdk v2 Jan 5, 2025
lib/kube/proxy/cluster_details.go Outdated Show resolved Hide resolved
lib/kube/proxy/cluster_details.go Outdated Show resolved Hide resolved
lib/integrations/awsoidc/eks_enroll_clusters.go Outdated Show resolved Hide resolved
lib/kube/proxy/cluster_details.go Outdated Show resolved Hide resolved
Comment on lines 352 to 359
type EKSClient interface {
eks.DescribeClusterAPIClient
}

// STSClient is the subset of the STS Client interface we use.
type STSClient interface {
stscreds.AssumeRoleAPIClient
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If our goal is to expose on those two interfaces, why not referring them directly?
This is mostly internal packages and I don't see them being expanded in the future

Copy link
Member Author

Choose a reason for hiding this comment

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

it is easier to have them split as one is implemented by sts.Client the other by eks.Client, used in different places.

lib/kube/proxy/cluster_details.go Outdated Show resolved Hide resolved
lib/kube/proxy/server.go Outdated Show resolved Hide resolved
lib/kube/utils/eks_token_signed.go Show resolved Hide resolved
lib/srv/discovery/discovery.go Outdated Show resolved Hide resolved
@creack creack force-pushed the creack/eks-sdk-v2 branch from fb8c72e to 1ef1f91 Compare January 9, 2025 23:36
Copy link
Contributor

@GavinFrazar GavinFrazar left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for putting up with all the merge conflicts 😉

lib/kube/proxy/cluster_details.go Outdated Show resolved Hide resolved
lib/kube/proxy/cluster_details.go Outdated Show resolved Hide resolved
lib/kube/proxy/cluster_details.go Outdated Show resolved Hide resolved
lib/kube/proxy/cluster_details.go Outdated Show resolved Hide resolved
lib/kube/proxy/kube_creds_test.go Show resolved Hide resolved
lib/kube/proxy/kube_creds_test.go Outdated Show resolved Hide resolved
lib/kube/proxy/kube_creds_test.go Outdated Show resolved Hide resolved
lib/kube/proxy/kube_creds_test.go Outdated Show resolved Hide resolved
lib/srv/discovery/fetchers/aws-sync/eks.go Outdated Show resolved Hide resolved
lib/srv/discovery/fetchers/eks.go Outdated Show resolved Hide resolved
lib/kube/proxy/cluster_details.go Outdated Show resolved Hide resolved
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from greedy52 January 10, 2025 14:47
@creack creack force-pushed the creack/eks-sdk-v2 branch from ee87f36 to 4039237 Compare January 10, 2025 15:09
@creack creack force-pushed the creack/eks-sdk-v2 branch from 4039237 to 47449d5 Compare January 10, 2025 15:34
@creack creack added this pull request to the merge queue Jan 10, 2025
Merged via the queue into master with commit 84956a8 Jan 10, 2025
41 checks passed
@creack creack deleted the creack/eks-sdk-v2 branch January 10, 2025 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database-access Database access related issues and PRs discovery kubernetes-access no-changelog Indicates that a PR does not require a changelog entry size/md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert EKS discovery code to use aws-sdk-go-v2
4 participants