mirror of https://github.com/linkerd/linkerd2.git
Stop ignoring the most significant labels of Destination names (#63)
Stop ignoring the most significant labels of Destination names Previously the destinations service was ignoring all the labels in a destination name after the first two labels. Thus, for example, "name.ns.another.domain.example.com" would be considered the same as "name.ns.svc.cluster.local". This was very wrong. Match destination names taking into consideration every label in the destination name. Provisions have been made for the case where the controller and the proxies with the zone name to use. However, currently neither the controller nor the proxies are actually configured with the zone, so the implementation was made to work in the current configuration too, as long as fully-qualified names are not used. A negative consequence of this change is that a name like "name.ns.svc.cluster.local" won't resolve in the current configuration, because the controller doesn't know the zone is "cluster.local" Unit tests are included for the new mapping rules. Signed-off-by: Brian Smith <brian@briansmith.org>
This commit is contained in:
Brian Smith
GitHub
parent
f7af375e73
commit
650dcdde1e
|
|
@ -17,6 +17,7 @@ func main() {
|
|||
addr := flag.String("addr", ":8089", "address to serve on")
|
||||
metricsAddr := flag.String("metrics-addr", ":9999", "address to serve scrapable metrics on")
|
||||
kubeConfigPath := flag.String("kubeconfig", "", "path to kube config")
|
||||
k8sDNSZone := flag.String("kubernetes-dns-zone", "", "The DNS suffix for the local Kubernetes zone.")
|
||||
flag.Parse()
|
||||
|
||||
log.SetLevel(log.DebugLevel) // TODO: make configurable
|
||||
|
|
@ -26,7 +27,7 @@ func main() {
|
|||
|
||||
done := make(chan struct{})
|
||||
|
||||
server, lis, err := destination.NewServer(*addr, *kubeConfigPath, done)
|
||||
server, lis, err := destination.NewServer(*addr, *kubeConfigPath, *k8sDNSZone, done)
|
||||
if err != nil {
|
||||
log.Fatal(err)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,17 +6,20 @@ import (
|
|||
"strconv"
|
||||
"strings"
|
||||
|
||||
"errors"
|
||||
common "github.com/runconduit/conduit/controller/gen/common"
|
||||
pb "github.com/runconduit/conduit/controller/gen/proxy/destination"
|
||||
"github.com/runconduit/conduit/controller/k8s"
|
||||
"github.com/runconduit/conduit/controller/util"
|
||||
log "github.com/sirupsen/logrus"
|
||||
"google.golang.org/grpc"
|
||||
"reflect"
|
||||
)
|
||||
|
||||
type (
|
||||
server struct {
|
||||
endpoints *k8s.EndpointsWatcher
|
||||
k8sDNSZoneLabels []string
|
||||
endpoints *k8s.EndpointsWatcher
|
||||
}
|
||||
)
|
||||
|
||||
|
|
@ -30,7 +33,7 @@ type (
|
|||
//
|
||||
// Addresses for the given destination are fetched from the Kubernetes Endpoints
|
||||
// API.
|
||||
func NewServer(addr, kubeconfig string, done chan struct{}) (*grpc.Server, net.Listener, error) {
|
||||
func NewServer(addr, kubeconfig string, k8sDNSZone string, done chan struct{}) (*grpc.Server, net.Listener, error) {
|
||||
clientSet, err := k8s.NewClientSet(kubeconfig)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
|
|
@ -42,8 +45,9 @@ func NewServer(addr, kubeconfig string, done chan struct{}) (*grpc.Server, net.L
|
|||
return nil, nil, err
|
||||
}
|
||||
|
||||
srv := &server{
|
||||
endpoints: endpoints,
|
||||
srv, err := newServer(k8sDNSZone, endpoints)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
lis, err := net.Listen("tcp", addr)
|
||||
|
|
@ -62,6 +66,24 @@ func NewServer(addr, kubeconfig string, done chan struct{}) (*grpc.Server, net.L
|
|||
return s, lis, nil
|
||||
}
|
||||
|
||||
// Split out from NewServer make it easy to write unit tests.
|
||||
func newServer(k8sDNSZone string, endpoints *k8s.EndpointsWatcher) (*server, error) {
|
||||
var k8sDNSZoneLabels []string
|
||||
if k8sDNSZone == "" {
|
||||
k8sDNSZoneLabels = []string{}
|
||||
} else {
|
||||
var err error
|
||||
k8sDNSZoneLabels, err = splitDNSName(k8sDNSZone)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
return &server{
|
||||
k8sDNSZoneLabels: k8sDNSZoneLabels,
|
||||
endpoints: endpoints,
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (s *server) Get(dest *common.Destination, stream pb.Destination_GetServer) error {
|
||||
log.Debugf("Get %v", dest)
|
||||
if dest.Scheme != "k8s" {
|
||||
|
|
@ -86,20 +108,25 @@ func (s *server) Get(dest *common.Destination, stream pb.Destination_GetServer)
|
|||
return err
|
||||
}
|
||||
}
|
||||
// service.namespace.svc.cluster.local
|
||||
domains := strings.Split(host, ".")
|
||||
|
||||
if len(domains) < 2 {
|
||||
err := fmt.Errorf("not a service: %s", host)
|
||||
id, err := s.localKubernetesServiceIdFromDNSName(host)
|
||||
if err != nil {
|
||||
log.Error(err)
|
||||
return err
|
||||
}
|
||||
|
||||
service := domains[0]
|
||||
namespace := domains[1]
|
||||
if id != nil {
|
||||
return s.resolveKubernetesService(*id, port, stream)
|
||||
}
|
||||
|
||||
id := namespace + "/" + service
|
||||
// TODO: Resolve name using DNS similar to Kubernetes' ClusterFirst
|
||||
// resolution.
|
||||
err = fmt.Errorf("cannot resolve service that isn't a local Kubernetes service: %s", host)
|
||||
log.Error(err)
|
||||
return err
|
||||
}
|
||||
|
||||
func (s *server) resolveKubernetesService(id string, port int, stream pb.Destination_GetServer) error {
|
||||
listener := endpointListener{stream: stream}
|
||||
|
||||
s.endpoints.Subscribe(id, uint32(port), listener)
|
||||
|
|
@ -111,6 +138,58 @@ func (s *server) Get(dest *common.Destination, stream pb.Destination_GetServer)
|
|||
return nil
|
||||
}
|
||||
|
||||
// localKubernetesServiceIdFromDNSName returns the name of the service in
|
||||
// "namespace-name/service-name" form if `host` is a DNS name in a form used
|
||||
// for local Kubernetes services. It returns nil if `host` isn't in such a
|
||||
// form.
|
||||
func (s *server) localKubernetesServiceIdFromDNSName(host string) (*string, error) {
|
||||
hostLabels, err := splitDNSName(host)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Verify that `host` ends with ".svc.$zone", ".svc.cluster.local," or ".svc".
|
||||
matched := false
|
||||
if len(s.k8sDNSZoneLabels) > 0 {
|
||||
hostLabels, matched = maybeStripSuffixLabels(hostLabels, s.k8sDNSZoneLabels)
|
||||
}
|
||||
// Accept "cluster.local" as an alias for "$zone". The Kubernetes DNS
|
||||
// specification
|
||||
// (https://github.com/kubernetes/dns/blob/master/docs/specification.md)
|
||||
// doesn't require Kubernetes to do this, but some hosting providers like
|
||||
// GKE do it, and so we need to support it for transparency.
|
||||
if !matched {
|
||||
hostLabels, matched = maybeStripSuffixLabels(hostLabels, []string{"cluster", "local"})
|
||||
}
|
||||
// TODO:
|
||||
// ```
|
||||
// if !matched {
|
||||
// return nil, nil
|
||||
// }
|
||||
// ```
|
||||
//
|
||||
// This is technically wrong since the protocol definition for the
|
||||
// Destination service indicates that `host` is a FQDN and so we should
|
||||
// never append a ".$zone" suffix to it, but we need to do this as a
|
||||
// workaround until the proxies are configured to know "$zone."
|
||||
hostLabels, matched = maybeStripSuffixLabels(hostLabels, []string{"svc"})
|
||||
if !matched {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// Extract the service name and namespace. TODO: Federated services have
|
||||
// *three* components before "svc"; see
|
||||
// https://github.com/runconduit/conduit/issues/156.
|
||||
if len(hostLabels) != 2 {
|
||||
return nil, fmt.Errorf("not a service: %s", host)
|
||||
}
|
||||
service := hostLabels[0]
|
||||
namespace := hostLabels[1]
|
||||
|
||||
id := namespace + "/" + service
|
||||
return &id, nil
|
||||
}
|
||||
|
||||
type endpointListener struct {
|
||||
stream pb.Destination_GetServer
|
||||
}
|
||||
|
|
@ -152,3 +231,37 @@ func toAddrSet(endpoints []common.TcpAddress) *pb.AddrSet {
|
|||
}
|
||||
return &pb.AddrSet{Addrs: addrs}
|
||||
}
|
||||
|
||||
func splitDNSName(dnsName string) ([]string, error) {
|
||||
// TODO: Validate that `dnsName` is a valid DNS name:
|
||||
// https://github.com/runconduit/conduit/issues/170.
|
||||
|
||||
// If the name is fully qualified, strip off the final dot.
|
||||
if strings.HasSuffix(dnsName, ".") {
|
||||
dnsName = dnsName[:len(dnsName)-1]
|
||||
}
|
||||
|
||||
labels := strings.Split(dnsName, ".")
|
||||
|
||||
// Rejects any empty labels, which is especially important to do for
|
||||
// the beginning and the end because we do matching based on labels'
|
||||
// relative positions. For example, we need to reject ".example.com"
|
||||
// instead of splitting it into ["", "example", "com"].
|
||||
for _, l := range labels {
|
||||
if l == "" {
|
||||
return []string{}, errors.New("Empty label in DNS name: " + dnsName)
|
||||
}
|
||||
}
|
||||
return labels, nil
|
||||
}
|
||||
|
||||
func maybeStripSuffixLabels(input []string, suffix []string) ([]string, bool) {
|
||||
n := len(input) - len(suffix)
|
||||
if n < 0 {
|
||||
return input, false
|
||||
}
|
||||
if !reflect.DeepEqual(input[n:], suffix) {
|
||||
return input, false
|
||||
}
|
||||
return input[:n], true
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,90 @@
|
|||
package destination
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestLocalKubernetesServiceIdFromDNSName(t *testing.T) {
|
||||
ns_name := "ns/name"
|
||||
|
||||
testCases := []struct {
|
||||
k8sDNSZone string
|
||||
host string
|
||||
result *string
|
||||
resultErr bool
|
||||
}{
|
||||
{"cluster.local", "", nil, true},
|
||||
{"cluster.local", "name", nil, false},
|
||||
{"cluster.local", "name.ns", nil, false},
|
||||
{"cluster.local", "name.ns.svc", &ns_name, false},
|
||||
{"cluster.local", "name.ns.pod", nil, false},
|
||||
{"cluster.local", "name.ns.other", nil, false},
|
||||
{"cluster.local", "name.ns.svc.cluster", nil, false},
|
||||
{"cluster.local", "name.ns.svc.cluster.local", &ns_name, false},
|
||||
{"cluster.local", "name.ns.svc.other.local", nil, false},
|
||||
{"cluster.local", "name.ns.pod.cluster.local", nil, false},
|
||||
{"cluster.local", "name.ns.other.cluster.local", nil, false},
|
||||
{"cluster.local", "name.ns.cluster.local", nil, false},
|
||||
{"cluster.local", "name.ns.svc.cluster", nil, false},
|
||||
{"cluster.local", "name.ns.svc.local", nil, false},
|
||||
{"cluster.local", "name.ns.svc.something.cluster.local", nil, false},
|
||||
{"cluster.local", "name.ns.svc.something.cluster.local", nil, false},
|
||||
{"cluster.local", "something.name.ns.svc.cluster.local", nil, true},
|
||||
{"k8s.example.com", "name.ns.svc.cluster.local", &ns_name, false},
|
||||
{"k8s.example.com", "name.ns.svc.cluster.local.k8s.example.com", nil, false},
|
||||
{"k8s.example.com", "name.ns.svc.k8s.example.com", &ns_name, false},
|
||||
{"k8s.example.com", "name.ns.svc.k8s.example.org", nil, false},
|
||||
{"cluster.local", "name.ns.svc.k8s.example.com", nil, false},
|
||||
{"", "name.ns.svc", &ns_name, false},
|
||||
{"", "name.ns.svc.cluster.local", &ns_name, false},
|
||||
{"", "name.ns.svc.cluster.local.", &ns_name, false},
|
||||
{"", "name.ns.svc.other.local", nil, false},
|
||||
{"example", "name.ns.svc.example", &ns_name, false},
|
||||
{"example", "name.ns.svc.example.", &ns_name, false},
|
||||
{"example", "name.ns.svc.example.com", nil, false},
|
||||
{"example", "name.ns.svc.cluster.local", &ns_name, false},
|
||||
|
||||
// XXX: See the comment about this issue in localKubernetesServiceIdFromDNSName.
|
||||
{"cluster.local", "name.ns.svc.", &ns_name, false},
|
||||
}
|
||||
|
||||
for i, tc := range testCases {
|
||||
t.Run(fmt.Sprintf("%d: (%s, %s)", i, tc.k8sDNSZone, tc.host), func(t *testing.T) {
|
||||
srv, err := newServer(tc.k8sDNSZone, nil)
|
||||
assert.Nil(t, err)
|
||||
result, err := srv.localKubernetesServiceIdFromDNSName(tc.host)
|
||||
assert.Equal(t, tc.result, result)
|
||||
assert.Equal(t, tc.resultErr, err != nil)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSplitDNSName(t *testing.T) {
|
||||
testCases := []struct {
|
||||
input string
|
||||
result []string
|
||||
resultErr bool
|
||||
}{
|
||||
{"example", []string{"example"}, false},
|
||||
{"example.", []string{"example"}, false},
|
||||
{"example.com", []string{"example", "com"}, false},
|
||||
{"example.com.", []string{"example", "com"}, false},
|
||||
{".example", []string{}, true},
|
||||
{".example.com", []string{}, true},
|
||||
{".example.com.", []string{}, true},
|
||||
{"example..com", []string{}, true},
|
||||
{"example.com..", []string{}, true},
|
||||
{"..example.com.", []string{}, true},
|
||||
{"foo.example.com", []string{"foo", "example", "com"}, false},
|
||||
}
|
||||
|
||||
for i, tc := range testCases {
|
||||
t.Run(fmt.Sprintf("%d: %s", i, tc.input), func(t *testing.T) {
|
||||
result, err := splitDNSName(tc.input)
|
||||
assert.Equal(t, tc.result, result)
|
||||
assert.Equal(t, tc.resultErr, err != nil)
|
||||
})
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue