✨ refactor: cluster scope and service refactor for future EKS support

pkg/cloud/interfaces.go

@@ -0,0 +1,94 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cloud
+
+import (
+	"github.com/go-logr/logr"
+
+	"sigs.k8s.io/cluster-api/util/conditions"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	awsclient "github.com/aws/aws-sdk-go/aws/client"
+
+	infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
+)
+
+// Session represents an AWS session
+type Session interface {
+	Session() awsclient.ConfigProvider
+}
+
+// ScopeUsage is used to indicate which controller is using a scope
+type ScopeUsage interface {
+	// ControllerName returns the name of the controller that created the scope
+	ControllerName() string
+}
+
+// ClusterObject represents a AWS cluster object
+type ClusterObject interface {
+	conditions.Setter
+}
+
+// ClusterScoper is the interface for a cluster scope
+type ClusterScoper interface {
+	logr.Logger
+	Session
+	ScopeUsage
+
+	// Name returns the cluster name.
+	Name() string
+	// Namespace returns the cluster namespace.
+	Namespace() string
+	// Region returns the cluster region.
+	Region() string
+
+	// InfraCluster returns the AWS infrastructure cluster object.
+	InfraCluster() ClusterObject
+
+	// Network returns the cluster network object.
+	Network() *infrav1.Network
+	// VPC returns the cluster VPC.
+	VPC() *infrav1.VPCSpec
+	// Subnets returns the cluster subnets.
+	Subnets() infrav1.Subnets
+	// SetSubnets updates the clusters subnets.
+	SetSubnets(subnets infrav1.Subnets)
+	// CNIIngressRules returns the CNI spec ingress rules.
+	CNIIngressRules() infrav1.CNIIngressRules
+	// SecurityGroups returns the cluster security groups as a map, it creates the map if empty.
+	SecurityGroups() map[infrav1.SecurityGroupRole]infrav1.SecurityGroup
+	// ListOptionsLabelSelector returns a ListOptions with a label selector for clusterName.
+	ListOptionsLabelSelector() client.ListOption
+	// APIServerPort returns the port to use when communicating with the API server.
+	APIServerPort() int32
+	// AdditionalTags returns any tags that you would like to attach to AWS resources. The returned value will never be nil.
+	AdditionalTags() infrav1.Tags
+	// SetFailureDomain sets the infrastructure provider failure domain key to the spec given as input.
+	SetFailureDomain(id string, spec clusterv1.FailureDomainSpec)
+	// Bastion returns the bastion details for the cluster.
+	Bastion() *infrav1.Bastion
+	// SetBastionInstance sets the bastion instance in the status of the cluster.
+	SetBastionInstance(instance *infrav1.Instance)
+	// SSHKeyName returns the SSH key name to use for instances.
+	SSHKeyName() *string
+
+	// PatchObject persists the cluster configuration and status.
+	PatchObject() error
+	// Close closes the current scope persisting the cluster configuration and status.
+	Close() error
+}

pkg/cloud/scope/clients.go

@@ -17,16 +17,79 @@ limitations under the License.
 package scope
 
 import (
+	"github.com/aws/aws-sdk-go/aws/awserr"
+	"github.com/aws/aws-sdk-go/aws/request"
+	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
+	"github.com/aws/aws-sdk-go/service/elb"
 	"github.com/aws/aws-sdk-go/service/elb/elbiface"
+	"github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi"
 	"github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi/resourcegroupstaggingapiiface"
+	"github.com/aws/aws-sdk-go/service/secretsmanager"
 	"github.com/aws/aws-sdk-go/service/secretsmanager/secretsmanageriface"
+
+	"k8s.io/apimachinery/pkg/runtime"
+
+	"sigs.k8s.io/cluster-api-provider-aws/pkg/cloud"
+	awsmetrics "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/metrics"
+	"sigs.k8s.io/cluster-api-provider-aws/pkg/record"
+	"sigs.k8s.io/cluster-api-provider-aws/version"
 )
 
