diff --git a/internal/adc/client/client.go b/internal/adc/client/client.go index 3019c671..8b75236b 100644 --- a/internal/adc/client/client.go +++ b/internal/adc/client/client.go @@ -174,6 +174,43 @@ func (c *Client) DeleteConfig(ctx context.Context, args Task) error { return err } +func (c *Client) Validate(ctx context.Context, task Task) error { + if len(task.Configs) == 0 || task.Resources == nil { + return nil + } + + fileIOStart := time.Now() + syncFilePath, cleanup, err := prepareSyncFile(task.Resources) + if err != nil { + pkgmetrics.RecordFileIODuration("prepare_sync_file", "failure", time.Since(fileIOStart).Seconds()) + return err + } + pkgmetrics.RecordFileIODuration("prepare_sync_file", adctypes.StatusSuccess, time.Since(fileIOStart).Seconds()) + defer cleanup() + + args := BuildADCExecuteArgs(syncFilePath, task.Labels, task.ResourceTypes) + + var errs types.ADCValidationErrors + for _, config := range task.Configs { + if config.BackendType == "" { + config.BackendType = c.defaultMode + } + if err := c.executor.Validate(ctx, config, args); err != nil { + var validationErr types.ADCValidationError + if errors.As(err, &validationErr) { + errs.Errors = append(errs.Errors, validationErr) + continue + } + return err + } + } + + if len(errs.Errors) > 0 { + return errs + } + return nil +} + func (c *Client) Sync(ctx context.Context) (map[string]types.ADCExecutionErrors, error) { c.syncMu.Lock() defer c.syncMu.Unlock() diff --git a/internal/adc/client/executor.go b/internal/adc/client/executor.go index 08608611..5664b4f2 100644 --- a/internal/adc/client/executor.go +++ b/internal/adc/client/executor.go @@ -43,6 +43,7 @@ const ( type ADCExecutor interface { Execute(ctx context.Context, config adctypes.Config, args []string) error + Validate(ctx context.Context, config adctypes.Config, args []string) error } func BuildADCExecuteArgs(filePath string, labels map[string]string, types []string) []string { @@ -81,6 +82,12 @@ type ADCServerOpts struct { CacheKey string `json:"cacheKey"` } +type ADCValidateResult struct { + Success *bool `json:"success,omitempty"` + ErrorMessage string `json:"message,omitempty"` + Errors []types.ADCValidationDetail `json:"errors,omitempty"` +} + // HTTPADCExecutor implements ADCExecutor interface using HTTP calls to ADC Server type HTTPADCExecutor struct { httpClient *http.Client @@ -123,6 +130,10 @@ func (e *HTTPADCExecutor) Execute(ctx context.Context, config adctypes.Config, a return e.runHTTPSync(ctx, config, args) } +func (e *HTTPADCExecutor) Validate(ctx context.Context, config adctypes.Config, args []string) error { + return e.runHTTPValidate(ctx, config, args) +} + // runHTTPSync performs HTTP sync to ADC Server for each server address func (e *HTTPADCExecutor) runHTTPSync(ctx context.Context, config adctypes.Config, args []string) error { var execErrs = types.ADCExecutionError{ @@ -157,6 +168,38 @@ func (e *HTTPADCExecutor) runHTTPSync(ctx context.Context, config adctypes.Confi return nil } +func (e *HTTPADCExecutor) runHTTPValidate(ctx context.Context, config adctypes.Config, args []string) error { + var validationErr = types.ADCValidationError{ + Name: config.Name, + } + var infraErrs []error + + serverAddrs := func() []string { + return config.ServerAddrs + }() + e.log.V(1).Info("running http validate", "serverAddrs", serverAddrs) + + for _, addr := range serverAddrs { + if err := e.runHTTPValidateForSingleServer(ctx, addr, config, args); err != nil { + e.log.Error(err, "failed to run http validate for server", "server", addr) + var validationServerErr types.ADCValidationServerAddrError + if errors.As(err, &validationServerErr) { + validationErr.FailedErrors = append(validationErr.FailedErrors, validationServerErr) + continue + } + infraErrs = append(infraErrs, err) + } + } + + if len(validationErr.FailedErrors) > 0 { + return validationErr + } + if len(infraErrs) > 0 { + return errors.Join(infraErrs...) + } + return nil +} + // runHTTPSyncForSingleServer performs HTTP sync to a single ADC Server func (e *HTTPADCExecutor) runHTTPSyncForSingleServer(ctx context.Context, serverAddr string, config adctypes.Config, args []string) error { ctx, cancel := context.WithTimeout(ctx, e.httpClient.Timeout) @@ -175,7 +218,7 @@ func (e *HTTPADCExecutor) runHTTPSyncForSingleServer(ctx context.Context, server } // Build HTTP request - req, err := e.buildHTTPRequest(ctx, serverAddr, config, labels, types, resources) + req, err := e.buildHTTPRequest(ctx, serverAddr, config, labels, types, resources, http.MethodPut, "/sync") if err != nil { return fmt.Errorf("failed to build HTTP request: %w", err) } @@ -195,6 +238,38 @@ func (e *HTTPADCExecutor) runHTTPSyncForSingleServer(ctx context.Context, server return e.handleHTTPResponse(resp, serverAddr) } +func (e *HTTPADCExecutor) runHTTPValidateForSingleServer(ctx context.Context, serverAddr string, config adctypes.Config, args []string) error { + ctx, cancel := context.WithTimeout(ctx, e.httpClient.Timeout) + defer cancel() + + labels, types, filePath, err := e.parseArgs(args) + if err != nil { + return fmt.Errorf("failed to parse args: %w", err) + } + + resources, err := e.loadResourcesFromFile(filePath) + if err != nil { + return fmt.Errorf("failed to load resources from file %s: %w", filePath, err) + } + + req, err := e.buildHTTPRequest(ctx, serverAddr, config, labels, types, resources, http.MethodPut, "/validate") + if err != nil { + return fmt.Errorf("failed to build validate request: %w", err) + } + + resp, err := e.httpClient.Do(req) + if err != nil { + return fmt.Errorf("failed to send HTTP request: %w", err) + } + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + e.log.Error(closeErr, "failed to close response body") + } + }() + + return e.handleHTTPValidateResponse(resp, serverAddr) +} + // parseArgs parses the command line arguments to extract labels, types, and file path func (e *HTTPADCExecutor) parseArgs(args []string) (map[string]string, []string, string, error) { labels := make(map[string]string) @@ -248,7 +323,7 @@ func (e *HTTPADCExecutor) loadResourcesFromFile(filePath string) (*adctypes.Reso } // buildHTTPRequest builds the HTTP request for ADC Server -func (e *HTTPADCExecutor) buildHTTPRequest(ctx context.Context, serverAddr string, config adctypes.Config, labels map[string]string, types []string, resources *adctypes.Resources) (*http.Request, error) { +func (e *HTTPADCExecutor) buildHTTPRequest(ctx context.Context, serverAddr string, config adctypes.Config, labels map[string]string, types []string, resources *adctypes.Resources, method string, path string) (*http.Request, error) { // Prepare request body tlsVerify := config.TlsVerify reqBody := ADCServerRequest{ @@ -274,7 +349,7 @@ func (e *HTTPADCExecutor) buildHTTPRequest(ctx context.Context, serverAddr strin } e.log.V(1).Info("sending HTTP request to ADC Server", - "url", e.serverURL+"/sync", + "url", e.serverURL+path, "server", serverAddr, "mode", config.BackendType, "cacheKey", config.Name, @@ -284,7 +359,7 @@ func (e *HTTPADCExecutor) buildHTTPRequest(ctx context.Context, serverAddr strin ) // Create HTTP request - req, err := http.NewRequestWithContext(ctx, "PUT", e.serverURL+"/sync", bytes.NewBuffer(jsonData)) + req, err := http.NewRequestWithContext(ctx, method, e.serverURL+path, bytes.NewBuffer(jsonData)) if err != nil { return nil, fmt.Errorf("failed to create HTTP request: %w", err) } @@ -357,3 +432,63 @@ func (e *HTTPADCExecutor) handleHTTPResponse(resp *http.Response, serverAddr str e.log.V(1).Info("ADC Server sync success", "result", result) return nil } + +func (e *HTTPADCExecutor) handleHTTPValidateResponse(resp *http.Response, serverAddr string) error { + body, err := io.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("failed to read response body: %w", err) + } + + e.log.V(1).Info("received HTTP validate response from ADC Server", + "server", serverAddr, + "status", resp.StatusCode, + "response", string(body), + ) + + parseValidationResult := func() *ADCValidateResult { + if len(body) == 0 { + return nil + } + var result ADCValidateResult + if err := json.Unmarshal(body, &result); err != nil { + return nil + } + return &result + } + + if resp.StatusCode == http.StatusBadRequest { + result := parseValidationResult() + errMsg := string(body) + if result != nil && result.ErrorMessage != "" { + errMsg = result.ErrorMessage + } + return types.ADCValidationServerAddrError{ + ServerAddr: serverAddr, + Err: errMsg, + ValidationErrors: func() []types.ADCValidationDetail { + if result == nil { + return nil + } + return result.Errors + }(), + } + } + + if resp.StatusCode/100 != 2 { + return fmt.Errorf("HTTP %d: %s", resp.StatusCode, string(body)) + } + + if result := parseValidationResult(); result != nil && result.Success != nil && !*result.Success { + errMsg := result.ErrorMessage + if errMsg == "" { + errMsg = "ADC validation failed" + } + return types.ADCValidationServerAddrError{ + ServerAddr: serverAddr, + Err: errMsg, + ValidationErrors: result.Errors, + } + } + + return nil +} diff --git a/internal/controller/webhook_validation.go b/internal/controller/webhook_validation.go new file mode 100644 index 00000000..53545c98 --- /dev/null +++ b/internal/controller/webhook_validation.go @@ -0,0 +1,117 @@ +// 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 controller + +import ( + "context" + + "github.com/go-logr/logr" + networkingv1 "k8s.io/api/networking/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1" + apiv2 "github.com/apache/apisix-ingress-controller/api/v2" + "github.com/apache/apisix-ingress-controller/internal/provider" + "github.com/apache/apisix-ingress-controller/internal/utils" +) + +func PrepareApisixRouteForValidation(ctx context.Context, c client.Client, log logr.Logger, route *apiv2.ApisixRoute) (*provider.TranslateContext, error) { + tctx := provider.NewDefaultTranslateContext(ctx) + + ingressClass, err := FindMatchingIngressClassByObject(tctx, c, log, route, networkingv1.SchemeGroupVersion.String()) + if err != nil { + return nil, err + } + if err := ProcessIngressClassParameters(tctx, c, log, route, ingressClass); err != nil { + return nil, err + } + + reconciler := &ApisixRouteReconciler{ + Client: c, + Log: log, + ICGV: networkingv1.SchemeGroupVersion, + supportsEndpointSlice: false, + } + if err := reconciler.processApisixRoute(tctx, route); err != nil { + return nil, err + } + return tctx, nil +} + +func PrepareApisixConsumerForValidation(ctx context.Context, c client.Client, log logr.Logger, consumer *apiv2.ApisixConsumer) (*provider.TranslateContext, error) { + tctx := provider.NewDefaultTranslateContext(ctx) + + ingressClass, err := FindMatchingIngressClassByObject(tctx, c, log, consumer, networkingv1.SchemeGroupVersion.String()) + if err != nil { + return nil, err + } + if err := ProcessIngressClassParameters(tctx, c, log, consumer, ingressClass); err != nil { + return nil, err + } + + reconciler := &ApisixConsumerReconciler{ + Client: c, + Log: log, + ICGV: networkingv1.SchemeGroupVersion, + } + if err := reconciler.processSpec(ctx, tctx, consumer); err != nil { + return nil, err + } + return tctx, nil +} + +func PrepareConsumerForValidation(ctx context.Context, c client.Client, log logr.Logger, consumer *v1alpha1.Consumer) (*provider.TranslateContext, error) { + tctx := provider.NewDefaultTranslateContext(ctx) + + reconciler := &ConsumerReconciler{ + Client: c, + Log: log, + } + gateway, err := reconciler.getGateway(ctx, consumer) + if err != nil { + return nil, err + } + if err := ProcessGatewayProxy(c, log, tctx, gateway, utils.NamespacedNameKind(consumer)); err != nil { + return nil, err + } + if err := reconciler.processSpec(ctx, tctx, consumer); err != nil { + return nil, err + } + return tctx, nil +} + +func PrepareApisixTlsForValidation(ctx context.Context, c client.Client, log logr.Logger, tls *apiv2.ApisixTls) (*provider.TranslateContext, error) { + tctx := provider.NewDefaultTranslateContext(ctx) + + ingressClass, err := FindMatchingIngressClassByObject(tctx, c, log, tls, networkingv1.SchemeGroupVersion.String()) + if err != nil { + return nil, err + } + if err := ProcessIngressClassParameters(tctx, c, log, tls, ingressClass); err != nil { + return nil, err + } + + reconciler := &ApisixTlsReconciler{ + Client: c, + Log: log, + } + if err := reconciler.processApisixTls(ctx, tctx, tls); err != nil { + return nil, err + } + return tctx, nil +} diff --git a/internal/types/error.go b/internal/types/error.go index 80dbf568..1388637d 100644 --- a/internal/types/error.go +++ b/internal/types/error.go @@ -92,3 +92,70 @@ type ADCExecutionServerAddrError struct { func (e ADCExecutionServerAddrError) Error() string { return fmt.Sprintf("ServerAddr: %s, Err: %s", e.ServerAddr, e.Err) } + +type ADCValidationErrors struct { + Errors []ADCValidationError +} + +func (e ADCValidationErrors) Error() string { + messages := make([]string, 0, len(e.Errors)) + for _, err := range e.Errors { + messages = append(messages, err.Error()) + } + return fmt.Sprintf("ADC validation errors: [%s]", strings.Join(messages, "; ")) +} + +type ADCValidationError struct { + Name string + FailedErrors []ADCValidationServerAddrError +} + +func (e ADCValidationError) Error() string { + messages := make([]string, 0, len(e.FailedErrors)) + for _, failed := range e.FailedErrors { + messages = append(messages, failed.Error()) + } + return fmt.Sprintf("ADC validation error for %s: [%s]", e.Name, strings.Join(messages, "; ")) +} + +type ADCValidationServerAddrError struct { + Err string + ServerAddr string + ValidationErrors []ADCValidationDetail +} + +func (e ADCValidationServerAddrError) Error() string { + if len(e.ValidationErrors) == 0 { + return fmt.Sprintf("ServerAddr: %s, Err: %s", e.ServerAddr, e.Err) + } + + messages := make([]string, 0, len(e.ValidationErrors)) + for _, detail := range e.ValidationErrors { + messages = append(messages, detail.Error()) + } + return fmt.Sprintf("ServerAddr: %s, Err: %s (%s)", e.ServerAddr, e.Err, strings.Join(messages, "; ")) +} + +type ADCValidationDetail struct { + ResourceType string `json:"resource_type,omitempty"` + ResourceName string `json:"resource_name,omitempty"` + Message string `json:"message,omitempty"` + Index int `json:"index,omitempty"` +} + +func (e ADCValidationDetail) Error() string { + var parts []string + if e.ResourceType != "" { + parts = append(parts, fmt.Sprintf("type=%s", e.ResourceType)) + } + if e.ResourceName != "" { + parts = append(parts, fmt.Sprintf("name=%s", e.ResourceName)) + } + if e.Message != "" { + parts = append(parts, e.Message) + } + if len(parts) == 0 { + return fmt.Sprintf("index=%d", e.Index) + } + return strings.Join(parts, ", ") +} diff --git a/internal/webhook/v1/adc_validation.go b/internal/webhook/v1/adc_validation.go new file mode 100644 index 00000000..f505fe9a --- /dev/null +++ b/internal/webhook/v1/adc_validation.go @@ -0,0 +1,234 @@ +// 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" + "errors" + + "github.com/go-logr/logr" + networkingv1 "k8s.io/api/networking/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + adctypes "github.com/apache/apisix-ingress-controller/api/adc" + v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1" + apiv2 "github.com/apache/apisix-ingress-controller/api/v2" + adcclient "github.com/apache/apisix-ingress-controller/internal/adc/client" + adctranslator "github.com/apache/apisix-ingress-controller/internal/adc/translator" + "github.com/apache/apisix-ingress-controller/internal/controller" + "github.com/apache/apisix-ingress-controller/internal/controller/config" + "github.com/apache/apisix-ingress-controller/internal/controller/label" + "github.com/apache/apisix-ingress-controller/internal/provider" + internaltypes "github.com/apache/apisix-ingress-controller/internal/types" + "github.com/apache/apisix-ingress-controller/internal/utils" +) + +type adcAdmissionValidator struct { + kubeClient client.Client + client *adcclient.Client + translator *adctranslator.Translator + log logr.Logger + defaultResolveEndpoint bool +} + +func newADCAdmissionValidator(kubeClient client.Client, log logr.Logger) (*adcAdmissionValidator, error) { + defaultMode := string(config.ControllerConfig.ProviderConfig.Type) + cli, err := adcclient.New(log, defaultMode, config.ControllerConfig.ExecADCTimeout.Duration) + if err != nil { + return nil, err + } + + return &adcAdmissionValidator{ + kubeClient: kubeClient, + client: cli, + translator: adctranslator.NewTranslator(log), + log: log.WithName("adc-validation"), + defaultResolveEndpoint: config.ControllerConfig.ProviderConfig.Type == config.ProviderTypeStandalone, + }, nil +} + +func (v *adcAdmissionValidator) Validate(ctx context.Context, obj client.Object) error { + if v == nil { + return nil + } + + task, err := v.buildTask(ctx, obj) + if err != nil { + return err + } + if task == nil { + return nil + } + + if err := v.client.Validate(ctx, *task); err != nil { + var validationErrs internaltypes.ADCValidationErrors + if errors.As(err, &validationErrs) { + return err + } + + v.log.Error(err, "ADC validation unavailable, allowing admission", "resource", utils.NamespacedNameKind(obj)) + return nil + } + + return nil +} + +func (v *adcAdmissionValidator) buildTask(ctx context.Context, obj client.Object) (*adcclient.Task, error) { + var ( + tctx *provider.TranslateContext + result *adctranslator.TranslateResult + resourceTypes []string + err error + ) + + switch resource := obj.(type) { + case *apiv2.ApisixRoute: + configs, err := v.buildIngressClassConfigs(ctx, resource.DeepCopy()) + if err != nil { + return nil, err + } + if len(configs) == 0 { + return nil, nil + } + tctx, err = controller.PrepareApisixRouteForValidation(ctx, v.kubeClient, v.log, resource.DeepCopy()) + if err != nil { + return nil, err + } + result, err = v.translator.TranslateApisixRoute(tctx, resource.DeepCopy()) + resourceTypes = append(resourceTypes, adctypes.TypeService) + if err != nil { + return nil, err + } + if result == nil { + return nil, nil + } + return v.newTask(obj, configs, resourceTypes, result), nil + case *apiv2.ApisixConsumer: + configs, err := v.buildIngressClassConfigs(ctx, resource.DeepCopy()) + if err != nil { + return nil, err + } + if len(configs) == 0 { + return nil, nil + } + tctx, err = controller.PrepareApisixConsumerForValidation(ctx, v.kubeClient, v.log, resource.DeepCopy()) + if err != nil { + return nil, err + } + result, err = v.translator.TranslateApisixConsumer(tctx, resource.DeepCopy()) + resourceTypes = append(resourceTypes, adctypes.TypeConsumer) + if err != nil { + return nil, err + } + if result == nil { + return nil, nil + } + return v.newTask(obj, configs, resourceTypes, result), nil + case *v1alpha1.Consumer: + tctx, err = controller.PrepareConsumerForValidation(ctx, v.kubeClient, v.log, resource.DeepCopy()) + if err != nil { + return nil, err + } + result, err = v.translator.TranslateConsumerV1alpha1(tctx, resource.DeepCopy()) + resourceTypes = append(resourceTypes, adctypes.TypeConsumer) + case *apiv2.ApisixTls: + configs, err := v.buildIngressClassConfigs(ctx, resource.DeepCopy()) + if err != nil { + return nil, err + } + if len(configs) == 0 { + return nil, nil + } + tctx, err = controller.PrepareApisixTlsForValidation(ctx, v.kubeClient, v.log, resource.DeepCopy()) + if err != nil { + return nil, err + } + result, err = v.translator.TranslateApisixTls(tctx, resource.DeepCopy()) + resourceTypes = append(resourceTypes, adctypes.TypeSSL) + if err != nil { + return nil, err + } + if result == nil { + return nil, nil + } + return v.newTask(obj, configs, resourceTypes, result), nil + default: + return nil, nil + } + if err != nil { + return nil, err + } + if result == nil { + return nil, nil + } + + configs, err := v.buildConfigs(tctx) + if err != nil { + return nil, err + } + if len(configs) == 0 { + return nil, nil + } + + return v.newTask(obj, configs, resourceTypes, result), nil +} + +func (v *adcAdmissionValidator) buildConfigs(tctx *provider.TranslateContext) (map[internaltypes.NamespacedNameKind]adctypes.Config, error) { + configs := make(map[internaltypes.NamespacedNameKind]adctypes.Config, len(tctx.GatewayProxies)) + for key, gp := range tctx.GatewayProxies { + cfg, err := v.translator.TranslateGatewayProxyToConfig(tctx, &gp, v.defaultResolveEndpoint) + if err != nil { + return nil, err + } + if cfg == nil { + continue + } + configs[key] = *cfg + } + return configs, nil +} + +func (v *adcAdmissionValidator) buildIngressClassConfigs(ctx context.Context, obj client.Object) (map[internaltypes.NamespacedNameKind]adctypes.Config, error) { + tctx := provider.NewDefaultTranslateContext(ctx) + + ingressClass, err := controller.FindMatchingIngressClassByObject(tctx, v.kubeClient, v.log, obj, networkingv1.SchemeGroupVersion.String()) + if err != nil { + return nil, err + } + if err := controller.ProcessIngressClassParameters(tctx, v.kubeClient, v.log, obj, ingressClass); err != nil { + return nil, err + } + return v.buildConfigs(tctx) +} + +func (v *adcAdmissionValidator) newTask(obj client.Object, configs map[internaltypes.NamespacedNameKind]adctypes.Config, resourceTypes []string, result *adctranslator.TranslateResult) *adcclient.Task { + return &adcclient.Task{ + Key: utils.NamespacedNameKind(obj), + Name: utils.NamespacedNameKind(obj).String(), + Labels: label.GenLabel(obj), + Configs: configs, + ResourceTypes: resourceTypes, + Resources: &adctypes.Resources{ + GlobalRules: result.GlobalRules, + PluginMetadata: result.PluginMetadata, + Services: result.Services, + SSLs: result.SSL, + Consumers: result.Consumers, + }, + } +} diff --git a/internal/webhook/v1/adc_validation_test.go b/internal/webhook/v1/adc_validation_test.go new file mode 100644 index 00000000..535051d6 --- /dev/null +++ b/internal/webhook/v1/adc_validation_test.go @@ -0,0 +1,91 @@ +// 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 ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + + apisixv1alpha1 "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" +) + +func withMockADCServer(t *testing.T, handler http.HandlerFunc) string { + t.Helper() + + server := httptest.NewServer(handler) + t.Setenv("ADC_SERVER_URL", server.URL) + t.Cleanup(server.Close) + return server.URL +} + +func managedIngressClassWithGatewayProxy(endpoint string) []runtime.Object { + return managedIngressClassWithGatewayProxyMode(endpoint, "apisix-standalone") +} + +func managedIngressClassWithGatewayProxyMode(endpoint, mode string) []runtime.Object { + namespace := "default" + + return []runtime.Object{ + &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{Name: "apisix"}, + Spec: networkingv1.IngressClassSpec{ + Controller: config.ControllerConfig.ControllerName, + Parameters: &networkingv1.IngressClassParametersReference{ + APIGroup: ptr.To(apisixv1alpha1.GroupVersion.Group), + Kind: internaltypes.KindGatewayProxy, + Name: "gateway-proxy", + Namespace: ptr.To(namespace), + }, + }, + }, + &apisixv1alpha1.GatewayProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-proxy", + Namespace: namespace, + }, + Spec: apisixv1alpha1.GatewayProxySpec{ + Provider: &apisixv1alpha1.GatewayProxyProvider{ + Type: apisixv1alpha1.ProviderTypeControlPlane, + ControlPlane: &apisixv1alpha1.ControlPlaneProvider{ + Mode: mode, + Endpoints: []string{endpoint}, + Auth: apisixv1alpha1.ControlPlaneAuth{ + Type: apisixv1alpha1.AuthTypeAdminKey, + AdminKey: &apisixv1alpha1.AdminKeyAuth{ + Value: "token", + }, + }, + }, + }, + }, + }, + } +} + +func requireValidateRequest(t *testing.T, r *http.Request) { + t.Helper() + require.Equal(t, http.MethodPut, r.Method) + require.Equal(t, "/validate", r.URL.Path) +} diff --git a/internal/webhook/v1/apisixconsumer_webhook.go b/internal/webhook/v1/apisixconsumer_webhook.go index b419b8da..35c5c3cd 100644 --- a/internal/webhook/v1/apisixconsumer_webhook.go +++ b/internal/webhook/v1/apisixconsumer_webhook.go @@ -42,19 +42,24 @@ func SetupApisixConsumerWebhookWithManager(mgr ctrl.Manager) error { Complete() } -// +kubebuilder:webhook:path=/validate-apisix-apache-org-v2-apisixconsumer,mutating=false,failurePolicy=fail,sideEffects=None,groups=apisix.apache.org,resources=apisixconsumers,verbs=create;update,versions=v2,name=vapisixconsumer-v2.kb.io,admissionReviewVersions=v1,failurePolicy=Ignore +// +kubebuilder:webhook:path=/validate-apisix-apache-org-v2-apisixconsumer,mutating=false,failurePolicy=Ignore,sideEffects=None,groups=apisix.apache.org,resources=apisixconsumers,verbs=create;update,versions=v2,name=vapisixconsumer-v2.kb.io,admissionReviewVersions=v1 type ApisixConsumerCustomValidator struct { - Client client.Client - checker reference.Checker + Client client.Client + checker reference.Checker + adcValidator *adcAdmissionValidator + initErr error } var _ webhook.CustomValidator = &ApisixConsumerCustomValidator{} func NewApisixConsumerCustomValidator(c client.Client) *ApisixConsumerCustomValidator { + adcValidator, err := newADCAdmissionValidator(c, apisixConsumerLog) return &ApisixConsumerCustomValidator{ - Client: c, - checker: reference.NewChecker(c, apisixConsumerLog), + Client: c, + checker: reference.NewChecker(c, apisixConsumerLog), + adcValidator: adcValidator, + initErr: err, } } @@ -69,7 +74,15 @@ func (v *ApisixConsumerCustomValidator) ValidateCreate(ctx context.Context, obj return nil, nil } - return v.collectWarnings(ctx, consumer), nil + warnings := v.collectWarnings(ctx, consumer) + if v.initErr != nil { + apisixConsumerLog.Error(v.initErr, "ADC validator init failed, skipping ADC validation") + return warnings, nil + } + if len(warnings) > 0 { + return warnings, nil + } + return warnings, v.adcValidator.Validate(ctx, consumer) } func (v *ApisixConsumerCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { @@ -82,7 +95,15 @@ func (v *ApisixConsumerCustomValidator) ValidateUpdate(ctx context.Context, oldO return nil, nil } - return v.collectWarnings(ctx, consumer), nil + warnings := v.collectWarnings(ctx, consumer) + if v.initErr != nil { + apisixConsumerLog.Error(v.initErr, "ADC validator init failed, skipping ADC validation") + return warnings, nil + } + if len(warnings) > 0 { + return warnings, nil + } + return warnings, v.adcValidator.Validate(ctx, consumer) } func (*ApisixConsumerCustomValidator) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) { diff --git a/internal/webhook/v1/apisixconsumer_webhook_test.go b/internal/webhook/v1/apisixconsumer_webhook_test.go index 8c31768c..e1be420d 100644 --- a/internal/webhook/v1/apisixconsumer_webhook_test.go +++ b/internal/webhook/v1/apisixconsumer_webhook_test.go @@ -17,6 +17,7 @@ package v1 import ( "context" + "net/http" "testing" "github.com/stretchr/testify/require" @@ -27,22 +28,35 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client/fake" + apisixv1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1" apisixv2 "github.com/apache/apisix-ingress-controller/api/v2" "github.com/apache/apisix-ingress-controller/internal/controller/config" ) +const managedIngressClassName = "apisix" + func buildApisixConsumerValidator(t *testing.T, objects ...runtime.Object) *ApisixConsumerCustomValidator { t.Helper() scheme := runtime.NewScheme() require.NoError(t, clientgoscheme.AddToScheme(scheme)) require.NoError(t, networkingv1.AddToScheme(scheme)) + require.NoError(t, apisixv1alpha1.AddToScheme(scheme)) require.NoError(t, apisixv2.AddToScheme(scheme)) - managed := []runtime.Object{ - &networkingv1.IngressClass{ + managed := []runtime.Object{} + hasManagedIngressClass := false + for _, obj := range objects { + ingressClass, ok := obj.(*networkingv1.IngressClass) + if ok && ingressClass.Name == managedIngressClassName { + hasManagedIngressClass = true + break + } + } + if !hasManagedIngressClass { + managed = append(managed, &networkingv1.IngressClass{ ObjectMeta: metav1.ObjectMeta{ - Name: "apisix", + Name: managedIngressClassName, Annotations: map[string]string{ "ingressclass.kubernetes.io/is-default-class": "true", }, @@ -50,7 +64,7 @@ func buildApisixConsumerValidator(t *testing.T, objects ...runtime.Object) *Apis Spec: networkingv1.IngressClassSpec{ Controller: config.ControllerConfig.ControllerName, }, - }, + }) } allObjects := append(managed, objects...) builder := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(allObjects...) @@ -152,3 +166,72 @@ func TestApisixConsumerValidator_NoWarningsWhenSecretsExist(t *testing.T) { require.NoError(t, err) require.Empty(t, warnings) } + +func TestApisixConsumerValidator_DeniesOnADCValidationFailure(t *testing.T) { + serverURL := withMockADCServer(t, func(w http.ResponseWriter, r *http.Request) { + requireValidateRequest(t, r) + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"message":"consumer rejected","errors":[{"resource_type":"consumers","resource_name":"demo","message":"duplicate credential"}]}`)) + }) + + consumer := &apisixv2.ApisixConsumer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + Namespace: "default", + }, + Spec: apisixv2.ApisixConsumerSpec{ + IngressClassName: "apisix", + AuthParameter: apisixv2.ApisixConsumerAuthParameter{ + KeyAuth: &apisixv2.ApisixConsumerKeyAuth{ + SecretRef: &corev1.LocalObjectReference{Name: "key-auth"}, + }, + }, + }, + } + + objects := append(managedIngressClassWithGatewayProxy(serverURL), + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "key-auth", Namespace: "default"}, + Data: map[string][]byte{ + "key": []byte("secret-key"), + }, + }, + ) + + validator := buildApisixConsumerValidator(t, objects...) + + warnings, err := validator.ValidateCreate(context.Background(), consumer) + require.Error(t, err) + require.Contains(t, err.Error(), "consumer rejected") + require.Empty(t, warnings) +} + +func TestApisixConsumerValidator_UsesADCValidateEndpointForControlPlane(t *testing.T) { + serverURL := withMockADCServer(t, func(w http.ResponseWriter, r *http.Request) { + requireValidateRequest(t, r) + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"message":"consumer rejected","errors":[{"resource_type":"consumers","resource_name":"demo","message":"duplicate credential"}]}`)) + }) + + consumer := &apisixv2.ApisixConsumer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + Namespace: "default", + }, + Spec: apisixv2.ApisixConsumerSpec{ + IngressClassName: managedIngressClassName, + AuthParameter: apisixv2.ApisixConsumerAuthParameter{ + KeyAuth: &apisixv2.ApisixConsumerKeyAuth{ + Value: &apisixv2.ApisixConsumerKeyAuthValue{Key: "shared-key"}, + }, + }, + }, + } + + validator := buildApisixConsumerValidator(t, managedIngressClassWithGatewayProxyMode(serverURL, "apisix")...) + + warnings, err := validator.ValidateCreate(context.Background(), consumer) + require.Error(t, err) + require.Contains(t, err.Error(), "consumer rejected") + require.Empty(t, warnings) +} diff --git a/internal/webhook/v1/apisixroute_webhook.go b/internal/webhook/v1/apisixroute_webhook.go index 6f028fdd..92d19dae 100644 --- a/internal/webhook/v1/apisixroute_webhook.go +++ b/internal/webhook/v1/apisixroute_webhook.go @@ -41,19 +41,24 @@ func SetupApisixRouteWebhookWithManager(mgr ctrl.Manager) error { Complete() } -// +kubebuilder:webhook:path=/validate-apisix-apache-org-v2-apisixroute,mutating=false,failurePolicy=fail,sideEffects=None,groups=apisix.apache.org,resources=apisixroutes,verbs=create;update,versions=v2,name=vapisixroute-v2.kb.io,admissionReviewVersions=v1,failurePolicy=Ignore +// +kubebuilder:webhook:path=/validate-apisix-apache-org-v2-apisixroute,mutating=false,failurePolicy=Ignore,sideEffects=None,groups=apisix.apache.org,resources=apisixroutes,verbs=create;update,versions=v2,name=vapisixroute-v2.kb.io,admissionReviewVersions=v1 type ApisixRouteCustomValidator struct { - Client client.Client - checker reference.Checker + Client client.Client + checker reference.Checker + adcValidator *adcAdmissionValidator + initErr error } var _ webhook.CustomValidator = &ApisixRouteCustomValidator{} func NewApisixRouteCustomValidator(c client.Client) *ApisixRouteCustomValidator { + adcValidator, err := newADCAdmissionValidator(c, apisixRouteLog) return &ApisixRouteCustomValidator{ - Client: c, - checker: reference.NewChecker(c, apisixRouteLog), + Client: c, + checker: reference.NewChecker(c, apisixRouteLog), + adcValidator: adcValidator, + initErr: err, } } @@ -67,7 +72,12 @@ func (v *ApisixRouteCustomValidator) ValidateCreate(ctx context.Context, obj run return nil, nil } - return v.collectWarnings(ctx, route), nil + warnings := v.collectWarnings(ctx, route) + if v.initErr != nil { + apisixRouteLog.Error(v.initErr, "ADC validator init failed, skipping ADC validation") + return warnings, nil + } + return warnings, v.adcValidator.Validate(ctx, route) } func (v *ApisixRouteCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { @@ -80,7 +90,12 @@ func (v *ApisixRouteCustomValidator) ValidateUpdate(ctx context.Context, oldObj, return nil, nil } - return v.collectWarnings(ctx, route), nil + warnings := v.collectWarnings(ctx, route) + if v.initErr != nil { + apisixRouteLog.Error(v.initErr, "ADC validator init failed, skipping ADC validation") + return warnings, nil + } + return warnings, v.adcValidator.Validate(ctx, route) } func (*ApisixRouteCustomValidator) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) { diff --git a/internal/webhook/v1/apisixroute_webhook_test.go b/internal/webhook/v1/apisixroute_webhook_test.go index b8ca3aa2..98bab580 100644 --- a/internal/webhook/v1/apisixroute_webhook_test.go +++ b/internal/webhook/v1/apisixroute_webhook_test.go @@ -17,6 +17,7 @@ package v1 import ( "context" + "net/http" "testing" "github.com/stretchr/testify/require" @@ -24,9 +25,11 @@ import ( networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client/fake" + apisixv1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1" apisixv2 "github.com/apache/apisix-ingress-controller/api/v2" "github.com/apache/apisix-ingress-controller/internal/controller/config" ) @@ -37,10 +40,20 @@ func buildApisixRouteValidator(t *testing.T, objects ...runtime.Object) *ApisixR scheme := runtime.NewScheme() require.NoError(t, clientgoscheme.AddToScheme(scheme)) require.NoError(t, networkingv1.AddToScheme(scheme)) + require.NoError(t, apisixv1alpha1.AddToScheme(scheme)) require.NoError(t, apisixv2.AddToScheme(scheme)) - managed := []runtime.Object{ - &networkingv1.IngressClass{ + managed := []runtime.Object{} + hasManagedIngressClass := false + for _, obj := range objects { + ingressClass, ok := obj.(*networkingv1.IngressClass) + if ok && ingressClass.Name == "apisix" { + hasManagedIngressClass = true + break + } + } + if !hasManagedIngressClass { + managed = append(managed, &networkingv1.IngressClass{ ObjectMeta: metav1.ObjectMeta{ Name: "apisix", Annotations: map[string]string{ @@ -50,7 +63,7 @@ func buildApisixRouteValidator(t *testing.T, objects ...runtime.Object) *ApisixR Spec: networkingv1.IngressClassSpec{ Controller: config.ControllerConfig.ControllerName, }, - }, + }) } allObjects := append(managed, objects...) builder := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(allObjects...) @@ -174,3 +187,92 @@ func TestApisixRouteValidator_NoWarnings(t *testing.T) { require.NoError(t, err) require.Empty(t, warnings) } + +func TestApisixRouteValidator_DeniesOnADCValidationFailure(t *testing.T) { + serverURL := withMockADCServer(t, func(w http.ResponseWriter, r *http.Request) { + requireValidateRequest(t, r) + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"message":"route rejected","errors":[{"resource_type":"routes","resource_name":"demo","message":"invalid plugin config"}]}`)) + }) + + route := &apisixv2.ApisixRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + Namespace: "default", + }, + Spec: apisixv2.ApisixRouteSpec{ + IngressClassName: "apisix", + HTTP: []apisixv2.ApisixRouteHTTP{{ + Name: "rule", + Backends: []apisixv2.ApisixRouteHTTPBackend{{ + ServiceName: "backend", + ServicePort: intstr.FromInt(80), + ResolveGranularity: apisixv2.ResolveGranularityService, + }}, + }}, + }, + } + + objects := append(managedIngressClassWithGatewayProxy(serverURL), + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "backend", Namespace: "default"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "10.0.0.1", + Ports: []corev1.ServicePort{{ + Port: 80, + }}, + }, + }, + ) + + validator := buildApisixRouteValidator(t, objects...) + + warnings, err := validator.ValidateCreate(context.Background(), route) + require.Error(t, err) + require.Contains(t, err.Error(), "route rejected") + require.Empty(t, warnings) +} + +func TestApisixRouteValidator_FailsOpenWhenADCUnavailable(t *testing.T) { + serverURL := withMockADCServer(t, func(w http.ResponseWriter, r *http.Request) { + requireValidateRequest(t, r) + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":"backend unavailable"}`)) + }) + + route := &apisixv2.ApisixRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + Namespace: "default", + }, + Spec: apisixv2.ApisixRouteSpec{ + IngressClassName: "apisix", + HTTP: []apisixv2.ApisixRouteHTTP{{ + Name: "rule", + Backends: []apisixv2.ApisixRouteHTTPBackend{{ + ServiceName: "backend", + ServicePort: intstr.FromInt(80), + ResolveGranularity: apisixv2.ResolveGranularityService, + }}, + }}, + }, + } + + objects := append(managedIngressClassWithGatewayProxy(serverURL), + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "backend", Namespace: "default"}, + Spec: corev1.ServiceSpec{ + ClusterIP: "10.0.0.1", + Ports: []corev1.ServicePort{{ + Port: 80, + }}, + }, + }, + ) + + validator := buildApisixRouteValidator(t, objects...) + + warnings, err := validator.ValidateCreate(context.Background(), route) + require.NoError(t, err) + require.Empty(t, warnings) +} diff --git a/internal/webhook/v1/apisixtls_webhook.go b/internal/webhook/v1/apisixtls_webhook.go index 16ba02d5..0b27ee28 100644 --- a/internal/webhook/v1/apisixtls_webhook.go +++ b/internal/webhook/v1/apisixtls_webhook.go @@ -42,19 +42,24 @@ func SetupApisixTlsWebhookWithManager(mgr ctrl.Manager) error { Complete() } -// +kubebuilder:webhook:path=/validate-apisix-apache-org-v2-apisixtls,mutating=false,failurePolicy=fail,sideEffects=None,groups=apisix.apache.org,resources=apisixtlses,verbs=create;update,versions=v2,name=vapisixtls-v2.kb.io,admissionReviewVersions=v1,failurePolicy=Ignore +// +kubebuilder:webhook:path=/validate-apisix-apache-org-v2-apisixtls,mutating=false,failurePolicy=Ignore,sideEffects=None,groups=apisix.apache.org,resources=apisixtlses,verbs=create;update,versions=v2,name=vapisixtls-v2.kb.io,admissionReviewVersions=v1 type ApisixTlsCustomValidator struct { - Client client.Client - checker reference.Checker + Client client.Client + checker reference.Checker + adcValidator *adcAdmissionValidator + initErr error } var _ webhook.CustomValidator = &ApisixTlsCustomValidator{} func NewApisixTlsCustomValidator(c client.Client) *ApisixTlsCustomValidator { + adcValidator, err := newADCAdmissionValidator(c, apisixTlsLog) return &ApisixTlsCustomValidator{ - Client: c, - checker: reference.NewChecker(c, apisixTlsLog), + Client: c, + checker: reference.NewChecker(c, apisixTlsLog), + adcValidator: adcValidator, + initErr: err, } } @@ -74,7 +79,15 @@ func (v *ApisixTlsCustomValidator) ValidateCreate(ctx context.Context, obj runti return nil, fmt.Errorf("%s", sslvalidator.FormatConflicts(conflicts)) } - return v.collectWarnings(ctx, tls), nil + warnings := v.collectWarnings(ctx, tls) + // Skip ADC validation when secrets are missing: the translator cannot + // load cert/key material, so validation would always fail. The missing- + // secret warnings are sufficient to inform the user. + if v.initErr != nil || len(warnings) > 0 { + return warnings, nil + } + + return warnings, v.adcValidator.Validate(ctx, tls) } func (v *ApisixTlsCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { @@ -93,7 +106,12 @@ func (v *ApisixTlsCustomValidator) ValidateUpdate(ctx context.Context, oldObj, n return nil, fmt.Errorf("%s", sslvalidator.FormatConflicts(conflicts)) } - return v.collectWarnings(ctx, tls), nil + warnings := v.collectWarnings(ctx, tls) + if v.initErr != nil || len(warnings) > 0 { + return warnings, nil + } + + return warnings, v.adcValidator.Validate(ctx, tls) } func (*ApisixTlsCustomValidator) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) { diff --git a/internal/webhook/v1/apisixtls_webhook_test.go b/internal/webhook/v1/apisixtls_webhook_test.go index 205236f6..c775478d 100644 --- a/internal/webhook/v1/apisixtls_webhook_test.go +++ b/internal/webhook/v1/apisixtls_webhook_test.go @@ -17,6 +17,7 @@ package v1 import ( "context" + "net/http" "testing" "github.com/stretchr/testify/require" @@ -27,6 +28,7 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client/fake" + apisixv1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1" apisixv2 "github.com/apache/apisix-ingress-controller/api/v2" "github.com/apache/apisix-ingress-controller/internal/controller/config" ) @@ -37,10 +39,20 @@ func buildApisixTlsValidator(t *testing.T, objects ...runtime.Object) *ApisixTls scheme := runtime.NewScheme() require.NoError(t, clientgoscheme.AddToScheme(scheme)) require.NoError(t, networkingv1.AddToScheme(scheme)) + require.NoError(t, apisixv1alpha1.AddToScheme(scheme)) require.NoError(t, apisixv2.AddToScheme(scheme)) - managed := []runtime.Object{ - &networkingv1.IngressClass{ + managed := []runtime.Object{} + hasManagedIngressClass := false + for _, obj := range objects { + ingressClass, ok := obj.(*networkingv1.IngressClass) + if ok && ingressClass.Name == "apisix" { + hasManagedIngressClass = true + break + } + } + if !hasManagedIngressClass { + managed = append(managed, &networkingv1.IngressClass{ ObjectMeta: metav1.ObjectMeta{ Name: "apisix", Annotations: map[string]string{ @@ -50,7 +62,7 @@ func buildApisixTlsValidator(t *testing.T, objects ...runtime.Object) *ApisixTls Spec: networkingv1.IngressClassSpec{ Controller: config.ControllerConfig.ControllerName, }, - }, + }) } allObjects := append(managed, objects...) builder := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(allObjects...) @@ -129,3 +141,30 @@ func TestApisixTlsValidator_NoWarningsWhenSecretsExist(t *testing.T) { require.NoError(t, err) require.Empty(t, warnings) } + +func TestApisixTlsValidator_DeniesOnADCValidationFailure(t *testing.T) { + serverURL := withMockADCServer(t, func(w http.ResponseWriter, r *http.Request) { + requireValidateRequest(t, r) + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"message":"tls rejected","errors":[{"resource_type":"ssls","resource_name":"demo","message":"invalid sni"}]}`)) + }) + + tls := newApisixTls() + + objects := append(managedIngressClassWithGatewayProxy(serverURL), + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "server-cert", Namespace: "default"}, + Data: map[string][]byte{ + corev1.TLSCertKey: []byte("cert"), + corev1.TLSPrivateKeyKey: []byte("key"), + }, + }, + ) + + validator := buildApisixTlsValidator(t, objects...) + + warnings, err := validator.ValidateCreate(context.Background(), tls) + require.Error(t, err) + require.Contains(t, err.Error(), "tls rejected") + require.Empty(t, warnings) +} diff --git a/internal/webhook/v1/consumer_webhook.go b/internal/webhook/v1/consumer_webhook.go index f9b3bd77..57767520 100644 --- a/internal/webhook/v1/consumer_webhook.go +++ b/internal/webhook/v1/consumer_webhook.go @@ -17,8 +17,11 @@ package v1 import ( "context" + "encoding/json" "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" @@ -29,6 +32,7 @@ import ( apisixv1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1" "github.com/apache/apisix-ingress-controller/internal/controller" + "github.com/apache/apisix-ingress-controller/internal/controller/indexer" "github.com/apache/apisix-ingress-controller/internal/webhook/v1/reference" ) @@ -41,19 +45,24 @@ func SetupConsumerWebhookWithManager(mgr ctrl.Manager) error { Complete() } -// +kubebuilder:webhook:path=/validate-apisix-apache-org-v1alpha1-consumer,mutating=false,failurePolicy=fail,sideEffects=None,groups=apisix.apache.org,resources=consumers,verbs=create;update,versions=v1alpha1,name=vconsumer-v1alpha1.kb.io,admissionReviewVersions=v1,failurePolicy=Ignore +// +kubebuilder:webhook:path=/validate-apisix-apache-org-v1alpha1-consumer,mutating=false,failurePolicy=Ignore,sideEffects=None,groups=apisix.apache.org,resources=consumers,verbs=create;update,versions=v1alpha1,name=vconsumer-v1alpha1.kb.io,admissionReviewVersions=v1 type ConsumerCustomValidator struct { - Client client.Client - checker reference.Checker + Client client.Client + checker reference.Checker + adcValidator *adcAdmissionValidator + initErr error } var _ webhook.CustomValidator = &ConsumerCustomValidator{} func NewConsumerCustomValidator(c client.Client) *ConsumerCustomValidator { + adcValidator, err := newADCAdmissionValidator(c, consumerLog) return &ConsumerCustomValidator{ - Client: c, - checker: reference.NewChecker(c, consumerLog), + Client: c, + checker: reference.NewChecker(c, consumerLog), + adcValidator: adcValidator, + initErr: err, } } @@ -67,7 +76,15 @@ func (v *ConsumerCustomValidator) ValidateCreate(ctx context.Context, obj runtim return nil, nil } - return v.collectWarnings(ctx, consumer), nil + warnings := v.collectWarnings(ctx, consumer) + if v.initErr != nil { + consumerLog.Error(v.initErr, "ADC validator init failed, skipping ADC validation") + return warnings, nil + } + if err := v.validateDuplicateKeyAuthCredentials(ctx, consumer); err != nil { + return warnings, err + } + return warnings, v.adcValidator.Validate(ctx, consumer) } func (v *ConsumerCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { @@ -80,7 +97,15 @@ func (v *ConsumerCustomValidator) ValidateUpdate(ctx context.Context, oldObj, ne return nil, nil } - return v.collectWarnings(ctx, consumer), nil + warnings := v.collectWarnings(ctx, consumer) + if v.initErr != nil { + consumerLog.Error(v.initErr, "ADC validator init failed, skipping ADC validation") + return warnings, nil + } + if err := v.validateDuplicateKeyAuthCredentials(ctx, consumer); err != nil { + return warnings, err + } + return warnings, v.adcValidator.Validate(ctx, consumer) } func (*ConsumerCustomValidator) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) { @@ -117,3 +142,100 @@ func (v *ConsumerCustomValidator) collectWarnings(ctx context.Context, consumer return warnings } + +func (v *ConsumerCustomValidator) validateDuplicateKeyAuthCredentials(ctx context.Context, consumer *apisixv1alpha1.Consumer) error { + keys, err := v.extractKeyAuthKeys(ctx, consumer) + if err != nil { + return err + } + if len(keys) == 0 { + return nil + } + + // Use the consumerGatewayRef field index to list only Consumers sharing the same gateway. + ns := consumer.Namespace + if consumer.Spec.GatewayRef.Namespace != nil && *consumer.Spec.GatewayRef.Namespace != "" { + ns = *consumer.Spec.GatewayRef.Namespace + } + indexKey := indexer.GenIndexKey(ns, consumer.Spec.GatewayRef.Name) + + var consumers apisixv1alpha1.ConsumerList + if err := v.Client.List(ctx, &consumers, client.MatchingFields{indexer.ConsumerGatewayRef: indexKey}); err != nil { + return err + } + + for i := range consumers.Items { + existing := &consumers.Items[i] + if existing.Namespace == consumer.Namespace && existing.Name == consumer.Name { + continue + } + + existingKeys, err := v.extractKeyAuthKeys(ctx, existing) + if err != nil { + return err + } + for key := range existingKeys { + if _, ok := keys[key]; ok { + return fmt.Errorf("duplicate key-auth credential key %q already used by Consumer %s/%s", key, existing.Namespace, existing.Name) + } + } + } + + return nil +} + +func (v *ConsumerCustomValidator) extractKeyAuthKeys(ctx context.Context, consumer *apisixv1alpha1.Consumer) (map[string]struct{}, error) { + keys := make(map[string]struct{}) + + for _, credential := range consumer.Spec.Credentials { + if credential.Type != "key-auth" { + continue + } + + key, err := v.extractCredentialKey(ctx, consumer, credential) + if err != nil { + return nil, err + } + if key == "" { + continue + } + keys[key] = struct{}{} + } + + return keys, nil +} + +func (v *ConsumerCustomValidator) extractCredentialKey(ctx context.Context, consumer *apisixv1alpha1.Consumer, credential apisixv1alpha1.Credential) (string, error) { + if credential.SecretRef != nil && credential.SecretRef.Name != "" { + namespace := consumer.Namespace + if credential.SecretRef.Namespace != nil && *credential.SecretRef.Namespace != "" { + namespace = *credential.SecretRef.Namespace + } + + var secret corev1.Secret + err := v.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: credential.SecretRef.Name}, &secret) + if err != nil { + if k8serrors.IsNotFound(err) { + return "", nil + } + return "", err + } + return string(secret.Data["key"]), nil + } + + if len(credential.Config.Raw) == 0 { + return "", nil + } + + var cfg struct { + Key string `json:"key"` + } + if err := json.Unmarshal(credential.Config.Raw, &cfg); err != nil { + // Malformed JSON is not a hard error: skip duplicate detection for this + // credential so existing consumers with bad config are not suddenly denied. + consumerLog.V(1).Info("skipping duplicate key-auth check: malformed credential config", + "consumer", consumer.Name, "error", err) + return "", nil + } + return cfg.Key, nil +} diff --git a/internal/webhook/v1/consumer_webhook_test.go b/internal/webhook/v1/consumer_webhook_test.go index 045bc12b..4dc32b84 100644 --- a/internal/webhook/v1/consumer_webhook_test.go +++ b/internal/webhook/v1/consumer_webhook_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -29,6 +30,7 @@ import ( apisixv1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1" "github.com/apache/apisix-ingress-controller/internal/controller/config" + "github.com/apache/apisix-ingress-controller/internal/controller/indexer" ) func buildConsumerValidator(t *testing.T, objects ...runtime.Object) *ConsumerCustomValidator { @@ -54,7 +56,10 @@ func buildConsumerValidator(t *testing.T, objects ...runtime.Object) *ConsumerCu }, } allObjects := append(managed, objects...) - builder := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(allObjects...) + builder := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(allObjects...). + WithIndex(&apisixv1alpha1.Consumer{}, indexer.ConsumerGatewayRef, indexer.ConsumerGatewayRefIndexFunc) return NewConsumerCustomValidator(builder.Build()) } @@ -146,3 +151,44 @@ func TestConsumerValidator_NoWarnings(t *testing.T) { require.NoError(t, err) require.Empty(t, warnings) } + +func TestConsumerValidator_DenyDuplicateKeyAuthCredential(t *testing.T) { + existing := &apisixv1alpha1.Consumer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing", + Namespace: "default", + }, + Spec: apisixv1alpha1.ConsumerSpec{ + GatewayRef: apisixv1alpha1.GatewayRef{Name: "test-gateway"}, + Credentials: []apisixv1alpha1.Credential{{ + Type: "key-auth", + Config: apiextensionsv1.JSON{ + Raw: []byte(`{"key":"shared-key"}`), + }, + }}, + }, + } + consumer := &apisixv1alpha1.Consumer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + Namespace: "default", + }, + Spec: apisixv1alpha1.ConsumerSpec{ + GatewayRef: apisixv1alpha1.GatewayRef{Name: "test-gateway"}, + Credentials: []apisixv1alpha1.Credential{{ + Type: "key-auth", + Config: apiextensionsv1.JSON{ + Raw: []byte(`{"key":"shared-key"}`), + }, + }}, + }, + } + + validator := buildConsumerValidator(t, existing) + + warnings, err := validator.ValidateCreate(context.Background(), consumer) + require.Empty(t, warnings) + require.Error(t, err) + require.Contains(t, err.Error(), `duplicate key-auth credential key "shared-key"`) + require.Contains(t, err.Error(), "default/existing") +} diff --git a/test/e2e/crds/v2/tls.go b/test/e2e/crds/v2/tls.go index c2a6e770..b8282d76 100644 --- a/test/e2e/crds/v2/tls.go +++ b/test/e2e/crds/v2/tls.go @@ -424,22 +424,25 @@ spec: assert.True(GinkgoT(), sniFound["api7.com"], "api7.com should be in SNIs") By("test HTTPS request to api6.com") - Eventually(func() int { - return s.NewAPISIXHttpsClient("api6.com"). - GET("/get"). - WithHost("api6.com"). - Expect(). - Raw().StatusCode - }).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(Equal(http.StatusOK)) + s.RequestAssert(&scaffold.RequestAssert{ + Client: s.NewAPISIXHttpsClient("api6.com"), + Path: "/get", + Host: "api6.com", + Checks: []scaffold.ResponseCheckFunc{scaffold.WithExpectedStatus(http.StatusOK)}, + Timeout: 30 * time.Second, + }) By("test HTTPS request to api7.com") - Eventually(func() int { - return s.NewAPISIXHttpsClient("api7.com"). - GET("/get"). - WithHost("api7.com"). - Expect(). - Raw().StatusCode - }).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(Equal(http.StatusOK)) + // Use RequestAssert so that transient TLS errors while the data plane + // is loading the freshly-created SSL object are retried via ErrorReporter + // instead of causing an immediate fatal failure through GinkgoT(). + s.RequestAssert(&scaffold.RequestAssert{ + Client: s.NewAPISIXHttpsClient("api7.com"), + Path: "/get", + Host: "api7.com", + Checks: []scaffold.ResponseCheckFunc{scaffold.WithExpectedStatus(http.StatusOK)}, + Timeout: 30 * time.Second, + }) }) }) diff --git a/test/e2e/webhook/apisixconsumer.go b/test/e2e/webhook/apisixconsumer.go index 7aa1a256..364e8784 100644 --- a/test/e2e/webhook/apisixconsumer.go +++ b/test/e2e/webhook/apisixconsumer.go @@ -19,11 +19,13 @@ package webhook import ( "fmt" + "strings" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/apache/apisix-ingress-controller/test/e2e/framework" "github.com/apache/apisix-ingress-controller/test/e2e/scaffold" ) @@ -45,7 +47,7 @@ var _ = Describe("Test ApisixConsumer Webhook", Label("webhook"), func() { time.Sleep(5 * time.Second) }) - It("should warn on missing authentication secrets", func() { + It("should warn on missing authentication secrets", func() { //nolint:dupl missingSecret := "missing-basic-secret" consumerName := "webhook-apisixconsumer" consumerYAML := ` @@ -85,4 +87,113 @@ stringData: Expect(err).ShouldNot(HaveOccurred()) Expect(output).NotTo(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), missingSecret))) }) + + It("should reject invalid plugin config during ADC validation", func() { + privateKeyYAML := " " + strings.ReplaceAll(framework.TestKey, "\n", "\n ") + + firstConsumer := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixConsumer +metadata: + name: webhook-apisixconsumer-a + namespace: %s +spec: + ingressClassName: %s + authParameter: + keyAuth: + value: + key: consumer-a-key +`, s.Namespace(), s.Namespace()) + + By("creating the first ApisixConsumer with valid key-auth config") + err := s.CreateResourceFromString(firstConsumer) + Expect(err).NotTo(HaveOccurred(), "creating first ApisixConsumer") + + invalidConsumer := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixConsumer +metadata: + name: webhook-apisixconsumer-b + namespace: %s +spec: + ingressClassName: %s + authParameter: + jwtAuth: + value: + key: consumer-b-key + algorithm: INVALID_ALGO + private_key: | +%s +`, s.Namespace(), s.Namespace(), privateKeyYAML) + + By("creating ApisixConsumer with an invalid jwt-auth algorithm") + err = s.CreateResourceFromString(invalidConsumer) + expectAdmissionDenied(s, "apisixconsumer", "webhook-apisixconsumer-b", err) + + correctedConsumer := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixConsumer +metadata: + name: webhook-apisixconsumer-b + namespace: %s +spec: + ingressClassName: %s + authParameter: + keyAuth: + value: + key: consumer-b-corrected-key +`, s.Namespace(), s.Namespace()) + + By("creating corrected ApisixConsumer with valid auth config") + err = s.CreateResourceFromString(correctedConsumer) + Expect(err).NotTo(HaveOccurred(), "creating corrected ApisixConsumer") + }) + + It("should reject consumer update that fails ADC validation", func() { + consumerName := "webhook-apisixconsumer-update" + + validConsumer := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixConsumer +metadata: + name: %s + namespace: %s +spec: + ingressClassName: %s + authParameter: + keyAuth: + value: + key: update-test-key +`, consumerName, s.Namespace(), s.Namespace()) + + By("creating valid ApisixConsumer") + err := s.CreateResourceFromString(validConsumer) + Expect(err).NotTo(HaveOccurred(), "creating initial valid ApisixConsumer") + + privateKeyYAML := " " + strings.ReplaceAll(framework.TestKey, "\n", "\n ") + invalidConsumer := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixConsumer +metadata: + name: %s + namespace: %s +spec: + ingressClassName: %s + authParameter: + jwtAuth: + value: + key: update-test-jwt-key + algorithm: INVALID_ALGO + private_key: | +%s +`, consumerName, s.Namespace(), s.Namespace(), privateKeyYAML) + + By("updating ApisixConsumer with invalid jwt-auth algorithm") + err = s.CreateResourceFromString(invalidConsumer) + expectUpdateDenied(err) + + By("updating ApisixConsumer with corrected config") + err = s.CreateResourceFromString(validConsumer) + Expect(err).NotTo(HaveOccurred(), "updating ApisixConsumer with corrected config") + }) }) diff --git a/test/e2e/webhook/apisixroute.go b/test/e2e/webhook/apisixroute.go index 51904f43..2e498f5f 100644 --- a/test/e2e/webhook/apisixroute.go +++ b/test/e2e/webhook/apisixroute.go @@ -45,9 +45,8 @@ var _ = Describe("Test ApisixRoute Webhook", Label("webhook"), func() { time.Sleep(5 * time.Second) }) - It("should warn on missing service or secret references", func() { + It("should warn on missing service references", func() { //nolint:dupl missingService := "missing-backend" - missingSecret := "missing-plugin-secret" routeName := "webhook-apisixroute" routeYAML := ` apiVersion: apisix.apache.org/v2 @@ -67,18 +66,13 @@ spec: backends: - serviceName: %s servicePort: 80 - plugins: - - name: echo - enable: true - secretRef: %s ` - output, err := s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(routeYAML, routeName, s.Namespace(), s.Namespace(), missingService, missingSecret)) + output, err := s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(routeYAML, routeName, s.Namespace(), s.Namespace(), missingService)) Expect(err).ShouldNot(HaveOccurred()) Expect(output).To(ContainSubstring(fmt.Sprintf("Warning: Referenced Service '%s/%s' not found", s.Namespace(), missingService))) - Expect(output).To(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), missingSecret))) - By("creating referenced Service and Secret") + By("creating referenced Service") serviceYAML := fmt.Sprintf(` apiVersion: v1 kind: Service @@ -96,22 +90,170 @@ spec: err = s.CreateResourceFromString(serviceYAML) Expect(err).NotTo(HaveOccurred(), "creating backend service placeholder") - secretYAML := fmt.Sprintf(` + time.Sleep(2 * time.Second) + + output, err = s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(routeYAML, routeName, s.Namespace(), s.Namespace(), missingService)) + Expect(err).ShouldNot(HaveOccurred()) + Expect(output).NotTo(ContainSubstring(fmt.Sprintf("Warning: Referenced Service '%s/%s' not found", s.Namespace(), missingService))) + }) + + It("should reject routes that fail ADC validation", func() { + backendService := "webhook-route-backend" + routeName := "webhook-apisixroute-invalid" + + By("creating referenced Service") + serviceYAML := fmt.Sprintf(` apiVersion: v1 -kind: Secret +kind: Service metadata: name: %s -stringData: - config: enabled -`, missingSecret) - err = s.CreateResourceFromString(secretYAML) - Expect(err).NotTo(HaveOccurred(), "creating plugin secret placeholder") +spec: + selector: + app: placeholder + ports: + - name: http + port: 80 + targetPort: 80 + type: ClusterIP +`, backendService) + err := s.CreateResourceFromString(serviceYAML) + Expect(err).NotTo(HaveOccurred(), "creating backend service") - time.Sleep(2 * time.Second) + invalidRouteYAML := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: %s + namespace: %s +spec: + ingressClassName: %s + http: + - name: rule-invalid + match: + hosts: + - webhook.example.com + paths: + - /invalid + backends: + - serviceName: %s + servicePort: 80 + resolveGranularity: service + plugins: + - name: response-rewrite + enable: true + config: + status_code: "500" +`, routeName, s.Namespace(), s.Namespace(), backendService) - output, err = s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(routeYAML, routeName, s.Namespace(), s.Namespace(), missingService, missingSecret)) - Expect(err).ShouldNot(HaveOccurred()) - Expect(output).NotTo(ContainSubstring(fmt.Sprintf("Warning: Referenced Service '%s/%s' not found", s.Namespace(), missingService))) - Expect(output).NotTo(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), missingSecret))) + By("creating ApisixRoute with invalid plugin config") + err = s.CreateResourceFromString(invalidRouteYAML) + expectAdmissionDenied(s, "apisixroute", routeName, err) + + validRouteYAML := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: %s + namespace: %s +spec: + ingressClassName: %s + http: + - name: rule-valid + match: + hosts: + - webhook.example.com + paths: + - /valid + backends: + - serviceName: %s + servicePort: 80 + resolveGranularity: service +`, routeName, s.Namespace(), s.Namespace(), backendService) + + By("creating corrected ApisixRoute") + err = s.CreateResourceFromString(validRouteYAML) + Expect(err).NotTo(HaveOccurred(), "creating corrected ApisixRoute") + }) + + It("should reject route update that fails ADC validation", func() { + backendService := "webhook-route-update-backend" + routeName := "webhook-apisixroute-update" + + By("creating referenced Service") + serviceYAML := fmt.Sprintf(` +apiVersion: v1 +kind: Service +metadata: + name: %s +spec: + selector: + app: placeholder + ports: + - name: http + port: 80 + targetPort: 80 + type: ClusterIP +`, backendService) + err := s.CreateResourceFromString(serviceYAML) + Expect(err).NotTo(HaveOccurred(), "creating backend service") + + validRouteYAML := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: %s + namespace: %s +spec: + ingressClassName: %s + http: + - name: rule-update + match: + hosts: + - webhook-update.example.com + paths: + - /update + backends: + - serviceName: %s + servicePort: 80 + resolveGranularity: service +`, routeName, s.Namespace(), s.Namespace(), backendService) + + By("creating valid ApisixRoute") + err = s.CreateResourceFromString(validRouteYAML) + Expect(err).NotTo(HaveOccurred(), "creating initial valid ApisixRoute") + + invalidRouteYAML := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: %s + namespace: %s +spec: + ingressClassName: %s + http: + - name: rule-update + match: + hosts: + - webhook-update.example.com + paths: + - /update + backends: + - serviceName: %s + servicePort: 80 + resolveGranularity: service + plugins: + - name: response-rewrite + enable: true + config: + status_code: "500" +`, routeName, s.Namespace(), s.Namespace(), backendService) + + By("updating ApisixRoute with invalid plugin config") + err = s.CreateResourceFromString(invalidRouteYAML) + expectUpdateDenied(err) + + By("updating ApisixRoute with corrected config") + err = s.CreateResourceFromString(validRouteYAML) + Expect(err).NotTo(HaveOccurred(), "updating ApisixRoute with corrected config") }) }) diff --git a/test/e2e/webhook/apisixtls.go b/test/e2e/webhook/apisixtls.go index 08defed9..0d24d0f8 100644 --- a/test/e2e/webhook/apisixtls.go +++ b/test/e2e/webhook/apisixtls.go @@ -73,8 +73,30 @@ spec: Expect(output).To(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), serverSecret))) Expect(output).To(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), clientSecret))) - By("creating referenced TLS secrets") - serverSecretYAML := fmt.Sprintf(` + By("creating referenced TLS secrets with valid certificate material") + serverCert, serverKey := s.GenerateCert(GinkgoT(), []string{"webhook.example.com"}) + err = s.NewKubeTlsSecret(serverSecret, serverCert.String(), serverKey.String()) + Expect(err).NotTo(HaveOccurred(), "creating server TLS secret") + + caCert, _, _, _, _ := s.GenerateMACert(GinkgoT(), []string{"webhook.example.com"}) + err = s.NewClientCASecret(clientSecret, caCert.String(), "") + Expect(err).NotTo(HaveOccurred(), "creating client CA secret") + + time.Sleep(2 * time.Second) + + output, err = s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(tlsYAML, tlsName, s.Namespace(), s.Namespace(), serverSecret, s.Namespace(), clientSecret, s.Namespace())) + Expect(err).ShouldNot(HaveOccurred()) + Expect(output).NotTo(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), serverSecret))) + Expect(output).NotTo(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), clientSecret))) + }) + + It("should reject invalid TLS material during ADC validation", func() { + serverSecret := "invalid-server-tls" + tlsName := "webhook-apisixtls-invalid" + host := "invalid-webhook.example.com" + + By("creating a referenced TLS secret with invalid certificate data") + invalidServerSecretYAML := fmt.Sprintf(` apiVersion: v1 kind: Secret metadata: @@ -82,30 +104,113 @@ metadata: namespace: %s type: kubernetes.io/tls stringData: - tls.crt: dummy-cert - tls.key: dummy-key + tls.crt: not-a-cert + tls.key: not-a-key `, serverSecret, s.Namespace()) - err = s.CreateResourceFromString(serverSecretYAML) - Expect(err).NotTo(HaveOccurred(), "creating server TLS secret") + err := s.CreateResourceFromString(invalidServerSecretYAML) + Expect(err).NotTo(HaveOccurred(), "creating invalid server TLS secret") + + tlsYAML := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixTls +metadata: + name: %s + namespace: %s +spec: + ingressClassName: %s + hosts: + - %s + secret: + name: %s + namespace: %s +`, tlsName, s.Namespace(), s.Namespace(), host, serverSecret, s.Namespace()) + + By("creating ApisixTls backed by invalid certificate material") + err = s.CreateResourceFromString(tlsYAML) + expectAdmissionDenied(s, "apisixtls", tlsName, err) + + By("replacing the secret with valid certificate material") + err = s.DeleteResource("Secret", serverSecret) + Expect(err).NotTo(HaveOccurred(), "deleting invalid server TLS secret") + + serverCert, serverKey := s.GenerateCert(GinkgoT(), []string{host}) + err = s.NewKubeTlsSecret(serverSecret, serverCert.String(), serverKey.String()) + Expect(err).NotTo(HaveOccurred(), "creating valid server TLS secret") + + // Wait for the webhook cache to reflect the recreated Secret before submitting ApisixTls. + time.Sleep(2 * time.Second) + + By("creating corrected ApisixTls") + err = s.CreateResourceFromString(tlsYAML) + Expect(err).NotTo(HaveOccurred(), "creating corrected ApisixTls") + }) + + It("should reject TLS update with invalid certificate material", func() { + validSecret := "update-valid-tls" + invalidSecret := "update-invalid-tls" + tlsName := "webhook-apisixtls-update" + host := "update-webhook.example.com" - clientSecretYAML := fmt.Sprintf(` + By("creating a valid TLS secret") + serverCert, serverKey := s.GenerateCert(GinkgoT(), []string{host}) + err := s.NewKubeTlsSecret(validSecret, serverCert.String(), serverKey.String()) + Expect(err).NotTo(HaveOccurred(), "creating valid server TLS secret") + + By("creating an invalid TLS secret with bad certificate material") + invalidSecretYAML := fmt.Sprintf(` apiVersion: v1 kind: Secret metadata: name: %s namespace: %s -type: Opaque +type: kubernetes.io/tls stringData: - ca.crt: dummy-ca -`, clientSecret, s.Namespace()) - err = s.CreateResourceFromString(clientSecretYAML) - Expect(err).NotTo(HaveOccurred(), "creating client CA secret") + tls.crt: not-a-cert + tls.key: not-a-key +`, invalidSecret, s.Namespace()) + err = s.CreateResourceFromString(invalidSecretYAML) + Expect(err).NotTo(HaveOccurred(), "creating invalid server TLS secret") - time.Sleep(2 * time.Second) + validTLSYAML := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixTls +metadata: + name: %s + namespace: %s +spec: + ingressClassName: %s + hosts: + - %s + secret: + name: %s + namespace: %s +`, tlsName, s.Namespace(), s.Namespace(), host, validSecret, s.Namespace()) - output, err = s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(tlsYAML, tlsName, s.Namespace(), s.Namespace(), serverSecret, s.Namespace(), clientSecret, s.Namespace())) - Expect(err).ShouldNot(HaveOccurred()) - Expect(output).NotTo(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), serverSecret))) - Expect(output).NotTo(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), clientSecret))) + By("creating valid ApisixTls") + err = s.CreateResourceFromString(validTLSYAML) + Expect(err).NotTo(HaveOccurred(), "creating initial valid ApisixTls") + + invalidTLSYAML := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixTls +metadata: + name: %s + namespace: %s +spec: + ingressClassName: %s + hosts: + - %s + secret: + name: %s + namespace: %s +`, tlsName, s.Namespace(), s.Namespace(), host, invalidSecret, s.Namespace()) + + By("updating ApisixTls to reference the invalid certificate secret") + err = s.CreateResourceFromString(invalidTLSYAML) + expectUpdateDenied(err) + + By("updating ApisixTls back to the valid certificate secret") + err = s.CreateResourceFromString(validTLSYAML) + Expect(err).NotTo(HaveOccurred(), "updating ApisixTls with valid certificate") }) }) diff --git a/test/e2e/webhook/consumer.go b/test/e2e/webhook/consumer.go index 676adbb8..6a051e25 100644 --- a/test/e2e/webhook/consumer.go +++ b/test/e2e/webhook/consumer.go @@ -90,4 +90,129 @@ stringData: Expect(err).ShouldNot(HaveOccurred()) Expect(output).NotTo(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), missingSecret))) }) + + It("should reject invalid plugin config during ADC validation", func() { + gatewayName := s.Namespace() + + firstConsumer := fmt.Sprintf(` +apiVersion: apisix.apache.org/v1alpha1 +kind: Consumer +metadata: + name: webhook-consumer-a +spec: + gatewayRef: + name: %s + credentials: + - type: key-auth + name: key-auth-a + config: + key: consumer-a-key +`, gatewayName) + + By("creating the first Consumer with valid key-auth config") + err := s.CreateResourceFromString(firstConsumer) + Expect(err).NotTo(HaveOccurred(), "creating first Consumer") + + invalidConsumer := fmt.Sprintf(` +apiVersion: apisix.apache.org/v1alpha1 +kind: Consumer +metadata: + name: webhook-consumer-b +spec: + gatewayRef: + name: %s + plugins: + - name: jwt-auth + config: + key: consumer-b-key + algorithm: INVALID_ALGO +`, gatewayName) + + By("creating Consumer with an invalid jwt-auth algorithm in plugins") + err = s.CreateResourceFromString(invalidConsumer) + expectAdmissionDenied(s, "consumer", "webhook-consumer-b", err) + + correctedConsumer := fmt.Sprintf(` +apiVersion: apisix.apache.org/v1alpha1 +kind: Consumer +metadata: + name: webhook-consumer-b +spec: + gatewayRef: + name: %s + plugins: + - name: jwt-auth + config: + key: consumer-b-key + algorithm: HS256 + secret: consumer-b-secret +`, gatewayName) + + By("creating corrected Consumer with a valid algorithm") + err = s.CreateResourceFromString(correctedConsumer) + Expect(err).NotTo(HaveOccurred(), "creating corrected Consumer") + }) + + It("should reject consumer update that fails ADC validation", func() { + gatewayName := s.Namespace() + consumerName := "webhook-consumer-update" + + validConsumer := fmt.Sprintf(` +apiVersion: apisix.apache.org/v1alpha1 +kind: Consumer +metadata: + name: %s +spec: + gatewayRef: + name: %s + credentials: + - type: key-auth + name: key-auth-update + config: + key: update-consumer-key +`, consumerName, gatewayName) + + By("creating valid Consumer") + err := s.CreateResourceFromString(validConsumer) + Expect(err).NotTo(HaveOccurred(), "creating initial valid Consumer") + + invalidConsumer := fmt.Sprintf(` +apiVersion: apisix.apache.org/v1alpha1 +kind: Consumer +metadata: + name: %s +spec: + gatewayRef: + name: %s + plugins: + - name: jwt-auth + config: + key: update-consumer-jwt-key + algorithm: INVALID_ALGO +`, consumerName, gatewayName) + + By("updating Consumer with an invalid jwt-auth algorithm in plugins") + err = s.CreateResourceFromString(invalidConsumer) + expectUpdateDenied(err) + + correctedConsumer := fmt.Sprintf(` +apiVersion: apisix.apache.org/v1alpha1 +kind: Consumer +metadata: + name: %s +spec: + gatewayRef: + name: %s + plugins: + - name: jwt-auth + config: + key: update-consumer-jwt-key + algorithm: HS256 + secret: update-consumer-secret +`, consumerName, gatewayName) + + By("updating Consumer with a valid algorithm") + err = s.CreateResourceFromString(correctedConsumer) + Expect(err).NotTo(HaveOccurred(), "updating Consumer with corrected config") + }) }) diff --git a/test/e2e/webhook/helpers.go b/test/e2e/webhook/helpers.go index 1b21c8b7..eec76ec7 100644 --- a/test/e2e/webhook/helpers.go +++ b/test/e2e/webhook/helpers.go @@ -235,3 +235,20 @@ spec: Expect(err).ShouldNot(HaveOccurred()) Expect(output).NotTo(ContainSubstring(missingBackendWarning)) } + +func expectAdmissionDenied(s *scaffold.Scaffold, resourceType, resourceName string, err error) { + Expect(err).To(HaveOccurred(), "expecting admission rejection") + Expect(err.Error()).To(ContainSubstring("denied the request")) + + _, getErr := s.GetOutputFromString(resourceType, resourceName, "-o", "yaml") + Expect(getErr).To(HaveOccurred(), fmt.Sprintf("resource %s/%s should not exist after admission rejection", resourceType, resourceName)) + Expect(getErr.Error()).To(ContainSubstring("not found"), fmt.Sprintf("expected NotFound error for %s/%s", resourceType, resourceName)) +} + +// expectUpdateDenied verifies that an UPDATE admission was rejected. Unlike +// expectAdmissionDenied it does not check resource non-existence, because the +// resource remains in its previous valid state after a denied update. +func expectUpdateDenied(err error) { + Expect(err).To(HaveOccurred(), "expecting update to be rejected by admission webhook") + Expect(err.Error()).To(ContainSubstring("denied the request")) +}