diff --git a/pkg/fleetautoscalers/fleetautoscalers.go b/pkg/fleetautoscalers/fleetautoscalers.go index e244f9b606..2a809e6729 100644 --- a/pkg/fleetautoscalers/fleetautoscalers.go +++ b/pkg/fleetautoscalers/fleetautoscalers.go @@ -52,6 +52,10 @@ func computeDesiredFleetSize(fas *autoscalingv1.FleetAutoscaler, f *agonesv1.Fle } func buildURLFromWebhookPolicy(w *autoscalingv1.WebhookPolicy) (*url.URL, error) { + if w.URL != nil && w.Service != nil { + return nil, errors.New("service and URL cannot be used simultaneously") + } + if w.URL != nil { if *w.URL == "" { return nil, errors.New("URL was not provided") @@ -60,6 +64,10 @@ func buildURLFromWebhookPolicy(w *autoscalingv1.WebhookPolicy) (*url.URL, error) return url.ParseRequestURI(*w.URL) } + if w.Service == nil { + return nil, errors.New("service was not provided, either URL or Service must be provided") + } + if w.Service.Name == "" { return nil, errors.New("service name was not provided") } @@ -111,15 +119,11 @@ func setCABundle(caBundle []byte) error { func applyWebhookPolicy(w *autoscalingv1.WebhookPolicy, f *agonesv1.Fleet) (replicas int32, limited bool, err error) { if w == nil { - return 0, false, errors.New("nil WebhookPolicy passed") + return 0, false, errors.New("webhookPolicy parameter must not be nil") } if f == nil { - return 0, false, errors.New("nil Fleet passed") - } - - if w.URL != nil && w.Service != nil { - return 0, false, errors.New("service and url cannot be used simultaneously") + return 0, false, errors.New("fleet parameter must not be nil") } u, err := buildURLFromWebhookPolicy(w) diff --git a/pkg/fleetautoscalers/fleetautoscalers_test.go b/pkg/fleetautoscalers/fleetautoscalers_test.go index 3bd14bba83..0c5971c0b5 100644 --- a/pkg/fleetautoscalers/fleetautoscalers_test.go +++ b/pkg/fleetautoscalers/fleetautoscalers_test.go @@ -327,7 +327,7 @@ func TestApplyWebhookPolicy(t *testing.T) { defer server.Close() _, f := defaultWebhookFixtures() - url := "/scale" + url := "scale" emptyString := "" invalidURL := ")1golang.org/" wrongServerURL := "http://127.0.0.1:1" @@ -401,7 +401,7 @@ func TestApplyWebhookPolicy(t *testing.T) { expected: expected{ replicas: 0, limited: false, - err: "nil WebhookPolicy passed", + err: "webhookPolicy parameter must not be nil", }, }, { @@ -417,7 +417,7 @@ func TestApplyWebhookPolicy(t *testing.T) { expected: expected{ replicas: 0, limited: false, - err: "service and url cannot be used simultaneously", + err: "service and URL cannot be used simultaneously", }, }, { @@ -520,6 +520,18 @@ func TestApplyWebhookPolicy(t *testing.T) { err: "invalid character 'i' looking for beginning of value", }, }, + { + description: "Service and URL are nil", + webhookPolicy: &autoscalingv1.WebhookPolicy{ + Service: nil, + URL: nil, + }, + expected: expected{ + replicas: 0, + limited: false, + err: "service was not provided, either URL or Service must be provided", + }, + }, } for _, tc := range testCases { @@ -545,7 +557,7 @@ func TestApplyWebhookPolicy(t *testing.T) { func TestApplyWebhookPolicyNilFleet(t *testing.T) { t.Parallel() - url := "/scale" + url := "scale" w := &autoscalingv1.WebhookPolicy{ Service: &admregv1b.ServiceReference{ Name: "service1", @@ -557,7 +569,7 @@ func TestApplyWebhookPolicyNilFleet(t *testing.T) { replicas, limited, err := applyWebhookPolicy(w, nil) if assert.NotNil(t, err) { - assert.Equal(t, "nil Fleet passed", err.Error()) + assert.Equal(t, "fleet parameter must not be nil", err.Error()) } assert.False(t, limited) @@ -613,7 +625,7 @@ func TestCreateURL(t *testing.T) { } func TestBuildURLFromWebhookPolicyNoNamespace(t *testing.T) { - url := "/testurl" + url := "testurl" type expected struct { url string