Skip to content

Commit 0b74a9c

Browse files
AlinsRanCopilot
andcommitted
fix: use validate API in webhook checks
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ba91ae8 commit 0b74a9c

4 files changed

Lines changed: 146 additions & 8 deletions

File tree

internal/adc/client/executor.go

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ func (e *HTTPADCExecutor) runHTTPValidateForSingleServer(ctx context.Context, se
262262
req, err = e.buildAPISIXValidateRequest(ctx, serverAddr, config, resources)
263263
httpClient = e.newBackendHTTPClient(config)
264264
} else {
265-
req, err = e.buildHTTPRequest(ctx, serverAddr, config, labels, types, resources, http.MethodPost, "/sync")
265+
req, err = e.buildHTTPRequest(ctx, serverAddr, config, labels, types, resources, http.MethodPost, "/validate")
266266
}
267267
if err != nil {
268268
return fmt.Errorf("failed to build validate request: %w", err)
@@ -367,7 +367,7 @@ func buildAPISIXValidatePayload(resources *adctypes.Resources) (*apisixValidateR
367367
}
368368

369369
for _, route := range service.Routes {
370-
routeMap, err := toMap(route)
370+
routeMap, err := buildAPISIXRouteValidateObject(route)
371371
if err != nil {
372372
return nil, err
373373
}
@@ -387,7 +387,7 @@ func buildAPISIXValidatePayload(resources *adctypes.Resources) (*apisixValidateR
387387
}
388388

389389
for _, consumer := range resources.Consumers {
390-
consumerMap, err := toMap(consumer)
390+
consumerMap, err := buildAPISIXConsumerValidateObject(consumer)
391391
if err != nil {
392392
return nil, err
393393
}
@@ -405,6 +405,40 @@ func buildAPISIXValidatePayload(resources *adctypes.Resources) (*apisixValidateR
405405
return body, nil
406406
}
407407

408+
func buildAPISIXRouteValidateObject(route *adctypes.Route) (map[string]any, error) {
409+
routeMap, err := toMap(route)
410+
if err != nil {
411+
return nil, err
412+
}
413+
414+
delete(routeMap, "description")
415+
return routeMap, nil
416+
}
417+
418+
func buildAPISIXConsumerValidateObject(consumer *adctypes.Consumer) (map[string]any, error) {
419+
consumerMap, err := toMap(consumer)
420+
if err != nil {
421+
return nil, err
422+
}
423+
424+
if len(consumer.Credentials) == 0 {
425+
return consumerMap, nil
426+
}
427+
428+
plugins, ok := consumerMap["plugins"].(map[string]any)
429+
if !ok || plugins == nil {
430+
plugins = make(map[string]any, len(consumer.Credentials))
431+
}
432+
433+
for _, credential := range consumer.Credentials {
434+
plugins[credential.Type] = credential.Config
435+
}
436+
437+
consumerMap["plugins"] = plugins
438+
delete(consumerMap, "credentials")
439+
return consumerMap, nil
440+
}
441+
408442
func buildAPISIXSSLValidateObject(ssl *adctypes.SSL) (map[string]any, error) {
409443
sslMap, err := toMap(ssl)
410444
if err != nil {

internal/adc/client/executor_test.go

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ package client
1818
import (
1919
"testing"
2020

21-
adctypes "github.com/apache/apisix-ingress-controller/api/adc"
2221
"github.com/stretchr/testify/require"
22+
23+
adctypes "github.com/apache/apisix-ingress-controller/api/adc"
2324
)
2425

2526
func TestBuildAPISIXValidatePayloadConvertsSSLCertificates(t *testing.T) {
@@ -80,3 +81,64 @@ func TestBuildAPISIXValidatePayloadConvertsSingleSSLCertificate(t *testing.T) {
8081
_, ok = ssl["keys"]
8182
require.False(t, ok)
8283
}
84+
85+
func TestBuildAPISIXValidatePayloadStripsRouteDescription(t *testing.T) {
86+
body, err := buildAPISIXValidatePayload(&adctypes.Resources{
87+
Services: []*adctypes.Service{
88+
{
89+
Metadata: adctypes.Metadata{ID: "svc-1"},
90+
Routes: []*adctypes.Route{
91+
{
92+
Metadata: adctypes.Metadata{
93+
ID: "route-1",
94+
Desc: "should not be sent to standalone validate",
95+
},
96+
Uris: []string{"/test"},
97+
},
98+
},
99+
},
100+
},
101+
})
102+
require.NoError(t, err)
103+
require.Len(t, body.Routes, 1)
104+
105+
route := body.Routes[0]
106+
require.Equal(t, "route-1", route["id"])
107+
_, ok := route["description"]
108+
require.False(t, ok)
109+
require.Equal(t, "svc-1", route["service_id"])
110+
}
111+
112+
func TestBuildAPISIXValidatePayloadConvertsConsumerCredentialsToPlugins(t *testing.T) {
113+
body, err := buildAPISIXValidatePayload(&adctypes.Resources{
114+
Consumers: []*adctypes.Consumer{
115+
{
116+
Metadata: adctypes.Metadata{ID: "consumer-1"},
117+
Username: "demo",
118+
Plugins: adctypes.Plugins{
119+
"limit-count": map[string]any{"count": 10},
120+
},
121+
Credentials: []adctypes.Credential{
122+
{
123+
Type: "key-auth",
124+
Config: adctypes.Plugins{
125+
"key": "shared-key",
126+
},
127+
},
128+
},
129+
},
130+
},
131+
})
132+
require.NoError(t, err)
133+
require.Len(t, body.Consumers, 1)
134+
135+
consumer := body.Consumers[0]
136+
require.Equal(t, "demo", consumer["username"])
137+
_, ok := consumer["credentials"]
138+
require.False(t, ok)
139+
140+
plugins, ok := consumer["plugins"].(map[string]any)
141+
require.True(t, ok)
142+
require.Contains(t, plugins, "key-auth")
143+
require.Contains(t, plugins, "limit-count")
144+
}

internal/webhook/v1/adc_validation_test.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,17 @@ import (
3434
func withMockADCServer(t *testing.T, handler http.HandlerFunc) string {
3535
t.Helper()
3636

37-
server := httptest.NewServer(http.HandlerFunc(handler))
37+
server := httptest.NewServer(handler)
3838
t.Setenv("ADC_SERVER_URL", server.URL)
3939
t.Cleanup(server.Close)
4040
return server.URL
4141
}
4242

4343
func managedIngressClassWithGatewayProxy(endpoint string) []runtime.Object {
44+
return managedIngressClassWithGatewayProxyMode(endpoint, "apisix-standalone")
45+
}
46+
47+
func managedIngressClassWithGatewayProxyMode(endpoint, mode string) []runtime.Object {
4448
namespace := "default"
4549

4650
return []runtime.Object{
@@ -65,7 +69,7 @@ func managedIngressClassWithGatewayProxy(endpoint string) []runtime.Object {
6569
Provider: &apisixv1alpha1.GatewayProxyProvider{
6670
Type: apisixv1alpha1.ProviderTypeControlPlane,
6771
ControlPlane: &apisixv1alpha1.ControlPlaneProvider{
68-
Mode: "apisix-standalone",
72+
Mode: mode,
6973
Endpoints: []string{endpoint},
7074
Auth: apisixv1alpha1.ControlPlaneAuth{
7175
Type: apisixv1alpha1.AuthTypeAdminKey,
@@ -86,3 +90,9 @@ func requireValidateRequest(t *testing.T, r *http.Request) {
8690
require.Equal(t, "/apisix/admin/configs/validate", r.URL.Path)
8791
require.Equal(t, "token", r.Header.Get("X-API-KEY"))
8892
}
93+
94+
func requireADCServerValidateRequest(t *testing.T, r *http.Request) {
95+
t.Helper()
96+
require.Equal(t, http.MethodPost, r.Method)
97+
require.Equal(t, "/validate", r.URL.Path)
98+
}

internal/webhook/v1/apisixconsumer_webhook_test.go

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ import (
3333
"github.com/apache/apisix-ingress-controller/internal/controller/config"
3434
)
3535

36+
const managedIngressClassName = "apisix"
37+
3638
func buildApisixConsumerValidator(t *testing.T, objects ...runtime.Object) *ApisixConsumerCustomValidator {
3739
t.Helper()
3840

@@ -46,15 +48,15 @@ func buildApisixConsumerValidator(t *testing.T, objects ...runtime.Object) *Apis
4648
hasManagedIngressClass := false
4749
for _, obj := range objects {
4850
ingressClass, ok := obj.(*networkingv1.IngressClass)
49-
if ok && ingressClass.Name == "apisix" {
51+
if ok && ingressClass.Name == managedIngressClassName {
5052
hasManagedIngressClass = true
5153
break
5254
}
5355
}
5456
if !hasManagedIngressClass {
5557
managed = append(managed, &networkingv1.IngressClass{
5658
ObjectMeta: metav1.ObjectMeta{
57-
Name: "apisix",
59+
Name: managedIngressClassName,
5860
Annotations: map[string]string{
5961
"ingressclass.kubernetes.io/is-default-class": "true",
6062
},
@@ -203,3 +205,33 @@ func TestApisixConsumerValidator_DeniesOnADCValidationFailure(t *testing.T) {
203205
require.Contains(t, err.Error(), "consumer rejected")
204206
require.Empty(t, warnings)
205207
}
208+
209+
func TestApisixConsumerValidator_UsesADCValidateEndpointForControlPlane(t *testing.T) {
210+
serverURL := withMockADCServer(t, func(w http.ResponseWriter, r *http.Request) {
211+
requireADCServerValidateRequest(t, r)
212+
w.WriteHeader(http.StatusBadRequest)
213+
_, _ = w.Write([]byte(`{"errorMessage":"consumer rejected","errors":[{"resource_type":"consumers","resource_name":"demo","message":"duplicate credential"}]}`))
214+
})
215+
216+
consumer := &apisixv2.ApisixConsumer{
217+
ObjectMeta: metav1.ObjectMeta{
218+
Name: "demo",
219+
Namespace: "default",
220+
},
221+
Spec: apisixv2.ApisixConsumerSpec{
222+
IngressClassName: managedIngressClassName,
223+
AuthParameter: apisixv2.ApisixConsumerAuthParameter{
224+
KeyAuth: &apisixv2.ApisixConsumerKeyAuth{
225+
Value: &apisixv2.ApisixConsumerKeyAuthValue{Key: "shared-key"},
226+
},
227+
},
228+
},
229+
}
230+
231+
validator := buildApisixConsumerValidator(t, managedIngressClassWithGatewayProxyMode(serverURL, "apisix")...)
232+
233+
warnings, err := validator.ValidateCreate(context.Background(), consumer)
234+
require.Error(t, err)
235+
require.Contains(t, err.Error(), "consumer rejected")
236+
require.Empty(t, warnings)
237+
}

0 commit comments

Comments
 (0)