Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Commit

Permalink
Fixing panic issue in Validate when properties are nil (#3242)
Browse files Browse the repository at this point in the history
  • Loading branch information
tariq1890 authored and jackfrancis committed Jun 15, 2018
1 parent d8ccad5 commit 109ab3d
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 0 deletions.
17 changes: 17 additions & 0 deletions pkg/api/apiloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"io/ioutil"
"reflect"

"fmt"

"github.com/Azure/acs-engine/pkg/api/agentPoolOnlyApi/v20170831"
"github.com/Azure/acs-engine/pkg/api/agentPoolOnlyApi/v20180331"
apvlabs "github.com/Azure/acs-engine/pkg/api/agentPoolOnlyApi/vlabs"
Expand Down Expand Up @@ -82,6 +84,9 @@ func (a *Apiloader) LoadContainerService(
}
}
setContainerServiceDefaultsv20160930(containerService)
if containerService.Properties == nil {
return nil, fmt.Errorf("missing ContainerService Properties")
}
if e := containerService.Properties.Validate(); validate && e != nil {
return nil, e
}
Expand All @@ -102,6 +107,9 @@ func (a *Apiloader) LoadContainerService(
}
}
setContainerServiceDefaultsv20160330(containerService)
if containerService.Properties == nil {
return nil, fmt.Errorf("missing ContainerService Properties")
}
if e := containerService.Properties.Validate(); validate && e != nil {
return nil, e
}
Expand All @@ -123,6 +131,9 @@ func (a *Apiloader) LoadContainerService(
}
}
setContainerServiceDefaultsv20170131(containerService)
if containerService.Properties == nil {
return nil, fmt.Errorf("missing ContainerService Properties")
}
if e := containerService.Properties.Validate(); validate && e != nil {
return nil, e
}
Expand All @@ -143,6 +154,9 @@ func (a *Apiloader) LoadContainerService(
return nil, e
}
}
if containerService.Properties == nil {
return nil, fmt.Errorf("missing ContainerService Properties")
}
if e := containerService.Properties.Validate(isUpdate); validate && e != nil {
return nil, e
}
Expand All @@ -168,6 +182,9 @@ func (a *Apiloader) LoadContainerService(
return nil, e
}
}
if containerService.Properties == nil {
return nil, fmt.Errorf("missing ContainerService Properties")
}
if e := containerService.Properties.Validate(isUpdate); validate && e != nil {
return nil, e
}
Expand Down
29 changes: 29 additions & 0 deletions pkg/api/apiloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"io/ioutil"
"os"
"path"
"testing"
)
Expand Down Expand Up @@ -211,3 +213,30 @@ func TestLoadContainerServiceForAgentPoolOnlyCluster(t *testing.T) {
})
})
}

func TestLoadContainerServiceWithNilProperties(t *testing.T) {
jsonWithoutProperties := `{
"type": "Microsoft.ContainerService/managedClusters",
"name": "[parameters('clusterName')]",
"apiVersion": "2017-07-01",
"location": "[resourceGroup().location]"
}`

tmpFile, err := ioutil.TempFile("", "containerService-invalid")
fileName := tmpFile.Name()
defer os.Remove(fileName)

err = ioutil.WriteFile(fileName, []byte(jsonWithoutProperties), os.ModeAppend)

apiloader := &Apiloader{}
existingContainerService := &ContainerService{Name: "test",
Properties: &Properties{OrchestratorProfile: &OrchestratorProfile{OrchestratorType: Kubernetes, OrchestratorVersion: "1.6.9"}}}
_, _, err = apiloader.LoadContainerServiceFromFile(fileName, true, false, existingContainerService)
if err == nil {
t.Errorf("Expected error to be thrown")
}
expectedMsg := "missing ContainerService Properties"
if err.Error() != expectedMsg {
t.Errorf("Expected error with message %s but got %s", expectedMsg, err.Error())
}
}
3 changes: 3 additions & 0 deletions pkg/api/vlabs/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ func init() {

// Validate implements APIObject
func (a *Properties) Validate(isUpdate bool) error {
if a == nil {
return fmt.Errorf("missing ContainerService Properties")
}
if e := validate.Struct(a); e != nil {
return handleValidationErrors(e.(validator.ValidationErrors))
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/api/vlabs/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,15 @@ func Test_Properties_ValidateAddons(t *testing.T) {
}
}

func TestPropertiesValidationNil(t *testing.T) {
containerService := &ContainerService{}
err := containerService.Properties.Validate(true)
expected := "missing ContainerService Properties"
if err == nil || err.Error() != expected {
t.Errorf("Expected error with message %s", expected)
}
}

func TestWindowsVersions(t *testing.T) {
for _, version := range common.GetAllSupportedKubernetesVersionsWindows() {
p := getK8sDefaultProperties(true)
Expand Down

0 comments on commit 109ab3d

Please sign in to comment.