test(e2e): fix and expand ADC validation webhook tests

AlinsRan · Copilot · AlinsRan · commit 4ffeb5e13871 · 2026-05-07T01:40:13.000+08:00
Restore original warn-on-missing-ref behavior for existing tests, and add
update-path coverage for all four webhook resource types.

Changes:
- apisixconsumer_webhook.go: add missing 'if len(warnings) &gt; 0' guard to
  skip ADC validation when references are missing (aligns with ApisixTls/
  ApisixRoute pattern)
- apisixconsumer.go: restore 'should warn on missing authentication secrets'
  (was incorrectly changed to deny)
- apisixtls.go: restore 'should warn on missing TLS secrets'
  (was incorrectly changed to deny; webhook already admits when warnings exist)
- apisixroute.go: add 'should reject route update that fails ADC validation'
- apisixconsumer.go: add 'should reject consumer update that fails ADC validation'
- apisixtls.go: add 'should reject TLS update with invalid certificate material'
- consumer.go: add 'should reject consumer update that fails ADC validation'

Co-authored-by: Copilot &lt;223556219+Copilot@users.noreply.github.com&gt;
diff --git a/internal/webhook/v1/apisixconsumer_webhook.go b/internal/webhook/v1/apisixconsumer_webhook.go
@@ -75,8 +75,7 @@ func (v *ApisixConsumerCustomValidator) ValidateCreate(ctx context.Context, obj
 	}
 
 	warnings := v.collectWarnings(ctx, consumer)
-	if v.initErr != nil {
-		apisixConsumerLog.Error(v.initErr, "ADC validator init failed, skipping ADC validation")
+	if v.initErr != nil || len(warnings) > 0 {
 		return warnings, nil
 	}
 	return warnings, v.adcValidator.Validate(ctx, consumer)
@@ -93,8 +92,7 @@ func (v *ApisixConsumerCustomValidator) ValidateUpdate(ctx context.Context, oldO
 	}
 
 	warnings := v.collectWarnings(ctx, consumer)
-	if v.initErr != nil {
-		apisixConsumerLog.Error(v.initErr, "ADC validator init failed, skipping ADC validation")
+	if v.initErr != nil || len(warnings) > 0 {
 		return warnings, nil
 	}
 	return warnings, v.adcValidator.Validate(ctx, consumer)
diff --git a/test/e2e/webhook/apisixconsumer.go b/test/e2e/webhook/apisixconsumer.go
@@ -47,7 +47,7 @@ var _ = Describe("Test ApisixConsumer Webhook", Label("webhook"), func() {
 		time.Sleep(5 * time.Second)
 	})
 
-	It("should reject missing authentication secrets", func() {
+	It("should warn on missing authentication secrets", func() {
 		missingSecret := "missing-basic-secret"
 		consumerName := "webhook-apisixconsumer"
 		consumerYAML := `
@@ -65,7 +65,7 @@ spec:
 `
 
 		output, err := s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(consumerYAML, consumerName, s.Namespace(), s.Namespace(), missingSecret))
-		expectAdmissionDenied(s, "apisixconsumer", consumerName, err, fmt.Sprintf("%s/%s", s.Namespace(), missingSecret))
+		Expect(err).ShouldNot(HaveOccurred())
 		Expect(output).To(ContainSubstring(fmt.Sprintf("Warning: Referenced Secret '%s/%s' not found", s.Namespace(), missingSecret)))
 
 		By("creating referenced secret")
@@ -152,4 +152,56 @@ spec:
 		err = s.CreateResourceFromString(correctedConsumer)
 		Expect(err).NotTo(HaveOccurred(), "creating corrected ApisixConsumer")
 	})
+
+	It("should reject consumer update that fails ADC validation", func() {
+		if framework.ProviderType != framework.ProviderTypeAPISIXStandalone {
+			Skip("ADC validation requires apisix-standalone backend")
+		}
+
+		consumerName := "webhook-apisixconsumer-update"
+
+		validConsumer := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2
+kind: ApisixConsumer
+metadata:
+  name: %s
+  namespace: %s
+spec:
+  ingressClassName: %s
+  authParameter:
+    keyAuth:
+      value:
+        key: update-test-key
+`, consumerName, s.Namespace(), s.Namespace())
+
+		By("creating valid ApisixConsumer")
+		err := s.CreateResourceFromString(validConsumer)
+		Expect(err).NotTo(HaveOccurred(), "creating initial valid ApisixConsumer")
+
+		privateKeyYAML := "          " + strings.ReplaceAll(framework.TestKey, "\n", "\n          ")
+		invalidConsumer := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2
+kind: ApisixConsumer
+metadata:
+  name: %s
+  namespace: %s
+spec:
+  ingressClassName: %s
+  authParameter:
+    jwtAuth:
+      value:
+        key: update-test-jwt-key
+        algorithm: INVALID_ALGO
+        private_key: |
+%s
+`, consumerName, s.Namespace(), s.Namespace(), privateKeyYAML)
+
+		By("updating ApisixConsumer with invalid jwt-auth algorithm")
+		err = s.CreateResourceFromString(invalidConsumer)
+		expectAdmissionDenied(s, "apisixconsumer", consumerName, err)
+
+		By("updating ApisixConsumer with corrected config")
+		err = s.CreateResourceFromString(validConsumer)
+		Expect(err).NotTo(HaveOccurred(), "updating ApisixConsumer with corrected config")
+	})
 })
