feat: support regex in path #779

lxm · 2021-11-30T12:44:46Z

Please answer these questions before submitting a pull request

Why submit this pull request?
New feature provided
Related issues request help: path like /api/*/action #768

New feature or improvement

support a new annotation k8s.apisix.apache.org/use-regex which to change path behavior using apisix's vars

tokers

We may also add some test cases.

pkg/kube/translation/annotations/types.go

pkg/kube/translation/ingress.go

lxm · 2021-12-01T09:43:01Z

We may also add some test cases.

have written more test cases.
retested the feature

tokers · 2021-12-02T01:40:43Z

We may also add some test cases.

have written more test cases. retested the feature

By say adding test cases, we'd better also add the E2E test cases.

lxm · 2021-12-02T06:37:16Z

We may also add some test cases.

have written more test cases. retested the feature

By say adding test cases, we'd better also add the E2E test cases.

working on it.

codecov-commenter · 2021-12-02T12:56:53Z

Codecov Report

Merging #779 (f0f5775) into master (1bbadf0) will increase coverage by 0.49%.
The diff coverage is 80.76%.

@@            Coverage Diff             @@
##           master     #779      +/-   ##
==========================================
+ Coverage   32.42%   32.91%   +0.49%     
==========================================
  Files          65       65              
  Lines        6872     6929      +57     
==========================================
+ Hits         2228     2281      +53     
- Misses       4396     4397       +1     
- Partials      248      251       +3

Impacted Files	Coverage Δ
pkg/kube/translation/annotations/cors.go	`100.00% <ø> (ø)`
pkg/kube/translation/annotations/iprestriction.go	`100.00% <ø> (ø)`
pkg/kube/translation/annotations/redirect.go	`0.00% <ø> (ø)`
pkg/kube/translation/annotations/rewrite.go	`0.00% <ø> (ø)`
pkg/kube/translation/annotations/types.go	`100.00% <ø> (ø)`
pkg/kube/translation/ingress.go	`73.16% <80.76%> (+4.32%)`	⬆️
pkg/kube/translation/apisix_route.go	`21.03% <0.00%> (+0.35%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bbadf0...f0f5775. Read the comment docs.

lxm · 2021-12-03T12:58:22Z

@tokers @tao12345666333 @lingsamuel @gxthrj

finally got the e2e testing work.
now, this PR needs a review again.

tao12345666333 · 2021-12-04T09:10:56Z

Thanks! I have put it in my list.

PS: I will submit a PR to fix the problem of e2e failure (Ingress LB status case)

tao12345666333 · 2021-12-04T09:12:52Z

In addition, can you please add documentation to introduce this feature?

lxm · 2021-12-05T02:55:24Z

In addition, can you please add documentation to introduce this feature?

sure.
Because exprs is complex, more configurations may be added in the future.

pkg/kube/translation/ingress.go

gxthrj · 2021-12-06T07:08:29Z

pkg/kube/translation/ingress.go

@@ -29,16 +29,23 @@ import (

 	"github.com/apache/apisix-ingress-controller/pkg/id"
 	apisixv12 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
+	configv2alpha1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2alpha1"


Please merge master， the v2alpha1 no longer exist, use v2beta3 instead.

a lot of changes were made.
I've done the changes

lxm · 2021-12-06T10:18:35Z

In addition, can you please add documentation to introduce this feature?

done for docs

Signed-off-by: lxm <lxm.xupt@gmail.com>

lxm · 2021-12-07T06:52:35Z

@gxthrj @tao12345666333 need a review

I think the e2e failure has nothing to do with this pr

tao12345666333 · 2021-12-07T07:12:56Z

re-run all jobs.

lxm · 2021-12-07T08:34:45Z

re-run all jobs.

E2E tests failed with "check the ingress lb status is updated" this time, maybe something wrong with test cases. I can pass this case with single run in a private environment.

lxm · 2021-12-07T11:53:13Z

ping @tao12345666333 @gxthrj @lingsamuel

tao12345666333

LGTM

tao12345666333 · 2021-12-08T01:52:33Z

Thd code LGTM

I added the label of do-not-merge/hold to it because this feature is not planned to be in the v1.4 version.

Do you have a strong demand for this feature? @lxm

lxm · 2021-12-08T02:10:57Z

Thd code LGTM

I added the label of do-not-merge/hold to it because this feature is not planned to be in the v1.4 version.

Do you have a strong demand for this feature? @lxm

right, I want to use apisix-ingress-controller to replace the nginx-ingress-controller

without this feature, this can not be done.

tao12345666333 · 2021-12-08T03:10:40Z

ok I understand your situation, I have added it to v1.4.

lxm force-pushed the feature/path-expr branch from 69c1764 to 978b344 Compare November 30, 2021 12:50

tao12345666333 requested review from tokers, lingsamuel and gxthrj November 30, 2021 23:39

tao12345666333 added the enhancement New feature or request label Nov 30, 2021

tao12345666333 self-requested a review November 30, 2021 23:46

tokers reviewed Dec 1, 2021

View reviewed changes

pkg/kube/translation/annotations/types.go Show resolved Hide resolved

pkg/kube/translation/ingress.go Outdated Show resolved Hide resolved

pkg/kube/translation/ingress.go Outdated Show resolved Hide resolved

pkg/kube/translation/ingress.go Outdated Show resolved Hide resolved

lxm force-pushed the feature/path-expr branch 2 times, most recently from ebd0747 to 6e7f685 Compare December 1, 2021 09:41

lxm requested a review from tokers December 1, 2021 12:41

lxm force-pushed the feature/path-expr branch 10 times, most recently from 1d0e27c to 33dd6c4 Compare December 3, 2021 12:56

lxm force-pushed the feature/path-expr branch from 33dd6c4 to a01a4fd Compare December 3, 2021 14:40

tao12345666333 added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Dec 4, 2021

tao12345666333 linked an issue Dec 4, 2021 that may be closed by this pull request

request help: path like /api/*/action #768

