diff --git a/.github/workflows/apisix-conformance-test.yml b/.github/workflows/apisix-conformance-test.yml index 82495fbb..ad469c40 100644 --- a/.github/workflows/apisix-conformance-test.yml +++ b/.github/workflows/apisix-conformance-test.yml @@ -58,11 +58,11 @@ jobs: go install sigs.k8s.io/kind@v0.23.0 - name: Login to Registry - uses: docker/login-action@v3 - with: - registry: ${{ secrets.DOCKER_REGISTRY }} - username: ${{ secrets.DOCKER_USERNAME }} - password: ${{ secrets.DOCKER_PASSWORD }} + run: | + echo "${{ secrets.DOCKER_PASSWORD }}" | docker login \ + "${{ secrets.DOCKER_REGISTRY }}" \ + --username "${{ secrets.DOCKER_USERNAME }}" \ + --password-stdin - name: Build images env: diff --git a/.github/workflows/apisix-e2e-test.yml b/.github/workflows/apisix-e2e-test.yml index 29a156ad..ad6e503e 100644 --- a/.github/workflows/apisix-e2e-test.yml +++ b/.github/workflows/apisix-e2e-test.yml @@ -57,11 +57,11 @@ jobs: go-version: "1.24" - name: Login to Registry - uses: docker/login-action@v3 - with: - registry: ${{ secrets.DOCKER_REGISTRY }} - username: ${{ secrets.DOCKER_USERNAME }} - password: ${{ secrets.DOCKER_PASSWORD }} + run: | + echo "${{ secrets.DOCKER_PASSWORD }}" | docker login \ + "${{ secrets.DOCKER_REGISTRY }}" \ + --username "${{ secrets.DOCKER_USERNAME }}" \ + --password-stdin - name: Setup Node.js uses: actions/setup-node@v3 diff --git a/.github/workflows/conformance-test.yml b/.github/workflows/conformance-test.yml index 4f3d1422..28aaa4c0 100644 --- a/.github/workflows/conformance-test.yml +++ b/.github/workflows/conformance-test.yml @@ -57,18 +57,18 @@ jobs: ./get_helm.sh - name: Login to Registry - uses: docker/login-action@v3 - with: - registry: ${{ secrets.DOCKER_REGISTRY }} - username: ${{ secrets.DOCKER_USERNAME }} - password: ${{ secrets.DOCKER_PASSWORD }} + run: | + echo "${{ secrets.DOCKER_PASSWORD }}" | docker login \ + "${{ secrets.DOCKER_REGISTRY }}" \ + --username "${{ secrets.DOCKER_USERNAME }}" \ + --password-stdin - name: Login to Private Registry - uses: docker/login-action@v3 - with: - registry: hkccr.ccs.tencentyun.com - username: ${{ secrets.PRIVATE_DOCKER_USERNAME }} - password: ${{ secrets.PRIVATE_DOCKER_PASSWORD }} + run: | + echo "${{ secrets.PRIVATE_DOCKER_PASSWORD }}" | docker login \ + hkccr.ccs.tencentyun.com \ + --username "${{ secrets.PRIVATE_DOCKER_USERNAME }}" \ + --password-stdin - name: Build images env: diff --git a/.github/workflows/e2e-test-k8s.yml b/.github/workflows/e2e-test-k8s.yml index 134d60c6..9b42ed6f 100644 --- a/.github/workflows/e2e-test-k8s.yml +++ b/.github/workflows/e2e-test-k8s.yml @@ -56,23 +56,32 @@ jobs: ./get_helm.sh - name: Login to Private Registry - uses: docker/login-action@v3 - with: - registry: hkccr.ccs.tencentyun.com - username: ${{ secrets.PRIVATE_DOCKER_USERNAME }} - password: ${{ secrets.PRIVATE_DOCKER_PASSWORD }} + run: | + echo "${{ secrets.PRIVATE_DOCKER_PASSWORD }}" | docker login \ + hkccr.ccs.tencentyun.com \ + --username "${{ secrets.PRIVATE_DOCKER_USERNAME }}" \ + --password-stdin - name: Launch Kind Cluster env: KIND_NODE_IMAGE: kindest/node:v1.18.15 run: | + make kind-down make kind-up - KIND_NODE_IP=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' apisix-ingress-cluster-control-plane) - echo $KIND_NODE_IP - - kubectl config get-clusters - kubectl config set-cluster kind-apisix-ingress-cluster --server=https://$KIND_NODE_IP:6443 - kubectl wait --for=condition=Ready nodes --all + mkdir -p "${HOME}/.kube" + kind get kubeconfig --name apisix-ingress-cluster --internal > "${HOME}/.kube/config" + kubectl config use-context kind-apisix-ingress-cluster + for attempt in {1..30}; do + if kubectl cluster-info >/dev/null 2>&1; then + break + fi + if [ "${attempt}" -eq 30 ]; then + echo "kind apiserver did not become reachable in time" >&2 + exit 1 + fi + sleep 2 + done + kubectl wait --for=condition=Ready nodes --all --timeout=120s - name: Build images env: @@ -95,6 +104,23 @@ jobs: - name: Loading Docker Image to Kind Cluster run: | make kind-load-images + loaded_postgres_image=false + for image in \ + swr.cn-north-4.myhuaweicloud.com/ddn-k8s/docker.io/bitnamilegacy/postgresql:15.4.0-debian-11-r45 \ + docker.aityp.com/bitnamilegacy/postgresql:15.4.0-debian-11-r45 \ + bitnamilegacy/postgresql:15.4.0-debian-11-r45 \ + docker.m.daocloud.io/bitnamilegacy/postgresql:15.4.0-debian-11-r45; do + if docker pull "${image}"; then + docker tag "${image}" docker.io/bitnamilegacy/postgresql:15.4.0-debian-11-r45 + kind load docker-image docker.io/bitnamilegacy/postgresql:15.4.0-debian-11-r45 --name apisix-ingress-cluster + loaded_postgres_image=true + break + fi + done + if [ "${loaded_postgres_image}" != "true" ]; then + echo "failed to preload postgres image for kind" >&2 + exit 1 + fi - name: Extract adc binary if: ${{ env.ADC_VERSION == 'dev' }} @@ -116,5 +142,8 @@ jobs: TEST_LABEL: ${{ matrix.cases_subset }} INGRESS_VERSION: v1beta1 TEST_ENV: CI + POSTGRESQL_IMAGE_REGISTRY: docker.io + POSTGRESQL_IMAGE_REPOSITORY: bitnamilegacy/postgresql + POSTGRESQL_IMAGE_TAG: 15.4.0-debian-11-r45 run: | make e2e-test diff --git a/.github/workflows/e2e-test.yml b/.github/workflows/e2e-test.yml index 94a7b383..c4b53a05 100644 --- a/.github/workflows/e2e-test.yml +++ b/.github/workflows/e2e-test.yml @@ -59,18 +59,18 @@ jobs: chmod 700 get_helm.sh ./get_helm.sh - name: Login to Registry - uses: docker/login-action@v3 - with: - registry: ${{ secrets.DOCKER_REGISTRY }} - username: ${{ secrets.DOCKER_USERNAME }} - password: ${{ secrets.DOCKER_PASSWORD }} + run: | + echo "${{ secrets.DOCKER_PASSWORD }}" | docker login \ + "${{ secrets.DOCKER_REGISTRY }}" \ + --username "${{ secrets.DOCKER_USERNAME }}" \ + --password-stdin - name: Login to Private Registry - uses: docker/login-action@v3 - with: - registry: hkccr.ccs.tencentyun.com - username: ${{ secrets.PRIVATE_DOCKER_USERNAME }} - password: ${{ secrets.PRIVATE_DOCKER_PASSWORD }} + run: | + echo "${{ secrets.PRIVATE_DOCKER_PASSWORD }}" | docker login \ + hkccr.ccs.tencentyun.com \ + --username "${{ secrets.PRIVATE_DOCKER_USERNAME }}" \ + --password-stdin - name: Build images env: diff --git a/.github/workflows/spell-checker.yml b/.github/workflows/spell-checker.yml index 5a0cdd03..e8b7e606 100644 --- a/.github/workflows/spell-checker.yml +++ b/.github/workflows/spell-checker.yml @@ -31,10 +31,14 @@ jobs: steps: - name: Check out code. uses: actions/checkout@v4 + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod - name: Install run: | - wget -O - -q https://git.io/misspell | sh -s -- -b . + go install github.com/client9/misspell/cmd/misspell@v0.3.4 - name: Misspell run: | - find . -name "*.go" -type f | xargs ./misspell -i mosquitto -error - find docs -type f | xargs ./misspell -error + find . -name "*.go" -type f | xargs "$(go env GOPATH)/bin/misspell" -i mosquitto -error + find docs -type f | xargs "$(go env GOPATH)/bin/misspell" -error diff --git a/Makefile b/Makefile index 76ff2d5e..60a6ad07 100644 --- a/Makefile +++ b/Makefile @@ -194,7 +194,7 @@ kind-down: || echo "kind cluster does not exist" .PHONY: kind-load-images -kind-load-images: pull-infra-images kind-load-ingress-image kind-load-adc-image +kind-load-images: pull-infra-images build-e2e-echo-server-image kind-load-ingress-image kind-load-adc-image @kind load docker-image hkccr.ccs.tencentyun.com/api7-dev/api7-ee-3-gateway:dev --name $(KIND_NAME) @kind load docker-image hkccr.ccs.tencentyun.com/api7-dev/api7-ee-dp-manager:$(DASHBOARD_VERSION) --name $(KIND_NAME) @kind load docker-image hkccr.ccs.tencentyun.com/api7-dev/api7-ee-3-integrated:$(DASHBOARD_VERSION) --name $(KIND_NAME) @@ -222,16 +222,39 @@ kind-load-adc-image: @docker tag ghcr.io/api7/adc:$(ADC_VERSION) ghcr.io/api7/adc:dev @kind load docker-image ghcr.io/api7/adc:dev --name $(KIND_NAME) +.PHONY: build-e2e-echo-server-image +build-e2e-echo-server-image: + @CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o bin/e2e-echo-server ./cmd/e2e-echo-server + @docker build -f test/e2e/images/echo-server.Dockerfile -t jmalloc/echo-server:latest . + .PHONY: pull-infra-images pull-infra-images: - @docker pull hkccr.ccs.tencentyun.com/api7-dev/api7-ee-3-gateway:dev - @docker pull hkccr.ccs.tencentyun.com/api7-dev/api7-ee-dp-manager:$(DASHBOARD_VERSION) - @docker pull hkccr.ccs.tencentyun.com/api7-dev/api7-ee-3-integrated:$(DASHBOARD_VERSION) - @docker pull kennethreitz/httpbin:latest - @docker pull jmalloc/echo-server:latest - @docker pull ghcr.io/api7/adc:dev - @docker pull apache/apisix:dev - @docker pull openresty/openresty:1.27.1.2-4-bullseye-fat + @retry_pull() { \ + source="$$1"; \ + target="$$2"; \ + for attempt in 1 2 3; do \ + if docker pull "$$source"; then \ + if [ "$$source" != "$$target" ]; then \ + docker tag "$$source" "$$target"; \ + fi; \ + return 0; \ + fi; \ + if [ $$attempt -eq 3 ]; then \ + echo "failed to pull $$source after $$attempt attempts" >&2; \ + exit 1; \ + fi; \ + echo "retrying docker pull for $$source (attempt $$((attempt + 1))/3)..." >&2; \ + sleep 5; \ + done; \ + }; \ + retry_pull "hkccr.ccs.tencentyun.com/api7-dev/api7-ee-3-gateway:dev" "hkccr.ccs.tencentyun.com/api7-dev/api7-ee-3-gateway:dev"; \ + retry_pull "hkccr.ccs.tencentyun.com/api7-dev/api7-ee-dp-manager:$(DASHBOARD_VERSION)" "hkccr.ccs.tencentyun.com/api7-dev/api7-ee-dp-manager:$(DASHBOARD_VERSION)"; \ + retry_pull "hkccr.ccs.tencentyun.com/api7-dev/api7-ee-3-integrated:$(DASHBOARD_VERSION)" "hkccr.ccs.tencentyun.com/api7-dev/api7-ee-3-integrated:$(DASHBOARD_VERSION)"; \ + dockerhub_proxy="$${DOCKERHUB_PROXY:-docker.m.daocloud.io}"; \ + retry_pull "$$dockerhub_proxy/kennethreitz/httpbin:latest" "kennethreitz/httpbin:latest"; \ + retry_pull "ghcr.io/api7/adc:dev" "ghcr.io/api7/adc:dev"; \ + retry_pull "$$dockerhub_proxy/apache/apisix:dev" "apache/apisix:dev"; \ + retry_pull "$$dockerhub_proxy/openresty/openresty:1.27.1.2-4-bullseye-fat" "openresty/openresty:1.27.1.2-4-bullseye-fat" ##@ Build @@ -398,8 +421,12 @@ $(ADC_BIN): ifeq ($(ADC_VERSION),dev) @echo "ADC_VERSION=dev, skip download" else - curl -sSfL https://github.com/api7/adc/releases/download/v${ADC_VERSION}/adc_${ADC_VERSION}_${GOOS}_${GOARCH}.tar.gz \ - | tar -xz -C $(LOCALBIN) + tmp_archive=$$(mktemp); \ + trap 'rm -f "$$tmp_archive"' EXIT; \ + curl --retry 5 --retry-delay 2 --retry-connrefused -sSfL \ + -o "$$tmp_archive" \ + https://github.com/api7/adc/releases/download/v${ADC_VERSION}/adc_${ADC_VERSION}_${GOOS}_${GOARCH}.tar.gz; \ + tar -xzf "$$tmp_archive" -C $(LOCALBIN) endif gofmt: ## Apply go fmt diff --git a/cmd/e2e-echo-server/main.go b/cmd/e2e-echo-server/main.go new file mode 100644 index 00000000..4e9a097b --- /dev/null +++ b/cmd/e2e-echo-server/main.go @@ -0,0 +1,44 @@ +package main + +import ( + "log" + "net/http" + + "github.com/gorilla/websocket" +) + +var upgrader = websocket.Upgrader{ + CheckOrigin: func(*http.Request) bool { + return true + }, +} + +func main() { + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + if websocket.IsWebSocketUpgrade(r) { + conn, err := upgrader.Upgrade(w, r, nil) + if err != nil { + return + } + defer func() { + _ = conn.Close() + }() + + for { + messageType, message, err := conn.ReadMessage() + if err != nil { + return + } + if err := conn.WriteMessage(messageType, message); err != nil { + return + } + } + } + + w.Header().Set("Content-Type", "text/plain") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("ok")) + }) + + log.Fatal(http.ListenAndServe(":8080", nil)) +} diff --git a/internal/adc/client/client.go b/internal/adc/client/client.go index 3019c671..8b75236b 100644 --- a/internal/adc/client/client.go +++ b/internal/adc/client/client.go @@ -174,6 +174,43 @@ func (c *Client) DeleteConfig(ctx context.Context, args Task) error { return err } +func (c *Client) Validate(ctx context.Context, task Task) error { + if len(task.Configs) == 0 || task.Resources == nil { + return nil + } + + fileIOStart := time.Now() + syncFilePath, cleanup, err := prepareSyncFile(task.Resources) + if err != nil { + pkgmetrics.RecordFileIODuration("prepare_sync_file", "failure", time.Since(fileIOStart).Seconds()) + return err + } + pkgmetrics.RecordFileIODuration("prepare_sync_file", adctypes.StatusSuccess, time.Since(fileIOStart).Seconds()) + defer cleanup() + + args := BuildADCExecuteArgs(syncFilePath, task.Labels, task.ResourceTypes) + + var errs types.ADCValidationErrors + for _, config := range task.Configs { + if config.BackendType == "" { + config.BackendType = c.defaultMode + } + if err := c.executor.Validate(ctx, config, args); err != nil { + var validationErr types.ADCValidationError + if errors.As(err, &validationErr) { + errs.Errors = append(errs.Errors, validationErr) + continue + } + return err + } + } + + if len(errs.Errors) > 0 { + return errs + } + return nil +} + func (c *Client) Sync(ctx context.Context) (map[string]types.ADCExecutionErrors, error) { c.syncMu.Lock() defer c.syncMu.Unlock() diff --git a/internal/adc/client/executor.go b/internal/adc/client/executor.go index 08608611..a62689f5 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,249 @@ 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, + "body", body, + ) + + 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 !config.TlsVerify { + if transport.TLSClientConfig == nil { + transport.TLSClientConfig = &tls.Config{} + } + 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) + } + + 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 +536,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 +562,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 +572,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 +645,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 00000000..16fd1511 --- /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 00000000..5cf3afca --- /dev/null +++ b/internal/controller/webhook_validation.go @@ -0,0 +1,117 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package controller + +import ( + "context" + + "github.com/go-logr/logr" + networkingv1 "k8s.io/api/networking/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1" + apiv2 "github.com/apache/apisix-ingress-controller/api/v2" + "github.com/apache/apisix-ingress-controller/internal/provider" + "github.com/apache/apisix-ingress-controller/internal/utils" +) + +func PrepareApisixRouteForValidation(ctx context.Context, c client.Client, log logr.Logger, route *apiv2.ApisixRoute) (*provider.TranslateContext, error) { + tctx := provider.NewDefaultTranslateContext(ctx) + + ingressClass, err := FindMatchingIngressClassByObject(tctx, c, log, route, networkingv1.SchemeGroupVersion.String()) + if err != nil { + return nil, err + } + if err := ProcessIngressClassParameters(tctx, c, log, route, ingressClass); err != nil { + return nil, err + } + + reconciler := &ApisixRouteReconciler{ + Client: c, + Log: log, + ICGV: networkingv1.SchemeGroupVersion, + supportsEndpointSlice: true, + } + if err := reconciler.processApisixRoute(tctx, route); err != nil { + return nil, err + } + return tctx, nil +} + +func PrepareApisixConsumerForValidation(ctx context.Context, c client.Client, log logr.Logger, consumer *apiv2.ApisixConsumer) (*provider.TranslateContext, error) { + tctx := provider.NewDefaultTranslateContext(ctx) + + ingressClass, err := FindMatchingIngressClassByObject(tctx, c, log, consumer, networkingv1.SchemeGroupVersion.String()) + if err != nil { + return nil, err + } + if err := ProcessIngressClassParameters(tctx, c, log, consumer, ingressClass); err != nil { + return nil, err + } + + reconciler := &ApisixConsumerReconciler{ + Client: c, + Log: log, + ICGV: networkingv1.SchemeGroupVersion, + } + if err := reconciler.processSpec(ctx, tctx, consumer); err != nil { + return nil, err + } + return tctx, nil +} + +func PrepareConsumerForValidation(ctx context.Context, c client.Client, log logr.Logger, consumer *v1alpha1.Consumer) (*provider.TranslateContext, error) { + tctx := provider.NewDefaultTranslateContext(ctx) + + reconciler := &ConsumerReconciler{ + Client: c, + Log: log, + } + gateway, err := reconciler.getGateway(ctx, consumer) + if err != nil { + return nil, err + } + if err := ProcessGatewayProxy(c, log, tctx, gateway, utils.NamespacedNameKind(consumer)); err != nil { + return nil, err + } + if err := reconciler.processSpec(ctx, tctx, consumer); err != nil { + return nil, err + } + return tctx, nil +} + +func PrepareApisixTlsForValidation(ctx context.Context, c client.Client, log logr.Logger, tls *apiv2.ApisixTls) (*provider.TranslateContext, error) { + tctx := provider.NewDefaultTranslateContext(ctx) + + ingressClass, err := FindMatchingIngressClassByObject(tctx, c, log, tls, networkingv1.SchemeGroupVersion.String()) + if err != nil { + return nil, err + } + if err := ProcessIngressClassParameters(tctx, c, log, tls, ingressClass); err != nil { + return nil, err + } + + reconciler := &ApisixTlsReconciler{ + Client: c, + Log: log, + } + if err := reconciler.processApisixTls(ctx, tctx, tls); err != nil { + return nil, err + } + return tctx, nil +} diff --git a/internal/types/error.go b/internal/types/error.go index 80dbf568..1388637d 100644 --- a/internal/types/error.go +++ b/internal/types/error.go @@ -92,3 +92,70 @@ type ADCExecutionServerAddrError struct { func (e ADCExecutionServerAddrError) Error() string { return fmt.Sprintf("ServerAddr: %s, Err: %s", e.ServerAddr, e.Err) } + +type ADCValidationErrors struct { + Errors []ADCValidationError +} + +func (e ADCValidationErrors) Error() string { + messages := make([]string, 0, len(e.Errors)) + for _, err := range e.Errors { + messages = append(messages, err.Error()) + } + return fmt.Sprintf("ADC validation errors: [%s]", strings.Join(messages, "; ")) +} + +type ADCValidationError struct { + Name string + FailedErrors []ADCValidationServerAddrError +} + +func (e ADCValidationError) Error() string { + messages := make([]string, 0, len(e.FailedErrors)) + for _, failed := range e.FailedErrors { + messages = append(messages, failed.Error()) + } + return fmt.Sprintf("ADC validation error for %s: [%s]", e.Name, strings.Join(messages, "; ")) +} + +type ADCValidationServerAddrError struct { + Err string + ServerAddr string + ValidationErrors []ADCValidationDetail +} + +func (e ADCValidationServerAddrError) Error() string { + if len(e.ValidationErrors) == 0 { + return fmt.Sprintf("ServerAddr: %s, Err: %s", e.ServerAddr, e.Err) + } + + messages := make([]string, 0, len(e.ValidationErrors)) + for _, detail := range e.ValidationErrors { + messages = append(messages, detail.Error()) + } + return fmt.Sprintf("ServerAddr: %s, Err: %s (%s)", e.ServerAddr, e.Err, strings.Join(messages, "; ")) +} + +type ADCValidationDetail struct { + ResourceType string `json:"resource_type,omitempty"` + ResourceName string `json:"resource_name,omitempty"` + Message string `json:"message,omitempty"` + Index int `json:"index,omitempty"` +} + +func (e ADCValidationDetail) Error() string { + var parts []string + if e.ResourceType != "" { + parts = append(parts, fmt.Sprintf("type=%s", e.ResourceType)) + } + if e.ResourceName != "" { + parts = append(parts, fmt.Sprintf("name=%s", e.ResourceName)) + } + if e.Message != "" { + parts = append(parts, e.Message) + } + if len(parts) == 0 { + return fmt.Sprintf("index=%d", e.Index) + } + return strings.Join(parts, ", ") +} diff --git a/internal/webhook/v1/adc_validation.go b/internal/webhook/v1/adc_validation.go new file mode 100644 index 00000000..f505fe9a --- /dev/null +++ b/internal/webhook/v1/adc_validation.go @@ -0,0 +1,234 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package v1 + +import ( + "context" + "errors" + + "github.com/go-logr/logr" + networkingv1 "k8s.io/api/networking/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + adctypes "github.com/apache/apisix-ingress-controller/api/adc" + v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1" + apiv2 "github.com/apache/apisix-ingress-controller/api/v2" + adcclient "github.com/apache/apisix-ingress-controller/internal/adc/client" + adctranslator "github.com/apache/apisix-ingress-controller/internal/adc/translator" + "github.com/apache/apisix-ingress-controller/internal/controller" + "github.com/apache/apisix-ingress-controller/internal/controller/config" + "github.com/apache/apisix-ingress-controller/internal/controller/label" + "github.com/apache/apisix-ingress-controller/internal/provider" + internaltypes "github.com/apache/apisix-ingress-controller/internal/types" + "github.com/apache/apisix-ingress-controller/internal/utils" +) + +type adcAdmissionValidator struct { + kubeClient client.Client + client *adcclient.Client + translator *adctranslator.Translator + log logr.Logger + defaultResolveEndpoint bool +} + +func newADCAdmissionValidator(kubeClient client.Client, log logr.Logger) (*adcAdmissionValidator, error) { + defaultMode := string(config.ControllerConfig.ProviderConfig.Type) + cli, err := adcclient.New(log, defaultMode, config.ControllerConfig.ExecADCTimeout.Duration) + if err != nil { + return nil, err + } + + return &adcAdmissionValidator{ + kubeClient: kubeClient, + client: cli, + translator: adctranslator.NewTranslator(log), + log: log.WithName("adc-validation"), + defaultResolveEndpoint: config.ControllerConfig.ProviderConfig.Type == config.ProviderTypeStandalone, + }, nil +} + +func (v *adcAdmissionValidator) Validate(ctx context.Context, obj client.Object) error { + if v == nil { + return nil + } + + task, err := v.buildTask(ctx, obj) + if err != nil { + return err + } + if task == nil { + return nil + } + + if err := v.client.Validate(ctx, *task); err != nil { + var validationErrs internaltypes.ADCValidationErrors + if errors.As(err, &validationErrs) { + return err + } + + v.log.Error(err, "ADC validation unavailable, allowing admission", "resource", utils.NamespacedNameKind(obj)) + return nil + } + + return nil +} + +func (v *adcAdmissionValidator) buildTask(ctx context.Context, obj client.Object) (*adcclient.Task, error) { + var ( + tctx *provider.TranslateContext + result *adctranslator.TranslateResult + resourceTypes []string + err error + ) + + switch resource := obj.(type) { + case *apiv2.ApisixRoute: + configs, err := v.buildIngressClassConfigs(ctx, resource.DeepCopy()) + if err != nil { + return nil, err + } + if len(configs) == 0 { + return nil, nil + } + tctx, err = controller.PrepareApisixRouteForValidation(ctx, v.kubeClient, v.log, resource.DeepCopy()) + if err != nil { + return nil, err + } + result, err = v.translator.TranslateApisixRoute(tctx, resource.DeepCopy()) + resourceTypes = append(resourceTypes, adctypes.TypeService) + if err != nil { + return nil, err + } + if result == nil { + return nil, nil + } + return v.newTask(obj, configs, resourceTypes, result), nil + case *apiv2.ApisixConsumer: + configs, err := v.buildIngressClassConfigs(ctx, resource.DeepCopy()) + if err != nil { + return nil, err + } + if len(configs) == 0 { + return nil, nil + } + tctx, err = controller.PrepareApisixConsumerForValidation(ctx, v.kubeClient, v.log, resource.DeepCopy()) + if err != nil { + return nil, err + } + result, err = v.translator.TranslateApisixConsumer(tctx, resource.DeepCopy()) + resourceTypes = append(resourceTypes, adctypes.TypeConsumer) + if err != nil { + return nil, err + } + if result == nil { + return nil, nil + } + return v.newTask(obj, configs, resourceTypes, result), nil + case *v1alpha1.Consumer: + tctx, err = controller.PrepareConsumerForValidation(ctx, v.kubeClient, v.log, resource.DeepCopy()) + if err != nil { + return nil, err + } + result, err = v.translator.TranslateConsumerV1alpha1(tctx, resource.DeepCopy()) + resourceTypes = append(resourceTypes, adctypes.TypeConsumer) + case *apiv2.ApisixTls: + configs, err := v.buildIngressClassConfigs(ctx, resource.DeepCopy()) + if err != nil { + return nil, err + } + if len(configs) == 0 { + return nil, nil + } + tctx, err = controller.PrepareApisixTlsForValidation(ctx, v.kubeClient, v.log, resource.DeepCopy()) + if err != nil { + return nil, err + } + result, err = v.translator.TranslateApisixTls(tctx, resource.DeepCopy()) + resourceTypes = append(resourceTypes, adctypes.TypeSSL) + if err != nil { + return nil, err + } + if result == nil { + return nil, nil + } + return v.newTask(obj, configs, resourceTypes, result), nil + default: + return nil, nil + } + if err != nil { + return nil, err + } + if result == nil { + return nil, nil + } + + configs, err := v.buildConfigs(tctx) + if err != nil { + return nil, err + } + if len(configs) == 0 { + return nil, nil + } + + return v.newTask(obj, configs, resourceTypes, result), nil +} + +func (v *adcAdmissionValidator) buildConfigs(tctx *provider.TranslateContext) (map[internaltypes.NamespacedNameKind]adctypes.Config, error) { + configs := make(map[internaltypes.NamespacedNameKind]adctypes.Config, len(tctx.GatewayProxies)) + for key, gp := range tctx.GatewayProxies { + cfg, err := v.translator.TranslateGatewayProxyToConfig(tctx, &gp, v.defaultResolveEndpoint) + if err != nil { + return nil, err + } + if cfg == nil { + continue + } + configs[key] = *cfg + } + return configs, nil +} + +func (v *adcAdmissionValidator) buildIngressClassConfigs(ctx context.Context, obj client.Object) (map[internaltypes.NamespacedNameKind]adctypes.Config, error) { + tctx := provider.NewDefaultTranslateContext(ctx) + + ingressClass, err := controller.FindMatchingIngressClassByObject(tctx, v.kubeClient, v.log, obj, networkingv1.SchemeGroupVersion.String()) + if err != nil { + return nil, err + } + if err := controller.ProcessIngressClassParameters(tctx, v.kubeClient, v.log, obj, ingressClass); err != nil { + return nil, err + } + return v.buildConfigs(tctx) +} + +func (v *adcAdmissionValidator) newTask(obj client.Object, configs map[internaltypes.NamespacedNameKind]adctypes.Config, resourceTypes []string, result *adctranslator.TranslateResult) *adcclient.Task { + return &adcclient.Task{ + Key: utils.NamespacedNameKind(obj), + Name: utils.NamespacedNameKind(obj).String(), + Labels: label.GenLabel(obj), + Configs: configs, + ResourceTypes: resourceTypes, + Resources: &adctypes.Resources{ + GlobalRules: result.GlobalRules, + PluginMetadata: result.PluginMetadata, + Services: result.Services, + SSLs: result.SSL, + Consumers: result.Consumers, + }, + } +} diff --git a/internal/webhook/v1/adc_validation_test.go b/internal/webhook/v1/adc_validation_test.go new file mode 100644 index 00000000..a87199dc --- /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 b419b8da..01f5f03c 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,11 @@ 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 { + return warnings, v.initErr + } + return warnings, v.adcValidator.Validate(ctx, consumer) } func (v *ApisixConsumerCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { @@ -82,7 +91,11 @@ 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 { + return warnings, v.initErr + } + return warnings, v.adcValidator.Validate(ctx, consumer) } func (*ApisixConsumerCustomValidator) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) { diff --git a/internal/webhook/v1/apisixconsumer_webhook_test.go b/internal/webhook/v1/apisixconsumer_webhook_test.go index 8c31768c..89ab50d2 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 6f028fdd..dc680da0 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,11 @@ 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 { + return warnings, v.initErr + } + return warnings, v.adcValidator.Validate(ctx, route) } func (v *ApisixRouteCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { @@ -80,7 +89,11 @@ 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 { + return warnings, v.initErr + } + return warnings, v.adcValidator.Validate(ctx, route) } func (*ApisixRouteCustomValidator) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) { diff --git a/internal/webhook/v1/apisixroute_webhook_test.go b/internal/webhook/v1/apisixroute_webhook_test.go index b8ca3aa2..339791f7 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 16ba02d5..b4a69626 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,12 @@ 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) + if v.initErr != nil { + return warnings, v.initErr + } + + return warnings, v.adcValidator.Validate(ctx, tls) } func (v *ApisixTlsCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { @@ -93,7 +103,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 { + return warnings, v.initErr + } + + return warnings, v.adcValidator.Validate(ctx, tls) } func (*ApisixTlsCustomValidator) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) { diff --git a/internal/webhook/v1/apisixtls_webhook_test.go b/internal/webhook/v1/apisixtls_webhook_test.go index 205236f6..f4ff87ff 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 f9b3bd77..c5a40ec9 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" @@ -44,16 +47,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 +75,14 @@ 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 { + return warnings, v.initErr + } + 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 +95,14 @@ 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 { + return warnings, v.initErr + } + 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 +139,106 @@ 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 + } + + var consumers apisixv1alpha1.ConsumerList + if err := v.Client.List(ctx, &consumers); err != nil { + return err + } + + for i := range consumers.Items { + existing := &consumers.Items[i] + if existing.Namespace == consumer.Namespace && existing.Name == consumer.Name { + continue + } + if !sameConsumerGatewayRef(existing, consumer) { + 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 { + return "", nil + } + return cfg.Key, nil +} + +func sameConsumerGatewayRef(left, right *apisixv1alpha1.Consumer) bool { + leftNamespace := left.Namespace + if left.Spec.GatewayRef.Namespace != nil && *left.Spec.GatewayRef.Namespace != "" { + leftNamespace = *left.Spec.GatewayRef.Namespace + } + + rightNamespace := right.Namespace + if right.Spec.GatewayRef.Namespace != nil && *right.Spec.GatewayRef.Namespace != "" { + rightNamespace = *right.Spec.GatewayRef.Namespace + } + + return left.Spec.GatewayRef.Name == right.Spec.GatewayRef.Name && leftNamespace == rightNamespace +} diff --git a/internal/webhook/v1/consumer_webhook_test.go b/internal/webhook/v1/consumer_webhook_test.go index 045bc12b..300d5522 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" @@ -146,3 +147,44 @@ func TestConsumerValidator_NoWarnings(t *testing.T) { require.NoError(t, err) require.Empty(t, warnings) } + +func TestConsumerValidator_DenyDuplicateKeyAuthCredential(t *testing.T) { + existing := &apisixv1alpha1.Consumer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing", + Namespace: "default", + }, + Spec: apisixv1alpha1.ConsumerSpec{ + GatewayRef: apisixv1alpha1.GatewayRef{Name: "test-gateway"}, + Credentials: []apisixv1alpha1.Credential{{ + Type: "key-auth", + Config: apiextensionsv1.JSON{ + Raw: []byte(`{"key":"shared-key"}`), + }, + }}, + }, + } + consumer := &apisixv1alpha1.Consumer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + Namespace: "default", + }, + Spec: apisixv1alpha1.ConsumerSpec{ + GatewayRef: apisixv1alpha1.GatewayRef{Name: "test-gateway"}, + Credentials: []apisixv1alpha1.Credential{{ + Type: "key-auth", + Config: apiextensionsv1.JSON{ + Raw: []byte(`{"key":"shared-key"}`), + }, + }}, + }, + } + + validator := buildConsumerValidator(t, existing) + + warnings, err := validator.ValidateCreate(context.Background(), consumer) + require.Empty(t, warnings) + require.Error(t, err) + require.Contains(t, err.Error(), `duplicate key-auth credential key "shared-key"`) + require.Contains(t, err.Error(), "default/existing") +} diff --git a/test/e2e/framework/api7_dashboard.go b/test/e2e/framework/api7_dashboard.go index c88151e0..91f93dc6 100644 --- a/test/e2e/framework/api7_dashboard.go +++ b/test/e2e/framework/api7_dashboard.go @@ -33,8 +33,11 @@ import ( ) var ( - valuesTemplate *template.Template - _db string + valuesTemplate *template.Template + _db string + postgresImageRegistry string + postgresImageRepository string + postgresImageTag string ) func init() { @@ -42,6 +45,14 @@ func init() { if _db == "" { _db = postgres } + + postgresImageRegistry = os.Getenv("POSTGRESQL_IMAGE_REGISTRY") + postgresImageRepository = os.Getenv("POSTGRESQL_IMAGE_REPOSITORY") + postgresImageTag = os.Getenv("POSTGRESQL_IMAGE_TAG") + if postgresImageRegistry != "" && postgresImageRepository == "" { + postgresImageRepository = "bitnami/postgresql" + } + tmpl, err := template.New("values.yaml").Parse(` dashboard: image: @@ -208,6 +219,14 @@ jaeger: postgresql: {{- if ne .DB "postgres" }} builtin: false +{{- end }} +{{- if .PostgresImageRegistry }} + image: + registry: {{ .PostgresImageRegistry }} + repository: {{ .PostgresImageRepository }} +{{- if .PostgresImageTag }} + tag: {{ .PostgresImageTag }} +{{- end }} {{- end }} primary: containerSecurityContext: diff --git a/test/e2e/framework/api7_framework.go b/test/e2e/framework/api7_framework.go index 793b76f3..27b87f5a 100644 --- a/test/e2e/framework/api7_framework.go +++ b/test/e2e/framework/api7_framework.go @@ -35,6 +35,7 @@ import ( "helm.sh/helm/v3/pkg/cli" "helm.sh/helm/v3/pkg/kube" k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var ( @@ -70,7 +71,10 @@ func (f *Framework) BeforeSuite() { f.DeployComponents() time.Sleep(1 * time.Minute) - err := f.newDashboardTunnel() + err := f.ensureServiceWithTimeout("api7ee3-dashboard", _namespace, 1, 300) + Expect(err).ShouldNot(HaveOccurred(), "ensuring dashboard service") + + err = f.newDashboardTunnel() f.Logf("Dashboard HTTP Tunnel:" + _dashboardHTTPTunnel.Endpoint()) Expect(err).ShouldNot(HaveOccurred(), "creating dashboard tunnel") @@ -134,9 +138,12 @@ func (f *Framework) deploy() { buf := bytes.NewBuffer(nil) _ = valuesTemplate.Execute(buf, map[string]any{ - "DB": _db, - "DSN": getDSN(), - "Tag": dashboardVersion, + "DB": _db, + "DSN": getDSN(), + "Tag": dashboardVersion, + "PostgresImageRegistry": postgresImageRegistry, + "PostgresImageRepository": postgresImageRepository, + "PostgresImageTag": postgresImageTag, }) f.Logf("values: %s", buf.String()) @@ -150,10 +157,15 @@ func (f *Framework) deploy() { } f.GomegaT.Expect(err).ShouldNot(HaveOccurred(), "install dashboard") - err = f.ensureService("api7ee3-dashboard", _namespace, 1) + _, err = k8s.GetServiceE(GinkgoT(), f.kubectlOpts, "api7ee3-dashboard") f.GomegaT.Expect(err).ShouldNot(HaveOccurred(), "ensuring dashboard service") - err = f.ensureService("api7-postgresql", _namespace, 1) + err = WaitPodsAvailableWithTimeout(f.GinkgoT, f.kubectlOpts, metav1.ListOptions{ + LabelSelector: "app.kubernetes.io/instance=api7ee3,app.kubernetes.io/name=postgresql,app.kubernetes.io/component=primary", + }, 5*time.Minute) + f.GomegaT.Expect(err).ShouldNot(HaveOccurred(), "waiting for postgres pod ready") + + err = f.ensureServiceWithTimeout("api7-postgresql", _namespace, 1, 300) f.GomegaT.Expect(err).ShouldNot(HaveOccurred(), "ensuring postgres service") } diff --git a/test/e2e/framework/k8s.go b/test/e2e/framework/k8s.go index c901332d..9a6d40b7 100644 --- a/test/e2e/framework/k8s.go +++ b/test/e2e/framework/k8s.go @@ -261,12 +261,19 @@ func (f *Framework) applySSLSecret(namespace, name string, cert, pkey, caCert [] } func WaitPodsAvailable(t testing.TestingT, kubeOps *k8s.KubectlOptions, opts metav1.ListOptions) error { + return WaitPodsAvailableWithTimeout(t, kubeOps, opts, time.Minute) +} + +func WaitPodsAvailableWithTimeout(t testing.TestingT, kubeOps *k8s.KubectlOptions, opts metav1.ListOptions, timeout time.Duration) error { + var lastErr error condFunc := func() (bool, error) { items, err := k8s.ListPodsE(t, kubeOps, opts) if err != nil { + lastErr = err return false, err } if len(items) == 0 { + lastErr = fmt.Errorf("no pods found for selector %q", opts.LabelSelector) return false, nil } for _, item := range items { @@ -277,23 +284,60 @@ func WaitPodsAvailable(t testing.TestingT, kubeOps *k8s.KubectlOptions, opts met } foundPodReady = true if cond.Status != "True" { + lastErr = fmt.Errorf("pod %s is not ready: %s", item.Name, describePodStatus(item)) return false, nil } } if !foundPodReady { + lastErr = fmt.Errorf("pod %s has no Ready condition: %s", item.Name, describePodStatus(item)) return false, nil } } return true, nil } - return waitExponentialBackoff(condFunc) + err := waitExponentialBackoffWithTimeout(condFunc, timeout) + if err != nil && lastErr != nil { + return lastErr + } + return err +} + +func describePodStatus(pod corev1.Pod) string { + conditions := make([]string, 0, len(pod.Status.Conditions)) + for _, cond := range pod.Status.Conditions { + conditions = append(conditions, fmt.Sprintf("%s=%s", cond.Type, cond.Status)) + } + + containerStates := make([]string, 0, len(pod.Status.ContainerStatuses)) + for _, status := range pod.Status.ContainerStatuses { + state := "unknown" + switch { + case status.State.Waiting != nil: + state = fmt.Sprintf("waiting(%s:%s)", status.State.Waiting.Reason, status.State.Waiting.Message) + case status.State.Terminated != nil: + state = fmt.Sprintf("terminated(%s:%s)", status.State.Terminated.Reason, status.State.Terminated.Message) + case status.State.Running != nil: + state = "running" + } + containerStates = append(containerStates, fmt.Sprintf("%s=%s", status.Name, state)) + } + + return fmt.Sprintf("phase=%s conditions=[%s] containers=[%s]", + pod.Status.Phase, + strings.Join(conditions, ", "), + strings.Join(containerStates, ", "), + ) } -func waitExponentialBackoff(condFunc func() (bool, error)) error { +func waitExponentialBackoffWithTimeout(condFunc func() (bool, error), timeout time.Duration) error { + steps := int(timeout / (2 * time.Second)) + if steps < 1 { + steps = 1 + } backoff := wait.Backoff{ - Duration: 500 * time.Millisecond, - Factor: 2, - Steps: 8, + Duration: 2 * time.Second, + Factor: 1, + Steps: steps, } return wait.ExponentialBackoff(backoff, condFunc) } diff --git a/test/e2e/images/echo-server.Dockerfile b/test/e2e/images/echo-server.Dockerfile new file mode 100644 index 00000000..071c2b14 --- /dev/null +++ b/test/e2e/images/echo-server.Dockerfile @@ -0,0 +1,7 @@ +FROM scratch + +COPY bin/e2e-echo-server /e2e-echo-server + +EXPOSE 8080 + +ENTRYPOINT ["/e2e-echo-server"] diff --git a/test/e2e/webhook/apisixconsumer.go b/test/e2e/webhook/apisixconsumer.go index 7aa1a256..e7119e5f 100644 --- a/test/e2e/webhook/apisixconsumer.go +++ b/test/e2e/webhook/apisixconsumer.go @@ -19,11 +19,13 @@ package webhook import ( "fmt" + "strings" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/apache/apisix-ingress-controller/test/e2e/framework" "github.com/apache/apisix-ingress-controller/test/e2e/scaffold" ) @@ -45,7 +47,7 @@ var _ = Describe("Test ApisixConsumer Webhook", Label("webhook"), func() { time.Sleep(5 * time.Second) }) - It("should warn on missing authentication secrets", func() { + It("should reject missing authentication secrets", func() { missingSecret := "missing-basic-secret" consumerName := "webhook-apisixconsumer" consumerYAML := ` @@ -63,7 +65,7 @@ spec: ` output, err := s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(consumerYAML, consumerName, s.Namespace(), s.Namespace(), missingSecret)) - Expect(err).ShouldNot(HaveOccurred()) + expectAdmissionDenied(s, "apisixconsumer", consumerName, err, fmt.Sprintf("%s/%s", s.Namespace(), missingSecret)) Expect(output).To(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), missingSecret))) By("creating referenced secret") @@ -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 51904f43..5a1c04d2 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" ) @@ -45,9 +46,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 +67,13 @@ spec: backends: - serviceName: %s servicePort: 80 - plugins: - - name: echo - enable: true - secretRef: %s ` - output, err := s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(routeYAML, routeName, s.Namespace(), s.Namespace(), missingService, missingSecret)) + output, err := s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(routeYAML, routeName, s.Namespace(), s.Namespace(), missingService)) Expect(err).ShouldNot(HaveOccurred()) Expect(output).To(ContainSubstring(fmt.Sprintf("Warning: Referenced Service '%s/%s' not found", s.Namespace(), missingService))) - Expect(output).To(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), missingSecret))) - By("creating referenced Service and Secret") + By("creating referenced Service") serviceYAML := fmt.Sprintf(` apiVersion: v1 kind: Service @@ -96,22 +91,92 @@ spec: err = s.CreateResourceFromString(serviceYAML) Expect(err).NotTo(HaveOccurred(), "creating backend service placeholder") - secretYAML := fmt.Sprintf(` + time.Sleep(2 * time.Second) + + output, err = s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(routeYAML, routeName, s.Namespace(), s.Namespace(), missingService)) + Expect(err).ShouldNot(HaveOccurred()) + Expect(output).NotTo(ContainSubstring(fmt.Sprintf("Warning: Referenced Service '%s/%s' not found", s.Namespace(), missingService))) + }) + + It("should reject routes that fail ADC validation", func() { + 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: Secret +kind: Service metadata: name: %s -stringData: - config: enabled -`, missingSecret) - err = s.CreateResourceFromString(secretYAML) - Expect(err).NotTo(HaveOccurred(), "creating plugin secret placeholder") +spec: + selector: + app: placeholder + ports: + - name: http + port: 80 + targetPort: 80 + type: ClusterIP +`, backendService) + err := s.CreateResourceFromString(serviceYAML) + Expect(err).NotTo(HaveOccurred(), "creating backend service") - time.Sleep(2 * time.Second) + invalidRouteYAML := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: %s + namespace: %s +spec: + ingressClassName: %s + http: + - name: rule-invalid + match: + hosts: + - webhook.example.com + paths: + - /invalid + backends: + - serviceName: %s + servicePort: 80 + resolveGranularity: service + plugins: + - name: response-rewrite + enable: true + config: + status_code: "500" +`, routeName, s.Namespace(), s.Namespace(), backendService) - output, err = s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(routeYAML, routeName, s.Namespace(), s.Namespace(), missingService, missingSecret)) - Expect(err).ShouldNot(HaveOccurred()) - Expect(output).NotTo(ContainSubstring(fmt.Sprintf("Warning: Referenced Service '%s/%s' not found", s.Namespace(), missingService))) - Expect(output).NotTo(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), missingSecret))) + By("creating ApisixRoute with invalid plugin config") + err = s.CreateResourceFromString(invalidRouteYAML) + expectAdmissionDenied(s, "apisixroute", routeName, err) + + validRouteYAML := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: %s + namespace: %s +spec: + ingressClassName: %s + http: + - name: rule-valid + match: + hosts: + - webhook.example.com + paths: + - /valid + backends: + - serviceName: %s + servicePort: 80 + resolveGranularity: service +`, routeName, s.Namespace(), s.Namespace(), backendService) + + By("creating corrected ApisixRoute") + err = s.CreateResourceFromString(validRouteYAML) + Expect(err).NotTo(HaveOccurred(), "creating corrected ApisixRoute") }) }) diff --git a/test/e2e/webhook/apisixtls.go b/test/e2e/webhook/apisixtls.go index 08defed9..b6951a13 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" ) @@ -45,7 +46,7 @@ var _ = Describe("Test ApisixTls Webhook", Label("webhook"), func() { time.Sleep(5 * time.Second) }) - It("should warn on missing TLS secrets", func() { + It("should reject missing TLS secrets", func() { serverSecret := "missing-server-tls" clientSecret := "missing-client-ca" tlsName := "webhook-apisixtls" @@ -69,12 +70,38 @@ spec: ` output, err := s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(tlsYAML, tlsName, s.Namespace(), s.Namespace(), serverSecret, s.Namespace(), clientSecret, s.Namespace())) - Expect(err).ShouldNot(HaveOccurred()) + expectAdmissionDenied(s, "apisixtls", tlsName, err, fmt.Sprintf("%s/%s", s.Namespace(), serverSecret)) 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,41 @@ 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()) - time.Sleep(2 * time.Second) + By("creating ApisixTls backed by invalid certificate material") + err = s.CreateResourceFromString(tlsYAML) + expectAdmissionDenied(s, "apisixtls", tlsName, err) - 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("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") + + 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 676adbb8..3c64562c 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 1b21c8b7..ffb06381 100644 --- a/test/e2e/webhook/helpers.go +++ b/test/e2e/webhook/helpers.go @@ -235,3 +235,14 @@ spec: Expect(err).ShouldNot(HaveOccurred()) Expect(output).NotTo(ContainSubstring(missingBackendWarning)) } + +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)) +}