Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 122 additions & 2 deletions internal/webhook/v1/gatewayproxy_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package v1
import (
"context"
"fmt"
"sort"
"strings"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -63,7 +65,12 @@ func (v *GatewayProxyCustomValidator) ValidateCreate(ctx context.Context, obj ru
}
gatewayProxyLog.Info("Validation for GatewayProxy upon creation", "name", gp.GetName(), "namespace", gp.GetNamespace())

return v.collectWarnings(ctx, gp), nil
warnings := v.collectWarnings(ctx, gp)
if err := v.validateGatewayProxyConflict(ctx, gp); err != nil {
return nil, err
}

return warnings, nil
}

func (v *GatewayProxyCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
Expand All @@ -73,7 +80,12 @@ func (v *GatewayProxyCustomValidator) ValidateUpdate(ctx context.Context, oldObj
}
gatewayProxyLog.Info("Validation for GatewayProxy upon update", "name", gp.GetName(), "namespace", gp.GetNamespace())

return v.collectWarnings(ctx, gp), nil
warnings := v.collectWarnings(ctx, gp)
if err := v.validateGatewayProxyConflict(ctx, gp); err != nil {
return nil, err
}

return warnings, nil
}

func (v *GatewayProxyCustomValidator) ValidateDelete(context.Context, runtime.Object) (admission.Warnings, error) {
Expand Down Expand Up @@ -111,3 +123,111 @@ func (v *GatewayProxyCustomValidator) collectWarnings(ctx context.Context, gp *v

return warnings
}

func (v *GatewayProxyCustomValidator) validateGatewayProxyConflict(ctx context.Context, gp *v1alpha1.GatewayProxy) error {
current := buildGatewayProxyConfig(gp)
if !current.readyForConflict() {
return nil
}

var list v1alpha1.GatewayProxyList
if err := v.Client.List(ctx, &list); err != nil {
gatewayProxyLog.Error(err, "failed to list GatewayProxy objects for conflict detection")
return fmt.Errorf("failed to list existing GatewayProxy resources: %w", err)
}

for _, other := range list.Items {
if other.GetNamespace() == gp.GetNamespace() && other.GetName() == gp.GetName() {
// skip self
continue
}
otherConfig := buildGatewayProxyConfig(&other)
if !otherConfig.readyForConflict() {
continue
}
if current.adminKeyKey != otherConfig.adminKeyKey {
continue
}
if current.serviceKey != "" && current.serviceKey == otherConfig.serviceKey {
return fmt.Errorf("gateway proxy configuration conflict: GatewayProxy %s/%s and %s/%s both target %s while sharing %s",
gp.GetNamespace(), gp.GetName(),
other.GetNamespace(), other.GetName(),
current.serviceDescription,
current.adminKeyDescription,
)
}
if len(current.endpoints) > 0 && len(otherConfig.endpoints) > 0 {
if overlap := current.endpointOverlap(otherConfig); len(overlap) > 0 {
return fmt.Errorf("gateway proxy configuration conflict: GatewayProxy %s/%s and %s/%s both target control plane endpoints [%s] while sharing %s",
gp.GetNamespace(), gp.GetName(),
other.GetNamespace(), other.GetName(),
strings.Join(overlap, ", "),
current.adminKeyDescription,
)
}
}
}

return nil
}

type gatewayProxyConfig struct {
adminKeyKey string
adminKeyDescription string
serviceKey string
serviceDescription string
endpoints map[string]struct{}
}
Comment on lines 174 to 180
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description is somewhat redundant,
AdminKey only does one thing, you can add a secretKey to distinguish it.


func buildGatewayProxyConfig(gp *v1alpha1.GatewayProxy) gatewayProxyConfig {
var cfg gatewayProxyConfig

if gp == nil || gp.Spec.Provider == nil || gp.Spec.Provider.Type != v1alpha1.ProviderTypeControlPlane || gp.Spec.Provider.ControlPlane == nil {
return cfg
}

cp := gp.Spec.Provider.ControlPlane

if cp.Auth.AdminKey != nil {
if value := strings.TrimSpace(cp.Auth.AdminKey.Value); value != "" {
cfg.adminKeyKey = "value:" + value
cfg.adminKeyDescription = "the same inline AdminKey value"
} else if cp.Auth.AdminKey.ValueFrom != nil && cp.Auth.AdminKey.ValueFrom.SecretKeyRef != nil {
ref := cp.Auth.AdminKey.ValueFrom.SecretKeyRef
cfg.adminKeyKey = fmt.Sprintf("secret:%s/%s:%s", gp.GetNamespace(), ref.Name, ref.Key)
cfg.adminKeyDescription = fmt.Sprintf("AdminKey secret %s/%s key %s", gp.GetNamespace(), ref.Name, ref.Key)
}
}

if cp.Service != nil && cp.Service.Name != "" {
cfg.serviceKey = fmt.Sprintf("service:%s/%s:%d", gp.GetNamespace(), cp.Service.Name, cp.Service.Port)
cfg.serviceDescription = fmt.Sprintf("Service %s/%s port %d", gp.GetNamespace(), cp.Service.Name, cp.Service.Port)
}

if len(cp.Endpoints) > 0 {
cfg.endpoints = make(map[string]struct{}, len(cp.Endpoints))
for _, endpoint := range cp.Endpoints {
cfg.endpoints[endpoint] = struct{}{}
}
}

return cfg
}

func (c gatewayProxyConfig) readyForConflict() bool {
if c.adminKeyKey == "" {
return false
}
return c.serviceKey != "" || len(c.endpoints) > 0
}

func (c gatewayProxyConfig) endpointOverlap(other gatewayProxyConfig) []string {
var overlap []string
for endpoint := range c.endpoints {
if _, ok := other.endpoints[endpoint]; ok {
overlap = append(overlap, endpoint)
}
}
sort.Strings(overlap)
return overlap
}
217 changes: 216 additions & 1 deletion internal/webhook/v1/gatewayproxy_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ import (
v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1"
)

const (
candidateName = "candidate"
)

func buildGatewayProxyValidator(t *testing.T, objects ...runtime.Object) *GatewayProxyCustomValidator {
t.Helper()

Expand All @@ -54,7 +58,7 @@ func newGatewayProxy() *v1alpha1.GatewayProxy {
Provider: &v1alpha1.GatewayProxyProvider{
Type: v1alpha1.ProviderTypeControlPlane,
ControlPlane: &v1alpha1.ControlPlaneProvider{
Service: &v1alpha1.ProviderService{Name: "control-plane"},
Service: &v1alpha1.ProviderService{Name: "control-plane", Port: 9180},
Auth: v1alpha1.ControlPlaneAuth{
Type: v1alpha1.AuthTypeAdminKey,
AdminKey: &v1alpha1.AdminKeyAuth{
Expand All @@ -72,6 +76,41 @@ func newGatewayProxy() *v1alpha1.GatewayProxy {
}
}

func newGatewayProxyWithEndpoints(name string, endpoints []string) *v1alpha1.GatewayProxy {
gp := newGatewayProxy()
gp.Name = name
gp.Spec.Provider.ControlPlane.Service = nil
gp.Spec.Provider.ControlPlane.Endpoints = endpoints
return gp
}

func setInlineAdminKey(gp *v1alpha1.GatewayProxy, value string) {
if gp == nil || gp.Spec.Provider == nil || gp.Spec.Provider.ControlPlane == nil {
return
}
if gp.Spec.Provider.ControlPlane.Auth.AdminKey == nil {
gp.Spec.Provider.ControlPlane.Auth.AdminKey = &v1alpha1.AdminKeyAuth{}
}
gp.Spec.Provider.ControlPlane.Auth.AdminKey.Value = value
gp.Spec.Provider.ControlPlane.Auth.AdminKey.ValueFrom = nil
}

func setSecretAdminKey(gp *v1alpha1.GatewayProxy, name, key string) {
if gp == nil || gp.Spec.Provider == nil || gp.Spec.Provider.ControlPlane == nil {
return
}
if gp.Spec.Provider.ControlPlane.Auth.AdminKey == nil {
gp.Spec.Provider.ControlPlane.Auth.AdminKey = &v1alpha1.AdminKeyAuth{}
}
gp.Spec.Provider.ControlPlane.Auth.AdminKey.Value = ""
gp.Spec.Provider.ControlPlane.Auth.AdminKey.ValueFrom = &v1alpha1.AdminKeyValueFrom{
SecretKeyRef: &v1alpha1.SecretKeySelector{
Name: name,
Key: key,
},
}
}

func TestGatewayProxyValidator_MissingService(t *testing.T) {
gp := newGatewayProxy()
gp.Spec.Provider.ControlPlane.Auth.AdminKey = nil
Expand Down Expand Up @@ -150,3 +189,179 @@ func TestGatewayProxyValidator_NoWarnings(t *testing.T) {
require.NoError(t, err)
require.Empty(t, warnings)
}

func TestGatewayProxyValidator_DetectsServiceConflict(t *testing.T) {
existing := newGatewayProxy()
existing.Name = "existing"

service := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "control-plane",
Namespace: "default",
},
}
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "admin-key",
Namespace: "default",
},
Data: map[string][]byte{
"token": []byte("value"),
},
}

validator := buildGatewayProxyValidator(t, existing, service, secret)

candidate := newGatewayProxy()
candidate.Name = candidateName

warnings, err := validator.ValidateCreate(context.Background(), candidate)
require.Error(t, err)
require.Len(t, warnings, 0)
require.Contains(t, err.Error(), "gateway proxy configuration conflict")
require.Contains(t, err.Error(), "Service default/control-plane port 9180")
require.Contains(t, err.Error(), "AdminKey secret default/admin-key key token")
}

func TestGatewayProxyValidator_DetectsEndpointConflict(t *testing.T) {
existing := newGatewayProxyWithEndpoints("existing", []string{"https://127.0.0.1:9443", "https://10.0.0.1:9443"})
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "admin-key",
Namespace: "default",
},
Data: map[string][]byte{
"token": []byte("value"),
},
}
validator := buildGatewayProxyValidator(t, existing, secret)

candidate := newGatewayProxyWithEndpoints(candidateName, []string{"https://10.0.0.1:9443", "https://127.0.0.1:9443"})

warnings, err := validator.ValidateCreate(context.Background(), candidate)
require.Error(t, err)
require.Len(t, warnings, 0)
require.Contains(t, err.Error(), "gateway proxy configuration conflict")
require.Contains(t, err.Error(), "endpoints [https://10.0.0.1:9443, https://127.0.0.1:9443]")
require.Contains(t, err.Error(), "AdminKey secret default/admin-key key token")
}

func TestGatewayProxyValidator_AllowsDistinctGatewayGroups(t *testing.T) {
existing := newGatewayProxyWithEndpoints("existing", []string{"https://127.0.0.1:9443"})
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "admin-key",
Namespace: "default",
},
Data: map[string][]byte{
"token": []byte("value"),
},
}
service := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "control-plane",
Namespace: "default",
},
}
validator := buildGatewayProxyValidator(t, existing, secret, service)

