Skip to content

Commit 88b0b6c

Browse files
aa1exaa1ex
authored
store optimization (#202)
1 parent e5e3a96 commit 88b0b6c

File tree

10 files changed

+219
-17
lines changed

10 files changed

+219
-17
lines changed

‎internal/grpcapi/utils.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,19 @@ func (s *UtilsService) VerifyDomains(_ context.Context, req *connect.Request[v1.
3636
return connect.NewResponse(&v1.VerifyDomainsResponse{Results: results}), nil
3737
}
3838

39-
func verifyDomainFromSecrets(domain string, secrets map[string]corev1.Secret) *v1.DomainVerificationResult {
39+
func verifyDomainFromSecrets(domain string, secrets map[string]*corev1.Secret) *v1.DomainVerificationResult {
4040
result := &v1.DomainVerificationResult{Domain: domain}
4141
var matchedSecret *corev1.Secret
4242
var matchedByWildcard bool
4343

4444
if secret, ok := secrets[domain]; ok {
45-
matchedSecret = &secret
45+
matchedSecret = secret
4646
} else {
4747
parts := strings.Split(domain, ".")
4848
for i := 1; i < len(parts)-1; i++ {
4949
wildcard := "*." + strings.Join(parts[i:], ".")
5050
if secret, ok := secrets[wildcard]; ok {
51-
matchedSecret = &secret
51+
matchedSecret = secret
5252
matchedByWildcard = true
5353
break
5454
}

‎internal/store/interfaces.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ type Store interface {
8282
DeleteSecret(name helpers.NamespacedName)
8383
IsExistingSecret(name helpers.NamespacedName) bool
8484
MapSecrets() map[helpers.NamespacedName]*corev1.Secret
85-
MapDomainSecrets() map[string]corev1.Secret
85+
MapDomainSecrets() map[string]*corev1.Secret
8686

8787
// Tracing
8888
GetTracing(name helpers.NamespacedName) *v1alpha1.Tracing

‎internal/store/optimized.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,13 +1287,14 @@ func (s *OptimizedStore) MapSpecClusters() map[string]*v1alpha1.Cluster {
12871287
return result
12881288
}
12891289

1290-
func (s *OptimizedStore) MapDomainSecrets() map[string]corev1.Secret {
1290+
func (s *OptimizedStore) MapDomainSecrets() map[string]*corev1.Secret {
12911291
s.mu.RLock()
12921292
defer s.mu.RUnlock()
12931293

1294-
result := make(map[string]corev1.Secret, len(s.domainSecrets))
1294+
result := make(map[string]*corev1.Secret, len(s.domainSecrets))
12951295
for k, v := range s.domainSecrets {
1296-
result[k] = v
1296+
secret := v // Create explicit copy for pointer safety
1297+
result[k] = &secret
12971298
}
12981299
return result
12991300
}

‎internal/store/secret.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,15 @@ func (s *LegacyStore) MapSecrets() map[helpers.NamespacedName]*corev1.Secret {
4343
return maps.Clone(s.secrets)
4444
}
4545

46-
func (s *LegacyStore) MapDomainSecrets() map[string]corev1.Secret {
46+
func (s *LegacyStore) MapDomainSecrets() map[string]*corev1.Secret {
4747
s.mu.RLock()
4848
defer s.mu.RUnlock()
49-
return maps.Clone(s.domainToSecretMap)
49+
result := make(map[string]*corev1.Secret, len(s.domainToSecretMap))
50+
for k, v := range s.domainToSecretMap {
51+
secret := v
52+
result[k] = &secret
53+
}
54+
return result
5055
}
5156

5257
func (s *LegacyStore) updateDomainSecretsMap() {

‎internal/store/secret_bench_test.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
package store
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
corev1 "k8s.io/api/core/v1"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
)
10+
11+
// BenchmarkMapDomainSecrets_OptimizedStore benchmarks the optimized implementation
12+
func BenchmarkMapDomainSecrets_OptimizedStore(b *testing.B) {
13+
store := NewOptimizedStore()
14+
15+
// Populate store with test secrets
16+
for i := 0; i < 100; i++ {
17+
domain := fmt.Sprintf("domain-%d.example.com", i)
18+
secret := &corev1.Secret{
19+
ObjectMeta: metav1.ObjectMeta{
20+
Name: fmt.Sprintf("secret-%d", i),
21+
Namespace: "default",
22+
Annotations: map[string]string{
23+
"envoy.kaasops.io/domains": domain,
24+
},
25+
},
26+
Data: map[string][]byte{
27+
"tls.crt": make([]byte, 1024),
28+
"tls.key": make([]byte, 1024),
29+
},
30+
}
31+
store.SetSecret(secret)
32+
}
33+
34+
b.ResetTimer()
35+
b.ReportAllocs()
36+
37+
var result map[string]*corev1.Secret
38+
for i := 0; i < b.N; i++ {
39+
result = store.MapDomainSecrets()
40+
}
41+
_ = result
42+
}
43+
44+
// BenchmarkMapDomainSecrets_LegacyStore benchmarks the legacy implementation
45+
func BenchmarkMapDomainSecrets_LegacyStore(b *testing.B) {
46+
store := New()
47+
48+
// Populate store with test secrets
49+
for i := 0; i < 100; i++ {
50+
domain := fmt.Sprintf("domain-%d.example.com", i)
51+
secret := &corev1.Secret{
52+
ObjectMeta: metav1.ObjectMeta{
53+
Name: fmt.Sprintf("secret-%d", i),
54+
Namespace: "default",
55+
Annotations: map[string]string{
56+
"envoy.kaasops.io/domains": domain,
57+
},
58+
},
59+
Data: map[string][]byte{
60+
"tls.crt": make([]byte, 1024),
61+
"tls.key": make([]byte, 1024),
62+
},
63+
}
64+
store.SetSecret(secret)
65+
}
66+
67+
b.ResetTimer()
68+
b.ReportAllocs()
69+
70+
var result map[string]*corev1.Secret
71+
for i := 0; i < b.N; i++ {
72+
result = store.MapDomainSecrets()
73+
}
74+
_ = result
75+
}

‎internal/store/secret_test.go‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
package store
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
corev1 "k8s.io/api/core/v1"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
)
10+
11+
// TestMapDomainSecrets_ReturnsPointers verifies that MapDomainSecrets returns pointers
12+
func TestMapDomainSecrets_ReturnsPointers(t *testing.T) {
13+
tests := []struct {
14+
name string
15+
store Store
16+
}{
17+
{"OptimizedStore", NewOptimizedStore()},
18+
{"LegacyStore", &StoreAdapter{LegacyStore: New()}},
19+
}
20+
21+
for _, tt := range tests {
22+
t.Run(tt.name, func(t *testing.T) {
23+
// Create test secrets
24+
secret1 := &corev1.Secret{
25+
ObjectMeta: metav1.ObjectMeta{
26+
Name: "secret1",
27+
Namespace: "default",
28+
Annotations: map[string]string{
29+
"envoy.kaasops.io/domains": "domain1.com",
30+
},
31+
},
32+
Data: map[string][]byte{
33+
"tls.crt": []byte("cert1"),
34+
"tls.key": []byte("key1"),
35+
},
36+
}
37+
38+
secret2 := &corev1.Secret{
39+
ObjectMeta: metav1.ObjectMeta{
40+
Name: "secret2",
41+
Namespace: "default",
42+
Annotations: map[string]string{
43+
"envoy.kaasops.io/domains": "domain2.com",
44+
},
45+
},
46+
Data: map[string][]byte{
47+
"tls.crt": []byte("cert2"),
48+
"tls.key": []byte("key2"),
49+
},
50+
}
51+
52+
tt.store.SetSecret(secret1)
53+
tt.store.SetSecret(secret2)
54+
55+
// Get domain secrets map
56+
domainSecrets := tt.store.MapDomainSecrets()
57+
58+
// Verify we got pointers
59+
assert.NotNil(t, domainSecrets)
60+
assert.Len(t, domainSecrets, 2)
61+
62+
// Verify correct data
63+
s1, ok := domainSecrets["domain1.com"]
64+
assert.True(t, ok, "domain1.com should exist")
65+
assert.NotNil(t, s1, "domain1.com secret should not be nil")
66+
assert.Equal(t, "secret1", s1.Name)
67+
assert.Equal(t, []byte("cert1"), s1.Data["tls.crt"])
68+
69+
s2, ok := domainSecrets["domain2.com"]
70+
assert.True(t, ok, "domain2.com should exist")
71+
assert.NotNil(t, s2, "domain2.com secret should not be nil")
72+
assert.Equal(t, "secret2", s2.Name)
73+
assert.Equal(t, []byte("cert2"), s2.Data["tls.crt"])
74+
75+
// Verify that each domain has a unique pointer
76+
assert.NotSame(t, s1, s2, "pointers should be different")
77+
})
78+
}
79+
}
80+
81+
// TestMapDomainSecrets_ImmutableCopy verifies that returned map is a copy
82+
func TestMapDomainSecrets_ImmutableCopy(t *testing.T) {
83+
tests := []struct {
84+
name string
85+
store Store
86+
}{
87+
{"OptimizedStore", NewOptimizedStore()},
88+
{"LegacyStore", &StoreAdapter{LegacyStore: New()}},
89+
}
90+
91+
for _, tt := range tests {
92+
t.Run(tt.name, func(t *testing.T) {
93+
secret := &corev1.Secret{
94+
ObjectMeta: metav1.ObjectMeta{
95+
Name: "secret",
96+
Namespace: "default",
97+
Annotations: map[string]string{
98+
"envoy.kaasops.io/domains": "test.com",
99+
},
100+
},
101+
Data: map[string][]byte{"key": []byte("value")},
102+
}
103+
104+
tt.store.SetSecret(secret)
105+
106+
// Get first copy
107+
map1 := tt.store.MapDomainSecrets()
108+
109+
// Get second copy
110+
map2 := tt.store.MapDomainSecrets()
111+
112+
// Verify both have the data
113+
assert.Len(t, map1, 1)
114+
assert.Len(t, map2, 1)
115+
116+
// Pointers should be different (new allocation each time)
117+
assert.NotSame(t, map1["test.com"], map2["test.com"],
118+
"each call should return new pointers to prevent accidental mutation")
119+
})
120+
}
121+
}

‎internal/xds/resbuilder/builder.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,11 +1007,11 @@ func getSecretNameToDomainsViaSecretRef(secretRef *v1alpha1.ResourceRef, vsNames
10071007
return m
10081008
}
10091009

1010-
func getSecretNameToDomainsViaAutoDiscovery(domains []string, domainToSecretMap map[string]v1.Secret) (map[helpers.NamespacedName][]string, error) {
1010+
func getSecretNameToDomainsViaAutoDiscovery(domains []string, domainToSecretMap map[string]*v1.Secret) (map[helpers.NamespacedName][]string, error) {
10111011
m := make(map[helpers.NamespacedName][]string)
10121012

10131013
for _, domain := range domains {
1014-
var secret v1.Secret
1014+
var secret *v1.Secret
10151015
secret, ok := domainToSecretMap[domain]
10161016
if !ok {
10171017
secret, ok = domainToSecretMap[getWildcardDomain(domain)]

‎internal/xds/resbuilder_v2/adapters/tls_adapter.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,12 @@ func (a *TLSAdapter) getSecretNameToDomainsViaSecretRef(
7373
// getSecretNameToDomainsViaAutoDiscovery maps domains to secrets based on auto-discovery
7474
func (a *TLSAdapter) getSecretNameToDomainsViaAutoDiscovery(
7575
domains []string,
76-
domainToSecretMap map[string]v1.Secret,
76+
domainToSecretMap map[string]*v1.Secret,
7777
) (map[helpers.NamespacedName][]string, error) {
7878
m := make(map[helpers.NamespacedName][]string)
7979

8080
for _, domain := range domains {
81-
var secret v1.Secret
81+
var secret *v1.Secret
8282
secret, ok := domainToSecretMap[domain]
8383
if !ok {
8484
secret, ok = domainToSecretMap[utils.GetWildcardDomain(domain)]

‎internal/xds/resbuilder_v2/builder.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -837,11 +837,11 @@ func getSecretNameToDomainsViaSecretRef(secretRef *v1alpha1.ResourceRef, vsNames
837837
return m
838838
}
839839

840-
func getSecretNameToDomainsViaAutoDiscovery(domains []string, domainToSecretMap map[string]v1.Secret) (map[helpers.NamespacedName][]string, error) {
840+
func getSecretNameToDomainsViaAutoDiscovery(domains []string, domainToSecretMap map[string]*v1.Secret) (map[helpers.NamespacedName][]string, error) {
841841
m := make(map[helpers.NamespacedName][]string)
842842

843843
for _, domain := range domains {
844-
var secret v1.Secret
844+
var secret *v1.Secret
845845
secret, ok := domainToSecretMap[domain]
846846
if !ok {
847847
secret, ok = domainToSecretMap[utils.GetWildcardDomain(domain)]

‎internal/xds/resbuilder_v2/tls/builder.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,11 @@ func (b *Builder) getSecretNameToDomainsViaSecretRef(secretRef *v1alpha1.Resourc
8080
}
8181

8282
// getSecretNameToDomainsViaAutoDiscovery maps domains to secrets based on auto-discovery
83-
func (b *Builder) getSecretNameToDomainsViaAutoDiscovery(domains []string, domainToSecretMap map[string]v1.Secret) (map[helpers.NamespacedName][]string, error) {
83+
func (b *Builder) getSecretNameToDomainsViaAutoDiscovery(domains []string, domainToSecretMap map[string]*v1.Secret) (map[helpers.NamespacedName][]string, error) {
8484
m := make(map[helpers.NamespacedName][]string)
8585

8686
for _, domain := range domains {
87-
var secret v1.Secret
87+
var secret *v1.Secret
8888
secret, ok := domainToSecretMap[domain]
8989
if !ok {
9090
secret, ok = domainToSecretMap[utils.GetWildcardDomain(domain)]

0 commit comments

Comments
 (0)