Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gateway-api: shorten the length of the value of the svc's label. #31292

Merged
merged 1 commit into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion operator/pkg/gateway-api/gateway_reconcile.go
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
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
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
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
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")
})
}
}