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 2021/07/06 01:28:01 UTC
[GitHub] [apisix-ingress-controller] fgksgf opened a new pull request #573: [WIP] Implement the admission server
fgksgf opened a new pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573
Please answer these questions before submitting a pull request
- Why submit this pull request?
- [ ] Bugfix
- [x] New feature provided
- [ ] Improve performance
- [ ] Backport patches
- Related issues
#244
___
### New feature or improvement
- Describe the details and related test reports.
Implement an admission server via [kubewebhook](https://github.com/slok/kubewebhook) to validate configurations like plugins.
#### TODO
- [x] implement an admission server
- [ ] add YAML of webhook
- [ ] add e2e test
--
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] fgksgf commented on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-908356237
--
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] fgksgf commented on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-908092120
> reply: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698118062
If we provide default TLS Certificates, they are not signed by CA, and the webhook still can't work. We can accomplish this after we have cert-manager.
I can help integrate cert-manager after webhooks are implemented.
--
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] fgksgf commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r693595386
##########
File path: Makefile
##########
@@ -106,6 +106,13 @@ ifeq ($(E2E_SKIP_BUILD), 0)
docker push $(LOCAL_REGISTRY)/jmalloc/echo-server:latest
endif
+# rebuild ingress controller image and push to the local registry for e2e tests.
Review comment:
Because when I'm debugging the e2e, I only modify the ingress controller code. So I want to only rebuild and push this image to re-run e2e tests for faster. I will remove this.
--
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 change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698495449
##########
File path: pkg/api/server.go
##########
@@ -67,13 +99,51 @@ func NewServer(cfg *config.Config) (*Server, error) {
func (srv *Server) Run(stopCh <-chan struct{}) error {
go func() {
<-stopCh
- if err := srv.httpListener.Close(); err != nil {
- log.Errorf("failed to close http listener: %s", err)
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+
+ closeHttp := make(chan struct{})
+ go srv.closeHttpServer(closeHttp)
Review comment:
> should I put the timeout into the config?
Don't have to. And I think the code is correct.
--
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 edited a comment on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-874397196
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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 [#573](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (240ffae) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/62b7590443e037ecd6b41521accea567e09ad340?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (62b7590) will **increase** coverage by `0.40%`.
> The diff coverage is `50.64%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
+ Coverage 34.31% 34.72% +0.40%
==========================================
Files 57 60 +3
Lines 5746 5892 +146
==========================================
+ Hits 1972 2046 +74
- Misses 3527 3596 +69
- Partials 247 250 +3
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/api/router/router.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvcm91dGVyLmdv) | `75.00% <ø> (ø)` | |
| [pkg/api/validation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL2FwaXNpeF9yb3V0ZS5nbw==) | `21.53% <21.53%> (ø)` | |
| [pkg/api/validation/utils.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL3V0aWxzLmdv) | `46.15% <46.15%> (ø)` | |
| [pkg/api/server.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9zZXJ2ZXIuZ28=) | `81.81% <79.31%> (+5.95%)` | :arrow_up: |
| [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.27% <100.00%> (+0.26%)` | :arrow_up: |
| [pkg/api/router/webhook.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvd2ViaG9vay5nbw==) | `100.00% <100.00%> (ø)` | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `86.44% <100.00%> (+0.72%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/573?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 [62b7590...240ffae](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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] gxthrj commented on a change in pull request #573: [WIP] Implement the admission server
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r689002617
##########
File path: pkg/api/validation/apisix_route.go
##########
@@ -0,0 +1,140 @@
+// 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 validation
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "net/http"
+ "strings"
+
+ kwhhttp "github.com/slok/kubewebhook/v2/pkg/http"
+ kwhmodel "github.com/slok/kubewebhook/v2/pkg/model"
+ kwhvalidating "github.com/slok/kubewebhook/v2/pkg/webhook/validating"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
+ v1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
+ "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2alpha1"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
+)
+
+func NewPluginValidatorHandler() http.Handler {
+ // Create a validating webhook.
+ wh, err := kwhvalidating.NewWebhook(kwhvalidating.WebhookConfig{
+ ID: "apisixRoute-plugin",
+ Validator: pluginValidator,
+ })
+ if err != nil {
+ log.Errorf("failed to create webhook: %s", err)
+ }
+
+ h, err := kwhhttp.HandlerFor(kwhhttp.HandlerConfig{Webhook: wh})
+ if err != nil {
+ log.Errorf("failed to create webhook handle: %s", err)
+ }
+
+ return h
+}
+
+// ErrNotApisixRoute will be used when the validating object is not ApisixRoute.
+var ErrNotApisixRoute = errors.New("object is not ApisixRoute")
+
+type apisixRoutePlugin struct {
+ Name string
+ Config interface{}
+}
+
+// pluginValidator validates plugins in ApisixRoute.
+// When the validation of one plugin fails, it will continue to validate the rest of plugins.
+var pluginValidator = kwhvalidating.ValidatorFunc(
+ func(ctx context.Context, review *kwhmodel.AdmissionReview, object metav1.Object) (result *kwhvalidating.ValidatorResult, err error) {
+ valid := true
+
+ var plugins []apisixRoutePlugin
+
+ switch ar := object.(type) {
+ case *v2alpha1.ApisixRoute:
+ for _, h := range ar.Spec.HTTP {
+ for _, p := range h.Plugins {
+ // only check plugins that are enabled.
+ if p.Enable {
+ plugins = append(plugins, apisixRoutePlugin{
+ p.Name, p.Config,
+ })
+ }
+ }
+ }
+ case *v1.ApisixRoute:
+ for _, r := range ar.Spec.Rules {
+ for _, path := range r.Http.Paths {
+ for _, p := range path.Plugins {
+ if p.Enable {
+ plugins = append(plugins, apisixRoutePlugin{
+ p.Name, p.Config,
+ })
+ }
+ }
+ }
+ }
+ default:
Review comment:
Need to support v2beta1 version of ApisixRoute resource
##########
File path: pkg/api/router/webhook.go
##########
@@ -0,0 +1,29 @@
+// 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 router
+
+import (
+ "github.com/gin-gonic/gin"
+
+ "github.com/apache/apisix-ingress-controller/pkg/api/validation"
+)
+
+func mountWebhook(r *gin.Engine) {
+ validating := r.Group("/validating")
Review comment:
Better to use /validation instead？
--
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 edited a comment on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-874397196
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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 [#573](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f4c086e) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/62b7590443e037ecd6b41521accea567e09ad340?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (62b7590) will **increase** coverage by `0.35%`.
> The diff coverage is `49.00%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
+ Coverage 34.31% 34.66% +0.35%
==========================================
Files 57 60 +3
Lines 5746 5887 +141
==========================================
+ Hits 1972 2041 +69
- Misses 3527 3596 +69
- Partials 247 250 +3
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/api/router/router.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvcm91dGVyLmdv) | `75.00% <ø> (ø)` | |
| [pkg/api/validation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL2FwaXNpeF9yb3V0ZS5nbw==) | `21.53% <21.53%> (ø)` | |
| [pkg/api/validation/utils.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL3V0aWxzLmdv) | `46.15% <46.15%> (ø)` | |
| [pkg/api/server.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9zZXJ2ZXIuZ28=) | `80.55% <77.35%> (+4.69%)` | :arrow_up: |
| [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.27% <100.00%> (+0.26%)` | :arrow_up: |
| [pkg/api/router/webhook.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvd2ViaG9vay5nbw==) | `100.00% <100.00%> (ø)` | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `86.44% <100.00%> (+0.72%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/573?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 [62b7590...f4c086e](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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] fgksgf commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r695759347
##########
File path: test/e2e/scaffold/ssl.go
##########
@@ -29,8 +29,8 @@ kind: Secret
metadata:
name: %s
data:
- cert: %s
- key: %s
+ cert.pem: %s
+ key.pem: %s
Review comment:
I modified this for webhook e2e case, not sure if it will break other e2e test cases.
--
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 pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-903372768
@fgksgf After this PR was approved, we need to change helm chart so that people can use 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] fgksgf edited a comment on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf edited a comment on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-908365726
Webhook E2E test:
--
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 edited a comment on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-874397196
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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 [#573](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (70d64f6) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/62b7590443e037ecd6b41521accea567e09ad340?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (62b7590) will **increase** coverage by `0.35%`.
> The diff coverage is `49.00%`.
> :exclamation: Current head 70d64f6 differs from pull request most recent head 9fd71ab. Consider uploading reports for the commit 9fd71ab to get more accurate results
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
+ Coverage 34.31% 34.66% +0.35%
==========================================
Files 57 60 +3
Lines 5746 5887 +141
==========================================
+ Hits 1972 2041 +69
- Misses 3527 3596 +69
- Partials 247 250 +3
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/api/router/router.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvcm91dGVyLmdv) | `75.00% <ø> (ø)` | |
| [pkg/api/validation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL2FwaXNpeF9yb3V0ZS5nbw==) | `21.53% <21.53%> (ø)` | |
| [pkg/api/validation/utils.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL3V0aWxzLmdv) | `46.15% <46.15%> (ø)` | |
| [pkg/api/server.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9zZXJ2ZXIuZ28=) | `80.55% <77.35%> (+4.69%)` | :arrow_up: |
| [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.27% <100.00%> (+0.26%)` | :arrow_up: |
| [pkg/api/router/webhook.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvd2ViaG9vay5nbw==) | `100.00% <100.00%> (ø)` | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `86.44% <100.00%> (+0.72%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/573?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 [62b7590...9fd71ab](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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] gxthrj commented on a change in pull request #573: [WIP] Implement the admission server
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r664187459
##########
File path: pkg/api/validation/plugin_schema.go
##########
@@ -0,0 +1,89 @@
+// 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 validation
+
+// PluginSchema stores all plugins' schema.
+// TODO: add more plugins' schema.
+var PluginSchema = map[string]string{
Review comment:
PluginSchema needs to get from admin API, such as `http://127.0.0.1:9080/apisix/admin/schema/plugins/api-breaker`.
--
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 edited a comment on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-874397196
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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 [#573](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (240ffae) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/62b7590443e037ecd6b41521accea567e09ad340?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (62b7590) will **increase** coverage by `0.40%`.
> The diff coverage is `50.64%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
+ Coverage 34.31% 34.72% +0.40%
==========================================
Files 57 60 +3
Lines 5746 5892 +146
==========================================
+ Hits 1972 2046 +74
- Misses 3527 3596 +69
- Partials 247 250 +3
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/api/router/router.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvcm91dGVyLmdv) | `75.00% <ø> (ø)` | |
| [pkg/api/validation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL2FwaXNpeF9yb3V0ZS5nbw==) | `21.53% <21.53%> (ø)` | |
| [pkg/api/validation/utils.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL3V0aWxzLmdv) | `46.15% <46.15%> (ø)` | |
| [pkg/api/server.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9zZXJ2ZXIuZ28=) | `81.81% <79.31%> (+5.95%)` | :arrow_up: |
| [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.27% <100.00%> (+0.26%)` | :arrow_up: |
| [pkg/api/router/webhook.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvd2ViaG9vay5nbw==) | `100.00% <100.00%> (ø)` | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `86.44% <100.00%> (+0.72%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/573?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 [62b7590...240ffae](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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] fgksgf commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698139235
##########
File path: test/e2e/scaffold/scaffold.go
##########
@@ -407,3 +417,16 @@ func waitExponentialBackoff(condFunc func() (bool, error)) error {
}
return wait.ExponentialBackoff(backoff, condFunc)
}
+
+// generateWebhookCert generates signed certs of webhook and create the corresponding secret by running a script.
+func generateWebhookCert(ns string) error {
+ commandTemplate := `../../utils/webhook-create-signed-cert.sh`
Review comment:
good idea, I didn't notice the `testdata` dir
--
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] gxthrj commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r695867474
##########
File path: test/e2e/scaffold/ssl.go
##########
@@ -29,8 +29,8 @@ kind: Secret
metadata:
name: %s
data:
- cert: %s
- key: %s
+ cert.pem: %s
+ key.pem: %s
Review comment:
Yes ，it will break other e2e test cases. Suggest to keep `cert` and `key`, add `cert.pem` and `key.pem`.
--
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] fgksgf commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r695761476
##########
File path: test/e2e/scaffold/scaffold.go
##########
@@ -281,7 +281,8 @@ func (s *Scaffold) APISIXGatewayServiceEndpoint() string {
func (s *Scaffold) beforeEach() {
var err error
- s.namespace = fmt.Sprintf("ingress-apisix-e2e-tests-%s-%d", s.opts.Name, time.Now().Nanosecond())
+ s.namespace = "ingress-apisix"
+ //s.namespace = fmt.Sprintf("ingress-apisix-e2e-tests-%s-%d", s.opts.Name, time.Now().Nanosecond())
Review comment:
To generate the webhook certs, I have to specify the namespace of webhook service, so I make it to a constant.
--
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] fgksgf commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698098181
##########
File path: samples/deploy/admission/webhook-registration.yaml
##########
@@ -0,0 +1,40 @@
+#
+# 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.
+#
+
+apiVersion: admissionregistration.k8s.io/v1beta1
+kind: ValidatingWebhookConfiguration
+metadata:
+ name: apisix-validations
+ labels:
+ app: apisixroute-plugin-validator-webhook
+ kind: validating
+webhooks:
+ - name: apisixroute-plugin-validator-webhook
+ clientConfig:
+ service:
+ name: apisix-admission-server
+ namespace: ingress-apisix
+ port: 8443
+ path: "/validation/apisixRoute/plugin"
+ caBundle: ${CA_BUNDLE}
+ rules:
+ - operations: [ "CREATE", "UPDATE" ]
+ apiGroups: ["apisix.apache.org"]
+ apiVersions: ["*"]
+ resources: ["apisixroutes"]
+ timeoutSeconds: 30
+ failurePolicy: Ignore
Review comment:
Because this feature is just implemented, I'm afraid that it may be not stable enough. I prefer to set to `Fail` after I implement other webhooks (route/upstream/consumer) and add more test cases.
--
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 edited a comment on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-874397196
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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 [#573](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bb99fa4) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/91d985edca67493acc22536c66d4947fe597052f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (91d985e) will **increase** coverage by `0.18%`.
> The diff coverage is `43.66%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
+ Coverage 34.46% 34.64% +0.18%
==========================================
Files 57 60 +3
Lines 5722 5856 +134
==========================================
+ Hits 1972 2029 +57
- Misses 3504 3578 +74
- Partials 246 249 +3
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/api/router/router.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvcm91dGVyLmdv) | `75.00% <ø> (ø)` | |
| [pkg/api/validation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL2FwaXNpeF9yb3V0ZS5nbw==) | `25.00% <25.00%> (ø)` | |
| [pkg/api/validation/utils.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL3V0aWxzLmdv) | `35.29% <35.29%> (ø)` | |
| [pkg/api/server.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9zZXJ2ZXIuZ28=) | `80.00% <79.41%> (+4.13%)` | :arrow_up: |
| [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.27% <100.00%> (+0.26%)` | :arrow_up: |
| [pkg/api/router/webhook.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvd2ViaG9vay5nbw==) | `100.00% <100.00%> (ø)` | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `86.44% <100.00%> (+0.72%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/573?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 [d8854c3...bb99fa4](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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] tokers commented on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-908357007
> > > > reply: [#573 (comment)](https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698118062)
> > >
> > >
> > > If we provide default TLS Certificates, they are not signed by CA, and the webhook still can't work. We can accomplish this after we have cert-manager.
> > > I can help integrate cert-manager after webhooks are implemented.
> >
> >
> > We may use `CertificateSigningRequest`, and give documentation to guide people.
>
> Good idea. But how about file a new PR to do this, because the current PR is a bit huge.
Sure.
--
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] gxthrj commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r695869813
##########
File path: utils/webhook-create-signed-cert.sh
##########
@@ -0,0 +1,127 @@
+#!/bin/sh
+
+# This script is copied from the istio project.
+# https://github.com/istio/istio/blob/release-0.7/install/kubernetes/webhook-create-signed-cert.sh
Review comment:
`Istio` is Apache-2.0 License too, The script needs to add a license header, but you can declare that it is a copy from `istio`.
--
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] fgksgf commented on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-905511489
@gxthrj @tokers @tao12345666333 PTAL
--
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 #573: [WIP] Implement the admission server
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-874397196
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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 [#573](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4f267d1) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/fe57c7509505ee1474ae19010d137d4f765d4825?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fe57c75) will **decrease** coverage by `0.09%`.
> The diff coverage is `25.45%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
- Coverage 35.03% 34.93% -0.10%
==========================================
Files 54 56 +2
Lines 4556 4608 +52
==========================================
+ Hits 1596 1610 +14
- Misses 2742 2779 +37
- Partials 218 219 +1
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/api/router/router.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvcm91dGVyLmdv) | `62.50% <0.00%> (-8.93%)` | :arrow_down: |
| [pkg/api/router/webhook.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvd2ViaG9vay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `84.00% <ø> (ø)` | |
| [pkg/api/validation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL2FwaXNpeF9yb3V0ZS5nbw==) | `24.39% <24.39%> (ø)` | |
| [pkg/api/server.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9zZXJ2ZXIuZ28=) | `68.57% <36.36%> (-5.51%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/573?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 [7e146b6...4f267d1](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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] tokers commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698117176
##########
File path: pkg/api/server.go
##########
@@ -12,14 +12,21 @@
// 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 api
import (
+ "context"
+ "crypto/tls"
"net"
"net/http"
"net/http/pprof"
+ "time"
Review comment:
Sort the import.
##########
File path: pkg/api/server.go
##########
@@ -67,13 +99,51 @@ func NewServer(cfg *config.Config) (*Server, error) {
func (srv *Server) Run(stopCh <-chan struct{}) error {
go func() {
<-stopCh
- if err := srv.httpListener.Close(); err != nil {
- log.Errorf("failed to close http listener: %s", err)
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+
+ closeHttp := make(chan struct{})
+ go srv.closeHttpServer(closeHttp)
Review comment:
The codes here are weird, we may both add a timeout for these two servers, and the closing can be parallel.
##########
File path: pkg/api/router/webhook.go
##########
@@ -0,0 +1,29 @@
+// 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 router
+
+import (
+ "github.com/gin-gonic/gin"
+
+ "github.com/apache/apisix-ingress-controller/pkg/api/validation"
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
+)
+
Review comment:
Please add some comments for this function.
##########
File path: pkg/api/server.go
##########
@@ -12,14 +12,21 @@
// 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 api
import (
+ "context"
+ "crypto/tls"
"net"
"net/http"
"net/http/pprof"
+ "time"
+
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
Review comment:
Sort the import.
##########
File path: pkg/api/router/webhook.go
##########
@@ -0,0 +1,29 @@
+// 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 router
+
+import (
+ "github.com/gin-gonic/gin"
+
+ "github.com/apache/apisix-ingress-controller/pkg/api/validation"
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
+)
+
+func MountWebhooks(r *gin.Engine, co *apisix.ClusterOptions) {
+ // init the schema client
+ _, _ = validation.GetSchemaClient(co)
+ r.POST("/validation/apisixroute/plugin", gin.WrapH(validation.NewPluginValidatorHandler()))
Review comment:
Maybe `/validation/apisixroutes/plugin`?
##########
File path: test/e2e/scaffold/scaffold.go
##########
@@ -340,12 +350,12 @@ func (s *Scaffold) afterEach() {
}
}
- err := k8s.DeleteNamespaceE(s.t, s.kubectlOptions, s.namespace)
- assert.Nilf(ginkgo.GinkgoT(), err, "deleting namespace %s", s.namespace)
-
- for _, f := range s.finializers {
- f()
- }
+ //err := k8s.DeleteNamespaceE(s.t, s.kubectlOptions, s.namespace)
Review comment:
Why comment these codes?
##########
File path: pkg/api/server.go
##########
@@ -41,23 +46,43 @@ func NewServer(cfg *config.Config) (*Server, error) {
return nil, err
}
gin.SetMode(gin.ReleaseMode)
- router := gin.New()
- router.Use(gin.Recovery(), gin.Logger())
- apirouter.Mount(router)
+ httpServer := gin.New()
+ httpServer.Use(gin.Recovery(), gin.Logger())
+ apirouter.Mount(httpServer)
srv := &Server{
- router: router,
+ httpServer: httpServer,
httpListener: httpListener,
}
-
if cfg.EnableProfiling {
srv.pprofMu = new(http.ServeMux)
srv.pprofMu.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
srv.pprofMu.HandleFunc("/debug/pprof/profile", pprof.Profile)
srv.pprofMu.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
srv.pprofMu.HandleFunc("/debug/pprof/trace", pprof.Trace)
srv.pprofMu.HandleFunc("/debug/pprof/", pprof.Index)
- router.GET("/debug/pprof/*profile", gin.WrapF(srv.pprofMu.ServeHTTP))
+ httpServer.GET("/debug/pprof/*profile", gin.WrapF(srv.pprofMu.ServeHTTP))
+ }
+
+ cert, err := tls.LoadX509KeyPair(cfg.CertFilePath, cfg.KeyFilePath)
Review comment:
Then you should prepare the default certificate and private key, or it's difficult to start the server in the local environment.
##########
File path: pkg/api/validation/apisix_route.go
##########
@@ -0,0 +1,149 @@
+// 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 validation
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "net/http"
+ "strings"
+
+ "github.com/hashicorp/go-multierror"
+ kwhhttp "github.com/slok/kubewebhook/v2/pkg/http"
+ kwhmodel "github.com/slok/kubewebhook/v2/pkg/model"
+ kwhvalidating "github.com/slok/kubewebhook/v2/pkg/webhook/validating"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
+ v1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
+ "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2alpha1"
+ "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2beta1"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
+)
+
+// NewPluginValidatorHandler returns a new http.Handler ready to handle admission reviews using the pluginValidator.
+func NewPluginValidatorHandler() http.Handler {
+ // Create a validating webhook.
+ wh, err := kwhvalidating.NewWebhook(kwhvalidating.WebhookConfig{
+ ID: "apisixRoute-plugin",
+ Validator: pluginValidator,
+ })
+ if err != nil {
+ log.Errorf("failed to create webhook: %s", err)
+ }
+
+ h, err := kwhhttp.HandlerFor(kwhhttp.HandlerConfig{Webhook: wh})
+ if err != nil {
+ log.Errorf("failed to create webhook handle: %s", err)
+ }
+
+ return h
+}
+
+// ErrNotApisixRoute will be used when the validating object is not ApisixRoute.
+var ErrNotApisixRoute = errors.New("object is not ApisixRoute")
+
+type apisixRoutePlugin struct {
+ Name string
+ Config interface{}
+}
+
+// pluginValidator validates plugins in ApisixRoute.
+// When the validation of one plugin fails, it will continue to validate the rest of plugins.
+var pluginValidator = kwhvalidating.ValidatorFunc(
+ func(ctx context.Context, review *kwhmodel.AdmissionReview, object metav1.Object) (result *kwhvalidating.ValidatorResult, err error) {
+ log.Debug("arrive plugin validator webhook")
+
+ valid := true
+ var plugins []apisixRoutePlugin
+
+ switch ar := object.(type) {
+ case *v2beta1.ApisixRoute:
+ for _, h := range ar.Spec.HTTP {
+ for _, p := range h.Plugins {
+ // only check plugins that are enabled.
+ if p.Enable {
+ plugins = append(plugins, apisixRoutePlugin{
+ p.Name, p.Config,
+ })
+ }
+ }
+ }
+ case *v2alpha1.ApisixRoute:
+ for _, h := range ar.Spec.HTTP {
+ for _, p := range h.Plugins {
+ if p.Enable {
+ plugins = append(plugins, apisixRoutePlugin{
+ p.Name, p.Config,
+ })
+ }
+ }
+ }
+ case *v1.ApisixRoute:
+ for _, r := range ar.Spec.Rules {
+ for _, path := range r.Http.Paths {
+ for _, p := range path.Plugins {
+ if p.Enable {
+ plugins = append(plugins, apisixRoutePlugin{
+ p.Name, p.Config,
+ })
+ }
+ }
+ }
+ }
+ default:
+ return &kwhvalidating.ValidatorResult{Valid: false, Message: ErrNotApisixRoute.Error()}, ErrNotApisixRoute
+ }
+
+ client, err := GetSchemaClient(&apisix.ClusterOptions{})
+ if err != nil {
+ log.Errorf("failed to get the schema client: %s", err)
+ return &kwhvalidating.ValidatorResult{Valid: false, Message: "failed to get the schema client"}, err
+ }
+
+ var msg []string
+ for _, p := range plugins {
+ if v, m, err := validatePlugin(client, p.Name, p.Config); !v {
+ valid = false
+ msg = append(msg, m)
+ log.Warnf("failed to validate plugin %s: %s", p.Name, err)
+ }
+ }
+
+ return &kwhvalidating.ValidatorResult{Valid: valid, Message: strings.Join(msg, "\n")}, nil
+ })
Review comment:
```suggestion
}
)
```
##########
File path: test/e2e/scaffold/scaffold.go
##########
@@ -407,3 +417,16 @@ func waitExponentialBackoff(condFunc func() (bool, error)) error {
}
return wait.ExponentialBackoff(backoff, condFunc)
}
+
+// generateWebhookCert generates signed certs of webhook and create the corresponding secret by running a script.
+func generateWebhookCert(ns string) error {
+ commandTemplate := `../../utils/webhook-create-signed-cert.sh`
Review comment:
Why not prepare static files and put them into `testdata` dir?
--
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] fgksgf commented on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-908356237
> > > reply: [#573 (comment)](https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698118062)
> >
> >
> > If we provide default TLS Certificates, they are not signed by CA, and the webhook still can't work. We can accomplish this after we have cert-manager.
> > I can help integrate cert-manager after webhooks are implemented.
>
> We may use `CertificateSigningRequest`, and give documentation to guide people.
Good idea. But how about file a new PR to do this, because the current PR is a bit huge.
--
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] fgksgf commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698138844
##########
File path: test/e2e/scaffold/scaffold.go
##########
@@ -340,12 +350,12 @@ func (s *Scaffold) afterEach() {
}
}
- err := k8s.DeleteNamespaceE(s.t, s.kubectlOptions, s.namespace)
- assert.Nilf(ginkgo.GinkgoT(), err, "deleting namespace %s", s.namespace)
-
- for _, f := range s.finializers {
- f()
- }
+ //err := k8s.DeleteNamespaceE(s.t, s.kubectlOptions, s.namespace)
Review comment:
Yes, I comment these for debugging and forgot to uncomment them... sry
--
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] fgksgf commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r695762515
##########
File path: utils/webhook-create-signed-cert.sh
##########
@@ -0,0 +1,127 @@
+#!/bin/sh
+
+# This script is copied from the istio project.
+# https://github.com/istio/istio/blob/release-0.7/install/kubernetes/webhook-create-signed-cert.sh
Review comment:
I have no idea if this script need to add license header, because it is copied from `istio`.
--
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] gxthrj commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698135325
##########
File path: test/e2e/scaffold/scaffold.go
##########
@@ -340,12 +350,12 @@ func (s *Scaffold) afterEach() {
}
}
- err := k8s.DeleteNamespaceE(s.t, s.kubectlOptions, s.namespace)
- assert.Nilf(ginkgo.GinkgoT(), err, "deleting namespace %s", s.namespace)
-
- for _, f := range s.finializers {
- f()
- }
+ //err := k8s.DeleteNamespaceE(s.t, s.kubectlOptions, s.namespace)
Review comment:
-_-||, This must be for debugging, please change it back.
--
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] gxthrj commented on a change in pull request #573: [WIP] Implement the admission server
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r664187459
##########
File path: pkg/api/validation/plugin_schema.go
##########
@@ -0,0 +1,89 @@
+// 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 validation
+
+// PluginSchema stores all plugins' schema.
+// TODO: add more plugins' schema.
+var PluginSchema = map[string]string{
Review comment:
PluginSchema needs to get from admin API, such as `http://127.0.0.1:9080/apisix/admin/schema/plugins/api-breaker`.
--
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 change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r693591456
##########
File path: Makefile
##########
@@ -106,6 +106,13 @@ ifeq ($(E2E_SKIP_BUILD), 0)
docker push $(LOCAL_REGISTRY)/jmalloc/echo-server:latest
endif
+# rebuild ingress controller image and push to the local registry for e2e tests.
Review comment:
Why reduplicate the same contents that were implemented in another make directive (`push-images-to-kind`)?
##########
File path: pkg/api/server.go
##########
@@ -41,23 +46,43 @@ func NewServer(cfg *config.Config) (*Server, error) {
return nil, err
}
gin.SetMode(gin.ReleaseMode)
- router := gin.New()
- router.Use(gin.Recovery(), gin.Logger())
- apirouter.Mount(router)
+ httpServer := gin.New()
+ httpServer.Use(gin.Recovery(), gin.Logger())
+ apirouter.Mount(httpServer)
srv := &Server{
- router: router,
+ httpServer: httpServer,
httpListener: httpListener,
}
-
if cfg.EnableProfiling {
srv.pprofMu = new(http.ServeMux)
srv.pprofMu.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
srv.pprofMu.HandleFunc("/debug/pprof/profile", pprof.Profile)
srv.pprofMu.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
srv.pprofMu.HandleFunc("/debug/pprof/trace", pprof.Trace)
srv.pprofMu.HandleFunc("/debug/pprof/", pprof.Index)
- router.GET("/debug/pprof/*profile", gin.WrapF(srv.pprofMu.ServeHTTP))
+ httpServer.GET("/debug/pprof/*profile", gin.WrapF(srv.pprofMu.ServeHTTP))
+ }
+
+ cert, err := tls.LoadX509KeyPair(cfg.CertFilePath, cfg.KeyFilePath)
Review comment:
Should it be optional? Only enable TLS if the certificate and private key are specified simultaneously.
##########
File path: pkg/api/validation/utils.go
##########
@@ -0,0 +1,89 @@
+// 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 validation
+
+import (
+ "context"
+ "fmt"
+ "sync"
+
+ "github.com/xeipuuv/gojsonschema"
+
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
+ "github.com/apache/apisix-ingress-controller/pkg/config"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
+)
+
+var (
+ once sync.Once
+ onceErr error
+ schemaClient apisix.Schema
+)
+
+// GetSchemaClient returns a Schema client in the singleton way.
+// It can query the schema of objects from APISIX.
+func GetSchemaClient() (apisix.Schema, error) {
+ once.Do(func() {
+ client, err := apisix.NewClient()
+ if err != nil {
+ onceErr = err
+ return
+ }
+
+ cfg := config.NewDefaultConfig()
+ if err := cfg.Validate(); err != nil {
+ onceErr = err
+ return
+ }
+
+ clusterOpts := &apisix.ClusterOptions{
+ Name: cfg.APISIX.DefaultClusterName,
+ AdminKey: cfg.APISIX.DefaultClusterAdminKey,
+ BaseURL: cfg.APISIX.DefaultClusterBaseURL,
+ }
+ if err := client.AddCluster(context.TODO(), clusterOpts); err != nil {
+ onceErr = err
+ return
+ }
+
+ schemaClient = client.Cluster(cfg.APISIX.DefaultClusterName).Schema()
+ })
+ return schemaClient, onceErr
+}
+
+// TODO: make this helper function more generic so that it can be used by other validating webhooks.
+func validateSchema(schema string, config interface{}) (error, []gojsonschema.ResultError) {
+ schemaLoader := gojsonschema.NewStringLoader(schema)
Review comment:
Schema loader can be cached (in the future).
##########
File path: pkg/api/server_test.go
##########
@@ -25,17 +27,85 @@ import (
"github.com/apache/apisix-ingress-controller/pkg/config"
)
+const (
+ certFileName = "cert.pem"
Review comment:
You may try to use `ioutil.TempFile` to generate different files, instead of specifying filename by yourself.
##########
File path: cmd/ingress/ingress.go
##########
@@ -139,6 +139,7 @@ the apisix cluster and others are created`,
cmd.PersistentFlags().StringVar(&cfg.LogLevel, "log-level", "info", "error log level")
cmd.PersistentFlags().StringVar(&cfg.LogOutput, "log-output", "stderr", "error log output file")
cmd.PersistentFlags().StringVar(&cfg.HTTPListen, "http-listen", ":8080", "the HTTP Server listen address")
+ cmd.PersistentFlags().StringVar(&cfg.HTTPSListen, "https-listen", ":443", "the HTTPS Server listen address")
Review comment:
We'd better not to use the well-known `443` port here:
1. Listen on `443` requiring root permission
2. it's not consistent with the HTTP listen (`8080`).
##########
File path: pkg/api/validation/apisix_route.go
##########
@@ -0,0 +1,152 @@
+// 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 validation
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "net/http"
+ "strings"
+
+ kwhhttp "github.com/slok/kubewebhook/v2/pkg/http"
+ kwhmodel "github.com/slok/kubewebhook/v2/pkg/model"
+ kwhvalidating "github.com/slok/kubewebhook/v2/pkg/webhook/validating"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
+ v1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
+ "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2alpha1"
+ "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2beta1"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
+)
+
+func NewPluginValidatorHandler() http.Handler {
Review comment:
Add some comments for this function.
##########
File path: pkg/api/server.go
##########
@@ -70,10 +95,29 @@ func (srv *Server) Run(stopCh <-chan struct{}) error {
if err := srv.httpListener.Close(); err != nil {
log.Errorf("failed to close http listener: %s", err)
}
+
+ if srv.admissionServer != nil {
+ if err := srv.admissionServer.Shutdown(context.TODO()); err != nil {
Review comment:
Add the timeout limitation.
##########
File path: pkg/api/validation/utils.go
##########
@@ -0,0 +1,89 @@
+// 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 validation
+
+import (
+ "context"
+ "fmt"
+ "sync"
+
+ "github.com/xeipuuv/gojsonschema"
+
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
+ "github.com/apache/apisix-ingress-controller/pkg/config"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
+)
+
+var (
+ once sync.Once
+ onceErr error
+ schemaClient apisix.Schema
+)
+
+// GetSchemaClient returns a Schema client in the singleton way.
+// It can query the schema of objects from APISIX.
+func GetSchemaClient() (apisix.Schema, error) {
Review comment:
Why fetch schema in the singleton way, schema is synchronized periodically.
##########
File path: test/e2e/ingress/webhook.go
##########
@@ -0,0 +1,67 @@
+// 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 ingress
+
+import (
+ "fmt"
+
+ "github.com/onsi/ginkgo"
+ "github.com/stretchr/testify/assert"
+
+ "github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
+)
+
+var _ = ginkgo.Describe("Enable webhooks", func() {
+ opts := &scaffold.Options{
+ Name: "default",
+ Kubeconfig: scaffold.GetKubeconfig(),
+ APISIXConfigPath: "testdata/apisix-gw-config.yaml",
+ IngressAPISIXReplicas: 1,
+ HTTPBinServicePort: 80,
+ APISIXRouteVersion: "apisix.apache.org/v2alpha1",
+ EnableWebhooks: true,
+ }
+ s := scaffold.NewScaffold(opts)
+
+ ginkgo.It("should fail to create the ApisixRoute with invalid plugin configuration", func() {
+ backendSvc, backendPorts := s.DefaultHTTPBackend()
+ ar := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2alpha1
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+ http:
+ - name: rule1
+ match:
+ hosts:
+ - httpbin.org
+ paths:
+ - /status/*
+ backends:
+ - serviceName: %s
+ servicePort: %d
+ resolveGranularity: service
+ plugins:
+ - name: api-breaker
+ enable: true
+ config:
+ break_response_code: 100 # should in [200, 599]
+`, backendSvc, backendPorts[0])
+
+ assert.Error(ginkgo.GinkgoT(), s.CreateResourceFromString(ar), "Failed to create ApisixRoute")
Review comment:
Also check out the error message.
##########
File path: pkg/api/validation/apisix_route.go
##########
@@ -0,0 +1,152 @@
+// 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 validation
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "net/http"
+ "strings"
+
+ kwhhttp "github.com/slok/kubewebhook/v2/pkg/http"
+ kwhmodel "github.com/slok/kubewebhook/v2/pkg/model"
+ kwhvalidating "github.com/slok/kubewebhook/v2/pkg/webhook/validating"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
+ v1 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v1"
+ "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2alpha1"
+ "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2beta1"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
+)
+
+func NewPluginValidatorHandler() http.Handler {
+ // Create a validating webhook.
+ wh, err := kwhvalidating.NewWebhook(kwhvalidating.WebhookConfig{
+ ID: "apisixRoute-plugin",
+ Validator: pluginValidator,
+ })
+ if err != nil {
+ log.Errorf("failed to create webhook: %s", err)
+ }
+
+ h, err := kwhhttp.HandlerFor(kwhhttp.HandlerConfig{Webhook: wh})
+ if err != nil {
+ log.Errorf("failed to create webhook handle: %s", err)
+ }
+
+ return h
+}
+
+// ErrNotApisixRoute will be used when the validating object is not ApisixRoute.
+var ErrNotApisixRoute = errors.New("object is not ApisixRoute")
+
+type apisixRoutePlugin struct {
+ Name string
+ Config interface{}
+}
+
+// pluginValidator validates plugins in ApisixRoute.
+// When the validation of one plugin fails, it will continue to validate the rest of plugins.
+var pluginValidator = kwhvalidating.ValidatorFunc(
+ func(ctx context.Context, review *kwhmodel.AdmissionReview, object metav1.Object) (result *kwhvalidating.ValidatorResult, err error) {
+ log.Debug("arrive plugin validator webhook")
+
+ valid := true
+ var plugins []apisixRoutePlugin
+
+ switch ar := object.(type) {
+ case *v2beta1.ApisixRoute:
+ for _, h := range ar.Spec.HTTP {
+ for _, p := range h.Plugins {
+ // only check plugins that are enabled.
+ if p.Enable {
+ plugins = append(plugins, apisixRoutePlugin{
+ p.Name, p.Config,
+ })
+ }
+ }
+ }
+ case *v2alpha1.ApisixRoute:
+ for _, h := range ar.Spec.HTTP {
+ for _, p := range h.Plugins {
+ if p.Enable {
+ plugins = append(plugins, apisixRoutePlugin{
+ p.Name, p.Config,
+ })
+ }
+ }
+ }
+ case *v1.ApisixRoute:
+ for _, r := range ar.Spec.Rules {
+ for _, path := range r.Http.Paths {
+ for _, p := range path.Plugins {
+ if p.Enable {
+ plugins = append(plugins, apisixRoutePlugin{
+ p.Name, p.Config,
+ })
+ }
+ }
+ }
+ }
+ default:
+ return &kwhvalidating.ValidatorResult{Valid: false, Message: ErrNotApisixRoute.Error()}, ErrNotApisixRoute
+ }
+
+ client, err := GetSchemaClient()
+ if err != nil {
+ log.Errorf("failed to get the schema client: %s", err)
+ return &kwhvalidating.ValidatorResult{Valid: false, Message: "failed to get the schema client"}, err
+ }
+
+ var msg []string
+ for _, p := range plugins {
+ if v, m, err := validatePlugin(client, p.Name, p.Config); !v {
+ valid = false
+ msg = append(msg, m)
+ log.Warnf("failed to validate plugin %s: %s", p.Name, err)
+ }
+ }
+
+ return &kwhvalidating.ValidatorResult{Valid: valid, Message: strings.Join(msg, "\n")}, nil
+ })
+
+func validatePlugin(client apisix.Schema, pluginName string, pluginConfig interface{}) (valid bool, msg string, err error) {
+ valid = true
+
+ pluginSchema, err := client.GetPluginSchema(context.TODO(), pluginName)
+ if err != nil {
+ log.Errorf("failed to get the schema of plugin %s: %s", pluginName, err)
+ valid = false
+ msg = fmt.Sprintf("failed to get the schema of plugin %s", pluginName)
+ return
+ }
+
+ var errorMsg []string
+ if err, re := validateSchema(pluginSchema.Content, pluginConfig); err != nil {
+ valid = false
+ msg = fmt.Sprintf("%s plugin's config is invalid\n", pluginName)
+ for _, desc := range re {
+ errorMsg = append(errorMsg, desc.String())
Review comment:
Use go-multierror package.
##########
File path: conf/config-default.yaml
##########
@@ -27,7 +27,11 @@ log_output: "stderr" # the output file path of error log, default is stderr, whe
# are marshalled in JSON format, which can be parsed by
# programs easily.
+cert_file: "/etc/webhook/certs/cert.pem" # the TLS certificate file path.
+key_file: "/etc/webhook/certs/key.pem" # the TLS key file path.
+
http_listen: ":8080" # the HTTP Server listen address, default is ":8080"
+https_listen: ":443" # the HTTPS Server listen address, default is ":443"
Review comment:
Ditto.
--
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] gxthrj commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r693595525
##########
File path: cmd/ingress/ingress.go
##########
@@ -139,6 +139,7 @@ the apisix cluster and others are created`,
cmd.PersistentFlags().StringVar(&cfg.LogLevel, "log-level", "info", "error log level")
cmd.PersistentFlags().StringVar(&cfg.LogOutput, "log-output", "stderr", "error log output file")
cmd.PersistentFlags().StringVar(&cfg.HTTPListen, "http-listen", ":8080", "the HTTP Server listen address")
+ cmd.PersistentFlags().StringVar(&cfg.HTTPSListen, "https-listen", ":443", "the HTTPS Server listen address")
Review comment:
Or use 8443 instead.
--
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 #573: [WIP] Implement the admission server
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-874397196
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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 [#573](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4f267d1) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/fe57c7509505ee1474ae19010d137d4f765d4825?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fe57c75) will **decrease** coverage by `0.09%`.
> The diff coverage is `25.45%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
- Coverage 35.03% 34.93% -0.10%
==========================================
Files 54 56 +2
Lines 4556 4608 +52
==========================================
+ Hits 1596 1610 +14
- Misses 2742 2779 +37
- Partials 218 219 +1
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/api/router/router.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvcm91dGVyLmdv) | `62.50% <0.00%> (-8.93%)` | :arrow_down: |
| [pkg/api/router/webhook.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvd2ViaG9vay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `84.00% <ø> (ø)` | |
| [pkg/api/validation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL2FwaXNpeF9yb3V0ZS5nbw==) | `24.39% <24.39%> (ø)` | |
| [pkg/api/server.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9zZXJ2ZXIuZ28=) | `68.57% <36.36%> (-5.51%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/573?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 [7e146b6...4f267d1](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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] codecov-commenter edited a comment on pull request #573: [WIP] Implement the admission server
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-874397196
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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 [#573](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4f267d1) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/fe57c7509505ee1474ae19010d137d4f765d4825?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fe57c75) will **decrease** coverage by `0.07%`.
> The diff coverage is `25.45%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
- Coverage 35.03% 34.95% -0.08%
==========================================
Files 54 57 +3
Lines 4556 4609 +53
==========================================
+ Hits 1596 1611 +15
- Misses 2742 2779 +37
- Partials 218 219 +1
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/api/router/router.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvcm91dGVyLmdv) | `62.50% <0.00%> (-8.93%)` | :arrow_down: |
| [pkg/api/router/webhook.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvd2ViaG9vay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `84.00% <ø> (ø)` | |
| [pkg/api/validation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL2FwaXNpeF9yb3V0ZS5nbw==) | `24.39% <24.39%> (ø)` | |
| [pkg/api/server.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9zZXJ2ZXIuZ28=) | `68.57% <36.36%> (-5.51%)` | :arrow_down: |
| [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-dGVzdC9lMmUvZTJlLmdv) | `100.00% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/573?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 [7e146b6...4f267d1](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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] codecov-commenter edited a comment on pull request #573: Implement the admission server
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-874397196
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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 [#573](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4fc3d6d) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/91d985edca67493acc22536c66d4947fe597052f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (91d985e) will **increase** coverage by `0.18%`.
> The diff coverage is `43.66%`.
> :exclamation: Current head 4fc3d6d differs from pull request most recent head d5f0f91. Consider uploading reports for the commit d5f0f91 to get more accurate results
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
+ Coverage 34.46% 34.64% +0.18%
==========================================
Files 57 60 +3
Lines 5722 5856 +134
==========================================
+ Hits 1972 2029 +57
- Misses 3504 3578 +74
- Partials 246 249 +3
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/api/router/router.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvcm91dGVyLmdv) | `75.00% <ø> (ø)` | |
| [pkg/api/validation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL2FwaXNpeF9yb3V0ZS5nbw==) | `25.00% <25.00%> (ø)` | |
| [pkg/api/validation/utils.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL3V0aWxzLmdv) | `35.29% <35.29%> (ø)` | |
| [pkg/api/server.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9zZXJ2ZXIuZ28=) | `80.00% <79.41%> (+4.13%)` | :arrow_up: |
| [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.27% <100.00%> (+0.26%)` | :arrow_up: |
| [pkg/api/router/webhook.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvd2ViaG9vay5nbw==) | `100.00% <100.00%> (ø)` | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `86.44% <100.00%> (+0.72%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/573?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 [d8854c3...d5f0f91](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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] codecov-commenter edited a comment on pull request #573: [WIP] Implement the admission server
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-874397196
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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 [#573](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4f267d1) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/fe57c7509505ee1474ae19010d137d4f765d4825?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fe57c75) will **decrease** coverage by `0.07%`.
> The diff coverage is `25.45%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
- Coverage 35.03% 34.95% -0.08%
==========================================
Files 54 57 +3
Lines 4556 4609 +53
==========================================
+ Hits 1596 1611 +15
- Misses 2742 2779 +37
- Partials 218 219 +1
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/api/router/router.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvcm91dGVyLmdv) | `62.50% <0.00%> (-8.93%)` | :arrow_down: |
| [pkg/api/router/webhook.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvd2ViaG9vay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `84.00% <ø> (ø)` | |
| [pkg/api/validation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL2FwaXNpeF9yb3V0ZS5nbw==) | `24.39% <24.39%> (ø)` | |
| [pkg/api/server.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9zZXJ2ZXIuZ28=) | `68.57% <36.36%> (-5.51%)` | :arrow_down: |
| [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-dGVzdC9lMmUvZTJlLmdv) | `100.00% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/573?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 [7e146b6...4f267d1](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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] codecov-commenter edited a comment on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-874397196
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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 [#573](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (372da26) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/62b7590443e037ecd6b41521accea567e09ad340?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (62b7590) will **increase** coverage by `0.35%`.
> The diff coverage is `49.00%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
+ Coverage 34.31% 34.66% +0.35%
==========================================
Files 57 60 +3
Lines 5746 5887 +141
==========================================
+ Hits 1972 2041 +69
- Misses 3527 3596 +69
- Partials 247 250 +3
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/api/router/router.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvcm91dGVyLmdv) | `75.00% <ø> (ø)` | |
| [pkg/api/validation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL2FwaXNpeF9yb3V0ZS5nbw==) | `21.53% <21.53%> (ø)` | |
| [pkg/api/validation/utils.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL3V0aWxzLmdv) | `46.15% <46.15%> (ø)` | |
| [pkg/api/server.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9zZXJ2ZXIuZ28=) | `80.55% <77.35%> (+4.69%)` | :arrow_up: |
| [cmd/ingress/ingress.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-Y21kL2luZ3Jlc3MvaW5ncmVzcy5nbw==) | `77.27% <100.00%> (+0.26%)` | :arrow_up: |
| [pkg/api/router/webhook.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvd2ViaG9vay5nbw==) | `100.00% <100.00%> (ø)` | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `86.44% <100.00%> (+0.72%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/573?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 [62b7590...372da26](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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] codecov-commenter edited a comment on pull request #573: [WIP] Implement the admission server
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-874397196
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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 [#573](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cacdc3b) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/91d985edca67493acc22536c66d4947fe597052f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (91d985e) will **decrease** coverage by `0.06%`.
> The diff coverage is `30.27%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
- Coverage 34.46% 34.40% -0.07%
==========================================
Files 57 60 +3
Lines 5722 5828 +106
==========================================
+ Hits 1972 2005 +33
- Misses 3504 3576 +72
- Partials 246 247 +1
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/api/router/router.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvcm91dGVyLmdv) | `69.23% <0.00%> (-5.77%)` | :arrow_down: |
| [pkg/api/router/webhook.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9yb3V0ZXIvd2ViaG9vay5nbw==) | `0.00% <0.00%> (ø)` | |
| [pkg/config/config.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `85.71% <ø> (ø)` | |
| [pkg/api/validation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL2FwaXNpeF9yb3V0ZS5nbw==) | `28.33% <28.33%> (ø)` | |
| [pkg/api/validation/utils.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS92YWxpZGF0aW9uL3V0aWxzLmdv) | `35.29% <35.29%> (ø)` | |
| [pkg/api/server.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573/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-cGtnL2FwaS9zZXJ2ZXIuZ28=) | `70.27% <36.36%> (-5.60%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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/573?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 [d8854c3...cacdc3b](https://codecov.io/gh/apache/apisix-ingress-controller/pull/573?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] fgksgf commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r695322438
##########
File path: pkg/api/validation/utils.go
##########
@@ -0,0 +1,89 @@
+// 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 validation
+
+import (
+ "context"
+ "fmt"
+ "sync"
+
+ "github.com/xeipuuv/gojsonschema"
+
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
+ "github.com/apache/apisix-ingress-controller/pkg/config"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
+)
+
+var (
+ once sync.Once
+ onceErr error
+ schemaClient apisix.Schema
+)
+
+// GetSchemaClient returns a Schema client in the singleton way.
+// It can query the schema of objects from APISIX.
+func GetSchemaClient() (apisix.Schema, error) {
Review comment:
I didn't get your point, it seems that the singleton will not interfere with the synchronization process.
--
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 pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-908354204
> > reply: [#573 (comment)](https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698118062)
>
> If we provide default TLS Certificates, they are not signed by CA, and the webhook still can't work. We can accomplish this after we have cert-manager.
>
> I can help integrate cert-manager after webhooks are implemented.
We may use `CertificateSigningRequest`, and give documentation to guide people.
--
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] fgksgf commented on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-908365726
Webhook E2E test:
--
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] fgksgf commented on pull request #573: [WIP] Implement the admission server
Posted by GitBox <gi...@apache.org>.
fgksgf commented on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-899005676
> BTW, Need to add e2e test case to cover the logic of validating with the schema from APISIX.
Ok, where should I put the code, `test/e2e/features/` or `test/e2e/ingress/`?
--
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] gxthrj merged pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
gxthrj merged pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573
--
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] gxthrj merged pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
gxthrj merged pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573
--
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] gxthrj commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r695877878
##########
File path: test/e2e/scaffold/scaffold.go
##########
@@ -281,7 +281,8 @@ func (s *Scaffold) APISIXGatewayServiceEndpoint() string {
func (s *Scaffold) beforeEach() {
var err error
- s.namespace = fmt.Sprintf("ingress-apisix-e2e-tests-%s-%d", s.opts.Name, time.Now().Nanosecond())
+ s.namespace = "ingress-apisix"
+ //s.namespace = fmt.Sprintf("ingress-apisix-e2e-tests-%s-%d", s.opts.Name, time.Now().Nanosecond())
Review comment:
In e2e, every test scene has a namespace alone. It cannot be a constant, because test cases may run concurrently.
The order of execution should be to generate namespace first and then certs.
--
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 change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698493769
##########
File path: pkg/api/validation/utils.go
##########
@@ -0,0 +1,89 @@
+// 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 validation
+
+import (
+ "context"
+ "fmt"
+ "sync"
+
+ "github.com/xeipuuv/gojsonschema"
+
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
+ "github.com/apache/apisix-ingress-controller/pkg/config"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
+)
+
+var (
+ once sync.Once
+ onceErr error
+ schemaClient apisix.Schema
+)
+
+// GetSchemaClient returns a Schema client in the singleton way.
+// It can query the schema of objects from APISIX.
+func GetSchemaClient() (apisix.Schema, error) {
Review comment:
Just ignore my comment as long as the schema cache is refreshed periodically.
--
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] fgksgf commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698236045
##########
File path: pkg/api/server.go
##########
@@ -67,13 +99,51 @@ func NewServer(cfg *config.Config) (*Server, error) {
func (srv *Server) Run(stopCh <-chan struct{}) error {
go func() {
<-stopCh
- if err := srv.httpListener.Close(); err != nil {
- log.Errorf("failed to close http listener: %s", err)
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+
+ closeHttp := make(chan struct{})
+ go srv.closeHttpServer(closeHttp)
Review comment:
should I put the timeout into the config?
--
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] fgksgf commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r693596269
##########
File path: pkg/api/server.go
##########
@@ -41,23 +46,43 @@ func NewServer(cfg *config.Config) (*Server, error) {
return nil, err
}
gin.SetMode(gin.ReleaseMode)
- router := gin.New()
- router.Use(gin.Recovery(), gin.Logger())
- apirouter.Mount(router)
+ httpServer := gin.New()
+ httpServer.Use(gin.Recovery(), gin.Logger())
+ apirouter.Mount(httpServer)
srv := &Server{
- router: router,
+ httpServer: httpServer,
httpListener: httpListener,
}
-
if cfg.EnableProfiling {
srv.pprofMu = new(http.ServeMux)
srv.pprofMu.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
srv.pprofMu.HandleFunc("/debug/pprof/profile", pprof.Profile)
srv.pprofMu.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
srv.pprofMu.HandleFunc("/debug/pprof/trace", pprof.Trace)
srv.pprofMu.HandleFunc("/debug/pprof/", pprof.Index)
- router.GET("/debug/pprof/*profile", gin.WrapF(srv.pprofMu.ServeHTTP))
+ httpServer.GET("/debug/pprof/*profile", gin.WrapF(srv.pprofMu.ServeHTTP))
+ }
+
+ cert, err := tls.LoadX509KeyPair(cfg.CertFilePath, cfg.KeyFilePath)
Review comment:
Because `cfg.CertFilePath` and `cfg.KeyFilePath` have default values.
--
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] fgksgf commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698240555
##########
File path: pkg/api/server.go
##########
@@ -12,14 +12,21 @@
// 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 api
import (
+ "context"
+ "crypto/tls"
"net"
"net/http"
"net/http/pprof"
+ "time"
+
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
Review comment:
`make lint` should identify this, but it didn't
--
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 change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698493769
##########
File path: pkg/api/validation/utils.go
##########
@@ -0,0 +1,89 @@
+// 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 validation
+
+import (
+ "context"
+ "fmt"
+ "sync"
+
+ "github.com/xeipuuv/gojsonschema"
+
+ "github.com/apache/apisix-ingress-controller/pkg/apisix"
+ "github.com/apache/apisix-ingress-controller/pkg/config"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
+)
+
+var (
+ once sync.Once
+ onceErr error
+ schemaClient apisix.Schema
+)
+
+// GetSchemaClient returns a Schema client in the singleton way.
+// It can query the schema of objects from APISIX.
+func GetSchemaClient() (apisix.Schema, error) {
Review comment:
Just ignore my comment as long as the schema cache is refreshed periodically.
##########
File path: pkg/api/server.go
##########
@@ -67,13 +99,51 @@ func NewServer(cfg *config.Config) (*Server, error) {
func (srv *Server) Run(stopCh <-chan struct{}) error {
go func() {
<-stopCh
- if err := srv.httpListener.Close(); err != nil {
- log.Errorf("failed to close http listener: %s", err)
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+
+ closeHttp := make(chan struct{})
+ go srv.closeHttpServer(closeHttp)
Review comment:
> should I put the timeout into the config?
Don't have to. And I think the code is correct.
--
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 pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-908354204
--
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] gxthrj commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698025707
##########
File path: samples/deploy/admission/webhook-registration.yaml
##########
@@ -0,0 +1,40 @@
+#
+# 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.
+#
+
+apiVersion: admissionregistration.k8s.io/v1beta1
+kind: ValidatingWebhookConfiguration
+metadata:
+ name: apisix-validations
+ labels:
+ app: apisixroute-plugin-validator-webhook
+ kind: validating
+webhooks:
+ - name: apisixroute-plugin-validator-webhook
+ clientConfig:
+ service:
+ name: apisix-admission-server
+ namespace: ingress-apisix
+ port: 8443
+ path: "/validation/apisixRoute/plugin"
+ caBundle: ${CA_BUNDLE}
+ rules:
+ - operations: [ "CREATE", "UPDATE" ]
+ apiGroups: ["apisix.apache.org"]
+ apiVersions: ["*"]
+ resources: ["apisixroutes"]
+ timeoutSeconds: 30
+ failurePolicy: Ignore
Review comment:
`failurePolicy` must be `Fail`
--
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] fgksgf commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698235840
##########
File path: pkg/api/server.go
##########
@@ -67,13 +99,51 @@ func NewServer(cfg *config.Config) (*Server, error) {
func (srv *Server) Run(stopCh <-chan struct{}) error {
go func() {
<-stopCh
- if err := srv.httpListener.Close(); err != nil {
- log.Errorf("failed to close http listener: %s", err)
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+
+ closeHttp := make(chan struct{})
+ go srv.closeHttpServer(closeHttp)
Review comment:
How about this:
```go
closed := make(chan struct{}, 2)
go srv.closeHttpServer(closed)
go srv.closeAdmissionServer(closed)
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
cnt := 2
for cnt > 0 {
select {
case <-ctx.Done():
log.Errorf("close servers timeout")
return
case <-closed:
cnt--
log.Debug("close a server")
}
}
```
--
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] fgksgf edited a comment on pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
fgksgf edited a comment on pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#issuecomment-908365726
Webhook E2E test:
--
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] gxthrj commented on a change in pull request #573: feat: Implement the admission server and a validation webhook for plugins
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #573:
URL: https://github.com/apache/apisix-ingress-controller/pull/573#discussion_r698014526
##########
File path: test/e2e/scaffold/ingress.go
##########
@@ -265,25 +272,67 @@ spec:
- --apisix-route-version
- %s
- --watch-endpointslices
+ volumeMounts:
+ - name: webhook-certs
+ mountPath: /etc/webhook/certs
+ readOnly: true
+ volumes:
+ - name: webhook-certs
+ secret:
+ secretName: %s
serviceAccount: ingress-apisix-e2e-test-service-account
`
+ _ingressAPISIXAdmissionService = `
+apiVersion: v1
+kind: Service
+metadata:
+ name: webhook
+ namespace: %s
+spec:
+ ports:
+ - name: https
+ protocol: TCP
+ port: 443
Review comment:
443 -> 8443
##########
File path: test/e2e/scaffold/ingress.go
##########
@@ -245,6 +247,9 @@ spec:
- containerPort: 8080
name: "http"
protocol: "TCP"
+ - containerPort: 443
Review comment:
443 -> 8443
--
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