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

Add EKS (aws) ProviderCluster implementation #24

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add EKS (aws) ProviderCluster implementation #24

wants to merge 1 commit into from

Conversation

sarataha
Copy link
Contributor

@sarataha sarataha commented Nov 21, 2023

This PR adds support for AWS EKS. That includes:

  • ListClusters: Retrieve EKS clusters and kubeconfigs.
  • ClusterID: Returns unique cluster ID given its name.

Copy link
Collaborator

@bigkevmcd bigkevmcd left a comment

Choose a reason for hiding this comment

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

We need to fix the ClusterID bits, we can return nothing here if we can't find something.

pkg/providers/aws/aws.go Outdated Show resolved Hide resolved
pkg/providers/aws/aws.go Outdated Show resolved Hide resolved
pkg/providers/aws/aws.go Outdated Show resolved Hide resolved
pkg/providers/aws/aws.go Outdated Show resolved Hide resolved
pkg/providers/aws/aws.go Outdated Show resolved Hide resolved

var err error
clusters, clusterID := []*providers.ProviderCluster{}, ""
logger := log.FromContext(ctx).WithValues("name", cd.Name, "type", cd.Spec.Type)
Copy link
Collaborator

Choose a reason for hiding this comment

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

add ns here too

Copy link
Collaborator

Choose a reason for hiding this comment

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

Its already included, rm name

@foot foot marked this pull request as ready for review December 7, 2023 10:45
tokenPrefix = "k8s-aws-v1."
tokenAgeMins = 15
clusterNameHeader = "x-k8s-aws-id"
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lift these to the top of the file or keep here?

return azure.NewAzureProvider(acd.Spec.AKS.SubscriptionID), nil
} else if acd.Spec.Type == "eks" {
return aws.NewAWSProvider(acd.Spec.EKS.Region), nil
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Be more defensive here to avoid a panic in case spec.EKS/spec.AKS is not present

This adds support for validating that the configuration for EKS and AKS
clusters is provided when the type is selected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants