Address PR comments.

gravitational · Jan 10, 2025 · ee87f36 · ee87f36
1 parent 1ef1f91
commit ee87f36
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 44 deletions.
diff --git a/lib/kube/proxy/cluster_details.go b/lib/kube/proxy/cluster_details.go
@@ -27,7 +27,6 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
-	"github.com/aws/aws-sdk-go-v2/credentials/stscreds"
 	"github.com/aws/aws-sdk-go-v2/service/eks"
 	"github.com/gravitational/trace"
 	"github.com/jonboulle/clockwork"
@@ -88,8 +87,9 @@ type kubeDetails struct {
 // clusterDetailsConfig contains the configuration for creating a proxied cluster.
 type clusterDetailsConfig struct {
 	// cloudClients is the cloud clients to use for dynamic clusters.
+	cloudClients cloud.Clients
+	// awsCloudClients provides AWS SDK clients.
 	awsCloudClients AWSClientGetter
-	cloudClients    cloud.Clients
 	// kubeCreds is the credentials to use for the cluster.
 	kubeCreds kubeCreds
 	// cluster is the cluster to create a proxied cluster for.
@@ -353,18 +353,13 @@ type EKSClient interface {
 	eks.DescribeClusterAPIClient
 }
 
-// STSClient is the subset of the STS Client interface we use.
-type STSClient interface {
-	stscreds.AssumeRoleAPIClient
-}
-
 // AWSClientGetter is an interface for getting an EKS client and an STS client.
 type AWSClientGetter interface {
 	awsconfig.Provider
 	// GetAWSEKSClient returns AWS EKS client for the specified config.
-	GetAWSEKSClient(aws.Config) (EKSClient, error)
+	GetAWSEKSClient(aws.Config) EKSClient
 	// GetAWSSTSPresignClient returns AWS STS presign client for the specified config.
-	GetAWSSTSPresignClient(aws.Config) (STSPresignClient, error)
+	GetAWSSTSPresignClient(aws.Config) STSPresignClient
 }
 
 // getAWSClientRestConfig creates a dynamicCredsClient that generates returns credentials to EKS clusters.
@@ -379,15 +374,12 @@ func getAWSClientRestConfig(cloudClients AWSClientGetter, clock clockwork.Clock,
 		}
 
 		cfg, err := cloudClients.GetConfig(ctx, region, opts...)
-		if err != nil {
-			return nil, time.Time{}, trace.Wrap(err, "cloudClients.GetConfig")
-		}
-
-		regionalClient, err := cloudClients.GetAWSEKSClient(cfg)
 		if err != nil {
 			return nil, time.Time{}, trace.Wrap(err)
 		}
 
+		regionalClient := cloudClients.GetAWSEKSClient(cfg)
+
 		eksCfg, err := regionalClient.DescribeCluster(ctx, &eks.DescribeClusterInput{
 			Name: aws.String(cluster.GetAWSConfig().Name),
 		})
@@ -405,10 +397,7 @@ func getAWSClientRestConfig(cloudClients AWSClientGetter, clock clockwork.Clock,
 			return nil, time.Time{}, trace.BadParameter("invalid api endpoint for cluster %q", cluster.GetAWSConfig().Name)
 		}
 
-		stsPresignClient, err := cloudClients.GetAWSSTSPresignClient(cfg)
-		if err != nil {
-			return nil, time.Time{}, trace.Wrap(err)
-		}
+		stsPresignClient := cloudClients.GetAWSSTSPresignClient(cfg)
 
 		token, exp, err := kubeutils.GenAWSEKSToken(ctx, stsPresignClient, cluster.GetAWSConfig().Name, clock)
 		if err != nil {

diff --git a/lib/kube/proxy/kube_creds_test.go b/lib/kube/proxy/kube_creds_test.go
@@ -21,7 +21,6 @@ package proxy
 import (
 	"context"
 	"encoding/base64"
-	"errors"
 	"net/url"
 	"strings"
 	"testing"
@@ -56,12 +55,12 @@ type mockEKSClientGetter struct {
 	eksClient        *mockEKSAPI
 }
 
-func (e *mockEKSClientGetter) GetAWSEKSClient(aws.Config) (EKSClient, error) {
-	return e.eksClient, nil
+func (e *mockEKSClientGetter) GetAWSEKSClient(aws.Config) EKSClient {
+	return e.eksClient
 }
 
-func (e *mockEKSClientGetter) GetAWSSTSPresignClient(aws.Config) (kubeutils.STSPresignClient, error) {
-	return e.stsPresignClient, nil
+func (e *mockEKSClientGetter) GetAWSSTSPresignClient(aws.Config) kubeutils.STSPresignClient {
+	return e.stsPresignClient
 }
 
 type mockSTSPresignAPI struct {
@@ -80,11 +79,8 @@ type mockEKSAPI struct {
 }
 
 func (m *mockEKSAPI) ListClusters(ctx context.Context, req *eks.ListClustersInput, _ ...func(*eks.Options)) (*eks.ListClustersOutput, error) {
-	defer func() {
-		if m.notify != nil {
-			m.notify <- struct{}{}
-		}
-	}()
+	defer func() { m.notify <- struct{}{} }()
+
 	var names []string
 	for _, cluster := range m.clusters {
 		names = append(names, aws.ToString(cluster.Name))
@@ -95,19 +91,16 @@ func (m *mockEKSAPI) ListClusters(ctx context.Context, req *eks.ListClustersInpu
 }
 
 func (m *mockEKSAPI) DescribeCluster(_ context.Context, req *eks.DescribeClusterInput, _ ...func(*eks.Options)) (*eks.DescribeClusterOutput, error) {
-	defer func() {
-		if m.notify != nil {
-			m.notify <- struct{}{}
-		}
-	}()
+	defer func() { m.notify <- struct{}{} }()
+
 	for _, cluster := range m.clusters {
 		if aws.ToString(cluster.Name) == aws.ToString(req.Name) {
 			return &eks.DescribeClusterOutput{
 				Cluster: cluster,
 			}, nil
 		}
 	}
-	return nil, errors.New("cluster not found")
+	return nil, trace.NotFound("cluster %q not found", aws.ToString(req.Name))
 }
 
 // Test_DynamicKubeCreds tests the dynamic kube credrentials generator for

diff --git a/lib/kube/proxy/server.go b/lib/kube/proxy/server.go
@@ -117,13 +117,13 @@ func (f *awsClientsGetter) GetConfig(ctx context.Context, region string, optFns
 	return awsconfig.GetConfig(ctx, region, optFns...)
 }
 
-func (f *awsClientsGetter) GetAWSEKSClient(cfg aws.Config) (EKSClient, error) {
-	return eks.NewFromConfig(cfg), nil
+func (f *awsClientsGetter) GetAWSEKSClient(cfg aws.Config) EKSClient {
+	return eks.NewFromConfig(cfg)
 }
 
-func (f *awsClientsGetter) GetAWSSTSPresignClient(cfg aws.Config) (STSPresignClient, error) {
+func (f *awsClientsGetter) GetAWSSTSPresignClient(cfg aws.Config) STSPresignClient {
 	stsClient := sts.NewFromConfig(cfg)
-	return sts.NewPresignClient(stsClient), nil
+	return sts.NewPresignClient(stsClient)
 }
 
 // CheckAndSetDefaults checks and sets default values

diff --git a/lib/srv/discovery/fetchers/aws-sync/eks.go b/lib/srv/discovery/fetchers/aws-sync/eks.go
@@ -291,7 +291,7 @@ func (a *awsFetcher) fetchAssociatedPolicies(ctx context.Context, eksClient EKSC
 			out, err := p.NextPage(ctx)
 			if err != nil {
 				errs = append(errs, err)
-				continue
+				break
 			}
 			for _, policy := range out.AssociatedAccessPolicies {
 				associatedPolicies = append(associatedPolicies,

diff --git a/lib/srv/discovery/fetchers/eks.go b/lib/srv/discovery/fetchers/eks.go
@@ -70,15 +70,14 @@ type eksFetcher struct {
 
 // EKSClient is the subset of the EKS interface we use in fetchers.
 type EKSClient interface {
-	eks.ListClustersAPIClient
 	eks.DescribeClusterAPIClient
+	eks.ListClustersAPIClient
 
-	DescribeAccessEntry(ctx context.Context, params *eks.DescribeAccessEntryInput, optFns ...func(*eks.Options)) (*eks.DescribeAccessEntryOutput, error)
-
+	AssociateAccessPolicy(ctx context.Context, params *eks.AssociateAccessPolicyInput, optFns ...func(*eks.Options)) (*eks.AssociateAccessPolicyOutput, error)
 	CreateAccessEntry(ctx context.Context, params *eks.CreateAccessEntryInput, optFns ...func(*eks.Options)) (*eks.CreateAccessEntryOutput, error)
-	UpdateAccessEntry(ctx context.Context, params *eks.UpdateAccessEntryInput, optFns ...func(*eks.Options)) (*eks.UpdateAccessEntryOutput, error)
 	DeleteAccessEntry(ctx context.Context, params *eks.DeleteAccessEntryInput, optFns ...func(*eks.Options)) (*eks.DeleteAccessEntryOutput, error)
-	AssociateAccessPolicy(ctx context.Context, params *eks.AssociateAccessPolicyInput, optFns ...func(*eks.Options)) (*eks.AssociateAccessPolicyOutput, error)
+	DescribeAccessEntry(ctx context.Context, params *eks.DescribeAccessEntryInput, optFns ...func(*eks.Options)) (*eks.DescribeAccessEntryOutput, error)
+	UpdateAccessEntry(ctx context.Context, params *eks.UpdateAccessEntryInput, optFns ...func(*eks.Options)) (*eks.UpdateAccessEntryOutput, error)
 }
 
 // STSClient is the subset of the STS interface we use in fetchers.