Skip to content

Commit 070abed

Browse files
porthorianporthorian
committed
implement more tests in secrets test, move validateca test to cert bundle, language feedback, comments
1 parent daf59f5 commit 070abed

File tree

7 files changed

+132
-60
lines changed

7 files changed

+132
-60
lines changed

‎internal/mode/static/state/dataplane/configuration.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"sort"
88

9-
apiv1 "k8s.io/api/core/v1"
109
discoveryV1 "k8s.io/api/discovery/v1"
1110
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1211
"k8s.io/apimachinery/pkg/types"
@@ -229,10 +228,10 @@ func buildSSLKeyPairs(
229228
id := generateSSLKeyPairID(*l.ResolvedSecret)
230229
secret := secrets[*l.ResolvedSecret]
231230
// The Data map keys are guaranteed to exist by the graph package.
232-
// the Source field is guaranteed to be non-nil by the graph package.
231+
// the CertBundle field is guaranteed to be non-nil by the graph package.
233232
keyPairs[id] = SSLKeyPair{
234-
Cert: secret.Source.Data[apiv1.TLSCertKey],
235-
Key: secret.Source.Data[apiv1.TLSPrivateKeyKey],
233+
Cert: secret.CertBundle.Cert.TLSCert,
234+
Key: secret.CertBundle.Cert.TLSPrivateKey,
236235
}
237236
}
238237
}

‎internal/mode/static/state/graph/backend_tls_policy.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ func validateBackendTLSCACertRef(
170170
return field.Invalid(path, selectedCertRef, err.Error())
171171
}
172172
default:
173-
return fmt.Errorf("invalid certificate reference supported %q", selectedCertRef.Kind)
173+
return fmt.Errorf("invalid certificate reference kind %q", selectedCertRef.Kind)
174174
}
175175
return nil
176176
}

‎internal/mode/static/state/graph/certificate_bundle.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,30 @@ import (
1111
v1 "sigs.k8s.io/gateway-api/apis/v1"
1212
)
1313

14+
// CAKey is the key that is stored in a secret or configmap to grab its data from.
15+
// This follows the convention setup by kubernetes service account root ca
16+
// key for optional root certificate authority
1417
const CAKey = "ca.crt"
1518

19+
// CertificateBundle is used to submit certificate data to nginx that is kubernetes aware
1620
type CertificateBundle struct {
1721
Cert *Certificate
1822

1923
Name types.NamespacedName
2024
Kind v1.Kind
2125
}
2226

27+
// Certificate houses the real certificate data that is sent to the configurator
2328
type Certificate struct {
24-
TLSCert []byte
29+
// TLSCert is the SSL certificate used to send to CA
30+
TLSCert []byte
31+
// TLSPrivateKey is the cryptographic key for encrpyting traffic during secure TLS
2532
TLSPrivateKey []byte
26-
CACert []byte
33+
// CACert is the root certificate authority
34+
CACert []byte
2735
}
2836

