Skip to content

Commit

Permalink
Responding to second round of code review
Browse files Browse the repository at this point in the history
Refactored NewCloud and NewCloudWithRole to call internal function so
the duplication is factored out.
  • Loading branch information
jonathanrainer committed Jun 8, 2022
1 parent 8f3682c commit e8cf834
Showing 1 changed file with 17 additions and 31 deletions.
48 changes: 17 additions & 31 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,42 +103,22 @@ type cloud struct {
// NewCloud returns a new instance of AWS cloud
// It panics if session is invalid
func NewCloud() (Cloud, error) {
sess := session.Must(session.NewSession(&aws.Config{}))
svc := ec2metadata.New(sess)
api, err := DefaultKubernetesAPIClient()

if err != nil && !isDriverBootedInECS() {
klog.Warning("Could not create Kubernetes Client")
}

metadataProvider, err := GetNewMetadataProvider(svc, api)

if err != nil {
return nil, fmt.Errorf("error creating MetadataProvider: %v", err)
}

metadata, err := metadataProvider.getMetadata()

if err != nil {
return nil, fmt.Errorf("could not get metadata from AWS: %v", err)
}

efsClient := efs.New(session.Must(session.NewSession(aws.NewConfig().WithRegion(metadata.GetRegion()))))
return &cloud{
metadata: metadata,
efs: efsClient,
}, nil
return createCloud("")
}

// NewCloudWithRole returns a new instance of AWS cloud after assuming an aws role
// It panics if driver does not have permissions to assume role.
func NewCloudWithRole(awsRoleArn string) (Cloud, error) {
return createCloud(awsRoleArn)
}

func createCloud(awsRoleArn string) (Cloud, error) {
sess := session.Must(session.NewSession(&aws.Config{}))
svc := ec2metadata.New(sess)
api, err := DefaultKubernetesAPIClient()

if err != nil {
return nil, fmt.Errorf("could not instantiate k8s api client: %v", err)
if err != nil && !isDriverBootedInECS() {
klog.Warningf("Could not create Kubernetes Client: %v", err)
}

metadataProvider, err := GetNewMetadataProvider(svc, api)
Expand All @@ -150,17 +130,23 @@ func NewCloudWithRole(awsRoleArn string) (Cloud, error) {
metadata, err := metadataProvider.getMetadata()

if err != nil {
return nil, fmt.Errorf("Could not get metadata from AWS: %v", err)
return nil, fmt.Errorf("could not get metadata from AWS: %v", err)
}

creds := stscreds.NewCredentials(sess, awsRoleArn)
efsClient := efs.New(session.Must(session.NewSession(aws.NewConfig().WithCredentials(creds).WithRegion(metadata.GetRegion()))))
return &cloud{
metadata: metadata,
efs: efsClient,
efs: createEfsClient(awsRoleArn, metadata, sess),
}, nil
}

func createEfsClient(awsRoleArn string, metadata MetadataService, sess *session.Session) Efs {
config := aws.NewConfig().WithRegion(metadata.GetRegion())
if awsRoleArn != "" {
config = config.WithCredentials(stscreds.NewCredentials(sess, awsRoleArn))
}
return efs.New(session.Must(session.NewSession(config)))
}

func (c *cloud) GetMetadata() MetadataService {
return c.metadata
}
Expand Down

0 comments on commit e8cf834

Please sign in to comment.