Skip to content

Commit 39456ca

Browse files
author
Alice Wasko
authored
Hash ir/infra resources (#559)
* infra: hash resources with long names Signed-off-by: AliceProxy <[email protected]> * add tests for hashing resources Signed-off-by: AliceProxy <[email protected]> * hashing: replace sha1 with sha256 Signed-off-by: AliceProxy <[email protected]> * hashing: only use 8 chars Signed-off-by: AliceProxy <[email protected]> * ir/infra: always hash resource names Signed-off-by: AliceProxy <[email protected]> * update all test manifests with hashed names Signed-off-by: AliceProxy <[email protected]> * only hash necessary resources Signed-off-by: AliceProxy <[email protected]> * update test manifests Signed-off-by: AliceProxy <[email protected]> Signed-off-by: AliceProxy <[email protected]>
1 parent 319bcb9 commit 39456ca

File tree

11 files changed

+204
-21
lines changed

11 files changed

+204
-21
lines changed

‎internal/envoygateway/config/config.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,8 @@ const (
1212
EnvoyGatewayNamespace = "envoy-gateway-system"
1313
// EnvoyGatewayServiceName is the name of the Envoy Gateway service.
1414
EnvoyGatewayServiceName = "envoy-gateway"
15-
// EnvoyConfigMapPrefix is the prefix applied to the Envoy ConfigMap.
16-
EnvoyConfigMapPrefix = "envoy"
17-
// EnvoyServicePrefix is the prefix applied to the Envoy Service.
18-
EnvoyServicePrefix = "envoy"
19-
// EnvoyDeploymentPrefix is the prefix applied to the Envoy Deployment.
20-
EnvoyDeploymentPrefix = "envoy"
15+
// EnvoyPrefix is the prefix applied to the Envoy ConfigMap, Service, Deployment, and ServiceAccount.
16+
EnvoyPrefix = "envoy"
2117
)
2218

2319
// Server wraps the EnvoyGateway configuration and additional parameters

‎internal/infrastructure/kubernetes/configmap.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/envoyproxy/gateway/internal/envoygateway/config"
1414
"github.com/envoyproxy/gateway/internal/gatewayapi"
1515
"github.com/envoyproxy/gateway/internal/ir"
16+
"github.com/envoyproxy/gateway/internal/provider/utils"
1617
)
1718

