feat: add mqtt-proxy plugin in ApisixRoute (#966) #1056

stillfox-lee · 2022-05-30T08:23:24Z

Type of change:

Bugfix
New feature provided
Improve performance
Backport patches

What this PR does / why we need it:

Add mqtt-proxy plugin in ApisixRoute.Stream.

ref: issue

Pre-submission checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

codecov-commenter · 2022-05-30T16:11:28Z

Codecov Report

Merging #1056 (cd9ee1a) into master (356b220) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1056      +/-   ##
==========================================
- Coverage   42.78%   42.72%   -0.06%     
==========================================
  Files          73       73              
  Lines        6477     6486       +9     
==========================================
  Hits         2771     2771              
- Misses       3409     3418       +9     
  Partials      297      297

Impacted Files	Coverage Δ
pkg/providers/apisix/translation/apisix_route.go	`19.14% <0.00%> (-0.22%)`	⬇️

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

tao12345666333 · 2022-05-30T16:12:45Z

Great! Thanks!

I will reivew this later. I'm working on an issue #1055 (high priority)
It blocked our CI jobs.

stillfox-lee · 2022-05-31T14:50:11Z

@tao12345666333
Hi, I saw you fix wolf-rbac test case issue. But I still can't pass e2e-test although I merged master branch. I'm not sure if three is something wrong with my env or wolf-rbac test case?
Here is the error message:

• Failure [111.542 seconds]
suite-features: ApisixConsumer
/Users/liaosiyi/Developer/github/stillfox-lee/apisix/apisix-ingress-controller/test/e2e/suite-features/consumer.go:30
  ApisixRoute with wolfRBAC consumer using secret [It]
  /Users/liaosiyi/Developer/github/stillfox-lee/apisix/apisix-ingress-controller/test/e2e/suite-features/consumer.go:511


  	Error Trace:	reporter.go:23
  	            				reporter.go:23
  	            				chain.go:21
  	            				response.go:585
  	            				response.go:151
  	            				consumer.go:614
  	            				runner.go:113
  	            				runner.go:64
  	            				it_node.go:26
  	            				spec.go:215
  	            				spec.go:138
  	            				spec_runner.go:200
  	            				spec_runner.go:170
  	            				spec_runner.go:66
  	            				suite.go:79
  	            				ginkgo_dsl.go:238
  	            				ginkgo_dsl.go:213
  	            				e2e_test.go:26
  	Error:
  	            		Error Trace:	reporter.go:23
  	            		            				chain.go:21
  	            		            				response.go:585
  	            		            				response.go:151
  	            		            				consumer.go:614
  	            		            				runner.go:113
  	            		            				runner.go:64
  	            		            				it_node.go:26
  	            		            				spec.go:215
  	            		            				spec.go:138
  	            		            				spec_runner.go:200
  	            		            				spec_runner.go:170
  	            		            				spec_runner.go:66
  	            		            				suite.go:79
  	            		            				ginkgo_dsl.go:238
  	            		            				ginkgo_dsl.go:213
  	            		            				e2e_test.go:26
  	            		Error:
  	            		            	expected status equal to:
  	            		            	 "200 OK"

  	            		            	but got:
  	            		            	 "500 Internal Server Error"
  	Test:       	suite-features: ApisixConsumer ApisixRoute with wolfRBAC consumer using secret

The second error looks like a string format issue. It got an unexpected '\n'. But I have no idea how to fix it.

------------------------------
• Failure [62.149 seconds]
suite-features: ApisixConsumer
/Users/liaosiyi/Developer/github/stillfox-lee/apisix/apisix-ingress-controller/test/e2e/suite-features/consumer.go:30
  ApisixRoute with wolfRBAC consumer [It]
  /Users/liaosiyi/Developer/github/stillfox-lee/apisix/apisix-ingress-controller/test/e2e/suite-features/consumer.go:393


  	Error Trace:	consumer.go:422
  	            				runner.go:113
  	            				runner.go:64
  	            				it_node.go:26
  	            				spec.go:215
  	            				spec.go:138
  	            				spec_runner.go:200
  	            				spec_runner.go:170
  	            				spec_runner.go:66
  	            				suite.go:79
  	            				ginkgo_dsl.go:238
  	            				ginkgo_dsl.go:213
  	            				e2e_test.go:26
  	Error:      	Not equal:
  	            	expected: map[string]interface {}{"appid":"test-app", "header_prefix":"X-", "server":"http://-n 172.25.0.1 :12180"}
  	            	actual  : map[string]interface {}{"appid":"test-app", "header_prefix":"X-", "server":"http://-n 172.25.0.1\n:12180"}

  	            	Diff:
  	            	--- Expected
  	            	+++ Actual
  	            	@@ -3,3 +3,3 @@
  	            	  (string) (len=13) "header_prefix": (string) (len=2) "X-",
  	            	- (string) (len=6) "server": (string) (len=27) "http://-n 172.25.0.1 :12180"
  	            	+ (string) (len=6) "server": (string) (len=27) "http://-n 172.25.0.1\n:12180"
  	            	 }
  	Test:       	suite-features: ApisixConsumer ApisixRoute with wolfRBAC consumer

