From 489b34d1b49f8ff887ba2d6bb546911c698ea0c8 Mon Sep 17 00:00:00 2001 From: rongxin Date: Wed, 6 May 2026 17:08:39 +0800 Subject: [PATCH 01/14] feat: add ADC-backed admission validation for APISIX CRDs Validate ApisixRoute, ApisixConsumer, ApisixTls and Consumer resources against a live APISIX instance during admission instead of only producing warnings. Key changes: - Add ADCValidationErrors / ADCValidationError types (internal/types) - Add Validate() to HTTPADCExecutor and Client; introduce the /configs/validate endpoint support in executor.go (TLS min-version set to 1.2, request body no longer logged in full) - Add internal/controller/webhook_validation.go: lightweight helpers (PrepareApisixRouteForValidation, PrepareApisixConsumerForValidation, PrepareConsumerForValidation, PrepareApisixTlsForValidation) that build a TranslateContext without starting the full reconciler loop - Add internal/webhook/v1/adc_validation.go: adcAdmissionValidator that translates a CRD into an ADC payload and posts it to APISIX for structural validation; fails open on infrastructure errors - Wire adcAdmissionValidator into all four webhook validators; init errors are logged and ignored (fail-open) - ApisixTls: skip ADC validation when secrets are missing to preserve the existing warn-only behaviour for that case - Consumer: validate duplicate key-auth credential keys scoped to the same GatewayRef using a field-index query; malformed inline JSON is logged and skipped (not a hard denial) - global_rules and plugin_metadata are now populated in the ADC validate payload so APISIX can resolve plugin references Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/adc/client/client.go | 37 ++ internal/adc/client/executor.go | 378 +++++++++++++++++- internal/adc/client/executor_test.go | 144 +++++++ internal/controller/webhook_validation.go | 113 ++++++ internal/types/error.go | 67 ++++ internal/webhook/v1/adc_validation.go | 233 +++++++++++ internal/webhook/v1/adc_validation_test.go | 98 +++++ internal/webhook/v1/apisixconsumer_webhook.go | 27 +- .../webhook/v1/apisixconsumer_webhook_test.go | 91 ++++- internal/webhook/v1/apisixroute_webhook.go | 27 +- .../webhook/v1/apisixroute_webhook_test.go | 108 ++++- internal/webhook/v1/apisixtls_webhook.go | 30 +- internal/webhook/v1/apisixtls_webhook_test.go | 45 ++- internal/webhook/v1/consumer_webhook.go | 135 ++++++- internal/webhook/v1/consumer_webhook_test.go | 48 ++- 15 files changed, 1542 insertions(+), 39 deletions(-) create mode 100644 internal/adc/client/executor_test.go create mode 100644 internal/controller/webhook_validation.go create mode 100644 internal/webhook/v1/adc_validation.go create mode 100644 internal/webhook/v1/adc_validation_test.go diff --git a/internal/adc/client/client.go b/internal/adc/client/client.go index 2419dde90a..203d9e1495 100644 --- a/internal/adc/client/client.go +++ b/internal/adc/client/client.go @@ -171,6 +171,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() + + args2 := 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, args2); 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 086086112e..a2faafc7b8 100644 --- a/internal/adc/client/executor.go +++ b/internal/adc/client/executor.go @@ -20,12 +20,14 @@ package client import ( "bytes" "context" + "crypto/tls" "encoding/json" "errors" "fmt" "io" "net" "net/http" + "net/url" "os" "strings" "time" @@ -43,6 +45,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 +84,12 @@ type ADCServerOpts struct { CacheKey string `json:"cacheKey"` } +type ADCValidateResult struct { + Success *bool `json:"success,omitempty"` + ErrorMessage string `json:"errorMessage,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 +132,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 +170,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 +220,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 +240,271 @@ 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) + } + + var ( + req *http.Request + httpClient = e.httpClient + ) + if config.BackendType == "apisix-standalone" { + req, err = e.buildAPISIXValidateRequest(ctx, serverAddr, config, resources) + httpClient = e.newBackendHTTPClient(config) + } else { + req, err = e.buildHTTPRequest(ctx, serverAddr, config, labels, types, resources, http.MethodPost, "/validate") + } + if err != nil { + return fmt.Errorf("failed to build validate request: %w", err) + } + + resp, err := 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) +} + +type apisixValidateRequest struct { + Routes []map[string]any `json:"routes,omitempty"` + Services []map[string]any `json:"services,omitempty"` + Consumers []map[string]any `json:"consumers,omitempty"` + SSLs []map[string]any `json:"ssls,omitempty"` + GlobalRules []map[string]any `json:"global_rules,omitempty"` + StreamRoutes []map[string]any `json:"stream_routes,omitempty"` + PluginMetadata []map[string]any `json:"plugin_metadata,omitempty"` + Upstreams []map[string]any `json:"upstreams,omitempty"` +} + +func (e *HTTPADCExecutor) buildAPISIXValidateRequest(ctx context.Context, serverAddr string, config adctypes.Config, resources *adctypes.Resources) (*http.Request, error) { + body, err := buildAPISIXValidatePayload(resources) + if err != nil { + return nil, err + } + + jsonData, err := json.Marshal(body) + if err != nil { + return nil, fmt.Errorf("failed to marshal APISIX validate request body: %w", err) + } + + validateURL, err := url.JoinPath(serverAddr, "/apisix/admin/configs/validate") + if err != nil { + return nil, fmt.Errorf("failed to build APISIX validate URL: %w", err) + } + + e.log.V(1).Info("sending APISIX validate request", + "url", validateURL, + "server", serverAddr, + "cacheKey", config.Name, + "bodyLen", len(jsonData), + ) + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, validateURL, bytes.NewBuffer(jsonData)) + if err != nil { + return nil, fmt.Errorf("failed to create APISIX validate request: %w", err) + } + + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-API-KEY", config.Token) + return req, nil +} + +func (e *HTTPADCExecutor) newBackendHTTPClient(config adctypes.Config) *http.Client { + transport := http.DefaultTransport.(*http.Transport).Clone() + if transport.TLSClientConfig == nil { + transport.TLSClientConfig = &tls.Config{} + } + transport.TLSClientConfig.MinVersion = tls.VersionTLS12 + if !config.TlsVerify { + transport.TLSClientConfig.InsecureSkipVerify = true + } + + return &http.Client{ + Timeout: e.httpClient.Timeout, + Transport: transport, + } +} + +func buildAPISIXValidatePayload(resources *adctypes.Resources) (*apisixValidateRequest, error) { + body := &apisixValidateRequest{} + + for _, service := range resources.Services { + if service == nil { + continue + } + + serviceMap, err := toMap(service) + if err != nil { + return nil, err + } + delete(serviceMap, "routes") + delete(serviceMap, "stream_routes") + delete(serviceMap, "upstreams") + + body.Services = append(body.Services, serviceMap) + + for _, upstream := range service.Upstreams { + upstreamMap, err := toMap(upstream) + if err != nil { + return nil, err + } + body.Upstreams = append(body.Upstreams, upstreamMap) + } + + for _, route := range service.Routes { + routeMap, err := buildAPISIXRouteValidateObject(route) + if err != nil { + return nil, err + } + if service.ID != "" { + routeMap["service_id"] = service.ID + } + body.Routes = append(body.Routes, routeMap) + } + + for _, streamRoute := range service.StreamRoutes { + streamRouteMap, err := toMap(streamRoute) + if err != nil { + return nil, err + } + body.StreamRoutes = append(body.StreamRoutes, streamRouteMap) + } + } + + for _, consumer := range resources.Consumers { + consumerMap, err := buildAPISIXConsumerValidateObject(consumer) + if err != nil { + return nil, err + } + body.Consumers = append(body.Consumers, consumerMap) + } + + for _, ssl := range resources.SSLs { + sslMap, err := buildAPISIXSSLValidateObject(ssl) + if err != nil { + return nil, err + } + body.SSLs = append(body.SSLs, sslMap) + } + + if len(resources.GlobalRules) > 0 { + globalRuleMap, err := toMap(&adctypes.GlobalRuleItem{ + Metadata: adctypes.Metadata{ID: "validation-global-rule"}, + Plugins: adctypes.Plugins(resources.GlobalRules), + }) + if err != nil { + return nil, err + } + body.GlobalRules = append(body.GlobalRules, globalRuleMap) + } + + for pluginName, pluginConfig := range resources.PluginMetadata { + m := map[string]any{"id": pluginName} + if cfg, ok := pluginConfig.(map[string]any); ok { + for k, v := range cfg { + m[k] = v + } + } + body.PluginMetadata = append(body.PluginMetadata, m) + } + + return body, nil +} + +func buildAPISIXRouteValidateObject(route *adctypes.Route) (map[string]any, error) { + routeMap, err := toMap(route) + if err != nil { + return nil, err + } + + delete(routeMap, "description") + return routeMap, nil +} + +func buildAPISIXConsumerValidateObject(consumer *adctypes.Consumer) (map[string]any, error) { + consumerMap, err := toMap(consumer) + if err != nil { + return nil, err + } + + if len(consumer.Credentials) == 0 { + return consumerMap, nil + } + + plugins, ok := consumerMap["plugins"].(map[string]any) + if !ok || plugins == nil { + plugins = make(map[string]any, len(consumer.Credentials)) + } + + for _, credential := range consumer.Credentials { + plugins[credential.Type] = credential.Config + } + + consumerMap["plugins"] = plugins + delete(consumerMap, "credentials") + return consumerMap, nil +} + +func buildAPISIXSSLValidateObject(ssl *adctypes.SSL) (map[string]any, error) { + sslMap, err := toMap(ssl) + if err != nil { + return nil, err + } + + delete(sslMap, "certificates") + + switch len(ssl.Certificates) { + case 0: + return sslMap, nil + case 1: + sslMap["cert"] = ssl.Certificates[0].Certificate + sslMap["key"] = ssl.Certificates[0].Key + default: + sslMap["cert"] = ssl.Certificates[0].Certificate + sslMap["key"] = ssl.Certificates[0].Key + + certs := make([]string, 0, len(ssl.Certificates)-1) + keys := make([]string, 0, len(ssl.Certificates)-1) + for _, certificate := range ssl.Certificates[1:] { + certs = append(certs, certificate.Certificate) + keys = append(keys, certificate.Key) + } + sslMap["certs"] = certs + sslMap["keys"] = keys + } + + return sslMap, nil +} + +func toMap(obj any) (map[string]any, error) { + data, err := json.Marshal(obj) + if err != nil { + return nil, fmt.Errorf("failed to marshal validation object: %w", err) + } + + var out map[string]any + if err := json.Unmarshal(data, &out); err != nil { + return nil, fmt.Errorf("failed to unmarshal validation object: %w", err) + } + return out, nil +} + // 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 +558,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 +584,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 +594,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 +667,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/adc/client/executor_test.go b/internal/adc/client/executor_test.go new file mode 100644 index 0000000000..16fd151170 --- /dev/null +++ b/internal/adc/client/executor_test.go @@ -0,0 +1,144 @@ +// 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 client + +import ( + "testing" + + "github.com/stretchr/testify/require" + + adctypes "github.com/apache/apisix-ingress-controller/api/adc" +) + +func TestBuildAPISIXValidatePayloadConvertsSSLCertificates(t *testing.T) { + body, err := buildAPISIXValidatePayload(&adctypes.Resources{ + SSLs: []*adctypes.SSL{ + { + Metadata: adctypes.Metadata{ID: "ssl-1"}, + Snis: []string{"example.com"}, + Certificates: []adctypes.Certificate{ + { + Certificate: "leaf-cert", + Key: "leaf-key", + }, + { + Certificate: "chain-cert", + Key: "chain-key", + }, + }, + }, + }, + }) + require.NoError(t, err) + require.Len(t, body.SSLs, 1) + + ssl := body.SSLs[0] + require.Equal(t, "ssl-1", ssl["id"]) + require.Equal(t, "leaf-cert", ssl["cert"]) + require.Equal(t, "leaf-key", ssl["key"]) + require.Equal(t, []string{"chain-cert"}, ssl["certs"]) + require.Equal(t, []string{"chain-key"}, ssl["keys"]) + _, ok := ssl["certificates"] + require.False(t, ok) +} + +func TestBuildAPISIXValidatePayloadConvertsSingleSSLCertificate(t *testing.T) { + body, err := buildAPISIXValidatePayload(&adctypes.Resources{ + SSLs: []*adctypes.SSL{ + { + Metadata: adctypes.Metadata{ID: "ssl-1"}, + Snis: []string{"example.com"}, + Certificates: []adctypes.Certificate{ + { + Certificate: "leaf-cert", + Key: "leaf-key", + }, + }, + }, + }, + }) + require.NoError(t, err) + require.Len(t, body.SSLs, 1) + + ssl := body.SSLs[0] + require.Equal(t, "leaf-cert", ssl["cert"]) + require.Equal(t, "leaf-key", ssl["key"]) + _, ok := ssl["certs"] + require.False(t, ok) + _, ok = ssl["keys"] + require.False(t, ok) +} + +func TestBuildAPISIXValidatePayloadStripsRouteDescription(t *testing.T) { + body, err := buildAPISIXValidatePayload(&adctypes.Resources{ + Services: []*adctypes.Service{ + { + Metadata: adctypes.Metadata{ID: "svc-1"}, + Routes: []*adctypes.Route{ + { + Metadata: adctypes.Metadata{ + ID: "route-1", + Desc: "should not be sent to standalone validate", + }, + Uris: []string{"/test"}, + }, + }, + }, + }, + }) + require.NoError(t, err) + require.Len(t, body.Routes, 1) + + route := body.Routes[0] + require.Equal(t, "route-1", route["id"]) + _, ok := route["description"] + require.False(t, ok) + require.Equal(t, "svc-1", route["service_id"]) +} + +func TestBuildAPISIXValidatePayloadConvertsConsumerCredentialsToPlugins(t *testing.T) { + body, err := buildAPISIXValidatePayload(&adctypes.Resources{ + Consumers: []*adctypes.Consumer{ + { + Metadata: adctypes.Metadata{ID: "consumer-1"}, + Username: "demo", + Plugins: adctypes.Plugins{ + "limit-count": map[string]any{"count": 10}, + }, + Credentials: []adctypes.Credential{ + { + Type: "key-auth", + Config: adctypes.Plugins{ + "key": "shared-key", + }, + }, + }, + }, + }, + }) + require.NoError(t, err) + require.Len(t, body.Consumers, 1) + + consumer := body.Consumers[0] + require.Equal(t, "demo", consumer["username"]) + _, ok := consumer["credentials"] + require.False(t, ok) + + plugins, ok := consumer["plugins"].(map[string]any) + require.True(t, ok) + require.Contains(t, plugins, "key-auth") + require.Contains(t, plugins, "limit-count") +} diff --git a/internal/controller/webhook_validation.go b/internal/controller/webhook_validation.go new file mode 100644 index 0000000000..877550afc3 --- /dev/null +++ b/internal/controller/webhook_validation.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 controller + +import ( + "context" + + "github.com/go-logr/logr" + "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 := FindMatchingIngressClass(tctx, c, log, route) + 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, + } + 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 := FindMatchingIngressClass(tctx, c, log, consumer) + 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, + } + 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 := FindMatchingIngressClass(tctx, c, log, tls) + 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 80dbf5689f..1388637da5 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 0000000000..668a65ab78 --- /dev/null +++ b/internal/webhook/v1/adc_validation.go @@ -0,0 +1,233 @@ +// 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" + "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.FindMatchingIngressClass(tctx, v.kubeClient, v.log, obj) + 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 0000000000..a87199dcd9 --- /dev/null +++ b/internal/webhook/v1/adc_validation_test.go @@ -0,0 +1,98 @@ +// 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.MethodPost, r.Method) + require.Equal(t, "/apisix/admin/configs/validate", r.URL.Path) + require.Equal(t, "token", r.Header.Get("X-API-KEY")) +} + +func requireADCServerValidateRequest(t *testing.T, r *http.Request) { + t.Helper() + require.Equal(t, http.MethodPost, 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 b0561259d6..93b4fb2e73 100644 --- a/internal/webhook/v1/apisixconsumer_webhook.go +++ b/internal/webhook/v1/apisixconsumer_webhook.go @@ -45,16 +45,21 @@ func SetupApisixConsumerWebhookWithManager(mgr ctrl.Manager) error { // +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 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,12 @@ 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 + } + return warnings, v.adcValidator.Validate(ctx, consumer) } func (v *ApisixConsumerCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { @@ -82,7 +92,12 @@ 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 + } + 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 8c31768c65..89ab50d232 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(`{"errorMessage":"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) { + requireADCServerValidateRequest(t, r) + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"errorMessage":"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 31ec796e78..298e7285e7 100644 --- a/internal/webhook/v1/apisixroute_webhook.go +++ b/internal/webhook/v1/apisixroute_webhook.go @@ -44,16 +44,21 @@ func SetupApisixRouteWebhookWithManager(mgr ctrl.Manager) error { // +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 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 b8ca3aa222..339791f724 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(`{"errorMessage":"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 985c3b87a2..62d85d1d27 100644 --- a/internal/webhook/v1/apisixtls_webhook.go +++ b/internal/webhook/v1/apisixtls_webhook.go @@ -45,16 +45,21 @@ func SetupApisixTlsWebhookWithManager(mgr ctrl.Manager) error { // +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 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 205236f6ea..f4ff87ff56 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(`{"errorMessage":"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 f9b3bd774a..0e96d245a2 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" ) @@ -44,16 +48,21 @@ func SetupConsumerWebhookWithManager(mgr ctrl.Manager) error { // +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 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,101 @@ 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 045bc12b8a..4dc32b84d9 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") +} From 6ec38eee694036c8a3d7c31e09a6b6f7adddb741 Mon Sep 17 00:00:00 2001 From: rongxin Date: Wed, 6 May 2026 23:34:33 +0800 Subject: [PATCH 02/14] fix: skip ADC validation when reference warnings exist; fix gofmt ApisixRoute and ApisixConsumer: skip ADC validation when collectWarnings already found missing references (services / secrets). This preserves the existing warn-and-admit behaviour for incomplete resources, matching the pattern already used for ApisixTls. Also fix trailing newline in consumer_webhook.go caught by gofmt. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/webhook/v1/apisixconsumer_webhook.go | 6 ++++++ internal/webhook/v1/apisixroute_webhook.go | 6 ++++++ internal/webhook/v1/consumer_webhook.go | 1 - 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/internal/webhook/v1/apisixconsumer_webhook.go b/internal/webhook/v1/apisixconsumer_webhook.go index 93b4fb2e73..34aab6f8ab 100644 --- a/internal/webhook/v1/apisixconsumer_webhook.go +++ b/internal/webhook/v1/apisixconsumer_webhook.go @@ -79,6 +79,9 @@ func (v *ApisixConsumerCustomValidator) ValidateCreate(ctx context.Context, obj 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) } @@ -97,6 +100,9 @@ func (v *ApisixConsumerCustomValidator) ValidateUpdate(ctx context.Context, oldO 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) } diff --git a/internal/webhook/v1/apisixroute_webhook.go b/internal/webhook/v1/apisixroute_webhook.go index 298e7285e7..c84aedcc25 100644 --- a/internal/webhook/v1/apisixroute_webhook.go +++ b/internal/webhook/v1/apisixroute_webhook.go @@ -77,6 +77,9 @@ func (v *ApisixRouteCustomValidator) ValidateCreate(ctx context.Context, obj run apisixRouteLog.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, route) } @@ -95,6 +98,9 @@ func (v *ApisixRouteCustomValidator) ValidateUpdate(ctx context.Context, oldObj, apisixRouteLog.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, route) } diff --git a/internal/webhook/v1/consumer_webhook.go b/internal/webhook/v1/consumer_webhook.go index 0e96d245a2..89014c7ede 100644 --- a/internal/webhook/v1/consumer_webhook.go +++ b/internal/webhook/v1/consumer_webhook.go @@ -239,4 +239,3 @@ func (v *ConsumerCustomValidator) extractCredentialKey(ctx context.Context, cons } return cfg.Key, nil } - From e821d195d839da2fcc535687cbb49d044bca90a2 Mon Sep 17 00:00:00 2001 From: rongxin Date: Thu, 7 May 2026 01:12:58 +0800 Subject: [PATCH 03/14] test(e2e): add ADC validation webhook e2e test cases Add e2e tests covering ADC-backed admission validation for all four webhook resource types. Each new test case is gated with a provider check so it only runs against the apisix-standalone backend. - ApisixRoute: reject route with invalid response-rewrite plugin config - ApisixConsumer: reject consumer with invalid jwt-auth algorithm - ApisixTls: reject TLS resource backed by invalid certificate material - Consumer: reject consumer with invalid jwt-auth algorithm Also add the expectAdmissionDenied helper to the webhook package and update the ApisixTls warn-on-missing test to use real generated certificate material for the success assertion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/e2e/webhook/apisixconsumer.go | 67 ++++++++++++++++++++++++ test/e2e/webhook/apisixroute.go | 83 ++++++++++++++++++++++++++++++ test/e2e/webhook/apisixtls.go | 79 +++++++++++++++++++++------- test/e2e/webhook/consumer.go | 69 +++++++++++++++++++++++++ test/e2e/webhook/helpers.go | 12 +++++ 5 files changed, 291 insertions(+), 19 deletions(-) diff --git a/test/e2e/webhook/apisixconsumer.go b/test/e2e/webhook/apisixconsumer.go index 7aa1a2568a..dbe02c78d4 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" ) @@ -85,4 +87,69 @@ 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() { + if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { + Skip("ADC validation requires apisix-standalone backend") + } + + 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") + }) }) diff --git a/test/e2e/webhook/apisixroute.go b/test/e2e/webhook/apisixroute.go index 51904f43a6..561bd4475b 100644 --- a/test/e2e/webhook/apisixroute.go +++ b/test/e2e/webhook/apisixroute.go @@ -24,6 +24,7 @@ import ( . "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" ) @@ -114,4 +115,86 @@ stringData: 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))) }) + + It("should reject routes that fail ADC validation", func() { + if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { + Skip("ADC validation requires apisix-standalone backend") + } + + backendService := "webhook-route-backend" + routeName := "webhook-apisixroute-invalid" + + 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") + + 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) + + 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") + }) }) diff --git a/test/e2e/webhook/apisixtls.go b/test/e2e/webhook/apisixtls.go index 08defed9e1..5607598f38 100644 --- a/test/e2e/webhook/apisixtls.go +++ b/test/e2e/webhook/apisixtls.go @@ -24,6 +24,7 @@ import ( . "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" ) @@ -73,8 +74,34 @@ 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() { + if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { + Skip("ADC validation requires apisix-standalone backend") + } + + 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 +109,44 @@ 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") - clientSecretYAML := fmt.Sprintf(` -apiVersion: v1 -kind: Secret + tlsYAML := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixTls metadata: name: %s namespace: %s -type: Opaque -stringData: - ca.crt: dummy-ca -`, clientSecret, s.Namespace()) - err = s.CreateResourceFromString(clientSecretYAML) - Expect(err).NotTo(HaveOccurred(), "creating client CA secret") +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) - 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 corrected ApisixTls") + err = s.CreateResourceFromString(tlsYAML) + Expect(err).NotTo(HaveOccurred(), "creating corrected ApisixTls") }) }) diff --git a/test/e2e/webhook/consumer.go b/test/e2e/webhook/consumer.go index 676adbb839..3c64562c2a 100644 --- a/test/e2e/webhook/consumer.go +++ b/test/e2e/webhook/consumer.go @@ -24,6 +24,7 @@ import ( . "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" ) @@ -90,4 +91,72 @@ 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() { + if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { + Skip("ADC validation requires apisix-standalone backend") + } + + 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 + credentials: + - type: jwt-auth + name: jwt-cred + config: + key: consumer-b-key + algorithm: INVALID_ALGO +`, gatewayName) + + By("creating Consumer with an invalid jwt-auth algorithm") + 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 + credentials: + - type: jwt-auth + name: jwt-cred + 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") + }) }) diff --git a/test/e2e/webhook/helpers.go b/test/e2e/webhook/helpers.go index 1b21c8b740..db86352889 100644 --- a/test/e2e/webhook/helpers.go +++ b/test/e2e/webhook/helpers.go @@ -168,6 +168,18 @@ spec: time.Sleep(5 * time.Second) } +func expectAdmissionDenied(s *scaffold.Scaffold, resourceType, resourceName string, err error, messageSubstrings ...string) { + Expect(err).To(HaveOccurred(), "expecting admission rejection") + Expect(err.Error()).To(ContainSubstring("denied the request")) + for _, substring := range messageSubstrings { + Expect(err.Error()).To(ContainSubstring(substring)) + } + + _, 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)) +} + func verifySimpleRouteMissingBackendWarnings(s *scaffold.Scaffold, tc simpleRouteWebhookTestCase) { gatewayName := s.Namespace() routeYAML := fmt.Sprintf(` From a0402c521242fb3d1bbdd302f18057be339ed989 Mon Sep 17 00:00:00 2001 From: rongxin Date: Thu, 7 May 2026 01:41:28 +0800 Subject: [PATCH 04/14] test(e2e): add ADC validation update path webhook tests Add update-path e2e coverage for all four webhook resource types. ValidateUpdate is wired to the same ADC validator as ValidateCreate, and these tests verify that invalid configs are rejected on update too. - ApisixRoute: reject update with invalid response-rewrite plugin config - ApisixConsumer: reject update with invalid jwt-auth algorithm - ApisixTls: reject update when referenced secret contains invalid cert data - Consumer: reject update with invalid jwt-auth algorithm Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/e2e/webhook/apisixconsumer.go | 52 ++++++++++++++++++ test/e2e/webhook/apisixroute.go | 86 ++++++++++++++++++++++++++++++ test/e2e/webhook/apisixtls.go | 72 +++++++++++++++++++++++++ test/e2e/webhook/consumer.go | 69 ++++++++++++++++++++++++ 4 files changed, 279 insertions(+) diff --git a/test/e2e/webhook/apisixconsumer.go b/test/e2e/webhook/apisixconsumer.go index dbe02c78d4..259d912b86 100644 --- a/test/e2e/webhook/apisixconsumer.go +++ b/test/e2e/webhook/apisixconsumer.go @@ -152,4 +152,56 @@ spec: err = s.CreateResourceFromString(correctedConsumer) Expect(err).NotTo(HaveOccurred(), "creating corrected ApisixConsumer") }) + + It("should reject consumer update that fails ADC validation", func() { + if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { + Skip("ADC validation requires apisix-standalone backend") + } + + 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) + expectAdmissionDenied(s, "apisixconsumer", consumerName, 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 561bd4475b..328ad56fda 100644 --- a/test/e2e/webhook/apisixroute.go +++ b/test/e2e/webhook/apisixroute.go @@ -197,4 +197,90 @@ spec: err = s.CreateResourceFromString(validRouteYAML) Expect(err).NotTo(HaveOccurred(), "creating corrected ApisixRoute") }) + + It("should reject route update that fails ADC validation", func() { + if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { + Skip("ADC validation requires apisix-standalone backend") + } + + 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) + expectAdmissionDenied(s, "apisixroute", routeName, 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 5607598f38..e58b176c1c 100644 --- a/test/e2e/webhook/apisixtls.go +++ b/test/e2e/webhook/apisixtls.go @@ -149,4 +149,76 @@ spec: err = s.CreateResourceFromString(tlsYAML) Expect(err).NotTo(HaveOccurred(), "creating corrected ApisixTls") }) + + It("should reject TLS update with invalid certificate material", func() { + if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { + Skip("ADC validation requires apisix-standalone backend") + } + + serverSecret := "update-server-tls" + tlsName := "webhook-apisixtls-update" + host := "update-webhook.example.com" + + By("creating a valid TLS secret") + serverCert, serverKey := s.GenerateCert(GinkgoT(), []string{host}) + err := s.NewKubeTlsSecret(serverSecret, serverCert.String(), serverKey.String()) + Expect(err).NotTo(HaveOccurred(), "creating initial valid 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 valid ApisixTls") + err = s.CreateResourceFromString(tlsYAML) + Expect(err).NotTo(HaveOccurred(), "creating initial valid ApisixTls") + + By("replacing secret with invalid certificate data") + err = s.DeleteResource("Secret", serverSecret) + Expect(err).NotTo(HaveOccurred(), "deleting valid server TLS secret") + invalidSecretYAML := fmt.Sprintf(` +apiVersion: v1 +kind: Secret +metadata: + name: %s + namespace: %s +type: kubernetes.io/tls +stringData: + tls.crt: not-a-cert + tls.key: not-a-key +`, serverSecret, s.Namespace()) + err = s.CreateResourceFromString(invalidSecretYAML) + Expect(err).NotTo(HaveOccurred(), "creating invalid server TLS secret") + + // Wait for the webhook cache to reflect the replaced Secret. + time.Sleep(2 * time.Second) + + By("updating ApisixTls with secret now containing invalid certificate data") + err = s.CreateResourceFromString(tlsYAML) + expectAdmissionDenied(s, "apisixtls", tlsName, err) + + By("replacing secret back with valid certificate data") + 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(), "recreating valid server TLS secret") + + // Wait for the webhook cache to reflect the restored Secret. + time.Sleep(2 * time.Second) + + By("updating ApisixTls with valid certificate data") + err = s.CreateResourceFromString(tlsYAML) + Expect(err).NotTo(HaveOccurred(), "updating ApisixTls with valid certificate") + }) }) diff --git a/test/e2e/webhook/consumer.go b/test/e2e/webhook/consumer.go index 3c64562c2a..566ee62d89 100644 --- a/test/e2e/webhook/consumer.go +++ b/test/e2e/webhook/consumer.go @@ -159,4 +159,73 @@ spec: err = s.CreateResourceFromString(correctedConsumer) Expect(err).NotTo(HaveOccurred(), "creating corrected Consumer") }) + + It("should reject consumer update that fails ADC validation", func() { + if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { + Skip("ADC validation requires apisix-standalone backend") + } + + 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 + credentials: + - type: jwt-auth + name: jwt-cred-update + config: + key: update-consumer-jwt-key + algorithm: INVALID_ALGO +`, consumerName, gatewayName) + + By("updating Consumer with an invalid jwt-auth algorithm") + err = s.CreateResourceFromString(invalidConsumer) + expectAdmissionDenied(s, "consumer", consumerName, err) + + correctedConsumer := fmt.Sprintf(` +apiVersion: apisix.apache.org/v1alpha1 +kind: Consumer +metadata: + name: %s +spec: + gatewayRef: + name: %s + credentials: + - type: jwt-auth + name: jwt-cred-update + 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") + }) }) From 7396cc7603484175c53a4f324678cd04cde0fa1a Mon Sep 17 00:00:00 2001 From: rongxin Date: Thu, 7 May 2026 02:02:17 +0800 Subject: [PATCH 05/14] fix: remove unused variadic parameter from expectAdmissionDenied Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/e2e/webhook/helpers.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/e2e/webhook/helpers.go b/test/e2e/webhook/helpers.go index db86352889..09c705457c 100644 --- a/test/e2e/webhook/helpers.go +++ b/test/e2e/webhook/helpers.go @@ -168,12 +168,9 @@ spec: time.Sleep(5 * time.Second) } -func expectAdmissionDenied(s *scaffold.Scaffold, resourceType, resourceName string, err error, messageSubstrings ...string) { +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")) - for _, substring := range messageSubstrings { - Expect(err.Error()).To(ContainSubstring(substring)) - } _, getErr := s.GetOutputFromString(resourceType, resourceName, "-o", "yaml") Expect(getErr).To(HaveOccurred(), fmt.Sprintf("resource %s/%s should not exist after admission rejection", resourceType, resourceName)) From 369364bbc3a0643881ba4cd085198806699a9b1e Mon Sep 17 00:00:00 2001 From: rongxin Date: Thu, 7 May 2026 02:54:34 +0800 Subject: [PATCH 06/14] fix(e2e): fix UPDATE path webhook tests - Add expectUpdateDenied helper: UPDATE denials leave the resource intact so the resource-not-found check in expectAdmissionDenied is wrong for update scenarios - Use expectUpdateDenied in all four UPDATE It blocks - Redesign ApisixTls UPDATE test: change the secret reference in the spec instead of swapping secret content; spec must actually change to trigger the UPDATE admission webhook Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/e2e/webhook/apisixconsumer.go | 2 +- test/e2e/webhook/apisixroute.go | 2 +- test/e2e/webhook/apisixtls.go | 75 +++++++++++++++--------------- test/e2e/webhook/consumer.go | 2 +- test/e2e/webhook/helpers.go | 8 ++++ 5 files changed, 49 insertions(+), 40 deletions(-) diff --git a/test/e2e/webhook/apisixconsumer.go b/test/e2e/webhook/apisixconsumer.go index 259d912b86..c11d714473 100644 --- a/test/e2e/webhook/apisixconsumer.go +++ b/test/e2e/webhook/apisixconsumer.go @@ -198,7 +198,7 @@ spec: By("updating ApisixConsumer with invalid jwt-auth algorithm") err = s.CreateResourceFromString(invalidConsumer) - expectAdmissionDenied(s, "apisixconsumer", consumerName, err) + expectUpdateDenied(err) By("updating ApisixConsumer with corrected config") err = s.CreateResourceFromString(validConsumer) diff --git a/test/e2e/webhook/apisixroute.go b/test/e2e/webhook/apisixroute.go index 328ad56fda..3bc356023a 100644 --- a/test/e2e/webhook/apisixroute.go +++ b/test/e2e/webhook/apisixroute.go @@ -277,7 +277,7 @@ spec: By("updating ApisixRoute with invalid plugin config") err = s.CreateResourceFromString(invalidRouteYAML) - expectAdmissionDenied(s, "apisixroute", routeName, err) + expectUpdateDenied(err) By("updating ApisixRoute with corrected config") err = s.CreateResourceFromString(validRouteYAML) diff --git a/test/e2e/webhook/apisixtls.go b/test/e2e/webhook/apisixtls.go index e58b176c1c..a6bc8526ae 100644 --- a/test/e2e/webhook/apisixtls.go +++ b/test/e2e/webhook/apisixtls.go @@ -155,16 +155,32 @@ spec: Skip("ADC validation requires apisix-standalone backend") } - serverSecret := "update-server-tls" + validSecret := "update-valid-tls" + invalidSecret := "update-invalid-tls" tlsName := "webhook-apisixtls-update" host := "update-webhook.example.com" By("creating a valid TLS secret") serverCert, serverKey := s.GenerateCert(GinkgoT(), []string{host}) - err := s.NewKubeTlsSecret(serverSecret, serverCert.String(), serverKey.String()) - Expect(err).NotTo(HaveOccurred(), "creating initial valid server TLS secret") + err := s.NewKubeTlsSecret(validSecret, serverCert.String(), serverKey.String()) + Expect(err).NotTo(HaveOccurred(), "creating valid server TLS secret") - tlsYAML := fmt.Sprintf(` + By("creating an invalid TLS secret with bad certificate material") + invalidSecretYAML := fmt.Sprintf(` +apiVersion: v1 +kind: Secret +metadata: + name: %s + namespace: %s +type: kubernetes.io/tls +stringData: + 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") + + validTLSYAML := fmt.Sprintf(` apiVersion: apisix.apache.org/v2 kind: ApisixTls metadata: @@ -177,48 +193,33 @@ spec: secret: name: %s namespace: %s -`, tlsName, s.Namespace(), s.Namespace(), host, serverSecret, s.Namespace()) +`, tlsName, s.Namespace(), s.Namespace(), host, validSecret, s.Namespace()) By("creating valid ApisixTls") - err = s.CreateResourceFromString(tlsYAML) + err = s.CreateResourceFromString(validTLSYAML) Expect(err).NotTo(HaveOccurred(), "creating initial valid ApisixTls") - By("replacing secret with invalid certificate data") - err = s.DeleteResource("Secret", serverSecret) - Expect(err).NotTo(HaveOccurred(), "deleting valid server TLS secret") - invalidSecretYAML := fmt.Sprintf(` -apiVersion: v1 -kind: Secret + invalidTLSYAML := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixTls metadata: name: %s namespace: %s -type: kubernetes.io/tls -stringData: - tls.crt: not-a-cert - tls.key: not-a-key -`, serverSecret, s.Namespace()) - err = s.CreateResourceFromString(invalidSecretYAML) - Expect(err).NotTo(HaveOccurred(), "creating invalid server TLS secret") - - // Wait for the webhook cache to reflect the replaced Secret. - time.Sleep(2 * time.Second) - - By("updating ApisixTls with secret now containing invalid certificate data") - err = s.CreateResourceFromString(tlsYAML) - expectAdmissionDenied(s, "apisixtls", tlsName, err) - - By("replacing secret back with valid certificate data") - 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(), "recreating valid server TLS secret") +spec: + ingressClassName: %s + hosts: + - %s + secret: + name: %s + namespace: %s +`, tlsName, s.Namespace(), s.Namespace(), host, invalidSecret, s.Namespace()) - // Wait for the webhook cache to reflect the restored Secret. - time.Sleep(2 * time.Second) + By("updating ApisixTls to reference the invalid certificate secret") + err = s.CreateResourceFromString(invalidTLSYAML) + expectUpdateDenied(err) - By("updating ApisixTls with valid certificate data") - err = s.CreateResourceFromString(tlsYAML) + 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 566ee62d89..32bbcfc893 100644 --- a/test/e2e/webhook/consumer.go +++ b/test/e2e/webhook/consumer.go @@ -205,7 +205,7 @@ spec: By("updating Consumer with an invalid jwt-auth algorithm") err = s.CreateResourceFromString(invalidConsumer) - expectAdmissionDenied(s, "consumer", consumerName, err) + expectUpdateDenied(err) correctedConsumer := fmt.Sprintf(` apiVersion: apisix.apache.org/v1alpha1 diff --git a/test/e2e/webhook/helpers.go b/test/e2e/webhook/helpers.go index 09c705457c..cb42c3d1bc 100644 --- a/test/e2e/webhook/helpers.go +++ b/test/e2e/webhook/helpers.go @@ -177,6 +177,14 @@ func expectAdmissionDenied(s *scaffold.Scaffold, resourceType, resourceName stri 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")) +} + func verifySimpleRouteMissingBackendWarnings(s *scaffold.Scaffold, tc simpleRouteWebhookTestCase) { gatewayName := s.Namespace() routeYAML := fmt.Sprintf(` From 4f8e0c6017264a4317d8dab5c557415e92c887cc Mon Sep 17 00:00:00 2001 From: rongxin Date: Thu, 7 May 2026 08:45:45 +0800 Subject: [PATCH 07/14] test: remove apisix-standalone-only skip in ADC validation e2e tests Both apisix and apisix-standalone backends now support /apisix/admin/configs/validate via ADC PR api7/adc#434. The CI uses apache/apisix:dev and ghcr.io/api7/adc:dev which include this support, so the skip guard is no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/e2e/webhook/apisixconsumer.go | 8 -------- test/e2e/webhook/apisixroute.go | 8 -------- test/e2e/webhook/apisixtls.go | 8 -------- test/e2e/webhook/consumer.go | 8 -------- 4 files changed, 32 deletions(-) diff --git a/test/e2e/webhook/apisixconsumer.go b/test/e2e/webhook/apisixconsumer.go index c11d714473..27a0c6a33c 100644 --- a/test/e2e/webhook/apisixconsumer.go +++ b/test/e2e/webhook/apisixconsumer.go @@ -89,10 +89,6 @@ stringData: }) It("should reject invalid plugin config during ADC validation", func() { - if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { - Skip("ADC validation requires apisix-standalone backend") - } - privateKeyYAML := " " + strings.ReplaceAll(framework.TestKey, "\n", "\n ") firstConsumer := fmt.Sprintf(` @@ -154,10 +150,6 @@ spec: }) It("should reject consumer update that fails ADC validation", func() { - if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { - Skip("ADC validation requires apisix-standalone backend") - } - consumerName := "webhook-apisixconsumer-update" validConsumer := fmt.Sprintf(` diff --git a/test/e2e/webhook/apisixroute.go b/test/e2e/webhook/apisixroute.go index 3bc356023a..be7b803ad8 100644 --- a/test/e2e/webhook/apisixroute.go +++ b/test/e2e/webhook/apisixroute.go @@ -117,10 +117,6 @@ stringData: }) It("should reject routes that fail ADC validation", func() { - if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { - Skip("ADC validation requires apisix-standalone backend") - } - backendService := "webhook-route-backend" routeName := "webhook-apisixroute-invalid" @@ -199,10 +195,6 @@ spec: }) It("should reject route update that fails ADC validation", func() { - if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { - Skip("ADC validation requires apisix-standalone backend") - } - backendService := "webhook-route-update-backend" routeName := "webhook-apisixroute-update" diff --git a/test/e2e/webhook/apisixtls.go b/test/e2e/webhook/apisixtls.go index a6bc8526ae..c125e464a3 100644 --- a/test/e2e/webhook/apisixtls.go +++ b/test/e2e/webhook/apisixtls.go @@ -92,10 +92,6 @@ spec: }) It("should reject invalid TLS material during ADC validation", func() { - if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { - Skip("ADC validation requires apisix-standalone backend") - } - serverSecret := "invalid-server-tls" tlsName := "webhook-apisixtls-invalid" host := "invalid-webhook.example.com" @@ -151,10 +147,6 @@ spec: }) It("should reject TLS update with invalid certificate material", func() { - if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { - Skip("ADC validation requires apisix-standalone backend") - } - validSecret := "update-valid-tls" invalidSecret := "update-invalid-tls" tlsName := "webhook-apisixtls-update" diff --git a/test/e2e/webhook/consumer.go b/test/e2e/webhook/consumer.go index 32bbcfc893..78b6e169e8 100644 --- a/test/e2e/webhook/consumer.go +++ b/test/e2e/webhook/consumer.go @@ -93,10 +93,6 @@ stringData: }) It("should reject invalid plugin config during ADC validation", func() { - if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { - Skip("ADC validation requires apisix-standalone backend") - } - gatewayName := s.Namespace() firstConsumer := fmt.Sprintf(` @@ -161,10 +157,6 @@ spec: }) It("should reject consumer update that fails ADC validation", func() { - if framework.ProviderType != framework.ProviderTypeAPISIXStandalone { - Skip("ADC validation requires apisix-standalone backend") - } - gatewayName := s.Namespace() consumerName := "webhook-consumer-update" From 7d5f1bce63b3a891baa83f0d1263fcb6204291bb Mon Sep 17 00:00:00 2001 From: rongxin Date: Thu, 7 May 2026 08:53:46 +0800 Subject: [PATCH 08/14] fix: remove unused framework import in webhook e2e tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/e2e/webhook/apisixroute.go | 1 - test/e2e/webhook/apisixtls.go | 1 - test/e2e/webhook/consumer.go | 1 - 3 files changed, 3 deletions(-) diff --git a/test/e2e/webhook/apisixroute.go b/test/e2e/webhook/apisixroute.go index be7b803ad8..c22dccd524 100644 --- a/test/e2e/webhook/apisixroute.go +++ b/test/e2e/webhook/apisixroute.go @@ -24,7 +24,6 @@ import ( . "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" ) diff --git a/test/e2e/webhook/apisixtls.go b/test/e2e/webhook/apisixtls.go index c125e464a3..0d24d0f8a5 100644 --- a/test/e2e/webhook/apisixtls.go +++ b/test/e2e/webhook/apisixtls.go @@ -24,7 +24,6 @@ import ( . "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" ) diff --git a/test/e2e/webhook/consumer.go b/test/e2e/webhook/consumer.go index 78b6e169e8..075428a042 100644 --- a/test/e2e/webhook/consumer.go +++ b/test/e2e/webhook/consumer.go @@ -24,7 +24,6 @@ import ( . "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" ) From 8052e6dfc6c4286b76203b3702165c9e91948fcd Mon Sep 17 00:00:00 2001 From: rongxin Date: Thu, 7 May 2026 09:42:27 +0800 Subject: [PATCH 09/14] refactor: use ADC server /validate endpoint for all backends Previously the apisix-standalone backend bypassed the ADC server and called APISIX's /apisix/admin/configs/validate directly. With api7/adc#440, the ADC server now exposes a /validate endpoint (same input format as /sync) that handles both apisix-standalone and apisix backends uniformly. Changes: - Remove apisix-standalone special-case in runHTTPValidateForSingleServer; all backends now call ADC server POST /validate - Fix ADCValidateResult.ErrorMessage JSON tag: errorMessage -> message to match the ADC server response format from api7/adc#440 - Remove buildAPISIXValidateRequest, apisixValidateRequest, newBackendHTTPClient, buildAPISIXValidatePayload and helpers - Update unit tests accordingly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/adc/client/executor.go | 241 +----------------- internal/adc/client/executor_test.go | 144 ----------- internal/webhook/v1/adc_validation_test.go | 7 - .../webhook/v1/apisixconsumer_webhook_test.go | 6 +- .../webhook/v1/apisixroute_webhook_test.go | 2 +- internal/webhook/v1/apisixtls_webhook_test.go | 2 +- 6 files changed, 8 insertions(+), 394 deletions(-) delete mode 100644 internal/adc/client/executor_test.go diff --git a/internal/adc/client/executor.go b/internal/adc/client/executor.go index a2faafc7b8..0f2385d8f8 100644 --- a/internal/adc/client/executor.go +++ b/internal/adc/client/executor.go @@ -20,14 +20,12 @@ package client import ( "bytes" "context" - "crypto/tls" "encoding/json" "errors" "fmt" "io" "net" "net/http" - "net/url" "os" "strings" "time" @@ -86,7 +84,7 @@ type ADCServerOpts struct { type ADCValidateResult struct { Success *bool `json:"success,omitempty"` - ErrorMessage string `json:"errorMessage,omitempty"` + ErrorMessage string `json:"message,omitempty"` Errors []types.ADCValidationDetail `json:"errors,omitempty"` } @@ -254,21 +252,12 @@ func (e *HTTPADCExecutor) runHTTPValidateForSingleServer(ctx context.Context, se return fmt.Errorf("failed to load resources from file %s: %w", filePath, err) } - var ( - req *http.Request - httpClient = e.httpClient - ) - if config.BackendType == "apisix-standalone" { - req, err = e.buildAPISIXValidateRequest(ctx, serverAddr, config, resources) - httpClient = e.newBackendHTTPClient(config) - } else { - req, err = e.buildHTTPRequest(ctx, serverAddr, config, labels, types, resources, http.MethodPost, "/validate") - } + req, err := e.buildHTTPRequest(ctx, serverAddr, config, labels, types, resources, http.MethodPost, "/validate") if err != nil { return fmt.Errorf("failed to build validate request: %w", err) } - resp, err := httpClient.Do(req) + resp, err := e.httpClient.Do(req) if err != nil { return fmt.Errorf("failed to send HTTP request: %w", err) } @@ -281,230 +270,6 @@ func (e *HTTPADCExecutor) runHTTPValidateForSingleServer(ctx context.Context, se return e.handleHTTPValidateResponse(resp, serverAddr) } -type apisixValidateRequest struct { - Routes []map[string]any `json:"routes,omitempty"` - Services []map[string]any `json:"services,omitempty"` - Consumers []map[string]any `json:"consumers,omitempty"` - SSLs []map[string]any `json:"ssls,omitempty"` - GlobalRules []map[string]any `json:"global_rules,omitempty"` - StreamRoutes []map[string]any `json:"stream_routes,omitempty"` - PluginMetadata []map[string]any `json:"plugin_metadata,omitempty"` - Upstreams []map[string]any `json:"upstreams,omitempty"` -} - -func (e *HTTPADCExecutor) buildAPISIXValidateRequest(ctx context.Context, serverAddr string, config adctypes.Config, resources *adctypes.Resources) (*http.Request, error) { - body, err := buildAPISIXValidatePayload(resources) - if err != nil { - return nil, err - } - - jsonData, err := json.Marshal(body) - if err != nil { - return nil, fmt.Errorf("failed to marshal APISIX validate request body: %w", err) - } - - validateURL, err := url.JoinPath(serverAddr, "/apisix/admin/configs/validate") - if err != nil { - return nil, fmt.Errorf("failed to build APISIX validate URL: %w", err) - } - - e.log.V(1).Info("sending APISIX validate request", - "url", validateURL, - "server", serverAddr, - "cacheKey", config.Name, - "bodyLen", len(jsonData), - ) - - req, err := http.NewRequestWithContext(ctx, http.MethodPost, validateURL, bytes.NewBuffer(jsonData)) - if err != nil { - return nil, fmt.Errorf("failed to create APISIX validate request: %w", err) - } - - req.Header.Set("Content-Type", "application/json") - req.Header.Set("X-API-KEY", config.Token) - return req, nil -} - -func (e *HTTPADCExecutor) newBackendHTTPClient(config adctypes.Config) *http.Client { - transport := http.DefaultTransport.(*http.Transport).Clone() - if transport.TLSClientConfig == nil { - transport.TLSClientConfig = &tls.Config{} - } - transport.TLSClientConfig.MinVersion = tls.VersionTLS12 - if !config.TlsVerify { - transport.TLSClientConfig.InsecureSkipVerify = true - } - - return &http.Client{ - Timeout: e.httpClient.Timeout, - Transport: transport, - } -} - -func buildAPISIXValidatePayload(resources *adctypes.Resources) (*apisixValidateRequest, error) { - body := &apisixValidateRequest{} - - for _, service := range resources.Services { - if service == nil { - continue - } - - serviceMap, err := toMap(service) - if err != nil { - return nil, err - } - delete(serviceMap, "routes") - delete(serviceMap, "stream_routes") - delete(serviceMap, "upstreams") - - body.Services = append(body.Services, serviceMap) - - for _, upstream := range service.Upstreams { - upstreamMap, err := toMap(upstream) - if err != nil { - return nil, err - } - body.Upstreams = append(body.Upstreams, upstreamMap) - } - - for _, route := range service.Routes { - routeMap, err := buildAPISIXRouteValidateObject(route) - if err != nil { - return nil, err - } - if service.ID != "" { - routeMap["service_id"] = service.ID - } - body.Routes = append(body.Routes, routeMap) - } - - for _, streamRoute := range service.StreamRoutes { - streamRouteMap, err := toMap(streamRoute) - if err != nil { - return nil, err - } - body.StreamRoutes = append(body.StreamRoutes, streamRouteMap) - } - } - - for _, consumer := range resources.Consumers { - consumerMap, err := buildAPISIXConsumerValidateObject(consumer) - if err != nil { - return nil, err - } - body.Consumers = append(body.Consumers, consumerMap) - } - - for _, ssl := range resources.SSLs { - sslMap, err := buildAPISIXSSLValidateObject(ssl) - if err != nil { - return nil, err - } - body.SSLs = append(body.SSLs, sslMap) - } - - if len(resources.GlobalRules) > 0 { - globalRuleMap, err := toMap(&adctypes.GlobalRuleItem{ - Metadata: adctypes.Metadata{ID: "validation-global-rule"}, - Plugins: adctypes.Plugins(resources.GlobalRules), - }) - if err != nil { - return nil, err - } - body.GlobalRules = append(body.GlobalRules, globalRuleMap) - } - - for pluginName, pluginConfig := range resources.PluginMetadata { - m := map[string]any{"id": pluginName} - if cfg, ok := pluginConfig.(map[string]any); ok { - for k, v := range cfg { - m[k] = v - } - } - body.PluginMetadata = append(body.PluginMetadata, m) - } - - return body, nil -} - -func buildAPISIXRouteValidateObject(route *adctypes.Route) (map[string]any, error) { - routeMap, err := toMap(route) - if err != nil { - return nil, err - } - - delete(routeMap, "description") - return routeMap, nil -} - -func buildAPISIXConsumerValidateObject(consumer *adctypes.Consumer) (map[string]any, error) { - consumerMap, err := toMap(consumer) - if err != nil { - return nil, err - } - - if len(consumer.Credentials) == 0 { - return consumerMap, nil - } - - plugins, ok := consumerMap["plugins"].(map[string]any) - if !ok || plugins == nil { - plugins = make(map[string]any, len(consumer.Credentials)) - } - - for _, credential := range consumer.Credentials { - plugins[credential.Type] = credential.Config - } - - consumerMap["plugins"] = plugins - delete(consumerMap, "credentials") - return consumerMap, nil -} - -func buildAPISIXSSLValidateObject(ssl *adctypes.SSL) (map[string]any, error) { - sslMap, err := toMap(ssl) - if err != nil { - return nil, err - } - - delete(sslMap, "certificates") - - switch len(ssl.Certificates) { - case 0: - return sslMap, nil - case 1: - sslMap["cert"] = ssl.Certificates[0].Certificate - sslMap["key"] = ssl.Certificates[0].Key - default: - sslMap["cert"] = ssl.Certificates[0].Certificate - sslMap["key"] = ssl.Certificates[0].Key - - certs := make([]string, 0, len(ssl.Certificates)-1) - keys := make([]string, 0, len(ssl.Certificates)-1) - for _, certificate := range ssl.Certificates[1:] { - certs = append(certs, certificate.Certificate) - keys = append(keys, certificate.Key) - } - sslMap["certs"] = certs - sslMap["keys"] = keys - } - - return sslMap, nil -} - -func toMap(obj any) (map[string]any, error) { - data, err := json.Marshal(obj) - if err != nil { - return nil, fmt.Errorf("failed to marshal validation object: %w", err) - } - - var out map[string]any - if err := json.Unmarshal(data, &out); err != nil { - return nil, fmt.Errorf("failed to unmarshal validation object: %w", err) - } - return out, nil -} - // 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) diff --git a/internal/adc/client/executor_test.go b/internal/adc/client/executor_test.go deleted file mode 100644 index 16fd151170..0000000000 --- a/internal/adc/client/executor_test.go +++ /dev/null @@ -1,144 +0,0 @@ -// 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 client - -import ( - "testing" - - "github.com/stretchr/testify/require" - - adctypes "github.com/apache/apisix-ingress-controller/api/adc" -) - -func TestBuildAPISIXValidatePayloadConvertsSSLCertificates(t *testing.T) { - body, err := buildAPISIXValidatePayload(&adctypes.Resources{ - SSLs: []*adctypes.SSL{ - { - Metadata: adctypes.Metadata{ID: "ssl-1"}, - Snis: []string{"example.com"}, - Certificates: []adctypes.Certificate{ - { - Certificate: "leaf-cert", - Key: "leaf-key", - }, - { - Certificate: "chain-cert", - Key: "chain-key", - }, - }, - }, - }, - }) - require.NoError(t, err) - require.Len(t, body.SSLs, 1) - - ssl := body.SSLs[0] - require.Equal(t, "ssl-1", ssl["id"]) - require.Equal(t, "leaf-cert", ssl["cert"]) - require.Equal(t, "leaf-key", ssl["key"]) - require.Equal(t, []string{"chain-cert"}, ssl["certs"]) - require.Equal(t, []string{"chain-key"}, ssl["keys"]) - _, ok := ssl["certificates"] - require.False(t, ok) -} - -func TestBuildAPISIXValidatePayloadConvertsSingleSSLCertificate(t *testing.T) { - body, err := buildAPISIXValidatePayload(&adctypes.Resources{ - SSLs: []*adctypes.SSL{ - { - Metadata: adctypes.Metadata{ID: "ssl-1"}, - Snis: []string{"example.com"}, - Certificates: []adctypes.Certificate{ - { - Certificate: "leaf-cert", - Key: "leaf-key", - }, - }, - }, - }, - }) - require.NoError(t, err) - require.Len(t, body.SSLs, 1) - - ssl := body.SSLs[0] - require.Equal(t, "leaf-cert", ssl["cert"]) - require.Equal(t, "leaf-key", ssl["key"]) - _, ok := ssl["certs"] - require.False(t, ok) - _, ok = ssl["keys"] - require.False(t, ok) -} - -func TestBuildAPISIXValidatePayloadStripsRouteDescription(t *testing.T) { - body, err := buildAPISIXValidatePayload(&adctypes.Resources{ - Services: []*adctypes.Service{ - { - Metadata: adctypes.Metadata{ID: "svc-1"}, - Routes: []*adctypes.Route{ - { - Metadata: adctypes.Metadata{ - ID: "route-1", - Desc: "should not be sent to standalone validate", - }, - Uris: []string{"/test"}, - }, - }, - }, - }, - }) - require.NoError(t, err) - require.Len(t, body.Routes, 1) - - route := body.Routes[0] - require.Equal(t, "route-1", route["id"]) - _, ok := route["description"] - require.False(t, ok) - require.Equal(t, "svc-1", route["service_id"]) -} - -func TestBuildAPISIXValidatePayloadConvertsConsumerCredentialsToPlugins(t *testing.T) { - body, err := buildAPISIXValidatePayload(&adctypes.Resources{ - Consumers: []*adctypes.Consumer{ - { - Metadata: adctypes.Metadata{ID: "consumer-1"}, - Username: "demo", - Plugins: adctypes.Plugins{ - "limit-count": map[string]any{"count": 10}, - }, - Credentials: []adctypes.Credential{ - { - Type: "key-auth", - Config: adctypes.Plugins{ - "key": "shared-key", - }, - }, - }, - }, - }, - }) - require.NoError(t, err) - require.Len(t, body.Consumers, 1) - - consumer := body.Consumers[0] - require.Equal(t, "demo", consumer["username"]) - _, ok := consumer["credentials"] - require.False(t, ok) - - plugins, ok := consumer["plugins"].(map[string]any) - require.True(t, ok) - require.Contains(t, plugins, "key-auth") - require.Contains(t, plugins, "limit-count") -} diff --git a/internal/webhook/v1/adc_validation_test.go b/internal/webhook/v1/adc_validation_test.go index a87199dcd9..0ad865a87b 100644 --- a/internal/webhook/v1/adc_validation_test.go +++ b/internal/webhook/v1/adc_validation_test.go @@ -85,13 +85,6 @@ func managedIngressClassWithGatewayProxyMode(endpoint, mode string) []runtime.Ob } func requireValidateRequest(t *testing.T, r *http.Request) { - t.Helper() - require.Equal(t, http.MethodPost, r.Method) - require.Equal(t, "/apisix/admin/configs/validate", r.URL.Path) - require.Equal(t, "token", r.Header.Get("X-API-KEY")) -} - -func requireADCServerValidateRequest(t *testing.T, r *http.Request) { t.Helper() require.Equal(t, http.MethodPost, r.Method) require.Equal(t, "/validate", r.URL.Path) diff --git a/internal/webhook/v1/apisixconsumer_webhook_test.go b/internal/webhook/v1/apisixconsumer_webhook_test.go index 89ab50d232..e1be420d15 100644 --- a/internal/webhook/v1/apisixconsumer_webhook_test.go +++ b/internal/webhook/v1/apisixconsumer_webhook_test.go @@ -171,7 +171,7 @@ 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(`{"errorMessage":"consumer rejected","errors":[{"resource_type":"consumers","resource_name":"demo","message":"duplicate credential"}]}`)) + _, _ = w.Write([]byte(`{"message":"consumer rejected","errors":[{"resource_type":"consumers","resource_name":"demo","message":"duplicate credential"}]}`)) }) consumer := &apisixv2.ApisixConsumer{ @@ -208,9 +208,9 @@ func TestApisixConsumerValidator_DeniesOnADCValidationFailure(t *testing.T) { func TestApisixConsumerValidator_UsesADCValidateEndpointForControlPlane(t *testing.T) { serverURL := withMockADCServer(t, func(w http.ResponseWriter, r *http.Request) { - requireADCServerValidateRequest(t, r) + requireValidateRequest(t, r) w.WriteHeader(http.StatusBadRequest) - _, _ = w.Write([]byte(`{"errorMessage":"consumer rejected","errors":[{"resource_type":"consumers","resource_name":"demo","message":"duplicate credential"}]}`)) + _, _ = w.Write([]byte(`{"message":"consumer rejected","errors":[{"resource_type":"consumers","resource_name":"demo","message":"duplicate credential"}]}`)) }) consumer := &apisixv2.ApisixConsumer{ diff --git a/internal/webhook/v1/apisixroute_webhook_test.go b/internal/webhook/v1/apisixroute_webhook_test.go index 339791f724..98bab58011 100644 --- a/internal/webhook/v1/apisixroute_webhook_test.go +++ b/internal/webhook/v1/apisixroute_webhook_test.go @@ -192,7 +192,7 @@ 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(`{"errorMessage":"route rejected","errors":[{"resource_type":"routes","resource_name":"demo","message":"invalid plugin config"}]}`)) + _, _ = w.Write([]byte(`{"message":"route rejected","errors":[{"resource_type":"routes","resource_name":"demo","message":"invalid plugin config"}]}`)) }) route := &apisixv2.ApisixRoute{ diff --git a/internal/webhook/v1/apisixtls_webhook_test.go b/internal/webhook/v1/apisixtls_webhook_test.go index f4ff87ff56..c775478df4 100644 --- a/internal/webhook/v1/apisixtls_webhook_test.go +++ b/internal/webhook/v1/apisixtls_webhook_test.go @@ -146,7 +146,7 @@ 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(`{"errorMessage":"tls rejected","errors":[{"resource_type":"ssls","resource_name":"demo","message":"invalid sni"}]}`)) + _, _ = w.Write([]byte(`{"message":"tls rejected","errors":[{"resource_type":"ssls","resource_name":"demo","message":"invalid sni"}]}`)) }) tls := newApisixTls() From 7f00af7dd985f62307494463c404417807e0c7ec Mon Sep 17 00:00:00 2001 From: rongxin Date: Thu, 7 May 2026 10:59:00 +0800 Subject: [PATCH 10/14] fix: use PUT method for ADC server /validate endpoint The ADC server registers PUT /validate (not POST), matching the same HTTP method used for PUT /sync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/adc/client/executor.go | 2 +- internal/webhook/v1/adc_validation_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/adc/client/executor.go b/internal/adc/client/executor.go index 0f2385d8f8..5664b4f2b5 100644 --- a/internal/adc/client/executor.go +++ b/internal/adc/client/executor.go @@ -252,7 +252,7 @@ func (e *HTTPADCExecutor) runHTTPValidateForSingleServer(ctx context.Context, se return fmt.Errorf("failed to load resources from file %s: %w", filePath, err) } - req, err := e.buildHTTPRequest(ctx, serverAddr, config, labels, types, resources, http.MethodPost, "/validate") + 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) } diff --git a/internal/webhook/v1/adc_validation_test.go b/internal/webhook/v1/adc_validation_test.go index 0ad865a87b..535051d6d9 100644 --- a/internal/webhook/v1/adc_validation_test.go +++ b/internal/webhook/v1/adc_validation_test.go @@ -86,6 +86,6 @@ func managedIngressClassWithGatewayProxyMode(endpoint, mode string) []runtime.Ob func requireValidateRequest(t *testing.T, r *http.Request) { t.Helper() - require.Equal(t, http.MethodPost, r.Method) + require.Equal(t, http.MethodPut, r.Method) require.Equal(t, "/validate", r.URL.Path) } From 1e2bbab2a96299003955fc4b0141334139e5d160 Mon Sep 17 00:00:00 2001 From: rongxin Date: Thu, 7 May 2026 12:02:11 +0800 Subject: [PATCH 11/14] test: use spec.plugins for Consumer ADC validation e2e tests APISIX validates plugin schemas but not credential schemas via /validate. Switch the invalid Consumer config in e2e tests to use spec.plugins with jwt-auth algorithm: INVALID_ALGO, which triggers proper ADC validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/e2e/webhook/consumer.go | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/test/e2e/webhook/consumer.go b/test/e2e/webhook/consumer.go index 075428a042..6a051e2543 100644 --- a/test/e2e/webhook/consumer.go +++ b/test/e2e/webhook/consumer.go @@ -121,15 +121,14 @@ metadata: spec: gatewayRef: name: %s - credentials: - - type: jwt-auth - name: jwt-cred + plugins: + - name: jwt-auth config: key: consumer-b-key algorithm: INVALID_ALGO `, gatewayName) - By("creating Consumer with an invalid jwt-auth algorithm") + By("creating Consumer with an invalid jwt-auth algorithm in plugins") err = s.CreateResourceFromString(invalidConsumer) expectAdmissionDenied(s, "consumer", "webhook-consumer-b", err) @@ -141,9 +140,8 @@ metadata: spec: gatewayRef: name: %s - credentials: - - type: jwt-auth - name: jwt-cred + plugins: + - name: jwt-auth config: key: consumer-b-key algorithm: HS256 @@ -186,15 +184,14 @@ metadata: spec: gatewayRef: name: %s - credentials: - - type: jwt-auth - name: jwt-cred-update + plugins: + - name: jwt-auth config: key: update-consumer-jwt-key algorithm: INVALID_ALGO `, consumerName, gatewayName) - By("updating Consumer with an invalid jwt-auth algorithm") + By("updating Consumer with an invalid jwt-auth algorithm in plugins") err = s.CreateResourceFromString(invalidConsumer) expectUpdateDenied(err) @@ -206,9 +203,8 @@ metadata: spec: gatewayRef: name: %s - credentials: - - type: jwt-auth - name: jwt-cred-update + plugins: + - name: jwt-auth config: key: update-consumer-jwt-key algorithm: HS256 From dd44890720514c9cba2e9a5013db1c857c580093 Mon Sep 17 00:00:00 2001 From: rongxin Date: Fri, 8 May 2026 04:49:20 +0800 Subject: [PATCH 12/14] fix: remove duplicate failurePolicy in webhook markers Remove redundant failurePolicy=fail from all 4 webhook marker lines, keeping only failurePolicy=Ignore (kubebuilder uses the last occurrence, having both was ambiguous and wrong). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/webhook/v1/apisixconsumer_webhook.go | 2 +- internal/webhook/v1/apisixroute_webhook.go | 2 +- internal/webhook/v1/apisixtls_webhook.go | 2 +- internal/webhook/v1/consumer_webhook.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/webhook/v1/apisixconsumer_webhook.go b/internal/webhook/v1/apisixconsumer_webhook.go index 34aab6f8ab..796491f55f 100644 --- a/internal/webhook/v1/apisixconsumer_webhook.go +++ b/internal/webhook/v1/apisixconsumer_webhook.go @@ -42,7 +42,7 @@ 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 diff --git a/internal/webhook/v1/apisixroute_webhook.go b/internal/webhook/v1/apisixroute_webhook.go index c84aedcc25..511c8d84bf 100644 --- a/internal/webhook/v1/apisixroute_webhook.go +++ b/internal/webhook/v1/apisixroute_webhook.go @@ -41,7 +41,7 @@ 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 diff --git a/internal/webhook/v1/apisixtls_webhook.go b/internal/webhook/v1/apisixtls_webhook.go index 62d85d1d27..9180c809c1 100644 --- a/internal/webhook/v1/apisixtls_webhook.go +++ b/internal/webhook/v1/apisixtls_webhook.go @@ -42,7 +42,7 @@ 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 diff --git a/internal/webhook/v1/consumer_webhook.go b/internal/webhook/v1/consumer_webhook.go index 89014c7ede..577675201e 100644 --- a/internal/webhook/v1/consumer_webhook.go +++ b/internal/webhook/v1/consumer_webhook.go @@ -45,7 +45,7 @@ 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 From 01e90042f03f56cc1fbdd94efb83988c038e7c7a Mon Sep 17 00:00:00 2001 From: rongxin Date: Fri, 8 May 2026 09:47:22 +0800 Subject: [PATCH 13/14] fix: always run ADC validation for ApisixRoute regardless of warnings Remove the len(warnings) > 0 guard that was skipping ADC validation when service or secret references were missing. ApisixRoute plugin schemas can be validated by ADC independently of whether the backend service exists. Running ADC even with missing references provides stronger safety guarantees. Also update the warning test to only test missing service references, consistent with the enterprise version. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/webhook/v1/apisixroute_webhook.go | 6 ----- test/e2e/webhook/apisixroute.go | 26 ++++------------------ 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/internal/webhook/v1/apisixroute_webhook.go b/internal/webhook/v1/apisixroute_webhook.go index 511c8d84bf..2702832142 100644 --- a/internal/webhook/v1/apisixroute_webhook.go +++ b/internal/webhook/v1/apisixroute_webhook.go @@ -77,9 +77,6 @@ func (v *ApisixRouteCustomValidator) ValidateCreate(ctx context.Context, obj run apisixRouteLog.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, route) } @@ -98,9 +95,6 @@ func (v *ApisixRouteCustomValidator) ValidateUpdate(ctx context.Context, oldObj, apisixRouteLog.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, route) } diff --git a/test/e2e/webhook/apisixroute.go b/test/e2e/webhook/apisixroute.go index c22dccd524..ffe5c77323 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() { 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,23 +90,11 @@ spec: err = s.CreateResourceFromString(serviceYAML) Expect(err).NotTo(HaveOccurred(), "creating backend service placeholder") - secretYAML := fmt.Sprintf(` -apiVersion: v1 -kind: Secret -metadata: - name: %s -stringData: - config: enabled -`, missingSecret) - err = s.CreateResourceFromString(secretYAML) - Expect(err).NotTo(HaveOccurred(), "creating plugin secret placeholder") - time.Sleep(2 * time.Second) - 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).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))) }) It("should reject routes that fail ADC validation", func() { From 3387bda82c7f4eb83bba3ec7557343d89d944bef Mon Sep 17 00:00:00 2001 From: rongxin Date: Fri, 8 May 2026 10:39:23 +0800 Subject: [PATCH 14/14] fix: add nolint:dupl to suppress false-positive duplicate code warnings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/e2e/webhook/apisixconsumer.go | 2 +- test/e2e/webhook/apisixroute.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/webhook/apisixconsumer.go b/test/e2e/webhook/apisixconsumer.go index 27a0c6a33c..364e87849b 100644 --- a/test/e2e/webhook/apisixconsumer.go +++ b/test/e2e/webhook/apisixconsumer.go @@ -47,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 := ` diff --git a/test/e2e/webhook/apisixroute.go b/test/e2e/webhook/apisixroute.go index ffe5c77323..2e498f5fde 100644 --- a/test/e2e/webhook/apisixroute.go +++ b/test/e2e/webhook/apisixroute.go @@ -45,7 +45,7 @@ var _ = Describe("Test ApisixRoute Webhook", Label("webhook"), func() { time.Sleep(5 * time.Second) }) - It("should warn on missing service references", func() { + It("should warn on missing service references", func() { //nolint:dupl missingService := "missing-backend" routeName := "webhook-apisixroute" routeYAML := `