Skip to content

Commit 253ca08

Browse files
nicolehanjingnicolehanjing
committed
tags: initial implementation of tags
1 parent 7b5dbaf commit 253ca08

File tree

6 files changed

+308
-76
lines changed

6 files changed

+308
-76
lines changed

Diff for: ‎go.mod

+1
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,5 @@ require (
1717
k8s.io/klog/v2 v2.4.0
1818
k8s.io/legacy-cloud-providers v0.20.0
1919
k8s.io/utils v0.0.0-20201110183641-67b214c5f920
20+
sigs.k8s.io/yaml v1.2.0
2021
)

Diff for: ‎pkg/providers/v2/cloud.go

+56-23
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ limitations under the License.
1919
package v2
2020

2121
import (
22+
"errors"
2223
"fmt"
2324
"io"
2425
"io/ioutil"
@@ -31,8 +32,10 @@ import (
3132
"github.com/aws/aws-sdk-go/aws/session"
3233
"github.com/aws/aws-sdk-go/service/ec2"
3334

35+
"k8s.io/apimachinery/pkg/util/validation/field"
3436
cloudprovider "k8s.io/cloud-provider"
3537
"k8s.io/cloud-provider-aws/pkg/apis/config/v1alpha1"
38+
"k8s.io/klog/v2"
3639
"sigs.k8s.io/yaml"
3740
)
3841

@@ -43,7 +46,12 @@ func init() {
4346
return nil, fmt.Errorf("failed to read AWS cloud provider config file: %v", err)
4447
}
4548

46-
return newCloud(*cfg)
49+
errs := validateAWSCloudConfig(cfg)
50+
if len(errs) > 0 {
51+
return nil, fmt.Errorf("failed to validate AWS cloud config: %v", errs.ToAggregate())
52+
}
53+
54+
return newCloud(cfg)
4755
})
4856
}
4957

@@ -61,7 +69,7 @@ type cloud struct {
6169
region string
6270
ec2 EC2
6371
metadata EC2Metadata
64-
tagging awsTagging
72+
tags awsTagging
6573
}
6674