tao12345666333 · 2022-06-01T10:53:13Z

Could you please merge master latest code? #1055 has been merged. We can make test cases pass.

samples/deploy/crd/v1/ApisixRoute.yaml

stillfox-lee · 2022-06-02T03:10:20Z

@tao12345666333 This PR also adds a feature for ApisixRoute Stream plugins. So may be we should update the doc about how to use stream plugin?
But the doc just for v2beta3 only, in origin issue, we dedciede add this feature to v2 APIversion only. Should I add this feature to other APIversion great than v2?

tao12345666333 · 2022-06-02T03:26:17Z

So may be we should update the doc about how to use stream plugin?

we should add apisix_route_v2 reference doc. And add stream plugin content.

Should I add this feature to other APIversion great than v2?

No, just keep this feature in v2 APIVersion. We will mark v2beta3 as deprecated in the future. You can check #707 for more details

tao12345666333 · 2022-06-02T03:34:47Z

I have created one issue for track #1063

stillfox-lee · 2022-06-02T06:53:10Z

I have created one issue for track #1063

I can work on it after this PR is merged.
For now, could you please check CI test? It seems like CI test env has some problems. These errors do not occur when running the e2e-test on my computer.

stillfox-lee · 2022-06-29T09:02:37Z

@tao12345666333 Hi. These errors do not occur when running e2e-test on my computer. It looks like e2e-test may have some random fail problem?
About the spell checker fails. I use docker image named eclipse-mosquitto:1.6 in e2e-test. But linter need use mosquito to replace mosquitto. I'm afraid the image name can't be changed.
When you have time can you help me look at this PR? Thanks!

tao12345666333 · 2022-06-29T12:40:03Z

Let me take a look.

tao12345666333 · 2022-07-18T05:59:12Z

@stillfox-lee As I said in my previous comment #831 (comment), we are addressing #954
The task is currently over, I can have time to advance this PR.

Can you merge the latest code and resolve conflicts? I think the previous e2e problem should have been solved.

AlinsRan · 2022-07-20T01:05:30Z

@stillfox-lee Please replace suite-plugins/mqtt-proxy.go with suite-plugins/suite-plugins-other/mqtt-proxy.go. Because the secondary directory will not be started when there is a tertiary directory.

stillfox-lee · 2022-07-22T15:33:09Z

@stillfox-lee Please replace suite-plugins/mqtt-proxy.go with suite-plugins/suite-plugins-other/mqtt-proxy.go. Because the secondary directory will not be started when there is a tertiary directory.

Yes, I found that too. And test code needs to adapt V2 e2e-test, so I converted this PR to Draft. Maybe I can do some refactor work when I have time this weekend.

tao12345666333 · 2022-07-27T09:50:31Z

@stillfox-lee hi, thanks for your contribution, we plan to enter v1.5 release window.

I have a few suggestions for this PR

I would like to put this PR into v1.6
Currently we have unified all resources to the v2 version of the API, I recommend implementing this feature only for the v2 version of ApisixRoute. Because when the v1.5 version is released, we will mark the v2beta3 version as deprecated. You can check proposal: use a uniform CRD APIVersion #707 for more detail (I have explained before)

tao12345666333 · 2022-07-27T09:52:16Z

I will add a commit to help you fix CI

AlinsRan · 2022-08-03T09:24:51Z

test/e2e/suite-plugins/suite-plugins-other/mqtt-proxy.go