diff --git a/test/e2e/webhook/apisixroute.go b/test/e2e/webhook/apisixroute.go
@@ -179,4 +179,90 @@ spec:
 		err = s.CreateResourceFromString(validRouteYAML)
 		Expect(err).NotTo(HaveOccurred(), "creating corrected ApisixRoute")
 	})
+
+	It("should reject route update that fails ADC validation", func() {
+		if framework.ProviderType != framework.ProviderTypeAPISIXStandalone {
+			Skip("ADC validation requires apisix-standalone backend")
+		}
+
+		backendService := "webhook-route-update-backend"
+		routeName := "webhook-apisixroute-update"
+
+		By("creating referenced Service")
+		serviceYAML := fmt.Sprintf(`
+apiVersion: v1
+kind: Service
+metadata:
+  name: %s
+spec:
+  selector:
+    app: placeholder
+  ports:
+  - name: http
+    port: 80
+    targetPort: 80
+  type: ClusterIP
+`, backendService)
+		err := s.CreateResourceFromString(serviceYAML)
+		Expect(err).NotTo(HaveOccurred(), "creating backend service")
+
+		validRouteYAML := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2
+kind: ApisixRoute
+metadata:
+  name: %s
+  namespace: %s
+spec:
+  ingressClassName: %s
+  http:
+  - name: rule-update
+    match:
+      hosts:
+      - webhook-update.example.com
+      paths:
+      - /update
+    backends:
+    - serviceName: %s
+      servicePort: 80
+      resolveGranularity: service
+`, routeName, s.Namespace(), s.Namespace(), backendService)
+
+		By("creating valid ApisixRoute")
+		err = s.CreateResourceFromString(validRouteYAML)
+		Expect(err).NotTo(HaveOccurred(), "creating initial valid ApisixRoute")
+
+		invalidRouteYAML := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2
+kind: ApisixRoute
+metadata:
+  name: %s
+  namespace: %s
+spec:
+  ingressClassName: %s
+  http:
+  - name: rule-update
+    match:
+      hosts:
+      - webhook-update.example.com
+      paths:
+      - /update
+    backends:
+    - serviceName: %s
+      servicePort: 80
+      resolveGranularity: service
+    plugins:
+    - name: response-rewrite
+      enable: true
+      config:
+        status_code: "500"
+`, routeName, s.Namespace(), s.Namespace(), backendService)
+
+		By("updating ApisixRoute with invalid plugin config")
+		err = s.CreateResourceFromString(invalidRouteYAML)
+		expectAdmissionDenied(s, "apisixroute", routeName, err)
+
+		By("updating ApisixRoute with corrected config")
+		err = s.CreateResourceFromString(validRouteYAML)
+		Expect(err).NotTo(HaveOccurred(), "updating ApisixRoute with corrected config")
+	})
 })
