You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "ronething (via GitHub)" <gi...@apache.org> on 2025/09/22 09:32:03 UTC
[PR] feat: add webhook for ingressclass and gateway [apisix-ingress-controller]
ronething opened a new pull request, #2572:
URL: https://github.com/apache/apisix-ingress-controller/pull/2572
<!-- Please answer these questions before submitting a pull request -->
### Type of change:
<!-- Please delete options that are not relevant. -->
<!-- Select all the options from below that matches the type your PR best -->
- [ ] Bugfix
- [ ] New feature provided
- [ ] Improve performance
- [ ] Backport patches
- [ ] Documentation
- [ ] Refactor
- [ ] Chore
- [ ] CI/CD or Tests
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
### Pre-submission checklist:
<!--
Please follow the requirements:
1. Use Draft if the PR is not ready to be reviewed
2. Test is required for the feat/fix PR, unless you have a good reason
3. Doc is required for the feat PR
4. Use a new commit to resolve review instead of `push -f`
5. Use "request review" to notify the reviewer once you have resolved the review
-->
- [ ] Did you explain what problem does this PR solve? Or what new features have been added?
- [ ] Have you added corresponding test cases?
- [ ] Have you modified the corresponding document?
- [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix-ingress-controller#community) first**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add webhook for ingressclass and gateway [apisix-ingress-controller]
Posted by "ronething (via GitHub)" <gi...@apache.org>.
ronething commented on code in PR #2572:
URL: https://github.com/apache/apisix-ingress-controller/pull/2572#discussion_r2371595321
##########
internal/webhook/v1/gateway_webhook.go:
##########
@@ -0,0 +1,118 @@
+// 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 v1
+
+import (
+ "context"
+ "fmt"
+
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/apimachinery/pkg/runtime"
+ ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ "sigs.k8s.io/controller-runtime/pkg/webhook"
+ "sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+ gatewaynetworkingk8siov1 "sigs.k8s.io/gateway-api/apis/v1"
+
+ v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1"
+ "github.com/apache/apisix-ingress-controller/internal/controller"
+)
+
+// nolint:unused
+// log is for logging in this package.
+var gatewaylog = logf.Log.WithName("gateway-resource")
+
+// SetupGatewayWebhookWithManager registers the webhook for Gateway in the manager.
+func SetupGatewayWebhookWithManager(mgr ctrl.Manager) error {
+ return ctrl.NewWebhookManagedBy(mgr).For(&gatewaynetworkingk8siov1.Gateway{}).
+ WithValidator(&GatewayCustomValidator{Client: mgr.GetClient()}).
+ Complete()
+}
+
+// NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
+// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
+// +kubebuilder:webhook:path=/validate-gateway-networking-k8s-io-v1-gateway,mutating=false,failurePolicy=fail,sideEffects=None,groups=gateway.networking.k8s.io,resources=gateways,verbs=create;update,versions=v1,name=vgateway-v1.kb.io,admissionReviewVersions=v1
+
+// GatewayCustomValidator struct is responsible for validating the Gateway resource
+// when it is created, updated, or deleted.
+//
+// NOTE: The +kubebuilder:object:generate=false marker prevents controller-gen from generating DeepCopy methods,
+// as this struct is used only for temporary operations and does not need to be deeply copied.
+type GatewayCustomValidator struct {
+ Client client.Client
+}
+
+var _ webhook.CustomValidator = &GatewayCustomValidator{}
+
+// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type Gateway.
+func (v *GatewayCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+ gateway, ok := obj.(*gatewaynetworkingk8siov1.Gateway)
Review Comment:
Sure. I'll handle it later
--
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
Re: [PR] feat: add webhook for ingressclass and gateway [apisix-ingress-controller]
Posted by "ronething (via GitHub)" <gi...@apache.org>.
ronething commented on code in PR #2572:
URL: https://github.com/apache/apisix-ingress-controller/pull/2572#discussion_r2372191210
##########
internal/webhook/v1/gateway_webhook.go:
##########
@@ -0,0 +1,130 @@
+// 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 v1
+
+import (
+ "context"
+ "fmt"
+
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/apimachinery/pkg/runtime"
+ ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ "sigs.k8s.io/controller-runtime/pkg/webhook"
+ "sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+ gatewaynetworkingk8siov1 "sigs.k8s.io/gateway-api/apis/v1"
+
+ v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1"
+ "github.com/apache/apisix-ingress-controller/internal/controller/config"
+ internaltypes "github.com/apache/apisix-ingress-controller/internal/types"
+)
+
+// nolint:unused
+// log is for logging in this package.
+var gatewaylog = logf.Log.WithName("gateway-resource")
+
+// SetupGatewayWebhookWithManager registers the webhook for Gateway in the manager.
+func SetupGatewayWebhookWithManager(mgr ctrl.Manager) error {
+ return ctrl.NewWebhookManagedBy(mgr).For(&gatewaynetworkingk8siov1.Gateway{}).
+ WithValidator(&GatewayCustomValidator{Client: mgr.GetClient()}).
+ Complete()
+}
+
+// NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
+// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
+// +kubebuilder:webhook:path=/validate-gateway-networking-k8s-io-v1-gateway,mutating=false,failurePolicy=fail,sideEffects=None,groups=gateway.networking.k8s.io,resources=gateways,verbs=create;update,versions=v1,name=vgateway-v1.kb.io,admissionReviewVersions=v1
+
+// GatewayCustomValidator struct is responsible for validating the Gateway resource
+// when it is created, updated, or deleted.
+//
+// NOTE: The +kubebuilder:object:generate=false marker prevents controller-gen from generating DeepCopy methods,
+// as this struct is used only for temporary operations and does not need to be deeply copied.
+type GatewayCustomValidator struct {
+ Client client.Client
+}
+
+var _ webhook.CustomValidator = &GatewayCustomValidator{}
+
+// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type Gateway.
+func (v *GatewayCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+ gateway, ok := obj.(*gatewaynetworkingk8siov1.Gateway)
+ if !ok {
+ return nil, fmt.Errorf("expected a Gateway object but got %T", obj)
+ }
+ gatewaylog.Info("Validation for Gateway upon creation", "name", gateway.GetName())
+
+ warnings := v.warnIfMissingGatewayProxyForGateway(ctx, gateway)
+
+ return warnings, nil
+}
+
+// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type Gateway.
+func (v *GatewayCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
+ gateway, ok := newObj.(*gatewaynetworkingk8siov1.Gateway)
+ if !ok {
+ return nil, fmt.Errorf("expected a Gateway object for the newObj but got %T", newObj)
+ }
+ gatewaylog.Info("Validation for Gateway upon update", "name", gateway.GetName())
+
+ warnings := v.warnIfMissingGatewayProxyForGateway(ctx, gateway)
+
+ return warnings, nil
+}
+
+// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type Gateway.
+func (v *GatewayCustomValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) {
+ return nil, nil
+}
+
+func (v *GatewayCustomValidator) warnIfMissingGatewayProxyForGateway(ctx context.Context, gateway *gatewaynetworkingk8siov1.Gateway) admission.Warnings {
+ var warnings admission.Warnings
+
+ // get gateway class
+ gatewayClass := &gatewaynetworkingk8siov1.GatewayClass{}
+ if err := v.Client.Get(ctx, client.ObjectKey{Name: string(gateway.Spec.GatewayClassName)}, gatewayClass); err != nil {
Review Comment:
> Is this a request to the cache or a direct request to the apiserver?
to the cache.
1. `Manager` embeds cluster.Cluster, manager.GetClient() delegates to cluster.GetClient()
- https://github.com/kubernetes-sigs/controller-runtime/blob/961fc2c233d64e40b5bdf449f12bfa22cf2d7b28/pkg/manager/manager.go#L54
- https://github.com/kubernetes-sigs/controller-runtime/blob/4fd4f6e7116453ea4e07a2cdd8ba0d38ad64680f/pkg/cluster/internal.go#L70-L72
2. Cache reader is automatically set when creating the client
- https://github.com/kubernetes-sigs/controller-runtime/blob/961fc2c233d64e40b5bdf449f12bfa22cf2d7b28/pkg/cluster/cluster.go#L209-L216
3. Read behavior is cache-first (https://github.com/kubernetes-sigs/controller-runtime/blob/961fc2c233d64e40b5bdf449f12bfa22cf2d7b28/pkg/client/client.go#L82-L89)
`Client.Get` Operation: https://github.com/kubernetes-sigs/controller-runtime/blob/948d554cd9c7692f21139384afaafaea8524626f/pkg/client/client.go#L342-L349
--
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
Re: [PR] feat: add webhook for ingressclass and gateway [apisix-ingress-controller]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #2572:
URL: https://github.com/apache/apisix-ingress-controller/pull/2572#discussion_r2374721262
##########
test/e2e/ingress/webhook.go:
##########
@@ -28,7 +28,7 @@ import (
"github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
)
-var _ = Describe("Test Ingress Webhook", Label("networking.k8s.io", "ingress"), func() {
+var _ = Describe("Test Ingress Webhook", Label("webhook"), func() {
Review Comment:
How about creating a separate directory for webhooks?
There are different resources inside.
--
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
Re: [PR] feat: add webhook for ingressclass and gateway [apisix-ingress-controller]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #2572:
URL: https://github.com/apache/apisix-ingress-controller/pull/2572#discussion_r2371570123
##########
internal/webhook/v1/gateway_webhook.go:
##########
@@ -0,0 +1,118 @@
+// 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 v1
+
+import (
+ "context"
+ "fmt"
+
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/apimachinery/pkg/runtime"
+ ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ "sigs.k8s.io/controller-runtime/pkg/webhook"
+ "sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+ gatewaynetworkingk8siov1 "sigs.k8s.io/gateway-api/apis/v1"
+
+ v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1"
+ "github.com/apache/apisix-ingress-controller/internal/controller"
+)
+
+// nolint:unused
+// log is for logging in this package.
+var gatewaylog = logf.Log.WithName("gateway-resource")
+
+// SetupGatewayWebhookWithManager registers the webhook for Gateway in the manager.
+func SetupGatewayWebhookWithManager(mgr ctrl.Manager) error {
+ return ctrl.NewWebhookManagedBy(mgr).For(&gatewaynetworkingk8siov1.Gateway{}).
+ WithValidator(&GatewayCustomValidator{Client: mgr.GetClient()}).
+ Complete()
+}
+
+// NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
+// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
+// +kubebuilder:webhook:path=/validate-gateway-networking-k8s-io-v1-gateway,mutating=false,failurePolicy=fail,sideEffects=None,groups=gateway.networking.k8s.io,resources=gateways,verbs=create;update,versions=v1,name=vgateway-v1.kb.io,admissionReviewVersions=v1
+
+// GatewayCustomValidator struct is responsible for validating the Gateway resource
+// when it is created, updated, or deleted.
+//
+// NOTE: The +kubebuilder:object:generate=false marker prevents controller-gen from generating DeepCopy methods,
+// as this struct is used only for temporary operations and does not need to be deeply copied.
+type GatewayCustomValidator struct {
+ Client client.Client
+}
+
+var _ webhook.CustomValidator = &GatewayCustomValidator{}
+
+// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type Gateway.
+func (v *GatewayCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+ gateway, ok := obj.(*gatewaynetworkingk8siov1.Gateway)
Review Comment:
Perhaps we only need to issue warnings for the resources managed by this ingress.
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add webhook for ingressclass and gateway [apisix-ingress-controller]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #2572:
URL: https://github.com/apache/apisix-ingress-controller/pull/2572#discussion_r2371570123
##########
internal/webhook/v1/gateway_webhook.go:
##########
@@ -0,0 +1,118 @@
+// 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 v1
+
+import (
+ "context"
+ "fmt"
+
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/apimachinery/pkg/runtime"
+ ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ "sigs.k8s.io/controller-runtime/pkg/webhook"
+ "sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+ gatewaynetworkingk8siov1 "sigs.k8s.io/gateway-api/apis/v1"
+
+ v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1"
+ "github.com/apache/apisix-ingress-controller/internal/controller"
+)
+
+// nolint:unused
+// log is for logging in this package.
+var gatewaylog = logf.Log.WithName("gateway-resource")
+
+// SetupGatewayWebhookWithManager registers the webhook for Gateway in the manager.
+func SetupGatewayWebhookWithManager(mgr ctrl.Manager) error {
+ return ctrl.NewWebhookManagedBy(mgr).For(&gatewaynetworkingk8siov1.Gateway{}).
+ WithValidator(&GatewayCustomValidator{Client: mgr.GetClient()}).
+ Complete()
+}
+
+// NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
+// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
+// +kubebuilder:webhook:path=/validate-gateway-networking-k8s-io-v1-gateway,mutating=false,failurePolicy=fail,sideEffects=None,groups=gateway.networking.k8s.io,resources=gateways,verbs=create;update,versions=v1,name=vgateway-v1.kb.io,admissionReviewVersions=v1
+
+// GatewayCustomValidator struct is responsible for validating the Gateway resource
+// when it is created, updated, or deleted.
+//
+// NOTE: The +kubebuilder:object:generate=false marker prevents controller-gen from generating DeepCopy methods,
+// as this struct is used only for temporary operations and does not need to be deeply copied.
+type GatewayCustomValidator struct {
+ Client client.Client
+}
+
+var _ webhook.CustomValidator = &GatewayCustomValidator{}
+
+// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type Gateway.
+func (v *GatewayCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+ gateway, ok := obj.(*gatewaynetworkingk8siov1.Gateway)
Review Comment:
Perhaps we only need to issue warnings for the resources we manage.
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add webhook for ingressclass and gateway [apisix-ingress-controller]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2572:
URL: https://github.com/apache/apisix-ingress-controller/pull/2572#discussion_r2371417699
##########
test/e2e/gatewayapi/webhook.go:
##########
@@ -0,0 +1,94 @@
+// 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 gatewayapi
+
+import (
+ "fmt"
+ "time"
+
+ . "github.com/onsi/ginkgo/v2"
+ . "github.com/onsi/gomega"
+
+ "github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
+)
+
+var _ = Describe("Test Gateway Webhook", Label("networking.k8s.io", "gateway"), func() {
+ s := scaffold.NewScaffold(scaffold.Options{
+ Name: "gateway-webhook-test",
+ EnableWebhook: true,
+ })
+
+ It("should warn when referenced GatewayProxy does not exist on create and update", func() {
+ By("creating GatewayClass with controller name")
+ err := s.CreateResourceFromString(s.GetGatewayClassYaml())
+ Expect(err).ShouldNot(HaveOccurred())
+
+ time.Sleep(2 * time.Second)
+
+ By("creating Gateway referencing a missing GatewayProxy")
+ missingName := "missing-proxy"
+ gwYAML := `
+apiVersion: gateway.networking.k8s.io/v1
+kind: Gateway
+metadata:
+ name: %s
+spec:
+ gatewayClassName: %s
+ listeners:
+ - name: http1
+ protocol: HTTP
+ port: 80
+ infrastructure:
+ parametersRef:
+ group: apisix.apache.org
+ kind: GatewayProxy
+ name: %s
+`
+
+ output, err := s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(gwYAML, s.Namespace(), s.Namespace(), missingName))
+ Expect(err).ShouldNot(HaveOccurred())
+ Expect(output).To(ContainSubstring(fmt.Sprintf("Warning: Referenced GatewayProxy '%s/%s' not found.", s.Namespace(), missingName)))
+
+ time.Sleep(2 * time.Second)
+
+ By("updating Gateway to reference another missing GatewayProxy")
+ missingName2 := "missing-proxy-2"
+ output, err = s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(gwYAML, s.Namespace(), s.Namespace(), missingName2))
Review Comment:
This appears to be creating a new resource with the same name rather than updating the existing Gateway. Consider using an update operation or a different resource name to make the test intention clearer.
```suggestion
output, err = s.ApplyResourceFromStringAndGetOutput(fmt.Sprintf(gwYAML, s.Namespace(), s.Namespace(), missingName2))
```
##########
test/e2e/ingress/ingressclass_webhook.go:
##########
@@ -0,0 +1,83 @@
+// 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"
+ "time"
+
+ . "github.com/onsi/ginkgo/v2"
+ . "github.com/onsi/gomega"
+
+ "github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
+)
+
+var _ = Describe("Test IngressClass Webhook", Label("networking.k8s.io", "ingressclass"), func() {
+ s := scaffold.NewScaffold(scaffold.Options{
+ Name: "ingressclass-webhook-test",
+ EnableWebhook: true,
+ })
+
+ Context("IngressClass Validation", func() {
+ It("should warn when referenced GatewayProxy does not exist on create and update", func() {
+ By("creating IngressClass referencing a missing GatewayProxy")
+ missingName := "missing-proxy"
+ icYAML := `
+apiVersion: networking.k8s.io/v1
+kind: IngressClass
+metadata:
+ name: apisix-with-missing
+spec:
+ controller: "%s"
+ parameters:
+ apiGroup: "apisix.apache.org"
+ kind: "GatewayProxy"
+ name: "%s"
+ namespace: "%s"
+ scope: "Namespace"
+`
+
+ output, err := s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(icYAML, s.GetControllerName(), missingName, s.Namespace()))
+ Expect(err).ShouldNot(HaveOccurred())
+ Expect(output).To(ContainSubstring(fmt.Sprintf("Warning: Referenced GatewayProxy '%s/%s' not found.", s.Namespace(), missingName)))
+
+ time.Sleep(2 * time.Second)
+
+ By("updating IngressClass to reference another missing GatewayProxy")
+ missingName2 := "missing-proxy-2"
+ output, err = s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(icYAML, s.GetControllerName(), missingName2, s.Namespace()))
Review Comment:
This appears to be creating a new resource with the same name 'apisix-with-missing' rather than updating the existing one. Consider using an update operation or a different resource name to make the test intention clearer.
```suggestion
output, err = s.UpdateResourceFromStringAndGetOutput(fmt.Sprintf(icYAML, s.GetControllerName(), missingName2, s.Namespace()))
```
--
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
Re: [PR] feat: add webhook for ingressclass and gateway [apisix-ingress-controller]
Posted by "ronething (via GitHub)" <gi...@apache.org>.
ronething commented on code in PR #2572:
URL: https://github.com/apache/apisix-ingress-controller/pull/2572#discussion_r2374764083
##########
test/e2e/ingress/webhook.go:
##########
@@ -28,7 +28,7 @@ import (
"github.com/apache/apisix-ingress-controller/test/e2e/scaffold"
)
-var _ = Describe("Test Ingress Webhook", Label("networking.k8s.io", "ingress"), func() {
+var _ = Describe("Test Ingress Webhook", Label("webhook"), func() {
Review Comment:
Sure, I think we can handle this matter separately, in the future PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add webhook for ingressclass and gateway [apisix-ingress-controller]
Posted by "ronething (via GitHub)" <gi...@apache.org>.
ronething merged PR #2572:
URL: https://github.com/apache/apisix-ingress-controller/pull/2572
--
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
Re: [PR] feat: add webhook for ingressclass and gateway [apisix-ingress-controller]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #2572:
URL: https://github.com/apache/apisix-ingress-controller/pull/2572#discussion_r2371570123
##########
internal/webhook/v1/gateway_webhook.go:
##########
@@ -0,0 +1,118 @@
+// 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 v1
+
+import (
+ "context"
+ "fmt"
+
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/apimachinery/pkg/runtime"
+ ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ "sigs.k8s.io/controller-runtime/pkg/webhook"
+ "sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+ gatewaynetworkingk8siov1 "sigs.k8s.io/gateway-api/apis/v1"
+
+ v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1"
+ "github.com/apache/apisix-ingress-controller/internal/controller"
+)
+
+// nolint:unused
+// log is for logging in this package.
+var gatewaylog = logf.Log.WithName("gateway-resource")
+
+// SetupGatewayWebhookWithManager registers the webhook for Gateway in the manager.
+func SetupGatewayWebhookWithManager(mgr ctrl.Manager) error {
+ return ctrl.NewWebhookManagedBy(mgr).For(&gatewaynetworkingk8siov1.Gateway{}).
+ WithValidator(&GatewayCustomValidator{Client: mgr.GetClient()}).
+ Complete()
+}
+
+// NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
+// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
+// +kubebuilder:webhook:path=/validate-gateway-networking-k8s-io-v1-gateway,mutating=false,failurePolicy=fail,sideEffects=None,groups=gateway.networking.k8s.io,resources=gateways,verbs=create;update,versions=v1,name=vgateway-v1.kb.io,admissionReviewVersions=v1
+
+// GatewayCustomValidator struct is responsible for validating the Gateway resource
+// when it is created, updated, or deleted.
+//
+// NOTE: The +kubebuilder:object:generate=false marker prevents controller-gen from generating DeepCopy methods,
+// as this struct is used only for temporary operations and does not need to be deeply copied.
+type GatewayCustomValidator struct {
+ Client client.Client
+}
+
+var _ webhook.CustomValidator = &GatewayCustomValidator{}
+
+// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type Gateway.
+func (v *GatewayCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+ gateway, ok := obj.(*gatewaynetworkingk8siov1.Gateway)
Review Comment:
Perhaps we only need to issue warnings for the resources controller manage.
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add webhook for ingressclass and gateway [apisix-ingress-controller]
Posted by "bzp2010 (via GitHub)" <gi...@apache.org>.
bzp2010 commented on code in PR #2572:
URL: https://github.com/apache/apisix-ingress-controller/pull/2572#discussion_r2371849468
##########
internal/webhook/v1/gateway_webhook.go:
##########
@@ -0,0 +1,130 @@
+// 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 v1
+
+import (
+ "context"
+ "fmt"
+
+ k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/apimachinery/pkg/runtime"
+ ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ logf "sigs.k8s.io/controller-runtime/pkg/log"
+ "sigs.k8s.io/controller-runtime/pkg/webhook"
+ "sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+ gatewaynetworkingk8siov1 "sigs.k8s.io/gateway-api/apis/v1"
+
+ v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1"
+ "github.com/apache/apisix-ingress-controller/internal/controller/config"
+ internaltypes "github.com/apache/apisix-ingress-controller/internal/types"
+)
+
+// nolint:unused
+// log is for logging in this package.
+var gatewaylog = logf.Log.WithName("gateway-resource")
+
+// SetupGatewayWebhookWithManager registers the webhook for Gateway in the manager.
+func SetupGatewayWebhookWithManager(mgr ctrl.Manager) error {
+ return ctrl.NewWebhookManagedBy(mgr).For(&gatewaynetworkingk8siov1.Gateway{}).
+ WithValidator(&GatewayCustomValidator{Client: mgr.GetClient()}).
+ Complete()
+}
+
+// NOTE: The 'path' attribute must follow a specific pattern and should not be modified directly here.
+// Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
+// +kubebuilder:webhook:path=/validate-gateway-networking-k8s-io-v1-gateway,mutating=false,failurePolicy=fail,sideEffects=None,groups=gateway.networking.k8s.io,resources=gateways,verbs=create;update,versions=v1,name=vgateway-v1.kb.io,admissionReviewVersions=v1
+
+// GatewayCustomValidator struct is responsible for validating the Gateway resource
+// when it is created, updated, or deleted.
+//
+// NOTE: The +kubebuilder:object:generate=false marker prevents controller-gen from generating DeepCopy methods,
+// as this struct is used only for temporary operations and does not need to be deeply copied.
+type GatewayCustomValidator struct {
+ Client client.Client
+}
+
+var _ webhook.CustomValidator = &GatewayCustomValidator{}
+
+// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type Gateway.
+func (v *GatewayCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+ gateway, ok := obj.(*gatewaynetworkingk8siov1.Gateway)
+ if !ok {
+ return nil, fmt.Errorf("expected a Gateway object but got %T", obj)
+ }
+ gatewaylog.Info("Validation for Gateway upon creation", "name", gateway.GetName())
+
+ warnings := v.warnIfMissingGatewayProxyForGateway(ctx, gateway)
+
+ return warnings, nil
+}
+
+// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type Gateway.
+func (v *GatewayCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
+ gateway, ok := newObj.(*gatewaynetworkingk8siov1.Gateway)
+ if !ok {
+ return nil, fmt.Errorf("expected a Gateway object for the newObj but got %T", newObj)
+ }
+ gatewaylog.Info("Validation for Gateway upon update", "name", gateway.GetName())
+
+ warnings := v.warnIfMissingGatewayProxyForGateway(ctx, gateway)
+
+ return warnings, nil
+}
+
+// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type Gateway.
+func (v *GatewayCustomValidator) ValidateDelete(_ context.Context, obj runtime.Object) (admission.Warnings, error) {
+ return nil, nil
+}
+
+func (v *GatewayCustomValidator) warnIfMissingGatewayProxyForGateway(ctx context.Context, gateway *gatewaynetworkingk8siov1.Gateway) admission.Warnings {
+ var warnings admission.Warnings
+
+ // get gateway class
+ gatewayClass := &gatewaynetworkingk8siov1.GatewayClass{}
+ if err := v.Client.Get(ctx, client.ObjectKey{Name: string(gateway.Spec.GatewayClassName)}, gatewayClass); err != nil {
Review Comment:
Is this a request to the cache or a direct request to the apiserver?
--
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