Skip to content

Commit 5112c43

Browse files
randmonkeyrandmonkey
authored
fix(translator): Re-fill client certificates of services after merging certificate (#6228) (#6232)
(cherry picked from commit d410be4)
1 parent 6e0a0e7 commit 5112c43

File tree

4 files changed

+224
-4
lines changed

4 files changed

+224
-4
lines changed

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,14 @@ Adding a new version? You'll need three changes:
7979
- [0.0.5](#005)
8080
- [0.0.4 and prior](#004-and-prior)
8181

82+
## Unreleased
83+
84+
### Fixed
85+
86+
- Services using `Secret`s containing the same certificate as client certificates
87+
by annotation `konghq.com/client-cert` can be correctly translated.
88+
[#6228](https://github.com/Kong/kubernetes-ingress-controller/pull/6228)
89+
8290
## [2.12.4]
8391

8492
> Release date: 2024-04-30

‎internal/dataplane/parser/parser.go‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,19 @@ func (p *Parser) BuildKongConfig() KongConfigBuildingResult {
256256
ingressCerts := p.getCerts(ingressRules.SecretNameToSNIs)
257257
gatewayCerts := p.getGatewayCerts()
258258
// note that ingress-derived certificates will take precedence over gateway-derived certificates for SNI assignment
259-
result.Certificates = mergeCerts(p.logger, ingressCerts, gatewayCerts)
259+
var certIDsSeen certIDToMergedCertID
260+
result.Certificates, certIDsSeen = mergeCerts(p.logger, ingressCerts, gatewayCerts)
261+
262+
// re-fill client certificate IDs of services after certificates are merged.
263+
for i, s := range result.Services {
264+
if s.ClientCertificate != nil && s.ClientCertificate.ID != nil {
265+
certID := s.ClientCertificate.ID
266+
mergedCertID := certIDsSeen[*certID]
267+
result.Services[i].ClientCertificate = &kong.Certificate{
268+
ID: kong.String(mergedCertID),
269+
}
270+
}
271+
}
260272

261273
// populate CA certificates in Kong
262274
result.CACertificates = p.getCACerts()
@@ -657,9 +669,18 @@ func (p *Parser) registerResourceFailureNotSupportedForExpressionRoutes(obj clie
657669
}
658670
}
659671

660-
func mergeCerts(log logrus.FieldLogger, certLists ...[]certWrapper) []kongstate.Certificate {
672+
type certIDToMergedCertID map[string]string
673+
674+
type identicalCertIDSet struct {
675+
mergedCertID string
676+
certIDs []string
677+
}
678+
679+
func mergeCerts(log logrus.FieldLogger, certLists ...[]certWrapper) ([]kongstate.Certificate, certIDToMergedCertID) {
661680
snisSeen := make(map[string]string)
662681
certsSeen := make(map[string]certWrapper)
682+
certIDSets := make(map[string]identicalCertIDSet)
683+
663684
for _, cl := range certLists {
664685
for _, cw := range cl {
665686
current, ok := certsSeen[cw.identifier]
@@ -700,6 +721,12 @@ func mergeCerts(log logrus.FieldLogger, certLists ...[]certWrapper) []kongstate.
700721
}
701722
}
702723
certsSeen[current.identifier] = current
724+
725+
idSet := certIDSets[current.identifier]
726+
idSet.mergedCertID = *current.cert.ID
727+
idSet.certIDs = append(idSet.certIDs, *cw.cert.ID)
728+
certIDSets[current.identifier] = idSet
729+
703730
}
704731
}
705732
var res []kongstate.Certificate
@@ -709,7 +736,14 @@ func mergeCerts(log logrus.FieldLogger, certLists ...[]certWrapper) []kongstate.
709736
})
710737
res = append(res, kongstate.Certificate{Certificate: cw.cert})
711738
}
712-
return res
739+
740+
idToMergedID := certIDToMergedCertID{}
741+
for _, idSet := range certIDSets {
742+
for _, certID := range idSet.certIDs {
743+
idToMergedID[certID] = idSet.mergedCertID
744+
}
745+
}
746+
return res, idToMergedID
713747
}
714748

