From 4a7b970011f50a89b6c5c61cfdbfb92b93e51fe1 Mon Sep 17 00:00:00 2001
From: "Steven E. Harris" <seh@panix.com>
Date: Fri, 22 Jan 2021 17:45:01 -0500
Subject: [PATCH] Validate CloudConfiguration values

---
 pkg/apis/kops/validation/validation.go      | 15 ++++
 pkg/apis/kops/validation/validation_test.go | 93 +++++++++++++++++++++
 2 files changed, 108 insertions(+)

diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go
index f1b464726a..6629dddcaf 100644
--- a/pkg/apis/kops/validation/validation.go
+++ b/pkg/apis/kops/validation/validation.go
@@ -242,6 +242,10 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie
 		}
 	}
 
+	if spec.CloudConfig != nil {
+		allErrs = append(allErrs, validateCloudConfiguration(spec.CloudConfig, fieldPath.Child("cloudConfig"))...)
+	}
+
 	return allErrs
 }
 
@@ -1302,3 +1306,14 @@ func validateAWSLoadBalancerController(cluster *kops.Cluster, spec *kops.AWSLoad
 	}
 	return allErrs
 }
+
+func validateCloudConfiguration(cloudConfig *kops.CloudConfiguration, fldPath *field.Path) (allErrs field.ErrorList) {
+	if cloudConfig.ManageStorageClasses != nil && cloudConfig.Openstack != nil &&
+		cloudConfig.Openstack.BlockStorage != nil && cloudConfig.Openstack.BlockStorage.CreateStorageClass != nil {
+		if *cloudConfig.Openstack.BlockStorage.CreateStorageClass != *cloudConfig.ManageStorageClasses {
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("manageStorageClasses"),
+				"Management of storage classes and OpenStack block storage classes are both specified but disagree"))
+		}
+	}
+	return allErrs
+}
diff --git a/pkg/apis/kops/validation/validation_test.go b/pkg/apis/kops/validation/validation_test.go
index 376473afe6..6644118d0e 100644
--- a/pkg/apis/kops/validation/validation_test.go
+++ b/pkg/apis/kops/validation/validation_test.go
@@ -1033,3 +1033,96 @@ func Test_Validate_NodeLocalDNS(t *testing.T) {
 		testErrors(t, g.Input, errs, g.ExpectedErrors)
 	}
 }
+
+func Test_Validate_CloudConfiguration(t *testing.T) {
+	grid := []struct {
+		Description    string
+		Input          kops.CloudConfiguration
+		ExpectedErrors []string
+	}{
+		{
+			Description: "neither",
+			Input:       kops.CloudConfiguration{},
+		},
+		{
+			Description: "all false",
+			Input: kops.CloudConfiguration{
+				ManageStorageClasses: fi.Bool(false),
+			},
+		},
+		{
+			Description: "all true",
+			Input: kops.CloudConfiguration{
+				ManageStorageClasses: fi.Bool(true),
+			},
+		},
+		{
+			Description: "os false",
+			Input: kops.CloudConfiguration{
+				Openstack: &kops.OpenstackConfiguration{
+					BlockStorage: &kops.OpenstackBlockStorageConfig{
+						CreateStorageClass: fi.Bool(false),
+					},
+				}},
+		},
+		{
+			Description: "os false",
+			Input: kops.CloudConfiguration{
+				Openstack: &kops.OpenstackConfiguration{
+					BlockStorage: &kops.OpenstackBlockStorageConfig{
+						CreateStorageClass: fi.Bool(true),
+					},
+				}},
+		},
+		{
+			Description: "all false, os false",
+			Input: kops.CloudConfiguration{
+				ManageStorageClasses: fi.Bool(false),
+				Openstack: &kops.OpenstackConfiguration{
+					BlockStorage: &kops.OpenstackBlockStorageConfig{
+						CreateStorageClass: fi.Bool(false),
+					},
+				}},
+		},
+		{
+			Description: "all false, os true",
+			Input: kops.CloudConfiguration{
+				ManageStorageClasses: fi.Bool(false),
+				Openstack: &kops.OpenstackConfiguration{
+					BlockStorage: &kops.OpenstackBlockStorageConfig{
+						CreateStorageClass: fi.Bool(true),
+					},
+				}},
+			ExpectedErrors: []string{"Forbidden::cloudConfig.manageStorageClasses"},
+		},
+		{
+			Description: "all true, os false",
+			Input: kops.CloudConfiguration{
+				ManageStorageClasses: fi.Bool(true),
+				Openstack: &kops.OpenstackConfiguration{
+					BlockStorage: &kops.OpenstackBlockStorageConfig{
+						CreateStorageClass: fi.Bool(false),
+					},
+				}},
+			ExpectedErrors: []string{"Forbidden::cloudConfig.manageStorageClasses"},
+		},
+		{
+			Description: "all true, os true",
+			Input: kops.CloudConfiguration{
+				ManageStorageClasses: fi.Bool(true),
+				Openstack: &kops.OpenstackConfiguration{
+					BlockStorage: &kops.OpenstackBlockStorageConfig{
+						CreateStorageClass: fi.Bool(true),
+					},
+				}},
+		},
+	}
+
+	for _, g := range grid {
+		fldPath := field.NewPath("cloudConfig")
+		t.Run(g.Description, func(t *testing.T) {
+			errs := validateCloudConfiguration(&g.Input, fldPath)
+			testErrors(t, g.Input, errs, g.ExpectedErrors)
+		})
+	}
+}