-// AWSClients contains all the aws clients used by the scopes.
-type AWSClients struct {
-	EC2             ec2iface.EC2API
-	ELB             elbiface.ELBAPI
-	SecretsManager  secretsmanageriface.SecretsManagerAPI
-	ResourceTagging resourcegroupstaggingapiiface.ResourceGroupsTaggingAPIAPI
+// NewEC2Client creates a new EC2 API client for a given session
+func NewEC2Client(scopeUser cloud.ScopeUsage, session cloud.Session, target runtime.Object) ec2iface.EC2API {
+	ec2Client := ec2.New(session.Session())
+	ec2Client.Handlers.Build.PushFrontNamed(getUserAgentHandler())
+	ec2Client.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(scopeUser.ControllerName()))
+	ec2Client.Handlers.Complete.PushBack(recordAWSPermissionsIssue(target))
+
+	return ec2Client
+}
+
+// NewELBClient creates a new ELB API client for a given session
+func NewELBClient(scopeUser cloud.ScopeUsage, session cloud.Session, target runtime.Object) elbiface.ELBAPI {
+	elbClient := elb.New(session.Session())
+	elbClient.Handlers.Build.PushFrontNamed(getUserAgentHandler())
+	elbClient.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(scopeUser.ControllerName()))
+	elbClient.Handlers.Complete.PushBack(recordAWSPermissionsIssue(target))
+
+	return elbClient
+}
+
+// NewResourgeTaggingClient creates a new Resource Tagging API client for a given session
+func NewResourgeTaggingClient(scopeUser cloud.ScopeUsage, session cloud.Session, target runtime.Object) resourcegroupstaggingapiiface.ResourceGroupsTaggingAPIAPI {
+	resourceTagging := resourcegroupstaggingapi.New(session.Session())
+	resourceTagging.Handlers.Build.PushFrontNamed(getUserAgentHandler())
+	resourceTagging.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(scopeUser.ControllerName()))
+	resourceTagging.Handlers.Complete.PushBack(recordAWSPermissionsIssue(target))
+
+	return resourceTagging
+}
+
+// NewSecretsManagerClient creates a new Secrets API client for a given session
+func NewSecretsManagerClient(scopeUser cloud.ScopeUsage, session cloud.Session, target runtime.Object) secretsmanageriface.SecretsManagerAPI {
+	secretsClient := secretsmanager.New(session.Session())
+	secretsClient.Handlers.Build.PushFrontNamed(getUserAgentHandler())
+	secretsClient.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(scopeUser.ControllerName()))
+	secretsClient.Handlers.Complete.PushBack(recordAWSPermissionsIssue(target))
+
+	return secretsClient
+}
+
+func recordAWSPermissionsIssue(target runtime.Object) func(r *request.Request) {
+	return func(r *request.Request) {
+		if awsErr, ok := r.Error.(awserr.Error); ok {
+			switch awsErr.Code() {
+			case "AuthFailure", "UnauthorizedOperation", "NoCredentialProviders":
+				record.Warnf(target, awsErr.Code(), "Operation %s failed with a credentials or permission issue", r.Operation.Name)
+			}
+		}
+	}
+}
+
+func getUserAgentHandler() request.NamedHandler {
+	return request.NamedHandler{
+		Name: "capa/user-agent",
+		Fn:   request.MakeAddToUserAgentHandler("aws.cluster.x-k8s.io", version.Get().String()),
+	}
 }

pkg/cloud/scope/cluster.go

@@ -20,33 +20,25 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/aws/aws-sdk-go/aws/awserr"
-	"github.com/aws/aws-sdk-go/aws/request"
-	"github.com/aws/aws-sdk-go/service/ec2"
-	"github.com/aws/aws-sdk-go/service/elb"
-	"github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi"
-	"github.com/aws/aws-sdk-go/service/secretsmanager"
+	awsclient "github.com/aws/aws-sdk-go/aws/client"
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog/klogr"
 	infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3"
