chore: version 2.10.2 (#1580)

* Faster requeue when object has been modified error happens (#1496)

* Faster requeue when object has been modified error happens

* Add api-gateway metrics

* Register metrics

* Add unit test

* Fix lint

* chore: prepare 2.10.2

* QOL updates to APIRule v2alpha1 integration tests (#1507)

* Remove dupplicated inits from apirule tests

* Add template file name

* switch files

* Scale down oathkeeper for faster startup

* Revert delay

* Conditional teardown

* Re-add len parameter to RNG

* Adapt according to review comments

* Full support for special characters in asterisk paths (#1569)

* Full support for special characters in asterisk paths

* Fix regex pattern

* Only use scenario initializer for v2alpha1 tests (#1521)

* Apply suggestions from code review

Co-authored-by: Natalia Sitko <80401180+nataliasitko@users.noreply.github.com>

---------

Co-authored-by: Patryk Strugacz <werdes72@users.noreply.github.com>
Co-authored-by: Bartosz Chwila <103247439+barchw@users.noreply.github.com>
Co-authored-by: Natalia Sitko <80401180+nataliasitko@users.noreply.github.com>

VERSION

@@ -1 +1 @@
-2.10.1
+2.10.2

apis/gateway/v2alpha1/apirule_types.go

@@ -129,7 +129,7 @@ type Rule struct {
 	//  The `{**}` operator must be the last operator in the path.
 	//  - Wildcard path `/*` - matches all paths. Equivalent to `/{**}` path.
 	//
-	// +kubebuilder:validation:Pattern=`^((\/[\w\.~\-]*)|(\/\{\*{1,2}\}))+$|^\/\*$`
+	// +kubebuilder:validation:Pattern=`^((\/([A-Za-z0-9-._~!$&'()+,;=:@]|%[0-9a-fA-F]{2})*)|(\/\{\*{1,2}\}))+$|^\/\*$`
 	Path string `json:"path"`
 	// Describes the service to expose. Overwrites the **spec** level service if defined.
 	// +optional

controllers/gateway/apirule_controller.go

@@ -157,6 +157,11 @@ func (r *APIRuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	s := processing.Reconcile(ctx, r.Client, &l, cmd)
 	if err := s.UpdateStatus(&apiRule.Status); err != nil {
 		l.Error(err, "Error updating APIRule status")
+		// Quick retry if the object has been modified
+		if strings.Contains(err.Error(), "the object has been modified") {
+			r.Metrics.IncreaseApiRuleObjectModifiedErrorsCounter()
+			return doneReconcileErrorRequeue(err, 10*time.Second)
+		}
 		return doneReconcileErrorRequeue(err, r.OnErrorReconcilePeriod)
 	}
 	return r.updateStatus(ctx, l, &apiRule)

controllers/gateway/suite_test.go

@@ -3,6 +3,7 @@ package gateway_test
 import (
 	"context"
 	"fmt"
+	"github.com/kyma-project/api-gateway/internal/metrics"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -153,7 +154,9 @@ var _ = BeforeSuite(func(specCtx SpecContext) {
 		ErrorReconciliationPeriod: 2,
 	}
 
-	apiReconciler := gateway.NewApiRuleReconciler(mgr, reconcilerConfig)
+	apiGatewayMetrics := metrics.NewApiGatewayMetrics()
+
+	apiReconciler := gateway.NewApiRuleReconciler(mgr, reconcilerConfig, apiGatewayMetrics)
 	rateLimiterCfg := controllers.RateLimiterConfig{
 		Burst:            200,
 		Frequency:        30,

controllers/gateway/types.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/kyma-project/api-gateway/internal/helpers"
+	"github.com/kyma-project/api-gateway/internal/metrics"
 	"github.com/kyma-project/api-gateway/internal/processing"
 	"istio.io/api/networking/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -23,6 +24,7 @@ type APIRuleReconciler struct {
 	ReconcilePeriod          time.Duration
 	OnErrorReconcilePeriod   time.Duration
 	MigrationReconcilePeriod time.Duration
+	Metrics                  *metrics.ApiGatewayMetrics
 }
 
 type ApiRuleReconcilerConfiguration struct {
@@ -34,7 +36,7 @@ type ApiRuleReconcilerConfiguration struct {
 	MigrationReconciliationPeriod                        uint
 }
 
-func NewApiRuleReconciler(mgr manager.Manager, config ApiRuleReconcilerConfiguration) *APIRuleReconciler {
+func NewApiRuleReconciler(mgr manager.Manager, config ApiRuleReconcilerConfiguration, apiGatewayMetrics *metrics.ApiGatewayMetrics) *APIRuleReconciler {
 	return &APIRuleReconciler{
 		Client: mgr.GetClient(),
 		Log:    mgr.GetLogger().WithName("apirule-controller"),
@@ -52,6 +54,7 @@ func NewApiRuleReconciler(mgr manager.Manager, config ApiRuleReconcilerConfigura
 		ReconcilePeriod:          time.Duration(config.ReconciliationPeriod) * time.Second,
 		OnErrorReconcilePeriod:   time.Duration(config.ErrorReconciliationPeriod) * time.Second,
 		MigrationReconcilePeriod: time.Duration(config.MigrationReconciliationPeriod) * time.Second,
+		Metrics:                  apiGatewayMetrics,
 	}
 }
 

docs/release-notes/2.10.2.md

@@ -0,0 +1,4 @@
+## New Features
+
+- We've implemented a faster requeue when the error `object has been modified` occurs [#1496](https://github.com/kyma-project/api-gateway/pull/1496)
+- We've added full support for special characters in asterisk paths [#1569](https://github.com/kyma-project/api-gateway/pull/1569)

internal/metrics/metrics.go

@@ -0,0 +1,27 @@
+package metrics
+
+import (
+	"github.com/prometheus/client_golang/prometheus"
+
+	ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
+)
+
+type ApiGatewayMetrics struct {
+	apiRuleObjectModifiedErrorsCounter prometheus.Counter
+}
+
+func NewApiGatewayMetrics() *ApiGatewayMetrics {
+	apiGatewayMetrics := &ApiGatewayMetrics{
+		apiRuleObjectModifiedErrorsCounter: prometheus.NewCounter(prometheus.CounterOpts{
+			Name:      "api_rule_object_modified_errors_total",
+			Namespace: "api_gateway",
+			Help:      "The total number of errors that occurred while modifying the APIRule object",
+		}),
+	}
+	ctrlmetrics.Registry.MustRegister(apiGatewayMetrics.apiRuleObjectModifiedErrorsCounter)
+	return apiGatewayMetrics
+}
+
+func (m *ApiGatewayMetrics) IncreaseApiRuleObjectModifiedErrorsCounter() {
+	m.apiRuleObjectModifiedErrorsCounter.Inc()
+}

internal/metrics/metrics_test.go

@@ -0,0 +1,23 @@
+package metrics
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	dto "github.com/prometheus/client_model/go"
+)
+
+var _ = Describe("ApiGateway metrics", func() {
+	It("ApiRule object modified errors should increase the counter", func() {
+		metrics := NewApiGatewayMetrics()
+		metrics.IncreaseApiRuleObjectModifiedErrorsCounter()
+
+		metric := metrics.apiRuleObjectModifiedErrorsCounter
+
+		pb := &dto.Metric{}
+		err := metric.Write(pb)
+
+		Expect(err).To(BeNil())
+		Expect(pb.GetCounter().GetValue()).To(Equal(float64(1)))
+	})
+})

internal/processing/processors/v2alpha1/reconciliation_test.go

@@ -266,9 +266,9 @@ var _ = Describe("Reconciliation", func() {
 						Expect(http.Route[0].Destination.Host).To(Equal("example-service.some-namespace.svc.cluster.local"))
 
 						switch http.Match[0].Uri.MatchType.(*v1beta1.StringMatch_Regex).Regex {
-						case "/test$":
+						case "^/test$":
 							break
-						case "/different-path$":
+						case "^/different-path$":
 							break
 						default:
 							Fail("Unexpected match type")
@@ -342,9 +342,9 @@ var _ = Describe("Reconciliation", func() {
 						Expect(http.Route[0].Destination.Host).To(Equal("example-service.some-namespace.svc.cluster.local"))
 
 						switch http.Match[0].Uri.MatchType.(*v1beta1.StringMatch_Regex).Regex {
-						case "/test$":
+						case "^/test$":
 							break
-						case "/different-path$":
+						case "^/different-path$":
 							break
 						default:
 							Fail("Unexpected match type")

internal/processing/processors/v2alpha1/virtualservice/regex_test.go

@@ -0,0 +1,42 @@
+package virtualservice
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"regexp"
+)
+
+var _ = Describe("Envoy templates regex matching", func() {
+	DescribeTable(`{\*} template`,
+		func(input string, shouldMatch bool) {
+			// when
+			matched, err := regexp.MatchString(prepareRegexPath("/{*}"), input)
+
+			// then
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(matched).To(Equal(shouldMatch))
+		},
+		Entry("should not match empty path", "/", false),
+		Entry("should match path with one segment", "/segment", true),
+		Entry("should match special characters", "/segment-._~!$&'()*+,;=:@", true),
+		Entry("should match with correct % encoding", "/segment%20", true),
+		Entry("should not match with incorrect % encoding", "/segment%2", false),
+		Entry("should not match path with multiple segments", "/segment1/segment2/segment3", false),
+	)
+
+	DescribeTable(`{\*\*} template`, func(input string, shouldMatch bool) {
+		// when
+		matched, err := regexp.MatchString(prepareRegexPath("/{**}"), input)
+
+		// then
+		Expect(err).To(Not(HaveOccurred()))
+		Expect(matched).To(Equal(shouldMatch))
+	},
+		Entry("should match empty path", "/", true),
+		Entry("should match path with one segment", "/segment", true),
+		Entry("should match special characters", "/segment-._~!$&'()*+,;=:@", true),
+		Entry("should match with correct % encoding", "/segment%20", true),
+		Entry("should not match with incorrect % encoding", "/segment%2", false),
+		Entry("should match path with multiple segments", "/segment1/segment2/segment3", true),
+	)
+})

internal/processing/processors/v2alpha1/virtualservice/virtual_service_processor.go

@@ -20,8 +20,8 @@ const defaultHttpTimeout uint32 = 180
 
 var (
 	envoyTemplatesTranslation = map[string]string{
-		`{**}`: `[\w\.~\-\/]*`,
-		`{*}`:  `[\w\.~\-]*`,
+		`{**}`: `([A-Za-z0-9-._~!$&'()*+,;=:@/]|%[0-9a-fA-F]{2})*`,
+		`{*}`:  `([A-Za-z0-9-._~!$&'()*+,;=:@]|%[0-9a-fA-F]{2})+`,
 	}
 )
 
@@ -201,7 +201,7 @@ func prepareRegexPath(path string) string {
 		path = strings.ReplaceAll(path, key, replace)
 	}
 
-	return fmt.Sprintf("%s$", path)
+	return fmt.Sprintf("^%s$", path)
 }
 
 func GetVirtualServiceHttpTimeout(apiRuleSpec gatewayv2alpha1.APIRuleSpec, rule gatewayv2alpha1.Rule) uint32 {

internal/processing/processors/v2alpha1/virtualservice/virtual_service_processor_test.go

@@ -96,7 +96,7 @@ var _ = Describe("Fully configured APIRule happy path", func() {
 				Expect(vs.Spec.Http).To(HaveLen(2))
 
 				Expect(vs.Spec.Http[0].Match[0].Method.GetRegex()).To(Equal("^(GET|POST)$"))
-				Expect(vs.Spec.Http[0].Match[0].Uri.GetRegex()).To(Equal("/$"))
+				Expect(vs.Spec.Http[0].Match[0].Uri.GetRegex()).To(Equal("^/$"))
 				Expect(vs.Spec.Http[0].Route[0].Destination.Host).To(Equal("another-service.another-namespace.svc.cluster.local"))
 				Expect(vs.Spec.Http[0].Route[0].Destination.Port.Number).To(Equal(uint32(9999)))
 
@@ -155,6 +155,74 @@ var _ = Describe("VirtualServiceProcessor", func() {
 		Expect(result[0].Action.String()).To(Equal("create"))
 	})
 
+	It("should create a VirtualService with prefix match for wildcard path /*", func() {
+		// given
+		apiRule := NewAPIRuleBuilder().
+			WithGateway("example/example").
+			WithHosts("example.com").
+			WithService("example-service", "example-namespace", 8080).
+			WithTimeout(180).
+			WithRules(
+				NewRuleBuilder().
+					WithMethods("GET").
+					WithPath("/*").
+					NoAuth().Build(),
+			).
+			Build()
+
+		client := GetFakeClient()
+		processor := processors.NewVirtualServiceProcessor(GetTestConfig(), apiRule, nil)
+
+		// when
+		checkVirtualServices(client, processor, []verifier{
+			func(vs *networkingv1beta1.VirtualService) {
+				Expect(vs.Spec.Hosts).To(ConsistOf("example.com"))
+				Expect(vs.Spec.Gateways).To(ConsistOf("example/example"))
+				Expect(vs.Spec.Http).To(HaveLen(1))
+
+				Expect(vs.Spec.Http[0].Match[0].Method.GetRegex()).To(Equal("^(GET)$"))
+				Expect(vs.Spec.Http[0].Match[0].Uri.GetPrefix()).To(Equal("/"))
+			},
+		}, nil, "create")
+	})
+
+	DescribeTable("should create a VirtualService with correct regex for path",
+		func(path string, expectedRegex string) {
+			// given
+			apiRule := NewAPIRuleBuilder().
+				WithGateway("example/example").
+				WithHosts("example.com").
+				WithService("example-service", "example-namespace", 8080).
+				WithTimeout(180).
+				WithRules(
+					NewRuleBuilder().
+						WithMethods("GET").
+						WithPath(path).
+						NoAuth().Build(),
+				).
+				Build()
+
+			client := GetFakeClient()
+			processor := processors.NewVirtualServiceProcessor(GetTestConfig(), apiRule, nil)
+
+			// when
+			checkVirtualServices(client, processor, []verifier{
+				func(vs *networkingv1beta1.VirtualService) {
+					Expect(vs.Spec.Hosts).To(ConsistOf("example.com"))
+					Expect(vs.Spec.Gateways).To(ConsistOf("example/example"))
+					Expect(vs.Spec.Http).To(HaveLen(1))
+
+					Expect(vs.Spec.Http[0].Match[0].Method.GetRegex()).To(Equal("^(GET)$"))
+					Expect(vs.Spec.Http[0].Match[0].Uri.GetRegex()).To(Equal(expectedRegex))
+				},
+			}, nil, "create")
+		},
+		Entry("path is /", "/", "^/$"),
+		Entry("path is /test", "/test", "^/test$"),
+		Entry("path is /test/{*}", "/test/{*}", "^/test/([A-Za-z0-9-._~!$&'()*+,;=:@]|%[0-9a-fA-F]{2})+$"),
+		Entry("path is /test/{**}", "/test/{**}", "^/test/([A-Za-z0-9-._~!$&'()*+,;=:@/]|%[0-9a-fA-F]{2})*$"),
+	)
+
 	It("should create a VirtualService with '/' prefix, when rule in APIRule applies to all paths", func() {
 		apiRule := NewAPIRuleBuilder().
 			WithGateway("example/example").

main.go

@@ -33,6 +33,7 @@ import (
 
 	gatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1"
 	gatewayv2alpha1 "github.com/kyma-project/api-gateway/apis/gateway/v2alpha1"
+	apiGatewayMetrics "github.com/kyma-project/api-gateway/internal/metrics"
 
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
 	// to ensure that exec-entrypoint and run can make use of them.
@@ -217,7 +218,9 @@ func main() {
 		FailureMaxDelay:  flagVar.rateLimiterFailureMaxDelay,
 	}
 
-	if err := gateway.NewApiRuleReconciler(mgr, reconcileConfig).SetupWithManager(mgr, rateLimiterCfg); err != nil {
+	apiGatewayMetrics := apiGatewayMetrics.NewApiGatewayMetrics()
+
+	if err := gateway.NewApiRuleReconciler(mgr, reconcileConfig, apiGatewayMetrics).SetupWithManager(mgr, rateLimiterCfg); err != nil {
 		setupLog.Error(err, "Unable to create controller", "controller", "APIRule")
 		os.Exit(1)
 	}

tests/integration/main_test.go

@@ -107,8 +107,17 @@ func runTestsuite(t *testing.T, testsuite testcontext.Testsuite, config testcont
 	opts := createGoDogOpts(t, testsuite.FeaturePath(), config.TestConcurrency)
 	suite := godog.TestSuite{
 		Name: testsuite.Name(),
-		// We are not using ScenarioInitializer, as this function only needs to set up global resources
+		ScenarioInitializer: func() func(*godog.ScenarioContext) {
+			if testsuite.Name() == "v2alpha1" {
+				return testsuite.InitScenarios
+			}
+			return nil
+		}(),
 		TestSuiteInitializer: func(ctx *godog.TestSuiteContext) {
+			if testsuite.Name() != "v2alpha1" {
+				testsuite.InitScenarios(ctx.ScenarioContext())
+			}
+
 			ctx.BeforeSuite(func() {
 				for _, hook := range testsuite.BeforeSuiteHooks() {
 					err := hook()
@@ -118,14 +127,21 @@ func runTestsuite(t *testing.T, testsuite testcontext.Testsuite, config testcont
 				}
 			})
 
-			testsuite.InitScenarios(ctx.ScenarioContext())
-
 			ctx.AfterSuite(func() {
-				for _, hook := range testsuite.AfterSuiteHooks() {
-					err := hook()
-					if err != nil {
-						t.Fatalf("Cannot run after suite hooks: %s", err.Error())
+				if t.Failed() {
+					log.Printf("Test suite failed, skipping after suite on success hooks")
+				} else {
+					log.Printf("Executing after suite on success hooks")
+					for _, hook := range testsuite.AfterSuiteHooks() {
+						err := hook()
+						if err != nil {
+							t.Fatalf("Cannot run after suite hooks: %s", err.Error())
+						}
 					}
+					log.Printf("After suite hooks executed")
+
+					log.Printf("Tearing down test suite")
+					testsuite.TearDown()
 				}
 			})
 		},
@@ -162,9 +178,6 @@ func createGoDogOpts(t *testing.T, featuresPath []string, concurrency int) godog
 }
 
 func cleanUp(c testcontext.Testsuite, orgJwtHandler string) {
-
-	c.TearDown()
-
 	_, err := SwitchJwtHandler(c, orgJwtHandler)
 	if err != nil {
 		log.Print(err.Error())