candidate := newGatewayProxy()
candidate.Name = candidateName
candidate.Spec.Provider.ControlPlane.Service = &v1alpha1.ProviderService{
Name: "control-plane",
Port: 9180,
}

warnings, err := validator.ValidateCreate(context.Background(), candidate)
require.NoError(t, err)
require.Empty(t, warnings)
}

func TestGatewayProxyValidator_AllowsServiceConflictWithDifferentAdminSecret(t *testing.T) {
existing := newGatewayProxy()
existing.Name = "existing"

candidate := newGatewayProxy()
candidate.Name = candidateName
setSecretAdminKey(candidate, "admin-key-alt", "token")

service := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "control-plane",
Namespace: "default",
},
}
existingSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "admin-key",
Namespace: "default",
},
Data: map[string][]byte{
"token": []byte("value"),
},
}
altSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "admin-key-alt",
Namespace: "default",
},
Data: map[string][]byte{
"token": []byte("value"),
},
}

validator := buildGatewayProxyValidator(t, existing, service, existingSecret, altSecret)

warnings, err := validator.ValidateCreate(context.Background(), candidate)
require.NoError(t, err)
require.Empty(t, warnings)
}

func TestGatewayProxyValidator_DetectsInlineAdminKeyConflict(t *testing.T) {
existing := newGatewayProxyWithEndpoints("existing", []string{"https://127.0.0.1:9443", "https://10.0.0.1:9443"})
setInlineAdminKey(existing, "inline-cred")

candidate := newGatewayProxyWithEndpoints(candidateName, []string{"https://10.0.0.1:9443"})
setInlineAdminKey(candidate, "inline-cred")

validator := buildGatewayProxyValidator(t, existing)

warnings, err := validator.ValidateCreate(context.Background(), candidate)
require.Error(t, err)
require.Len(t, warnings, 0)
require.Contains(t, err.Error(), "gateway proxy configuration conflict")
require.Contains(t, err.Error(), "control plane endpoints [https://10.0.0.1:9443]")
require.Contains(t, err.Error(), "inline AdminKey value")
}

func TestGatewayProxyValidator_AllowsEndpointOverlapWithDifferentAdminKey(t *testing.T) {
existing := newGatewayProxyWithEndpoints("existing", []string{"https://127.0.0.1:9443", "https://10.0.0.1:9443"})

candidate := newGatewayProxyWithEndpoints(candidateName, []string{"https://10.0.0.1:9443", "https://192.168.0.1:9443"})
setSecretAdminKey(candidate, "admin-key-alt", "token")

existingSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "admin-key",
Namespace: "default",
},
Data: map[string][]byte{
"token": []byte("value"),
},
}
altSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "admin-key-alt",
Namespace: "default",
},
Data: map[string][]byte{
"token": []byte("value"),
},
}

validator := buildGatewayProxyValidator(t, existing, existingSecret, altSecret)

warnings, err := validator.ValidateCreate(context.Background(), candidate)
require.NoError(t, err)
require.Empty(t, warnings)
}
Loading
Loading