diff --git a/pkg/asset/installconfig/gcp/client.go b/pkg/asset/installconfig/gcp/client.go index 46e8c791142..a3290c3cd7c 100644 --- a/pkg/asset/installconfig/gcp/client.go +++ b/pkg/asset/installconfig/gcp/client.go @@ -3,6 +3,7 @@ package gcp import ( "context" "fmt" + "net/http" "strings" "time" @@ -257,7 +258,10 @@ func (c *Client) GetDNSZone(ctx context.Context, project, baseDomain string, isP } if res == nil { if isPublic { - return nil, errors.New("no matching public DNS Zone found") + return nil, &googleapi.Error{ + Code: http.StatusNotFound, + Message: "no matching public DNS Zone found", + } } // A Private DNS Zone may be created (if the correct permissions exist) return nil, nil diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index 7aa0af4f777..3f55796750d 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/pkg/asset/installconfig/gcp/validation.go @@ -68,6 +68,7 @@ func Validate(client API, ic *types.InstallConfig) error { allErrs = append(allErrs, validateInstanceTypes(client, ic)...) allErrs = append(allErrs, ValidateCredentialMode(client, ic)...) allErrs = append(allErrs, validatePreexistingServiceAccount(client, ic)...) + allErrs = append(allErrs, ValidatePreExistingPublicDNS(client, ic)...) allErrs = append(allErrs, validateServiceAccountPresent(client, ic)...) allErrs = append(allErrs, validateMarketplaceImages(client, ic)...) allErrs = append(allErrs, validatePlatformKMSKeys(client, ic, field.NewPath("platform").Child("gcp"))...) @@ -390,20 +391,25 @@ func validatePreexistingServiceAccount(client API, ic *types.InstallConfig) fiel // DNS zone for cluster's Kubernetes API. If a PublicDNSZone is provided, the provided // zone is verified against the BaseDomain. If no zone is provided, the base domain is // checked for any public zone that can be used. -func ValidatePreExistingPublicDNS(client API, ic *types.InstallConfig) *field.Error { +func ValidatePreExistingPublicDNS(client API, ic *types.InstallConfig) field.ErrorList { // If this is an internal cluster, this check is not necessary if ic.Publish == types.InternalPublishingStrategy { return nil } + allErrs := field.ErrorList{} zone, err := client.GetDNSZone(context.TODO(), ic.Platform.GCP.ProjectID, ic.BaseDomain, true) if err != nil { if IsNotFound(err) { - return field.NotFound(field.NewPath("baseDomain"), fmt.Sprintf("Public DNS Zone (%s/%s)", ic.Platform.GCP.ProjectID, ic.BaseDomain)) + return append(allErrs, field.NotFound(field.NewPath("baseDomain"), fmt.Sprintf("Public DNS Zone (%s/%s)", ic.Platform.GCP.ProjectID, ic.BaseDomain))) } - return field.InternalError(field.NewPath("baseDomain"), err) + return append(allErrs, field.InternalError(field.NewPath("baseDomain"), err)) + } + + if err := checkRecordSets(client, ic, zone, []string{apiRecordType(ic)}); err != nil { + allErrs = append(allErrs, err) } - return checkRecordSets(client, ic, zone, []string{apiRecordType(ic)}) + return allErrs } // ValidatePrivateDNSZone ensure no pre-existing DNS record exists in the private dns zone @@ -461,10 +467,6 @@ func ValidateForProvisioning(ic *types.InstallConfig) error { return err } - if err := ValidatePreExistingPublicDNS(client, ic); err != nil { - allErrs = append(allErrs, err) - } - if err := ValidatePrivateDNSZone(client, ic); err != nil { allErrs = append(allErrs, err) } diff --git a/pkg/asset/installconfig/gcp/validation_test.go b/pkg/asset/installconfig/gcp/validation_test.go index 93d8c3d9cca..73667700ac0 100644 --- a/pkg/asset/installconfig/gcp/validation_test.go +++ b/pkg/asset/installconfig/gcp/validation_test.go @@ -46,6 +46,7 @@ var ( validPublicZone = "valid-short-public-zone" invalidPublicZone = "invalid-short-public-zone" validBaseDomain = "example.installer.domain." + invalidBaseDomain = "invalid.installer.domain." validXpnSA = "valid-example-sa@gcloud.serviceaccount.com" invalidXpnSA = "invalid-example-sa@gcloud.serviceaccount.com" validServiceEndpointURL = "https://computeexample.googleapis.com/compute/v1/" @@ -114,6 +115,7 @@ var ( validNetworkProject = func(ic *types.InstallConfig) { ic.GCP.NetworkProjectID = validProjectName } validateXpnSA = func(ic *types.InstallConfig) { ic.ControlPlane.Platform.GCP.ServiceAccount = validXpnSA } invalidateXpnSA = func(ic *types.InstallConfig) { ic.ControlPlane.Platform.GCP.ServiceAccount = invalidXpnSA } + invalidateBaseDomain = func(ic *types.InstallConfig) { ic.BaseDomain = invalidBaseDomain } validServiceEndpoint = func(ic *types.InstallConfig) { ic.GCP.ServiceEndpoints = append(ic.GCP.ServiceEndpoints, @@ -257,191 +259,230 @@ func TestGCPInstallConfigValidation(t *testing.T) { cases := []struct { name string edits editFunctions + records []*dns.ResourceRecordSet expectedError bool expectedErrMsg string }{ { name: "Valid network & subnets", edits: editFunctions{}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: false, expectedErrMsg: "", }, { name: "Invalid ClusterName", edits: editFunctions{invalidClusterName}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: `clusterName: Invalid value: "testgoogletest": cluster name must not start with "goog" or contain variations of "google"`, }, { name: "Valid install config without network & subnets", edits: editFunctions{removeVPC, removeSubnets}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: false, expectedErrMsg: "", }, { name: "Invalid subnet range", edits: editFunctions{invalidateMachineCIDR}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "computeSubnet: Invalid value.*subnet CIDR range start 10.0.0.0 is outside of the specified machine networks", }, { name: "Invalid network", edits: editFunctions{invalidateNetwork}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "network: Invalid value", }, { name: "Invalid compute subnet", edits: editFunctions{invalidateComputeSubnet}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "computeSubnet: Invalid value", }, { name: "Invalid control plane subnet", edits: editFunctions{invalidateCPSubnet}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "controlPlaneSubnet: Invalid value", }, { name: "Invalid both subnets", edits: editFunctions{invalidateCPSubnet, invalidateComputeSubnet}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "computeSubnet: Invalid value.*controlPlaneSubnet: Invalid value", }, { name: "Valid machine types", edits: editFunctions{validMachineTypes}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: false, expectedErrMsg: "", }, { name: "Invalid default machine type", edits: editFunctions{invalidateDefaultMachineTypes}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: `\[platform.gcp.defaultMachinePlatform.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 4 vCPUs, platform.gcp.defaultMachinePlatform.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 15360 MB Memory, controlPlane.platform.gcp.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 4 vCPUs, controlPlane.platform.gcp.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 15360 MB Memory, compute\[0\].platform.gcp.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 2 vCPUs, compute\[0\].platform.gcp.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 7680 MB Memory\]`, }, { name: "Invalid control plane machine disk types", edits: editFunctions{validMachineTypes, invalidateControlPlaneDiskTypes}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: `controlPlane.type: Unsupported value: "pd-standard": supported values: "hyperdisk-balanced", "pd-balanced", "pd-ssd"`, }, { name: "Invalid control plane machine types", edits: editFunctions{invalidateControlPlaneMachineTypes}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: `[controlPlane.platform.gcp.type: Invalid value: "n1\-standard\-1": instance type does not meet minimum resource requirements of 4 vCPUs, controlPlane.platform.gcp.type: Invalid value: "n1\-standard\-1": instance type does not meet minimum resource requirements of 15361 MB Memory]`, }, { name: "Invalid compute machine types", edits: editFunctions{invalidateComputeMachineTypes}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: `\[compute\[0\].platform.gcp.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 2 vCPUs, compute\[0\].platform.gcp.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 7680 MB Memory\]`, }, { name: "Undefined default machine types", edits: editFunctions{undefinedDefaultMachineTypes}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: `Internal error: 404`, }, { name: "Invalid region", edits: editFunctions{invalidateRegion}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "could not find subnet valid-compute-subnet in network valid-vpc and region us-east4", }, { name: "Invalid project", edits: editFunctions{invalidateProject}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "network: Invalid value", }, { name: "Invalid project & region", edits: editFunctions{invalidateRegion, invalidateProject}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "network: Invalid value", }, { name: "Invalid project ID", edits: editFunctions{invalidateProject, removeSubnets, removeVPC}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "platform.gcp.project: Invalid value: \"invalid-project\": invalid project ID", }, { name: "Invalid network project ID", edits: editFunctions{invalidateNetworkProject}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "platform.gcp.networkProjectID: Invalid value: \"invalid-project\": invalid project ID", }, { name: "Valid Region", edits: editFunctions{}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: false, expectedErrMsg: "", }, { name: "Invalid region not found", edits: editFunctions{invalidateRegion, invalidateProject}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "platform.gcp.project: Invalid value: \"invalid-project\": invalid project ID", }, { name: "Region not validated", edits: editFunctions{invalidateRegion}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "platform.gcp.region: Invalid value: \"us-east4\": invalid region", }, { name: "Valid XPN Service Account", edits: editFunctions{validNetworkProject, validateXpnSA}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: false, }, { name: "Invalid XPN Service Account", edits: editFunctions{validNetworkProject, invalidateXpnSA}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "controlPlane.platform.gcp.serviceAccount: Internal error\"", }, { name: "Valid Control Plane KMS Key", edits: editFunctions{validCPKMSKeyRing}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: false, }, { name: "Invalid Control Plane KMS Key", edits: editFunctions{invalidateCPKMSKeyRing}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "platform.gcp.controlPlane.encryptionKey.kmsKey.keyRing: Invalid value: \"invalidKeyRingName\": failed to find key ring invalidKeyRingName: data", }, { name: "Valid Compute KMS Key", edits: editFunctions{validComputeKMSKeyRing}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: false, }, { name: "Invalid Compute KMS Key", edits: editFunctions{invalidateComputeKMSKeyRing}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "platform.gcp.compute.encryptionKey.kmsKey.keyRing: Invalid value: \"invalidKeyRingName\": failed to find key ring invalidKeyRingName: data", }, { name: "Valid Control Plane Invalid Compute Invalid Default Machine KMS Key", edits: editFunctions{validCPKMSKeyRing, invalidateComputeKMSKeyRing, invalidDefaultMachineKeyRing}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: "platform.gcp.compute.encryptionKey.kmsKey.keyRing: Invalid value: \"invalidKeyRingName\": failed to find key ring invalidKeyRingName: data, platform.gcp.defaultMachinePool.encryptionKey.kmsKey.keyRing: Invalid value: \"invalidKeyRingName\": failed to find key ring invalidKeyRingName: data", }, { name: "Valid Service Endpoint Override", edits: editFunctions{validServiceEndpoint}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: false, }, { name: "Invalid Service Endpoint Override Bad Format", edits: editFunctions{invalidServiceEndpointBadFormat}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.example.installer.domain."}}, expectedError: true, expectedErrMsg: `[platform.gcp.serviceEndpoint\[0\]: Invalid value: \"http://badstorage.googleapis\": Head \"http://badstorage.googleapis\": dial tcp: lookup badstorage.googleapis: no such host]`, }, + { + name: "Invalid Base Domain", + edits: editFunctions{invalidateBaseDomain}, + records: []*dns.ResourceRecordSet{}, + expectedError: true, + expectedErrMsg: `baseDomain: Not found: "invalid.installer.domain."`, + }, } mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() @@ -504,6 +545,10 @@ func TestGCPInstallConfigValidation(t *testing.T) { gcpClient.EXPECT().GetKeyRing(gomock.Any(), &validKeyRing).Return(validKeyRingRet, nil).AnyTimes() gcpClient.EXPECT().GetKeyRing(gomock.Any(), &invalidKeyRing).Return(nil, fmt.Errorf("failed to find key ring invalidKeyRingName: data")).AnyTimes() + gcpClient.EXPECT().GetDNSZone(gomock.Any(), validProjectName, validBaseDomain, true).Return(&dns.ManagedZone{Name: validZone}, nil).AnyTimes() + gcpClient.EXPECT().GetDNSZone(gomock.Any(), invalidProjectName, validBaseDomain, true).Return(&dns.ManagedZone{Name: validZone}, nil).AnyTimes() + gcpClient.EXPECT().GetDNSZone(gomock.Any(), validProjectName, invalidBaseDomain, true).Return(nil, fmt.Errorf("baseDomain: Not found: \"%s\"", invalidBaseDomain)).AnyTimes() + httpmock.Activate() defer httpmock.DeactivateAndReset() @@ -533,6 +578,8 @@ func TestGCPInstallConfigValidation(t *testing.T) { edit(editedInstallConfig) } + gcpClient.EXPECT().GetRecordSets(gomock.Any(), gomock.Eq(editedInstallConfig.GCP.ProjectID), gomock.Eq(validZone)).Return(tc.records, nil).AnyTimes() + errs := Validate(gcpClient, editedInstallConfig) if tc.expectedError { assert.Regexp(t, tc.expectedErrMsg, errs) @@ -577,7 +624,7 @@ func TestValidatePreExistingPublicDNS(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "cluster-name"}, BaseDomain: "base-domain", Platform: types.Platform{GCP: &gcp.Platform{ProjectID: "project-id"}}, - }) + }).ToAggregate() if test.err == "" { assert.True(t, err == nil) } else {