Skip to content

Commit 9fc12dc

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#658 from srm09/feature/add-validation-for-osdisk-spec
Adds validation for OS Disk spec
2 parents 45f49d9 + 4a0b011 commit 9fc12dc

File tree

5 files changed

+153
-18
lines changed

5 files changed

+153
-18
lines changed

‎api/v1alpha3/azuremachine_default_test.go‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,26 +43,22 @@ func TestAzureMachine_SetDefaultSSHPublicKey(t *testing.T) {
4343
}
4444

4545
func createMachineWithSSHPublicKey(t *testing.T, sshPublicKey string) *AzureMachine {
46-
return &AzureMachine{
47-
Spec: AzureMachineSpec{
48-
SSHPublicKey: sshPublicKey,
49-
Image: &Image{
50-
SharedGallery: &AzureSharedGalleryImage{
51-
SubscriptionID: "SUB123",
52-
ResourceGroup: "RG123",
53-
Name: "NAME123",
54-
Gallery: "GALLERY1",
55-
Version: "1.0.0",
56-
},
57-
},
58-
},
59-
}
46+
machine := hardcodedAzureMachineWithSSHKey(sshPublicKey)
47+
return machine
6048
}
6149

6250
func createMachineWithUserAssignedIdentities(t *testing.T, identitiesList []UserAssignedIdentity) *AzureMachine {
51+
machine := hardcodedAzureMachineWithSSHKey(generateSSHPublicKey())
52+
machine.Spec.Identity = VMIdentityUserAssigned
53+
machine.Spec.UserAssignedIdentities = identitiesList
54+
return machine
55+
}
56+
57+
func hardcodedAzureMachineWithSSHKey(sshPublicKey string) *AzureMachine {
6358
return &AzureMachine{
6459
Spec: AzureMachineSpec{
65-
SSHPublicKey: generateSSHPublicKey(),
60+
SSHPublicKey: sshPublicKey,
61+
OSDisk: generateValidOSDisk(),
6662
Image: &Image{
6763
SharedGallery: &AzureSharedGalleryImage{
6864
SubscriptionID: "SUB123",
@@ -72,8 +68,6 @@ func createMachineWithUserAssignedIdentities(t *testing.T, identitiesList []User
7268
Version: "1.0.0",
7369
},
7470
},
75-
Identity: VMIdentityUserAssigned,
76-
UserAssignedIdentities: identitiesList,
7771
},
7872
}
7973
}

‎api/v1alpha3/azuremachine_validation.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ package v1alpha3
1818

1919
import (
2020
"encoding/base64"
21+
"fmt"
2122

23+
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-12-01/compute"
2224
"golang.org/x/crypto/ssh"
2325
"k8s.io/apimachinery/pkg/util/validation/field"
2426
)
@@ -51,3 +53,38 @@ func ValidateUserAssignedIdentity(identityType VMIdentity, userAssignedIdentetie
5153

5254
return allErrs
5355
}
56+
57+
// ValidateOSDisk validates the OSDisk spec
58+
func ValidateOSDisk(osDisk OSDisk, fieldPath *field.Path) field.ErrorList {
59+
allErrs := field.ErrorList{}
60+
61+
if osDisk.DiskSizeGB <= 0 || osDisk.DiskSizeGB > 2048 {
62+
allErrs = append(allErrs, field.Invalid(fieldPath.Child("DiskSizeGB"), "", "the Disk size should be a value between 1 and 2048"))
63+
}
64+
65+
if osDisk.OSType == "" {
66+
allErrs = append(allErrs, field.Required(fieldPath.Child("OSType"), "the OS type cannot be empty"))
67+
}
68+
69+
allErrs = append(allErrs, validateStorageAccountType(osDisk.ManagedDisk.StorageAccountType, fieldPath)...)
70+
71+
return allErrs
72+
}
73+
74+
func validateStorageAccountType(storageAccountType string, fieldPath *field.Path) field.ErrorList {
75+
allErrs := field.ErrorList{}
76+
storageAccTypeChildPath := fieldPath.Child("ManagedDisk").Child("StorageAccountType")
77+
78+
if storageAccountType == "" {
79+
allErrs = append(allErrs, field.Required(storageAccTypeChildPath, "the Storage Account Type for Managed Disk cannot be empty"))
80+
return allErrs
81+
}
82+
83+
for _, possibleStorageAccountType := range compute.PossibleDiskStorageAccountTypesValues() {
84+
if string(possibleStorageAccountType) == storageAccountType {
85+
return allErrs
86+
}
87+
}
88+
allErrs = append(allErrs, field.Invalid(storageAccTypeChildPath, "", fmt.Sprintf("allowed values are %v", compute.PossibleDiskStorageAccountTypesValues())))
89+
return allErrs
90+
}

‎api/v1alpha3/azuremachine_validation_test.go‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,97 @@ func generateSSHPublicKey() string {
6464
publicRsaKey, _ := ssh.NewPublicKey(&privateKey.PublicKey)
6565
return base64.StdEncoding.EncodeToString(ssh.MarshalAuthorizedKey(publicRsaKey))
6666
}
67+
68+
type osDiskTestInput struct {
69+
name string
70+
wantErr bool
71+
osDisk OSDisk
72+
}
73+
74+
func TestAzureMachine_ValidateOSDisk(t *testing.T) {
75+
g := NewWithT(t)
76+
77+
testcases := []osDiskTestInput{
78+
{
79+
name: "valid os disk spec",
80+
wantErr: false,
81+
osDisk: generateValidOSDisk(),
82+
},
83+
}
84+
testcases = append(testcases, generateNegativeTestCases()...)
85+
86+
for _, test := range testcases {
87+
t.Run(test.name, func(t *testing.T) {
88+
err := ValidateOSDisk(test.osDisk, field.NewPath("osDisk"))
89+
if test.wantErr {
90+
g.Expect(err).NotTo(HaveLen(0))
91+
} else {
92+
g.Expect(err).To(HaveLen(0))
93+
}
94+
})
95+
}
96+
}
97+
98+
func generateNegativeTestCases() []osDiskTestInput {
99+
inputs := []osDiskTestInput{}
100+
testCaseName := "invalid os disk spec"
101+
102+
invalidDiskSpecs := []OSDisk{
103+
{},
104+
{
105+
DiskSizeGB: 0,
106+
OSType: "blah",
107+
},
108+
{
109+
DiskSizeGB: -10,
110+
OSType: "blah",
111+
},
112+
{
113+
DiskSizeGB: 2050,
114+
OSType: "blah",
115+
},
116+
{
117+
DiskSizeGB: 20,
118+
OSType: "",
119+
},
120+
{
121+
DiskSizeGB: 30,
122+
OSType: "blah",
123+
ManagedDisk: ManagedDisk{},
124+
},
125+
{
126+
DiskSizeGB: 30,
127+
OSType: "blah",
128+
ManagedDisk: ManagedDisk{
129+
StorageAccountType: "",
130+
},
131+
},
132+
{
133+
DiskSizeGB: 30,
134+
OSType: "blah",
135+
ManagedDisk: ManagedDisk{
136+
StorageAccountType: "invalid_type",
137+
},
138+
},
139+
}
140+
141+
for _, input := range invalidDiskSpecs {
142+
inputs = append(inputs, osDiskTestInput{
143+
name: testCaseName,
144+
wantErr: true,
145+
osDisk: input,
146+
})
147+
}
148+
149+
return inputs
150+
}
151+
152+
func generateValidOSDisk() OSDisk {
153+
return OSDisk{
154+
DiskSizeGB: 30,
155+
OSType: "Linux",
156+
ManagedDisk: ManagedDisk{
157+
StorageAccountType: "Premium_LRS",
158+
},
159+
}
160+
}

‎api/v1alpha3/azuremachine_webhook.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ func (m *AzureMachine) ValidateCreate() error {
4848
allErrs = append(allErrs, errs...)
4949
}
5050

51+
if errs := ValidateOSDisk(m.Spec.OSDisk, field.NewPath("osDisk")); len(errs) > 0 {
52+
allErrs = append(allErrs, errs...)
53+
}
54+
5155
if errs := ValidateSSHKey(m.Spec.SSHPublicKey, field.NewPath("sshPublicKey")); len(errs) > 0 {
5256
allErrs = append(allErrs, errs...)
5357
}

‎api/v1alpha3/azuremachine_webhook_test.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ import (
2222
. "github.com/onsi/gomega"
2323
)
2424

25-
var validSSHPublicKey = generateSSHPublicKey()
25+
var (
26+
validSSHPublicKey = generateSSHPublicKey()
27+
validOSDisk = generateValidOSDisk()
28+
)
2629

2730
func TestAzureMachine_ValidateCreate(t *testing.T) {
2831
g := NewWithT(t)
@@ -185,6 +188,7 @@ func createMachineWithSharedImage(t *testing.T, subscriptionID, resourceGroup, n
185188
Spec: AzureMachineSpec{
186189
Image: image,
187190
SSHPublicKey: validSSHPublicKey,
191+
OSDisk: validOSDisk,
188192
},
189193
}
190194

@@ -204,6 +208,7 @@ func createMachineWithtMarketPlaceImage(t *testing.T, publisher, offer, sku, ver
204208
Spec: AzureMachineSpec{
205209
Image: image,
206210
SSHPublicKey: validSSHPublicKey,
211+
OSDisk: validOSDisk,
207212
},
208213
}
209214
}
@@ -217,6 +222,7 @@ func createMachineWithImageByID(t *testing.T, imageID string) *AzureMachine {
217222
Spec: AzureMachineSpec{
218223
Image: image,
219224
SSHPublicKey: validSSHPublicKey,
225+
OSDisk: validOSDisk,
220226
},
221227
}
222228
}

0 commit comments

Comments
 (0)