Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use regexp2 pkg for path validation #4465

Merged
merged 2 commits into from
Oct 4, 2023
Merged

Use regexp2 pkg for path validation #4465

merged 2 commits into from
Oct 4, 2023

Conversation

jjngx
Copy link
Contributor

@jjngx jjngx commented Oct 3, 2023

Proposed changes

This PR fixes issue #797 and introduces following change:

  • paths with regex expressions are verified using regexp2 package. The package implements Perl5 and .NET compatible regex engine

Testing:

Create VS:

apiVersion: k8s.nginx.org/v1
kind: VirtualServer
metadata:
  name: cafe
  namespace: default
spec:
  host: cafe.example.com
  upstreams:
  - name: tea
    service: tea-svc
    port: 80
  routes:
  - path: /tea
    action:
      pass: tea
  - path: /coffee
    route: default/coffee

Create VSR:

apiVersion: k8s.nginx.org/v1
kind: VirtualServerRoute
metadata:
  name: coffee
  namespace: default
spec:
  host: cafe.example.com
  upstreams:
  - name: latte
    service: latte-svc
    port: 80
  - name: espresso
    service: espresso-svc
    port: 80
  subroutes:
  - path: ~ ^/coffee/latte$ # it's works
    action:
      pass: latte
  - path: ~ ^/coffee/(?!.*\/latte)(?!.*\/americano)(.*) # it isn't working
    action:
      pass: espresso

Verify VSR is created and NIC is not reporting errors:

$ kubectl describe vsr coffee
Name:         coffee
Namespace:    default
Labels:       <none>
Annotations:  <none>
API Version:  k8s.nginx.org/v1
Kind:         VirtualServerRoute
Metadata:
  Creation Timestamp:  2023-10-03T16:34:18Z
  Generation:          1
  Resource Version:    291887
  UID:                 f57cd1fd-d8eb-4864-ba63-f2cb36578b1b
Spec:
  Host:  cafe.example.com
  Subroutes:
    Action:
      Pass:  latte
    Path:    ~ ^/coffee/latte$
    Action:
      Pass:  espresso
    Path:    ~ ^/coffee/(?!.*\/latte)(?!.*\/americano)(.*)
  Upstreams:
    Name:     latte
    Port:     80
    Service:  latte-svc
    Name:     espresso
    Port:     80
    Service:  espresso-svc
Status:
  Message:        Configuration for default/coffee was added or updated
  Reason:         AddedOrUpdated
  Referenced By:
  State:          Valid
Events:
  Type    Reason          Age   From                      Message
  ----    ------          ----  ----                      -------
  Normal  AddedOrUpdated  15s   nginx-ingress-controller  Configuration for default/coffee was added or updated

IC logs:

2023/10/03 16:34:19 [notice] 22#22: signal 29 (SIGIO) received
I1003 16:34:19.046547       1 event.go:298] Event(v1.ObjectReference{Kind:"VirtualServer", Namespace:"default", Name:"cafe", UID:"7f98147e-5127-4cb2-b4ec-5a11e178513c", APIVersion:"k8s.nginx.org/v1", ResourceVersion:"291855", FieldPath:""}): type: 'Normal' reason: 'AddedOrUpdated' Configuration for default/cafe was added or updated
I1003 16:34:19.046746       1 event.go:298] Event(v1.ObjectReference{Kind:"VirtualServerRoute", Namespace:"default", Name:"coffee", UID:"f57cd1fd-d8eb-4864-ba63-f2cb36578b1b", APIVersion:"k8s.nginx.org/v1", ResourceVersion:"291863", FieldPath:""}): type: 'Normal' reason: 'AddedOrUpdated' Configuration for default/coffee was added or updated
I1003 16:34:33.426801       1 leaderelection.go:260] successfully acquired lease nginx-ingress/nginx-ingress-leader-election

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@jjngx jjngx requested a review from a team as a code owner October 3, 2023 16:56
@github-actions github-actions bot added chore Pull requests for routine tasks dependencies Pull requests that update a dependency file labels Oct 3, 2023
@jjngx jjngx linked an issue Oct 3, 2023 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

Merging #4465 (ab72f32) into main (c75d041) will not change coverage.
The diff coverage is 100.00%.

❗ Current head ab72f32 differs from pull request most recent head 408c919. Consider uploading reports for the commit 408c919 to get more accurate results

@@           Coverage Diff           @@
##             main    #4465   +/-   ##
=======================================
  Coverage   51.97%   51.97%           
=======================================
  Files          59       59           
  Lines       16956    16956           
=======================================
  Hits         8813     8813           
  Misses       7848     7848           
  Partials      295      295           
Files Coverage Δ
internal/k8s/validation.go 93.56% <100.00%> (ø)
pkg/apis/configuration/validation/virtualserver.go 93.98% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jjngx jjngx changed the title Verify paths using Perl5-compatible regex engine Use regexp2 engine for path validation Oct 4, 2023
@jjngx jjngx changed the title Use regexp2 engine for path validation Use regexp2 pkg for path validation Oct 4, 2023
@jjngx jjngx requested a review from a team October 4, 2023 09:03
@jjngx jjngx merged commit 38b8751 into main Oct 4, 2023
63 checks passed
@jjngx jjngx deleted the chore/regexlib branch October 4, 2023 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Pull requests for routine tasks dependencies Pull requests that update a dependency file
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Regular expressions with "?!" are not supported in VirtualServer
3 participants