Skip to content

Commit fb56cd9

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

File tree

6 files changed

+307
-79
lines changed

6 files changed

+307
-79
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

+55-26
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"
35-
"k8s.io/cloud-provider-aws/pkg/apis/config/v1alpha1"
37+
awsconfigv1alpha1 "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.
@@ -77,9 +85,9 @@ type EC2Metadata interface {
7785
GetMetadata(path string) (string, error)
7886
}
7987

80-
func readAWSCloudConfig(config io.Reader) (*v1alpha1.AWSCloudConfig, error) {
88+
func readAWSCloudConfig(config io.Reader) (*awsconfigv1alpha1.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
@@ -88,19 +96,37 @@ func readAWSCloudConfig(config io.Reader) (*v1alpha1.AWSCloudConfig, error) {
8896
return nil, fmt.Errorf("unable to read cloud configuration from %q [%v]", config, err)
8997
}
9098

91-
var cfg v1alpha1.AWSCloudConfig
99+
var cfg awsconfigv1alpha1.AWSCloudConfig
92100
err = yaml.Unmarshal(data, &cfg)
93101
if err != nil {
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 *awsconfigv1alpha1.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 != awsconfigv1alpha1.SchemeGroupVersion.String() {
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 len(config.Config.ClusterName) == 0 {
124+
allErrs = append(allErrs, field.Required(fieldPath.Child("clusterName"), "cluster name cannot be empty"))
125+
}
126+
127+
return allErrs
128+
}
129+
104130
func getAvailabilityZone(metadata EC2Metadata) (string, error) {
105131
return metadata.GetMetadata("placement/availability-zone")
106132
}
@@ -122,7 +148,7 @@ func azToRegion(az string) (string, error) {
122148
}
123149

124150
// newCloud creates a new instance of AWSCloud.
125-
func newCloud(cfg v1alpha1.AWSCloudConfig) (cloudprovider.Interface, error) {
151+
func newCloud(cfg *awsconfigv1alpha1.AWSCloudConfig) (cloudprovider.Interface, error) {
126152
sess, err := session.NewSession(&aws.Config{})
127153
if err != nil {
128154
return nil, fmt.Errorf("unable to initialize AWS session: %v", err)
@@ -152,11 +178,6 @@ func newCloud(cfg v1alpha1.AWSCloudConfig) (cloudprovider.Interface, error) {
152178
return nil, err
153179
}
154180

155-
instances, err := newInstances(az, creds)
156-
if err != nil {
157-
return nil, err
158-
}
159-
160181
ec2Sess, err := session.NewSession(&aws.Config{
161182
Region: aws.String(region),
162183
Credentials: creds,
@@ -170,21 +191,29 @@ func newCloud(cfg v1alpha1.AWSCloudConfig) (cloudprovider.Interface, error) {
170191
return nil, fmt.Errorf("error creating AWS ec2 client: %q", err)
171192
}
172193

173-
awsCloud := &cloud{
174-
creds: creds,
175-
instances: instances,
176-
region: region,
177-
metadata: metadataClient,
178-
ec2: ec2Service,
179-
}
180-
194+
var tags awsTagging
181195
if cfg.Config.ClusterName != "" {
182-
if err := awsCloud.tagging.init(cfg.Config.ClusterName); err != nil {
196+
tags, err = newAWSTags(cfg.Config.ClusterName)
197+
if err != nil {
183198
return nil, err
184199
}
200+
} else {
201+
klog.Warning("misconfigured cluster: no clusterName")
202+
}
203+
204+
instances, err := newInstances(az, creds, tags)
205+
if err != nil {
206+
return nil, err
185207
}
186208

187-
return awsCloud, nil
209+
return &cloud{
210+
creds: creds,
211+
instances: instances,
212+
region: region,
213+
metadata: metadataClient,
214+
ec2: ec2Service,
215+
tags: tags,
216+
}, nil
188217
}
189218

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

224253
// HasClusterID returns true if the cluster has a clusterID
225254
func (c *cloud) HasClusterID() bool {
226-
return len(c.tagging.clusterName()) > 0
255+
return len(c.tags.clusterName()) > 0
227256
}
228257

229258
// 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+
}

0 commit comments

Comments
 (0)