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
Browse files Browse the repository at this point in the history
  • Loading branch information
tariq1890 committed Jun 11, 2018
1 parent 132003a commit 1f8044c
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 0 deletions.
6 changes: 6 additions & 0 deletions pkg/acsengine/testdata/v20170701/cs-without-properties.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "Microsoft.ContainerService/managedClusters",
"name": "[parameters('clusterName')]",
"apiVersion": "2017-07-01",
"location": "[resourceGroup().location]"
}
16 changes: 16 additions & 0 deletions pkg/api/apiloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
14 changes: 14 additions & 0 deletions pkg/api/apiloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
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 1f8044c

Please sign in to comment.