From 85338f0f71db7fef888db8dcd6f96f72d46ae22d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Tue, 8 Nov 2022 21:34:26 +0100 Subject: [PATCH 1/9] wip --- internal/dataplane/parser/parser.go | 11 +++--- test/integration/translation_failures_test.go | 35 +++++++++++++++++++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/internal/dataplane/parser/parser.go b/internal/dataplane/parser/parser.go index 1619844719..8ec7e6561e 100644 --- a/internal/dataplane/parser/parser.go +++ b/internal/dataplane/parser/parser.go @@ -121,7 +121,7 @@ func (p *Parser) Build() (*kongstate.KongState, []TranslationFailure) { // generate Certificates and SNIs ingressCerts := getCerts(p.logger, p.storer, ingressRules.SecretNameToSNIs) - gatewayCerts := getGatewayCerts(p.logger, p.storer) + gatewayCerts := p.getGatewayCerts() // note that ingress-derived certificates will take precedence over gateway-derived certificates for SNI assignment result.Certificates = mergeCerts(p.logger, ingressCerts, gatewayCerts) @@ -384,7 +384,9 @@ type certWrapper struct { CreationTimestamp metav1.Time } -func getGatewayCerts(log logrus.FieldLogger, s store.Storer) []certWrapper { +func (p *Parser) getGatewayCerts() []certWrapper { + log := p.logger + s := p.storer certs := []certWrapper{} gateways, err := s.ListGateways() if err != nil { @@ -429,10 +431,7 @@ func getGatewayCerts(log logrus.FieldLogger, s store.Storer) []certWrapper { if len(listener.TLS.CertificateRefs) > 1 { // TODO support cert_alt and key_alt if there are 2 SecretObjectReferences // https://github.com/Kong/kubernetes-ingress-controller/issues/2604 - log.WithFields(logrus.Fields{ - "gateway": gateway.Name, - "listener": listener.Name, - }).Error("Gateway Listeners with more than one certificateRef are not supported") + p.registerTranslationFailure("listener '%s' has more than one certificateRef, it's not supported", gateway) continue } diff --git a/test/integration/translation_failures_test.go b/test/integration/translation_failures_test.go index 15f9f9e58c..a66442bfa5 100644 --- a/test/integration/translation_failures_test.go +++ b/test/integration/translation_failures_test.go @@ -11,6 +11,7 @@ import ( "github.com/kong/go-kong/kong" "github.com/kong/kubernetes-testing-framework/pkg/clusters" + ktfkong "github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/kong" "github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -147,6 +148,40 @@ func TestTranslationFailures(t *testing.T) { return []client.Object{service} }, }, + { + name: "more than one certificate ref specified for a gateway listener", + translationFailureTrigger: func(t *testing.T, cleaner *clusters.Cleaner, ns string) []client.Object { + gatewayClient, err := gatewayclient.NewForConfig(env.Cluster().Config()) + require.NoError(t, err) + + gatewayClassName := testutils.RandomName(testTranslationFailuresObjectsPrefix) + gwc, err := DeployGatewayClass(ctx, gatewayClient, gatewayClassName) + require.NoError(t, err) + cleaner.Add(gwc) + + gatewayName := testutils.RandomName(testTranslationFailuresObjectsPrefix) + hostname := gatewayv1beta1.Hostname(tlsRouteHostname) + gateway, err := DeployGateway(ctx, gatewayClient, ns, gatewayClassName, func(gw *gatewayv1beta1.Gateway) { + gw.Name = gatewayName + gw.Spec.Listeners = []gatewayv1beta1.Listener{{ + Name: gatewayv1beta1.SectionName(testutils.RandomName(testTranslationFailuresObjectsPrefix)), + Protocol: gatewayv1beta1.TLSProtocolType, + Port: gatewayv1beta1.PortNumber(ktfkong.DefaultTLSServicePort), + Hostname: &hostname, + TLS: &gatewayv1beta1.GatewayTLSConfig{ + CertificateRefs: []gatewayv1beta1.SecretObjectReference{ + {Name: "tls-secret-name"}, + {Name: "another-tls-secret-name"}, + }, + }, + }} + }) + require.NoError(t, err) + cleaner.Add(gateway) + + return []client.Object{gateway} + }, + }, } for _, tt := range testCases { From 0f1328183830744e356d512d00941f3e9055f9ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Wed, 9 Nov 2022 11:05:41 +0100 Subject: [PATCH 2/9] add test --- test/integration/translation_failures_test.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/test/integration/translation_failures_test.go b/test/integration/translation_failures_test.go index a66442bfa5..f8c102ad95 100644 --- a/test/integration/translation_failures_test.go +++ b/test/integration/translation_failures_test.go @@ -151,6 +151,19 @@ func TestTranslationFailures(t *testing.T) { { name: "more than one certificate ref specified for a gateway listener", translationFailureTrigger: func(t *testing.T, cleaner *clusters.Cleaner, ns string) []client.Object { + secret1 := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ + Name: testutils.RandomName(testTranslationFailuresObjectsPrefix), + }} + secret2 := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ + Name: testutils.RandomName(testTranslationFailuresObjectsPrefix), + }} + secret1, err := env.Cluster().Client().CoreV1().Secrets(ns).Create(ctx, secret1, metav1.CreateOptions{}) + require.NoError(t, err) + cleaner.Add(secret1) + secret2, err = env.Cluster().Client().CoreV1().Secrets(ns).Create(ctx, secret2, metav1.CreateOptions{}) + require.NoError(t, err) + cleaner.Add(secret2) + gatewayClient, err := gatewayclient.NewForConfig(env.Cluster().Config()) require.NoError(t, err) @@ -170,8 +183,8 @@ func TestTranslationFailures(t *testing.T) { Hostname: &hostname, TLS: &gatewayv1beta1.GatewayTLSConfig{ CertificateRefs: []gatewayv1beta1.SecretObjectReference{ - {Name: "tls-secret-name"}, - {Name: "another-tls-secret-name"}, + {Name: gatewayv1beta1.ObjectName(secret1.Name)}, + {Name: gatewayv1beta1.ObjectName(secret2.Name)}, }, }, }} From 731457a36390b742f629360349de0af3eaacdcdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Wed, 9 Nov 2022 11:10:38 +0100 Subject: [PATCH 3/9] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7dc24d927..01313293c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -134,6 +134,8 @@ Adding a new version? You'll need three changes: - Gateway API: Implement port matching for routes as defined in [GEP-957](https://gateway-api.sigs.k8s.io/geps/gep-957/) [#3129](https://github.com/Kong/kubernetes-ingress-controller/pull/3129) +- Warning events are recorded when a Gateway Listener has more than one CertificateRef specified. + [#3147](https://github.com/Kong/kubernetes-ingress-controller/pull/3147) ### Fixed From 856172e0473d400ddd06858f082840b3d58dc34e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Wed, 9 Nov 2022 12:36:19 +0100 Subject: [PATCH 4/9] more validations --- internal/dataplane/parser/parser.go | 24 ++--- test/integration/translation_failures_test.go | 87 ++++++++++++++----- 2 files changed, 70 insertions(+), 41 deletions(-) diff --git a/internal/dataplane/parser/parser.go b/internal/dataplane/parser/parser.go index 8ec7e6561e..b24150bceb 100644 --- a/internal/dataplane/parser/parser.go +++ b/internal/dataplane/parser/parser.go @@ -439,11 +439,9 @@ func (p *Parser) getGatewayCerts() []certWrapper { // SecretObjectReference is a misnomer; it can reference any Group+Kind, but defaults to Secret if ref.Kind != nil { - if string(*ref.Kind) != "Secret" { - log.WithFields(logrus.Fields{ - "gateway": gateway.Name, - "listener": listener.Name, - }).Error("CertificateRefs to kinds other than Secret are not supported") + if kind := *ref.Kind; kind != "Secret" { + p.registerTranslationFailure(fmt.Sprintf("CertificateRefs to kinds other than Secret (%s) are not supported", kind), gateway) + continue } } @@ -460,13 +458,8 @@ func (p *Parser) getGatewayCerts() []certWrapper { }, grants) if !newRefChecker(ref).IsRefAllowedByGrant(allowed) { - log.WithFields(logrus.Fields{ - "gateway": gateway.Name, - "gateway_namespace": gateway.Namespace, - "listener": listener.Name, - "secret_name": string(ref.Name), - "secret_namespace": namespace, - }).WithError(err).Error("secret reference not allowed by ReferenceGrant") + secretName := namespace + "/" + string(ref.Name) + p.registerTranslationFailure(fmt.Sprintf("secret reference (%s) not allowed by ReferenceGrant", secretName), gateway) continue } } @@ -484,12 +477,7 @@ func (p *Parser) getGatewayCerts() []certWrapper { } cert, key, err := getCertFromSecret(secret) if err != nil { - log.WithFields(logrus.Fields{ - "gateway": gateway.Name, - "listener": listener.Name, - "secret_name": string(ref.Name), - "secret_namespace": namespace, - }).WithError(err).Error("failed to construct certificate from secret") + p.registerTranslationFailure("failed to construct certificate from secret", secret, gateway) continue } diff --git a/test/integration/translation_failures_test.go b/test/integration/translation_failures_test.go index f8c102ad95..1ea3c8e94b 100644 --- a/test/integration/translation_failures_test.go +++ b/test/integration/translation_failures_test.go @@ -164,33 +164,39 @@ func TestTranslationFailures(t *testing.T) { require.NoError(t, err) cleaner.Add(secret2) - gatewayClient, err := gatewayclient.NewForConfig(env.Cluster().Config()) - require.NoError(t, err) + gateway := deployGatewayReferringSecrets(t, cleaner, ns, secret1, secret2) - gatewayClassName := testutils.RandomName(testTranslationFailuresObjectsPrefix) - gwc, err := DeployGatewayClass(ctx, gatewayClient, gatewayClassName) + return []client.Object{gateway} + }, + }, + { + name: "invalid secret referred by a gateway listener", + translationFailureTrigger: func(t *testing.T, cleaner *clusters.Cleaner, ns string) []client.Object { + emptySecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ + Name: testutils.RandomName(testTranslationFailuresObjectsPrefix), + }} + emptySecret, err := env.Cluster().Client().CoreV1().Secrets(ns).Create(ctx, emptySecret, metav1.CreateOptions{}) require.NoError(t, err) - cleaner.Add(gwc) + cleaner.Add(emptySecret) - gatewayName := testutils.RandomName(testTranslationFailuresObjectsPrefix) - hostname := gatewayv1beta1.Hostname(tlsRouteHostname) - gateway, err := DeployGateway(ctx, gatewayClient, ns, gatewayClassName, func(gw *gatewayv1beta1.Gateway) { - gw.Name = gatewayName - gw.Spec.Listeners = []gatewayv1beta1.Listener{{ - Name: gatewayv1beta1.SectionName(testutils.RandomName(testTranslationFailuresObjectsPrefix)), - Protocol: gatewayv1beta1.TLSProtocolType, - Port: gatewayv1beta1.PortNumber(ktfkong.DefaultTLSServicePort), - Hostname: &hostname, - TLS: &gatewayv1beta1.GatewayTLSConfig{ - CertificateRefs: []gatewayv1beta1.SecretObjectReference{ - {Name: gatewayv1beta1.ObjectName(secret1.Name)}, - {Name: gatewayv1beta1.ObjectName(secret2.Name)}, - }, - }, - }} - }) + gateway := deployGatewayReferringSecrets(t, cleaner, ns, emptySecret) + + return []client.Object{gateway, emptySecret} + }, + }, + { + name: "secret reference not allowed by ReferenceGrant", + translationFailureTrigger: func(t *testing.T, cleaner *clusters.Cleaner, ns string) []client.Object { + otherNs := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: testutils.RandomName(testTranslationFailuresObjectsPrefix)}} + otherNs, err := env.Cluster().Client().CoreV1().Namespaces().Create(ctx, otherNs, metav1.CreateOptions{}) + secretInDifferentNamespace := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ + Name: testutils.RandomName(testTranslationFailuresObjectsPrefix), + }} + secretInDifferentNamespace, err = env.Cluster().Client().CoreV1().Secrets(otherNs.Name).Create(ctx, secretInDifferentNamespace, metav1.CreateOptions{}) require.NoError(t, err) - cleaner.Add(gateway) + cleaner.Add(secretInDifferentNamespace) + + gateway := deployGatewayReferringSecrets(t, cleaner, ns, secretInDifferentNamespace) return []client.Object{gateway} }, @@ -381,3 +387,38 @@ func ingressWithPathBackedByService(service *corev1.Service) *netv1.Ingress { }, } } + +func deployGatewayReferringSecrets(t *testing.T, cleaner *clusters.Cleaner, ns string, secrets ...*corev1.Secret) *gatewayv1beta1.Gateway { + gatewayClient, err := gatewayclient.NewForConfig(env.Cluster().Config()) + require.NoError(t, err) + + gatewayClassName := testutils.RandomName(testTranslationFailuresObjectsPrefix) + gwc, err := DeployGatewayClass(ctx, gatewayClient, gatewayClassName) + require.NoError(t, err) + cleaner.Add(gwc) + + certificateRefs := make([]gatewayv1beta1.SecretObjectReference, 0, len(secrets)) + for _, s := range secrets { + sn := gatewayv1beta1.Namespace(s.GetNamespace()) + certificateRefs = append(certificateRefs, gatewayv1beta1.SecretObjectReference{ + Name: gatewayv1beta1.ObjectName(s.GetName()), + Namespace: &sn, + }) + } + + gatewayName := testutils.RandomName(testTranslationFailuresObjectsPrefix) + hostname := gatewayv1beta1.Hostname(tlsRouteHostname) + gateway, err := DeployGateway(ctx, gatewayClient, ns, gatewayClassName, func(gw *gatewayv1beta1.Gateway) { + gw.Name = gatewayName + gw.Spec.Listeners = []gatewayv1beta1.Listener{{ + Name: gatewayv1beta1.SectionName(testutils.RandomName(testTranslationFailuresObjectsPrefix)), + Protocol: gatewayv1beta1.TLSProtocolType, + Port: gatewayv1beta1.PortNumber(ktfkong.DefaultTLSServicePort), + Hostname: &hostname, + TLS: &gatewayv1beta1.GatewayTLSConfig{CertificateRefs: certificateRefs}, + }} + }) + require.NoError(t, err) + cleaner.Add(gateway) + return gateway +} From 3dad388581edfd3d984eca387c76c0b6635b3875 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Wed, 9 Nov 2022 16:57:18 +0100 Subject: [PATCH 5/9] changelog update --- CHANGELOG.md | 3 ++- test/integration/translation_failures_test.go | 17 ----------------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01313293c7..ca19674f83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -134,7 +134,8 @@ Adding a new version? You'll need three changes: - Gateway API: Implement port matching for routes as defined in [GEP-957](https://gateway-api.sigs.k8s.io/geps/gep-957/) [#3129](https://github.com/Kong/kubernetes-ingress-controller/pull/3129) -- Warning events are recorded when a Gateway Listener has more than one CertificateRef specified. +- Warning events are recorded when a Gateway Listener has more than one CertificateRef specified + or refers to a Secret that has no valid TLS key-pair. [#3147](https://github.com/Kong/kubernetes-ingress-controller/pull/3147) ### Fixed diff --git a/test/integration/translation_failures_test.go b/test/integration/translation_failures_test.go index 1ea3c8e94b..5ebd35f9a4 100644 --- a/test/integration/translation_failures_test.go +++ b/test/integration/translation_failures_test.go @@ -184,23 +184,6 @@ func TestTranslationFailures(t *testing.T) { return []client.Object{gateway, emptySecret} }, }, - { - name: "secret reference not allowed by ReferenceGrant", - translationFailureTrigger: func(t *testing.T, cleaner *clusters.Cleaner, ns string) []client.Object { - otherNs := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: testutils.RandomName(testTranslationFailuresObjectsPrefix)}} - otherNs, err := env.Cluster().Client().CoreV1().Namespaces().Create(ctx, otherNs, metav1.CreateOptions{}) - secretInDifferentNamespace := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{ - Name: testutils.RandomName(testTranslationFailuresObjectsPrefix), - }} - secretInDifferentNamespace, err = env.Cluster().Client().CoreV1().Secrets(otherNs.Name).Create(ctx, secretInDifferentNamespace, metav1.CreateOptions{}) - require.NoError(t, err) - cleaner.Add(secretInDifferentNamespace) - - gateway := deployGatewayReferringSecrets(t, cleaner, ns, secretInDifferentNamespace) - - return []client.Object{gateway} - }, - }, } for _, tt := range testCases { From c47a5f01b73c7923cda71746920cf92fd1f1feef Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Tue, 8 Nov 2022 18:53:25 -0800 Subject: [PATCH 6/9] chore(ci) use E2E tests as release gate (#3035) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidate E2E environment builders into helper. Add GKE support for E2E tests. Add GKE E2E workflow and make target. Refactor image loads to always succeed during builder construction. Change final release test from integration to E2E. Co-authored-by: Grzegorz Burzyński --- .github/workflows/e2e.yaml | 58 +++++++++++++++++++++++ .github/workflows/release.yaml | 12 ++--- Makefile | 14 ++++++ hack/e2e/cluster/deploy/main.go | 30 ++++++------ test/e2e/all_in_one_test.go | 56 +++++------------------ test/e2e/environment.go | 19 ++++++++ test/e2e/features_test.go | 71 ++++++++--------------------- test/e2e/helpers_test.go | 81 ++++++++++++++++++++++++++++----- test/e2e/kuma_test.go | 25 +++------- test/e2e/utils_test.go | 14 ++---- 10 files changed, 218 insertions(+), 162 deletions(-) create mode 100644 test/e2e/environment.go diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 12d551e62d..4e833aa09c 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -149,10 +149,68 @@ jobs: name: tests-report path: istio-${{ matrix.kubernetes-version }}-${{ matrix.istio-version }}-tests.xml + e2e-gke-tests: + environment: "gcloud" + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + kubernetes-version: # the GKE setup script ignores the patch version here and uses the latest, but we still include it for consistency with the other E2E jobs + - 'v1.24.2' + steps: + - name: setup golang + uses: actions/setup-go@v3 + with: + go-version: '^1.19' + + - name: cache go modules + uses: actions/cache@v3 + with: + path: ~/go/pkg/mod + key: ${{ runner.os }}-build-codegen-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-build-codegen- + + - name: checkout repository + uses: actions/checkout@v3 + with: + fetch-depth: 0 + + - uses: Kong/kong-license@master + id: license + with: + # PULP_PASSWORD secret is set in "Configure ci" environment + password: ${{ secrets.PULP_PASSWORD }} + + - name: run ${{ matrix.kubernetes-version }} + run: make test.e2e.gke + env: + GOOGLE_APPLICATION_CREDENTIALS: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }} + GOOGLE_PROJECT: ${{ secrets.GOOGLE_PROJECT }} + GOOGLE_LOCATION: ${{ secrets.GOOGLE_LOCATION }} + KONG_CLUSTER_VERSION: ${{ matrix.kubernetes-version }} + KONG_LICENSE_DATA: ${{ steps.license.outputs.license }} + GOTESTSUM_JUNITFILE: "gke-${{ matrix.kubernetes-version }}-tests.xml" + + - name: upload diagnostics + if: ${{ always() }} + uses: actions/upload-artifact@v3 + with: + name: diagnostics-e2e-tests + path: /tmp/ktf-diag* + if-no-files-found: ignore + + - name: collect test report + uses: actions/upload-artifact@v3 + with: + name: tests-report + path: gke-${{ matrix.kubernetes-version }}-tests.xml + buildpulse-report: environment: "Configure ci" needs: - "e2e-tests" + - "e2e-gke-tests" - "istio-tests" if: ${{ always() }} runs-on: ubuntu-latest diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 20a3d705aa..42db52834c 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -206,9 +206,6 @@ jobs: matrix: kubernetes-version: - 'v1.24.3' - dbmode: - - 'dbless' - - 'postgres' steps: - name: setup golang uses: actions/setup-go@v3 @@ -225,7 +222,7 @@ jobs: uses: actions/checkout@v3 with: fetch-depth: 0 - - name: Kubernetes ${{ matrix.kubernetes-version }} ${{ matrix.dbmode }} Integration Tests + - name: Kubernetes ${{ matrix.kubernetes-version }} E2E Tests run: KONG_CLUSTER_VERSION=${{ matrix.kubernetes-version }} make test.integration.${{ matrix.dbmode }} test-previous-kubernetes: @@ -238,9 +235,6 @@ jobs: - '21' - '22' - '23' - dbmode: - - 'dbless' - - 'postgres' steps: - name: setup golang uses: actions/setup-go@v3 @@ -257,8 +251,8 @@ jobs: uses: actions/checkout@v3 with: fetch-depth: 0 - - name: test ${{ matrix.dbmode }} on GKE v1.${{ matrix.minor }} - run: make test.integration.gke + - name: E2E on GKE v1.${{ matrix.minor }} + run: make test.e2e.gke env: KUBERNETES_MAJOR_VERSION: 1 KUBERNETES_MINOR_VERSION: ${{ matrix.minor }} diff --git a/Makefile b/Makefile index 3363728f41..a6c98e89da 100644 --- a/Makefile +++ b/Makefile @@ -338,6 +338,20 @@ test.integration.kind: @$(MAKE) _test.integration.cp \ CP="kind" +.PHONY: test.e2e.gke +test.e2e.gke: + CLUSTER_NAME="e2e-$(uuidgen)" \ + KUBERNETES_CLUSTER_NAME="${CLUSTER_NAME}" go run hack/e2e/cluster/deploy/main.go \ + KONG_TEST_CLUSTER="gke:${CLUSTER_NAME}" \ + GOFLAGS="-tags=e2e_tests" $(GOTESTSUM) -- $(GOTESTFLAGS) \ + -race \ + -run $(E2E_TEST_RUN) \ + -parallel $(NCPU) \ + -timeout $(E2E_TEST_TIMEOUT) \ + ./test/e2e/... \ + go run hack/e2e/cluster/cleanup/main.go ${CLUSTER_NAME} \ + trap cleanup EXIT SIGINT SIGQUIT + .PHONY: test.e2e test.e2e: gotestsum GOFLAGS="-tags=e2e_tests" \ diff --git a/hack/e2e/cluster/deploy/main.go b/hack/e2e/cluster/deploy/main.go index c325869d82..131fdf2f52 100644 --- a/hack/e2e/cluster/deploy/main.go +++ b/hack/e2e/cluster/deploy/main.go @@ -8,16 +8,17 @@ import ( "context" "fmt" "os" - "strconv" + "strings" + "github.com/blang/semver/v4" + "github.com/google/uuid" "github.com/kong/kubernetes-testing-framework/pkg/clusters" "github.com/kong/kubernetes-testing-framework/pkg/clusters/types/gke" ) const ( - k8sNameVar = "KUBERNETES_CLUSTER_NAME" - k8sMajorVar = "KUBERNETES_MAJOR_VERSION" - k8sMinorVar = "KUBERNETES_MINOR_VERSION" + k8sNameVar = "KUBERNETES_CLUSTER_NAME" + k8sVersionVar = "KONG_CLUSTER_VERSION" ) var ( @@ -28,23 +29,22 @@ var ( gkeProject = os.Getenv(gke.GKEProjectVar) gkeLocation = os.Getenv(gke.GKELocationVar) k8sName = os.Getenv(k8sNameVar) - k8sMajor = os.Getenv(k8sMajorVar) - k8sMinor = os.Getenv(k8sMinorVar) + k8sVersion = semver.MustParse(strings.TrimPrefix(os.Getenv(k8sVersionVar), "v")) ) func main() { mustNotBeEmpty(gke.GKECredsVar, gkeCreds) mustNotBeEmpty(gke.GKEProjectVar, gkeProject) mustNotBeEmpty(gke.GKELocationVar, gkeLocation) - mustNotBeEmpty(k8sNameVar, k8sName) - mustNotBeEmpty(k8sMajorVar, k8sMajor) - mustNotBeEmpty(k8sMinorVar, k8sMinor) + mustNotBeEmpty(k8sVersionVar, k8sVersion.String()) + if k8sName == "" { + k8sName = "kic-" + uuid.NewString() + fmt.Println("INFO: no cluster name provided, using generated name " + k8sName) + } fmt.Println("INFO: validating cluster version requirements") - major, err := strconv.Atoi(k8sMajor) - mustNotError(err) - minor, err := strconv.Atoi(k8sMinor) - mustNotError(err) + major := k8sVersion.Major + minor := k8sVersion.Minor if len(os.Args) > 1 && os.Args[1] == "cleanup" { fmt.Printf("INFO: cleanup called, deleting GKE cluster %s\n", k8sName) @@ -57,10 +57,10 @@ func main() { fmt.Printf("INFO: configuring the GKE cluster NAME=(%s) VERSION=(v%d.%d) PROJECT=(%s) LOCATION=(%s)\n", k8sName, major, minor, gkeProject, gkeLocation) builder := gke.NewBuilder([]byte(gkeCreds), gkeProject, gkeLocation).WithName(k8sName) - builder.WithClusterMinorVersion(uint64(major), uint64(minor)) + builder.WithClusterMinorVersion(major, minor) fmt.Printf("INFO: building cluster %s (this can take some time)\n", builder.Name) - cluster, err = builder.Build(ctx) + cluster, err := builder.Build(ctx) mustNotError(err) fmt.Println("INFO: verifying that the cluster can be communicated with") diff --git a/test/e2e/all_in_one_test.go b/test/e2e/all_in_one_test.go index 90ee2bdec9..d15ca30830 100644 --- a/test/e2e/all_in_one_test.go +++ b/test/e2e/all_in_one_test.go @@ -13,8 +13,6 @@ import ( "github.com/kong/kubernetes-testing-framework/pkg/clusters" "github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/kong" - "github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb" - "github.com/kong/kubernetes-testing-framework/pkg/environments" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" autoscalingv1 "k8s.io/api/autoscaling/v1" @@ -46,13 +44,8 @@ func TestDeployAllInOneDBLESS(t *testing.T) { defer cancel() t.Log("building test cluster and environment") - addons := []clusters.Addon{} - addons = append(addons, metallb.New()) - - addons = append(addons, buildImageLoadAddons(t, imageLoad, kongImageLoad)...) - - builder := setBuilderKubernetesVersion(t, - environments.NewBuilder().WithAddons(addons...), clusterVersionStr) + builder, err := getEnvironmentBuilder(ctx) + require.NoError(t, err) env, err := builder.Build(ctx) require.NoError(t, err) defer func() { @@ -112,13 +105,8 @@ func TestDeployAndUpgradeAllInOneDBLESS(t *testing.T) { defer cancel() t.Log("building test cluster and environment") - addons := []clusters.Addon{} - addons = append(addons, metallb.New()) - - addons = append(addons, buildImageLoadAddons(t, imageLoad, kongImageLoad)...) - - builder := setBuilderKubernetesVersion(t, - environments.NewBuilder().WithAddons(addons...), clusterVersionStr) + builder, err := getEnvironmentBuilder(ctx) + require.NoError(t, err) env, err := builder.Build(ctx) require.NoError(t, err) @@ -164,13 +152,8 @@ func TestDeployAllInOneEnterpriseDBLESS(t *testing.T) { defer cancel() t.Log("building test cluster and environment") - addons := []clusters.Addon{} - addons = append(addons, metallb.New()) - - addons = append(addons, buildImageLoadAddons(t, imageLoad, kongImageLoad)...) - - builder := setBuilderKubernetesVersion(t, - environments.NewBuilder().WithAddons(addons...), clusterVersionStr) + builder, err := getEnvironmentBuilder(ctx) + require.NoError(t, err) env, err := builder.Build(ctx) require.NoError(t, err) @@ -220,13 +203,8 @@ func TestDeployAllInOnePostgres(t *testing.T) { defer cancel() t.Log("building test cluster and environment") - addons := []clusters.Addon{} - addons = append(addons, metallb.New()) - - addons = append(addons, buildImageLoadAddons(t, imageLoad, kongImageLoad)...) - - builder := setBuilderKubernetesVersion(t, - environments.NewBuilder().WithAddons(addons...), clusterVersionStr) + builder, err := getEnvironmentBuilder(ctx) + require.NoError(t, err) env, err := builder.Build(ctx) require.NoError(t, err) @@ -261,13 +239,8 @@ func TestDeployAllInOnePostgresWithMultipleReplicas(t *testing.T) { defer cancel() t.Log("building test cluster and environment") - addons := []clusters.Addon{} - addons = append(addons, metallb.New()) - - addons = append(addons, buildImageLoadAddons(t, imageLoad, kongImageLoad)...) - - builder := setBuilderKubernetesVersion(t, - environments.NewBuilder().WithAddons(addons...), clusterVersionStr) + builder, err := getEnvironmentBuilder(ctx) + require.NoError(t, err) env, err := builder.Build(ctx) require.NoError(t, err) @@ -417,13 +390,8 @@ func TestDeployAllInOneEnterprisePostgres(t *testing.T) { defer cancel() t.Log("building test cluster and environment") - addons := []clusters.Addon{} - addons = append(addons, metallb.New()) - - addons = append(addons, buildImageLoadAddons(t, imageLoad, kongImageLoad)...) - - builder := setBuilderKubernetesVersion(t, - environments.NewBuilder().WithAddons(addons...), clusterVersionStr) + builder, err := getEnvironmentBuilder(ctx) + require.NoError(t, err) env, err := builder.Build(ctx) require.NoError(t, err) createKongImagePullSecret(ctx, t, env) diff --git a/test/e2e/environment.go b/test/e2e/environment.go new file mode 100644 index 0000000000..e750063712 --- /dev/null +++ b/test/e2e/environment.go @@ -0,0 +1,19 @@ +package e2e + +import "os" + +var ( + // clusterVersionStr indicates the Kubernetes cluster version to use when + // generating a testing environment and allows the caller to provide a specific + // version. If no version is provided the default version for the cluster + // provisioner in the testing framework will be used. + clusterVersionStr = os.Getenv("KONG_CLUSTER_VERSION") + + imageOverride = os.Getenv("TEST_KONG_CONTROLLER_IMAGE_OVERRIDE") + imageLoad = os.Getenv("TEST_KONG_CONTROLLER_IMAGE_LOAD") + kongImageOverride = os.Getenv("TEST_KONG_IMAGE_OVERRIDE") + kongImageLoad = os.Getenv("TEST_KONG_IMAGE_LOAD") + kongImagePullUsername = os.Getenv("TEST_KONG_PULL_USERNAME") + kongImagePullPassword = os.Getenv("TEST_KONG_PULL_PASSWORD") + existingCluster = os.Getenv("KONG_TEST_CLUSTER") +) diff --git a/test/e2e/features_test.go b/test/e2e/features_test.go index 9cef3f70a5..7267ce0476 100644 --- a/test/e2e/features_test.go +++ b/test/e2e/features_test.go @@ -135,6 +135,18 @@ func TestWebhookUpdate(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() + // on KIND, this test requires webhookKINDConfig. the generic getEnvironmentBuilder we use for most tests doesn't + // support this: the configuration is specific to KIND but should not be used by default, and the scaffolding isn't + // flexible enough to support tests building their own clusters or passing additional builder functions. this still + // uses the setup style from before getEnvironmentBuilder/GKE support as such, and just skips if it's attempting + // to run on GKE + if existingCluster != "" { + clusterType := strings.Split(existingCluster, ":")[0] + if clusterType != string(kind.KindClusterType) { + t.Skip("test not supported on non-KIND clusters") + } + } + t.Log("building test cluster and environment") configFile, err := os.CreateTemp(os.TempDir(), "webhook-kind-config-") require.NoError(t, err) @@ -302,14 +314,8 @@ func TestDeployAllInOneDBLESSGateway(t *testing.T) { defer cancel() t.Log("building test cluster and environment") - addons := []clusters.Addon{} - addons = append(addons, metallb.New()) - if b, err := loadimage.NewBuilder().WithImage(imageLoad); err == nil { - addons = append(addons, b.Build()) - } - - builder := setBuilderKubernetesVersion(t, - environments.NewBuilder().WithAddons(addons...), clusterVersionStr) + builder, err := getEnvironmentBuilder(ctx) + require.NoError(t, err) env, err := builder.Build(ctx) require.NoError(t, err) @@ -498,13 +504,8 @@ func TestDeployAllInOneDBLESSNoLoadBalancer(t *testing.T) { defer cancel() t.Log("building test cluster and environment") - addons := []clusters.Addon{} - if b, err := loadimage.NewBuilder().WithImage(imageLoad); err == nil { - addons = append(addons, b.Build()) - } - - builder := setBuilderKubernetesVersion(t, - environments.NewBuilder().WithAddons(addons...), clusterVersionStr) + builder, err := getEnvironmentBuilder(ctx) + require.NoError(t, err) env, err := builder.Build(ctx) require.NoError(t, err) cleaner := clusters.NewCleaner(env.Cluster()) @@ -568,29 +569,8 @@ func TestDefaultIngressClass(t *testing.T) { defer cancel() t.Log("building test cluster and environment") - configFile, err := os.CreateTemp(os.TempDir(), "test-default-ingress-class") - require.NoError(t, err) - defer os.Remove(configFile.Name()) - defer configFile.Close() - written, err := configFile.Write([]byte(webhookKINDConfig)) + builder, err := getEnvironmentBuilder(ctx) require.NoError(t, err) - require.Equal(t, len(webhookKINDConfig), written) - - clusterBuilder := kind.NewBuilder() - clusterBuilder.WithConfig(configFile.Name()) - if clusterVersionStr != "" { - clusterVersion, err := semver.ParseTolerant(clusterVersionStr) - require.NoError(t, err) - clusterBuilder.WithClusterVersion(clusterVersion) - } - cluster, err := clusterBuilder.Build(ctx) - require.NoError(t, err) - addons := []clusters.Addon{} - addons = append(addons, metallb.New()) - if b, err := loadimage.NewBuilder().WithImage(imageLoad); err == nil { - addons = append(addons, b.Build()) - } - builder := environments.NewBuilder().WithExistingCluster(cluster).WithAddons(addons...) env, err := builder.Build(ctx) require.NoError(t, err) @@ -712,21 +692,8 @@ func TestMissingCRDsDontCrashTheController(t *testing.T) { defer cancel() t.Log("building test cluster and environment") - clusterBuilder := kind.NewBuilder() - if clusterVersionStr != "" { - clusterVersion, err := semver.ParseTolerant(clusterVersionStr) - require.NoError(t, err) - clusterBuilder.WithClusterVersion(clusterVersion) - } - cluster, err := clusterBuilder.Build(ctx) + builder, err := getEnvironmentBuilder(ctx) require.NoError(t, err) - addons := []clusters.Addon{metallb.New()} - - if b, err := loadimage.NewBuilder().WithImage(imageLoad); err == nil { - addons = append(addons, b.Build()) - } - - builder := environments.NewBuilder().WithExistingCluster(cluster).WithAddons(addons...) env, err := builder.Build(ctx) require.NoError(t, err) @@ -739,7 +706,7 @@ func TestMissingCRDsDontCrashTheController(t *testing.T) { t.Logf("%s failed, dumped diagnostics to %s", t.Name(), output) } } - assert.NoError(t, cluster.Cleanup(ctx)) + assert.NoError(t, cleaner.Cleanup(ctx)) }() t.Log("deploying kong components") diff --git a/test/e2e/helpers_test.go b/test/e2e/helpers_test.go index 46b56bafde..1a4a76ef23 100644 --- a/test/e2e/helpers_test.go +++ b/test/e2e/helpers_test.go @@ -20,6 +20,9 @@ import ( "github.com/blang/semver/v4" "github.com/kong/kubernetes-testing-framework/pkg/clusters" "github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/loadimage" + "github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb" + "github.com/kong/kubernetes-testing-framework/pkg/clusters/types/gke" + "github.com/kong/kubernetes-testing-framework/pkg/clusters/types/kind" "github.com/kong/kubernetes-testing-framework/pkg/environments" "github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators" "github.com/stretchr/testify/assert" @@ -62,14 +65,68 @@ const ( tcpListnerPort = 8888 ) -var ( - imageOverride = os.Getenv("TEST_KONG_CONTROLLER_IMAGE_OVERRIDE") - imageLoad = os.Getenv("TEST_KONG_CONTROLLER_IMAGE_LOAD") - kongImageOverride = os.Getenv("TEST_KONG_IMAGE_OVERRIDE") - kongImageLoad = os.Getenv("TEST_KONG_IMAGE_LOAD") - kongImagePullUsername = os.Getenv("TEST_KONG_PULL_USERNAME") - kongImagePullPassword = os.Getenv("TEST_KONG_PULL_PASSWORD") -) +func getEnvironmentBuilder(ctx context.Context) (*environments.Builder, error) { + if existingCluster != "" { + if clusterVersionStr != "" { + return nil, fmt.Errorf("cannot provide cluster version with existing cluster") + } + clusterParts := strings.Split(existingCluster, ":") + if len(clusterParts) != 2 { + return nil, fmt.Errorf("existing cluster in wrong format (%s): format is : (e.g. kind:test-cluster)", existingCluster) + } + clusterType, clusterName := clusterParts[0], clusterParts[1] + + fmt.Printf("INFO: using existing %s cluster %s\n", clusterType, clusterName) + switch clusterType { + case string(kind.KindClusterType): + return createExistingKINDBuilder(clusterName) + case string(gke.GKEClusterType): + return createExistingGKEBuilder(ctx, clusterName) + default: + return nil, fmt.Errorf("unrecognized cluster type %s", clusterType) + } + } + return createDefaultKINDBuilder(), nil +} + +func createDefaultKINDBuilder() *environments.Builder { + builder := environments.NewBuilder() + clusterBuilder := kind.NewBuilder() + if clusterVersionStr != "" { + clusterVersion := semver.MustParse(strings.TrimPrefix(clusterVersionStr, "v")) + clusterBuilder = clusterBuilder.WithClusterVersion(clusterVersion) + } + builder = builder.WithClusterBuilder(clusterBuilder) + builder = builder.WithAddons(metallb.New()) + builder = builder.WithAddons(buildImageLoadAddons(imageLoad, kongImageLoad)...) + return builder +} + +func createExistingKINDBuilder(name string) (*environments.Builder, error) { + builder := environments.NewBuilder() + cluster, err := kind.NewFromExisting(name) + if err != nil { + return nil, err + } + builder = builder.WithExistingCluster(cluster) + builder = builder.WithAddons(metallb.New()) + builder = builder.WithAddons(buildImageLoadAddons(imageLoad, kongImageLoad)...) + return builder, nil +} + +// existing GKE patterns rely on running hack/e2e/cluster/deploy/main.go (integration too) and then just loading +// an existing cluster. this could probably avoid the hack script, and may be reasonable to integrate into KTF. +// there is no default GKE builder as such: https://github.com/Kong/kubernetes-ingress-controller/issues/1613 + +func createExistingGKEBuilder(ctx context.Context, name string) (*environments.Builder, error) { + cluster, err := gke.NewFromExistingWithEnv(ctx, name) + if err != nil { + return nil, err + } + builder := environments.NewBuilder() + builder = builder.WithExistingCluster(cluster) + return builder, nil +} func deployKong(ctx context.Context, t *testing.T, env environments.Environment, manifest io.Reader, additionalSecrets ...*corev1.Secret) *appsv1.Deployment { t.Log("creating a tempfile for kubeconfig") @@ -333,13 +390,13 @@ func killKong(ctx context.Context, t *testing.T, env environments.Environment, p } // buildImageLoadAddons creates addons to load KIC and kong images. -func buildImageLoadAddons(t *testing.T, images ...string) []clusters.Addon { +func buildImageLoadAddons(images ...string) []clusters.Addon { addons := []clusters.Addon{} for _, image := range images { if image != "" { - t.Logf("load image %s", image) - b, err := loadimage.NewBuilder().WithImage(image) - require.NoError(t, err) + // https://github.com/Kong/kubernetes-testing-framework/issues/440 this error only occurs if image == "" + // it will eventually be removed from the WithImage return signature + b, _ := loadimage.NewBuilder().WithImage(image) addons = append(addons, b.Build()) } } diff --git a/test/e2e/kuma_test.go b/test/e2e/kuma_test.go index bca9560935..f209411cf8 100644 --- a/test/e2e/kuma_test.go +++ b/test/e2e/kuma_test.go @@ -10,8 +10,6 @@ import ( "github.com/kong/kubernetes-testing-framework/pkg/clusters" "github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/kuma" - "github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/metallb" - "github.com/kong/kubernetes-testing-framework/pkg/environments" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" autoscalingv1 "k8s.io/api/autoscaling/v1" @@ -26,15 +24,9 @@ func TestDeployAllInOneDBLESSKuma(t *testing.T) { defer cancel() t.Log("building test cluster and environment") - addons := []clusters.Addon{} - addons = append(addons, metallb.New()) - - addons = append(addons, buildImageLoadAddons(t, imageLoad, kongImageLoad)...) - - addons = append(addons, kuma.New()) - - builder := setBuilderKubernetesVersion(t, - environments.NewBuilder().WithAddons(addons...), clusterVersionStr) + builder, err := getEnvironmentBuilder(ctx) + require.NoError(t, err) + builder = builder.WithAddons(kuma.New()) env, err := builder.Build(ctx) require.NoError(t, err) @@ -113,14 +105,9 @@ func TestDeployAllInOnePostgresKuma(t *testing.T) { defer cancel() t.Log("building test cluster and environment") - addons := []clusters.Addon{} - addons = append(addons, metallb.New()) - addons = append(addons, kuma.New()) - - addons = append(addons, buildImageLoadAddons(t, imageLoad, kongImageLoad)...) - - builder := setBuilderKubernetesVersion(t, - environments.NewBuilder().WithAddons(addons...), clusterVersionStr) + builder, err := getEnvironmentBuilder(ctx) + require.NoError(t, err) + builder = builder.WithAddons(kuma.New()) env, err := builder.Build(ctx) require.NoError(t, err) defer func() { diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index 85e4b41328..269399f0c2 100644 --- a/test/e2e/utils_test.go +++ b/test/e2e/utils_test.go @@ -29,17 +29,9 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" ) -var ( - // clusterVersionStr indicates the Kubernetes cluster version to use when - // generating a testing environment and allows the caller to provide a specific - // version. If no version is provided the default version for the cluster - // provisioner in the testing framework will be used. - clusterVersionStr = os.Getenv("KONG_CLUSTER_VERSION") - - // httpc is a standard HTTP client for tests to use that has a low default - // timeout instead of the longer default provided by the http stdlib. - httpc = http.Client{Timeout: time.Second * 10} -) +// httpc is a standard HTTP client for tests to use that has a low default +// timeout instead of the longer default provided by the http stdlib. +var httpc = http.Client{Timeout: time.Second * 10} const ( // adminPasswordSecretName is the name of the secret which will house the admin From 43009bbc70d254ba9d44411c0559eea32e0b3829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Wed, 9 Nov 2022 19:04:42 +0100 Subject: [PATCH 7/9] lint --- test/integration/translation_failures_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/translation_failures_test.go b/test/integration/translation_failures_test.go index b2ad57a4c7..88b3d6b926 100644 --- a/test/integration/translation_failures_test.go +++ b/test/integration/translation_failures_test.go @@ -208,7 +208,7 @@ func TestTranslationFailures(t *testing.T) { return expectedTranslationFailure{ causingObjects: []client.Object{gateway}, - reasonContains: fmt.Sprintf("more than one certificateRef"), + reasonContains: "more than one certificateRef", } }, }, From e9bbb3a2a3850bda7ee17ee0d69abd58193bdd0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Wed, 9 Nov 2022 21:35:07 +0100 Subject: [PATCH 8/9] don't validate reference grant in the parser as it's already done in the controller --- internal/dataplane/parser/parser.go | 29 +---------------------------- 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/internal/dataplane/parser/parser.go b/internal/dataplane/parser/parser.go index a6164d4416..ac3419d8c5 100644 --- a/internal/dataplane/parser/parser.go +++ b/internal/dataplane/parser/parser.go @@ -395,11 +395,6 @@ func (p *Parser) getGatewayCerts() []certWrapper { log.WithError(err).Error("failed to list Gateways") return certs } - grants, err := s.ListReferenceGrants() - if err != nil { - log.WithError(err).Error("failed to list ReferenceGrants") - return certs - } for _, gateway := range gateways { statuses := make(map[gatewayv1beta1.SectionName]gatewayv1beta1.ListenerStatus, len(gateway.Status.Listeners)) for _, status := range gateway.Status.Listeners { @@ -437,34 +432,12 @@ func (p *Parser) getGatewayCerts() []certWrapper { continue } + // determine the Secret Namespace ref := listener.TLS.CertificateRefs[0] - - // SecretObjectReference is a misnomer; it can reference any Group+Kind, but defaults to Secret - if ref.Kind != nil { - if kind := *ref.Kind; kind != "Secret" { - p.registerTranslationFailure(fmt.Sprintf("CertificateRefs to kinds other than Secret (%s) are not supported", kind), gateway) - continue - } - } - - // determine the Secret Namespace and validate against ReferenceGrant if needed namespace := gateway.Namespace if ref.Namespace != nil { namespace = string(*ref.Namespace) } - if namespace != gateway.Namespace { - allowed := getPermittedForReferenceGrantFrom(gatewayv1alpha2.ReferenceGrantFrom{ - Group: gatewayv1alpha2.Group(gateway.GetObjectKind().GroupVersionKind().Group), - Kind: gatewayv1alpha2.Kind(gateway.GetObjectKind().GroupVersionKind().Kind), - Namespace: gatewayv1alpha2.Namespace(gateway.GetNamespace()), - }, grants) - - if !newRefChecker(ref).IsRefAllowedByGrant(allowed) { - secretName := namespace + "/" + string(ref.Name) - p.registerTranslationFailure(fmt.Sprintf("secret reference (%s) not allowed by ReferenceGrant", secretName), gateway) - continue - } - } // retrieve the Secret and extract the PEM strings secret, err := s.GetSecret(namespace, string(ref.Name)) From 8dc2ac6e173beceb23cb5ddd67e5019dbe8cd4f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Wed, 9 Nov 2022 21:53:56 +0100 Subject: [PATCH 9/9] add tests to the controller side --- .../gateway/gateway_controller_test.go | 17 ++- .../dataplane/parser/translate_utils_test.go | 114 ------------------ 2 files changed, 16 insertions(+), 115 deletions(-) diff --git a/internal/controllers/gateway/gateway_controller_test.go b/internal/controllers/gateway/gateway_controller_test.go index 9ec9321bfb..1441fb2e57 100644 --- a/internal/controllers/gateway/gateway_controller_test.go +++ b/internal/controllers/gateway/gateway_controller_test.go @@ -557,6 +557,11 @@ func TestGetReferenceGrantConditionReason(t *testing.T) { referenceGrants []gatewayv1alpha2.ReferenceGrant expectedReason string }{ + { + name: "empty reference", + certRef: gatewayv1beta1.SecretObjectReference{}, + expectedReason: string(gatewayv1alpha2.ListenerReasonResolvedRefs), + }, { name: "no need for reference", gatewayNamespace: "test", @@ -567,7 +572,7 @@ func TestGetReferenceGrantConditionReason(t *testing.T) { expectedReason: string(gatewayv1alpha2.ListenerReasonResolvedRefs), }, { - name: "reference not granted", + name: "reference not granted - secret name not matching", gatewayNamespace: "test", certRef: gatewayv1beta1.SecretObjectReference{ Kind: util.StringToGatewayAPIKindPtr("Secret"), @@ -599,6 +604,16 @@ func TestGetReferenceGrantConditionReason(t *testing.T) { }, expectedReason: string(gatewayv1alpha2.ListenerReasonRefNotPermitted), }, + { + name: "reference not granted - no grants specified", + gatewayNamespace: "test", + certRef: gatewayv1beta1.SecretObjectReference{ + Kind: util.StringToGatewayAPIKindPtr("Secret"), + Name: "testSecret", + Namespace: (*Namespace)(pointer.StringPtr("otherNamespace")), + }, + expectedReason: string(gatewayv1alpha2.ListenerReasonRefNotPermitted), + }, { name: "reference granted, secret name not specified", gatewayNamespace: "test", diff --git a/internal/dataplane/parser/translate_utils_test.go b/internal/dataplane/parser/translate_utils_test.go index b288977ca4..48b46b3efd 100644 --- a/internal/dataplane/parser/translate_utils_test.go +++ b/internal/dataplane/parser/translate_utils_test.go @@ -178,120 +178,6 @@ func TestConvertGatewayMatchHeadersToKongRouteMatchHeaders(t *testing.T) { } } -func TestIsRefAllowedByGrant(t *testing.T) { - fitrat := gatewayv1beta1.Namespace("fitrat") - cholpon := gatewayv1beta1.Namespace("cholpon") - behbudiy := gatewayv1beta1.Namespace("behbudiy") - - group := gatewayv1beta1.Group("fake.example.com") - kind := gatewayv1beta1.Kind("fakeKind") - badKind := gatewayv1beta1.Kind("badFakeKind") - cholponName := gatewayv1beta1.ObjectName("cholpon") - - refGrantGroup := gatewayv1alpha2.Group(group) - refGrantKind := gatewayv1alpha2.Kind(kind) - refGrantBadKind := gatewayv1alpha2.Kind(badKind) - refGrantName := gatewayv1alpha2.ObjectName(cholponName) - - fakeMap := map[gatewayv1beta1.Namespace][]gatewayv1alpha2.ReferenceGrantTo{ - fitrat: { - {Group: refGrantGroup, Kind: refGrantKind}, - {Group: gatewayv1alpha2.Group("extra.example"), Kind: refGrantBadKind}, - }, - cholpon: { - {Group: refGrantGroup, Kind: refGrantKind, Name: &refGrantName}, - }, - behbudiy: {}, - } - tests := []struct { - msg string - ref gatewayv1beta1.BackendRef - result bool - }{ - { - msg: "empty", - ref: gatewayv1beta1.BackendRef{}, - result: true, - }, - { - msg: "no namespace", - ref: gatewayv1beta1.BackendRef{ - BackendObjectReference: gatewayv1beta1.BackendObjectReference{ - Name: gatewayv1beta1.ObjectName("foo"), - }, - }, - result: true, - }, - { - msg: "valid namespace+group+kind", - ref: gatewayv1beta1.BackendRef{ - BackendObjectReference: gatewayv1beta1.BackendObjectReference{ - Name: gatewayv1beta1.ObjectName("foo"), - Group: &group, - Kind: &kind, - Namespace: &fitrat, - }, - }, - result: true, - }, - { - msg: "valid namespace+group+kind+name", - ref: gatewayv1beta1.BackendRef{ - BackendObjectReference: gatewayv1beta1.BackendObjectReference{ - Name: cholponName, - Group: &group, - Kind: &kind, - Namespace: &cholpon, - }, - }, - result: true, - }, - { - msg: "invalid namespace+group+kind", - ref: gatewayv1beta1.BackendRef{ - BackendObjectReference: gatewayv1beta1.BackendObjectReference{ - Name: gatewayv1beta1.ObjectName("foo"), - Group: &group, - Kind: &badKind, - Namespace: &fitrat, - }, - }, - result: false, - }, - { - msg: "invalid namespace+group+kind+name", - ref: gatewayv1beta1.BackendRef{ - BackendObjectReference: gatewayv1beta1.BackendObjectReference{ - Name: gatewayv1beta1.ObjectName("sadness"), - Group: &group, - Kind: &kind, - Namespace: &cholpon, - }, - }, - result: false, - }, - { - msg: "no grants in target namespace", - ref: gatewayv1beta1.BackendRef{ - BackendObjectReference: gatewayv1beta1.BackendObjectReference{ - Name: gatewayv1beta1.ObjectName("foo"), - Group: &group, - Kind: &kind, - Namespace: &behbudiy, - }, - }, - result: false, - }, - } - for _, tt := range tests { - t.Run(tt.msg, func(t *testing.T) { - result := newRefChecker(tt.ref).IsRefAllowedByGrant(fakeMap) - - assert.Equal(t, tt.result, result) - }) - } -} - func TestGetPermittedForReferenceGrantFrom(t *testing.T) { grants := []*gatewayv1alpha2.ReferenceGrant{ {