Skip to content

OCPBUGS-59430: Fail install config validation when base domain is invalid #9840

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.

Sign up for GitHub

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

Merged
openshift-merge-bot merged 1 commit into from
Jul 24, 2025
Merged

OCPBUGS-59430: Fail install config validation when base domain is invalid #9840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion pkg/asset/installconfig/gcp/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package gcp
import (
"context"
"fmt"
"net/http"
"strings"
"time"

Expand Down Expand Up @@ -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
Expand Down
18 changes: 10 additions & 8 deletions pkg/asset/installconfig/gcp/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"))...)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
49 changes: 48 additions & 1 deletion pkg/asset/installconfig/gcp/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ var (
validPublicZone = "valid-short-public-zone"
invalidPublicZone = "invalid-short-public-zone"
validBaseDomain = "example.installer.domain."
invalidBaseDomain = "invalid.installer.domain."
validXpnSA = "[email protected]"
invalidXpnSA = "[email protected]"
validServiceEndpointURL = "https://computeexample.googleapis.com/compute/v1/"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down