-	awsmetrics "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/metrics"
-	"sigs.k8s.io/cluster-api-provider-aws/pkg/record"
-	"sigs.k8s.io/cluster-api-provider-aws/version"
+	"sigs.k8s.io/cluster-api-provider-aws/pkg/cloud"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // ClusterScopeParams defines the input parameters used to create a new Scope.
 type ClusterScopeParams struct {
-	AWSClients
 	Client         client.Client
 	Logger         logr.Logger
 	Cluster        *clusterv1.Cluster
 	AWSCluster     *infrav1.AWSCluster
 	ControllerName string
+	Session        awsclient.ConfigProvider
 }
 
 // NewClusterScope creates a new Scope from the supplied parameters.
@@ -68,77 +60,32 @@ func NewClusterScope(params ClusterScopeParams) (*ClusterScope, error) {
 		return nil, errors.Errorf("failed to create aws session: %v", err)
 	}
 
-	userAgentHandler := request.NamedHandler{
-		Name: "capa/user-agent",
-		Fn:   request.MakeAddToUserAgentHandler("aws.cluster.x-k8s.io", version.Get().String()),
-	}
-
-	if params.AWSClients.EC2 == nil {
-		ec2Client := ec2.New(session)
-		ec2Client.Handlers.Build.PushFrontNamed(userAgentHandler)
-		ec2Client.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(params.ControllerName))
-		ec2Client.Handlers.Complete.PushBack(recordAWSPermissionsIssue(params.AWSCluster))
-		params.AWSClients.EC2 = ec2Client
-	}
-
-	if params.AWSClients.ELB == nil {
-		elbClient := elb.New(session)
-		elbClient.Handlers.Build.PushFrontNamed(userAgentHandler)
-		elbClient.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(params.ControllerName))
-		elbClient.Handlers.Complete.PushBack(recordAWSPermissionsIssue(params.AWSCluster))
-		params.AWSClients.ELB = elbClient
-	}
-
-	if params.AWSClients.ResourceTagging == nil {
-		resourceTagging := resourcegroupstaggingapi.New(session)
-		resourceTagging.Handlers.Build.PushFrontNamed(userAgentHandler)
-		resourceTagging.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(params.ControllerName))
-		resourceTagging.Handlers.Complete.PushBack(recordAWSPermissionsIssue(params.AWSCluster))
-		params.AWSClients.ResourceTagging = resourceTagging
-	}
-
-	if params.AWSClients.SecretsManager == nil {
-		sClient := secretsmanager.New(session)
-		sClient.Handlers.Build.PushFrontNamed(userAgentHandler)
-		sClient.Handlers.CompleteAttempt.PushFront(awsmetrics.CaptureRequestMetrics(params.ControllerName))
-		sClient.Handlers.Complete.PushBack(recordAWSPermissionsIssue(params.AWSCluster))
-		params.AWSClients.SecretsManager = sClient
-	}
-
 	helper, err := patch.NewHelper(params.AWSCluster, params.Client)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to init patch helper")
 	}
 	return &ClusterScope{
-		Logger:      params.Logger,
-		client:      params.Client,
-		AWSClients:  params.AWSClients,
-		Cluster:     params.Cluster,
-		AWSCluster:  params.AWSCluster,
-		patchHelper: helper,
+		Logger:         params.Logger,
+		client:         params.Client,
+		Cluster:        params.Cluster,
+		AWSCluster:     params.AWSCluster,
+		patchHelper:    helper,
+		session:        session,
+		controllerName: params.ControllerName,
 	}, nil
 }
 
-func recordAWSPermissionsIssue(target runtime.Object) func(r *request.Request) {
-	return func(r *request.Request) {
-		if awsErr, ok := r.Error.(awserr.Error); ok {
-			switch awsErr.Code() {
-			case "AuthFailure", "UnauthorizedOperation", "NoCredentialProviders":
-				record.Warnf(target, awsErr.Code(), "Operation %s failed with a credentials or permission issue", r.Operation.Name)
-			}
-		}
-	}
-}
-
 // ClusterScope defines the basic context for an actuator to operate upon.
 type ClusterScope struct {
 	logr.Logger
 	client      client.Client
 	patchHelper *patch.Helper
 
-	AWSClients
 	Cluster    *clusterv1.Cluster
 	AWSCluster *infrav1.AWSCluster
+
+	session        awsclient.ConfigProvider
+	controllerName string
 }
 
 // Network returns the cluster network object.
