Skip to content

Commit

Permalink
add w.Service nil check
Browse files Browse the repository at this point in the history
  • Loading branch information
alexey-kremsa-globant committed Jun 8, 2020
1 parent 82f0320 commit 139140e
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 12 deletions.
16 changes: 10 additions & 6 deletions pkg/fleetautoscalers/fleetautoscalers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
}
Expand Down Expand Up @@ -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)
Expand Down
24 changes: 18 additions & 6 deletions pkg/fleetautoscalers/fleetautoscalers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
},
},
{
Expand All @@ -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",
},
},
{
Expand Down Expand Up @@ -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 {
Expand All @@ -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",
Expand All @@ -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)
Expand Down Expand Up @@ -613,7 +625,7 @@ func TestCreateURL(t *testing.T) {
}

func TestBuildURLFromWebhookPolicyNoNamespace(t *testing.T) {
url := "/testurl"
url := "testurl"

type expected struct {
url string
Expand Down

0 comments on commit 139140e

Please sign in to comment.