diff --git a/test/e2e/webhook/apisixtls.go b/test/e2e/webhook/apisixtls.go
@@ -46,7 +46,7 @@ var _ = Describe("Test ApisixTls Webhook", Label("webhook"), func() {
 		time.Sleep(5 * time.Second)
 	})
 
-	It("should reject missing TLS secrets", func() {
+	It("should warn on missing TLS secrets", func() {
 		serverSecret := "missing-server-tls"
 		clientSecret := "missing-client-ca"
 		tlsName := "webhook-apisixtls"
@@ -70,7 +70,7 @@ spec:
 `
 
 		output, err := s.CreateResourceFromStringAndGetOutput(fmt.Sprintf(tlsYAML, tlsName, s.Namespace(), s.Namespace(), serverSecret, s.Namespace(), clientSecret, s.Namespace()))
-		expectAdmissionDenied(s, "apisixtls", tlsName, err, fmt.Sprintf("%s/%s", s.Namespace(), serverSecret))
+		Expect(err).ShouldNot(HaveOccurred())
 		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)))
 
@@ -149,4 +149,76 @@ spec:
 		err = s.CreateResourceFromString(tlsYAML)
 		Expect(err).NotTo(HaveOccurred(), "creating corrected ApisixTls")
 	})
+
+	It("should reject TLS update with invalid certificate material", func() {
+		if framework.ProviderType != framework.ProviderTypeAPISIXStandalone {
+			Skip("ADC validation requires apisix-standalone backend")
+		}
+
+		serverSecret := "update-server-tls"
+		tlsName := "webhook-apisixtls-update"
+		host := "update-webhook.example.com"
+
+		By("creating a valid TLS secret")
+		serverCert, serverKey := s.GenerateCert(GinkgoT(), []string{host})
+		err := s.NewKubeTlsSecret(serverSecret, serverCert.String(), serverKey.String())
+		Expect(err).NotTo(HaveOccurred(), "creating initial valid server TLS secret")
+
+		tlsYAML := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2
+kind: ApisixTls
+metadata:
+  name: %s
+  namespace: %s
+spec:
+  ingressClassName: %s
+  hosts:
+  - %s
+  secret:
+    name: %s
+    namespace: %s
+`, tlsName, s.Namespace(), s.Namespace(), host, serverSecret, s.Namespace())
+
+		By("creating valid ApisixTls")
+		err = s.CreateResourceFromString(tlsYAML)
+		Expect(err).NotTo(HaveOccurred(), "creating initial valid ApisixTls")
+
+		By("replacing secret with invalid certificate data")
+		err = s.DeleteResource("Secret", serverSecret)
+		Expect(err).NotTo(HaveOccurred(), "deleting valid server TLS secret")
+		invalidSecretYAML := fmt.Sprintf(`
+apiVersion: v1
+kind: Secret
+metadata:
+  name: %s
+  namespace: %s
+type: kubernetes.io/tls
+stringData:
+  tls.crt: not-a-cert
+  tls.key: not-a-key
+`, serverSecret, s.Namespace())
+		err = s.CreateResourceFromString(invalidSecretYAML)
+		Expect(err).NotTo(HaveOccurred(), "creating invalid server TLS secret")
+
+		// Wait for the webhook cache to reflect the replaced Secret.
+		time.Sleep(2 * time.Second)
+
+		By("updating ApisixTls with secret now containing invalid certificate data")
+		err = s.CreateResourceFromString(tlsYAML)
+		expectAdmissionDenied(s, "apisixtls", tlsName, err)
+
+		By("replacing secret back with valid certificate data")
+		err = s.DeleteResource("Secret", serverSecret)
+		Expect(err).NotTo(HaveOccurred(), "deleting invalid server TLS secret")
+		serverCert, serverKey = s.GenerateCert(GinkgoT(), []string{host})
+		err = s.NewKubeTlsSecret(serverSecret, serverCert.String(), serverKey.String())
+		Expect(err).NotTo(HaveOccurred(), "recreating valid server TLS secret")
+
+		// Wait for the webhook cache to reflect the restored Secret.
+		time.Sleep(2 * time.Second)
+
+		By("updating ApisixTls with valid certificate data")
+		err = s.CreateResourceFromString(tlsYAML)
+		Expect(err).NotTo(HaveOccurred(), "updating ApisixTls with valid certificate")
+	})
 })
