Moved and renamed all Predicate names and Filter names for consistenc…

…y and ease of use (#1828) * Moved and renamed all predicate name constants Moved all predicate name constants to the predicate package, added constants for the predicates that used string values. By having all predicates in the same place, with consistent naming, it makes it easier to use Predicates if you use Skipper as a Library and build routes using code. Signed-off-by: Peter Klijn <[email protected]> * Fixed a typo Signed-off-by: Peter Klijn <[email protected]> * Moved and renamed all filter name constants Moved all filter name constants to the filters package, added constants for the filters that used string values. By having all filters in the same place, with consistent naming, it makes it easier to use Filters if you use Skipper as a Library and build routes using code. Signed-off-by: Peter Klijn <[email protected]> * Move deprecated constants back, as the staticcheck does not approve Signed-off-by: Peter Klijn <[email protected]> * Removed breaking changes marking old public constants as deprecated Signed-off-by: Peter Klijn <[email protected]> * Fix staticcheck errors Fixed the following staticcheck errors: filters/auth/grantcallback.go:11:1: comment on exported const GrantCallbackName should be of the form "GrantCallbackName ..." (ST1022) filters/auth/grantclaimsquery.go:12:1: comment on exported const GrantClaimsQueryName should be of the form "GrantClaimsQueryName ..." (ST1022) filters/diag/absorb.go:14:1: comment on exported const AbsorbName should be of the form "AbsorbName ..." (ST1022) filters/diag/absorb.go:17:1: comment on exported const AbsorbSilentName should be of the form "AbsorbSilentName ..." (ST1022) filters/rfc/rfc.go:8:1: comment on exported const Name should be of the form "Name ..." (ST1022) filters/tee/teeloopback.go:9:1: comment on exported const FilterName should be of the form "FilterName ..." (ST1022) predicates/cookie/cookie.go:14:1: comment on exported const Name should be of the form "Name ..." (ST1022) predicates/methods/methods.go:29:1: comment on exported const Name should be of the form "Name ..." (ST1022) Signed-off-by: Peter Klijn <[email protected]> * Solved merge conflict, added NameHost in Rfc filter Signed-off-by: Peter Klijn <[email protected]>
zalando · Oct 1, 2021 · 24f08cf · 24f08cf
1 parent cbff642
commit 24f08cf
Show file tree

Hide file tree

Showing 110 changed files with 866 additions and 574 deletions.
diff --git a/dataclients/kubernetes/ingress.go b/dataclients/kubernetes/ingress.go
@@ -11,8 +11,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/zalando/skipper/dataclients/kubernetes/definitions"
 	"github.com/zalando/skipper/eskip"
-	"github.com/zalando/skipper/predicates/primitive"
-	"github.com/zalando/skipper/predicates/traffic"
+	"github.com/zalando/skipper/predicates"
 )
 
 const (
@@ -249,14 +248,14 @@ func setTraffic(r *eskip.Route, svcName string, weight float64, noopCount int) {
 	// add traffic predicate if traffic weight is between 0.0 and 1.0
 	if 0.0 < weight && weight < 1.0 {
 		r.Predicates = append([]*eskip.Predicate{{
-			Name: traffic.PredicateName,
+			Name: predicates.TrafficName,
 			Args: []interface{}{weight},
 		}}, r.Predicates...)
 		log.Debugf("Traffic weight %.2f for backend '%s'", weight, svcName)
 	}
 	for i := 0; i < noopCount; i++ {
 		r.Predicates = append([]*eskip.Predicate{{
-			Name: primitive.NameTrue,
+			Name: predicates.TrueName,
 			Args: []interface{}{},
 		}}, r.Predicates...)
 	}

diff --git a/dataclients/kubernetes/kube.go b/dataclients/kubernetes/kube.go
@@ -13,9 +13,8 @@ import (
 
 	log "github.com/sirupsen/logrus"
 	"github.com/zalando/skipper/eskip"
-	"github.com/zalando/skipper/filters/accesslog"
-	"github.com/zalando/skipper/filters/builtin"
-	"github.com/zalando/skipper/predicates/source"
+	"github.com/zalando/skipper/filters"
+	"github.com/zalando/skipper/predicates"
 )
 
 const (
@@ -359,11 +358,11 @@ func (c *Client) loadAndConvert() ([]*eskip.Route, error) {
 func shuntRoute(r *eskip.Route) {
 	r.Filters = []*eskip.Filter{
 		{
-			Name: builtin.StatusName,
+			Name: filters.StatusName,
 			Args: []interface{}{502.0},
 		},
 		{
-			Name: builtin.InlineContentName,
+			Name: filters.InlineContentName,
 			Args: []interface{}{"no endpoints"},
 		},
 	}
@@ -373,7 +372,7 @@ func shuntRoute(r *eskip.Route) {
 
 func healthcheckRoute(healthy, reverseSourcePredicate bool) *eskip.Route {
 	logFilters := []*eskip.Filter{{
-		Name: builtin.StatusName,
+		Name: filters.StatusName,
 		Args: []interface{}{http.StatusOK}},
 	}
 	if !healthy {
@@ -382,20 +381,20 @@ func healthcheckRoute(healthy, reverseSourcePredicate bool) *eskip.Route {
 	// log if unhealthy or a debug loglevel
 	if healthy && !log.IsLevelEnabled(log.DebugLevel) {
 		logFilters = append(logFilters, &eskip.Filter{
-			Name: accesslog.DisableAccessLogName,
+			Name: filters.DisableAccessLogName,
 			Args: []interface{}{200},
 		})
 	}
 
 	var p []*eskip.Predicate
 	if reverseSourcePredicate {
 		p = []*eskip.Predicate{{
-			Name: source.NameLast,
+			Name: predicates.SourceFromLastName,
 			Args: internalIPs,
 		}}
 	} else {
 		p = []*eskip.Predicate{{
-			Name: source.Name,
+			Name: predicates.SourceName,
 			Args: internalIPs,
 		}}
 	}

diff --git a/dataclients/kubernetes/kube_test.go b/dataclients/kubernetes/kube_test.go
@@ -32,8 +32,8 @@ import (
 
 	"github.com/zalando/skipper/dataclients/kubernetes/definitions"
 	"github.com/zalando/skipper/eskip"
-	"github.com/zalando/skipper/filters/builtin"
-	"github.com/zalando/skipper/predicates/source"
+	"github.com/zalando/skipper/filters"
+	"github.com/zalando/skipper/predicates"
 )
 
 type testAPI struct {
@@ -364,10 +364,10 @@ func checkHealthcheck(t *testing.T, got []*eskip.Route, expected, healthy, rever
 
 		var found bool
 		for _, p := range r.Predicates {
-			if reversed && p.Name != source.NameLast {
+			if reversed && p.Name != predicates.SourceFromLastName {
 				continue
 			}
-			if !reversed && p.Name != source.Name {
+			if !reversed && p.Name != predicates.SourceName {
 				continue
 			}
 
@@ -398,7 +398,7 @@ func checkHealthcheck(t *testing.T, got []*eskip.Route, expected, healthy, rever
 		}
 
 		for _, f := range r.Filters {
-			if f.Name != builtin.StatusName {
+			if f.Name != filters.StatusName {
 				continue
 			}
 

diff --git a/dataclients/kubernetes/redirect.go b/dataclients/kubernetes/redirect.go
@@ -8,7 +8,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/zalando/skipper/dataclients/kubernetes/definitions"
 	"github.com/zalando/skipper/eskip"
-	"github.com/zalando/skipper/routing"
+	"github.com/zalando/skipper/predicates"
 )
 
 const (
@@ -91,7 +91,7 @@ func initRedirectRoute(r *eskip.Route, code int) {
 
 	// Give this route a higher weight so that it will get precedence over existing routes
 	r.Predicates = append([]*eskip.Predicate{{
-		Name: routing.WeightPredicateName,
+		Name: predicates.WeightName,
 		Args: []interface{}{float64(1000)},
 	}}, r.Predicates...)
 
@@ -112,7 +112,7 @@ func initDisableRedirectRoute(r *eskip.Route) {
 
 	// Give this route a higher weight so that it will get precedence over existing routes
 	r.Predicates = append([]*eskip.Predicate{{
-		Name: routing.WeightPredicateName,
+		Name: predicates.WeightName,
 		Args: []interface{}{float64(1000)},
 	}}, r.Predicates...)
 }

diff --git a/filters/accesslog/control.go b/filters/accesslog/control.go
@@ -3,11 +3,11 @@ package accesslog
 import "github.com/zalando/skipper/filters"
 
 const (
-	// DisableAccessLogName is the filter name seen by the user
-	DisableAccessLogName = "disableAccessLog"
+	// Deprecated, use filters.DisableAccessLogName instead
+	DisableAccessLogName = filters.DisableAccessLogName
 
-	// EnableAccessLogName is the filter name seen by the user
-	EnableAccessLogName = "enableAccessLog"
+	// Deprecated, use filters.EnableAccessLogName instead
+	EnableAccessLogName = filters.EnableAccessLogName
 
 	// AccessLogEnabledKey is the key used in the state bag to pass the access log state to the proxy.
 	AccessLogEnabledKey = "statebag:access_log:proxy:enabled"
@@ -64,7 +64,7 @@ func NewDisableAccessLog() filters.Spec {
 	return &disableAccessLog{}
 }
 
-func (*disableAccessLog) Name() string { return DisableAccessLogName }
+func (*disableAccessLog) Name() string { return filters.DisableAccessLogName }
 
 func (al *disableAccessLog) CreateFilter(args []interface{}) (filters.Filter, error) {
 	return extractFilterValues(args, false)
@@ -82,7 +82,7 @@ func NewEnableAccessLog() filters.Spec {
 	return &enableAccessLog{}
 }
 
-func (*enableAccessLog) Name() string { return EnableAccessLogName }
+func (*enableAccessLog) Name() string { return filters.EnableAccessLogName }
 
 func (al *enableAccessLog) CreateFilter(args []interface{}) (filters.Filter, error) {
 	return extractFilterValues(args, true)

diff --git a/filters/accesslog/disable.go b/filters/accesslog/disable.go
@@ -5,7 +5,7 @@ import (
 )
 
 const (
-	// AccessLogDisabledName is the filter name seen by the user
+	// Deprecated: use DisableAccessLogName or EnableAccessLogName
 	AccessLogDisabledName = "accessLogDisabled"
 )
 

diff --git a/filters/apiusagemonitoring/filter.go b/filters/apiusagemonitoring/filter.go
@@ -24,7 +24,7 @@ const (
 )
 
 const (
-	stateBagKey = "filter." + Name
+	stateBagKey = "filter." + filters.ApiUsageMonitoringName
 )
 
 const (

diff --git a/filters/apiusagemonitoring/noop.go b/filters/apiusagemonitoring/noop.go
@@ -7,7 +7,7 @@ type noopSpec struct {
 }
 
 func (*noopSpec) Name() string {
-	return Name
+	return filters.ApiUsageMonitoringName
 }
 
 func (s *noopSpec) CreateFilter(config []interface{}) (filters.Filter, error) {

diff --git a/filters/apiusagemonitoring/spec.go b/filters/apiusagemonitoring/spec.go
@@ -13,15 +13,16 @@ import (
 )
 
 const (
-	Name = "apiUsageMonitoring"
+	// Deprecated, use filters.ApiUsageMonitoringName instead
+	Name = filters.ApiUsageMonitoringName
 
 	unknownPlaceholder = "{unknown}"
 	noMatchPlaceholder = "{no-match}"
 	noTagPlaceholder   = "{no-tag}"
 )
 
 var (
-	log      = logrus.WithField("filter", Name)
+	log      = logrus.WithField("filter", filters.ApiUsageMonitoringName)
 	regCache = sync.Map{}
 )
 
@@ -47,7 +48,7 @@ func NewApiUsageMonitoring(
 	realmsTrackingPattern string,
 ) filters.Spec {
 	if !enabled {
-		log.Debugf("filter %q is not enabled. spec returns `noop` filters.", Name)
+		log.Debugf("filter %q is not enabled. spec returns `noop` filters.", filters.ApiUsageMonitoringName)
 		return &noopSpec{&noopFilter{}}
 	}
 
@@ -122,7 +123,7 @@ type apiUsageMonitoringSpec struct {
 }
 
 func (s *apiUsageMonitoringSpec) Name() string {
-	return Name
+	return filters.ApiUsageMonitoringName
 }
 
 func (s *apiUsageMonitoringSpec) CreateFilter(args []interface{}) (filter filters.Filter, err error) {

diff --git a/filters/auth/basic.go b/filters/auth/basic.go
@@ -7,7 +7,9 @@ import (
 )
 
 const (
-	Name                      = "basicAuth"
+	// Deprecated, use filters.BasicAuthName instead
+	Name = filters.BasicAuthName
+
 	ForceBasicAuthHeaderName  = "WWW-Authenticate"
 	ForceBasicAuthHeaderValue = "Basic realm="
 	DefaultRealmName          = "Basic Realm"
@@ -72,4 +74,4 @@ func (spec *basicSpec) CreateFilter(config []interface{}) (filters.Filter, error
 	}, nil
 }
 
-func (spec *basicSpec) Name() string { return Name }
+func (spec *basicSpec) Name() string { return filters.BasicAuthName }
diff --git a/filters/auth/bearer.go b/filters/auth/bearer.go
@@ -6,7 +6,8 @@ import (
 )
 
 const (
-	BearerInjectorName = "bearerinjector"
+	// Deprecated, use filters.BearerInjectorName instead
+	BearerInjectorName = filters.BearerInjectorName
 )
 
 type (
@@ -26,7 +27,7 @@ func NewBearerInjector(sr secrets.SecretsReader) filters.Spec {
 }
 
 func (*bearerInjectorSpec) Name() string {
-	return BearerInjectorName
+	return filters.BearerInjectorName
 }
 
 func (b *bearerInjectorSpec) CreateFilter(args []interface{}) (filters.Filter, error) {

diff --git a/filters/auth/bearer_test.go b/filters/auth/bearer_test.go
@@ -15,8 +15,8 @@ import (
 
 func Test_bearerInjectorSpec_Name(t *testing.T) {
 	b := &bearerInjectorSpec{}
-	if got := b.Name(); got != BearerInjectorName {
-		t.Errorf("bearerInjectorSpec.Name() = %v, want %v", got, BearerInjectorName)
+	if got := b.Name(); got != filters.BearerInjectorName {
+		t.Errorf("bearerInjectorSpec.Name() = %v, want %v", got, filters.BearerInjectorName)
 	}
 }
 

diff --git a/filters/auth/forwardtoken.go b/filters/auth/forwardtoken.go
@@ -10,7 +10,8 @@ import (
 )
 
 const (
-	ForwardTokenName = "forwardToken"
+	// Deprecated, use filters.ForwardTokenName instead
+	ForwardTokenName = filters.ForwardTokenName
 )
 
 type (
@@ -28,7 +29,7 @@ func NewForwardToken() filters.Spec {
 }
 
 func (s *forwardTokenSpec) Name() string {
-	return ForwardTokenName
+	return filters.ForwardTokenName
 }
 
 func (*forwardTokenSpec) CreateFilter(args []interface{}) (filters.Filter, error) {

diff --git a/filters/auth/grant.go b/filters/auth/grant.go
@@ -13,7 +13,8 @@ import (
 )
 
 const (
-	OAuthGrantName = "oauthGrant"
+	// Deprecated, use filters.OAuthGrantName instead
+	OAuthGrantName = filters.OAuthGrantName
 
 	secretsRefreshInternal = time.Minute
 	tokenWasRefreshed      = "oauth-did-refresh"
@@ -31,7 +32,7 @@ type grantFilter struct {
 	config OAuthConfig
 }
 
-func (s *grantSpec) Name() string { return OAuthGrantName }
+func (s *grantSpec) Name() string { return filters.OAuthGrantName }
 
 func (s *grantSpec) CreateFilter([]interface{}) (filters.Filter, error) {
 	return &grantFilter{

diff --git a/filters/auth/grant_test.go b/filters/auth/grant_test.go
@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/zalando/skipper/eskip"
+	"github.com/zalando/skipper/filters"
 	"github.com/zalando/skipper/filters/auth"
 	"github.com/zalando/skipper/filters/builtin"
 	"github.com/zalando/skipper/proxy/proxytest"
@@ -178,8 +179,8 @@ func newAuthProxy(config *auth.OAuthConfig, routes ...*eskip.Route) (*proxytest.
 func newSimpleGrantAuthProxy(t *testing.T, config *auth.OAuthConfig) *proxytest.TestProxy {
 	proxy, err := newAuthProxy(config, &eskip.Route{
 		Filters: []*eskip.Filter{
-			{Name: auth.OAuthGrantName},
-			{Name: "status", Args: []interface{}{http.StatusNoContent}},
+			{Name: filters.OAuthGrantName},
+			{Name: filters.StatusName, Args: []interface{}{http.StatusNoContent}},
 		},
 		BackendType: eskip.ShuntBackend,
 	})

diff --git a/filters/auth/grantcallback.go b/filters/auth/grantcallback.go
@@ -8,7 +8,9 @@ import (
 	"golang.org/x/oauth2"
 )
 
-const GrantCallbackName = "grantCallback"
+// GrantCallbackName is the filter name
+// Deprecated, use filters.GrantCallbackName instead
+const GrantCallbackName = filters.GrantCallbackName
 
 type grantCallbackSpec struct {
 	config OAuthConfig
@@ -18,7 +20,7 @@ type grantCallbackFilter struct {
 	config OAuthConfig
 }
 
-func (*grantCallbackSpec) Name() string { return GrantCallbackName }
+func (*grantCallbackSpec) Name() string { return filters.GrantCallbackName }
 
 func (s *grantCallbackSpec) CreateFilter([]interface{}) (filters.Filter, error) {
 	return &grantCallbackFilter{

diff --git a/filters/auth/grantclaimsquery.go b/filters/auth/grantclaimsquery.go
@@ -9,14 +9,16 @@ package auth
 
 import "github.com/zalando/skipper/filters"
 
-const GrantClaimsQueryName = "grantClaimsQuery"
+// GrantClaimsQueryName is the filter name
+// Deprecated, use filters.GrantClaimsQueryName instead
+const GrantClaimsQueryName = filters.GrantClaimsQueryName
 
 type grantClaimsQuerySpec struct {
 	oidcSpec oidcIntrospectionSpec
 }
 
 func (s *grantClaimsQuerySpec) Name() string {
-	return GrantClaimsQueryName
+	return filters.GrantClaimsQueryName
 }
 
 func (s *grantClaimsQuerySpec) CreateFilter(args []interface{}) (filters.Filter, error) {
-Original file line number
+Diff line change
@@ Expand Up / @@ -5,7 +5,7 @@ import ( @@
     )
     const (
-    	// AccessLogDisabledName is the filter name seen by the user
+    	// Deprecated: use DisableAccessLogName or EnableAccessLogName
     	AccessLogDisabledName = "accessLogDisabled"
     )
@@ Expand Down @@