Skip to content

Commit fc81e15

Browse files
committed
OCPBUGS-54566: Apply defaults then validate confidentialCompute
When confidentialCompute is not set to Disabled (or an empty value), the value of onHostMaintenance has to be "Terminate". Also, a supported machine instance type hast to be set. This validation occurred previously in ValidateMachinePool in pkg/types/gcp/validation/machinepool.go. This validation is applied separately onto defaultMachinePlatform, compute and controlPlane configurations in the install-config file. This makes it appropriate to validate single values of some of the configuration parameters. However, it's not the best choice when it comes to validating parameter combinations, as in the case of confidentialCompute. This is due to compute and controlPlane configs "inheriting" the defaults set in defaultMachinePlatform. This lead to the validation raising some false errors on properly configured install-config files. To avoid the issue, this patch moves the validation from the previous location into pkg/asset/installconfig/gcp/validation.go, where validations take the default value inheritance into account. It also removes the unit tests from pkg/types/gcp/validation/machinepool_test.go and implements them into pkg/asset/installconfig/gcp/validation_test.go.
1 parent fc674e0 commit fc81e15

File tree

4 files changed

+350
-238
lines changed

4 files changed

+350
-238
lines changed

pkg/asset/installconfig/gcp/validation.go

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"net"
88
"net/http"
99
"net/url"
10+
"slices"
1011
"strings"
1112

1213
"github.com/pkg/errors"
@@ -110,8 +111,33 @@ func validateInstanceAndDiskType(fldPath *field.Path, diskType, instanceType, ar
110111
return nil
111112
}
112113