diff --git a/test/e2e/webhook/consumer.go b/test/e2e/webhook/consumer.go
@@ -159,4 +159,73 @@ spec:
 		err = s.CreateResourceFromString(correctedConsumer)
 		Expect(err).NotTo(HaveOccurred(), "creating corrected Consumer")
 	})
+
+	It("should reject consumer update that fails ADC validation", func() {
+		if framework.ProviderType != framework.ProviderTypeAPISIXStandalone {
+			Skip("ADC validation requires apisix-standalone backend")
+		}
+
+		gatewayName := s.Namespace()
+		consumerName := "webhook-consumer-update"
+
+		validConsumer := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v1alpha1
+kind: Consumer
+metadata:
+  name: %s
+spec:
+  gatewayRef:
+    name: %s
+  credentials:
+  - type: key-auth
+    name: key-auth-update
+    config:
+      key: update-consumer-key
+`, consumerName, gatewayName)
+
+		By("creating valid Consumer")
+		err := s.CreateResourceFromString(validConsumer)
+		Expect(err).NotTo(HaveOccurred(), "creating initial valid Consumer")
+
+		invalidConsumer := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v1alpha1
+kind: Consumer
+metadata:
+  name: %s
+spec:
+  gatewayRef:
+    name: %s
+  credentials:
+  - type: jwt-auth
+    name: jwt-cred-update
+    config:
+      key: update-consumer-jwt-key
+      algorithm: INVALID_ALGO
+`, consumerName, gatewayName)
+
+		By("updating Consumer with an invalid jwt-auth algorithm")
+		err = s.CreateResourceFromString(invalidConsumer)
+		expectAdmissionDenied(s, "consumer", consumerName, err)
+
+		correctedConsumer := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v1alpha1
+kind: Consumer
+metadata:
+  name: %s
+spec:
+  gatewayRef:
+    name: %s
+  credentials:
+  - type: jwt-auth
+    name: jwt-cred-update
+    config:
+      key: update-consumer-jwt-key
+      algorithm: HS256
+      secret: update-consumer-secret
+`, consumerName, gatewayName)
+
+		By("updating Consumer with a valid algorithm")
+		err = s.CreateResourceFromString(correctedConsumer)
+		Expect(err).NotTo(HaveOccurred(), "updating Consumer with corrected config")
+	})
 })

Original file line number	Diff line number	Diff line change
`@@ -75,8 +75,7 @@ func (v *ApisixConsumerCustomValidator) ValidateCreate(ctx context.Context, obj`
`75`	`75`	`}`
`76`	`76`
`77`	`77`	`warnings := v.collectWarnings(ctx, consumer)`
`78`		`- if v.initErr != nil {`
`79`		`- apisixConsumerLog.Error(v.initErr, "ADC validator init failed, skipping ADC validation")`
	`78`	`+ if v.initErr != nil \|\| len(warnings) > 0 {`
`80`	`79`	`return warnings, nil`
`81`	`80`	`}`
`82`	`81`	`return warnings, v.adcValidator.Validate(ctx, consumer)`
`@@ -93,8 +92,7 @@ func (v *ApisixConsumerCustomValidator) ValidateUpdate(ctx context.Context, oldO`
`93`	`92`	`}`
`94`	`93`
`95`	`94`	`warnings := v.collectWarnings(ctx, consumer)`
`96`		`- if v.initErr != nil {`
`97`		`- apisixConsumerLog.Error(v.initErr, "ADC validator init failed, skipping ADC validation")`
	`95`	`+ if v.initErr != nil \|\| len(warnings) > 0 {`
`98`	`96`	`return warnings, nil`
`99`	`97`	`}`
`100`	`98`	`return warnings, v.adcValidator.Validate(ctx, consumer)`