This is an automated email from the ASF dual-hosted git repository. ronething pushed a commit to branch feat/add_checker in repository https://gitbox.apache.org/repos/asf/apisix-ingress-controller.git
commit b306394fe3b0fca76a75179fb260c989868fe2d3 Author: Ashing Zheng <[email protected]> AuthorDate: Sun Sep 28 15:00:42 2025 +0800 feat: add secret/service resource checker for webhook Signed-off-by: Ashing Zheng <[email protected]> --- internal/manager/webhooks.go | 6 ++ internal/webhook/v1/gateway_webhook.go | 64 ++++++++++- internal/webhook/v1/gateway_webhook_test.go | 113 ++++++++++++++++++++ internal/webhook/v1/grpcroute_webhook.go | 146 ++++++++++++++++++++++++++ internal/webhook/v1/grpcroute_webhook_test.go | 92 ++++++++++++++++ internal/webhook/v1/httproute_webhook.go | 146 ++++++++++++++++++++++++++ internal/webhook/v1/httproute_webhook_test.go | 114 ++++++++++++++++++++ internal/webhook/v1/ingress_webhook.go | 79 +++++++++++++- internal/webhook/v1/ingress_webhook_test.go | 85 ++++++++++++++- 9 files changed, 834 insertions(+), 11 deletions(-) diff --git a/internal/manager/webhooks.go b/internal/manager/webhooks.go index 6907d762..424fe67c 100644 --- a/internal/manager/webhooks.go +++ b/internal/manager/webhooks.go @@ -38,6 +38,12 @@ func setupWebhooks(_ context.Context, mgr manager.Manager) error { if err := webhookv1.SetupGatewayProxyWebhookWithManager(mgr); err != nil { return err } + if err := webhookv1.SetupHTTPRouteWebhookWithManager(mgr); err != nil { + return err + } + if err := webhookv1.SetupGRPCRouteWebhookWithManager(mgr); err != nil { + return err + } if err := webhookv1.SetupApisixConsumerWebhookWithManager(mgr); err != nil { return err } diff --git a/internal/webhook/v1/gateway_webhook.go b/internal/webhook/v1/gateway_webhook.go index e2c11ff4..07bdf32f 100644 --- a/internal/webhook/v1/gateway_webhook.go +++ b/internal/webhook/v1/gateway_webhook.go @@ -19,8 +19,10 @@ import ( "context" "fmt" + corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -31,6 +33,7 @@ import ( 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" + "github.com/apache/apisix-ingress-controller/internal/webhook/v1/reference" ) // nolint:unused @@ -40,7 +43,7 @@ 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()}). + WithValidator(NewGatewayCustomValidator(mgr.GetClient())). Complete() } @@ -54,11 +57,19 @@ func SetupGatewayWebhookWithManager(mgr ctrl.Manager) error { // 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 + Client client.Client + checker reference.Checker } var _ webhook.CustomValidator = &GatewayCustomValidator{} +func NewGatewayCustomValidator(c client.Client) *GatewayCustomValidator { + return &GatewayCustomValidator{ + Client: c, + checker: reference.NewChecker(c, gatewaylog), + } +} + // 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) @@ -68,6 +79,7 @@ func (v *GatewayCustomValidator) ValidateCreate(ctx context.Context, obj runtime gatewaylog.Info("Validation for Gateway upon creation", "name", gateway.GetName()) warnings := v.warnIfMissingGatewayProxyForGateway(ctx, gateway) + warnings = append(warnings, v.collectReferenceWarnings(ctx, gateway)...) return warnings, nil } @@ -81,6 +93,7 @@ func (v *GatewayCustomValidator) ValidateUpdate(ctx context.Context, oldObj, new gatewaylog.Info("Validation for Gateway upon update", "name", gateway.GetName()) warnings := v.warnIfMissingGatewayProxyForGateway(ctx, gateway) + warnings = append(warnings, v.collectReferenceWarnings(ctx, gateway)...) return warnings, nil } @@ -90,6 +103,53 @@ func (v *GatewayCustomValidator) ValidateDelete(_ context.Context, obj runtime.O return nil, nil } +func (v *GatewayCustomValidator) collectReferenceWarnings(ctx context.Context, gateway *gatewaynetworkingk8siov1.Gateway) admission.Warnings { + if gateway == nil { + return nil + } + + var warnings admission.Warnings + secretVisited := make(map[types.NamespacedName]struct{}) + + addSecretWarning := func(nn types.NamespacedName) { + if nn.Name == "" || nn.Namespace == "" { + return + } + if _, seen := secretVisited[nn]; seen { + return + } + secretVisited[nn] = struct{}{} + warnings = append(warnings, v.checker.Secret(ctx, reference.SecretRef{ + Object: gateway, + NamespacedName: nn, + })...) + } + + for _, listener := range gateway.Spec.Listeners { + if listener.TLS == nil { + continue + } + for _, ref := range listener.TLS.CertificateRefs { + if ref.Kind != nil && *ref.Kind != internaltypes.KindSecret { + continue + } + if ref.Group != nil && string(*ref.Group) != corev1.GroupName { + continue + } + nn := types.NamespacedName{ + Namespace: gateway.GetNamespace(), + Name: string(ref.Name), + } + if ref.Namespace != nil && *ref.Namespace != "" { + nn.Namespace = string(*ref.Namespace) + } + addSecretWarning(nn) + } + } + + return warnings +} + func (v *GatewayCustomValidator) warnIfMissingGatewayProxyForGateway(ctx context.Context, gateway *gatewaynetworkingk8siov1.Gateway) admission.Warnings { var warnings admission.Warnings diff --git a/internal/webhook/v1/gateway_webhook_test.go b/internal/webhook/v1/gateway_webhook_test.go new file mode 100644 index 00000000..37a0695b --- /dev/null +++ b/internal/webhook/v1/gateway_webhook_test.go @@ -0,0 +1,113 @@ +// 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" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + gatewaynetworkingk8siov1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/apache/apisix-ingress-controller/internal/controller/config" +) + +func buildGatewayValidator(t *testing.T, objects ...runtime.Object) *GatewayCustomValidator { + t.Helper() + + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, gatewaynetworkingk8siov1.Install(scheme)) + + builder := fake.NewClientBuilder().WithScheme(scheme) + if len(objects) > 0 { + builder = builder.WithRuntimeObjects(objects...) + } + + return NewGatewayCustomValidator(builder.Build()) +} + +func TestGatewayCustomValidator_WarnsWhenTLSSecretMissing(t *testing.T) { + className := gatewaynetworkingk8siov1.ObjectName("apisix") + gatewayClass := &gatewaynetworkingk8siov1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{Name: string(className)}, + Spec: gatewaynetworkingk8siov1.GatewayClassSpec{ + ControllerName: gatewaynetworkingk8siov1.GatewayController(config.ControllerConfig.ControllerName), + }, + } + validator := buildGatewayValidator(t, gatewayClass) + + gateway := &gatewaynetworkingk8siov1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Name: "example", Namespace: "default"}, + Spec: gatewaynetworkingk8siov1.GatewaySpec{ + GatewayClassName: className, + Listeners: []gatewaynetworkingk8siov1.Listener{{ + Name: "https", + Port: 443, + Protocol: gatewaynetworkingk8siov1.HTTPSProtocolType, + TLS: &gatewaynetworkingk8siov1.GatewayTLSConfig{ + CertificateRefs: []gatewaynetworkingk8siov1.SecretObjectReference{{ + Name: "missing-cert", + }}, + }, + }}, + }, + } + + warnings, err := validator.ValidateCreate(context.Background(), gateway) + require.NoError(t, err) + require.Len(t, warnings, 1) + assert.Equal(t, warnings[0], "Referenced Secret 'default/missing-cert' not found") +} + +func TestGatewayCustomValidator_NoWarningsWhenSecretExists(t *testing.T) { + className := gatewaynetworkingk8siov1.ObjectName("apisix") + gatewayClass := &gatewaynetworkingk8siov1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{Name: string(className)}, + Spec: gatewaynetworkingk8siov1.GatewayClassSpec{ + ControllerName: gatewaynetworkingk8siov1.GatewayController(config.ControllerConfig.ControllerName), + }, + } + secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "tls-cert", Namespace: "default"}} + validator := buildGatewayValidator(t, gatewayClass, secret) + + gateway := &gatewaynetworkingk8siov1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Name: "example", Namespace: "default"}, + Spec: gatewaynetworkingk8siov1.GatewaySpec{ + GatewayClassName: className, + Listeners: []gatewaynetworkingk8siov1.Listener{{ + Name: "https", + Port: 443, + Protocol: gatewaynetworkingk8siov1.HTTPSProtocolType, + TLS: &gatewaynetworkingk8siov1.GatewayTLSConfig{ + CertificateRefs: []gatewaynetworkingk8siov1.SecretObjectReference{{ + Name: "tls-cert", + }}, + }, + }}, + }, + } + + warnings, err := validator.ValidateCreate(context.Background(), gateway) + require.NoError(t, err) + assert.Empty(t, warnings) +} diff --git a/internal/webhook/v1/grpcroute_webhook.go b/internal/webhook/v1/grpcroute_webhook.go new file mode 100644 index 00000000..a6530c2e --- /dev/null +++ b/internal/webhook/v1/grpcroute_webhook.go @@ -0,0 +1,146 @@ +// 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" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + 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" + + internaltypes "github.com/apache/apisix-ingress-controller/internal/types" + "github.com/apache/apisix-ingress-controller/internal/webhook/v1/reference" +) + +var grpcRouteLog = logf.Log.WithName("grpcroute-resource") + +func SetupGRPCRouteWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&gatewaynetworkingk8siov1.GRPCRoute{}). + WithValidator(NewGRPCRouteCustomValidator(mgr.GetClient())). + Complete() +} + +// +kubebuilder:webhook:path=/validate-gateway-networking-k8s-io-v1-grpcroute,mutating=false,failurePolicy=fail,sideEffects=None,groups=gateway.networking.k8s.io,resources=grpcroutes,verbs=create;update,versions=v1,name=vgrpcroute-v1.kb.io,admissionReviewVersions=v1 + +type GRPCRouteCustomValidator struct { + Client client.Client + checker reference.Checker +} + +var _ webhook.CustomValidator = &GRPCRouteCustomValidator{} + +func NewGRPCRouteCustomValidator(c client.Client) *GRPCRouteCustomValidator { + return &GRPCRouteCustomValidator{ + Client: c, + checker: reference.NewChecker(c, grpcRouteLog), + } +} + +func (v *GRPCRouteCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + route, ok := obj.(*gatewaynetworkingk8siov1.GRPCRoute) + if !ok { + return nil, fmt.Errorf("expected a GRPCRoute object but got %T", obj) + } + grpcRouteLog.Info("Validation for GRPCRoute upon creation", "name", route.GetName(), "namespace", route.GetNamespace()) + + return v.collectWarnings(ctx, route), nil +} + +func (v *GRPCRouteCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { + route, ok := newObj.(*gatewaynetworkingk8siov1.GRPCRoute) + if !ok { + return nil, fmt.Errorf("expected a GRPCRoute object for the newObj but got %T", newObj) + } + grpcRouteLog.Info("Validation for GRPCRoute upon update", "name", route.GetName(), "namespace", route.GetNamespace()) + + return v.collectWarnings(ctx, route), nil +} + +func (*GRPCRouteCustomValidator) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) { + return nil, nil +} + +func (v *GRPCRouteCustomValidator) collectWarnings(ctx context.Context, route *gatewaynetworkingk8siov1.GRPCRoute) admission.Warnings { + serviceVisited := make(map[types.NamespacedName]struct{}) + namespace := route.GetNamespace() + + var warnings admission.Warnings + + addServiceWarning := func(nn types.NamespacedName) { + if nn.Name == "" || nn.Namespace == "" { + return + } + if _, seen := serviceVisited[nn]; seen { + return + } + serviceVisited[nn] = struct{}{} + warnings = append(warnings, v.checker.Service(ctx, reference.ServiceRef{ + Object: route, + NamespacedName: nn, + })...) + } + + addBackendRef := func(ns string, name string, group *gatewaynetworkingk8siov1.Group, kind *gatewaynetworkingk8siov1.Kind) { + if name == "" { + return + } + if group != nil && string(*group) != corev1.GroupName { + return + } + if kind != nil && *kind != internaltypes.KindService { + return + } + nn := types.NamespacedName{Namespace: ns, Name: name} + addServiceWarning(nn) + } + + processFilters := func(filters []gatewaynetworkingk8siov1.GRPCRouteFilter) { + for _, filter := range filters { + if filter.RequestMirror != nil { + targetNamespace := namespace + if filter.RequestMirror.BackendRef.Namespace != nil && *filter.RequestMirror.BackendRef.Namespace != "" { + targetNamespace = string(*filter.RequestMirror.BackendRef.Namespace) + } + addBackendRef(targetNamespace, string(filter.RequestMirror.BackendRef.Name), + filter.RequestMirror.BackendRef.Group, filter.RequestMirror.BackendRef.Kind) + } + } + } + + for _, rule := range route.Spec.Rules { + for _, backend := range rule.BackendRefs { + targetNamespace := namespace + if backend.Namespace != nil && *backend.Namespace != "" { + targetNamespace = string(*backend.Namespace) + } + addBackendRef(targetNamespace, string(backend.Name), backend.Group, backend.Kind) + processFilters(backend.Filters) + } + + processFilters(rule.Filters) + } + + return warnings +} diff --git a/internal/webhook/v1/grpcroute_webhook_test.go b/internal/webhook/v1/grpcroute_webhook_test.go new file mode 100644 index 00000000..c2460b62 --- /dev/null +++ b/internal/webhook/v1/grpcroute_webhook_test.go @@ -0,0 +1,92 @@ +// 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" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + gatewaynetworkingk8siov1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func buildGRPCRouteValidator(t *testing.T, objects ...runtime.Object) *GRPCRouteCustomValidator { + t.Helper() + + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, gatewaynetworkingk8siov1.Install(scheme)) + + builder := fake.NewClientBuilder().WithScheme(scheme) + if len(objects) > 0 { + builder = builder.WithRuntimeObjects(objects...) + } + + return NewGRPCRouteCustomValidator(builder.Build()) +} + +func TestGRPCRouteCustomValidator_WarnsForMissingService(t *testing.T) { + route := &gatewaynetworkingk8siov1.GRPCRoute{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: gatewaynetworkingk8siov1.GRPCRouteSpec{ + Rules: []gatewaynetworkingk8siov1.GRPCRouteRule{{ + BackendRefs: []gatewaynetworkingk8siov1.GRPCBackendRef{{ + BackendRef: gatewaynetworkingk8siov1.BackendRef{ + BackendObjectReference: gatewaynetworkingk8siov1.BackendObjectReference{ + Name: gatewaynetworkingk8siov1.ObjectName("missing"), + }, + }, + }}, + }}, + }, + } + + validator := buildGRPCRouteValidator(t) + warnings, err := validator.ValidateCreate(context.Background(), route) + require.NoError(t, err) + require.Len(t, warnings, 1) + assert.Equal(t, warnings[0], "Referenced Service 'default/missing' not found") +} + +func TestGRPCRouteCustomValidator_NoWarningsWhenServiceExists(t *testing.T) { + service := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "backend", Namespace: "default"}} + validator := buildGRPCRouteValidator(t, service) + + route := &gatewaynetworkingk8siov1.GRPCRoute{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: gatewaynetworkingk8siov1.GRPCRouteSpec{ + Rules: []gatewaynetworkingk8siov1.GRPCRouteRule{{ + BackendRefs: []gatewaynetworkingk8siov1.GRPCBackendRef{{ + BackendRef: gatewaynetworkingk8siov1.BackendRef{ + BackendObjectReference: gatewaynetworkingk8siov1.BackendObjectReference{ + Name: gatewaynetworkingk8siov1.ObjectName("backend"), + }, + }, + }}, + }}, + }, + } + + warnings, err := validator.ValidateCreate(context.Background(), route) + require.NoError(t, err) + assert.Empty(t, warnings) +} diff --git a/internal/webhook/v1/httproute_webhook.go b/internal/webhook/v1/httproute_webhook.go new file mode 100644 index 00000000..5bee2e57 --- /dev/null +++ b/internal/webhook/v1/httproute_webhook.go @@ -0,0 +1,146 @@ +// 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" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + 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" + + internaltypes "github.com/apache/apisix-ingress-controller/internal/types" + "github.com/apache/apisix-ingress-controller/internal/webhook/v1/reference" +) + +var httpRouteLog = logf.Log.WithName("httproute-resource") + +func SetupHTTPRouteWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&gatewaynetworkingk8siov1.HTTPRoute{}). + WithValidator(NewHTTPRouteCustomValidator(mgr.GetClient())). + Complete() +} + +// +kubebuilder:webhook:path=/validate-gateway-networking-k8s-io-v1-httproute,mutating=false,failurePolicy=fail,sideEffects=None,groups=gateway.networking.k8s.io,resources=httproutes,verbs=create;update,versions=v1,name=vhttproute-v1.kb.io,admissionReviewVersions=v1 + +type HTTPRouteCustomValidator struct { + Client client.Client + checker reference.Checker +} + +var _ webhook.CustomValidator = &HTTPRouteCustomValidator{} + +func NewHTTPRouteCustomValidator(c client.Client) *HTTPRouteCustomValidator { + return &HTTPRouteCustomValidator{ + Client: c, + checker: reference.NewChecker(c, httpRouteLog), + } +} + +func (v *HTTPRouteCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + route, ok := obj.(*gatewaynetworkingk8siov1.HTTPRoute) + if !ok { + return nil, fmt.Errorf("expected a HTTPRoute object but got %T", obj) + } + httpRouteLog.Info("Validation for HTTPRoute upon creation", "name", route.GetName(), "namespace", route.GetNamespace()) + + return v.collectWarnings(ctx, route), nil +} + +func (v *HTTPRouteCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { + route, ok := newObj.(*gatewaynetworkingk8siov1.HTTPRoute) + if !ok { + return nil, fmt.Errorf("expected a HTTPRoute object for the newObj but got %T", newObj) + } + httpRouteLog.Info("Validation for HTTPRoute upon update", "name", route.GetName(), "namespace", route.GetNamespace()) + + return v.collectWarnings(ctx, route), nil +} + +func (*HTTPRouteCustomValidator) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) { + return nil, nil +} + +func (v *HTTPRouteCustomValidator) collectWarnings(ctx context.Context, route *gatewaynetworkingk8siov1.HTTPRoute) admission.Warnings { + serviceVisited := make(map[types.NamespacedName]struct{}) + namespace := route.GetNamespace() + + var warnings admission.Warnings + + addServiceWarning := func(nn types.NamespacedName) { + if nn.Name == "" || nn.Namespace == "" { + return + } + if _, seen := serviceVisited[nn]; seen { + return + } + serviceVisited[nn] = struct{}{} + warnings = append(warnings, v.checker.Service(ctx, reference.ServiceRef{ + Object: route, + NamespacedName: nn, + })...) + } + + addBackendRef := func(ns string, name string, group *gatewaynetworkingk8siov1.Group, kind *gatewaynetworkingk8siov1.Kind) { + if name == "" { + return + } + if group != nil && string(*group) != corev1.GroupName { + return + } + if kind != nil && *kind != internaltypes.KindService { + return + } + nn := types.NamespacedName{Namespace: ns, Name: name} + addServiceWarning(nn) + } + + processFilters := func(filters []gatewaynetworkingk8siov1.HTTPRouteFilter) { + for _, filter := range filters { + if filter.RequestMirror != nil { + targetNamespace := namespace + if filter.RequestMirror.BackendRef.Namespace != nil && *filter.RequestMirror.BackendRef.Namespace != "" { + targetNamespace = string(*filter.RequestMirror.BackendRef.Namespace) + } + addBackendRef(targetNamespace, string(filter.RequestMirror.BackendRef.Name), + filter.RequestMirror.BackendRef.Group, filter.RequestMirror.BackendRef.Kind) + } + } + } + + for _, rule := range route.Spec.Rules { + for _, backend := range rule.BackendRefs { + targetNamespace := namespace + if backend.Namespace != nil && *backend.Namespace != "" { + targetNamespace = string(*backend.Namespace) + } + addBackendRef(targetNamespace, string(backend.Name), backend.Group, backend.Kind) + processFilters(backend.Filters) + } + + processFilters(rule.Filters) + } + + return warnings +} diff --git a/internal/webhook/v1/httproute_webhook_test.go b/internal/webhook/v1/httproute_webhook_test.go new file mode 100644 index 00000000..40c27835 --- /dev/null +++ b/internal/webhook/v1/httproute_webhook_test.go @@ -0,0 +1,114 @@ +// 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" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + gatewaynetworkingk8siov1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func buildHTTPRouteValidator(t *testing.T, objects ...runtime.Object) *HTTPRouteCustomValidator { + t.Helper() + + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, gatewaynetworkingk8siov1.Install(scheme)) + + builder := fake.NewClientBuilder().WithScheme(scheme) + if len(objects) > 0 { + builder = builder.WithRuntimeObjects(objects...) + } + + return NewHTTPRouteCustomValidator(builder.Build()) +} + +func TestHTTPRouteCustomValidator_WarnsForMissingReferences(t *testing.T) { + route := &gatewaynetworkingk8siov1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: gatewaynetworkingk8siov1.HTTPRouteSpec{ + Rules: []gatewaynetworkingk8siov1.HTTPRouteRule{{ + BackendRefs: []gatewaynetworkingk8siov1.HTTPBackendRef{{ + BackendRef: gatewaynetworkingk8siov1.BackendRef{ + BackendObjectReference: gatewaynetworkingk8siov1.BackendObjectReference{ + Name: gatewaynetworkingk8siov1.ObjectName("missing-svc"), + }, + }, + }}, + Filters: []gatewaynetworkingk8siov1.HTTPRouteFilter{{ + Type: gatewaynetworkingk8siov1.HTTPRouteFilterRequestMirror, + RequestMirror: &gatewaynetworkingk8siov1.HTTPRequestMirrorFilter{ + BackendRef: gatewaynetworkingk8siov1.BackendObjectReference{ + Name: gatewaynetworkingk8siov1.ObjectName("mirror-svc"), + }, + }, + }}, + }}, + }, + } + + validator := buildHTTPRouteValidator(t) + warnings, err := validator.ValidateCreate(context.Background(), route) + require.NoError(t, err) + assert.ElementsMatch(t, []string{ + "Referenced Service 'default/mirror-svc' not found", + "Referenced Service 'default/missing-svc' not found", + }, warnings) +} + +func TestHTTPRouteCustomValidator_NoWarningsWhenResourcesExist(t *testing.T) { + objects := []runtime.Object{ + &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "primary", Namespace: "default"}}, + &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "mirror", Namespace: "default"}}, + } + + validator := buildHTTPRouteValidator(t, objects...) + + route := &gatewaynetworkingk8siov1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: gatewaynetworkingk8siov1.HTTPRouteSpec{ + Rules: []gatewaynetworkingk8siov1.HTTPRouteRule{{ + BackendRefs: []gatewaynetworkingk8siov1.HTTPBackendRef{{ + BackendRef: gatewaynetworkingk8siov1.BackendRef{ + BackendObjectReference: gatewaynetworkingk8siov1.BackendObjectReference{ + Name: gatewaynetworkingk8siov1.ObjectName("primary"), + }, + }, + }}, + Filters: []gatewaynetworkingk8siov1.HTTPRouteFilter{{ + Type: gatewaynetworkingk8siov1.HTTPRouteFilterRequestMirror, + RequestMirror: &gatewaynetworkingk8siov1.HTTPRequestMirrorFilter{ + BackendRef: gatewaynetworkingk8siov1.BackendObjectReference{ + Name: gatewaynetworkingk8siov1.ObjectName("mirror"), + }, + }, + }}, + }}, + }, + } + + warnings, err := validator.ValidateCreate(context.Background(), route) + require.NoError(t, err) + assert.Empty(t, warnings) +} diff --git a/internal/webhook/v1/ingress_webhook.go b/internal/webhook/v1/ingress_webhook.go index 777d1e1b..b452030d 100644 --- a/internal/webhook/v1/ingress_webhook.go +++ b/internal/webhook/v1/ingress_webhook.go @@ -22,10 +22,14 @@ import ( networkingk8siov1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" 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" + + "github.com/apache/apisix-ingress-controller/internal/webhook/v1/reference" ) var ingresslog = logf.Log.WithName("ingress-resource") @@ -99,7 +103,7 @@ func checkUnsupportedAnnotations(ingress *networkingk8siov1.Ingress) admission.W // SetupIngressWebhookWithManager registers the webhook for Ingress in the manager. func SetupIngressWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr).For(&networkingk8siov1.Ingress{}). - WithValidator(&IngressCustomValidator{}). + WithValidator(NewIngressCustomValidator(mgr.GetClient())). Complete() } @@ -112,12 +116,22 @@ func SetupIngressWebhookWithManager(mgr ctrl.Manager) error { // // 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 IngressCustomValidator struct{} +type IngressCustomValidator struct { + Client client.Client + checker reference.Checker +} var _ webhook.CustomValidator = &IngressCustomValidator{} +func NewIngressCustomValidator(c client.Client) *IngressCustomValidator { + return &IngressCustomValidator{ + Client: c, + checker: reference.NewChecker(c, ingresslog), + } +} + // ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type Ingress. -func (v *IngressCustomValidator) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { +func (v *IngressCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { ingress, ok := obj.(*networkingk8siov1.Ingress) if !ok { return nil, fmt.Errorf("expected a Ingress object but got %T", obj) @@ -126,12 +140,13 @@ func (v *IngressCustomValidator) ValidateCreate(_ context.Context, obj runtime.O // Check for unsupported annotations and generate warnings warnings := checkUnsupportedAnnotations(ingress) + warnings = append(warnings, v.collectReferenceWarnings(ctx, ingress)...) return warnings, nil } // ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type Ingress. -func (v *IngressCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { +func (v *IngressCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { ingress, ok := newObj.(*networkingk8siov1.Ingress) if !ok { return nil, fmt.Errorf("expected a Ingress object for the newObj but got %T", newObj) @@ -140,6 +155,7 @@ func (v *IngressCustomValidator) ValidateUpdate(_ context.Context, oldObj, newOb // Check for unsupported annotations and generate warnings warnings := checkUnsupportedAnnotations(ingress) + warnings = append(warnings, v.collectReferenceWarnings(ctx, ingress)...) return warnings, nil } @@ -148,3 +164,58 @@ func (v *IngressCustomValidator) ValidateUpdate(_ context.Context, oldObj, newOb func (v *IngressCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { return nil, nil } + +func (v *IngressCustomValidator) collectReferenceWarnings(ctx context.Context, ingress *networkingk8siov1.Ingress) admission.Warnings { + serviceVisited := make(map[types.NamespacedName]struct{}) + secretVisited := make(map[types.NamespacedName]struct{}) + namespace := ingress.GetNamespace() + + var warnings admission.Warnings + + addServiceWarning := func(name string) { + if name == "" { + return + } + nn := types.NamespacedName{Namespace: namespace, Name: name} + if _, seen := serviceVisited[nn]; seen { + return + } + serviceVisited[nn] = struct{}{} + warnings = append(warnings, v.checker.Service(ctx, reference.ServiceRef{ + Object: ingress, + NamespacedName: nn, + })...) + } + + addSecretWarning := func(name string) { + if name == "" { + return + } + nn := types.NamespacedName{Namespace: namespace, Name: name} + if _, seen := secretVisited[nn]; seen { + return + } + secretVisited[nn] = struct{}{} + warnings = append(warnings, v.checker.Secret(ctx, reference.SecretRef{ + Object: ingress, + NamespacedName: nn, + })...) + } + + for _, rule := range ingress.Spec.Rules { + if rule.HTTP == nil { + continue + } + for _, path := range rule.HTTP.Paths { + if path.Backend.Service != nil { + addServiceWarning(path.Backend.Service.Name) + } + } + } + + for _, tls := range ingress.Spec.TLS { + addSecretWarning(tls.SecretName) + } + + return warnings +} diff --git a/internal/webhook/v1/ingress_webhook_test.go b/internal/webhook/v1/ingress_webhook_test.go index 5ae9bd2a..67d78c0f 100644 --- a/internal/webhook/v1/ingress_webhook_test.go +++ b/internal/webhook/v1/ingress_webhook_test.go @@ -21,12 +21,35 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" networkingk8siov1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + apisixv2 "github.com/apache/apisix-ingress-controller/api/v2" ) +func buildIngressValidator(t *testing.T, objects ...runtime.Object) *IngressCustomValidator { + t.Helper() + + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, networkingk8siov1.AddToScheme(scheme)) + require.NoError(t, apisixv2.AddToScheme(scheme)) + + builder := fake.NewClientBuilder().WithScheme(scheme) + if len(objects) > 0 { + builder = builder.WithRuntimeObjects(objects...) + } + + return NewIngressCustomValidator(builder.Build()) +} + func TestIngressCustomValidator_ValidateCreate_UnsupportedAnnotations(t *testing.T) { - validator := IngressCustomValidator{} + validator := buildIngressValidator(t) obj := &networkingk8siov1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "test-ingress", @@ -49,7 +72,7 @@ func TestIngressCustomValidator_ValidateCreate_UnsupportedAnnotations(t *testing } func TestIngressCustomValidator_ValidateCreate_SupportedAnnotations(t *testing.T) { - validator := IngressCustomValidator{} + validator := buildIngressValidator(t) obj := &networkingk8siov1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "test-ingress", @@ -66,7 +89,7 @@ func TestIngressCustomValidator_ValidateCreate_SupportedAnnotations(t *testing.T } func TestIngressCustomValidator_ValidateUpdate_UnsupportedAnnotations(t *testing.T) { - validator := IngressCustomValidator{} + validator := buildIngressValidator(t) oldObj := &networkingk8siov1.Ingress{} obj := &networkingk8siov1.Ingress{ ObjectMeta: metav1.ObjectMeta{ @@ -90,7 +113,7 @@ func TestIngressCustomValidator_ValidateUpdate_UnsupportedAnnotations(t *testing } func TestIngressCustomValidator_ValidateDelete_NoWarnings(t *testing.T) { - validator := IngressCustomValidator{} + validator := buildIngressValidator(t) obj := &networkingk8siov1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "test-ingress", @@ -107,7 +130,7 @@ func TestIngressCustomValidator_ValidateDelete_NoWarnings(t *testing.T) { } func TestIngressCustomValidator_ValidateCreate_NoAnnotations(t *testing.T) { - validator := IngressCustomValidator{} + validator := buildIngressValidator(t) obj := &networkingk8siov1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "test-ingress", @@ -119,3 +142,55 @@ func TestIngressCustomValidator_ValidateCreate_NoAnnotations(t *testing.T) { assert.NoError(t, err) assert.Empty(t, warnings) } + +func TestIngressCustomValidator_WarnsForMissingServiceAndSecret(t *testing.T) { + validator := buildIngressValidator(t) + obj := &networkingk8siov1.Ingress{ + ObjectMeta: metav1.ObjectMeta{Name: "test-ingress", Namespace: "default"}, + Spec: networkingk8siov1.IngressSpec{ + Rules: []networkingk8siov1.IngressRule{{ + IngressRuleValue: networkingk8siov1.IngressRuleValue{HTTP: &networkingk8siov1.HTTPIngressRuleValue{ + Paths: []networkingk8siov1.HTTPIngressPath{{ + Backend: networkingk8siov1.IngressBackend{ + Service: &networkingk8siov1.IngressServiceBackend{Name: "default-svc"}, + }, + }}, + }}, + }}, + TLS: []networkingk8siov1.IngressTLS{{SecretName: "missing-cert"}}, + }, + } + + warnings, err := validator.ValidateCreate(context.Background(), obj) + require.NoError(t, err) + assert.ElementsMatch(t, []string{ + "Referenced Service 'default/default-svc' not found", + "Referenced Secret 'default/missing-cert' not found", + }, warnings) +} + +func TestIngressCustomValidator_NoWarningsWhenReferencesExist(t *testing.T) { + service := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "default-svc", Namespace: "default"}} + secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "tls-cert", Namespace: "default"}} + validator := buildIngressValidator(t, service, secret) + + obj := &networkingk8siov1.Ingress{ + ObjectMeta: metav1.ObjectMeta{Name: "test-ingress", Namespace: "default"}, + Spec: networkingk8siov1.IngressSpec{ + Rules: []networkingk8siov1.IngressRule{{ + IngressRuleValue: networkingk8siov1.IngressRuleValue{HTTP: &networkingk8siov1.HTTPIngressRuleValue{ + Paths: []networkingk8siov1.HTTPIngressPath{{ + Backend: networkingk8siov1.IngressBackend{ + Service: &networkingk8siov1.IngressServiceBackend{Name: "default-svc"}, + }, + }}, + }}, + }}, + TLS: []networkingk8siov1.IngressTLS{{SecretName: "tls-cert"}}, + }, + } + + warnings, err := validator.ValidateCreate(context.Background(), obj) + require.NoError(t, err) + assert.Empty(t, warnings) +}