715749
func getServiceEndpoints(

‎internal/dataplane/parser/parser_test.go‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,17 @@ func TestServiceClientCertificate(t *testing.T) {
859859
},
860860
},
861861
},
862+
{
863+
Path: "/bar",
864+
Backend: netv1.IngressBackend{
865+
Service: &netv1.IngressServiceBackend{
866+
Name: "bar-svc",
867+
Port: netv1.ServiceBackendPort{
868+
Number: 80,
869+
},
870+
},
871+
},
872+
},
862873
},
863874
},
864875
},
@@ -886,6 +897,17 @@ func TestServiceClientCertificate(t *testing.T) {
886897
"tls.key": key,
887898
},
888899
},
900+
{
901+
ObjectMeta: metav1.ObjectMeta{
902+
UID: k8stypes.UID("ffaabbcc-180b-4702-a91f-61351a33c6e4"),
903+
Name: "secret2",
904+
Namespace: "default",
905+
},
906+
Data: map[string][]byte{
907+
"tls.crt": crt,
908+
"tls.key": key,
909+
},
910+
},
889911
}
890912
services := []*corev1.Service{
891913
{
@@ -897,6 +919,16 @@ func TestServiceClientCertificate(t *testing.T) {
897919
},
898920
},
899921
},
922+
{
923+
ObjectMeta: metav1.ObjectMeta{
924+
Name: "bar-svc",
925+
Namespace: "default",
926+
Annotations: map[string]string{
927+
"konghq.com/client-cert": "secret2",
928+
"konghq.com/protocol": "https",
929+
},
930+
},
931+
},
900932
}
901933
store, err := store.NewFakeStore(store.FakeObjects{
902934
IngressesV1: ingresses,
@@ -914,9 +946,11 @@ func TestServiceClientCertificate(t *testing.T) {
914946
assert.Equal("7428fb98-180b-4702-a91f-61351a33c6e4",
915947
*state.Certificates[0].ID)
916948

917-
assert.Equal(1, len(state.Services))
949+
assert.Equal(2, len(state.Services))
918950
assert.Equal("7428fb98-180b-4702-a91f-61351a33c6e4",
919951
*state.Services[0].ClientCertificate.ID)
952+
assert.Equal("7428fb98-180b-4702-a91f-61351a33c6e4",
953+
*state.Services[1].ClientCertificate.ID)
920954
})
921955
t.Run("client-cert secret doesn't exist", func(t *testing.T) {
922956
ingresses := []*netv1.Ingress{

‎internal/dataplane/parser/translate_certs_test.go‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
package parser
2+
3+
import (
4+
"sort"
5+
"testing"
6+
7+
"github.com/kong/go-kong/kong"
8+
"github.com/sirupsen/logrus"
9+
"github.com/stretchr/testify/require"
10+
11+
"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
12+
"github.com/kong/kubernetes-ingress-controller/v2/test/helpers/certificate"
13+
)
14+
15+
func TestMergeCerts(t *testing.T) {
16+
crt1, key1 := certificate.MustGenerateSelfSignedCertPEMFormat(certificate.WithCommonName("foo.com"))
17+
crt2, key2 := certificate.MustGenerateSelfSignedCertPEMFormat(certificate.WithCommonName("bar.com"))
18+
testCases := []struct {
19+
name string
20+
certs []certWrapper
21+
mergedCerts []kongstate.Certificate
22+
idToMergedID certIDToMergedCertID
23+
}{
24+
{
25+
name: "single certificate",
26+
certs: []certWrapper{
27+
{
28+
identifier: string(crt1) + string(key1),
29+
cert: kong.Certificate{
30+
ID: kong.String("certificate-1"),
31+
Cert: kong.String(string(crt1)),
32+
Key: kong.String(string(key1)),
33+
},
34+
snis: []string{"foo.com"},
35+
},
36+
},
37+
mergedCerts: []kongstate.Certificate{
38+
{
39+
Certificate: kong.Certificate{
40+
ID: kong.String("certificate-1"),
41+
Cert: kong.String(string(crt1)),
42+
Key: kong.String(string(key1)),
43+
SNIs: kong.StringSlice("foo.com"),
44+
},
45+
},
46+
},
47+
idToMergedID: certIDToMergedCertID{"certificate-1": "certificate-1"},
48+
},
49+
{
50+
name: "multiple different certifcates",
51+
certs: []certWrapper{
52+
{
53+
identifier: string(crt1) + string(key1),
54+
cert: kong.Certificate{
55+
ID: kong.String("certificate-1"),
56+
Cert: kong.String(string(crt1)),
57+
Key: kong.String(string(key1)),
58+
},
59+
snis: []string{"foo.com"},
60+
},
61+
{
62+
identifier: string(crt2) + string(key2),
63+
cert: kong.Certificate{
64+
ID: kong.String("certificate-2"),
65+
Cert: kong.String(string(crt2)),
66+
Key: kong.String(string(key2)),
67+
},
68+
snis: []string{"bar.com"},
69+
},
70+
},
71+
mergedCerts: []kongstate.Certificate{
72+
{
73+
Certificate: kong.Certificate{
74+
ID: kong.String("certificate-1"),
75+
Cert: kong.String(string(crt1)),
76+
Key: kong.String(string(key1)),
77+
SNIs: kong.StringSlice("foo.com"),
78+
},
79+
},
80+
{
81+
Certificate: kong.Certificate{
82+
ID: kong.String("certificate-2"),
83+
Cert: kong.String(string(crt2)),
84+
Key: kong.String(string(key2)),
85+
SNIs: kong.StringSlice("bar.com"),
86+
},
87+
},
88+
},
89+
idToMergedID: certIDToMergedCertID{
90+
"certificate-1": "certificate-1",
91+
"certificate-2": "certificate-2",
92+
},
93+
},
94+
{
95+
name: "multiple certs with same content should be merged",
96+
certs: []certWrapper{
97+
{
98+
identifier: string(crt1) + string(key1),
99+
cert: kong.Certificate{
100+
ID: kong.String("certificate-1"),
101+
Cert: kong.String(string(crt1)),
102+
Key: kong.String(string(key1)),
103+
},
104+
snis: []string{"foo.com"},
105+
},
106+
{
107+
identifier: string(crt1) + string(key1),
108+
cert: kong.Certificate{
109+
ID: kong.String("certificate-1-1"),
110+
Cert: kong.String(string(crt1)),
111+
Key: kong.String(string(key1)),
112+
},
113+
snis: []string{"baz.com"},
114+
},
115+
},
116+
mergedCerts: []kongstate.Certificate{
117+
{
118+
Certificate: kong.Certificate{
119+
ID: kong.String("certificate-1"),
120+
Cert: kong.String(string(crt1)),
121+
Key: kong.String(string(key1)),
122+
// SNIs should be sorted
123+
SNIs: kong.StringSlice("baz.com", "foo.com"),
124+
},
125+
},
126+
},
127+
idToMergedID: certIDToMergedCertID{
128+
"certificate-1": "certificate-1",
129+
"certificate-1-1": "certificate-1",
130+
},
131+
},
132+
}
133+
for _, tc := range testCases {
134+
t.Run(tc.name, func(t *testing.T) {
135+
mergedCerts, idToMergedID := mergeCerts(logrus.StandardLogger(), tc.certs)
136+
// sort certs by their IDs to make a stable order of the result merged certs.
137+
sort.SliceStable(mergedCerts, func(i, j int) bool {
138+
return *mergedCerts[i].Certificate.ID < *mergedCerts[j].Certificate.ID
139+
})
140+
require.Equal(t, tc.mergedCerts, mergedCerts)
141+
require.Equal(t, tc.idToMergedID, idToMergedID)
142+
})
143+
}
144+
}

0 commit comments

Comments
 (0)