From ee3e2b8b7fdca06f796771f4402d878312f08558 Mon Sep 17 00:00:00 2001
From: tariq1890 <tariq181290@gmail.com>
Date: Fri, 8 Jun 2018 23:39:23 -0700
Subject: [PATCH] Fixing panic issue in Validate when properties are nil

---
 pkg/api/apiloader.go           | 17 +++++++++++++++++
 pkg/api/apiloader_test.go      | 29 +++++++++++++++++++++++++++++
 pkg/api/vlabs/validate.go      |  3 +++
 pkg/api/vlabs/validate_test.go |  9 +++++++++
 4 files changed, 58 insertions(+)

diff --git a/pkg/api/apiloader.go b/pkg/api/apiloader.go
index f23acd9920..f86f03e261 100644
--- a/pkg/api/apiloader.go
+++ b/pkg/api/apiloader.go
@@ -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"
@@ -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
 		}
@@ -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
 		}
@@ -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
 		}
@@ -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
 		}
@@ -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
 		}
diff --git a/pkg/api/apiloader_test.go b/pkg/api/apiloader_test.go
index 2c7c45f6de..c15e078950 100644
--- a/pkg/api/apiloader_test.go
+++ b/pkg/api/apiloader_test.go
@@ -5,6 +5,8 @@ import (
 	"github.com/Azure/acs-engine/pkg/i18n"
 	"github.com/leonelquinteros/gotext"
 
+	"io/ioutil"
+	"os"
 	"path"
 	"testing"
 )
@@ -85,3 +87,30 @@ 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) {
+	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())
+	}
+}
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)