From 165ef197b4ed38cee5c9d04c1e6aa59c6c5886d1 Mon Sep 17 00:00:00 2001 From: whalecold Date: Wed, 27 Dec 2023 15:29:45 +0800 Subject: [PATCH 1/5] feat(fqdn): support using short name access service --- core/manager/bootstrap.go | 46 +++++++++++++++++++++++++++++++------ core/manager/client.go | 28 ++++++++++++++++++---- core/manager/client_test.go | 42 +++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 11 deletions(-) diff --git a/core/manager/bootstrap.go b/core/manager/bootstrap.go index f6bdd9d..ed6c1ec 100644 --- a/core/manager/bootstrap.go +++ b/core/manager/bootstrap.go @@ -19,6 +19,7 @@ package manager import ( "fmt" "os" + "strings" "time" "github.com/cloudwego/kitex/pkg/klog" @@ -30,6 +31,7 @@ import ( const ( PodNamespace = "POD_NAMESPACE" PodName = "POD_NAME" + MetaNamespace = "NAMESPACE" InstanceIP = "INSTANCE_IP" IstiodAddr = "istiod.istio-system.svc:15010" KitexXdsDomain = "KITEX_XDS_DOMAIN" @@ -40,8 +42,12 @@ const ( ) type BootstrapConfig struct { - node *v3core.Node - xdsSvrCfg *XDSServerConfig + // The namespace to make up fqdn. + // Use POD_NAMESPACE default, the meta namespace will override that if set. + configNamespace string + nodeDomain string + node *v3core.Node + xdsSvrCfg *XDSServerConfig } type XDSServerConfig struct { @@ -88,6 +94,23 @@ func nodeId(podIP, podName, namespace, nodeDomain string) string { return fmt.Sprintf("sidecar~%s~%s.%s~%s.svc.%s", podIP, podName, namespace, namespace, nodeDomain) } +// tryExpandFQDN try expand fully qualified domain name. +func (bc *BootstrapConfig) tryExpandFQDN(host string) string { + // The kubernetes services following the ..svc. naming convention + // and that share a suffix with the domain. If it already been expanded, ignore it. + if strings.Contains(host, ".svc.") { + return host + } + var b strings.Builder + b.Grow(len(host) + len(bc.configNamespace) + len(bc.nodeDomain) + 10) + b.WriteString(host) + b.WriteString(".") + b.WriteString(bc.configNamespace) + b.WriteString(".svc.") + b.WriteString(bc.nodeDomain) + return b.String() +} + // newBootstrapConfig constructs the bootstrapConfig func newBootstrapConfig(config *XDSServerConfig) (*BootstrapConfig, error) { // Get info from env @@ -114,13 +137,22 @@ func newBootstrapConfig(config *XDSServerConfig) (*BootstrapConfig, error) { nodeDomain = "cluster.local" } - metaEnvs := os.Getenv(KitexXdsMetas) - - return &BootstrapConfig{ + bsConfig := &BootstrapConfig{ + nodeDomain: nodeDomain, + configNamespace: namespace, node: &v3core.Node{ Id: nodeId(podIP, podName, namespace, nodeDomain), - Metadata: parseMetaEnvs(metaEnvs, istioVersion), + Metadata: parseMetaEnvs(os.Getenv(KitexXdsMetas), istioVersion), }, xdsSvrCfg: config, - }, nil + } + + // the priority of NAMESPACE in metadata is higher than POD_NAMESPACE. + // ref: https://github.com/istio/istio/blob/30446a7b88aba4a0fcd5f71bae8d397a874e846f/pilot/pkg/model/context.go#L1024 + if field, ok := bsConfig.node.Metadata.Fields[MetaNamespace]; ok { + if val := field.GetStringValue(); val != "" { + bsConfig.configNamespace = val + } + } + return bsConfig, nil } diff --git a/core/manager/client.go b/core/manager/client.go index 2f65c92..d6d0bc1 100644 --- a/core/manager/client.go +++ b/core/manager/client.go @@ -385,6 +385,27 @@ func (c *xdsClient) sendRequest(req *discoveryv3.DiscoveryRequest) { c.reqCh <- req } +func (c *xdsClient) resolveAddr(host string) string { + // In the worst case, lookupHost is called twice, try to reduce it. + // May exists three kind host: + // 1. fqdn host in Kubernetes, such as example.default.svc.cluster.local, invoke once always. + // 2. short name in Kubernetes, such as example, invoke once when the host exists in cipResolver, and twice when the host does not exist in cipResolver. + // 3. service outside Kubernetes, such as www.example.com, invoke twice always. + // FIXME: format as . is not supported. + fqdn := c.config.tryExpandFQDN(host) + cip, ok := c.cipResolver.lookupHost(fqdn) + if ok && len(cip) > 0 { + return cip[0] + } + if fqdn != host { + cip, ok := c.cipResolver.lookupHost(host) + if ok && len(cip) > 0 { + return cip[0] + } + } + return "" +} + // getListenerName returns the listener name in this format: ${clusterIP}_${port} // lookup the clusterIP using the cipResolver and return the listenerName func (c *xdsClient) getListenerName(rName string) (string, error) { @@ -393,10 +414,9 @@ func (c *xdsClient) getListenerName(rName string) (string, error) { return "", fmt.Errorf("invalid listener name: %s", rName) } addr, port := tmp[0], tmp[1] - cip, ok := c.cipResolver.lookupHost(addr) - if ok && len(cip) > 0 { - clusterIPPort := cip[0] + "_" + port - return clusterIPPort, nil + cip := c.resolveAddr(addr) + if len(cip) > 0 { + return cip + "_" + port, nil } return "", fmt.Errorf("failed to convert listener name for %s", rName) } diff --git a/core/manager/client_test.go b/core/manager/client_test.go index 214b26b..ef2a910 100644 --- a/core/manager/client_test.go +++ b/core/manager/client_test.go @@ -218,3 +218,45 @@ func TestClearCh(t *testing.T) { clearRequestCh(ch, 10) assert.Equal(t, 0, len(ch)) } + +func TestResolveAddr(t *testing.T) { + resolver := newNdsResolver() + resolver.updateLookupTable(map[string][]string{ + "echoa.default.svc.cluster.local": {"1.1.1.1"}, + "echob.default.svc.cluster.local": {"1.1.1.1"}, + }) + cli := &xdsClient{ + cipResolver: resolver, + config: &BootstrapConfig{ + configNamespace: "default", + nodeDomain: "cluster.local", + }, + } + testCases := []struct { + desc string + host string + want string + }{ + { + desc: "fqdn", + host: "echoa.default.svc.cluster.local", + want: "1.1.1.1", + }, + { + desc: "short name", + host: "echoa", + want: "1.1.1.1", + }, + { + desc: "not found", + host: "echoa.default", + want: "", + }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + got := cli.resolveAddr(tc.host) + assert.Equal(t, tc.want, got) + }) + } +} From 41bf97e63b366758a466cd7326a1019e5348c178 Mon Sep 17 00:00:00 2001 From: whalecold Date: Wed, 3 Jan 2024 16:22:44 +0800 Subject: [PATCH 2/5] support parser the host contains namespace --- core/manager/bootstrap.go | 11 ++++++-- core/manager/bootstrap_test.go | 46 ++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/core/manager/bootstrap.go b/core/manager/bootstrap.go index ed6c1ec..fabb5d7 100644 --- a/core/manager/bootstrap.go +++ b/core/manager/bootstrap.go @@ -101,11 +101,18 @@ func (bc *BootstrapConfig) tryExpandFQDN(host string) string { if strings.Contains(host, ".svc.") { return host } + var b strings.Builder b.Grow(len(host) + len(bc.configNamespace) + len(bc.nodeDomain) + 10) b.WriteString(host) - b.WriteString(".") - b.WriteString(bc.configNamespace) + + // the regex for Kubernetes service is [a-z]([-a-z0-9]*[a-z0-9])?, it should not contains `.` in it + // if the host not contains namespace. + if !strings.Contains(host, ".") { + b.WriteString(".") + b.WriteString(bc.configNamespace) + } + b.WriteString(".svc.") b.WriteString(bc.nodeDomain) return b.String() diff --git a/core/manager/bootstrap_test.go b/core/manager/bootstrap_test.go index 46d013e..dad30fa 100644 --- a/core/manager/bootstrap_test.go +++ b/core/manager/bootstrap_test.go @@ -24,6 +24,52 @@ import ( "google.golang.org/protobuf/types/known/structpb" ) +func TestTryExpandFQDN(t *testing.T) { + testCases := []struct { + desc string + host string + want string + bsc *BootstrapConfig + }{ + { + desc: "success", + host: "servicea", + want: "servicea.default.svc.cluster.local", + bsc: &BootstrapConfig{ + nodeDomain: "cluster.local", + configNamespace: "default", + }, + }, + { + desc: "success", + host: "servicea.bookinfo", + want: "servicea.bookinfo.svc.cluster.local", + bsc: &BootstrapConfig{ + nodeDomain: "cluster.local", + configNamespace: "default", + }, + }, + { + desc: "fqdn", + host: "servicea.bookinfo.svc.cluster.local", + want: "servicea.bookinfo.svc.cluster.local", + bsc: &BootstrapConfig{ + nodeDomain: "cluster.local", + configNamespace: "default", + }, + }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + got := tc.bsc.tryExpandFQDN(tc.host) + if got != tc.want { + t.Errorf("tryExpandFQDN() got = %v, want %v", got, tc.want) + } + }) + } + +} + func TestParseMetaEnvs(t *testing.T) { testCases := []struct { desc string From b60566b0d8772f514abd3b07dd87a635d6f546dc Mon Sep 17 00:00:00 2001 From: whalecold Date: Wed, 3 Jan 2024 16:45:20 +0800 Subject: [PATCH 3/5] fix lint --- core/manager/bootstrap_test.go | 1 - core/manager/client_test.go | 17 ++++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/core/manager/bootstrap_test.go b/core/manager/bootstrap_test.go index dad30fa..82fc14c 100644 --- a/core/manager/bootstrap_test.go +++ b/core/manager/bootstrap_test.go @@ -67,7 +67,6 @@ func TestTryExpandFQDN(t *testing.T) { } }) } - } func TestParseMetaEnvs(t *testing.T) { diff --git a/core/manager/client_test.go b/core/manager/client_test.go index ef2a910..63c2706 100644 --- a/core/manager/client_test.go +++ b/core/manager/client_test.go @@ -222,8 +222,9 @@ func TestClearCh(t *testing.T) { func TestResolveAddr(t *testing.T) { resolver := newNdsResolver() resolver.updateLookupTable(map[string][]string{ - "echoa.default.svc.cluster.local": {"1.1.1.1"}, - "echob.default.svc.cluster.local": {"1.1.1.1"}, + "echoa.default.svc.cluster.local": {"1.1.1.1"}, + "echoa.default1.svc.cluster.local": {"1.1.1.1"}, + "echob.default.svc.cluster.local": {"1.1.1.1"}, }) cli := &xdsClient{ cipResolver: resolver, @@ -248,8 +249,18 @@ func TestResolveAddr(t *testing.T) { want: "1.1.1.1", }, { - desc: "not found", + desc: "contain namespace", host: "echoa.default", + want: "1.1.1.1", + }, + { + desc: "contain namespace", + host: "echoa.default1", + want: "1.1.1.1", + }, + { + desc: "not found", + host: "echoa.default.svc", want: "", }, } From 0a0218ca2116243a847a6c1a9e2a19adea52c22b Mon Sep 17 00:00:00 2001 From: whalecold Date: Mon, 8 Jan 2024 10:20:57 +0800 Subject: [PATCH 4/5] tweak the policy of make up fqdn --- core/manager/bootstrap.go | 20 ++++++++++++++++---- core/manager/bootstrap_test.go | 9 +++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/core/manager/bootstrap.go b/core/manager/bootstrap.go index fabb5d7..e218a6f 100644 --- a/core/manager/bootstrap.go +++ b/core/manager/bootstrap.go @@ -106,15 +106,27 @@ func (bc *BootstrapConfig) tryExpandFQDN(host string) string { b.Grow(len(host) + len(bc.configNamespace) + len(bc.nodeDomain) + 10) b.WriteString(host) - // the regex for Kubernetes service is [a-z]([-a-z0-9]*[a-z0-9])?, it should not contains `.` in it + // the regex for Kubernetes service is [a-z]([-a-z0-9]*[a-z0-9])?, it should not contains `.`, so we can split the host + // by `.` // if the host not contains namespace. - if !strings.Contains(host, ".") { + parts := strings.Split(host, ".") + switch len(parts) { + case 1: b.WriteString(".") b.WriteString(bc.configNamespace) + fallthrough + case 2: + b.WriteString(".svc.") + b.WriteString(bc.nodeDomain) + case 3: + if parts[2] == "svc" { + b.WriteString(".") + b.WriteString(bc.nodeDomain) + } + default: + // ignore the other cases } - b.WriteString(".svc.") - b.WriteString(bc.nodeDomain) return b.String() } diff --git a/core/manager/bootstrap_test.go b/core/manager/bootstrap_test.go index 82fc14c..1aa75cc 100644 --- a/core/manager/bootstrap_test.go +++ b/core/manager/bootstrap_test.go @@ -58,6 +58,15 @@ func TestTryExpandFQDN(t *testing.T) { configNamespace: "default", }, }, + { + desc: "error", + host: "servicea.bookinfo.svc", + want: "servicea.bookinfo.svc.cluster.local", + bsc: &BootstrapConfig{ + nodeDomain: "cluster.local", + configNamespace: "default", + }, + }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { From ee9f0cb54c8cb528008a59a22c35e7fd28e308dc Mon Sep 17 00:00:00 2001 From: whalecold Date: Mon, 8 Jan 2024 10:33:05 +0800 Subject: [PATCH 5/5] fix uint test --- core/manager/client_test.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/core/manager/client_test.go b/core/manager/client_test.go index 63c2706..5bd43f7 100644 --- a/core/manager/client_test.go +++ b/core/manager/client_test.go @@ -259,8 +259,23 @@ func TestResolveAddr(t *testing.T) { want: "1.1.1.1", }, { - desc: "not found", + desc: "make up svc", host: "echoa.default.svc", + want: "1.1.1.1", + }, + { + desc: "not found", + host: "echoa.default.svc.cluster", + want: "", + }, + { + desc: "not found", + host: "echoa.default2", + want: "", + }, + { + desc: "not found", + host: "echoc.default", want: "", }, }