From 1f8044c27203ff188878f1265006f5ea16ebd9a3 Mon Sep 17 00:00:00 2001 From: tariq1890 Date: Fri, 8 Jun 2018 23:39:23 -0700 Subject: [PATCH] Fixing panic issue in Validate when properties are nil --- .../v20170701/cs-without-properties.json | 6 ++++++ pkg/api/apiloader.go | 16 ++++++++++++++++ pkg/api/apiloader_test.go | 14 ++++++++++++++ pkg/api/vlabs/validate.go | 3 +++ pkg/api/vlabs/validate_test.go | 9 +++++++++ 5 files changed, 48 insertions(+) create mode 100644 pkg/acsengine/testdata/v20170701/cs-without-properties.json diff --git a/pkg/acsengine/testdata/v20170701/cs-without-properties.json b/pkg/acsengine/testdata/v20170701/cs-without-properties.json new file mode 100644 index 0000000000..267c9809e4 --- /dev/null +++ b/pkg/acsengine/testdata/v20170701/cs-without-properties.json @@ -0,0 +1,6 @@ +{ + "type": "Microsoft.ContainerService/managedClusters", + "name": "[parameters('clusterName')]", + "apiVersion": "2017-07-01", + "location": "[resourceGroup().location]" +} \ No newline at end of file diff --git a/pkg/api/apiloader.go b/pkg/api/apiloader.go index f23acd9920..6e418f93e4 100644 --- a/pkg/api/apiloader.go +++ b/pkg/api/apiloader.go @@ -6,6 +6,7 @@ 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" @@ -82,6 +83,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 } @@ -102,6 +106,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 } @@ -123,6 +130,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 } @@ -143,6 +153,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 } @@ -168,6 +181,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 } diff --git a/pkg/api/apiloader_test.go b/pkg/api/apiloader_test.go index 2c7c45f6de..cd6a909c6d 100644 --- a/pkg/api/apiloader_test.go +++ b/pkg/api/apiloader_test.go @@ -85,3 +85,17 @@ func TestLoadContainerServiceFromFile(t *testing.T) { t.Errorf("Failed to set orcherstator version to windows default when it is not set in the json API v20170131, got %s but expected %s", containerService.Properties.OrchestratorProfile.OrchestratorVersion, common.GetDefaultKubernetesVersionWindows()) } } + +func TestLoadContainerServiceWithNilProperties(t *testing.T) { + apiloader := &Apiloader{} + existingContainerService := &ContainerService{Name: "test", + Properties: &Properties{OrchestratorProfile: &OrchestratorProfile{OrchestratorType: Kubernetes, OrchestratorVersion: "1.6.9"}}} + _, _, err := apiloader.LoadContainerServiceFromFile("../acsengine/testdata/v20170701/cs-without-properties.json", 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()) + } +} diff --git a/pkg/api/vlabs/validate.go b/pkg/api/vlabs/validate.go index 2dbf1c26da..943a0790c0 100644 --- a/pkg/api/vlabs/validate.go +++ b/pkg/api/vlabs/validate.go @@ -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)) } diff --git a/pkg/api/vlabs/validate_test.go b/pkg/api/vlabs/validate_test.go index 808856dc45..8c8f9fdb6d 100644 --- a/pkg/api/vlabs/validate_test.go +++ b/pkg/api/vlabs/validate_test.go @@ -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)