1819
const (
@@ -113,5 +114,6 @@ func (i *Infra) deleteConfigMap(ctx context.Context, infra *ir.Infra) error {
113114
}
114115

115116
func expectedConfigMapName(proxyName string) string {
116-
return fmt.Sprintf("%s-%s", config.EnvoyConfigMapPrefix, proxyName)
117+
cMapName := utils.GetHashedName(proxyName)
118+
return fmt.Sprintf("%s-%s", config.EnvoyPrefix, cMapName)
117119
}

‎internal/infrastructure/kubernetes/configmap_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ func TestExpectedConfigMap(t *testing.T) {
2222
cli := fakeclient.NewClientBuilder().WithScheme(envoygateway.GetScheme()).WithObjects().Build()
2323
kube := NewInfra(cli)
2424
infra := ir.NewInfra()
25+
2526
infra.Proxy.Name = "test"
2627

2728
// An infra without Gateway owner labels should trigger
@@ -35,7 +36,7 @@ func TestExpectedConfigMap(t *testing.T) {
3536
cm, err := kube.expectedConfigMap(infra)
3637
require.NoError(t, err)
3738

38-
require.Equal(t, "envoy-test", cm.Name)
39+
require.Equal(t, "envoy-test-74657374", cm.Name)
3940
require.Equal(t, "envoy-gateway-system", cm.Namespace)
4041
require.Contains(t, cm.Data, sdsCAFilename)
4142
assert.Equal(t, sdsCAConfigMapData, cm.Data[sdsCAFilename])
@@ -65,7 +66,7 @@ func TestCreateOrUpdateConfigMap(t *testing.T) {
6566
expect: &corev1.ConfigMap{
6667
ObjectMeta: metav1.ObjectMeta{
6768
Namespace: config.EnvoyGatewayNamespace,
68-
Name: "envoy-test",
69+
Name: "envoy-test-74657374",
6970
Labels: map[string]string{
7071
"app.gateway.envoyproxy.io/name": "envoy",
7172
gatewayapi.OwningGatewayNamespaceLabel: "default",
@@ -92,7 +93,7 @@ func TestCreateOrUpdateConfigMap(t *testing.T) {
9293
expect: &corev1.ConfigMap{
9394
ObjectMeta: metav1.ObjectMeta{
9495
Namespace: config.EnvoyGatewayNamespace,
95-
Name: "envoy-test",
96+
Name: "envoy-test-74657374",
9697
Labels: map[string]string{
9798
"app.gateway.envoyproxy.io/name": "envoy",
9899
gatewayapi.OwningGatewayNamespaceLabel: "default",

‎internal/infrastructure/kubernetes/deployment.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/envoyproxy/gateway/internal/envoygateway/config"
1919
"github.com/envoyproxy/gateway/internal/gatewayapi"
2020
"github.com/envoyproxy/gateway/internal/ir"
21+
"github.com/envoyproxy/gateway/internal/provider/utils"
2122
xdsrunner "github.com/envoyproxy/gateway/internal/xds/server/runner"
2223
)
2324

@@ -94,7 +95,8 @@ func (b *bootstrapConfig) render() error {
9495
}
9596

9697
func expectedDeploymentName(proxyName string) string {
97-
return fmt.Sprintf("%s-%s", config.EnvoyDeploymentPrefix, proxyName)
98+
deploymentName := utils.GetHashedName(proxyName)
99+
return fmt.Sprintf("%s-%s", config.EnvoyPrefix, deploymentName)
98100
}
99101

100102
// expectedDeployment returns the expected Deployment based on the provided infra.

‎internal/infrastructure/kubernetes/service.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ import (
1414
"github.com/envoyproxy/gateway/internal/envoygateway/config"
1515
"github.com/envoyproxy/gateway/internal/gatewayapi"
1616
"github.com/envoyproxy/gateway/internal/ir"
17+
"github.com/envoyproxy/gateway/internal/provider/utils"
1718
)
1819

1920
func expectedServiceName(proxyName string) string {
20-
return fmt.Sprintf("%s-%s", config.EnvoyServicePrefix, proxyName)
21+
svcName := utils.GetHashedName(proxyName)
22+
return fmt.Sprintf("%s-%s", config.EnvoyPrefix, svcName)
2123
}
2224

2325
// expectedService returns the expected Service based on the provided infra.

‎internal/infrastructure/kubernetes/serviceaccount.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,15 @@ import (
99
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1010
"k8s.io/apimachinery/pkg/types"
1111

12+
"github.com/envoyproxy/gateway/internal/envoygateway/config"
1213
"github.com/envoyproxy/gateway/internal/gatewayapi"
1314
"github.com/envoyproxy/gateway/internal/ir"
14-
)
15-
16-
const (
17-
envoyServiceAccountPrefix = "envoy"
15+
"github.com/envoyproxy/gateway/internal/provider/utils"
1816
)
1917

2018
func expectedServiceAccountName(proxyName string) string {
21-
return fmt.Sprintf("%s-%s", envoyServiceAccountPrefix, proxyName)
19+
svcActName := utils.GetHashedName(proxyName)
20+
return fmt.Sprintf("%s-%s", config.EnvoyPrefix, svcActName)
2221
}
2322

2423
// expectedServiceAccount returns the expected proxy serviceAccount.

‎internal/infrastructure/kubernetes/serviceaccount_test.go‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func TestCreateOrUpdateServiceAccount(t *testing.T) {
7373
},
7474
ObjectMeta: metav1.ObjectMeta{
7575
Namespace: "test",
76-
Name: "envoy-test",
76+
Name: "envoy-test-74657374",
7777
Labels: map[string]string{
7878
"app.gateway.envoyproxy.io/name": "envoy",
7979
gatewayapi.OwningGatewayNamespaceLabel: "default",
@@ -118,7 +118,52 @@ func TestCreateOrUpdateServiceAccount(t *testing.T) {
118118
},
119119
ObjectMeta: metav1.ObjectMeta{
120120
Namespace: "test",
121-
Name: "envoy-test",
121+
Name: "envoy-test-74657374",
122+
Labels: map[string]string{
123+
"app.gateway.envoyproxy.io/name": "envoy",
124+
gatewayapi.OwningGatewayNamespaceLabel: "default",
125+
gatewayapi.OwningGatewayNameLabel: "gateway-1",
126+
},
127+
},
128+
},
129+
},
130+
{
131+
name: "hashed-name",
132+
ns: "test",
133+
in: &ir.Infra{
134+
Proxy: &ir.ProxyInfra{
135+
Name: "very-long-name-that-will-be-hashed-and-cut-off-because-its-too-long",
136+
Metadata: &ir.InfraMetadata{
137+
Labels: map[string]string{
138+
gatewayapi.OwningGatewayNamespaceLabel: "default",
139+
gatewayapi.OwningGatewayNameLabel: "gateway-1",
140+
},
141+
},
142+
},
143+
},
144+
current: &corev1.ServiceAccount{
145+
TypeMeta: metav1.TypeMeta{
146+
Kind: "ServiceAccount",
147+
APIVersion: "v1",
148+
},
149+
ObjectMeta: metav1.ObjectMeta{
150+
Namespace: "test",
151+
Name: "very-long-name-that-will-be-hashed-and-cut-off-because-its-too-long",
152+
Labels: map[string]string{
153+
"app.gateway.envoyproxy.io/name": "envoy",
154+
gatewayapi.OwningGatewayNamespaceLabel: "default",
155+
gatewayapi.OwningGatewayNameLabel: "gateway-1",
156+
},
157+
},
158+
},
159+
want: &corev1.ServiceAccount{
160+
TypeMeta: metav1.TypeMeta{
161+
Kind: "ServiceAccount",
162+
APIVersion: "v1",
163+
},
164+
ObjectMeta: metav1.ObjectMeta{
165+
Namespace: "test",
166+
Name: "envoy-very-long-name-that-will-be-hashed-and-cut-off-b-76657279",
122167
Labels: map[string]string{
123168
"app.gateway.envoyproxy.io/name": "envoy",
124169
gatewayapi.OwningGatewayNamespaceLabel: "default",

‎internal/provider/kubernetes/gateway.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -649,9 +649,11 @@ func (r *gatewayReconciler) subscribeAndUpdateStatus(ctx context.Context) {
649649
}
650650

651651
func infraServiceName(gateway *gwapiv1b1.Gateway) string {
652-
return fmt.Sprintf("%s-%s-%s", config.EnvoyServicePrefix, gateway.Namespace, gateway.Name)
652+
infraName := utils.GetHashedName(fmt.Sprintf("%s-%s", gateway.Namespace, gateway.Name))
653+
return fmt.Sprintf("%s-%s", config.EnvoyPrefix, infraName)
653654
}
654655

655656
func infraDeploymentName(gateway *gwapiv1b1.Gateway) string {
656-
return fmt.Sprintf("%s-%s-%s", config.EnvoyDeploymentPrefix, gateway.Namespace, gateway.Name)
657+
infraName := utils.GetHashedName(fmt.Sprintf("%s-%s", gateway.Namespace, gateway.Name))
658+
return fmt.Sprintf("%s-%s", config.EnvoyPrefix, infraName)
657659
}

‎internal/provider/kubernetes/gateway_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,23 @@ func TestGatewayHasMatchingController(t *testing.T) {
9999
},
100100
expect: false,
101101
},
102+
{
103+
name: "matching but very long name",
104+
obj: &gwapiv1b1.Gateway{
105+
TypeMeta: metav1.TypeMeta{
106+
Kind: "Gateway",
107+
APIVersion: fmt.Sprintf("%s/%s", gwapiv1b1.GroupName, gwapiv1b1.GroupVersion.Version),
108+
},
109+
ObjectMeta: metav1.ObjectMeta{
110+
Name: "superdupermegalongnamethatisridiculouslylongandwaylongerthanitshouldeverbeinsideofkubernetes",
111+
Namespace: "test",
112+
},
113+
Spec: gwapiv1b1.GatewaySpec{
114+
GatewayClassName: gwapiv1b1.ObjectName(match.Name),
115+
},
116+
},
117+
expect: true,
118+
},
102119
}
103120

104121
// Create the reconciler.

‎internal/provider/kubernetes/kubernetes_test.go‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,106 @@ func testGatewayScheduledStatus(ctx context.Context, t *testing.T, provider *Pro
254254
assert.Equal(t, gw.Spec, gws.Spec)
255255
}
256256

257+
// Test that even when resources such as the Service/Deployment get hashed names (because of a gateway with a very long name)
258+
func testLongNameHashedResources(ctx context.Context, t *testing.T, provider *Provider, resources *message.ProviderResources) {
259+
cli := provider.manager.GetClient()
260+
261+
gc := getGatewayClass("envoy-gateway-class")
262+
require.NoError(t, cli.Create(ctx, gc))
263+
264+
// Ensure the GatewayClass reports "Ready".
265+
require.Eventually(t, func() bool {
266+
if err := cli.Get(ctx, types.NamespacedName{Name: gc.Name}, gc); err != nil {
267+
return false
268+
}
269+
270+
for _, cond := range gc.Status.Conditions {
271+
if cond.Type == string(gwapiv1b1.GatewayClassConditionStatusAccepted) && cond.Status == metav1.ConditionTrue {
272+
return true
273+
}
274+
}
275+
276+
return false
277+
}, defaultWait, defaultTick)
278+
279+
defer func() {
280+
require.NoError(t, cli.Delete(ctx, gc))
281+
}()
282+
283+
// Create the namespace for the Gateway under test.
284+
ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "envoy-gateway"}}
285+
require.NoError(t, cli.Create(ctx, ns))
286+
287+
gw := &gwapiv1b1.Gateway{
288+
ObjectMeta: metav1.ObjectMeta{
289+
Name: "gatewaywithaverylongnamethatwillresultinhashedresources",
290+
Namespace: ns.Name,
291+
},
292+
Spec: gwapiv1b1.GatewaySpec{
293+
GatewayClassName: gwapiv1b1.ObjectName(gc.Name),
294+
Listeners: []gwapiv1b1.Listener{
295+
{
296+
Name: "test",
297+
Port: gwapiv1b1.PortNumber(int32(8080)),
298+
Protocol: gwapiv1b1.HTTPProtocolType,
299+
},
300+
},
301+
},
302+
}
303+
require.NoError(t, cli.Create(ctx, gw))
304+
305+
// Ensure the Gateway is ready and gets an address.
306+
ready := false
307+
hasAddress := false
308+
require.Eventually(t, func() bool {
309+
if err := cli.Get(ctx, types.NamespacedName{Namespace: gw.Namespace, Name: gw.Name}, gw); err != nil {
310+
return false
311+
}
312+
313+
for _, cond := range gw.Status.Conditions {
314+
fmt.Printf("Condition: %v", cond)
315+
if cond.Type == string(gwapiv1b1.GatewayConditionReady) && cond.Status == metav1.ConditionTrue {
316+
ready = true
317+
}
318+
}
319+
320+
if gw.Status.Addresses != nil {
321+
hasAddress = len(gw.Status.Addresses) >= 1
322+
}
323+
324+
return ready && hasAddress
325+
}, defaultWait, defaultTick)
326+
327+
defer func() {
328+
require.NoError(t, cli.Delete(ctx, gw))
329+
}()
330+
331+
// Ensure the gatewayclass has been finalized.
332+
require.NoError(t, cli.Get(ctx, types.NamespacedName{Name: gc.Name}, gc))
333+
require.Contains(t, gc.Finalizers, gatewayClassFinalizer)
334+
335+
// Ensure the number of Gateways in the Gateway resource table is as expected.
336+
require.Eventually(t, func() bool {
337+
return resources.Gateways.Len() == 1
338+
}, defaultWait, defaultTick)
339+
340+
// Ensure the test Gateway in the Gateway resources is as expected.
341+
key := types.NamespacedName{
342+
Namespace: gw.Namespace,
343+
Name: gw.Name,
344+
}
345+
require.Eventually(t, func() bool {
346+
return cli.Get(ctx, key, gw) == nil
347+
}, defaultWait, defaultTick)
348+
gws, _ := resources.Gateways.Load(key)
349+
// Only check if the spec is equal
350+
// The watchable map will not store a resource
351+
// with an updated status if the spec has not changed
352+
// to eliminate this endless loop:
353+
// reconcile->store->translate->update-status->reconcile
354+
assert.Equal(t, gw.Spec, gws.Spec)
355+
}
356+
257357
func testHTTPRoute(ctx context.Context, t *testing.T, provider *Provider, resources *message.ProviderResources) {
258358
cli := provider.manager.GetClient()
259359

0 commit comments

Comments
 (0)