You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by zh...@apache.org on 2022/12/16 14:21:30 UTC

[apisix-ingress-controller] branch master updated: feat: support for specifying port in external services (#1500)

This is an automated email from the ASF dual-hosted git repository.

zhangjintao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-ingress-controller.git


The following commit(s) were added to refs/heads/master by this push:
     new f162f711 feat: support for specifying port in external services (#1500)
f162f711 is described below

commit f162f7119abd76b5a71c285fbfae68ed2faf88fb
Author: Gallardot <tt...@163.com>
AuthorDate: Fri Dec 16 22:21:24 2022 +0800

    feat: support for specifying port in external services (#1500)
    
    
    Signed-off-by: Gallardot <tt...@163.com>
    Co-authored-by: Jintao Zhang <zh...@apache.org>
---
 Makefile                                           |   2 +-
 pkg/kube/apisix/apis/config/v2/types.go            |   3 +
 .../apisix/apis/config/v2/zz_generated.deepcopy.go |   5 +
 .../apisix/translation/apisix_upstream.go          |  40 +++----
 .../apisix/translation/apisix_upstream_test.go     | 127 +++++++++++++++++++++
 pkg/providers/gateway/provider.go                  |   3 +-
 pkg/providers/utils/schema.go                      |  31 +++++
 pkg/providers/utils/schema_test.go                 |  40 +++++++
 pkg/providers/utils/scheme.go                      |  38 ++++++
 pkg/providers/utils/scheme_test.go                 |  38 ++++++
 samples/deploy/crd/v1/ApisixUpstream.yaml          |   2 +
 11 files changed, 305 insertions(+), 24 deletions(-)

diff --git a/Makefile b/Makefile
index b63344af..fb4a865a 100644
--- a/Makefile
+++ b/Makefile
@@ -239,7 +239,7 @@ update-license:
 ### update-mdlint:        Update markdown files lint rules.
 .PHONY: update-mdlint
 update-mdlint:
-	docker run -it --rm -v $(PWD):/work tmknom/markdownlint '**/*.md' -f --ignore node_modules --ignore vendor
+	docker run -it --rm -v $(PWD):/work tmknom/markdownlint '**/*.md' -f --ignore node_modules --ignore vendor --ignore CHANGELOG.md
 
 ### update-gofmt:         Format all go codes
 .PHONY: update-gofmt
diff --git a/pkg/kube/apisix/apis/config/v2/types.go b/pkg/kube/apisix/apis/config/v2/types.go
index d87a8a56..a4a43fbb 100644
--- a/pkg/kube/apisix/apis/config/v2/types.go
+++ b/pkg/kube/apisix/apis/config/v2/types.go
@@ -519,6 +519,9 @@ type ApisixUpstreamExternalNode struct {
 	Type ApisixUpstreamExternalType `json:"type,omitempty" yaml:"type"`
 	// +optional
 	Weight *int `json:"weight,omitempty" yaml:"weight"`
+	// Port defines the port of the external node
+	// +optional
+	Port *int `json:"port,omitempty" yaml:"port"`
 }
 
 // ApisixUpstreamSubset defines a single endpoints group of one Service.
diff --git a/pkg/kube/apisix/apis/config/v2/zz_generated.deepcopy.go b/pkg/kube/apisix/apis/config/v2/zz_generated.deepcopy.go
index b5892305..a5bab875 100644
--- a/pkg/kube/apisix/apis/config/v2/zz_generated.deepcopy.go
+++ b/pkg/kube/apisix/apis/config/v2/zz_generated.deepcopy.go
@@ -1300,6 +1300,11 @@ func (in *ApisixUpstreamExternalNode) DeepCopyInto(out *ApisixUpstreamExternalNo
 		*out = new(int)
 		**out = **in
 	}
+	if in.Port != nil {
+		in, out := &in.Port, &out.Port
+		*out = new(int)
+		**out = **in
+	}
 	return
 }
 
diff --git a/pkg/providers/apisix/translation/apisix_upstream.go b/pkg/providers/apisix/translation/apisix_upstream.go
index ee4eb89f..287df401 100644
--- a/pkg/providers/apisix/translation/apisix_upstream.go
+++ b/pkg/providers/apisix/translation/apisix_upstream.go
@@ -12,20 +12,19 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
+
 package translation
 
 import (
 	"fmt"
-	"strconv"
-	"strings"
 
-	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 
 	"github.com/apache/apisix-ingress-controller/pkg/id"
 	v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
 	"github.com/apache/apisix-ingress-controller/pkg/providers/translation"
+	"github.com/apache/apisix-ingress-controller/pkg/providers/utils"
 	"github.com/apache/apisix-ingress-controller/pkg/types"
 	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
 )
@@ -57,35 +56,31 @@ func (t *translator) translateService(namespace, svcName, subset, svcResolveGran
 	return ups, nil
 }
 
-// TODO: Support Port field
 func (t *translator) TranslateApisixUpstreamExternalNodes(au *v2.ApisixUpstream) ([]apisixv1.UpstreamNode, error) {
 	var nodes []apisixv1.UpstreamNode
 	for i, node := range au.Spec.ExternalNodes {
 		if node.Type == v2.ExternalTypeDomain {
-			arr := strings.Split(node.Name, ":")
 
 			weight := translation.DefaultWeight
 			if node.Weight != nil {
 				weight = *node.Weight
 			}
+
+			if !utils.MatchHostDef(node.Name) {
+				return nil, fmt.Errorf("ApisixUpstream %s/%s ExternalNodes[%v]'s name %s as Domain must match lowercase RFC 1123 subdomain.  "+
+					"a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character",
+					au.Namespace, au.Name, i, node.Name)
+			}
+
 			n := apisixv1.UpstreamNode{
-				Host:   arr[0],
+				Host:   node.Name,
 				Weight: weight,
 			}
 
-			if len(arr) == 1 {
-				if strings.HasPrefix(arr[0], "https://") {
-					n.Port = 443
-				} else {
-					n.Port = 80
-				}
-			} else if len(arr) == 2 {
-				port, err := strconv.Atoi(arr[1])
-				if err != nil {
-					return nil, errors.Wrap(err, fmt.Sprintf("failed to parse ApisixUpstream %s/%s port: at ExternalNodes[%v]: %s", au.Namespace, au.Name, i, node.Name))
-				}
-
-				n.Port = port
+			if node.Port != nil {
+				n.Port = *node.Port
+			} else {
+				n.Port = utils.SchemeToPort(au.Spec.Scheme)
 			}
 
 			nodes = append(nodes, n)
@@ -113,8 +108,11 @@ func (t *translator) TranslateApisixUpstreamExternalNodes(au *v2.ApisixUpstream)
 				Weight: weight,
 			}
 
-			// TODO: Support Port field. This is a temporary solution.
-			n.Port = 80
+			if node.Port != nil {
+				n.Port = *node.Port
+			} else {
+				n.Port = utils.SchemeToPort(au.Spec.Scheme)
+			}
 
 			nodes = append(nodes, n)
 		}
diff --git a/pkg/providers/apisix/translation/apisix_upstream_test.go b/pkg/providers/apisix/translation/apisix_upstream_test.go
new file mode 100644
index 00000000..183df5f1
--- /dev/null
+++ b/pkg/providers/apisix/translation/apisix_upstream_test.go
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package translation
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
+)
+
+func TestTranslateApisixUpstreamExternalNodesDomainType(t *testing.T) {
+	tr := &translator{}
+	defaultPort := 80
+	defaultWeight := 80
+	specifiedPort := 8080
+	testCases := map[*v2.ApisixUpstream][]apisixv1.UpstreamNode{
+		{
+			Spec: &v2.ApisixUpstreamSpec{
+				ExternalNodes: []v2.ApisixUpstreamExternalNode{{
+					Name:   "domain.foobar.com",
+					Type:   v2.ExternalTypeDomain,
+					Weight: &defaultWeight,
+					Port:   &defaultPort,
+				}},
+			},
+		}: {{
+			Host:   "domain.foobar.com",
+			Port:   defaultPort,
+			Weight: defaultWeight,
+		}},
+		{
+			Spec: &v2.ApisixUpstreamSpec{
+				ExternalNodes: []v2.ApisixUpstreamExternalNode{{
+					Name:   "domain.foobar.com",
+					Type:   v2.ExternalTypeDomain,
+					Weight: &defaultWeight,
+					Port:   &specifiedPort,
+				}},
+			},
+		}: {{
+			Host:   "domain.foobar.com",
+			Port:   specifiedPort,
+			Weight: defaultWeight,
+		}},
+		{
+			Spec: &v2.ApisixUpstreamSpec{
+				ExternalNodes: []v2.ApisixUpstreamExternalNode{{
+					Name:   "domain.foobar.com",
+					Type:   v2.ExternalTypeDomain,
+					Weight: &defaultWeight,
+				}},
+			},
+		}: {{
+			Host:   "domain.foobar.com",
+			Port:   defaultPort,
+			Weight: defaultWeight,
+		}},
+	}
+	for k, v := range testCases {
+		result, _ := tr.TranslateApisixUpstreamExternalNodes(k)
+		assert.Equal(t, v, result)
+	}
+}
+
+func TestTranslateApisixUpstreamExternalNodesDomainTypeError(t *testing.T) {
+	tr := &translator{}
+	testCases := []v2.ApisixUpstream{
+		{
+			Spec: &v2.ApisixUpstreamSpec{
+				ExternalNodes: []v2.ApisixUpstreamExternalNode{{
+					Name: "https://domain.foobar.com",
+					Type: v2.ExternalTypeDomain,
+				}},
+			}},
+		{
+			Spec: &v2.ApisixUpstreamSpec{
+				ExternalNodes: []v2.ApisixUpstreamExternalNode{{
+					Name: "grpc://domain.foobar.com",
+					Type: v2.ExternalTypeDomain,
+				}},
+			}},
+		{
+			Spec: &v2.ApisixUpstreamSpec{
+				ExternalNodes: []v2.ApisixUpstreamExternalNode{{
+					Name: "-domain.foobar.com",
+					Type: v2.ExternalTypeDomain,
+				}},
+			}},
+		{
+			Spec: &v2.ApisixUpstreamSpec{
+				ExternalNodes: []v2.ApisixUpstreamExternalNode{{
+					Name: "-123.foobar.com",
+					Type: v2.ExternalTypeDomain,
+				}},
+			}},
+		{
+			Spec: &v2.ApisixUpstreamSpec{
+				ExternalNodes: []v2.ApisixUpstreamExternalNode{{
+					Name: "-123.FOOBAR.com",
+					Type: v2.ExternalTypeDomain,
+				}},
+			}},
+	}
+
+	for _, k := range testCases {
+		_, err := tr.TranslateApisixUpstreamExternalNodes(&k)
+		assert.Error(t, err)
+	}
+}
diff --git a/pkg/providers/gateway/provider.go b/pkg/providers/gateway/provider.go
index aeed5cae..05663181 100644
--- a/pkg/providers/gateway/provider.go
+++ b/pkg/providers/gateway/provider.go
@@ -24,11 +24,10 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
+	gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
 	gatewayclientset "sigs.k8s.io/gateway-api/pkg/client/clientset/versioned"
 	gatewayexternalversions "sigs.k8s.io/gateway-api/pkg/client/informers/externalversions"
 	gatewaylistersv1alpha2 "sigs.k8s.io/gateway-api/pkg/client/listers/apis/v1alpha2"
-
-	gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
 	gatewaylistersv1beta1 "sigs.k8s.io/gateway-api/pkg/client/listers/apis/v1beta1"
 
 	"github.com/apache/apisix-ingress-controller/pkg/apisix"
diff --git a/pkg/providers/utils/schema.go b/pkg/providers/utils/schema.go
new file mode 100644
index 00000000..daeade28
--- /dev/null
+++ b/pkg/providers/utils/schema.go
@@ -0,0 +1,31 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package utils
+
+import "regexp"
+
+var hostDef = "^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$"
+var hostDefRegex = regexp.MustCompile(hostDef)
+
+// MatchHostDef checks that host matches host's shcema
+// ref to : https://github.com/apache/apisix/blob/c5fc10d9355a0c177a7532f01c77745ff0639a7f/apisix/schema_def.lua#L40
+// ref to : https://github.com/kubernetes/kubernetes/blob/976a940f4a4e84fe814583848f97b9aafcdb083f/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go#L205
+// They define regex differently, but k8s's dns is more accurate
+func MatchHostDef(host string) bool {
+	return hostDefRegex.MatchString(host)
+}
diff --git a/pkg/providers/utils/schema_test.go b/pkg/providers/utils/schema_test.go
new file mode 100644
index 00000000..ab4adf21
--- /dev/null
+++ b/pkg/providers/utils/schema_test.go
@@ -0,0 +1,40 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package utils
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMatchHostDef(t *testing.T) {
+	tcs := map[string]bool{
+		"163.com":            true,
+		"github.com":         true,
+		"GITHUB.com":         false,
+		"-github.COM":        false,
+		"https://github.com": false,
+		"http://github.com":  false,
+	}
+
+	for k, v := range tcs {
+		ret := MatchHostDef(k)
+		assert.Equal(t, ret, v)
+	}
+}
diff --git a/pkg/providers/utils/scheme.go b/pkg/providers/utils/scheme.go
new file mode 100644
index 00000000..123c1e9a
--- /dev/null
+++ b/pkg/providers/utils/scheme.go
@@ -0,0 +1,38 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package utils
+
+import (
+	apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
+)
+
+var schemeToPortMaps = map[string]int{
+	apisixv1.SchemeHTTP:  80,
+	apisixv1.SchemeHTTPS: 443,
+	apisixv1.SchemeGRPC:  80,
+	apisixv1.SchemeGRPCS: 443,
+}
+
+// SchemeToPort scheme converts to the default port
+// ref https://github.com/apache/apisix/blob/c5fc10d9355a0c177a7532f01c77745ff0639a7f/apisix/upstream.lua#L167-L172
+func SchemeToPort(schema string) int {
+	if val, ok := schemeToPortMaps[schema]; ok {
+		return val
+	}
+	return 80
+}
diff --git a/pkg/providers/utils/scheme_test.go b/pkg/providers/utils/scheme_test.go
new file mode 100644
index 00000000..2f8d5eeb
--- /dev/null
+++ b/pkg/providers/utils/scheme_test.go
@@ -0,0 +1,38 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package utils
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSchemeToPort(t *testing.T) {
+	testCases := map[string]int{
+		"http":    80,
+		"https":   443,
+		"grpc":    80,
+		"grpcs":   443,
+		"default": 80,
+	}
+	for k, v := range testCases {
+		val := SchemeToPort(k)
+		assert.Equal(t, val, v)
+	}
+}
diff --git a/samples/deploy/crd/v1/ApisixUpstream.yaml b/samples/deploy/crd/v1/ApisixUpstream.yaml
index 337cff2a..848c56ab 100644
--- a/samples/deploy/crd/v1/ApisixUpstream.yaml
+++ b/samples/deploy/crd/v1/ApisixUpstream.yaml
@@ -437,6 +437,8 @@ spec:
                         type: string
                       weight:
                         type: integer
+                      port:
+                        type: integer
                 subsets:
                   type: array
                   items: