feat: Use dlclark/regexp2 over standard library's package

Signed-off-by: Pranshu Srivastava <[email protected]>

Author: rexagod

docs/developer/cli-arguments.md

@@ -58,7 +58,7 @@ Flags:
       --logtostderr                                log to standard error instead of files (default true)
       --metric-allowlist string                    Comma-separated list of metrics to be exposed. This list comprises of exact metric names and/or regex patterns. The allowlist and denylist are mutually exclusive.
       --metric-annotations-allowlist string        Comma-separated list of Kubernetes annotations keys that will be used in the resource' labels metric. By default the annotations metrics are not exposed. To include them, provide a list of resource names in their plural form and Kubernetes annotation keys you would like to allow for them (Example: '=namespaces=[kubernetes.io/team,...],pods=[kubernetes.io/team],...)'. A single '*' can be provided per resource instead to allow any annotations, but that has severe performance implications (Example: '=pods=[*]').
-      --metric-denylist string                     Comma-separated list of metrics not to be enabled. This list comprises of exact metric names and/or regex patterns. The allowlist and denylist are mutually exclusive.
+      --metric-denylist string                     Comma-separated list of metrics not to be enabled. This list comprises of exact metric names and/or *ECMAScript-based* regex patterns. The allowlist and denylist are mutually exclusive.
       --metric-labels-allowlist string             Comma-separated list of additional Kubernetes label keys that will be used in the resource' labels metric. By default the labels metrics are not exposed. To include them, provide a list of resource names in their plural form and Kubernetes label keys you would like to allow for them (Example: '=namespaces=[k8s-label-1,k8s-label-n,...],pods=[app],...)'. A single '*' can be provided per resource instead to allow any labels, but that has severe performance implications (Example: '=pods=[*]'). Additionally, an asterisk (*) can be provided as a key, which will resolve to all resources, i.e., assuming '--resources=deployments,pods', '=*=[*]' will resolve to '=deployments=[*],pods=[*]'.
       --metric-opt-in-list string                  Comma-separated list of metrics which are opt-in and not enabled by default. This is in addition to the metric allow- and denylists
       --namespaces string                          Comma-separated list of namespaces to be enabled. Defaults to ""

go.mod

@@ -5,6 +5,7 @@ go 1.23.0
 require (
 	github.com/KimMachineGun/automemlimit v0.7.0
 	github.com/dgryski/go-jump v0.0.0-20211018200510-ba001c3ffce0
+	github.com/dlclark/regexp2 v1.11.5
 	github.com/fsnotify/fsnotify v1.8.0
 	github.com/go-logr/logr v1.4.2
 	github.com/gobuffalo/flect v1.0.3

go.sum

@@ -16,6 +16,8 @@ github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1
 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dgryski/go-jump v0.0.0-20211018200510-ba001c3ffce0 h1:0wH6nO9QEa02Qx8sIQGw6ieKdz+BXjpccSOo9vXNl4U=
 github.com/dgryski/go-jump v0.0.0-20211018200510-ba001c3ffce0/go.mod h1:4hKCXuwrJoYvHZxJ86+bRVTOMyJ0Ej+RqfSm8mHi6KA=
+github.com/dlclark/regexp2 v1.11.5 h1:Q/sSnsKerHeCkc/jSTNq1oCm7KiVgUMZRDUoRu0JQZQ=
+github.com/dlclark/regexp2 v1.11.5/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8=
 github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g=
 github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
 github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8=

pkg/allowdenylist/allowdenylist.go

@@ -18,13 +18,19 @@ package allowdenylist
 
 import (
 	"errors"
-	"regexp"
 	"strings"
+	"time"
+
+	regexp "github.com/dlclark/regexp2"
+	"k8s.io/klog/v2"
 
 	generator "k8s.io/kube-state-metrics/v2/pkg/metric_generator"
 )
 
-// AllowDenyList encapsulates the logic needed to filter based on a string.
+// Use ECMAScript as the default regexp spec to support lookarounds (#2594).
+var regexpDefaultSpec regexp.RegexOptions = regexp.ECMAScript
+
+// AllowDenyList namespaceencapsulates the logic needed to filter based on a string.
 type AllowDenyList struct {
 	list        map[string]struct{}
 	rList       []*regexp.Regexp
@@ -34,6 +40,7 @@ type AllowDenyList struct {
 // New constructs a new AllowDenyList based on a allow- and a
 // denylist. Only one of them can be not empty.
 func New(allow, deny map[string]struct{}) (*AllowDenyList, error) {
+	regexp.DefaultMatchTimeout = time.Minute
 	if len(allow) != 0 && len(deny) != 0 {
 		return nil, errors.New(
 			"allowlist and denylist are both set, they are mutually exclusive, only one of them can be set",
@@ -62,7 +69,7 @@ func New(allow, deny map[string]struct{}) (*AllowDenyList, error) {
 func (l *AllowDenyList) Parse() error {
 	regexes := make([]*regexp.Regexp, 0, len(l.list))
 	for item := range l.list {
-		r, err := regexp.Compile(item)
+		r, err := regexp.Compile(item, regexpDefaultSpec)
 		if err != nil {
 			return err
 		}
@@ -99,25 +106,36 @@ func (l *AllowDenyList) Exclude(items []string) {
 }
 
 // IsIncluded returns if the given item is included.
-func (l *AllowDenyList) IsIncluded(item string) bool {
-	var matched bool
+func (l *AllowDenyList) IsIncluded(item string) (bool, error) {
+	var (
+		matched bool
+		err     error
+	)
 	for _, r := range l.rList {
-		matched = r.MatchString(item)
+		matched, err = r.MatchString(item)
+		if err != nil {
+			return false, err
+		}
 		if matched {
 			break
 		}
 	}
 
 	if l.isAllowList {
-		return matched
+		return matched, nil
 	}
 
-	return !matched
+	return !matched, nil
 }
 
 // IsExcluded returns if the given item is excluded.
-func (l *AllowDenyList) IsExcluded(item string) bool {
-	return !l.IsIncluded(item)
+func (l *AllowDenyList) IsExcluded(item string) (bool, error) {
+	isIncluded, err := l.IsIncluded(item)
+	if err != nil {
+		return false, err
+	}
+
+	return !isIncluded, nil
 }
 
 // Status returns the status of the AllowDenyList that can e.g. be passed into
@@ -137,7 +155,13 @@ func (l *AllowDenyList) Status() string {
 
 // Test returns if the given family generator passes (is included in) the AllowDenyList
 func (l *AllowDenyList) Test(generator generator.FamilyGenerator) bool {
-	return l.IsIncluded(generator.Name)
+	isIncluded, err := l.IsIncluded(generator.Name)
+	if err != nil {
+		klog.ErrorS(err, "Error while processing allow-deny entries for generator", "generator", generator.Name)
+		return false
+	}
+
+	return isIncluded
 }
 
 func copyList(l map[string]struct{}) map[string]struct{} {

pkg/allowdenylist/allowdenylist_test.go

@@ -17,8 +17,12 @@ limitations under the License.
 package allowdenylist
 
 import (
-	"regexp"
+	"fmt"
+	"strings"
 	"testing"
+	"time"
+
+	regexp "github.com/dlclark/regexp2"
 )
 
 func TestNew(t *testing.T) {
@@ -76,7 +80,11 @@ func TestInclude(t *testing.T) {
 			t.Fatal("expected Parse() to not fail")
 		}
 
-		if !allowlist.IsIncluded("item1") {
+		isIncluded, err := allowlist.IsIncluded("item1")
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if !isIncluded {
 			t.Fatal("expected included item to be included")
 		}
 	})
@@ -93,7 +101,11 @@ func TestInclude(t *testing.T) {
 			t.Fatalf("expected Parse() to not fail, but got error : %v", err)
 		}
 
-		if !denylist.IsIncluded(item1) {
+		isIncluded, err := denylist.IsIncluded(item1)
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if !isIncluded {
 			t.Fatal("expected included item to be included")
 		}
 	})
@@ -103,13 +115,17 @@ func TestInclude(t *testing.T) {
 			t.Fatal("expected New() to not fail")
 		}
 
-		allowlist.Include([]string{"kube_.*_info"})
+		allowlist.Include([]string{"kube_(?=secret).*_info"})
 		err = allowlist.Parse()
 		if err != nil {
 			t.Fatalf("expected Parse() to not fail, but got error : %v", err)
 		}
 
-		if !allowlist.IsIncluded("kube_secret_info") {
+		isIncluded, err := allowlist.IsIncluded("kube_secret_info")
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if !isIncluded {
 			t.Fatal("expected included item to be included")
 		}
 	})
@@ -124,22 +140,38 @@ func TestInclude(t *testing.T) {
 			t.Fatal("expected New() to not fail")
 		}
 
-		denylist.Exclude([]string{"kube_node_.*_cores", "kube_pod_.*_bytes"})
+		denylist.Exclude([]string{"kube_(?=node.*cores|pod.*bytes)"})
 		err = denylist.Parse()
 		if err != nil {
 			t.Fatalf("expected Parse() to not fail, but got error : %v", err)
 		}
 
-		if denylist.IsExcluded(item1) {
+		isExcluded, err := denylist.IsExcluded(item1)
+		if err != nil {
+			t.Fatal("expected IsExcluded() to not fail")
+		}
+		if isExcluded {
 			t.Fatalf("expected included %s to be included", item1)
 		}
-		if denylist.IsIncluded(item2) {
+		isIncluded, err := denylist.IsIncluded(item2)
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if isIncluded {
 			t.Fatalf("expected included %s to be excluded", item2)
 		}
-		if denylist.IsIncluded(item3) {
+		isIncluded, err = denylist.IsIncluded(item3)
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if isIncluded {
 			t.Fatalf("expected included %s to be excluded", item3)
 		}
-		if denylist.IsExcluded(item4) {
+		isExcluded, err = denylist.IsExcluded(item4)
+		if err != nil {
+			t.Fatal("expected IsExcluded() to not fail")
+		}
+		if isExcluded {
 			t.Fatalf("expected included %s to be included", item4)
 		}
 	})
@@ -159,7 +191,11 @@ func TestExclude(t *testing.T) {
 			t.Fatalf("expected Parse() to not fail, but got error : %v", err)
 		}
 
-		if allowlist.IsIncluded(item1) {
+		isIncluded, err := allowlist.IsIncluded(item1)
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if isIncluded {
 			t.Fatal("expected excluded item to be excluded")
 		}
 	})
@@ -176,7 +212,11 @@ func TestExclude(t *testing.T) {
 			t.Fatalf("expected Parse() to not fail, but got error : %v", err)
 		}
 
-		if denylist.IsIncluded(item1) {
+		isIncluded, err := denylist.IsIncluded(item1)
+		if err != nil {
+			t.Fatal("expected IsIncluded() to not fail")
+		}
+		if isIncluded {
 			t.Fatal("expected excluded item to be excluded")
 		}
 	})
@@ -224,7 +264,8 @@ func TestStatus(t *testing.T) {
 		allowlist, _ := New(map[string]struct{}{item1: {}, item2: {}}, map[string]struct{}{})
 		actualStatusString := allowlist.Status()
 		expectedRegexPattern := `^Including the following lists that were on allowlist: (item1|item2), (item2|item1)$`
-		matched, _ := regexp.MatchString(expectedRegexPattern, actualStatusString)
+		re := regexp.MustCompile(expectedRegexPattern, regexpDefaultSpec)
+		matched, _ := re.MatchString(actualStatusString)
 		if !matched {
 			t.Errorf("expected status %q but got %q", expectedRegexPattern, actualStatusString)
 		}
@@ -244,9 +285,38 @@ func TestStatus(t *testing.T) {
 		denylist, _ := New(map[string]struct{}{}, map[string]struct{}{item1: {}, item2: {}})
 		actualStatusString := denylist.Status()
 		expectedRegexPattern := `^Excluding the following lists that were on denylist: (item1|item2), (item2|item1)$`
-		matched, _ := regexp.MatchString(expectedRegexPattern, actualStatusString)
+		re := regexp.MustCompile(expectedRegexPattern, regexpDefaultSpec)
+		matched, _ := re.MatchString(actualStatusString)
 		if !matched {
 			t.Errorf("expected status %q but got %q", expectedRegexPattern, actualStatusString)
 		}
 	})
 }
+
+func TestCatastrophicBacktrackTimeout(t *testing.T) {
+	r, err := regexp.Compile("(.+)*\\?", 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var exp = "Lorem ipsum dolor sit amet, consectetur adipiscing elit"
+	exp = strings.Repeat(exp, 2^10)
+
+	timeout := regexp.DefaultMatchTimeout
+	t.Logf("regexp.DefaultMatchTimeout set to: %v", timeout)
+	buffer := 500 * time.Millisecond
+	t.Run(fmt.Sprint(timeout), func(t *testing.T) {
+		r.MatchTimeout = timeout
+		start := time.Now()
+		_, err = r.FindStringMatch(exp)
+		if err != nil && !strings.HasPrefix(err.Error(), "match timeout") {
+			t.Fatal(err)
+		}
+		if err == nil {
+			t.Fatal("expected catastrophic backtracking error")
+		}
+		elapsed := time.Since(start)
+		if elapsed > timeout+buffer {
+			t.Fatalf("timeout %v exceeded: %v", timeout, elapsed)
+		}
+	})
+}

pkg/options/options.go

@@ -163,7 +163,7 @@ func (o *Options) AddFlags(cmd *cobra.Command) {
 	o.cmd.Flags().Var(&o.AnnotationsAllowList, "metric-annotations-allowlist", "Comma-separated list of Kubernetes annotations keys that will be used in the resource' labels metric. By default the annotations metrics are not exposed. To include them, provide a list of resource names in their plural form and Kubernetes annotation keys you would like to allow for them (Example: '=namespaces=[kubernetes.io/team,...],pods=[kubernetes.io/team],...)'. A single '*' can be provided per resource instead to allow any annotations, but that has severe performance implications (Example: '=pods=[*]').")
 	o.cmd.Flags().Var(&o.LabelsAllowList, "metric-labels-allowlist", "Comma-separated list of additional Kubernetes label keys that will be used in the resource' labels metric. By default the labels metrics are not exposed. To include them, provide a list of resource names in their plural form and Kubernetes label keys you would like to allow for them (Example: '=namespaces=[k8s-label-1,k8s-label-n,...],pods=[app],...)'. A single '*' can be provided per resource instead to allow any labels, but that has severe performance implications (Example: '=pods=[*]'). Additionally, an asterisk (*) can be provided as a key, which will resolve to all resources, i.e., assuming '--resources=deployments,pods', '=*=[*]' will resolve to '=deployments=[*],pods=[*]'.")
 	o.cmd.Flags().Var(&o.MetricAllowlist, "metric-allowlist", "Comma-separated list of metrics to be exposed. This list comprises of exact metric names and/or regex patterns. The allowlist and denylist are mutually exclusive.")
-	o.cmd.Flags().Var(&o.MetricDenylist, "metric-denylist", "Comma-separated list of metrics not to be enabled. This list comprises of exact metric names and/or regex patterns. The allowlist and denylist are mutually exclusive.")
+	o.cmd.Flags().Var(&o.MetricDenylist, "metric-denylist", "Comma-separated list of metrics not to be enabled. This list comprises of exact metric names and/or *ECMAScript-based* regex patterns. The allowlist and denylist are mutually exclusive.")
 	o.cmd.Flags().Var(&o.MetricOptInList, "metric-opt-in-list", "Comma-separated list of metrics which are opt-in and not enabled by default. This is in addition to the metric allow- and denylists")
 	o.cmd.Flags().Var(&o.Namespaces, "namespaces", fmt.Sprintf("Comma-separated list of namespaces to be enabled. Defaults to %q", &DefaultNamespaces))
 	o.cmd.Flags().Var(&o.NamespacesDenylist, "namespaces-denylist", "Comma-separated list of namespaces not to be enabled. If namespaces and namespaces-denylist are both set, only namespaces that are excluded in namespaces-denylist will be used.")