Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OCPCLOUD-1423] Machine webhook validation for Azure Ultra Disks #1001

Merged
merged 3 commits into from
Mar 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ require (
github.com/google/uuid v1.1.2
github.com/onsi/ginkgo v1.16.5
github.com/onsi/gomega v1.17.0
github.com/openshift/api v0.0.0-20220304163151-654ca07c2567
github.com/openshift/api v0.0.0-20220322000322-9c4998a4d646
github.com/openshift/client-go v0.0.0-20211209144617-7385dd6338e3
github.com/openshift/library-go v0.0.0-20220121154930-b7889002d63e
github.com/operator-framework/operator-sdk v0.5.1-0.20190301204940-c2efe6f74e7b
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -535,8 +535,8 @@ github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQ
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
github.com/openshift/api v0.0.0-20211209135129-c58d9f695577/go.mod h1:DoslCwtqUpr3d/gsbq4ZlkaMEdYqKxuypsDjorcHhME=
github.com/openshift/api v0.0.0-20220304163151-654ca07c2567 h1:T2CuavyWrsL8zEMuqFQFtMoNCDYsOqCzcoM1ziaNA0I=
github.com/openshift/api v0.0.0-20220304163151-654ca07c2567/go.mod h1:F/eU6jgr6Q2VhMu1mSpMmygxAELd7+BUxs3NHZ25jV4=
github.com/openshift/api v0.0.0-20220322000322-9c4998a4d646 h1:V68+yLIF5FGKRSnurrqr56KLpcuQsv5RYxwNF0XI2Jw=
github.com/openshift/api v0.0.0-20220322000322-9c4998a4d646/go.mod h1:F/eU6jgr6Q2VhMu1mSpMmygxAELd7+BUxs3NHZ25jV4=
github.com/openshift/build-machinery-go v0.0.0-20210712174854-1bb7fd1518d3/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
github.com/openshift/build-machinery-go v0.0.0-20211213093930-7e33a7eb4ce3/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
github.com/openshift/client-go v0.0.0-20211209144617-7385dd6338e3 h1:SG1aqwleU6bGD0X4mhkTNupjVnByMYYuW4XbnCPavQU=
Expand Down
65 changes: 65 additions & 0 deletions pkg/webhooks/machine_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"net/http"
"regexp"
"runtime"
"strings"

Expand Down Expand Up @@ -896,6 +897,16 @@ func validateAzure(m *machinev1.Machine, config *admissionConfig) (bool, []strin
errs = append(errs, field.Invalid(field.NewPath("providerSpec", "osDisk", "cachingType"), providerSpec.OSDisk.CachingType, "Instances using an ephemeral OS disk support only Readonly caching"))
}

switch providerSpec.UltraSSDCapability {
case machinev1.AzureUltraSSDCapabilityEnabled, machinev1.AzureUltraSSDCapabilityDisabled, "":
// Valid scenarios, do nothing
default:
errs = append(errs, field.Invalid(field.NewPath("providerSpec", "ultraSSDCapability"), providerSpec.UltraSSDCapability,
fmt.Sprintf("ultraSSDCapability can be only %s, %s or omitted", machinev1.AzureUltraSSDCapabilityEnabled, machinev1.AzureUltraSSDCapabilityDisabled)))
}

errs = append(errs, validateAzureDataDisks(m.Name, providerSpec, field.NewPath("providerSpec", "dataDisks"))...)

if isAzureGovCloud(config.platformStatus) && providerSpec.SpotVMOptions != nil {
warnings = append(warnings, "spot VMs may not be supported when using GovCloud region")
}
Expand Down Expand Up @@ -1332,6 +1343,60 @@ func validateMachineLifecycleHooks(m, oldM *machinev1.Machine) []error {
return errs
}

func validateAzureDataDisks(machineName string, spec *machinev1.AzureMachineProviderSpec, parentPath *field.Path) []error {

var errs []error
dataDiskLuns := make(map[int32]struct{})
dataDiskNames := make(map[string]struct{})
damdo marked this conversation as resolved.
Show resolved Hide resolved
// defines rules for matching. strings must start and finish with an alphanumeric character
// and can only contain letters, numbers, underscores, periods or hyphens.
reg := regexp.MustCompile(`^[a-zA-Z0-9](?:[\w\.-]*[a-zA-Z0-9])?$`)
damdo marked this conversation as resolved.
Show resolved Hide resolved

for i, disk := range spec.DataDisks {
fldPath := parentPath.Index(i)

dataDiskName := machineName + "_" + disk.NameSuffix

if len(dataDiskName) > 80 {
errs = append(errs, field.Invalid(fldPath.Child("nameSuffix"), disk.NameSuffix, "too long, the overall disk name must not exceed 80 chars"))
}

if matched := reg.MatchString(disk.NameSuffix); !matched {
errs = append(errs, field.Invalid(fldPath.Child("nameSuffix"), disk.NameSuffix, "nameSuffix must be provided, must start and finish with an alphanumeric character and can only contain letters, numbers, underscores, periods or hyphens"))
}

if _, exists := dataDiskNames[disk.NameSuffix]; exists {
errs = append(errs, field.Invalid(fldPath.Child("nameSuffix"), disk.NameSuffix, "each Data Disk must have a unique nameSuffix"))
}

if disk.DiskSizeGB < 4 {
errs = append(errs, field.Invalid(fldPath.Child("diskSizeGB"), disk.DiskSizeGB, "diskSizeGB must be provided and at least 4GB in size"))
}

if disk.Lun < 0 || disk.Lun > 63 {
errs = append(errs, field.Invalid(fldPath.Child("lun"), disk.Lun, "must be greater than or equal to 0 and less than 64"))
}

if _, exists := dataDiskLuns[disk.Lun]; exists {
errs = append(errs, field.Invalid(fldPath.Child("lun"), disk.Lun, "each Data Disk must have a unique lun"))
}

if (disk.ManagedDisk.StorageAccountType == machinev1.StorageAccountUltraSSDLRS) &&
(disk.CachingType != machinev1.CachingTypeNone && disk.CachingType != "") {
errs = append(errs,
field.Invalid(fldPath.Child("cachingType"),
disk.CachingType,
fmt.Sprintf("must be \"None\" or omitted when storageAccountType is \"%s\"", machinev1.StorageAccountUltraSSDLRS)),
)
}

dataDiskLuns[disk.Lun] = struct{}{}
dataDiskNames[disk.NameSuffix] = struct{}{}
}

return errs
}

func isDeleting(obj metav1.Object) bool {
return obj.GetDeletionTimestamp() != nil
}
Loading