@@ -156,6 +103,11 @@ func (s *ClusterScope) Subnets() infrav1.Subnets {
 	return s.AWSCluster.Spec.NetworkSpec.Subnets
 }
 
+// SetSubnets updates the clusters subnets.
+func (s *ClusterScope) SetSubnets(subnets infrav1.Subnets) {
+	s.AWSCluster.Spec.NetworkSpec.Subnets = subnets
+}
+
 // CNIIngressRules returns the CNI spec ingress rules.
 func (s *ClusterScope) CNIIngressRules() infrav1.CNIIngressRules {
 	if s.AWSCluster.Spec.NetworkSpec.CNI != nil {
@@ -256,3 +208,36 @@ func (s *ClusterScope) SetFailureDomain(id string, spec clusterv1.FailureDomainS
 	}
 	s.AWSCluster.Status.FailureDomains[id] = spec
 }
+
+// InfraCluster returns the AWS infrastructure cluster object.
+// Initially this will be AWSCluster but in the future it
+// could also be AWSManagedCluster
+func (s *ClusterScope) InfraCluster() cloud.ClusterObject {
+	return s.AWSCluster
+}
+
+// Session returns the AWS SDK session. Used for creating clients
+func (s *ClusterScope) Session() awsclient.ConfigProvider {
+	return s.session
+}
+
+// Bastion returns the bastion details.
+func (s *ClusterScope) Bastion() *infrav1.Bastion {
+	return &s.AWSCluster.Spec.Bastion
+}
+
+// SetBastionInstance sets the bastion instance in the status of the cluster.
+func (s *ClusterScope) SetBastionInstance(instance *infrav1.Instance) {
+	s.AWSCluster.Status.Bastion = instance
+}
+
+// SSHKeyName returns the SSH key name to use for instances.
+func (s *ClusterScope) SSHKeyName() *string {
+	return s.AWSCluster.Spec.SSHKeyName
+}
+
+// ControllerName returns the name of the controller that
+// created the ClusterScope.
+func (s *ClusterScope) ControllerName() string {
+	return s.controllerName
+}

pkg/cloud/scope/machine.go

@@ -38,7 +38,6 @@ import (
 
 // MachineScopeParams defines the input parameters used to create a new MachineScope.
 type MachineScopeParams struct {
-	AWSClients
 	Client     client.Client
 	Logger     logr.Logger
 	Cluster    *clusterv1.Cluster

pkg/cloud/services/ec2/account.go

@@ -26,11 +26,11 @@ import (
 )
 
 func (s *Service) getAvailableZones() ([]string, error) {
-	out, err := s.scope.EC2.DescribeAvailabilityZones(&ec2.DescribeAvailabilityZonesInput{
+	out, err := s.EC2Client.DescribeAvailabilityZones(&ec2.DescribeAvailabilityZonesInput{
 		Filters: []*ec2.Filter{filter.EC2.Available()},
 	})
 	if err != nil {
-		record.Eventf(s.scope.AWSCluster, "FailedDescribeAvailableZone", "Failed getting available zones: %v", err)
+		record.Eventf(s.scope.InfraCluster(), "FailedDescribeAvailableZone", "Failed getting available zones: %v", err)
 		return nil, errors.Wrap(err, "failed to describe availability zones")
 	}
 

pkg/cloud/services/ec2/ami.go

@@ -114,9 +114,9 @@ func (s *Service) defaultAMILookup(amiNameFormat, ownerID, baseOS, kubernetesVer
 		},
 	}
 
-	out, err := s.scope.EC2.DescribeImages(describeImageInput)
+	out, err := s.EC2Client.DescribeImages(describeImageInput)
 	if err != nil {
-		record.Eventf(s.scope.AWSCluster, "FailedDescribeImages", "Failed to find ami %q: %v", amiName, err)
+		record.Eventf(s.scope.InfraCluster(), "FailedDescribeImages", "Failed to find ami %q: %v", amiName, err)
 		return "", errors.Wrapf(err, "failed to find ami: %q", amiName)
 	}
 	if len(out.Images) == 0 {