Closed

tokers reviewed Dec 5, 2021

View reviewed changes

pkg/kube/translation/ingress.go Show resolved Hide resolved

lxm mentioned this pull request Dec 6, 2021

bug: need vailidation for ingress resources #786

Closed

gxthrj reviewed Dec 6, 2021

View reviewed changes

lxm force-pushed the feature/path-expr branch 2 times, most recently from fbadc91 to 1faf7cf Compare December 6, 2021 10:03

tokers approved these changes Dec 6, 2021

View reviewed changes

lxm force-pushed the feature/path-expr branch from 1faf7cf to 973d745 Compare December 6, 2021 10:13

lxm requested a review from gxthrj December 6, 2021 10:13

lxm force-pushed the feature/path-expr branch from 973d745 to f0f5775 Compare December 6, 2021 10:40

feat: support regex in path

0742306

Signed-off-by: lxm <lxm.xupt@gmail.com>

lxm force-pushed the feature/path-expr branch from f0f5775 to 0742306 Compare December 6, 2021 11:12

tao12345666333 added the do-not-merge/hold label Dec 8, 2021

tao12345666333 approved these changes Dec 8, 2021

View reviewed changes

tao12345666333 added changelog Issues with this label should be added to changelog when public a new release and removed do-not-merge/hold labels Dec 8, 2021

tao12345666333 added this to In progress in v1.4 Planning via automation Dec 8, 2021

tao12345666333 added this to the 1.4.0 milestone Dec 8, 2021

tao12345666333 merged commit 4e84eb8 into apache:master Dec 8, 2021

v1.4 Planning automation moved this from In progress to Done Dec 8, 2021

Sindweller pushed a commit to Sindweller/apisix-ingress-controller that referenced this pull request Dec 18, 2021

feat: support regex in path (apache#779)

246013b

tao12345666333 pushed a commit to tao12345666333/ingress-controller that referenced this pull request Dec 23, 2021

feat: support regex in path (apache#779)

1072690

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support regex in path #779

feat: support regex in path #779

lxm commented Nov 30, 2021 •

edited by tao12345666333

Loading

tokers left a comment

lxm commented Dec 1, 2021

tokers commented Dec 2, 2021

lxm commented Dec 2, 2021

codecov-commenter commented Dec 2, 2021 •

edited

Loading

lxm commented Dec 3, 2021

tao12345666333 commented Dec 4, 2021

tao12345666333 commented Dec 4, 2021

lxm commented Dec 5, 2021

gxthrj Dec 6, 2021

lxm Dec 6, 2021

lxm commented Dec 6, 2021

lxm commented Dec 7, 2021

tao12345666333 commented Dec 7, 2021

lxm commented Dec 7, 2021 •

edited

Loading

lxm commented Dec 7, 2021

tao12345666333 left a comment

tao12345666333 commented Dec 8, 2021

lxm commented Dec 8, 2021

tao12345666333 commented Dec 8, 2021

feat: support regex in path #779

feat: support regex in path #779

Conversation

lxm commented Nov 30, 2021 • edited by tao12345666333 Loading

New feature or improvement

tokers left a comment

Choose a reason for hiding this comment

lxm commented Dec 1, 2021

tokers commented Dec 2, 2021

lxm commented Dec 2, 2021

codecov-commenter commented Dec 2, 2021 • edited Loading

Codecov Report

lxm commented Dec 3, 2021

tao12345666333 commented Dec 4, 2021

tao12345666333 commented Dec 4, 2021

lxm commented Dec 5, 2021

gxthrj Dec 6, 2021

Choose a reason for hiding this comment

lxm Dec 6, 2021

Choose a reason for hiding this comment

lxm commented Dec 6, 2021

lxm commented Dec 7, 2021

tao12345666333 commented Dec 7, 2021

lxm commented Dec 7, 2021 • edited Loading

lxm commented Dec 7, 2021

tao12345666333 left a comment

Choose a reason for hiding this comment

tao12345666333 commented Dec 8, 2021

lxm commented Dec 8, 2021

tao12345666333 commented Dec 8, 2021

lxm commented Nov 30, 2021 •

edited by tao12345666333

Loading

codecov-commenter commented Dec 2, 2021 •

edited

Loading

lxm commented Dec 7, 2021 •

edited

Loading