Skip to content

Commit

Permalink
gateway-api: shorten the length of the value of the svc's label.
Browse files Browse the repository at this point in the history 
Fixes #31285

When creating a gateway-api with a name exceeding 64 characters, it is impossible to create svc.

This is because the label of svc references the name of gateway-api.

Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
  • Loading branch information
@chaunceyjiang @sayboras
chaunceyjiang authored and sayboras committed Mar 11, 2024
1 parent c095caa commit d6fbccf
Show file tree
Hide file tree
Showing 5 changed files with 213 additions and 42 deletions.
2 changes: 1 addition & 1 deletion operator/pkg/gateway-api/gateway_reconcile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ func isAttachable(_ context.Context, gw *gatewayv1.Gateway, route metav1.Object,
func (r *gatewayReconciler) setAddressStatus(ctx context.Context, gw *gatewayv1.Gateway) error {
svcList := &corev1.ServiceList{}
if err := r.Client.List(ctx, svcList, client.MatchingLabels{
owningGatewayLabel: gw.GetName(),
owningGatewayLabel: model.Shorten(gw.GetName()),
}); err != nil {
return err
}
Expand Down
105 changes: 104 additions & 1 deletion operator/pkg/gateway-api/gateway_reconcile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ var gwFixture = []client.Object{
},
},
},
&corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "cilium-gateway-test-long-long-long-long-long-long-lo-8tfth549c6",
Namespace: "long-name-test",
Annotations: map[string]string{
"pre-existing-annotation": "true",
},
},
},

// Service in another namespace
&corev1.Service{
Expand Down Expand Up @@ -180,7 +189,28 @@ var gwFixture = []client.Object{
},
},
},

// Valid gateway
&gatewayv1.Gateway{
TypeMeta: metav1.TypeMeta{
Kind: "Gateway",
APIVersion: gatewayv1.GroupName,
},
ObjectMeta: metav1.ObjectMeta{
Name: "test-long-long-long-long-long-long-long-long-long-long-long-long-name",
Namespace: "long-name-test",
},
Spec: gatewayv1.GatewaySpec{
GatewayClassName: "cilium",
Listeners: []gatewayv1.Listener{
{
Name: "http",
Port: 80,
Hostname: model.AddressOf[gatewayv1.Hostname]("*.cilium.io"),
Protocol: "HTTP",
},
},
},
},
// gateway with non-existent gateway class
&gatewayv1.Gateway{
TypeMeta: metav1.TypeMeta{
Expand Down Expand Up @@ -351,6 +381,79 @@ func Test_gatewayReconciler_Reconcile(t *testing.T) {
require.Equal(t, "True", string(gw.Status.Listeners[0].Conditions[2].Status))
})

t.Run("valid http gateway - long name", func(t *testing.T) {
key := client.ObjectKey{
Namespace: "long-name-test",
Name: "test-long-long-long-long-long-long-long-long-long-long-long-long-name",
}
result, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: key})

// First reconcile should wait for LB status before writing addresses into Ingress status
require.NoError(t, err)
require.Equal(t, ctrl.Result{}, result)

gw := &gatewayv1.Gateway{}
err = c.Get(context.Background(), key, gw)
require.NoError(t, err)
require.Empty(t, gw.Status.Addresses)

// Simulate LB service update
lb := &corev1.Service{}
err = c.Get(context.Background(), client.ObjectKey{Namespace: "long-name-test", Name: "cilium-gateway-test-long-long-long-long-long-long-lo-8tfth549c6"}, lb)
require.NoError(t, err)
require.Equal(t, corev1.ServiceTypeLoadBalancer, lb.Spec.Type)
require.Equal(t, "test-long-long-long-long-long-long-long-long-long-lo-4bftbgh5ht", lb.Labels["io.cilium.gateway/owning-gateway"])
require.Equal(t, "true", lb.Annotations["pre-existing-annotation"])

// Update LB status
lb.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{
{
IP: "10.10.10.20",
Ports: []corev1.PortStatus{
{
Port: 80,
Protocol: "TCP",
},
},
},
}
err = c.Status().Update(context.Background(), lb)
require.NoError(t, err)

// Perform second reconciliation
result, err = r.Reconcile(context.Background(), ctrl.Request{NamespacedName: key})
require.NoError(t, err)
require.Equal(t, ctrl.Result{}, result)

// Check that the gateway status has been updated
err = c.Get(context.Background(), key, gw)
require.NoError(t, err)

require.Len(t, gw.Status.Conditions, 2)
require.Equal(t, "Accepted", gw.Status.Conditions[0].Type)
require.Equal(t, "True", string(gw.Status.Conditions[0].Status))
require.Equal(t, "Gateway successfully scheduled", gw.Status.Conditions[0].Message)
require.Equal(t, "Programmed", gw.Status.Conditions[1].Type)
require.Equal(t, "True", string(gw.Status.Conditions[1].Status))
require.Equal(t, "Gateway successfully reconciled", gw.Status.Conditions[1].Message)

require.Len(t, gw.Status.Addresses, 1)
require.Equal(t, "IPAddress", string(*gw.Status.Addresses[0].Type))
require.Equal(t, "10.10.10.20", gw.Status.Addresses[0].Value)

require.Len(t, gw.Status.Listeners, 1)
require.Equal(t, "http", string(gw.Status.Listeners[0].Name))
require.Len(t, gw.Status.Listeners[0].Conditions, 3)
require.Equal(t, "Programmed", gw.Status.Listeners[0].Conditions[0].Type)
require.Equal(t, "True", string(gw.Status.Listeners[0].Conditions[0].Status))
require.Equal(t, "Programmed", gw.Status.Listeners[0].Conditions[0].Reason)
require.Equal(t, "Listener Programmed", gw.Status.Listeners[0].Conditions[0].Message)
require.Equal(t, "Accepted", gw.Status.Listeners[0].Conditions[1].Type)
require.Equal(t, "True", string(gw.Status.Listeners[0].Conditions[1].Status))
require.Equal(t, "ResolvedRefs", gw.Status.Listeners[0].Conditions[2].Type)
require.Equal(t, "True", string(gw.Status.Listeners[0].Conditions[2].Status))
})

