Skip to content

Commit 5e2af84

Browse files
jetstack-botjetstack-bot
authored
Merge pull request #114 from JoshVanL/111-san-space-trimming
For attributes that are lists, trim space for each entry to improve UX.
2 parents 09db233 + f8a1425 commit 5e2af84

4 files changed

Lines changed: 35 additions & 24 deletions

File tree

‎deploy/example/example-app.yaml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ spec:
6565
readOnly: true
6666
volumeAttributes:
6767
csi.cert-manager.io/issuer-name: ca-issuer
68-
csi.cert-manager.io/dns-names: "${POD_NAME}.${POD_NAMESPACE}.svc.cluster.local"
68+
csi.cert-manager.io/dns-names: >-
69+
${POD_NAME}.${POD_NAMESPACE}.svc.cluster.local,
70+
${POD_NAME}.${POD_NAMESPACE}.svc
6971
csi.cert-manager.io/uri-sans: "spiffe://cluster.local/ns/${POD_NAMESPACE}/pod/${POD_NAME}/${POD_UID}"
7072
csi.cert-manager.io/pkcs12-enable: "true"
7173
csi.cert-manager.io/pkcs12-password: "my-password"

‎pkg/requestgen/generator.go‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func parseDNSNames(meta metadata.Metadata, dnsNames string) ([]string, error) {
105105
if err != nil {
106106
return nil, err
107107
}
108-
return strings.Split(dns, ","), nil
108+
return splitList(dns), nil
109109
}
110110

