You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/05/30 08:23:26 UTC
[GitHub] [apisix-ingress-controller] stillfox-lee opened a new pull request, #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
stillfox-lee opened a new pull request, #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056
<!-- Please answer these questions before submitting a pull request -->
### Type of change:
<!-- Please delete options that are not relevant. -->
- [ ] Bugfix
- [x] New feature provided
- [ ] Improve performance
- [ ] Backport patches
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
Add mqtt-proxy plugin in ApisixRoute.Stream.
### Pre-submission checklist:
<!--
Please follow the requirements:
1. Use Draft if the PR is not ready to be reviewed
2. Test is required for the feat/fix PR, unless you have a good reason
3. Doc is required for the feat PR
4. Use a new commit to resolve review instead of `push -f`
5. Use "request review" to notify the reviewer once you have resolved the review
-->
* [ ] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] 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](https://github.com/apache/apisix-ingress-controller#community) first**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tokers commented on a diff in pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tokers commented on code in PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#discussion_r887414862
##########
samples/deploy/crd/v1/ApisixRoute.yaml:
##########
@@ -803,6 +803,22 @@ spec:
required:
- serviceName
- servicePort
+ plugins:
+ type: array
+ items:
+ type: object
+ properties:
+ name:
+ type: string
+ minLength: 1
+ enable:
+ type: boolean
+ config:
+ type: object
+ x-kubernetes-preserve-unknown-fields: true # we have to enable it since plugin config
Review Comment:
Incomplete comment?
##########
samples/deploy/crd/v1/ApisixRoute.yaml:
##########
@@ -803,6 +803,22 @@ spec:
required:
- serviceName
- servicePort
+ plugins:
+ type: array
+ items:
+ type: object
+ properties:
+ name:
+ type: string
+ minLength: 1
+ enable:
+ type: boolean
+ config:
+ type: object
+ x-kubernetes-preserve-unknown-fields: true # we have to enable it since plugin config
Review Comment:
Also, it looks like we don't add this field for the `plugins` of HTTP route.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] stillfox-lee commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
stillfox-lee commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1192693574
> @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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] AlinsRan commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
AlinsRan commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1189694529
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1217672245
I re-runed it and the problem is gone
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1144394317
> So may be we should update the [doc](https://apisix.apache.org/zh/docs/ingress-controller/references/apisix_route_v2beta3) 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] stillfox-lee commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
stillfox-lee commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1144506166
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1186797563
@stillfox-lee As I said in my previous comment https://github.com/apache/apisix-ingress-controller/issues/831#issuecomment-1148789741, 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] stillfox-lee commented on a diff in pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
stillfox-lee commented on code in PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#discussion_r936725837
##########
test/e2e/suite-plugins/suite-plugins-other/mqtt-proxy.go:
##########
@@ -0,0 +1,122 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package plugins
+
+import (
+ "time"
+
+ ginkgo "github.com/onsi/ginkgo/v2"
+ "github.com/stretchr/testify/assert"
+
+ "github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
+)
+
+var _ = ginkgo.FDescribe("suite-plugins: mqtt-proxy plugin", func() {
Review Comment:
Oops, My fault.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1169930496
Let me take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] stillfox-lee commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
stillfox-lee commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1142238732
@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
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] stillfox-lee commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
stillfox-lee commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1144386957
@tao12345666333 This PR also adds a feature for ApisixRoute Stream plugins. So may be we should update the [doc](https://apisix.apache.org/zh/docs/ingress-controller/references/apisix_route_v2beta3) about how to use stream plugin?
But the doc just for v2beta3 only, in [origin issue](https://github.com/apache/apisix-ingress-controller/issues/966#issuecomment-1131556353), we dedciede add this feature to v2 APIversion only. Should I add this feature to other APIversion great than v2?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#discussion_r887514851
##########
samples/deploy/crd/v1/ApisixRoute.yaml:
##########
@@ -803,6 +803,22 @@ spec:
required:
- serviceName
- servicePort
+ plugins:
+ type: array
+ items:
+ type: object
+ properties:
+ name:
+ type: string
+ minLength: 1
+ enable:
+ type: boolean
+ config:
+ type: object
+ x-kubernetes-preserve-unknown-fields: true # we have to enable it since plugin config
Review Comment:
We can improve comments in subsequent PRs
But for CRD configuration, they are the same, direct copy-paste is ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1196512780
@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 https://github.com/apache/apisix-ingress-controller/issues/707 for more detail (I have explained before)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1216145083
hi, there has one conflicting file, do you have time to resolve it? thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] stillfox-lee commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
stillfox-lee commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1202727651
@tao12345666333 Thanks for your help. I think PR is ready for review now. Sorry for the delay, and this feature didn't catch v1.5 window.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] AlinsRan commented on a diff in pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
AlinsRan commented on code in PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#discussion_r936440607
##########
test/e2e/suite-plugins/suite-plugins-other/mqtt-proxy.go:
##########
@@ -0,0 +1,122 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package plugins
+
+import (
+ "time"
+
+ ginkgo "github.com/onsi/ginkgo/v2"
+ "github.com/stretchr/testify/assert"
+
+ "github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
+)
+
+var _ = ginkgo.FDescribe("suite-plugins: mqtt-proxy plugin", func() {
Review Comment:
`suite-plugins-other`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] stillfox-lee commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
stillfox-lee commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1169719323
@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!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] stillfox-lee commented on a diff in pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
stillfox-lee commented on code in PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#discussion_r887453538
##########
samples/deploy/crd/v1/ApisixRoute.yaml:
##########
@@ -803,6 +803,22 @@ spec:
required:
- serviceName
- servicePort
+ plugins:
+ type: array
+ items:
+ type: object
+ properties:
+ name:
+ type: string
+ minLength: 1
+ enable:
+ type: boolean
+ config:
+ type: object
+ x-kubernetes-preserve-unknown-fields: true # we have to enable it since plugin config
Review Comment:
> We can keep it consistent for now
Any suggestion to make it better? I also think copy and paste are ugly. But I don't know how to improve it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] codecov-commenter commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1141317176
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1056?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1056](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1056?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b7a0670) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/df7a724ce11d23ad441209cf2592426f251f597c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (df7a724) will **decrease** coverage by `0.03%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #1056 +/- ##
==========================================
- Coverage 30.97% 30.93% -0.04%
==========================================
Files 75 75
Lines 8792 8801 +9
==========================================
Hits 2723 2723
- Misses 5775 5784 +9
Partials 294 294
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1056?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/kube/translation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1056/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2t1YmUvdHJhbnNsYXRpb24vYXBpc2l4X3JvdXRlLmdv) | `20.31% <0.00%> (-0.30%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1056?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1056?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [df7a724...b7a0670](https://codecov.io/gh/apache/apisix-ingress-controller/pull/1056?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#discussion_r950506354
##########
pkg/kube/apisix/apis/config/v2/types.go:
##########
@@ -236,6 +237,34 @@ type ApisixRouteStreamBackend struct {
Subset string `json:"subset,omitempty" yaml:"subset,omitempty"`
}
+// ApisixRouteStreamPlugin represents an APISIX strem plugin.
+type ApisixRouteStreamPlugin struct {
+ // The plugin name.
+ Name string `json:"name" yaml:"name"`
+ // Whether this plugin is in use, default is true.
+ Enable bool `json:"enable" yaml:"enable"`
+ // Plugin configuration.
+ Config ApisixRouteStreamPluginConfig `json:"config" yaml:"config"`
+}
+
+// ApisixRouteStreamPluginConfig is the configuration for
+// any stream plugins.
+type ApisixRouteStreamPluginConfig map[string]interface{}
+
+func (p ApisixRouteStreamPluginConfig) DeepCopyInto(out *ApisixRouteStreamPluginConfig) {
+ b, _ := json.Marshal(&p)
+ _ = json.Unmarshal(b, out)
+}
+
+func (p *ApisixRouteStreamPluginConfig) DeepCopy() *ApisixRouteStreamPluginConfig {
+ if p == nil {
+ return nil
+ }
+ out := new(ApisixRouteStreamPluginConfig)
+ p.DeepCopyInto(out)
+ return out
+}
+
Review Comment:
I don't think it's necessary to introduce a new structure.
In stream and http, the structure of the plugin is the same, and it will be the same in the future.
We can easily rename `ApisixRouteHTTPPlugin` to `ApisixRoutePlugin`, what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] stillfox-lee commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
stillfox-lee commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1216815674
@tao12345666333 The [issue](https://github.com/apache/apisix-ingress-controller/issues/1249) cause CI failed. It's not related to this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 merged pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 merged PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] stillfox-lee commented on a diff in pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
stillfox-lee commented on code in PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#discussion_r887443915
##########
samples/deploy/crd/v1/ApisixRoute.yaml:
##########
@@ -803,6 +803,22 @@ spec:
required:
- serviceName
- servicePort
+ plugins:
+ type: array
+ items:
+ type: object
+ properties:
+ name:
+ type: string
+ minLength: 1
+ enable:
+ type: boolean
+ config:
+ type: object
+ x-kubernetes-preserve-unknown-fields: true # we have to enable it since plugin config
Review Comment:
Thanks for your review! I just copy this field from HTTP route: https://github.com/apache/apisix-ingress-controller/blob/deb044039e889e88310523f369833bb137c0e145/samples/deploy/crd/v1/ApisixRoute.yaml#L733-L745
Three are three `plugin` field defiend for HTTP route in this yaml before my change. So, may be keep the same for Stream route?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1204337410
the failed job due to https://github.com/apache/apisix-ingress-controller/issues/1199
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1196514538
I will add a commit to help you fix CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1196539445
https://github.com/stillfox-lee/apisix-ingress-controller/pull/13/commits/ed5bd38d46da327d6f535f96893b6787694672ac this one
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on a diff in pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on code in PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#discussion_r887447120
##########
samples/deploy/crd/v1/ApisixRoute.yaml:
##########
@@ -803,6 +803,22 @@ spec:
required:
- serviceName
- servicePort
+ plugins:
+ type: array
+ items:
+ type: object
+ properties:
+ name:
+ type: string
+ minLength: 1
+ enable:
+ type: boolean
+ config:
+ type: object
+ x-kubernetes-preserve-unknown-fields: true # we have to enable it since plugin config
Review Comment:
We can keep it consistent for now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1144398619
I have created one issue for track https://github.com/apache/apisix-ingress-controller/issues/1063
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1143447068
Could you please merge master latest code? #1055 has been merged. We can make test cases pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1141318305
Great! Thanks!
I will reivew this later. I'm working on an issue #1055 (high priority)
It blocked our CI jobs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1056: feat: add mqtt-proxy plugin in ApisixRoute (#966)
Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on PR #1056:
URL: https://github.com/apache/apisix-ingress-controller/pull/1056#issuecomment-1205958383
please merge master branch latest code to fix CI job errors. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org