Skip to content

Commit

Permalink
internal/contour: truncate object names to 60 characters
Browse files Browse the repository at this point in the history 
Fixes projectcontour#25

Envoy requires Name fields to be 60 characters or less. Rather than
doing the simple thing and using hashes as UUIDs everywhere, this PR
tries hard to preserve as much of the original name as possible, while
progressively truncating the longest fields to stay under the 60 char
limit.

Signed-off-by: Dave Cheney <dave@cheney.net>
  • Loading branch information
@davecheney
davecheney committed Nov 2, 2017
1 parent 5db06c7 commit 1b4f7a6
Show file tree
Hide file tree
Showing 6 changed files with 200 additions and 15 deletions.
11 changes: 5 additions & 6 deletions internal/contour/cds.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package contour

import (
"fmt"
"strconv"

"github.com/heptio/contour/internal/envoy"
"github.com/pkg/errors"
Expand Down Expand Up @@ -48,21 +49,19 @@ func ServiceToClusters(s *v1.Service) ([]envoy.Cluster, error) {
// service port is named, so we must generate both a cluster for the port name
// and a cluster for the port number.
clusters = append(clusters, envoy.Cluster{
Name: fmt.Sprintf("%s/%s/%s", s.ObjectMeta.Namespace, s.ObjectMeta.Name, p.Name),
Name: hashname(60, s.ObjectMeta.Namespace, s.ObjectMeta.Name, p.Name),
Type: "sds", // hard coded to specify SDS Endpoint discovery
ConnectTimeoutMs: defaultConnectTimeoutMs,
LBType: defaultLBType,
// TODO(dfc) need check if targetport is missing.
ServiceName: fmt.Sprintf("%s/%s/%s", s.ObjectMeta.Namespace, s.ObjectMeta.Name, p.TargetPort.String()),
ServiceName: fmt.Sprintf("%s/%s/%s", s.ObjectMeta.Namespace, s.ObjectMeta.Name, p.TargetPort.String()),
})
}
clusters = append(clusters, envoy.Cluster{
Name: fmt.Sprintf("%s/%s/%d", s.ObjectMeta.Namespace, s.ObjectMeta.Name, p.Port),
Name: hashname(60, s.ObjectMeta.Namespace, s.ObjectMeta.Name, strconv.Itoa(int(p.Port))),
Type: "sds", // hard coded to specify SDS Endpoint discovery
ConnectTimeoutMs: defaultConnectTimeoutMs,
LBType: defaultLBType,
// TODO(dfc) need check if targetport is missing.
ServiceName: fmt.Sprintf("%s/%s/%s", s.ObjectMeta.Namespace, s.ObjectMeta.Name, p.TargetPort.String()),
ServiceName: fmt.Sprintf("%s/%s/%s", s.ObjectMeta.Namespace, s.ObjectMeta.Name, p.TargetPort.String()),
})
default:
// ignore UDP and other port types.
Expand Down
25 changes: 25 additions & 0 deletions internal/contour/cds_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,31 @@ func TestServiceToClusters(t *testing.T) {
LBType: "round_robin",
ServiceName: "default/simple/6502",
}},
}, {
name: "long namespace and service name",
s: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "tiny-cog-department-test-instance",
Namespace: "beurocratic-company-test-domain-1",
},
Spec: v1.ServiceSpec{
Selector: map[string]string{
"app": "simple",
},
Ports: []v1.ServicePort{{
Protocol: "TCP",
Port: 80,
TargetPort: intstr.FromInt(6502),
}},
},
},
want: []envoy.Cluster{{
Name: "beurocratic-company-test-domain-1/tiny-cog-depa-52e801/80",
Type: "sds",
ConnectTimeoutMs: 250,
LBType: "round_robin",
ServiceName: "beurocratic-company-test-domain-1/tiny-cog-department-test-instance/6502", // ServiceName is not subject to the 60 char limit
}},
}, {
name: "single named service port",
s: &v1.Service{
Expand Down
51 changes: 51 additions & 0 deletions internal/contour/json.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@
package contour

import (
"crypto/sha256"
"encoding/json"
"fmt"
"io"
"net/http"
"net/http/pprof"
"strconv"
"strings"

"k8s.io/api/core/v1"
"k8s.io/api/extensions/v1beta1"
Expand Down Expand Up @@ -183,3 +186,51 @@ func (a *jsonAPI) writeJSON(w io.Writer, v interface{}) {
a.Errorf("failed to encode JSON: %v", err)
}
}

// hashname takes a lenth l and a varargs of strings s and returns a string whose length
// which does not exceed l. Internally s is joined with strings.Join(s, "/"). If the
// combined length exceeds l then hashname truncates each element in s, starting from the
// end using a hash derived from the contents of s (not the current element). This process
// continues until the length of s does not exceed l, or all elements have been truncated.
// In which case, the entire string is replaced with a hash not exceeding the length of l.
func hashname(l int, s ...string) string {
const shorthash = 6 // the length of the shorthash

r := strings.Join(s, "/")
if l > len(r) {
// we're under the limit, nothing to do
return r
}
hash := fmt.Sprintf("%x", sha256.Sum256([]byte(r)))
for n := len(s) - 1; n >= 0; n-- {
s[n] = truncate(l/len(s), s[n], hash[:shorthash])
r = strings.Join(s, "/")
if l > len(r) {
return r
}
}
// truncated everything, but we're still too long
// just return the hash truncated to l.
return hash[:min(len(hash), l)]
}

// truncate truncates s to l length by replacing the
// end of s with -suffix.
func truncate(l int, s, suffix string) string {
if l >= len(s) {
// under the limit, nothing to do
return s
}
if l <= len(suffix) {
// easy case, just return the start of the suffix
return suffix[:min(l, len(suffix))]
}
return s[:l-len(suffix)-1] + "-" + suffix
}

func min(a, b int) int {
if a > b {
return b
}
return a
}
79 changes: 79 additions & 0 deletions internal/contour/json_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,31 @@ func TestAPIServer(t *testing.T) {
}},
path: "/v1/routes/ingress_http/cluster0/node0",
want: `{"virtual_hosts":[{"name":"default/httbin-org","domains":["*"],"routes":[{"prefix":"/","cluster":"default/backend/80"}]}]}` + "\n",
}, {
name: "rds/long vhost name",
ingresses: []*v1beta1.Ingress{{
ObjectMeta: metav1.ObjectMeta{
Name: "my-service-name",
Namespace: "default",
},
Spec: v1beta1.IngressSpec{
Rules: []v1beta1.IngressRule{{
Host: "my-very-very-long-service-host-name.my.domainname",
IngressRuleValue: v1beta1.IngressRuleValue{
HTTP: &v1beta1.HTTPIngressRuleValue{
Paths: []v1beta1.HTTPIngressPath{{
Backend: v1beta1.IngressBackend{
ServiceName: "my-service-name",
ServicePort: intstr.FromInt(8080),
},
}},
},
},
}},
},
}},
path: "/v1/routes/ingress_http/cluster0/node0",
want: `{"virtual_hosts":[{"name":"default/my-service-name/my-very-very--c4d2d4","domains":["my-very-very-long-service-host-name.my.domainname"],"routes":[{"prefix":"/","cluster":"default/my-service-name/8080"}]}]}` + "\n",
}, {
name: "rds/ingress class",
ingresses: []*v1beta1.Ingress{{
Expand Down Expand Up @@ -358,6 +383,60 @@ func TestAPIServer(t *testing.T) {
}
}

func TestHashname(t *testing.T) {
tests := []struct {
name string
l int
s []string
want string
}{
{name: "empty s", l: 99, s: nil, want: ""},
{name: "single element", l: 99, s: []string{"alpha"}, want: "alpha"},
{name: "long single element, hashed", l: 12, s: []string{"gammagammagamma"}, want: "0d350ea5c204"},
{name: "single element, truncated", l: 4, s: []string{"alpha"}, want: "8ed3"},
{name: "two elements, truncated", l: 19, s: []string{"gammagamma", "betabeta"}, want: "ga-edf159/betabeta"},
{name: "three elements", l: 99, s: []string{"alpha", "beta", "gamma"}, want: "alpha/beta/gamma"},
{name: "issue/25", l: 60, s: []string{"default", "my-sevice-name", "my-very-very-long-service-host-name.my.domainname"}, want: "default/my-sevice-name/my-very-very--665863"},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := hashname(tc.l, append([]string{}, tc.s...)...)
if got != tc.want {
t.Fatalf("hashname(%d, %q): got %q, want %q", tc.l, tc.s, got, tc.want)
}
})
}
}