37+
// NewCertificateBundle generates a kubernetes aware certificate that is used during the configurator for nginx
2938
func NewCertificateBundle(name types.NamespacedName, kind string, cert *Certificate) *CertificateBundle {
3039
return &CertificateBundle{
3140
Name: name,
@@ -34,6 +43,7 @@ func NewCertificateBundle(name types.NamespacedName, kind string, cert *Certific
3443
}
3544
}
3645

46+
// validateTLS ...
3747
func validateTLS(tlsCert, tlsPrivateKey []byte) error {
3848
_, err := tls.X509KeyPair(tlsCert, tlsPrivateKey)
3949
if err != nil {

‎internal/mode/static/state/graph/certificate_bundle_test.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package graph
2+
3+
import (
4+
"encoding/base64"
5+
"testing"
6+
7+
. "github.com/onsi/gomega"
8+
)
9+
10+
func TestValidateCA(t *testing.T) {
11+
t.Parallel()
12+
base64Data := make([]byte, base64.StdEncoding.EncodedLen(len(caBlock)))
13+
base64.StdEncoding.Encode(base64Data, []byte(caBlock))
14+
15+
tests := []struct {
16+
name string
17+
data []byte
18+
errorExpected bool
19+
}{
20+
{
21+
name: "valid base64",
22+
data: base64Data,
23+
errorExpected: false,
24+
},
25+
{
26+
name: "valid plain text",
27+
data: []byte(caBlock),
28+
errorExpected: false,
29+
},
30+
{
31+
name: "invalid pem",
32+
data: []byte("invalid"),
33+
errorExpected: true,
34+
},
35+
{
36+
name: "invalid type",
37+
data: []byte(caBlockInvalidType),
38+
errorExpected: true,
39+
},
40+
}
41+
42+
for _, test := range tests {
43+
t.Run(test.name, func(t *testing.T) {
44+
t.Parallel()
45+
g := NewWithT(t)
46+
47+
err := validateCA(test.data)
48+
if test.errorExpected {
49+
g.Expect(err).To(HaveOccurred())
50+
} else {
51+
g.Expect(err).ToNot(HaveOccurred())
52+
}
53+
})
54+
}
55+
}

‎internal/mode/static/state/graph/configmaps_test.go‎

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package graph
22

33
import (
4-
"encoding/base64"
54
"testing"
65

76
. "github.com/onsi/gomega"
@@ -87,53 +86,6 @@ UdxohGqleWFMQ3UNLOvc9Fk+q72ryg==
8786
`
8887
)
8988

90-
func TestValidateCA(t *testing.T) {
91-
t.Parallel()
92-
base64Data := make([]byte, base64.StdEncoding.EncodedLen(len(caBlock)))
93-
base64.StdEncoding.Encode(base64Data, []byte(caBlock))
94-
95-
tests := []struct {
96-
name string
97-
data []byte
98-
errorExpected bool
99-
}{
100-
{
101-
name: "valid base64",
102-
data: base64Data,
103-
errorExpected: false,
104-
},
105-
{
106-
name: "valid plain text",
107-
data: []byte(caBlock),
108-
errorExpected: false,
109-
},
110-
{
111-
name: "invalid pem",
112-
data: []byte("invalid"),
113-
errorExpected: true,
114-
},
115-
{
116-
name: "invalid type",
117-
data: []byte(caBlockInvalidType),
118-
errorExpected: true,
119-
},
120-
}
121-
122-
for _, test := range tests {
123-
t.Run(test.name, func(t *testing.T) {
124-
t.Parallel()
125-
g := NewWithT(t)
126-
127-
err := validateCA(test.data)
128-
if test.errorExpected {
129-
g.Expect(err).To(HaveOccurred())
130-
} else {
131-
g.Expect(err).ToNot(HaveOccurred())
132-
}
133-
})
134-
}
135-
}
136-
13789
func TestResolve(t *testing.T) {
13890
configMaps := map[types.NamespacedName]*v1.ConfigMap{
13991
{Namespace: "test", Name: "configmap1"}: {

‎internal/mode/static/state/graph/secret.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ func (r *secretResolver) resolve(nsname types.NamespacedName) error {
6363
validationErr = validateTLS(cert.TLSCert, cert.TLSPrivateKey)
6464

6565
// Not always guaranteed to have a ca certificate in the secret.
66+
// Cert-Manager puts this at ca.crt and thus this is statically placed like so.
67+
// To follow the convention setup by kubernetes for a service account root ca
68+
// for optional root certificate authority
6669
if _, exists := secret.Data[CAKey]; exists {
6770
cert.CACert = secret.Data[CAKey]
6871
validationErr = validateCA(cert.CACert)

‎internal/mode/static/state/graph/secret_test.go‎

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,19 @@ func TestSecretResolver(t *testing.T) {
9393
Type: apiv1.SecretTypeTLS,
9494
}
9595

96+
validSecret3 = &apiv1.Secret{
97+
ObjectMeta: metav1.ObjectMeta{
98+
Namespace: "test",
99+
Name: "secret-3",
100+
},
101+
Data: map[string][]byte{
102+
apiv1.TLSCertKey: cert,
103+
apiv1.TLSPrivateKeyKey: key,
104+
CAKey: []byte(caBlock),
105+
},
106+
Type: apiv1.SecretTypeTLS,
107+
}
108+
96109
invalidSecretType = &apiv1.Secret{
97110
ObjectMeta: metav1.ObjectMeta{
98111
Namespace: "test",
@@ -129,6 +142,19 @@ func TestSecretResolver(t *testing.T) {
129142
Type: apiv1.SecretTypeTLS,
130143
}
131144

145+
invalidSecretCaCert = &apiv1.Secret{
146+
ObjectMeta: metav1.ObjectMeta{
147+
Namespace: "test",
148+
Name: "invalid-ca-key",
149+
},
150+
Data: map[string][]byte{
151+
apiv1.TLSCertKey: cert,
152+
apiv1.TLSPrivateKeyKey: key,
153+
CAKey: invalidCert,
154+
},
155+
Type: apiv1.SecretTypeTLS,
156+
}
157+
132158
secretNotExistNsName = types.NamespacedName{
133159
Namespace: "test",
134160
Name: "not-exist",
@@ -137,11 +163,13 @@ func TestSecretResolver(t *testing.T) {
137163

138164
resolver := newSecretResolver(
139165
map[types.NamespacedName]*apiv1.Secret{
140-
client.ObjectKeyFromObject(validSecret1): validSecret1,
141-
client.ObjectKeyFromObject(validSecret2): validSecret2, // we're not going to resolve it
142-
client.ObjectKeyFromObject(invalidSecretType): invalidSecretType,
143-
client.ObjectKeyFromObject(invalidSecretCert): invalidSecretCert,
144-
client.ObjectKeyFromObject(invalidSecretKey): invalidSecretKey,
166+
client.ObjectKeyFromObject(validSecret1): validSecret1,
167+
client.ObjectKeyFromObject(validSecret2): validSecret2, // we're not going to resolve it
168+
client.ObjectKeyFromObject(validSecret3): validSecret3,
169+
client.ObjectKeyFromObject(invalidSecretType): invalidSecretType,
170+
client.ObjectKeyFromObject(invalidSecretCert): invalidSecretCert,
171+
client.ObjectKeyFromObject(invalidSecretKey): invalidSecretKey,
172+
client.ObjectKeyFromObject(invalidSecretCaCert): invalidSecretCaCert,
145173
})
146174

147175
tests := []struct {
@@ -157,6 +185,10 @@ func TestSecretResolver(t *testing.T) {
157185
name: "valid secret, again",
158186
nsname: client.ObjectKeyFromObject(validSecret1),
159187
},
188+
{
189+
name: "valid secret, with ca certificate",
190+
nsname: client.ObjectKeyFromObject(validSecret3),
191+
},
160192
{
161193
name: "doesn't exist",
162194
nsname: secretNotExistNsName,
@@ -182,6 +214,11 @@ func TestSecretResolver(t *testing.T) {
182214
nsname: client.ObjectKeyFromObject(invalidSecretKey),
183215
expectedErrMsg: "TLS secret is invalid: tls: failed to parse private key",
184216
},
217+
{
218+
name: "invalid secret ca cert",
219+
nsname: client.ObjectKeyFromObject(invalidSecretCaCert),
220+
expectedErrMsg: "failed to validate certificate: x509: malformed certificate",
221+
},
185222
}
186223

187224
// Not running tests with t.Run(...) because the last one (getResolvedSecrets) depends on the execution of
@@ -206,6 +243,14 @@ func TestSecretResolver(t *testing.T) {
206243
TLSPrivateKey: key,
207244
}),
208245
},
246+
client.ObjectKeyFromObject(validSecret3): {
247+
Source: validSecret3,
248+
CertBundle: NewCertificateBundle(client.ObjectKeyFromObject(validSecret3), "Secret", &Certificate{
249+
TLSCert: cert,
250+
TLSPrivateKey: key,
251+
CACert: []byte(caBlock),
252+
}),
253+
},
209254
client.ObjectKeyFromObject(invalidSecretType): {
210255
Source: invalidSecretType,
211256
},
@@ -223,6 +268,14 @@ func TestSecretResolver(t *testing.T) {
223268
TLSPrivateKey: invalidKey,
224269
}),
225270
},
271+
client.ObjectKeyFromObject(invalidSecretCaCert): {
272+
Source: invalidSecretCaCert,
273+
CertBundle: NewCertificateBundle(client.ObjectKeyFromObject(invalidSecretCaCert), "Secret", &Certificate{
274+
TLSCert: cert,
275+
TLSPrivateKey: key,
276+
CACert: invalidCert,
277+
}),
278+
},
226279
secretNotExistNsName: {
227280
Source: nil,
228281
},

0 commit comments

Comments
 (0)