Skip to content

Commit 5a8c4f7

Browse files
deliahudeliahu
authored
Support AWS tags with spaces and valid special characters (#1374)
1 parent 63e084b commit 5a8c4f7

File tree

11 files changed

+239
-26
lines changed

11 files changed

+239
-26
lines changed

‎manager/cluster_config_env.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
def export(base_key, value):
2121
if base_key.lower().startswith("cortex_tags"):
2222
inlined_tags = ",".join([f"{k}={v}" for k, v in value.items()])
23-
print(f"export CORTEX_TAGS={inlined_tags}")
23+
print(f"export CORTEX_TAGS='{inlined_tags}'")
2424
print(f"export CORTEX_TAGS_JSON='{json.dumps(value)}'")
2525
return
2626

‎manager/install.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ function main() {
189189
fi
190190

191191
# create VPC Link
192-
create_vpc_link_output=$(aws apigatewayv2 create-vpc-link --region $CORTEX_REGION --tags $CORTEX_TAGS --name $CORTEX_CLUSTER_NAME --subnet-ids $private_subnets --security-group-ids $default_security_group)
192+
create_vpc_link_output=$(aws apigatewayv2 create-vpc-link --region $CORTEX_REGION --tags "$CORTEX_TAGS_JSON" --name $CORTEX_CLUSTER_NAME --subnet-ids $private_subnets --security-group-ids $default_security_group)
193193
vpc_link_id=$(echo $create_vpc_link_output | jq .VpcLinkId | tr -d '"')
194194
if [ "$vpc_link_id" = "" ] || [ "$vpc_link_id" = "null" ]; then
195195
echo -e "unable to extract vpc link ID from create-vpc-link output:\n$create_vpc_link_output"

‎manager/manifests/fluentd.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ data:
145145
region "#{ENV['AWS_REGION']}"
146146
log_group_name_key group_name
147147
log_stream_name_key stream_name
148-
log_group_aws_tags ${CORTEX_TAGS_JSON}
148+
log_group_aws_tags "${CORTEX_TAGS_JSON}"
149149
remove_log_stream_name_key true
150150
remove_log_group_name_key true
151151
auto_create_stream true

‎manager/manifests/istio-values.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ gateways:
4141
serviceAnnotations:
4242
${CORTEX_OPERATOR_LOAD_BALANCER_ANNOTATION}
4343
service.beta.kubernetes.io/aws-load-balancer-type: "nlb"
44-
service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags: ${CORTEX_TAGS}
44+
service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags: "${CORTEX_TAGS}"
4545
type: LoadBalancer
4646
externalTrafficPolicy: Local # https://medium.com/pablo-perez/k8s-externaltrafficpolicy-local-or-cluster-40b259a19404, https://www.asykim.com/blog/deep-dive-into-kubernetes-external-traffic-policies
4747
ports:
@@ -84,7 +84,7 @@ gateways:
8484
serviceAnnotations:
8585
${CORTEX_API_LOAD_BALANCER_ANNOTATION}
8686
service.beta.kubernetes.io/aws-load-balancer-type: "nlb"
87-
service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags: ${CORTEX_TAGS}
87+
service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags: "${CORTEX_TAGS}"
8888
${CORTEX_SSL_CERTIFICATE_ANNOTATION}
8989
service.beta.kubernetes.io/aws-load-balancer-ssl-ports: "443"
9090
type: LoadBalancer

‎pkg/lib/configreader/errors.go‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ const (
3232
ErrInvalidYAML = "configreader.invalid_yaml"
3333
ErrTooLong = "configreader.too_long"
3434
ErrTooShort = "configreader.too_short"
35+
ErrLeadingWhitespace = "configreader.leading_whitespace"
36+
ErrTrailingWhitespace = "configreader.trailing_whitespace"
3537
ErrAlphaNumericDashUnderscore = "configreader.alpha_numeric_dash_underscore"
3638
ErrAlphaNumericDashDotUnderscore = "configreader.alpha_numeric_dash_dot_underscore"
3739
ErrInvalidAWSTag = "configreader.invalid_aws_tag"
@@ -114,6 +116,20 @@ func ErrorTooShort(provided string, minLen int) error {
114116
})
115117
}
116118

119+
func ErrorLeadingWhitespace(provided string) error {
120+
return errors.WithStack(&errors.Error{
121+
Kind: ErrLeadingWhitespace,
122+
Message: fmt.Sprintf("%s cannot start with whitespace", s.UserStr(provided)),
123+
})
124+
}
125+
126+
func ErrorTrailingWhitespace(provided string) error {
127+
return errors.WithStack(&errors.Error{
128+
Kind: ErrTrailingWhitespace,
129+
Message: fmt.Sprintf("%s cannot end with whitespace", s.UserStr(provided)),
130+
})
131+
}
132+
117133
func ErrorAlphaNumericDashUnderscore(provided string) error {
118134
return errors.WithStack(&errors.Error{
119135
Kind: ErrAlphaNumericDashUnderscore,
@@ -131,7 +147,7 @@ func ErrorAlphaNumericDashDotUnderscore(provided string) error {
131147
func ErrorInvalidAWSTag(provided string) error {
132148
return errors.WithStack(&errors.Error{
133149
Kind: ErrInvalidAWSTag,
134-
Message: fmt.Sprintf("%s must contain only letters, numbers, spaces, and the following characters: _ . : / = + - @", s.UserStr(provided)),
150+
Message: fmt.Sprintf("%s must contain only letters, numbers, spaces, and the following characters: _ . : / + - @", s.UserStr(provided)),
135151
})
136152
}
137153

‎pkg/lib/configreader/string.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ type StringValidation struct {
4242
InvalidPrefixes []string
4343
MaxLength int
4444
MinLength int
45+
DisallowLeadingWhitespace bool
46+
DisallowTrailingWhitespace bool
4547
AlphaNumericDashDotUnderscoreOrEmpty bool
4648
AlphaNumericDashDotUnderscore bool
4749
AlphaNumericDashUnderscore bool
@@ -257,6 +259,18 @@ func ValidateStringVal(val string, v *StringValidation) error {
257259
}
258260
}
259261

262+
if v.DisallowLeadingWhitespace {
263+
if regex.HasLeadingWhitespace(val) {
264+
return ErrorLeadingWhitespace(val)
265+
}
266+
}
267+
268+
if v.DisallowTrailingWhitespace {
269+
if regex.HasTrailingWhitespace(val) {
270+
return ErrorTrailingWhitespace(val)
271+
}
272+
}
273+
260274
if v.AlphaNumericDashDotUnderscore {
261275
if !regex.IsAlphaNumericDashDotUnderscore(val) {
262276
return ErrorAlphaNumericDashDotUnderscore(val)

‎pkg/lib/configreader/string_ptr.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ type StringPtrValidation struct {
3535
InvalidPrefixes []string
3636
MaxLength int
3737
MinLength int
38+
DisallowLeadingWhitespace bool
39+
DisallowTrailingWhitespace bool
3840
AlphaNumericDashDotUnderscoreOrEmpty bool
3941
AlphaNumericDashDotUnderscore bool
4042
AlphaNumericDashUnderscore bool
@@ -59,6 +61,8 @@ func makeStringValValidation(v *StringPtrValidation) *StringValidation {
5961
InvalidPrefixes: v.InvalidPrefixes,
6062
MaxLength: v.MaxLength,
6163
MinLength: v.MinLength,
64+
DisallowLeadingWhitespace: v.DisallowLeadingWhitespace,
65+
DisallowTrailingWhitespace: v.DisallowTrailingWhitespace,
6266
AlphaNumericDashDotUnderscoreOrEmpty: v.AlphaNumericDashDotUnderscoreOrEmpty,
6367
AlphaNumericDashDotUnderscore: v.AlphaNumericDashDotUnderscore,
6468
AlphaNumericDashUnderscore: v.AlphaNumericDashUnderscore,

‎pkg/lib/regex/regex.go‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,21 @@ func MatchAnyRegex(s string, regexes []*regexp.Regexp) bool {
2929
return false
3030
}
3131

32-
// letters, numbers, spaces representable in UTF-8, and the following characters: _ . : / = + - @
33-
var _awsTagRegex = regexp.MustCompile(`^[\sa-zA-Z0-9_\-\.:/=+@]+$`)
32+
var _leadingWhitespaceRegex = regexp.MustCompile(`^\s+`)
33+
34+
func HasLeadingWhitespace(s string) bool {
35+
return _leadingWhitespaceRegex.MatchString(s)
36+
}
37+
38+
var _trailingWhitespaceRegex = regexp.MustCompile(`\s+$`)
39+
40+
func HasTrailingWhitespace(s string) bool {
41+
return _trailingWhitespaceRegex.MatchString(s)
42+
}
43+
44+
// letters, numbers, spaces representable in UTF-8, and the following characters: _ . : / + - @
45+
// = is not supported because it doesn't propagate to the NLB correctly (via the k8s service annotation)
46+
var _awsTagRegex = regexp.MustCompile(`^[\sa-zA-Z0-9_\-\.:/+@]+$`)
3447

3548
func IsValidAWSTag(s string) bool {
3649
return _awsTagRegex.MatchString(s)

‎pkg/lib/regex/regex_test.go‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ package regex
1919
import (
2020
"strings"
2121
"testing"
22+
23+
"github.com/stretchr/testify/assert"
2224
)
2325

2426
type regexpMatch struct {
@@ -27,6 +29,170 @@ type regexpMatch struct {
2729
subs []string
2830
}
2931

32+
func TestHasLeadingWhitespace(t *testing.T) {
33+
testcases := []regexpMatch{
34+
{
35+
input: " test",
36+
match: true,
37+
},
38+
{
39+
input: " test",
40+
match: true,
41+
},
42+
{
43+
input: "\ntest",
44+
match: true,
45+
},
46+
{
47+
input: " test ",
48+
match: true,
49+
},
50+
{
51+
input: " test ",
52+
match: true,
53+
},
54+
{
55+
input: "\ntest\n",
56+
match: true,
57+
},
58+
{
59+
input: "test",
60+
match: false,
61+
},
62+
{
63+
input: "te st",
64+
match: false,
65+
},
66+
{
67+
input: "te st",
68+
match: false,
69+
},
70+
{
71+
input: "te\nst",
72+
match: false,
73+
},
74+
{
75+
input: "_",
76+
match: false,
77+
},
78+
{
79+
input: "test ",
80+
match: false,
81+
},
82+
{
83+
input: "test ",
84+
match: false,
85+
},
86+
{
87+
input: "test\n",
88+
match: false,
89+
},
90+
{
91+
input: " ",
92+
match: true,
93+
},
94+
{
95+
input: " ",
96+
match: true,
97+
},
98+
{
99+
input: "\n",
100+
match: true,
101+
},
102+
{
103+
input: "",
104+
match: false,
105+
},
106+
}
107+
108+
for i := range testcases {
109+
match := HasLeadingWhitespace(testcases[i].input)
110+
assert.Equal(t, testcases[i].match, match, "input: "+testcases[i].input)
111+
}
112+
}
113+
114+
func TestHasTrailingWhitespace(t *testing.T) {
115+
testcases := []regexpMatch{
116+
{
117+
input: " test",
118+
match: false,
119+
},
120+
{
121+
input: " test",
122+
match: false,
123+
},
124+
{
125+
input: "\ntest",
126+
match: false,
127+
},
128+
{
129+
input: " test ",
130+
match: true,
131+
},
132+
{
133+
input: " test ",
134+
match: true,
135+
},
136+
{
137+
input: "\ntest\n",
138+
match: true,
139+
},
140+
{
141+
input: "test",
142+
match: false,
143+
},
144+
{
145+
input: "te st",
146+
match: false,
147+
},
148+
{
149+
input: "te st",
150+
match: false,
151+
},
152+
{
153+
input: "te\nst",
154+
match: false,
155+
},
156+
{
157+
input: "_",
158+
match: false,
159+
},
160+
{
161+
input: "test ",
162+
match: true,
163+
},
164+
{
165+
input: "test ",
166+
match: true,
167+
},
168+
{
169+
input: "test\n",
170+
match: true,
171+
},
172+
{
173+
input: " ",
174+
match: true,
175+
},
176+
{
177+
input: " ",
178+
match: true,
179+
},
180+
{
181+
input: "\n",
182+
match: true,
183+
},
184+
{
185+
input: "",
186+
match: false,
187+
},
188+
}
189+
190+
for i := range testcases {
191+
match := HasTrailingWhitespace(testcases[i].input)
192+
assert.Equal(t, testcases[i].match, match, "input: "+testcases[i].input)
193+
}
194+
}
195+
30196
func TestAlphaNumericDashDotUnderscoreRegex(t *testing.T) {
31197
testcases := []regexpMatch{
32198
{

‎pkg/types/clusterconfig/clusterconfig.go‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ var (
4949
_cachedCNISupportedInstances *string
5050
// This regex is stricter than the actual S3 rules
5151
_strictS3BucketRegex = regexp.MustCompile(`^([a-z0-9])+(-[a-z0-9]+)*$`)
52+
_invalidTagPrefixes = []string{"kubernetes.io/", "k8s.io/", "eksctl.", "alpha.eksctl.", "beta.eksctl.", "aws:", "Aws:", "aWs:", "awS:", "aWS:", "AwS:", "aWS:", "AWS:"}
5253
)
5354

5455
type Config struct {
@@ -165,16 +166,20 @@ var UserValidation = &cr.StructValidation{
165166
AllowEmpty: true,
166167
ConvertNullToEmpty: true,
167168
KeyStringValidator: &cr.StringValidation{
168-
MinLength: 1,
169-
MaxLength: 127,
170-
InvalidPrefixes: []string{"aws:", "Aws:", "aWs:", "awS:", "aWS:", "AwS:", "aWS:", "AWS:"},
171-
AWSTag: true,
169+
MinLength: 1,
170+
MaxLength: 127,
171+
DisallowLeadingWhitespace: true,
172+
DisallowTrailingWhitespace: true,
173+
InvalidPrefixes: _invalidTagPrefixes,
174+
AWSTag: true,
172175
},
173176
ValueStringValidator: &cr.StringValidation{
174-
MinLength: 1,
175-
MaxLength: 255,
176-
InvalidPrefixes: []string{"aws:", "Aws:", "aWs:", "awS:", "aWS:", "AwS:", "aWS:", "AWS:"},
177-
AWSTag: true,
177+
MinLength: 1,
178+
MaxLength: 255,
179+
DisallowLeadingWhitespace: true,
180+
DisallowTrailingWhitespace: true,
181+
InvalidPrefixes: _invalidTagPrefixes,
182+
AWSTag: true,
178183
},
179184
},
180185
},
@@ -547,6 +552,7 @@ func (cc *Config) SQSNamePrefix() string {
547552
return SQSNamePrefix(cc.ClusterName)
548553
}
549554

555+
// this validates the user-provided cluster config
550556
func (cc *Config) Validate(awsClient *aws.Client) error {
551557
fmt.Print("verifying your configuration ...\n\n")
552558

@@ -620,8 +626,10 @@ func (cc *Config) Validate(awsClient *aws.Client) error {
620626
}
621627
}
622628

623-
if cc.Tags[ClusterNameTag] != "" && cc.Tags[ClusterNameTag] != cc.ClusterName {
624-
return ErrorCantOverrideDefaultTag()
629+
for tagName := range cc.Tags {
630+
if strings.HasPrefix(tagName, "cortex.dev/") {
631+
return errors.Wrap(cr.ErrorCantHavePrefix(tagName, "cortex.dev/"), TagsKey)
632+
}
625633
}
626634
cc.Tags[ClusterNameTag] = cc.ClusterName
627635

0 commit comments

Comments
 (0)