func TestTruncate(t *testing.T) {
tests := []struct {
name string
l int
s string
suffix string
want string
}{
{name: "no truncate", l: 10, s: "quijibo", suffix: "a8c5e6", want: "quijibo"},
{name: "limit", l: len("quijibo"), s: "quijibo", suffix: "a8c5e6", want: "quijibo"},
{name: "truncate some", l: 6, s: "quijibo", suffix: "a8c5", want: "q-a8c5"},
{name: "truncate suffix", l: 4, s: "quijibo", suffix: "a8c5", want: "a8c5"},
{name: "truncate more", l: 3, s: "quijibo", suffix: "a8c5", want: "a8c"},
{name: "long single element, truncated", l: 9, s: "gammagamma", suffix: "0d350e", want: "ga-0d350e"},
{name: "long single element, truncated", l: 12, s: "gammagammagamma", suffix: "0d350e", want: "gamma-0d350e"},
{name: "issue/25", l: 60 / 3, s: "my-very-very-long-service-host-name.my.domainname", suffix: "a8c5e6", want: "my-very-very--a8c5e6"},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := truncate(tc.l, tc.s, tc.suffix)
if got != tc.want {
t.Fatalf("hashname(%d, %q, %q): got %q, want %q", tc.l, tc.s, tc.suffix, got, tc.want)
}
})
}
}