+	"github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
+)
+
+var _ = ginkgo.FDescribe("suite-plugins: mqtt-proxy plugin", func() {


suite-plugins-other

Oops, My fault.

tao12345666333 · 2022-08-03T18:38:17Z

the failed job due to #1199

tao12345666333 · 2022-08-05T02:05:01Z

please merge master branch latest code to fix CI job errors. Thanks

tao12345666333 · 2022-08-16T04:54:42Z

hi, there has one conflicting file, do you have time to resolve it? thanks

stillfox-lee · 2022-08-16T15:43:17Z

@tao12345666333 The issue cause CI failed. It's not related to this PR.

tao12345666333 · 2022-08-17T08:22:23Z

I re-runed it and the problem is gone

pkg/kube/apisix/apis/config/v2/types.go

tao12345666333 requested review from tao12345666333, lingsamuel, tokers, gxthrj and nic-6443 May 30, 2022 16:09

tao12345666333 added enhancement New feature or request area/controller labels May 30, 2022

tao12345666333 self-assigned this Jun 1, 2022

stillfox-lee force-pushed the master branch from d6e0776 to 4de4af9 Compare June 1, 2022 16:06

tokers reviewed Jun 2, 2022

View reviewed changes

samples/deploy/crd/v1/ApisixRoute.yaml Show resolved Hide resolved

tao12345666333 mentioned this pull request Jun 7, 2022

discuss: New feature in Apache APISIX Ingress controller v1.5 #831

Closed

2 tasks

stillfox-lee force-pushed the master branch from ffa34ab to 84beaa9 Compare June 29, 2022 06:50

stillfox-lee force-pushed the master branch 2 times, most recently from 2db627a to 516df9b Compare July 18, 2022 15:08

stillfox-lee marked this pull request as draft July 19, 2022 03:26

AlinsRan requested changes Aug 3, 2022

View reviewed changes

stillfox-lee added 10 commits August 5, 2022 23:40

feat: add mqtt-proxy plugin in ApisixRoute (apache#966)

624fe52

fix: mqtt e2e-test use Describe instead FDescribe

e31af21

fix e2e-test spell mistake and code-gen

a4565a5

fix: mqtt-proxy e2e-test use correct suite name

3c82eaf

resolve conflict

7e13002

doc: add route steam plugin field

58d7697

test: adapt v2 e2e-test

9ba6cef

ci: fix spell check

073004b

ci: make markdown linter happy

480bd80

test: fix spec name

f110477

stillfox-lee force-pushed the master branch from 1823644 to f110477 Compare August 5, 2022 15:42

Merge remote-tracking branch 'upstream/master'

62444a5

tao12345666333 mentioned this pull request Aug 17, 2022

doc: markdown dead link #1249

Closed

tao12345666333 added this to the v1.6.0 milestone Aug 17, 2022

lingsamuel approved these changes Aug 17, 2022

View reviewed changes

tao12345666333 linked an issue Aug 19, 2022 that may be closed by this pull request

feat request: native MQTT Protocol proxy #966

Closed

tao12345666333 reviewed Aug 19, 2022

View reviewed changes

pkg/kube/apisix/apis/config/v2/types.go Outdated Show resolved Hide resolved

refector: ApisixRoutePlugin represent route plugin

cd9ee1a

tao12345666333 approved these changes Aug 21, 2022

View reviewed changes

tao12345666333 merged commit 530ce52 into apache:master Aug 21, 2022

tao12345666333 added the changelog Issues with this label should be added to changelog when public a new release label Aug 21, 2022

tao12345666333 mentioned this pull request Aug 21, 2022

feat: support stream route plugin #918

Closed

4 tasks

tao12345666333 linked an issue Aug 21, 2022 that may be closed by this pull request

Feature suggestion: support stream route with plugins #744

Closed

tao12345666333 mentioned this pull request Aug 21, 2022

Feature suggestion: support stream route with plugins #744

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add mqtt-proxy plugin in ApisixRoute (#966) #1056

feat: add mqtt-proxy plugin in ApisixRoute (#966) #1056

stillfox-lee commented May 30, 2022 •

edited

Loading

codecov-commenter commented May 30, 2022 •

edited

Loading

tao12345666333 commented May 30, 2022

stillfox-lee commented May 31, 2022

tao12345666333 commented Jun 1, 2022

stillfox-lee commented Jun 2, 2022

tao12345666333 commented Jun 2, 2022

tao12345666333 commented Jun 2, 2022

stillfox-lee commented Jun 2, 2022

stillfox-lee commented Jun 29, 2022

tao12345666333 commented Jun 29, 2022

tao12345666333 commented Jul 18, 2022

AlinsRan commented Jul 20, 2022

stillfox-lee commented Jul 22, 2022

tao12345666333 commented Jul 27, 2022

tao12345666333 commented Jul 27, 2022

AlinsRan Aug 3, 2022

stillfox-lee Aug 3, 2022

tao12345666333 commented Aug 3, 2022

tao12345666333 commented Aug 5, 2022

tao12345666333 commented Aug 16, 2022

stillfox-lee commented Aug 16, 2022

tao12345666333 commented Aug 17, 2022

feat: add mqtt-proxy plugin in ApisixRoute (#966) #1056

feat: add mqtt-proxy plugin in ApisixRoute (#966) #1056

Conversation

stillfox-lee commented May 30, 2022 • edited Loading

Type of change:

What this PR does / why we need it:

Pre-submission checklist:

codecov-commenter commented May 30, 2022 • edited Loading

Codecov Report

tao12345666333 commented May 30, 2022

stillfox-lee commented May 31, 2022

tao12345666333 commented Jun 1, 2022

stillfox-lee commented Jun 2, 2022

tao12345666333 commented Jun 2, 2022

tao12345666333 commented Jun 2, 2022

stillfox-lee commented Jun 2, 2022

stillfox-lee commented Jun 29, 2022

tao12345666333 commented Jun 29, 2022

tao12345666333 commented Jul 18, 2022

AlinsRan commented Jul 20, 2022

stillfox-lee commented Jul 22, 2022

tao12345666333 commented Jul 27, 2022

tao12345666333 commented Jul 27, 2022

AlinsRan Aug 3, 2022

Choose a reason for hiding this comment

stillfox-lee Aug 3, 2022

Choose a reason for hiding this comment

tao12345666333 commented Aug 3, 2022

tao12345666333 commented Aug 5, 2022

tao12345666333 commented Aug 16, 2022

stillfox-lee commented Aug 16, 2022

tao12345666333 commented Aug 17, 2022

stillfox-lee commented May 30, 2022 •

edited

Loading

codecov-commenter commented May 30, 2022 •

edited

Loading