diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go index 95c1157019f..03cfe9c0c22 100644 --- a/internal/gatewayapi/backendtrafficpolicy.go +++ b/internal/gatewayapi/backendtrafficpolicy.go @@ -22,6 +22,7 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/ir" "github.com/envoyproxy/gateway/internal/status" + "github.com/envoyproxy/gateway/internal/utils/regex" ) type policyTargetRouteKey struct { @@ -535,6 +536,9 @@ func buildRateLimitRule(rule egv1a1.RateLimitRule) (*ir.RateLimitRule, error) { } irRule.HeaderMatches = append(irRule.HeaderMatches, m) case *header.Type == egv1a1.HeaderMatchRegularExpression && header.Value != nil: + if err := regex.Validate(*header.Value); err != nil { + return nil, err + } m := &ir.StringMatch{ Name: header.Name, SafeRegex: header.Value, diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index 87ba9a4a25c..07e06cb04a4 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -19,6 +19,8 @@ import ( mcsapi "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/status" + "github.com/envoyproxy/gateway/internal/utils/regex" ) var ( @@ -96,7 +98,16 @@ func (t *Translator) processHTTPRouteParentRefs(httpRoute *HTTPRouteContext, res // Need to compute Route rules within the parentRef loop because // any conditions that come out of it have to go on each RouteParentStatus, // not on the Route as a whole. - routeRoutes := t.processHTTPRouteRules(httpRoute, parentRef, resources) + routeRoutes, err := t.processHTTPRouteRules(httpRoute, parentRef, resources) + if err != nil { + parentRef.SetCondition(httpRoute, + gwapiv1.RouteConditionAccepted, + metav1.ConditionFalse, + gwapiv1.RouteReasonUnsupportedValue, // TODO: better reason + status.Error2ConditionMsg(err), + ) + continue + } // If no negative condition has been set for ResolvedRefs, set "ResolvedRefs=True" if !parentRef.HasCondition(httpRoute, gwapiv1.RouteConditionResolvedRefs, metav1.ConditionFalse) { @@ -137,7 +148,7 @@ func (t *Translator) processHTTPRouteParentRefs(httpRoute *HTTPRouteContext, res } } -func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRef *RouteParentContext, resources *Resources) []*ir.HTTPRoute { +func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRef *RouteParentContext, resources *Resources) ([]*ir.HTTPRoute, error) { var routeRoutes []*ir.HTTPRoute // compute matches, filters, backends @@ -147,7 +158,10 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe // A rule is matched if any one of its matches // is satisfied (i.e. a logical "OR"), so generate // a unique Xds IR HTTPRoute per match. - var ruleRoutes = t.processHTTPRouteRule(httpRoute, ruleIdx, httpFiltersContext, rule) + ruleRoutes, err := t.processHTTPRouteRule(httpRoute, ruleIdx, httpFiltersContext, rule) + if err != nil { + return nil, err + } dstAddrTypeMap := make(map[ir.DestinationAddressType]int) @@ -201,7 +215,7 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe routeRoutes = append(routeRoutes, ruleRoutes...) } - return routeRoutes + return routeRoutes, nil } func processTimeout(irRoute *ir.HTTPRoute, rule gwapiv1.HTTPRouteRule) { @@ -222,7 +236,7 @@ func processTimeout(irRoute *ir.HTTPRoute, rule gwapiv1.HTTPRouteRule) { } } -func (t *Translator) processHTTPRouteRule(httpRoute *HTTPRouteContext, ruleIdx int, httpFiltersContext *HTTPFiltersContext, rule gwapiv1.HTTPRouteRule) []*ir.HTTPRoute { +func (t *Translator) processHTTPRouteRule(httpRoute *HTTPRouteContext, ruleIdx int, httpFiltersContext *HTTPFiltersContext, rule gwapiv1.HTTPRouteRule) ([]*ir.HTTPRoute, error) { var ruleRoutes []*ir.HTTPRoute // If no matches are specified, the implementation MUST match every HTTP request. @@ -255,6 +269,9 @@ func (t *Translator) processHTTPRouteRule(httpRoute *HTTPRouteContext, ruleIdx i Exact: match.Path.Value, } case gwapiv1.PathMatchRegularExpression: + if err := regex.Validate(*match.Path.Value); err != nil { + return nil, err + } irRoute.PathMatch = &ir.StringMatch{ SafeRegex: match.Path.Value, } @@ -268,6 +285,9 @@ func (t *Translator) processHTTPRouteRule(httpRoute *HTTPRouteContext, ruleIdx i Exact: ptr.To(headerMatch.Value), }) case gwapiv1.HeaderMatchRegularExpression: + if err := regex.Validate(headerMatch.Value); err != nil { + return nil, err + } irRoute.HeaderMatches = append(irRoute.HeaderMatches, &ir.StringMatch{ Name: string(headerMatch.Name), SafeRegex: ptr.To(headerMatch.Value), @@ -282,6 +302,9 @@ func (t *Translator) processHTTPRouteRule(httpRoute *HTTPRouteContext, ruleIdx i Exact: ptr.To(queryParamMatch.Value), }) case gwapiv1.QueryParamMatchRegularExpression: + if err := regex.Validate(queryParamMatch.Value); err != nil { + return nil, err + } irRoute.QueryParamMatches = append(irRoute.QueryParamMatches, &ir.StringMatch{ Name: string(queryParamMatch.Name), SafeRegex: ptr.To(queryParamMatch.Value), @@ -299,7 +322,7 @@ func (t *Translator) processHTTPRouteRule(httpRoute *HTTPRouteContext, ruleIdx i ruleRoutes = append(ruleRoutes, irRoute) } - return ruleRoutes + return ruleRoutes, nil } func applyHTTPFiltersContextToIRRoute(httpFiltersContext *HTTPFiltersContext, irRoute *ir.HTTPRoute) { @@ -341,7 +364,16 @@ func (t *Translator) processGRPCRouteParentRefs(grpcRoute *GRPCRouteContext, res // Need to compute Route rules within the parentRef loop because // any conditions that come out of it have to go on each RouteParentStatus, // not on the Route as a whole. - routeRoutes := t.processGRPCRouteRules(grpcRoute, parentRef, resources) + routeRoutes, err := t.processGRPCRouteRules(grpcRoute, parentRef, resources) + if err != nil { + parentRef.SetCondition(grpcRoute, + gwapiv1.RouteConditionAccepted, + metav1.ConditionFalse, + gwapiv1.RouteReasonUnsupportedValue, // TODO: better reason + status.Error2ConditionMsg(err), + ) + continue + } // If no negative condition has been set for ResolvedRefs, set "ResolvedRefs=True" if !parentRef.HasCondition(grpcRoute, gwapiv1.RouteConditionResolvedRefs, metav1.ConditionFalse) { @@ -380,7 +412,7 @@ func (t *Translator) processGRPCRouteParentRefs(grpcRoute *GRPCRouteContext, res } } -func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRef *RouteParentContext, resources *Resources) []*ir.HTTPRoute { +func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRef *RouteParentContext, resources *Resources) ([]*ir.HTTPRoute, error) { var routeRoutes []*ir.HTTPRoute // compute matches, filters, backends @@ -390,7 +422,10 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe // A rule is matched if any one of its matches // is satisfied (i.e. a logical "OR"), so generate // a unique Xds IR HTTPRoute per match. - var ruleRoutes = t.processGRPCRouteRule(grpcRoute, ruleIdx, httpFiltersContext, rule) + ruleRoutes, err := t.processGRPCRouteRule(grpcRoute, ruleIdx, httpFiltersContext, rule) + if err != nil { + return nil, err + } for _, backendRef := range rule.BackendRefs { ds, backendWeight := t.processDestination(backendRef.BackendRef, parentRef, grpcRoute, resources) @@ -430,10 +465,10 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe routeRoutes = append(routeRoutes, ruleRoutes...) } - return routeRoutes + return routeRoutes, nil } -func (t *Translator) processGRPCRouteRule(grpcRoute *GRPCRouteContext, ruleIdx int, httpFiltersContext *HTTPFiltersContext, rule gwapiv1a1.GRPCRouteRule) []*ir.HTTPRoute { +func (t *Translator) processGRPCRouteRule(grpcRoute *GRPCRouteContext, ruleIdx int, httpFiltersContext *HTTPFiltersContext, rule gwapiv1a1.GRPCRouteRule) ([]*ir.HTTPRoute, error) { var ruleRoutes []*ir.HTTPRoute // If no matches are specified, the implementation MUST match every gRPC request. @@ -461,6 +496,9 @@ func (t *Translator) processGRPCRouteRule(grpcRoute *GRPCRouteContext, ruleIdx i Exact: ptr.To(headerMatch.Value), }) case gwapiv1.HeaderMatchRegularExpression: + if err := regex.Validate(headerMatch.Value); err != nil { + return nil, err + } irRoute.HeaderMatches = append(irRoute.HeaderMatches, &ir.StringMatch{ Name: string(headerMatch.Name), SafeRegex: ptr.To(headerMatch.Value), @@ -474,6 +512,16 @@ func (t *Translator) processGRPCRouteRule(grpcRoute *GRPCRouteContext, ruleIdx i case gwapiv1a1.GRPCMethodMatchExact: t.processGRPCRouteMethodExact(match.Method, irRoute) case gwapiv1a1.GRPCMethodMatchRegularExpression: + if match.Method.Service != nil { + if err := regex.Validate(*match.Method.Service); err != nil { + return nil, err + } + } + if match.Method.Method != nil { + if err := regex.Validate(*match.Method.Method); err != nil { + return nil, err + } + } t.processGRPCRouteMethodRegularExpression(match.Method, irRoute) } } @@ -481,7 +529,7 @@ func (t *Translator) processGRPCRouteRule(grpcRoute *GRPCRouteContext, ruleIdx i ruleRoutes = append(ruleRoutes, irRoute) applyHTTPFiltersContextToIRRoute(httpFiltersContext, irRoute) } - return ruleRoutes + return ruleRoutes, nil } func (t *Translator) processGRPCRouteMethodExact(method *gwapiv1a1.GRPCMethodMatch, irRoute *ir.HTTPRoute) { diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index adcd0bd292b..f3c3e47930a 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -27,6 +27,7 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/ir" "github.com/envoyproxy/gateway/internal/status" + "github.com/envoyproxy/gateway/internal/utils/regex" ) func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.SecurityPolicy, @@ -403,8 +404,8 @@ func (t *Translator) buildCORS(cors *egv1a1.CORS) (*ir.CORS, error) { Suffix: &origin.Value, }) case egv1a1.StringMatchRegularExpression: - if err := validateRegex(origin.Value); err != nil { - return nil, err // TODO zhaohuabing: also check regex in other places + if err := regex.Validate(origin.Value); err != nil { + return nil, err } allowOrigins = append(allowOrigins, &ir.StringMatch{ SafeRegex: &origin.Value, diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-regex.in.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-regex.in.yaml new file mode 100644 index 00000000000..72a5e4d8b7c --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-regex.in.yaml @@ -0,0 +1,56 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +grpcRoutes: +- apiVersion: gateway.networking.k8s.io/v1alpha2 + kind: GRPCRoute + metadata: + namespace: default + name: grpcroute-1 + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + namespace: envoy-gateway + name: policy-for-gateway + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + rateLimit: + type: Global + global: + rules: + - clientSelectors: + - headers: + - name: x-user-id + type: RegularExpression + value: "*.illegal.regex" + - name: x-org-id + type: Distinct + limit: + requests: 10 + unit: Hour diff --git a/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-regex.out.yaml b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-regex.out.yaml new file mode 100755 index 00000000000..643b6546eed --- /dev/null +++ b/internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-regex.out.yaml @@ -0,0 +1,153 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + creationTimestamp: null + name: policy-for-gateway + namespace: envoy-gateway + spec: + rateLimit: + global: + rules: + - clientSelectors: + - headers: + - name: x-user-id + type: RegularExpression + value: '*.illegal.regex' + - name: x-org-id + type: Distinct + limit: + requests: 10 + unit: Hour + type: Global + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + status: + conditions: + - lastTransitionTime: null + message: 'Regex "*.illegal.regex" is invalid: error parsing regexp: missing + argument to repetition operator: `*`' + reason: Invalid + status: "False" + type: Accepted +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +grpcRoutes: +- apiVersion: gateway.networking.k8s.io/v1alpha2 + kind: GRPCRoute + metadata: + creationTimestamp: null + name: grpcroute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: null + name: envoy-gateway/gateway-1/http + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-1 +xdsIR: + envoy-gateway/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: true + name: envoy-gateway/gateway-1/http + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: grpcroute/default/grpcroute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: GRPC + weight: 1 + hostname: '*' + name: grpcroute/default/grpcroute-1/rule/0/match/-1/* diff --git a/internal/gatewayapi/testdata/httproute-with-invalid-regex.in.yaml b/internal/gatewayapi/testdata/httproute-with-invalid-regex.in.yaml new file mode 100644 index 00000000000..1b114bdc58e --- /dev/null +++ b/internal/gatewayapi/testdata/httproute-with-invalid-regex.in.yaml @@ -0,0 +1,48 @@ +gateways: + - apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All + - apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-2 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 81 + allowedRoutes: + namespaces: + from: All +httpRoutes: + - apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-1 + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + + rules: + - matches: + - path: + type: RegularExpression + value: "*.foo.bar.com" + backendRefs: + - name: service-1 + port: 8080 diff --git a/internal/gatewayapi/testdata/httproute-with-invalid-regex.out.yaml b/internal/gatewayapi/testdata/httproute-with-invalid-regex.out.yaml new file mode 100755 index 00000000000..cb180f9912c --- /dev/null +++ b/internal/gatewayapi/testdata/httproute-with-invalid-regex.out.yaml @@ -0,0 +1,167 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-2 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 81 + protocol: HTTP + status: + listeners: + - attachedRoutes: 0 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + type: RegularExpression + value: '*.foo.bar.com' + status: + parents: + - conditions: + - lastTransitionTime: null + message: 'Regex "*.foo.bar.com" is invalid: error parsing regexp: missing + argument to repetition operator: `*`' + reason: UnsupportedValue + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: null + name: envoy-gateway/gateway-1/http + ports: + - containerPort: 10080 + name: http + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-1 + envoy-gateway/gateway-2: + proxy: + listeners: + - address: null + name: envoy-gateway/gateway-2/http + ports: + - containerPort: 10081 + name: http + protocol: HTTP + servicePort: 81 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-2 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-2 +xdsIR: + envoy-gateway/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-1/http + port: 10080 + envoy-gateway/gateway-2: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-2/http + port: 10081 diff --git a/internal/gatewayapi/testdata/httproute-with-mirror-filter.out.yaml b/internal/gatewayapi/testdata/httproute-with-mirror-filter.out.yaml old mode 100644 new mode 100755 diff --git a/internal/gatewayapi/testdata/httproute-with-single-rule-with-multiple-rules.out.yaml b/internal/gatewayapi/testdata/httproute-with-single-rule-with-multiple-rules.out.yaml index df007d9e006..15600686c5b 100644 --- a/internal/gatewayapi/testdata/httproute-with-single-rule-with-multiple-rules.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-single-rule-with-multiple-rules.out.yaml @@ -92,15 +92,11 @@ httpRoutes: parents: - conditions: - lastTransitionTime: null - message: Route is accepted - reason: Accepted - status: "True" + message: 'Regex "*regex*" is invalid: error parsing regexp: missing argument + to repetition operator: `*`' + reason: UnsupportedValue + status: "False" type: Accepted - - lastTransitionTime: null - message: Resolved all the Object references for the Route - reason: ResolvedRefs - status: "True" - type: ResolvedRefs controllerName: gateway.envoyproxy.io/gatewayclass-controller parentRef: name: gateway-1 @@ -133,74 +129,3 @@ xdsIR: isHTTP2: false name: envoy-gateway/gateway-1/http port: 10080 - routes: - - backendWeights: - invalid: 0 - valid: 0 - destination: - name: httproute/default/httproute-1/rule/0 - settings: - - addressType: IP - endpoints: - - host: 7.7.7.7 - port: 8080 - protocol: HTTP - weight: 1 - headerMatches: - - distinct: false - exact: exact - name: Header-1 - hostname: '*' - name: httproute/default/httproute-1/rule/0/match/0/* - pathMatch: - distinct: false - exact: /exact - name: "" - queryParamMatches: - - distinct: false - exact: exact - name: QueryParam-1 - - backendWeights: - invalid: 0 - valid: 0 - destination: - name: httproute/default/httproute-1/rule/1 - settings: - - addressType: IP - endpoints: - - host: 7.7.7.7 - port: 8080 - protocol: HTTP - weight: 1 - hostname: '*' - name: httproute/default/httproute-1/rule/1/match/0/* - pathMatch: - distinct: false - name: "" - prefix: /prefix - - backendWeights: - invalid: 0 - valid: 0 - destination: - name: httproute/default/httproute-1/rule/2 - settings: - - addressType: IP - endpoints: - - host: 7.7.7.7 - port: 8080 - protocol: HTTP - weight: 1 - headerMatches: - - distinct: false - name: Header-1 - safeRegex: '*regex*' - hostname: '*' - name: httproute/default/httproute-1/rule/2/match/0/* - pathMatch: - distinct: false - name: "" - safeRegex: '*regex*' - queryParamMatches: - - distinct: false - name: QueryParam-1 - safeRegex: '*regex*' diff --git a/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml index 9cb47f30567..0af09853aab 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml @@ -167,7 +167,7 @@ securityPolicies: status: conditions: - lastTransitionTime: null - message: Secret default/client2-secret does not exist. + message: Secret default/client2-secret does not exist reason: Invalid status: "False" type: Accepted @@ -205,7 +205,7 @@ securityPolicies: status: conditions: - lastTransitionTime: null - message: Secret envoy-gateway/client1-secret does not exist. + message: Secret envoy-gateway/client1-secret does not exist reason: Invalid status: "False" type: Accepted diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml index a19cb0d988b..14e0c27ca1e 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml @@ -79,7 +79,7 @@ securityPolicies: conditions: - lastTransitionTime: null message: 'Error fetching endpoints from issuer: invalid character ''<'' looking - for beginning of value.' + for beginning of value' reason: Invalid status: "False" type: Accepted diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml index 50446b5f819..daec348c156 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml @@ -190,7 +190,7 @@ securityPolicies: status: conditions: - lastTransitionTime: null - message: Secret default/client1-secret does not exist. + message: Secret default/client1-secret does not exist reason: Invalid status: "False" type: Accepted @@ -219,7 +219,7 @@ securityPolicies: status: conditions: - lastTransitionTime: null - message: Secret ref namespace must be unspecified/empty or default. + message: Secret ref namespace must be unspecified/empty or default reason: Invalid status: "False" type: Accepted @@ -248,7 +248,7 @@ securityPolicies: status: conditions: - lastTransitionTime: null - message: Client secret not found in secret default/client3-secret. + message: Client secret not found in secret default/client3-secret reason: Invalid status: "False" type: Accepted diff --git a/internal/gatewayapi/validate.go b/internal/gatewayapi/validate.go index 8acc278a46d..fce666cfb0a 100644 --- a/internal/gatewayapi/validate.go +++ b/internal/gatewayapi/validate.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "net/netip" - "regexp" "strings" v1 "k8s.io/api/core/v1" @@ -783,10 +782,3 @@ func (t *Translator) validateSecretRef( return secret, nil } - -func validateRegex(regex string) error { - if _, err := regexp.Compile(regex); err != nil { - return fmt.Errorf("regex %q is invalid: %v", regex, err) - } - return nil -} diff --git a/internal/status/conditions.go b/internal/status/conditions.go index eb6419d1541..092513184bb 100644 --- a/internal/status/conditions.go +++ b/internal/status/conditions.go @@ -161,11 +161,6 @@ func Error2ConditionMsg(err error) string { runes[0] = unicode.ToUpper(runes[0]) } - // check if the last rune is . - if runes[len(runes)-1] != '.' { - return string(runes) + "." - } - // Convert the rune slice back to a string return string(runes) } diff --git a/internal/status/conditions_test.go b/internal/status/conditions_test.go index 88620b7aa0c..a1181b89b98 100644 --- a/internal/status/conditions_test.go +++ b/internal/status/conditions_test.go @@ -356,7 +356,7 @@ func TestError2ConditionMsg(t *testing.T) { { name: "error with message", err: errors.New("something is wrong"), - expect: "Something is wrong.", + expect: "Something is wrong", }, } for _, tt := range testCases { diff --git a/internal/utils/regex/regex.go b/internal/utils/regex/regex.go new file mode 100644 index 00000000000..cefd1603ddd --- /dev/null +++ b/internal/utils/regex/regex.go @@ -0,0 +1,19 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package regex + +import ( + "fmt" + "regexp" +) + +// Validate validates a regex string. +func Validate(regex string) error { + if _, err := regexp.Compile(regex); err != nil { + return fmt.Errorf("regex %q is invalid: %v", regex, err) + } + return nil +} diff --git a/internal/gatewayapi/validate_test.go b/internal/utils/regex/regex_test.go similarity index 53% rename from internal/gatewayapi/validate_test.go rename to internal/utils/regex/regex_test.go index 86dd9317025..b3dfaf4c4e3 100644 --- a/internal/gatewayapi/validate_test.go +++ b/internal/utils/regex/regex_test.go @@ -3,35 +3,31 @@ // The full text of the Apache license is available in the LICENSE file at // the root of the repo. -package gatewayapi +package regex -import ( - "testing" -) +import "testing" -func Test_validateRegex(t *testing.T) { +func TestValidate(t *testing.T) { tests := []struct { name string regex string wantErr bool }{ { - name: "valid regex", - regex: "^[a-zA-Z0-9-]+$", + name: "Valid regex", + regex: "^[a-z0-9-]+$", wantErr: false, }, { - name: "invalid regex", - regex: "*.foo.com", + name: "Invalid regex", + regex: "^[a-z0-9-++$", wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateRegex(tt.regex) - if (err != nil) != tt.wantErr { - t.Errorf("validateRegex() error = %v, wantErr %v", err, tt.wantErr) - return + if err := Validate(tt.regex); (err != nil) != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) } }) } diff --git a/internal/xds/bootstrap/bootstrap.go b/internal/xds/bootstrap/bootstrap.go index f8418e0e750..867ef313b40 100644 --- a/internal/xds/bootstrap/bootstrap.go +++ b/internal/xds/bootstrap/bootstrap.go @@ -15,6 +15,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/envoyproxy/gateway/internal/utils/regex" ) const ( @@ -168,6 +169,9 @@ func GetRenderedBootstrapConfig(proxyMetrics *egv1a1.ProxyMetrics) (string, erro case egv1a1.StringMatchSuffix: StatsMatcher.Suffixs = append(StatsMatcher.Suffixs, match.Value) case egv1a1.StringMatchRegularExpression: + if err := regex.Validate(match.Value); err != nil { + return "", err + } StatsMatcher.RegularExpressions = append(StatsMatcher.RegularExpressions, match.Value) } }