Skip to content

tags: initial implementation of tags #149

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

Merged
merged 2 commits into from
Feb 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ require (
k8s.io/klog/v2 v2.4.0
k8s.io/legacy-cloud-providers v0.20.0
k8s.io/utils v0.0.0-20201110183641-67b214c5f920
sigs.k8s.io/yaml v1.2.0
)
99 changes: 87 additions & 12 deletions pkg/providers/v2/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ limitations under the License.
package v2

import (
"errors"
"fmt"
"io"
"io/ioutil"
"regexp"

"github.com/aws/aws-sdk-go/aws"
Expand All @@ -30,12 +32,26 @@ import (
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/ec2"

"k8s.io/apimachinery/pkg/util/validation/field"
cloudprovider "k8s.io/cloud-provider"
awsconfigv1alpha1 "k8s.io/cloud-provider-aws/pkg/apis/config/v1alpha1"
"k8s.io/klog/v2"
"sigs.k8s.io/yaml"
)

func init() {
cloudprovider.RegisterCloudProvider(ProviderName, func(config io.Reader) (cloudprovider.Interface, error) {
return newCloud()
cfg, err := readAWSCloudConfig(config)
if err != nil {
return nil, fmt.Errorf("failed to read AWS cloud provider config file: %v", err)
}

errs := validateAWSCloudConfig(cfg)
Copy link
Member Author

Choose a reason for hiding this comment

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

@andrewsykim updated! PTAL :)

  • added validation function & unit tests
  • update the Instances implementation to use the clusterName tags for API calls

if len(errs) > 0 {
return nil, fmt.Errorf("failed to validate AWS cloud config: %v", errs.ToAggregate())
}

return newCloud(cfg)
})
}

Expand All @@ -53,6 +69,14 @@ type cloud struct {
region string
ec2 EC2
metadata EC2Metadata
tags awsTagging
}

// EC2 is an interface defining only the methods we call from the AWS EC2 SDK.
type EC2 interface {
DescribeInstances(request *ec2.DescribeInstancesInput) (*ec2.DescribeInstancesOutput, error)

CreateTags(*ec2.CreateTagsInput) (*ec2.CreateTagsOutput, error)
}

// EC2Metadata is an abstraction over the AWS metadata service.
Expand All @@ -61,6 +85,48 @@ type EC2Metadata interface {
GetMetadata(path string) (string, error)
}

func readAWSCloudConfig(config io.Reader) (*awsconfigv1alpha1.AWSCloudConfig, error) {
if config == nil {
return nil, errors.New("no AWS cloud provider config file given")
}

// read the config file
data, err := ioutil.ReadAll(config)
if err != nil {
return nil, fmt.Errorf("unable to read cloud configuration from %q [%v]", config, err)
}

var cfg awsconfigv1alpha1.AWSCloudConfig
err = yaml.Unmarshal(data, &cfg)
if err != nil {
// we got an error where the decode wasn't related to a missing type
return nil, err
}

return &cfg, nil
}

// validateAWSCloudConfig validates AWSCloudConfig
// clusterName is required
func validateAWSCloudConfig(config *awsconfigv1alpha1.AWSCloudConfig) field.ErrorList {
allErrs := field.ErrorList{}

if config.Kind != "AWSCloudConfig" {
allErrs = append(allErrs, field.Invalid(field.NewPath("kind"), "invalid kind for cloud config: %q", config.Kind))
}

if config.APIVersion != awsconfigv1alpha1.SchemeGroupVersion.String() {
allErrs = append(allErrs, field.Invalid(field.NewPath("apiVersion"), "invalid apiVersion for cloud config: %q", config.APIVersion))
}

fieldPath := field.NewPath("config")
if len(config.Config.ClusterName) == 0 {
allErrs = append(allErrs, field.Required(fieldPath.Child("clusterName"), "cluster name cannot be empty"))
}

return allErrs
}

func getAvailabilityZone(metadata EC2Metadata) (string, error) {
return metadata.GetMetadata("placement/availability-zone")
}
Expand All @@ -82,7 +148,7 @@ func azToRegion(az string) (string, error) {
}