111111
// parseIPAddresses parses a csi.cert-manager.io/ip-sans value, and returns the
@@ -117,7 +117,7 @@ func parseIPAddresses(ipCSV string) ([]net.IP, error) {
117117

118118
var ips []net.IP
119119
var errs []string
120-
for _, ipStr := range strings.Split(ipCSV, ",") {
120+
for _, ipStr := range splitList(ipCSV) {
121121
ip := net.ParseIP(ipStr)
122122
if ip == nil {
123123
errs = append(errs, ipStr)
@@ -147,8 +147,8 @@ func parseURIs(meta metadata.Metadata, uriCSV string) ([]*url.URL, error) {
147147

148148
var uris []*url.URL
149149
var errs []string
150-
for _, uriS := range strings.Split(csv, ",") {
151-
uri, err := url.Parse(uriS)
150+
for _, uriS := range splitList(csv) {
151+
uri, err := url.ParseRequestURI(uriS)
152152
if err != nil {
153153
errs = append(errs, err.Error())
154154
continue
@@ -170,8 +170,8 @@ func keyUsagesFromAttributes(usagesCSV string) []cmapi.KeyUsage {
170170
}
171171

172172
var keyUsages []cmapi.KeyUsage
173-
for _, usage := range strings.Split(usagesCSV, ",") {
174-
keyUsages = append(keyUsages, cmapi.KeyUsage(strings.TrimSpace(usage)))
173+
for _, usage := range splitList(usagesCSV) {
174+
keyUsages = append(keyUsages, cmapi.KeyUsage(usage))
175175
}
176176

177177
return keyUsages
@@ -205,3 +205,12 @@ func expand(meta metadata.Metadata, csv string) (string, error) {
205205

206206
return exp, nil
207207
}
208+
209+
// splitList returns the given csv as a slice. Trims space of each element.
210+
func splitList(csv string) []string {
211+
var list []string
212+
for _, s := range strings.Split(csv, ",") {
213+
list = append(list, strings.TrimSpace(s))
214+
}
215+
return list
216+
}

‎pkg/requestgen/generator_test.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func Test_RequestForMetadata(t *testing.T) {
4747
}
4848

4949
mustParseURI := func(t *testing.T, uri string) *url.URL {
50-
puri, err := url.Parse(uri)
50+
puri, err := url.ParseRequestURI(uri)
5151
assert.NoError(t, err)
5252
return puri
5353
}
@@ -138,9 +138,9 @@ func Test_RequestForMetadata(t *testing.T) {
138138
"csi.cert-manager.io/issuer-group": "joshvanl.com",
139139
"csi.cert-manager.io/duration": "1h",
140140
"csi.cert-manager.io/common-name": "${POD_NAME}.$POD_NAMESPACE",
141-
"csi.cert-manager.io/dns-names": "${POD_NAME}-my-dns-$POD_NAMESPACE-$POD_UID,$POD_NAME,${POD_NAME}.${POD_NAMESPACE},$POD_NAME.$POD_NAMESPACE.svc,$POD_UID",
142-
"csi.cert-manager.io/uri-sans": "spiffe://foo.bar/${POD_NAMESPACE}/${POD_NAME}/$POD_UID,file://foo-bar,${POD_UID}",
143-
"csi.cert-manager.io/ip-sans": "1.2.3.4,5.6.7.8",
141+
"csi.cert-manager.io/dns-names": "${POD_NAME}-my-dns-$POD_NAMESPACE-$POD_UID,$POD_NAME,\n ${POD_NAME}.${POD_NAMESPACE},$POD_NAME.$POD_NAMESPACE.svc,$POD_UID",
142+
"csi.cert-manager.io/uri-sans": "spiffe://foo.bar/${POD_NAMESPACE}/${POD_NAME}/$POD_UID,file://foo-bar, foo://${POD_UID}",
143+
"csi.cert-manager.io/ip-sans": "1.2.3.4,\n \t 5.6.7.8",
144144
"csi.cert-manager.io/is-ca": "true",
145145
"csi.cert-manager.io/key-usages": "server auth,client auth",
146146
}}),
@@ -156,7 +156,7 @@ func Test_RequestForMetadata(t *testing.T) {
156156
URIs: []*url.URL{
157157
mustParseURI(t, "spiffe://foo.bar/my-namespace/my-pod-name/my-pod-uuid"),
158158
mustParseURI(t, "file://foo-bar"),
159-
mustParseURI(t, "my-pod-uuid"),
159+
mustParseURI(t, "foo://my-pod-uuid"),
160160
},
161161
},
162162
IsCA: true,
@@ -263,26 +263,26 @@ func Test_URIs(t *testing.T) {
263263
expErr: nil,
264264
},
265265
"a csv with multiple entries should expect those entries returned": {
266-
csv: "spiffe://foo.bar,file://hello-world/1234,1234",
266+
csv: "spiffe://foo.bar,file://hello-world/1234,foo://1234",
267267
expURIs: func(t *testing.T) []*url.URL {
268268
return []*url.URL{
269269
mustParse(t, "spiffe://foo.bar"),
270270
mustParse(t, "file://hello-world/1234"),
271-
mustParse(t, "1234"),
271+
mustParse(t, "foo://1234"),
272272
}
273273
},
274274
expErr: nil,
275275
},
276276
"a csv with a bad URI should return an error": {
277-
csv: "spiffe://foo.bar,\n,file://hello-world/1234,1234",
277+
csv: "spiffe://foo.bar,\n\nx\n,foo://foo\nbar,file://hello-world/1234,1234",
278278
expURIs: nil,
279-
expErr: errors.New(`parse "\n": net/url: invalid control character in URL`),
279+
expErr: errors.New(`parse "x": invalid URI for request, parse "foo://foo\nbar": net/url: invalid control character in URL, parse "1234": invalid URI for request`),
280280
},
281281
"a single csv which uses variables should be substituted correctly": {
282-
csv: `$POD_NAME-my-dns-${POD_NAMESPACE}-${POD_UID}`,
282+
csv: `foo://$POD_NAME-my-dns-${POD_NAMESPACE}-${POD_UID}`,
283283
expURIs: func(t *testing.T) []*url.URL {
284284
return []*url.URL{
285-
mustParse(t, "my-pod-name-my-dns-my-namespace-my-pod-uuid"),
285+
mustParse(t, "foo://my-pod-name-my-dns-my-namespace-my-pod-uuid"),
286286
}
287287
},
288288
expErr: nil,
@@ -293,13 +293,13 @@ func Test_URIs(t *testing.T) {
293293
expErr: errors.New(`undefined variable "Foo", known variables: [POD_NAME POD_NAMESPACE POD_UID SERVICE_ACCOUNT_NAME]`),
294294
},
295295
"a csv containing multiple entries which uses variables should be substituted correctly": {
296-
csv: `spiffe://$POD_NAME-my-dns-${POD_NAMESPACE}-$POD_UID,spiffe://$POD_NAME,file://${POD_NAME}.$POD_NAMESPACE,$POD_NAME.$POD_NAMESPACE.svc,spiffe://$POD_UID`,
296+
csv: `spiffe://$POD_NAME-my-dns-${POD_NAMESPACE}-$POD_UID,spiffe://$POD_NAME,file://${POD_NAME}.$POD_NAMESPACE,foo://$POD_NAME.$POD_NAMESPACE.svc,spiffe://$POD_UID`,
297297
expURIs: func(t *testing.T) []*url.URL {
298298
return []*url.URL{
299299
mustParse(t, "spiffe://my-pod-name-my-dns-my-namespace-my-pod-uuid"),
300300
mustParse(t, "spiffe://my-pod-name"),
301301
mustParse(t, "file://my-pod-name.my-namespace"),
302-
mustParse(t, "my-pod-name.my-namespace.svc"),
302+
mustParse(t, "foo://my-pod-name.my-namespace.svc"),
303303
mustParse(t, "spiffe://my-pod-uuid"),
304304
}
305305
},

‎test/e2e/suite/cases/variables.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ var _ = framework.CasesDescribe("Should correctly substitute out SANs with varia
6262
}
6363

6464
mustParseURI := func(uri string) *url.URL {
65-
puri, err := url.Parse(uri)
65+
puri, err := url.ParseRequestURI(uri)
6666
Expect(err).NotTo(HaveOccurred())
6767
return puri
6868
}
@@ -74,8 +74,8 @@ var _ = framework.CasesDescribe("Should correctly substitute out SANs with varia
7474
"csi.cert-manager.io/issuer-kind": f.Issuer.Kind,
7575
"csi.cert-manager.io/issuer-group": f.Issuer.Group,
7676
"csi.cert-manager.io/common-name": "$POD_NAME.${POD_NAMESPACE}",
77-
"csi.cert-manager.io/dns-names": "$POD_NAME-my-dns-$POD_NAMESPACE-${POD_UID},${POD_NAME},${POD_NAME}.${POD_NAMESPACE},$POD_NAME.${POD_NAMESPACE}.svc,${POD_UID},${SERVICE_ACCOUNT_NAME}",
78-
"csi.cert-manager.io/uri-sans": "spiffe://foo.bar/${POD_NAMESPACE}/$POD_NAME/$POD_UID,file://foo-bar,${POD_UID}",
77+
"csi.cert-manager.io/dns-names": "$POD_NAME-my-dns-$POD_NAMESPACE-${POD_UID},${POD_NAME},\n${POD_NAME}.${POD_NAMESPACE},$POD_NAME.${POD_NAMESPACE}.svc,${POD_UID},${SERVICE_ACCOUNT_NAME}",
78+
"csi.cert-manager.io/uri-sans": "spiffe://foo.bar/${POD_NAMESPACE}/$POD_NAME/$POD_UID,\nfile://foo-bar,\nbar://${POD_UID}",
7979
})
8080

8181
request, err := pki.DecodeX509CertificateRequestBytes(cr.Spec.Request)
@@ -93,7 +93,7 @@ var _ = framework.CasesDescribe("Should correctly substitute out SANs with varia
9393
Expect(request.URIs).To(ConsistOf([]*url.URL{
9494
mustParseURI(fmt.Sprintf("spiffe://foo.bar/%s/%s/%s", pod.Namespace, pod.Name, pod.UID)),
9595
mustParseURI("file://foo-bar"),
96-
mustParseURI(string(pod.UID)),
96+
mustParseURI(fmt.Sprintf("bar://%s", pod.UID)),
9797
}))
9898
})
9999
})

0 commit comments

Comments
 (0)