type discardWriter struct{}

func (w discardWriter) Write(buf []byte) (int, error) { return len(buf), nil }
Expand Down
6 changes: 3 additions & 3 deletions internal/contour/rds.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func IngressToVirtualHosts(i *v1beta1.Ingress) ([]*envoy.VirtualHost, error) {
// validateIngress ensures this.
if i.Spec.Backend != nil {
v := envoy.VirtualHost{
Name: i.Namespace + "/" + i.Name,
Name: hashname(60, i.Namespace, i.Name),
}
v.AddDomain("*")
v.AddRoute(envoy.Route{
Expand All @@ -53,7 +53,7 @@ func IngressToVirtualHosts(i *v1beta1.Ingress) ([]*envoy.VirtualHost, error) {
var vhosts []*envoy.VirtualHost
for _, rule := range i.Spec.Rules {
v := envoy.VirtualHost{
Name: i.Namespace + "/" + i.Name + "/" + rule.Host,
Name: hashname(60, i.Namespace, i.Name, rule.Host),
}
v.AddDomain(rule.Host)
if rule.IngressRuleValue.HTTP == nil {
Expand Down Expand Up @@ -95,7 +95,7 @@ func validateIngress(i *v1beta1.Ingress) error {

// ingressBackendToClusterName renders a cluster name from an Ingress and an IngressBackend.
func ingressBackendToClusterName(i *v1beta1.Ingress, b *v1beta1.IngressBackend) string {
return i.ObjectMeta.Namespace + "/" + b.ServiceName + "/" + b.ServicePort.String()
return hashname(60, i.ObjectMeta.Namespace, b.ServiceName, b.ServicePort.String())
}

// pathToRoute converts a HTTPIngressPath to a partial envoy.Route.
Expand Down
43 changes: 37 additions & 6 deletions internal/contour/rds_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,37 @@ func TestIngressToVirtualHost(t *testing.T) {
Cluster: "default/paul/paul",
}),
}},
}, {
name: "vhost name exceeds 60 chars", // heptio/contour#25
i: &v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "my-service-name",
Namespace: "default",
},
Spec: v1beta1.IngressSpec{
Rules: []v1beta1.IngressRule{{
Host: "my-very-very-long-service-host-name.my.domainname",
IngressRuleValue: v1beta1.IngressRuleValue{
HTTP: &v1beta1.HTTPIngressRuleValue{
Paths: []v1beta1.HTTPIngressPath{{
Backend: v1beta1.IngressBackend{
ServiceName: "my-service-name",
ServicePort: intstr.FromInt(80),
},
}},
},
},
}},
},
},
want: []*envoy.VirtualHost{{
Name: "default/my-service-name/my-very-very--c4d2d4",
Domains: domains("my-very-very-long-service-host-name.my.domainname"),
Routes: routes(envoy.Route{
Prefix: "/",
Cluster: "default/my-service-name/80",
}),
}},
}}

for _, tc := range tests {
Expand All @@ -289,7 +320,7 @@ func TestIngressToVirtualHost(t *testing.T) {
t.Fatal(err)
}
if !reflect.DeepEqual(got, tc.want) {
t.Fatalf("got: %#v, want: %#v", got, tc.want)
t.Fatalf("got: %v, want: %v", got, tc.want)
}
})
}
Expand Down Expand Up @@ -355,27 +386,27 @@ func TestIngressBackendToVirtualHostName(t *testing.T) {
func TestValidateIngress(t *testing.T) {
tests := []struct {
name string
i *v1beta1.Ingress
i v1beta1.Ingress
want error
}{{
name: "missing Ingress.Meta.Name",
i: &v1beta1.Ingress{
i: v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "",
},
},
want: errors.New("Ingress.Meta.Name is blank"),
}, {
name: "missing Ingress.Meta.Namespace",
i: &v1beta1.Ingress{
i: v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "simple",
},
},
want: errors.New("Ingress.Meta.Namespace is blank"),
}, {
name: "missing Ingress.Spec.Backend and Ingress.Spec.Rules",
i: &v1beta1.Ingress{
i: v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "simple",
Namespace: "default",
Expand All @@ -386,7 +417,7 @@ func TestValidateIngress(t *testing.T) {

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := validateIngress(tc.i)
got := validateIngress(&tc.i)
if tc.want != nil && got == nil || got.Error() != tc.want.Error() {
t.Errorf("got: %v, expected: %v", tc.want, got)
}
Expand Down

0 comments on commit 1b4f7a6

Please sign in to comment.