// newCloud creates a new instance of AWSCloud.
func newCloud() (cloudprovider.Interface, error) {
func newCloud(cfg *awsconfigv1alpha1.AWSCloudConfig) (cloudprovider.Interface, error) {
sess, err := session.NewSession(&aws.Config{})
if err != nil {
return nil, fmt.Errorf("unable to initialize AWS session: %v", err)
Expand Down Expand Up @@ -112,11 +178,6 @@ func newCloud() (cloudprovider.Interface, error) {
return nil, err
}

instances, err := newInstances(az, creds)
if err != nil {
return nil, err
}

ec2Sess, err := session.NewSession(&aws.Config{
Region: aws.String(region),
Credentials: creds,
Expand All @@ -130,15 +191,29 @@ func newCloud() (cloudprovider.Interface, error) {
return nil, fmt.Errorf("error creating AWS ec2 client: %q", err)
}

awsCloud := &cloud{
var tags awsTagging
if cfg.Config.ClusterName != "" {
tags, err = newAWSTags(cfg.Config.ClusterName)
if err != nil {
return nil, err
}
} else {
klog.Warning("misconfigured cluster: no clusterName")
}

instances, err := newInstances(az, creds, tags)
if err != nil {
return nil, err
}

return &cloud{
creds: creds,
instances: instances,
region: region,
metadata: metadataClient,
ec2: ec2Service,
}

return awsCloud, nil
tags: tags,
}, nil
}

// Initialize passes a Kubernetes clientBuilder interface to the cloud provider
Expand Down Expand Up @@ -177,7 +252,7 @@ func (c *cloud) Routes() (cloudprovider.Routes, bool) {

// HasClusterID returns true if the cluster has a clusterID
func (c *cloud) HasClusterID() bool {
return false
return len(c.tags.clusterName()) > 0
}

// InstancesV2 is an implementation for instances and should only be implemented by external cloud providers.
Expand Down
176 changes: 176 additions & 0 deletions pkg/providers/v2/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,14 @@ limitations under the License.
package v2

import (
"bytes"
"reflect"
"testing"

"github.com/stretchr/testify/assert"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/cloud-provider-aws/pkg/apis/config/v1alpha1"
)

func TestAzToRegion(t *testing.T) {
Expand All @@ -44,3 +49,174 @@ func TestAzToRegion(t *testing.T) {
assert.Equal(t, testCase.region, ret)
}
}

func TestReadAWSCloudConfig(t *testing.T) {
testcases := []struct {
name string
configData string
config *v1alpha1.AWSCloudConfig
expectErr bool
}{
{
name: "config with valid cluster name",
configData: `---
kind: AWSCloudConfig
apiVersion: config.aws.io/v1alpha1
config:
clusterName: test
`,
config: &v1alpha1.AWSCloudConfig{
TypeMeta: metav1.TypeMeta{
Kind: "AWSCloudConfig",
APIVersion: "config.aws.io/v1alpha1",
},
Config: v1alpha1.AWSConfig{
ClusterName: "test",
},
},
},
{
name: "config with empty cluster name",
configData: `---
kind: AWSCloudConfig
apiVersion: config.aws.io/v1alpha1
config:
clusterName: ""
`,
config: &v1alpha1.AWSCloudConfig{
TypeMeta: metav1.TypeMeta{
Kind: "AWSCloudConfig",
APIVersion: "config.aws.io/v1alpha1",
},
Config: v1alpha1.AWSConfig{
ClusterName: "",
},
},
},
{
name: "config with only kind and apiVersion",
configData: `---
kind: AWSCloudConfig
apiVersion: config.aws.io/v1alpha1
`,
config: &v1alpha1.AWSCloudConfig{
TypeMeta: metav1.TypeMeta{
Kind: "AWSCloudConfig",
APIVersion: "config.aws.io/v1alpha1",
},
Config: v1alpha1.AWSConfig{
ClusterName: "",
},
},
},
{
name: "config with wrong Kind",
configData: `---
kind: WrongCloudConfig
apiVersion: config.aws.io/v1alpha1
config:
clusterName: test
`,
config: nil,
expectErr: true,
},
{
name: "config with wrong apiversion",
configData: `---
kind: AWSCloudConfig
apiVersion: wrong.aws.io/v1alpha1
config:
clusterName: test
`,
config: nil,
expectErr: true,
},
}

for _, testcase := range testcases {
t.Run(testcase.name, func(t *testing.T) {
buffer := bytes.NewBufferString(testcase.configData)
cloudConfig, err := readAWSCloudConfig(buffer)
if err != nil && !testcase.expectErr {
t.Fatal(err)
}

if err == nil && testcase.expectErr {
t.Error("expected error but got none")
}

if !reflect.DeepEqual(cloudConfig, testcase.config) {
t.Logf("actual cloud config: %#v", cloudConfig)
t.Logf("expected cloud config: %#v", testcase.config)
t.Error("AWS cloud config did not match")
}
})
}
}

func TestValidateAWSCloudConfig(t *testing.T) {
testcases := []struct {
name string
config *v1alpha1.AWSCloudConfig
expectErr bool
}{
{
name: "valid config",
config: &v1alpha1.AWSCloudConfig{
TypeMeta: metav1.TypeMeta{
Kind: "AWSCloudConfig",
APIVersion: "config.aws.io/v1alpha1",
},
Config: v1alpha1.AWSConfig{
ClusterName: "test",
},
},
},
{
name: "empty cluster name",
config: &v1alpha1.AWSCloudConfig{
TypeMeta: metav1.TypeMeta{
Kind: "AWSCloudConfig",
APIVersion: "config.aws.io/v1alpha1",
},
Config: v1alpha1.AWSConfig{
ClusterName: "",
},
},
expectErr: true,
},
{
name: "empty config",
config: &v1alpha1.AWSCloudConfig{
TypeMeta: metav1.TypeMeta{
Kind: "AWSCloudConfig",
APIVersion: "config.aws.io/v1alpha1",
},
Config: v1alpha1.AWSConfig{},
},
expectErr: true,
},
{
name: "invalid config",
config: &v1alpha1.AWSCloudConfig{
TypeMeta: metav1.TypeMeta{
Kind: "AWSCloudConfig",
APIVersion: "config.aws.io/v1alpha1",
},
},
expectErr: true,
},
}

for _, testcase := range testcases {
t.Run(testcase.name, func(t *testing.T) {
errs := validateAWSCloudConfig(testcase.config)

if testcase.expectErr && len(errs) == 0 {
t.Errorf("expected error but got none")
} else if !testcase.expectErr && len(errs) > 0 {
t.Errorf("expected no error but received errors: %v", errs.ToAggregate())
}
})
}
}
Loading