t.Run("valid tls gateway", func(t *testing.T) {
key := client.ObjectKey{
Namespace: "default",
Expand Down
37 changes: 37 additions & 0 deletions operator/pkg/model/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package model

import (
"crypto/sha256"
"fmt"
"sort"
"strings"

Expand Down Expand Up @@ -89,3 +91,38 @@ func hostnameMatchesWildcardHostname(hostname, wildcardHostname string) bool {
func AddressOf[T any](v T) *T {
return &v
}

// Shorten shortens the string to 63 characters.
// this is the implicit required for all the resource naming in k8s.
func Shorten(s string) string {
if len(s) > 63 {
return s[:52] + "-" + encodeHash(hash(s))
}
return s
}

// encodeHash encodes the first 10 characters of the hex string.
// https://github.com/kubernetes/kubernetes/blob/f0dcf0614036d8c3cd1c9f3b3cf8df4bb1d8e44e/staging/src/k8s.io/kubectl/pkg/util/hash/hash.go#L105
func encodeHash(hex string) string {
enc := []rune(hex[:10])
for i := range enc {
switch enc[i] {
case '0':
enc[i] = 'g'
case '1':
enc[i] = 'h'
case '3':
enc[i] = 'k'
case 'a':
enc[i] = 'm'
case 'e':
enc[i] = 't'
}
}
return string(enc)
}

// hash hashes `data` with sha256 and returns the hex string
func hash(data string) string {
return fmt.Sprintf("%x", sha256.Sum256([]byte(data)))
}
44 changes: 4 additions & 40 deletions operator/pkg/model/translation/gateway-api/translator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package gateway_api

import (
"crypto/sha256"
"fmt"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -105,9 +104,9 @@ func getService(resource *model.FullyQualifiedResource, allPorts []uint32, label

return &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: shorten(ciliumGatewayPrefix + resource.Name),
Name: model.Shorten(ciliumGatewayPrefix + resource.Name),
Namespace: resource.Namespace,
Labels: mergeMap(map[string]string{owningGatewayLabel: resource.Name}, labels),
Labels: mergeMap(map[string]string{owningGatewayLabel: model.Shorten(resource.Name)}, labels),
Annotations: annotations,
OwnerReferences: []metav1.OwnerReference{
{
Expand All @@ -129,9 +128,9 @@ func getService(resource *model.FullyQualifiedResource, allPorts []uint32, label
func getEndpoints(resource model.FullyQualifiedResource) *corev1.Endpoints {
return &corev1.Endpoints{
ObjectMeta: metav1.ObjectMeta{
Name: shorten(ciliumGatewayPrefix + resource.Name),
Name: model.Shorten(ciliumGatewayPrefix + resource.Name),
Namespace: resource.Namespace,
Labels: map[string]string{owningGatewayLabel: resource.Name},
Labels: map[string]string{owningGatewayLabel: model.Shorten(resource.Name)},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: gatewayv1beta1.GroupVersion.String(),
Expand All @@ -154,41 +153,6 @@ func getEndpoints(resource model.FullyQualifiedResource) *corev1.Endpoints {
}
}

// shorten shortens the string to 63 characters.
// this is the implicit required for all the resource naming in k8s.
func shorten(s string) string {
if len(s) > 63 {
return s[:52] + "-" + encodeHash(hash(s))
}
return s
}

// encodeHash encodes the first 10 characters of the hex string.
// https://github.com/kubernetes/kubernetes/blob/f0dcf0614036d8c3cd1c9f3b3cf8df4bb1d8e44e/staging/src/k8s.io/kubectl/pkg/util/hash/hash.go#L105
func encodeHash(hex string) string {
enc := []rune(hex[:10])
for i := range enc {
switch enc[i] {
case '0':
enc[i] = 'g'
case '1':
enc[i] = 'h'
case '3':
enc[i] = 'k'
case 'a':
enc[i] = 'm'
case 'e':
enc[i] = 't'
}
}
return string(enc)
}

// hash hashes `data` with sha256 and returns the hex string
func hash(data string) string {
return fmt.Sprintf("%x", sha256.Sum256([]byte(data)))
}

func mergeMap(left, right map[string]string) map[string]string {
if left == nil {
return right
Expand Down
67 changes: 67 additions & 0 deletions operator/pkg/model/translation/gateway-api/translator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package gateway_api

import (
"fmt"
"testing"

envoy_config_route_v3 "github.com/cilium/proxy/go/envoy/config/route/v3"
Expand All @@ -12,6 +13,9 @@ import (
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/testing/protocmp"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1"

"github.com/cilium/cilium/operator/pkg/model"
"github.com/cilium/cilium/operator/pkg/model/translation"
Expand Down Expand Up @@ -448,3 +452,66 @@ func Test_translator_Translate_WithXffNumTrustedHops(t *testing.T) {
})
}
}

func Test_getService(t *testing.T) {
type args struct {
resource *model.FullyQualifiedResource
allPorts []uint32
labels map[string]string
annotations map[string]string
}
tests := []struct {
name string
args args
want *corev1.Service
}{
{
name: "long name - more than 64 characters",
args: args{
resource: &model.FullyQualifiedResource{
Name: "test-long-long-long-long-long-long-long-long-long-long-long-long-name",
Namespace: "default",
Version: "v1",
Kind: "Gateway",
UID: "57889650-380b-4c05-9a2e-3baee7fd5271",
},
allPorts: []uint32{80},
},
want: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "cilium-gateway-test-long-long-long-long-long-long-lo-8tfth549c6",
Namespace: "default",
Labels: map[string]string{
owningGatewayLabel: "test-long-long-long-long-long-long-long-long-long-lo-4bftbgh5ht",
},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: gatewayv1beta1.GroupVersion.String(),
Kind: "Gateway",
Name: "test-long-long-long-long-long-long-long-long-long-long-long-long-name",
UID: types.UID("57889650-380b-4c05-9a2e-3baee7fd5271"),
Controller: model.AddressOf(true),
},
},
},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{
Name: fmt.Sprintf("port-%d", 80),
Port: 80,
Protocol: corev1.ProtocolTCP,
},
},
Type: corev1.ServiceTypeLoadBalancer,
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := getService(tt.args.resource, tt.args.allPorts, tt.args.labels, tt.args.annotations)
assert.Equalf(t, tt.want, got, "getService(%v, %v, %v, %v)", tt.args.resource, tt.args.allPorts, tt.args.labels, tt.args.annotations)
assert.Equal(t, true, len(got.Name) <= 63, "Service name is too long")
})
}
}

0 comments on commit d6fbccf

Please sign in to comment.