114+
func validateInstanceAndConfidentialCompute(fldPath *field.Path, instanceType string, onHostMaintenance gcp.OnHostMaintenanceType, confidentialCompute gcp.ConfidentialComputePolicy) field.ErrorList {
115+
allErrs := field.ErrorList{}
116+
if confidentialCompute == gcp.ConfidentialComputePolicy(gcp.DisabledFeature) {
117+
// Nothing to validate here
118+
return allErrs
119+
}
120+
121+
if onHostMaintenance != gcp.OnHostMaintenanceTerminate {
122+
allErrs = append(allErrs, field.Invalid(fldPath.Child("onHostMaintenance"), onHostMaintenance, fmt.Sprintf("onHostMaintenace must be set to Terminate when confidentialCompute is %s", confidentialCompute)))
123+
}
124+
125+
machineType, _, _ := strings.Cut(instanceType, "-")
126+
if confidentialCompute == gcp.ConfidentialComputePolicy(gcp.EnabledFeature) {
127+
confidentialCompute = gcp.ConfidentialComputePolicySEV
128+
}
129+
supportedMachineTypes, ok := gcp.ConfidentialComputePolicyToSupportedInstanceType[confidentialCompute]
130+
if !ok {
131+
allErrs = append(allErrs, field.Invalid(fldPath.Child("confidentialCompute"), confidentialCompute, fmt.Sprintf("Unknown confidential computing technology %s", confidentialCompute)))
132+
} else if !slices.Contains(supportedMachineTypes, machineType) {
133+
allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), instanceType, fmt.Sprintf("Machine type do not support %s. Machine types supporting %s: %s", confidentialCompute, confidentialCompute, strings.Join(supportedMachineTypes, ", "))))
134+
}
135+
136+
return allErrs
137+
}
138+
113139
// ValidateInstanceType ensures the instance type has sufficient Vcpu and Memory.
114-
func ValidateInstanceType(client API, fieldPath *field.Path, project, region string, zones []string, diskType string, instanceType string, req resourceRequirements, arch string) field.ErrorList {
140+
func ValidateInstanceType(client API, fieldPath *field.Path, project, region string, zones []string, diskType string, instanceType string, req resourceRequirements, arch string, onHostMaintenance string, confidentialCompute string) field.ErrorList {
115141
allErrs := field.ErrorList{}
116142

117143
typeMeta, typeZones, err := client.GetMachineTypeWithZones(context.TODO(), project, region, instanceType)
@@ -126,6 +152,14 @@ func ValidateInstanceType(client API, fieldPath *field.Path, project, region str
126152
return append(allErrs, fieldErr)
127153
}
128154

155+
allErrs = append(allErrs,
156+
validateInstanceAndConfidentialCompute(
157+
fieldPath,
158+
instanceType,
159+
gcp.OnHostMaintenanceType(onHostMaintenance),
160+
gcp.ConfidentialComputePolicy(confidentialCompute),
161+
)...)
162+
129163
userZones := sets.New(zones...)
130164
if len(userZones) == 0 {
131165
userZones = typeZones
@@ -183,6 +217,8 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
183217

184218
defaultInstanceType := ""
185219
defaultDiskType := gcp.PDSSD
220+
defaultOnHostMaintenance := string(gcp.OnHostMaintenanceMigrate)
221+
defaultConfidentialCompute := string(gcp.DisabledFeature)
186222
defaultZones := []string{}
187223

188224
// Default requirements need to be sufficient to support Control Plane instances.
@@ -199,6 +235,14 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
199235
defaultDiskType = ic.GCP.DefaultMachinePlatform.DiskType
200236
}
201237

238+
if ic.GCP.DefaultMachinePlatform.OnHostMaintenance != "" {
239+
defaultOnHostMaintenance = ic.GCP.DefaultMachinePlatform.OnHostMaintenance
240+
}
241+
242+
if ic.GCP.DefaultMachinePlatform.ConfidentialCompute != "" {
243+
defaultConfidentialCompute = ic.GCP.DefaultMachinePlatform.ConfidentialCompute
244+
}
245+
202246
if ic.GCP.DefaultMachinePlatform.InstanceType != "" {
203247
allErrs = append(allErrs,
204248
ValidateInstanceType(
@@ -211,6 +255,8 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
211255
ic.GCP.DefaultMachinePlatform.InstanceType,
212256
defaultInstanceReq,
213257
unknownArchitecture,
258+
defaultOnHostMaintenance,
259+
defaultConfidentialCompute,
214260
)...)
215261
}
216262
}
@@ -219,6 +265,8 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
219265
instanceType := defaultInstanceType
220266
arch := types.ArchitectureAMD64
221267
cpDiskType := defaultDiskType
268+
cpOnHostMaintenance := defaultOnHostMaintenance
269+
cpConfidentialCompute := defaultConfidentialCompute
222270
if ic.ControlPlane != nil {
223271
arch = string(ic.ControlPlane.Architecture)
224272
if instanceType == "" {
@@ -234,6 +282,12 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
234282
if ic.ControlPlane.Platform.GCP.DiskType != "" {
235283
cpDiskType = ic.ControlPlane.Platform.GCP.DiskType
236284
}
285+
if ic.ControlPlane.Platform.GCP.OnHostMaintenance != "" {
286+
cpOnHostMaintenance = ic.ControlPlane.Platform.GCP.OnHostMaintenance
287+
}
288+
if ic.ControlPlane.Platform.GCP.ConfidentialCompute != "" {
289+
cpConfidentialCompute = ic.ControlPlane.Platform.GCP.ConfidentialCompute
290+
}
237291
}
238292
}
239293

@@ -256,6 +310,8 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
256310
instanceType,
257311
controlPlaneReq,
258312
arch,
313+
cpOnHostMaintenance,
314+
cpConfidentialCompute,
259315
)...)
260316
}
261317

@@ -264,6 +320,8 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
264320
zones := defaultZones
265321
instanceType := defaultInstanceType
266322
diskType := defaultDiskType
323+
onHostMaintenance := defaultOnHostMaintenance
324+
confidentialCompute := defaultConfidentialCompute
267325
if instanceType == "" {
268326
instanceType = DefaultInstanceTypeForArch(compute.Architecture)
269327
}
@@ -278,6 +336,12 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
278336
if len(compute.Platform.GCP.Zones) > 0 {
279337
zones = compute.Platform.GCP.Zones
280338
}
339+
if compute.Platform.GCP.OnHostMaintenance != "" {
340+
onHostMaintenance = compute.Platform.GCP.OnHostMaintenance
341+
}
342+
if compute.Platform.GCP.ConfidentialCompute != "" {
343+
confidentialCompute = compute.Platform.GCP.ConfidentialCompute
344+
}
281345
}
282346

283347
if compute.Platform.GCP != nil && compute.Platform.GCP.DiskType != "" {
@@ -295,6 +359,8 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
295359
instanceType,
296360
computeReq,
297361
string(arch),
362+
onHostMaintenance,
363+
confidentialCompute,
298364
)...)
299365
}
300366

0 commit comments

Comments
 (0)