6775
// EC2 is an interface defining only the methods we call from the AWS EC2 SDK.
@@ -79,7 +87,7 @@ type EC2Metadata interface {
7987

8088
func readAWSCloudConfig(config io.Reader) (*v1alpha1.AWSCloudConfig, error) {
8189
if config == nil {
82-
return nil, fmt.Errorf("no AWS cloud provider config file given")
90+
return nil, errors.New("no AWS cloud provider config file given")
8391
}
8492

8593
// read the config file
@@ -94,13 +102,35 @@ func readAWSCloudConfig(config io.Reader) (*v1alpha1.AWSCloudConfig, error) {
94102
// we got an error where the decode wasn't related to a missing type
95103
return nil, err
96104
}
97-
if cfg.Kind != "AWSCloudConfig" {
98-
return nil, fmt.Errorf("invalid cloud configuration object %q", cfg.Kind)
99-
}
100105

101106
return &cfg, nil
102107
}
103108

109+
// validateAWSCloudConfig validates AWSCloudConfig
110+
// clusterName is required
111+
func validateAWSCloudConfig(config *v1alpha1.AWSCloudConfig) field.ErrorList {
112+
allErrs := field.ErrorList{}
113+
114+
if config.Kind != "AWSCloudConfig" {
115+
allErrs = append(allErrs, field.Invalid(field.NewPath("Kind"), "invalid Kind for cloud config: %q", config.Kind))
116+
}
117+
118+
if config.APIVersion != "config.aws.io/v1alpha1" {
119+
allErrs = append(allErrs, field.Invalid(field.NewPath("APIVersion"), "invalid APIVersion for cloud config: %q", config.APIVersion))
120+
}
121+
122+
fieldPath := field.NewPath("Config")
123+
if config.Config == (v1alpha1.AWSConfig{}) {
124+
allErrs = append(allErrs, field.Required(fieldPath, "Config is required, should not be empty"))
125+
}
126+
127+
if len(config.Config.ClusterName) == 0 {
128+
allErrs = append(allErrs, field.Required(fieldPath.Child("ClusterName"), "cluster name cannot be empty"))
129+
}
130+
131+
return allErrs
132+
}
133+
104134
func getAvailabilityZone(metadata EC2Metadata) (string, error) {
105135
return metadata.GetMetadata("placement/availability-zone")
106136
}
@@ -122,7 +152,7 @@ func azToRegion(az string) (string, error) {
122152
}
123153

124154
// newCloud creates a new instance of AWSCloud.
125-
func newCloud(cfg v1alpha1.AWSCloudConfig) (cloudprovider.Interface, error) {
155+
func newCloud(cfg *v1alpha1.AWSCloudConfig) (cloudprovider.Interface, error) {
126156
sess, err := session.NewSession(&aws.Config{})
127157
if err != nil {
128158
return nil, fmt.Errorf("unable to initialize AWS session: %v", err)
@@ -152,11 +182,6 @@ func newCloud(cfg v1alpha1.AWSCloudConfig) (cloudprovider.Interface, error) {
152182
return nil, err
153183
}
154184

155-
instances, err := newInstances(az, creds)
156-
if err != nil {
157-
return nil, err
158-
}
159-
160185
ec2Sess, err := session.NewSession(&aws.Config{
161186
Region: aws.String(region),
162187
Credentials: creds,
@@ -170,21 +195,29 @@ func newCloud(cfg v1alpha1.AWSCloudConfig) (cloudprovider.Interface, error) {
170195
return nil, fmt.Errorf("error creating AWS ec2 client: %q", err)
171196
}
172197

173-
awsCloud := &cloud{
174-
creds: creds,
175-
instances: instances,
176-
region: region,
177-
metadata: metadataClient,
178-
ec2: ec2Service,
179-
}
180-
198+
var tags awsTagging
181199
if cfg.Config.ClusterName != "" {
182-
if err := awsCloud.tagging.init(cfg.Config.ClusterName); err != nil {
200+
tags, err = newAWSTags(cfg.Config.ClusterName)
201+
if err != nil {
183202
return nil, err
184203
}
204+
} else {
205+
klog.Warning("misconfigured cluster: no clusterName")
185206
}
186207

187-
return awsCloud, nil
208+
instances, err := newInstances(az, creds, tags)
209+
if err != nil {
210+
return nil, err
211+
}
212+
213+
return &cloud{
214+
creds: creds,
215+
instances: instances,
216+
region: region,
217+
metadata: metadataClient,
218+
ec2: ec2Service,
219+
tags: tags,
220+
}, nil
188221
}
189222

190223
// Initialize passes a Kubernetes clientBuilder interface to the cloud provider
@@ -223,7 +256,7 @@ func (c *cloud) Routes() (cloudprovider.Routes, bool) {
223256

224257
// HasClusterID returns true if the cluster has a clusterID
225258
func (c *cloud) HasClusterID() bool {
226-
return len(c.tagging.clusterName()) > 0
259+
return len(c.tags.clusterName()) > 0
227260
}
228261

229262
// InstancesV2 is an implementation for instances and should only be implemented by external cloud providers.

Diff for: ‎pkg/providers/v2/cloud_test.go

+176
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,14 @@ limitations under the License.
1919
package v2
2020

2121
import (
22+
"bytes"
23+
"reflect"
2224
"testing"
2325

2426
"github.com/stretchr/testify/assert"
27+
28+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
29+
"k8s.io/cloud-provider-aws/pkg/apis/config/v1alpha1"
2530
)
2631

2732
func TestAzToRegion(t *testing.T) {
@@ -44,3 +49,174 @@ func TestAzToRegion(t *testing.T) {
4449
assert.Equal(t, testCase.region, ret)
4550
}
4651
}
52+
53+
func TestReadAWSCloudConfig(t *testing.T) {
54+
testcases := []struct {
55+
name string
56+
configData string
57+
config *v1alpha1.AWSCloudConfig
58+
expectErr bool
59+
}{
60+
{
61+
name: "config with valid cluster name",
62+
configData: `---
63+
kind: AWSCloudConfig
64+
apiVersion: config.aws.io/v1alpha1
65+
config:
66+
clusterName: test
67+
`,
68+
config: &v1alpha1.AWSCloudConfig{
69+
TypeMeta: metav1.TypeMeta{
70+
Kind: "AWSCloudConfig",
71+
APIVersion: "config.aws.io/v1alpha1",
72+
},
73+
Config: v1alpha1.AWSConfig{
74+
ClusterName: "test",
75+
},
76+
},
77+
},
78+
{
79+
name: "config with empty cluster name",
80+
configData: `---
81+
kind: AWSCloudConfig
82+
apiVersion: config.aws.io/v1alpha1
83+
config:
84+
clusterName: ""
85+
`,
86+
config: &v1alpha1.AWSCloudConfig{
87+
TypeMeta: metav1.TypeMeta{
88+
Kind: "AWSCloudConfig",
89+
APIVersion: "config.aws.io/v1alpha1",
90+
},
91+
Config: v1alpha1.AWSConfig{
92+
ClusterName: "",
93+
},
94+
},
95+
},
96+
{
97+
name: "config with only kind and apiVersion",
98+
configData: `---
99+
kind: AWSCloudConfig
100+
apiVersion: config.aws.io/v1alpha1
101+
`,
102+
config: &v1alpha1.AWSCloudConfig{
103+
TypeMeta: metav1.TypeMeta{
104+
Kind: "AWSCloudConfig",
105+
APIVersion: "config.aws.io/v1alpha1",
106+
},
107+
Config: v1alpha1.AWSConfig{
108+
ClusterName: "",
109+
},
110+
},
111+
},
112+
{
113+
name: "config with wrong Kind",
114+
configData: `---
115+
kind: WrongCloudConfig
116+
apiVersion: config.aws.io/v1alpha1
117+
config:
118+
clusterName: test
119+
`,
120+
config: nil,
121+
expectErr: true,
122+
},
123+
{
124+
name: "config with wrong apiversion",
125+
configData: `---
126+
kind: AWSCloudConfig
127+
apiVersion: wrong.aws.io/v1alpha1
128+
config:
129+
clusterName: test
130+
`,
131+
config: nil,
132+
expectErr: true,
133+
},
134+
}
135+
136+
for _, testcase := range testcases {
137+
t.Run(testcase.name, func(t *testing.T) {
138+
buffer := bytes.NewBufferString(testcase.configData)
139+
cloudConfig, err := readAWSCloudConfig(buffer)
140+
if err != nil && !testcase.expectErr {
141+
t.Fatal(err)
142+
}
143+
144+
if err == nil && testcase.expectErr {
145+
t.Error("expected error but got none")
146+
}
147+
148+
if !reflect.DeepEqual(cloudConfig, testcase.config) {
149+
t.Logf("actual cloud config: %#v", cloudConfig)
150+
t.Logf("expected cloud config: %#v", testcase.config)
151+
t.Error("AWS cloud config did not match")
152+
}
153+
})
154+
}
155+
}
156+
157+
func TestValidateAWSCloudConfig(t *testing.T) {
158+
testcases := []struct {
159+
name string
160+
config *v1alpha1.AWSCloudConfig
161+
expectErr bool
162+
}{
163+
{
164+
name: "valid config",
165+
config: &v1alpha1.AWSCloudConfig{
166+
TypeMeta: metav1.TypeMeta{
167+
Kind: "AWSCloudConfig",
168+
APIVersion: "config.aws.io/v1alpha1",
169+
},
170+
Config: v1alpha1.AWSConfig{
171+
ClusterName: "test",
172+
},
173+
},
174+
},
175+
{
176+
name: "empty cluster name",
177+
config: &v1alpha1.AWSCloudConfig{
178+
TypeMeta: metav1.TypeMeta{
179+
Kind: "AWSCloudConfig",
180+
APIVersion: "config.aws.io/v1alpha1",
181+
},
182+
Config: v1alpha1.AWSConfig{
183+
ClusterName: "",
184+
},
185+
},
186+
expectErr: true,
187+
},
188+
{
189+
name: "empty config",
190+
config: &v1alpha1.AWSCloudConfig{
191+
TypeMeta: metav1.TypeMeta{
192+
Kind: "AWSCloudConfig",
193+
APIVersion: "config.aws.io/v1alpha1",
194+
},
195+
Config: v1alpha1.AWSConfig{},
196+
},
197+
expectErr: true,
198+
},
199+
{
200+
name: "invalid config",
201+
config: &v1alpha1.AWSCloudConfig{
202+
TypeMeta: metav1.TypeMeta{
203+
Kind: "AWSCloudConfig",
204+
APIVersion: "config.aws.io/v1alpha1",
205+
},
206+
},
207+
expectErr: true,
208+
},
209+
}
210+
211+
for _, testcase := range testcases {
212+
t.Run(testcase.name, func(t *testing.T) {
213+
errs := validateAWSCloudConfig(testcase.config)
214+
215+
if testcase.expectErr && len(errs) == 0 {
216+
t.Errorf("expected error but got none")
217+
} else if !testcase.expectErr && len(errs) > 0 {
218+
t.Errorf("expected no error but received errors: %v", errs.ToAggregate())
219+
}
220+
})
221+
}
222+
}

Diff for: ‎pkg/providers/v2/instances.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import (
3434
)
3535

3636
// newInstances returns an implementation of cloudprovider.InstancesV2
37-
func newInstances(az string, creds *credentials.Credentials) (cloudprovider.InstancesV2, error) {
37+
func newInstances(az string, creds *credentials.Credentials, tags awsTagging) (cloudprovider.InstancesV2, error) {
3838
region, err := azToRegion(az)
3939
if err != nil {
4040
return nil, err
@@ -56,6 +56,7 @@ func newInstances(az string, creds *credentials.Credentials) (cloudprovider.Inst
5656
availabilityZone: az,
5757
ec2: ec2Service,
5858
region: region,
59+
tags: tags,
5960
}, nil
6061
}
6162

@@ -64,6 +65,7 @@ type instances struct {
6465
availabilityZone string
6566
ec2 EC2
6667
region string
68+
tags awsTagging
6769
}
6870

6971
// InstanceExists indicates whether a given node exists according to the cloud provider
@@ -155,6 +157,13 @@ func (i *instances) getInstance(ctx context.Context, node *v1.Node) (*ec2.Instan
155157
klog.V(4).Infof("looking for node by provider ID %v", node.Spec.ProviderID)
156158
}
157159

160+
// TODO: add additional tags from users
161+
tags := i.tags.buildTags(map[string]string{}, ResourceLifecycleOwned)
162+
for tagKey, tagValue := range tags {
163+
request.Filters = append(request.Filters,
164+
newEc2Filter("tag:"+tagKey, tagValue))
165+
}
166+
158167
instances := []*ec2.Instance{}
159168
var nextToken *string
160169
for {

0 commit comments

Comments
 (0)