diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index 601e2478e3c..c0b4c409192 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -5805,6 +5805,28 @@ spec: type: string type: array type: object + dns: + description: |- + DNS contains the dns zone information for the cluster. The DNS information can + only be supplied during Shared VPC (XPN) installs. + properties: + privateZone: + description: |- + PrivateZone contains the information for a private DNS zone. The Private DNS Zone can + only be supplied during Shared VPC (XPN) installs. The PrivateZone can exist or be + created in a second service project; a project other than the one matching projectID + or networkProjectID. + properties: + name: + description: Name is the name of the dns-managed zone. + type: string + projectID: + description: ProjectID is the project where the zone resides. + type: string + required: + - name + type: object + type: object network: description: |- Network specifies an existing VPC where the cluster should be created diff --git a/pkg/asset/cluster/gcp/gcp.go b/pkg/asset/cluster/gcp/gcp.go index d850bb2ed56..452237bb0ce 100644 --- a/pkg/asset/cluster/gcp/gcp.go +++ b/pkg/asset/cluster/gcp/gcp.go @@ -12,15 +12,22 @@ import ( func Metadata(config *types.InstallConfig) *gcp.Metadata { // leave the private zone domain blank when not using a pre-created private zone privateZoneDomain := fmt.Sprintf("%s.", config.ClusterDomain()) + privateZoneProject := config.GCP.ProjectID if config.GCP.Network == "" || config.GCP.NetworkProjectID == "" { privateZoneDomain = "" } + if config.GCP.DNS != nil && config.GCP.DNS.PrivateZone != nil { + if config.GCP.DNS.PrivateZone.ProjectID != "" { + privateZoneProject = config.GCP.DNS.PrivateZone.ProjectID + } + } return &gcp.Metadata{ - Region: config.Platform.GCP.Region, - ProjectID: config.Platform.GCP.ProjectID, - NetworkProjectID: config.Platform.GCP.NetworkProjectID, - PrivateZoneDomain: privateZoneDomain, - ServiceEndpoints: config.Platform.GCP.ServiceEndpoints, + Region: config.Platform.GCP.Region, + ProjectID: config.Platform.GCP.ProjectID, + NetworkProjectID: config.Platform.GCP.NetworkProjectID, + PrivateZoneDomain: privateZoneDomain, + PrivateZoneProjectID: privateZoneProject, + ServiceEndpoints: config.Platform.GCP.ServiceEndpoints, } } diff --git a/pkg/asset/cluster/tfvars/tfvars.go b/pkg/asset/cluster/tfvars/tfvars.go index 35c6920f883..ee0f9898b32 100644 --- a/pkg/asset/cluster/tfvars/tfvars.go +++ b/pkg/asset/cluster/tfvars/tfvars.go @@ -517,7 +517,7 @@ func (t *TerraformVariables) Generate(ctx context.Context, parents asset.Parents } // Set the private zone - privateZoneName, err = manifests.GetGCPPrivateZoneName(ctx, client, installConfig, clusterID.InfraID) + privateZoneName, _, err = manifests.GetGCPPrivateZoneName(ctx, client, installConfig, clusterID.InfraID) if err != nil { return fmt.Errorf("failed to find gcp private dns zone: %w", err) } diff --git a/pkg/asset/installconfig/gcp/client.go b/pkg/asset/installconfig/gcp/client.go index 66132623973..7744a1aaf31 100644 --- a/pkg/asset/installconfig/gcp/client.go +++ b/pkg/asset/installconfig/gcp/client.go @@ -42,6 +42,7 @@ type API interface { GetMachineTypeWithZones(ctx context.Context, project, region, machineType string) (*compute.MachineType, sets.Set[string], error) GetPublicDomains(ctx context.Context, project string) ([]string, error) GetDNSZone(ctx context.Context, project, baseDomain string, isPublic bool) (*dns.ManagedZone, error) + GetDNSZoneFromParams(ctx context.Context, params gcptypes.DNSZoneParams) (*dns.ManagedZone, error) GetDNSZoneByName(ctx context.Context, project, zoneName string) (*dns.ManagedZone, error) GetSubnetworks(ctx context.Context, network, project, region string) ([]*compute.Subnetwork, error) GetProjects(ctx context.Context) (map[string]string, error) @@ -58,6 +59,7 @@ type API interface { GetProjectTags(ctx context.Context, projectID string) (sets.Set[string], error) GetNamespacedTagValue(ctx context.Context, tagNamespacedName string) (*cloudresourcemanager.TagValue, error) GetKeyRing(ctx context.Context, kmsKeyRef *gcptypes.KMSKeyReference) (*kmspb.KeyRing, error) + UpdateDNSPrivateZoneLabels(ctx context.Context, baseDomain, project, zoneName string, labels map[string]string) error } // Client makes calls to the GCP API. @@ -243,41 +245,138 @@ func (c *Client) GetPublicDomains(ctx context.Context, project string) ([]string return publicZones, nil } +func getDNSZoneByName(ctx context.Context, svc *dns.Service, project, zoneName string) (*dns.ManagedZone, error) { + returnedZone, err := svc.ManagedZones.Get(project, zoneName).Context(ctx).Do() + if err != nil { + return nil, fmt.Errorf("failed to get DNS Zones: %w", err) + } + return returnedZone, nil +} + // GetDNSZoneByName returns a DNS zone matching the `zoneName` if the DNS zone exists // and can be seen (correct permissions for a private zone) in the project. func (c *Client) GetDNSZoneByName(ctx context.Context, project, zoneName string) (*dns.ManagedZone, error) { - ctx, cancel := context.WithTimeout(ctx, defaultTimeout) - defer cancel() - svc, err := c.getDNSService(ctx) if err != nil { return nil, err } - returnedZone, err := svc.ManagedZones.Get(project, zoneName).Context(ctx).Do() + return getDNSZoneByName(ctx, svc, project, zoneName) +} + +// UpdateDNSPrivateZoneLabels will find a private DNS zone in the project with the name passed in. The labels +// for the zone will be updated to include the provided labels. The labels that match will be overwritten +// and all other labels will remain. +func (c *Client) UpdateDNSPrivateZoneLabels(ctx context.Context, baseDomain, project, zoneName string, labels map[string]string) error { + params := gcptypes.DNSZoneParams{ + Project: project, + Name: zoneName, + BaseDomain: baseDomain, + IsPublic: false, + } + zone, err := c.GetDNSZoneFromParams(ctx, params) if err != nil { - return nil, errors.Wrap(err, "failed to get DNS Zones") + return err + } + if zone == nil { + return fmt.Errorf("failed to find matching DNS zone for %s in project %s", zoneName, project) } - return returnedZone, nil -} -// GetDNSZone returns a DNS zone for a basedomain. -func (c *Client) GetDNSZone(ctx context.Context, project, baseDomain string, isPublic bool) (*dns.ManagedZone, error) { - ctx, cancel := context.WithTimeout(ctx, defaultTimeout) - defer cancel() + if zone.Labels == nil { + zone.Labels = make(map[string]string) + } - svc, err := c.getDNSService(ctx) + for key, value := range labels { + zone.Labels[key] = value + } + + if zone.Description == "" { + // It is possible to create a managed zone without a description using the GCP web console. + // If the description is missing the managed zone modification will fail. + zone.Description = "Used by OpenShift Installer" + } + + dnsService, err := c.getDNSService(ctx) if err != nil { - return nil, err + return fmt.Errorf("failed to get dns service during dns managed zone update: %w", err) + } + + return UpdateDNSManagedZone(ctx, dnsService, project, zoneName, zone) +} + +// UpdateDNSManagedZone will update a dns managed zone with the matching name and project. The new zone +// information is contained in the zone parameter. +func UpdateDNSManagedZone(ctx context.Context, svc *dns.Service, project, zoneName string, zone *dns.ManagedZone) error { + _, err := svc.ManagedZones.Update(project, zoneName, zone).Context(ctx).Do() + if err != nil { + return fmt.Errorf("failed updating DNS Zone %s in project %s: %w", zoneName, project, err) } - if !strings.HasSuffix(baseDomain, ".") { - baseDomain = fmt.Sprintf("%s.", baseDomain) + return nil +} + +func formatBaseDomain(domain string) string { + if !strings.HasSuffix(domain, ".") { + domain = fmt.Sprintf("%s.", domain) } + return domain +} - // currently, only private and public are supported. All peering zones are private. +func getZoneVisibility(isPublic bool) string { visibility := "private" if isPublic { visibility = "public" } + return visibility +} + +// GetDNSZoneFromParams allows the user to enter parameters found in `DNSZoneParams` to find a +// dns managed zone by name or by base domain. +func GetDNSZoneFromParams(ctx context.Context, svc *dns.Service, params gcptypes.DNSZoneParams) (*dns.ManagedZone, error) { + switch { + case params.Name == "" && params.BaseDomain != "": + return getDNSZone(ctx, svc, params.Project, params.BaseDomain, params.IsPublic) + case params.Name != "": + managedZone, err := getDNSZoneByName(ctx, svc, params.Project, params.Name) + if params.BaseDomain == "" { + return managedZone, err + } + if err != nil { + if IsNotFound(err) { + return nil, nil + } + return nil, err + } + if managedZone == nil { + return nil, nil + } + baseDomain := formatBaseDomain(params.BaseDomain) + if !strings.HasSuffix(managedZone.DnsName, baseDomain) { + return nil, fmt.Errorf("failed to find matching DNS zone for %s with DNS name %s", params.Name, params.BaseDomain) + } + visibility := getZoneVisibility(params.IsPublic) + if managedZone.Visibility != visibility { + return nil, fmt.Errorf("failed to find matching DNS zone for %s with visibility %s", params.Name, visibility) + } + return managedZone, nil + } + return nil, fmt.Errorf("invalid dns zone parameters, please provide a base domain or name") +} + +// GetDNSZoneFromParams allows the user to enter parameters found in DNSZoneParams. The user must enter at +// least a base domain or a zone name to make a valid request. When both fields are populated extra validation +// steps occur to ensure that the correct zone is found. +func (c *Client) GetDNSZoneFromParams(ctx context.Context, params gcptypes.DNSZoneParams) (*dns.ManagedZone, error) { + svc, err := c.getDNSService(ctx) + if err != nil { + return nil, err + } + return GetDNSZoneFromParams(ctx, svc, params) +} + +func getDNSZone(ctx context.Context, svc *dns.Service, project, baseDomain string, isPublic bool) (*dns.ManagedZone, error) { + baseDomain = formatBaseDomain(baseDomain) + + // currently, only private and public are supported. All peering zones are private. + visibility := getZoneVisibility(isPublic) req := svc.ManagedZones.List(project).DnsName(baseDomain).Context(ctx) var res *dns.ManagedZone @@ -290,7 +389,7 @@ func (c *Client) GetDNSZone(ctx context.Context, project, baseDomain string, isP } return nil }); err != nil { - return nil, errors.Wrap(err, "failed to list DNS Zones") + return nil, fmt.Errorf("failed to list DNS Zones: %w", err) } if res == nil { if isPublic { @@ -305,6 +404,15 @@ func (c *Client) GetDNSZone(ctx context.Context, project, baseDomain string, isP return res, nil } +// GetDNSZone returns a DNS zone for a basedomain. +func (c *Client) GetDNSZone(ctx context.Context, project, baseDomain string, isPublic bool) (*dns.ManagedZone, error) { + svc, err := c.getDNSService(ctx) + if err != nil { + return nil, err + } + return getDNSZone(ctx, svc, project, baseDomain, isPublic) +} + // GetRecordSets returns all the records for a DNS zone. func (c *Client) GetRecordSets(ctx context.Context, project, zone string) ([]*dns.ResourceRecordSet, error) { ctx, cancel := context.WithTimeout(ctx, defaultTimeout) diff --git a/pkg/asset/installconfig/gcp/dns.go b/pkg/asset/installconfig/gcp/dns.go index f7eb6bec806..c73f3334387 100644 --- a/pkg/asset/installconfig/gcp/dns.go +++ b/pkg/asset/installconfig/gcp/dns.go @@ -70,6 +70,9 @@ func IsThrottled(err error) bool { // IsNotFound checks whether a response from the GPC API was not found. func IsNotFound(err error) bool { - gErr, ok := err.(*googleapi.Error) - return ok && gErr.Code == http.StatusNotFound + var gErr *googleapi.Error + if errors.As(err, &gErr) { + return gErr.Code == http.StatusNotFound + } + return false } diff --git a/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go b/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go index 002e1e03175..4286054da61 100644 --- a/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go +++ b/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go @@ -91,6 +91,21 @@ func (mr *MockAPIMockRecorder) GetDNSZoneByName(ctx, project, zoneName any) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDNSZoneByName", reflect.TypeOf((*MockAPI)(nil).GetDNSZoneByName), ctx, project, zoneName) } +// GetDNSZoneFromParams mocks base method. +func (m *MockAPI) GetDNSZoneFromParams(ctx context.Context, params gcp.DNSZoneParams) (*dns.ManagedZone, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetDNSZoneFromParams", ctx, params) + ret0, _ := ret[0].(*dns.ManagedZone) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetDNSZoneFromParams indicates an expected call of GetDNSZoneFromParams. +func (mr *MockAPIMockRecorder) GetDNSZoneFromParams(ctx, params any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDNSZoneFromParams", reflect.TypeOf((*MockAPI)(nil).GetDNSZoneFromParams), ctx, params) +} + // GetEnabledServices mocks base method. func (m *MockAPI) GetEnabledServices(ctx context.Context, project string) ([]string, error) { m.ctrl.T.Helper() @@ -347,6 +362,20 @@ func (mr *MockAPIMockRecorder) GetZones(ctx, project, filter any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetZones", reflect.TypeOf((*MockAPI)(nil).GetZones), ctx, project, filter) } +// UpdateDNSPrivateZoneLabels mocks base method. +func (m *MockAPI) UpdateDNSPrivateZoneLabels(ctx context.Context, baseDomain, project, zoneName string, labels map[string]string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateDNSPrivateZoneLabels", ctx, baseDomain, project, zoneName, labels) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateDNSPrivateZoneLabels indicates an expected call of UpdateDNSPrivateZoneLabels. +func (mr *MockAPIMockRecorder) UpdateDNSPrivateZoneLabels(ctx, baseDomain, project, zoneName, labels any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDNSPrivateZoneLabels", reflect.TypeOf((*MockAPI)(nil).UpdateDNSPrivateZoneLabels), ctx, baseDomain, project, zoneName, labels) +} + // ValidateServiceAccountHasPermissions mocks base method. func (m *MockAPI) ValidateServiceAccountHasPermissions(ctx context.Context, project string, permissions []string) (bool, error) { m.ctrl.T.Helper() diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index 9283d45af87..158b5d07614 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/pkg/asset/installconfig/gcp/validation.go @@ -69,6 +69,7 @@ func Validate(client API, ic *types.InstallConfig) error { allErrs = append(allErrs, ValidateCredentialMode(client, ic)...) allErrs = append(allErrs, validatePreexistingServiceAccount(client, ic)...) allErrs = append(allErrs, ValidatePreExistingPublicDNS(client, ic)...) + allErrs = append(allErrs, ValidatePrivateDNSZone(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"))...) @@ -406,7 +407,7 @@ func ValidatePreExistingPublicDNS(client API, ic *types.InstallConfig) field.Err return append(allErrs, field.InternalError(field.NewPath("baseDomain"), err)) } - if err := checkRecordSets(client, ic, zone, []string{apiRecordType(ic)}); err != nil { + if err := checkRecordSets(client, ic, ic.Platform.GCP.ProjectID, zone, []string{apiRecordType(ic)}); err != nil { allErrs = append(allErrs, err) } return allErrs @@ -414,29 +415,61 @@ func ValidatePreExistingPublicDNS(client API, ic *types.InstallConfig) field.Err // ValidatePrivateDNSZone ensure no pre-existing DNS record exists in the private dns zone // matching the name that will be used for this installation. -func ValidatePrivateDNSZone(client API, ic *types.InstallConfig) *field.Error { +func ValidatePrivateDNSZone(client API, ic *types.InstallConfig) field.ErrorList { if ic.GCP.Network == "" || ic.GCP.NetworkProjectID == "" { return nil } + allErrs := field.ErrorList{} - zone, err := client.GetDNSZone(context.TODO(), ic.GCP.ProjectID, ic.ClusterDomain(), false) - if err != nil { - logrus.Debug("No private DNS Zone found") - if IsNotFound(err) { - return field.NotFound(field.NewPath("baseDomain"), fmt.Sprintf("Private DNS Zone (%s/%s)", ic.Platform.GCP.ProjectID, ic.BaseDomain)) + // The private zone does NOT need to exist. When the zone does exist it will be used, but when + // the zone does not exist one will be created with the specified zone name. + project := ic.GCP.ProjectID + zoneName := "" + icdns := ic.GCP.DNS + if icdns != nil && icdns.PrivateZone != nil { + if icdns.PrivateZone.ProjectID != "" { + project = icdns.PrivateZone.ProjectID } - return field.InternalError(field.NewPath("baseDomain"), err) + zoneName = icdns.PrivateZone.Name } - // Private Zone can be nil, check to see if it was found or not - if zone != nil { - return checkRecordSets(client, ic, zone, []string{apiRecordType(ic), apiIntRecordName(ic)}) + // The base check will determine if any of the private zone exists with the specified base domain. + params := []gcp.DNSZoneParams{{Project: project, IsPublic: false, BaseDomain: ic.ClusterDomain()}} + if zoneName != "" { + // When a private dns zone is specified in the install-config then the test should + // determine if the private zone found is the only one matching the specified base domain. + params = append(params, gcp.DNSZoneParams{Project: project, IsPublic: false, BaseDomain: ic.ClusterDomain(), Name: zoneName}) } - return nil + + for _, paramSet := range params { + zone, err := client.GetDNSZoneFromParams(context.TODO(), paramSet) + if err != nil { + logrus.Debug("No private DNS Zone found") + if IsNotFound(err) { + // Ignore the not found error, because the zone will be created in this instance. + continue + } + return append(allErrs, field.Invalid(field.NewPath("baseDomain"), ic.BaseDomain, err.Error())) + } + + // Private Zone can be nil, check to see if it was found or not + if zone != nil { + if icdns != nil && icdns.PrivateZone != nil && zoneName != zone.Name { + allErrs = append(allErrs, field.Invalid( + field.NewPath("platform").Child("gcp").Child("dns").Child("privateZone").Child("name"), + zoneName, + fmt.Sprintf("found existing private zone %s in project %s with base domain %s", zone.Name, project, ic.BaseDomain), + )) + } else if err := checkRecordSets(client, ic, project, zone, []string{apiRecordType(ic), apiIntRecordName(ic)}); err != nil { + allErrs = append(allErrs, err) + } + } + } + return allErrs } -func checkRecordSets(client API, ic *types.InstallConfig, zone *dns.ManagedZone, records []string) *field.Error { - rrSets, err := client.GetRecordSets(context.TODO(), ic.GCP.ProjectID, zone.Name) +func checkRecordSets(client API, ic *types.InstallConfig, project string, zone *dns.ManagedZone, records []string) *field.Error { + rrSets, err := client.GetRecordSets(context.TODO(), project, zone.Name) if err != nil { return field.InternalError(field.NewPath("baseDomain"), err) } @@ -448,7 +481,7 @@ func checkRecordSets(client API, ic *types.InstallConfig, zone *dns.ManagedZone, preexistingRecords := sets.New[string](records...).Intersection(setOfReturnedRecords) if preexistingRecords.Len() > 0 { - errMsg := fmt.Sprintf("record(s) %q already exists in DNS Zone (%s/%s) and might be in use by another cluster, please remove it to continue", sets.List(preexistingRecords), ic.GCP.ProjectID, zone.Name) + errMsg := fmt.Sprintf("record(s) %q already exists in DNS Zone (%s/%s) and might be in use by another cluster, please remove it to continue", sets.List(preexistingRecords), project, zone.Name) return field.Invalid(field.NewPath("metadata", "name"), ic.ObjectMeta.Name, errMsg) } return nil @@ -462,15 +495,6 @@ func ValidateForProvisioning(ic *types.InstallConfig) error { allErrs := field.ErrorList{} - client, err := NewClient(context.TODO(), ic.GCP.ServiceEndpoints) - if err != nil { - return err - } - - if err := ValidatePrivateDNSZone(client, ic); err != nil { - allErrs = append(allErrs, err) - } - return allErrs.ToAggregate() } diff --git a/pkg/asset/installconfig/gcp/validation_test.go b/pkg/asset/installconfig/gcp/validation_test.go index fd771407918..f57035b0e48 100644 --- a/pkg/asset/installconfig/gcp/validation_test.go +++ b/pkg/asset/installconfig/gcp/validation_test.go @@ -541,11 +541,21 @@ func TestGCPInstallConfigValidation(t *testing.T) { // Return fake credentials when asked gcpClient.EXPECT().GetCredentials().Return(&googleoauth.Credentials{JSON: []byte(fakeCreds)}).AnyTimes() + params := gcp.DNSZoneParams{ + Project: validProjectName, + IsPublic: false, + BaseDomain: "valid-cluster.example.installer.domain", + } + gcpClient.EXPECT().GetDNSZoneFromParams(gomock.Any(), params).Return(&validPrivateDNSZone, nil).AnyTimes() + // Expected results for the managed zone tests gcpClient.EXPECT().GetDNSZoneByName(gomock.Any(), gomock.Any(), validPublicZone).Return(&validPublicDNSZone, nil).AnyTimes() gcpClient.EXPECT().GetDNSZoneByName(gomock.Any(), gomock.Any(), validPrivateZone).Return(&validPrivateDNSZone, nil).AnyTimes() gcpClient.EXPECT().GetDNSZoneByName(gomock.Any(), gomock.Any(), invalidPublicZone).Return(nil, fmt.Errorf("no matching DNS Zone found")).AnyTimes() + records := []*dns.ResourceRecordSet{{Name: "valid-cluster.example.installer.domain."}} + gcpClient.EXPECT().GetRecordSets(gomock.Any(), validProjectName, validPrivateZone).Return(records, nil).AnyTimes() + gcpClient.EXPECT().GetServiceAccount(gomock.Any(), validProjectName, validXpnSA).Return(validXpnSA, nil).AnyTimes() gcpClient.EXPECT().GetServiceAccount(gomock.Any(), validProjectName, invalidXpnSA).Return("", fmt.Errorf("controlPlane.platform.gcp.serviceAccount: Internal error\"")).AnyTimes() @@ -602,23 +612,60 @@ func TestGCPInstallConfigValidation(t *testing.T) { func TestValidatePreExistingPublicDNS(t *testing.T) { cases := []struct { - name string - records []*dns.ResourceRecordSet - err string + name string + records []*dns.ResourceRecordSet + platform types.Platform + err string }{{ - name: "no pre-existing", - records: nil, + name: "no pre-existing", + platform: types.Platform{GCP: &gcp.Platform{ProjectID: "project-id"}}, + records: nil, }, { - name: "no pre-existing", - records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.base-domain."}}, + name: "no pre-existing", + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.base-domain."}}, + platform: types.Platform{GCP: &gcp.Platform{ProjectID: "project-id"}}, }, { - name: "pre-existing", - records: []*dns.ResourceRecordSet{{Name: "api.cluster-name.base-domain."}}, - err: `^metadata\.name: Invalid value: "cluster-name": record\(s\) \["api\.cluster-name\.base-domain\."\] already exists in DNS Zone \(project-id/zone-name\) and might be in use by another cluster, please remove it to continue$`, + name: "pre-existing", + records: []*dns.ResourceRecordSet{{Name: "api.cluster-name.base-domain."}}, + platform: types.Platform{GCP: &gcp.Platform{ProjectID: "project-id"}}, + err: `^metadata\.name: Invalid value: "cluster-name": record\(s\) \["api\.cluster-name\.base-domain\."\] already exists in DNS Zone \(project-id/zone-name\) and might be in use by another cluster, please remove it to continue$`, }, { - name: "pre-existing", - records: []*dns.ResourceRecordSet{{Name: "api.cluster-name.base-domain."}, {Name: "api.cluster-name.base-domain."}}, - err: `^metadata\.name: Invalid value: "cluster-name": record\(s\) \["api\.cluster-name\.base-domain\."\] already exists in DNS Zone \(project-id/zone-name\) and might be in use by another cluster, please remove it to continue$`, + name: "pre-existing", + records: []*dns.ResourceRecordSet{{Name: "api.cluster-name.base-domain."}, {Name: "api.cluster-name.base-domain."}}, + platform: types.Platform{GCP: &gcp.Platform{ProjectID: "project-id"}}, + err: `^metadata\.name: Invalid value: "cluster-name": record\(s\) \["api\.cluster-name\.base-domain\."\] already exists in DNS Zone \(project-id/zone-name\) and might be in use by another cluster, please remove it to continue$`, + }, { + name: "no pre-existing with specified zone", + platform: types.Platform{GCP: &gcp.Platform{ + ProjectID: "other-project-id", + DNS: &gcp.DNS{ + PrivateZone: &gcp.DNSZone{ + Name: "zone-name", + ProjectID: "project-id", + }, + }, + NetworkProjectID: "network-project-id", + Network: "test-network", + ControlPlaneSubnet: "test-subnet", + ComputeSubnet: "test-subnet", + }}, + }, { + name: "pre-existing with specified zone", + records: []*dns.ResourceRecordSet{{Name: "api.cluster-name.base-domain."}}, + platform: types.Platform{GCP: &gcp.Platform{ + ProjectID: "other-project-id", + DNS: &gcp.DNS{ + PrivateZone: &gcp.DNSZone{ + Name: "zone-name", + ProjectID: "project-id", + }, + }, + NetworkProjectID: "network-project-id", + Network: "test-network", + ControlPlaneSubnet: "test-subnet", + ComputeSubnet: "test-subnet", + }}, + err: `^metadata\.name: Invalid value: "cluster-name": record\(s\) \["api\.cluster-name\.base-domain\."\] already exists in DNS Zone \(project-id/zone-name\) and might be in use by another cluster, please remove it to continue$`, }} for _, test := range cases { @@ -646,39 +693,83 @@ func TestValidatePreExistingPublicDNS(t *testing.T) { func TestValidatePrivateDNSZone(t *testing.T) { cases := []struct { - name string - records []*dns.ResourceRecordSet - err string + name string + records []*dns.ResourceRecordSet + platform types.Platform + err string }{{ - name: "no pre-existing", - records: nil, + name: "no pre-existing", + platform: types.Platform{GCP: &gcp.Platform{ProjectID: "project-id", Network: "shared-vpc", NetworkProjectID: "test-network-project"}}, + records: nil, }, { - name: "no pre-existing", - records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.base-domain."}}, + name: "no pre-existing", + platform: types.Platform{GCP: &gcp.Platform{ProjectID: "project-id", Network: "shared-vpc", NetworkProjectID: "test-network-project"}}, + records: []*dns.ResourceRecordSet{{Name: "api.another-cluster-name.base-domain."}}, }, { - name: "pre-existing", - records: []*dns.ResourceRecordSet{{Name: "api.cluster-name.base-domain."}}, - err: `^metadata\.name: Invalid value: "cluster-name": record\(s\) \["api\.cluster-name\.base-domain\."\] already exists in DNS Zone \(project-id/zone-name\) and might be in use by another cluster, please remove it to continue$`, + name: "pre-existing", + records: []*dns.ResourceRecordSet{{Name: "api.cluster-name.base-domain."}}, + platform: types.Platform{GCP: &gcp.Platform{ProjectID: "project-id", Network: "shared-vpc", NetworkProjectID: "test-network-project"}}, + err: `^metadata\.name: Invalid value: "cluster-name": record\(s\) \["api\.cluster-name\.base-domain\."\] already exists in DNS Zone \(project-id/zone-name\) and might be in use by another cluster, please remove it to continue$`, + }, { + name: "pre-existing", + records: []*dns.ResourceRecordSet{{Name: "api.cluster-name.base-domain."}, {Name: "api.cluster-name.base-domain."}}, + platform: types.Platform{GCP: &gcp.Platform{ProjectID: "project-id", Network: "shared-vpc", NetworkProjectID: "test-network-project"}}, + err: `^metadata\.name: Invalid value: "cluster-name": record\(s\) \["api\.cluster-name\.base-domain\."\] already exists in DNS Zone \(project-id/zone-name\) and might be in use by another cluster, please remove it to continue$`, + }, { + name: "no pre-existing with specified zone", + platform: types.Platform{GCP: &gcp.Platform{ + ProjectID: "other-project-id", + DNS: &gcp.DNS{ + PrivateZone: &gcp.DNSZone{ + Name: "zone-name", + ProjectID: "project-id", + }, + }, + NetworkProjectID: "network-project-id", + Network: "test-network", + ControlPlaneSubnet: "test-subnet", + ComputeSubnet: "test-subnet", + }}, }, { - name: "pre-existing", - records: []*dns.ResourceRecordSet{{Name: "api.cluster-name.base-domain."}, {Name: "api.cluster-name.base-domain."}}, - err: `^metadata\.name: Invalid value: "cluster-name": record\(s\) \["api\.cluster-name\.base-domain\."\] already exists in DNS Zone \(project-id/zone-name\) and might be in use by another cluster, please remove it to continue$`, + name: "pre-existing with specified zone", + records: []*dns.ResourceRecordSet{{Name: "api.cluster-name.base-domain."}}, + platform: types.Platform{GCP: &gcp.Platform{ + ProjectID: "other-project-id", + DNS: &gcp.DNS{ + PrivateZone: &gcp.DNSZone{ + Name: "zone-name", + ProjectID: "project-id", + }, + }, + NetworkProjectID: "network-project-id", + Network: "test-network", + ControlPlaneSubnet: "test-subnet", + ComputeSubnet: "test-subnet", + }}, + err: `^metadata\.name: Invalid value: "cluster-name": record\(s\) \["api\.cluster-name\.base-domain\."\] already exists in DNS Zone \(project-id/zone-name\) and might be in use by another cluster, please remove it to continue$`, }} + params := gcp.DNSZoneParams{Project: "project-id", IsPublic: false, BaseDomain: "cluster-name.base-domain"} + for _, test := range cases { t.Run(test.name, func(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() gcpClient := mock.NewMockAPI(mockCtrl) + gcpClient.EXPECT().GetDNSZoneFromParams(gomock.Any(), params).Return(&dns.ManagedZone{Name: "zone-name"}, nil).AnyTimes() + if test.platform.GCP.DNS != nil && test.platform.GCP.DNS.PrivateZone != nil { + paramsWithName := gcp.DNSZoneParams{Project: "project-id", IsPublic: false, BaseDomain: "cluster-name.base-domain", Name: test.platform.GCP.DNS.PrivateZone.Name} + gcpClient.EXPECT().GetDNSZoneFromParams(gomock.Any(), paramsWithName).Return(&dns.ManagedZone{Name: "zone-name"}, nil).AnyTimes() + } gcpClient.EXPECT().GetDNSZone(gomock.Any(), "project-id", "cluster-name.base-domain", false).Return(&dns.ManagedZone{Name: "zone-name"}, nil).AnyTimes() gcpClient.EXPECT().GetRecordSets(gomock.Any(), gomock.Eq("project-id"), gomock.Eq("zone-name")).Return(test.records, nil).AnyTimes() err := ValidatePrivateDNSZone(gcpClient, &types.InstallConfig{ ObjectMeta: metav1.ObjectMeta{Name: "cluster-name"}, BaseDomain: "base-domain", - Platform: types.Platform{GCP: &gcp.Platform{ProjectID: "project-id", Network: "shared-vpc", NetworkProjectID: "test-network-project"}}, - }) + Platform: test.platform, + }).ToAggregate() if test.err == "" { assert.True(t, err == nil) } else { diff --git a/pkg/asset/manifests/dns.go b/pkg/asset/manifests/dns.go index 5ea6702b9d8..d702eb7c346 100644 --- a/pkg/asset/manifests/dns.go +++ b/pkg/asset/manifests/dns.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/pkg/errors" + "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/yaml" @@ -135,7 +136,7 @@ func (d *DNS) Generate(ctx context.Context, dependencies asset.Parents) error { } } case gcptypes.Name: - // We donot want to configure cloud DNS when `UserProvisionedDNS` is enabled. + // We do not want to configure cloud DNS when `UserProvisionedDNS` is enabled. // So, do not set PrivateZone and PublicZone fields in the DNS manifest. if installConfig.Config.GCP.UserProvisionedDNS == dnstypes.UserProvisionedDNSEnabled { config.Spec.PublicZone = &configv1.DNSZone{ID: ""} @@ -157,15 +158,23 @@ func (d *DNS) Generate(ctx context.Context, dependencies asset.Parents) error { if err != nil { return errors.Wrapf(err, "failed to get public zone for %q", installConfig.Config.BaseDomain) } - config.Spec.PublicZone = &configv1.DNSZone{ID: zone.Name} + + publicZoneName := fmt.Sprintf("projects/%s/managedZones/%s", installConfig.Config.GCP.ProjectID, zone.Name) + logrus.Infof("generating GCP Public DNS Zone %s", publicZoneName) + config.Spec.PublicZone = &configv1.DNSZone{ID: publicZoneName} } - // Set the private zone - privateZoneID, err := GetGCPPrivateZoneName(ctx, client, installConfig, clusterID.InfraID) - if err != nil { - return fmt.Errorf("failed to find gcp private dns zone: %w", err) + // Ingress operator can handle a zone with the following format: + // projects/{projectID}/managedZones/{zoneID}. This will allow + // the installer to pass the project without a new field in the + // DNSZone struct. + dnsZoneProject, privateZoneID := GetPrivateDNSZoneAndProject(installConfig) + if privateZoneID == "" { + privateZoneID = GCPDefaultPrivateZoneID(clusterID.InfraID) } - config.Spec.PrivateZone = &configv1.DNSZone{ID: privateZoneID} + privateZoneName := fmt.Sprintf("projects/%s/managedZones/%s", dnsZoneProject, privateZoneID) + logrus.Infof("generating GCP Private DNS Zone %s", privateZoneName) + config.Spec.PrivateZone = &configv1.DNSZone{ID: privateZoneName} case ibmcloudtypes.Name: client, err := icibmcloud.NewClient(installConfig.Config.Platform.IBMCloud.ServiceEndpoints) @@ -230,29 +239,69 @@ func GCPNetworkName(project, network string) string { return fmt.Sprintf("https://www.googleapis.com/compute/v1/projects/%s/global/networks/%s", project, network) } +// GetPrivateDNSZoneAndProject gets the private dns zone name and project where the dns records should reside. +func GetPrivateDNSZoneAndProject(installConfig *installconfig.InstallConfig) (string, string) { + project := installConfig.Config.GCP.ProjectID + zone := "" + if installConfig.Config.GCP.Network == "" || installConfig.Config.GCP.NetworkProjectID == "" { + return project, zone + } + + icdns := installConfig.Config.GCP.DNS + if icdns != nil && icdns.PrivateZone != nil { + if icdns.PrivateZone.ProjectID != "" { + project = icdns.PrivateZone.ProjectID + } + zone = icdns.PrivateZone.Name + } + return project, zone +} + +// GCPDefaultPrivateZoneID returns the default name for a gcp private dns zone. This zone name will be used during +// installations where the user has not provided a private zone name (xpn installs only), no +// preexisting private dns zone is found (xpn installs only), and default installation cases. +func GCPDefaultPrivateZoneID(clusterID string) string { + return fmt.Sprintf("%s-private-zone", clusterID) +} + // GetGCPPrivateZoneName attempts to find the name of the private zone for GCP installs. When a shared vpc install // occurs, a precreated zone may be used. If a zone is found (in this instance), then the zone should be paired with // the network that is supplied through the install config (when applicable). -func GetGCPPrivateZoneName(ctx context.Context, client *icgcp.Client, installConfig *installconfig.InstallConfig, clusterID string) (string, error) { - privateZoneID := fmt.Sprintf("%s-private-zone", clusterID) +func GetGCPPrivateZoneName(ctx context.Context, client *icgcp.Client, installConfig *installconfig.InstallConfig, clusterID string) (string, bool, error) { + privateZoneID := GCPDefaultPrivateZoneID(clusterID) + shouldCreateZone := true + if installConfig.Config.GCP.NetworkProjectID != "" { - zone, err := client.GetDNSZone(ctx, installConfig.Config.GCP.ProjectID, installConfig.Config.ClusterDomain(), false) + project, privateZoneName := GetPrivateDNSZoneAndProject(installConfig) + if privateZoneName != "" { + // Override the default with the name provided. If this zone does not exist, then + // this should still be returned. + privateZoneID = privateZoneName + } + + zone, err := client.GetDNSZoneFromParams(ctx, gcptypes.DNSZoneParams{ + Project: project, + Name: privateZoneID, + IsPublic: false, + BaseDomain: installConfig.Config.ClusterDomain(), + }) if err != nil { - return "", fmt.Errorf("failed to get private zone for %q: %w", installConfig.Config.BaseDomain, err) + // Currently, the only time that a private zone lookup will produce an error is if we + // failed to find the dns zones. That should result in an error returned here too. + return privateZoneID, true, fmt.Errorf("private dns zone %s does not exist or is invalid: %w", privateZoneID, err) } - if zone != nil { - if installConfig.Config.GCP.Network != "" { - expectedNetworkURL := GCPNetworkName(installConfig.Config.GCP.NetworkProjectID, installConfig.Config.GCP.Network) - for _, network := range zone.PrivateVisibilityConfig.Networks { - if network.NetworkUrl == expectedNetworkURL { - privateZoneID = zone.Name - break - } - } - } + if zone == nil { + // CORS-4012: The user may specify a zone to be created if it does not exist. + // Do not fail if the specified zone does not exist. + return privateZoneID, true, nil + } + + if installConfig.Config.GCP.Network != "" { + privateZoneID = zone.Name + shouldCreateZone = false } } - return privateZoneID, nil + return privateZoneID, shouldCreateZone, nil } // Files returns the files generated by the asset. diff --git a/pkg/destroy/gcp/dns.go b/pkg/destroy/gcp/dns.go index fe1ae7bbcac..b12308410bd 100644 --- a/pkg/destroy/gcp/dns.go +++ b/pkg/destroy/gcp/dns.go @@ -9,12 +9,17 @@ import ( "github.com/sirupsen/logrus" dns "google.golang.org/api/dns/v1" "k8s.io/apimachinery/pkg/util/sets" + + "github.com/openshift/installer/pkg/asset/installconfig/gcp" + gcpconsts "github.com/openshift/installer/pkg/constants/gcp" + gcptypes "github.com/openshift/installer/pkg/types/gcp" ) type dnsZone struct { name string domain string project string + labels map[string]string } func (o *ClusterUninstaller) listDNSZones(ctx context.Context) (private *dnsZone, public []dnsZone, err error) { @@ -26,18 +31,30 @@ func (o *ClusterUninstaller) listDNSZones(ctx context.Context) (private *dnsZone if o.NetworkProjectID != "" { projects = append(projects, o.NetworkProjectID) } + if o.PrivateZoneProjectID != "" { + projects = append(projects, o.PrivateZoneProjectID) + } for _, project := range projects { - req := o.dnsSvc.ManagedZones.List(project).Fields("managedZones(name,dnsName,visibility),nextPageToken") + req := o.dnsSvc.ManagedZones.List(project).Fields("managedZones(name,dnsName,visibility,labels),nextPageToken") err = req.Pages(ctx, func(response *dns.ManagedZonesListResponse) error { for _, zone := range response.ManagedZones { switch zone.Visibility { case "private": - if o.isClusterResource(zone.Name) || (o.PrivateZoneDomain != "" && o.PrivateZoneDomain == zone.DnsName) { - private = &dnsZone{name: zone.Name, domain: zone.DnsName, project: project} + // if a dns private managed zone is provided make sure the correct zone is found and + // added to the list for destroy. + if o.PrivateZoneProjectID != "" && project != o.PrivateZoneProjectID { + // The private zone should exist in the private zone project if one was provided. + continue + } + if o.isClusterResource(zone.Name) || (o.PrivateZoneDomain != "" && o.PrivateZoneDomain == zone.DnsName) || + o.isManagedResource(zone.Labels) { + private = &dnsZone{name: zone.Name, domain: zone.DnsName, project: project, labels: zone.Labels} } default: - public = append(public, dnsZone{name: zone.Name, domain: zone.DnsName, project: project}) + if project == o.ProjectID { + public = append(public, dnsZone{name: zone.Name, domain: zone.DnsName, project: project, labels: zone.Labels}) + } } } return nil @@ -49,16 +66,63 @@ func (o *ClusterUninstaller) listDNSZones(ctx context.Context) (private *dnsZone return } -func (o *ClusterUninstaller) deleteDNSZone(ctx context.Context, name string) error { - if !o.isClusterResource(name) { +// removeSharedTag will remove the shared tag associated with this cluster from the DNS Managed Zone. +func (o *ClusterUninstaller) removeSharedTag(ctx context.Context, svc *dns.Service, clusterID, baseDomain, project, zoneName string, isPublic bool) error { + params := gcptypes.DNSZoneParams{ + Project: project, + Name: zoneName, + BaseDomain: baseDomain, + IsPublic: isPublic, + } + zone, err := gcp.GetDNSZoneFromParams(ctx, svc, params) + if err != nil { + return err + } + + if zone == nil { + return fmt.Errorf("failed to find matching DNS zone for %s in project %s", zoneName, project) + } + + if zone.Labels == nil { + return nil + } + + // Remove the shared tag for this cluster from the DNS Managed Zone. + // First, make sure that the tag value is shared and not owned or anything else. + labelKey := fmt.Sprintf(gcpconsts.ClusterIDLabelFmt, clusterID) + if value, ok := zone.Labels[labelKey]; ok { + if value == sharedLabelValue { + delete(zone.Labels, labelKey) + } + } + + return gcp.UpdateDNSManagedZone(ctx, svc, project, zoneName, zone) +} + +func (o *ClusterUninstaller) deleteDNSZone(ctx context.Context, name string, labels map[string]string) error { + // The destroy code is executed in a loop. On the first pass it is possible that the + // zone contained the "shared" label and the label is removed. The zone should NOT + // be deleted in this case, so the process is skipped. + if !o.isManagedResource(labels) && !o.isClusterResource(name) { + o.Logger.Debugf("DNS Zone %s is not owned or shared by the cluster, skipping deletion", name) + return nil + } + + if o.isSharedResource(labels) { o.Logger.Warnf("Skipping deletion of DNS Zone %s, not created by installer", name) + // currently, this function only runs for the private managed zone, but + // let's add a check just to be sure. + err := o.removeSharedTag(ctx, o.dnsSvc, o.ClusterID, o.PrivateZoneDomain, o.PrivateZoneProjectID, name, false) + if err != nil { + return fmt.Errorf("failed to remove shared tag from DNS Zone %s: %w", name, err) + } return nil } o.Logger.Debugf("Deleting DNS zones %s", name) ctx, cancel := context.WithTimeout(ctx, defaultTimeout) defer cancel() - err := o.dnsSvc.ManagedZones.Delete(o.ProjectID, name).Context(ctx).Do() + err := o.dnsSvc.ManagedZones.Delete(o.PrivateZoneProjectID, name).Context(ctx).Do() if err != nil && !isNoOp(err) { return errors.Wrapf(err, "failed to delete DNS zone %s", name) } @@ -194,7 +258,7 @@ func (o *ClusterUninstaller) destroyDNS(ctx context.Context) error { if err != nil { return err } - err = o.deleteDNSZone(ctx, privateZone.name) + err = o.deleteDNSZone(ctx, privateZone.name, privateZone.labels) if err != nil { return err } diff --git a/pkg/destroy/gcp/gcp.go b/pkg/destroy/gcp/gcp.go index eb8e0fc1417..d3511fd3f83 100644 --- a/pkg/destroy/gcp/gcp.go +++ b/pkg/destroy/gcp/gcp.go @@ -40,7 +40,8 @@ const ( // used for resources created by the Cluster API GCP provider. capgProviderOwnedLabelFmt = "capg-cluster-%s" - ownedLabelValue = "owned" + ownedLabelValue = "owned" + sharedLabelValue = "shared" // DONE represents the done status for a compute service operation. DONE = "DONE" @@ -48,12 +49,13 @@ const ( // ClusterUninstaller holds the various options for the cluster we want to delete type ClusterUninstaller struct { - Logger logrus.FieldLogger - Region string - ProjectID string - NetworkProjectID string - PrivateZoneDomain string - ClusterID string + Logger logrus.FieldLogger + Region string + ProjectID string + NetworkProjectID string + PrivateZoneDomain string + ClusterID string + PrivateZoneProjectID string computeSvc *compute.Service iamSvc *iam.Service @@ -84,16 +86,17 @@ type ClusterUninstaller struct { // New returns a GCP destroyer from ClusterMetadata. func New(logger logrus.FieldLogger, metadata *types.ClusterMetadata) (providers.Destroyer, error) { return &ClusterUninstaller{ - Logger: logger, - Region: metadata.ClusterPlatformMetadata.GCP.Region, - ProjectID: metadata.ClusterPlatformMetadata.GCP.ProjectID, - NetworkProjectID: metadata.ClusterPlatformMetadata.GCP.NetworkProjectID, - PrivateZoneDomain: metadata.ClusterPlatformMetadata.GCP.PrivateZoneDomain, - ClusterID: metadata.InfraID, - cloudControllerUID: gcptypes.CloudControllerUID(metadata.InfraID), - requestIDTracker: newRequestIDTracker(), - pendingItemTracker: newPendingItemTracker(), - serviceEndpoints: metadata.ClusterPlatformMetadata.GCP.ServiceEndpoints, + Logger: logger, + Region: metadata.ClusterPlatformMetadata.GCP.Region, + ProjectID: metadata.ClusterPlatformMetadata.GCP.ProjectID, + NetworkProjectID: metadata.ClusterPlatformMetadata.GCP.NetworkProjectID, + PrivateZoneDomain: metadata.ClusterPlatformMetadata.GCP.PrivateZoneDomain, + PrivateZoneProjectID: metadata.ClusterPlatformMetadata.GCP.PrivateZoneProjectID, + ClusterID: metadata.InfraID, + cloudControllerUID: gcptypes.CloudControllerUID(metadata.InfraID), + requestIDTracker: newRequestIDTracker(), + pendingItemTracker: newPendingItemTracker(), + serviceEndpoints: metadata.ClusterPlatformMetadata.GCP.ServiceEndpoints, }, nil } @@ -256,6 +259,28 @@ func (o *ClusterUninstaller) isClusterResource(name string) bool { return strings.HasPrefix(name, o.ClusterID+"-") } +func (o *ClusterUninstaller) isLabeledResource(labels map[string]string, clusterLabelValue string) bool { + if labels == nil { + return false + } + if val, ok := labels[fmt.Sprintf(gcpconsts.ClusterIDLabelFmt, o.ClusterID)]; ok { + return val == clusterLabelValue + } + return false +} + +func (o *ClusterUninstaller) isOwnedResource(labels map[string]string) bool { + return o.isLabeledResource(labels, ownedLabelValue) +} + +func (o *ClusterUninstaller) isSharedResource(labels map[string]string) bool { + return o.isLabeledResource(labels, sharedLabelValue) +} + +func (o *ClusterUninstaller) isManagedResource(labels map[string]string) bool { + return o.isOwnedResource(labels) || o.isSharedResource(labels) +} + func (o *ClusterUninstaller) clusterIDFilter() string { return fmt.Sprintf("name : \"%s-*\"", o.ClusterID) } diff --git a/pkg/infrastructure/gcp/clusterapi/dns.go b/pkg/infrastructure/gcp/clusterapi/dns.go index 79935372d79..6b096fecf31 100644 --- a/pkg/infrastructure/gcp/clusterapi/dns.go +++ b/pkg/infrastructure/gcp/clusterapi/dns.go @@ -31,12 +31,14 @@ func getDNSZoneName(ctx context.Context, ic *installconfig.InstallConfig, isPubl cctx, ccancel := context.WithTimeout(ctx, time.Minute*1) defer ccancel() - domain := ic.Config.ClusterDomain() - if isPublic { - domain = ic.Config.BaseDomain + domain := ic.Config.BaseDomain + project := ic.Config.GCP.ProjectID + if !isPublic { + project, _ = manifests.GetPrivateDNSZoneAndProject(ic) + domain = ic.Config.ClusterDomain() } - zone, err := client.GetDNSZone(cctx, ic.Config.GCP.ProjectID, domain, isPublic) + zone, err := client.GetDNSZone(cctx, project, domain, isPublic) if err != nil { return "", fmt.Errorf("failed to get dns zone name: %w", err) } @@ -59,22 +61,15 @@ func createRecordSets(ctx context.Context, ic *installconfig.InstallConfig, clus ctx, cancel := context.WithTimeout(ctx, time.Minute*1) defer cancel() - // A shared VPC install allows a user to preconfigure a private zone. If one exists it will be found - // with the call to GetGCPPrivateZone. When a zone does not exist the default pattern - // "{clusterID}-private-zone" is used. - client, err := gcpic.NewClient(context.Background(), ic.Config.GCP.ServiceEndpoints) - if err != nil { - return nil, err - } - privateZoneName, err := manifests.GetGCPPrivateZoneName(ctx, client, ic, clusterID) - if err != nil { - return nil, fmt.Errorf("failed to find gcp private dns zone: %w", err) + project, privateZoneName := manifests.GetPrivateDNSZoneAndProject(ic) + if privateZoneName == "" { + privateZoneName = manifests.GCPDefaultPrivateZoneID(clusterID) } records := []recordSet{ { // api_internal - projectID: ic.Config.GCP.ProjectID, + projectID: project, zoneName: privateZoneName, record: &dns.ResourceRecordSet{ Name: fmt.Sprintf("api-int.%s.", ic.Config.ClusterDomain()), @@ -85,7 +80,7 @@ func createRecordSets(ctx context.Context, ic *installconfig.InstallConfig, clus }, { // api_external_internal_zone - projectID: ic.Config.GCP.ProjectID, + projectID: project, zoneName: privateZoneName, record: &dns.ResourceRecordSet{ Name: fmt.Sprintf("api.%s.", ic.Config.ClusterDomain()), @@ -152,15 +147,25 @@ func createPrivateManagedZone(ctx context.Context, ic *installconfig.InstallConf return err } - defaultPrivateZoneID := fmt.Sprintf("%s-private-zone", clusterID) - // A shared VPC install allows a user to preconfigure a private zone. If there is a private zone found, do not create a new one. - if privateZoneID, err := manifests.GetGCPPrivateZoneName(ctx, client, ic, clusterID); err != nil { - return fmt.Errorf("failed to get GCP private zone: %w", err) - } else if privateZoneID != defaultPrivateZoneID { - // skip if the private zone is attached to the network for the install - logrus.Debugf("found private zone %s, skipping creation of private zone", privateZoneID) - return nil + privateZoneID := manifests.GCPDefaultPrivateZoneID(clusterID) + if ic.Config.GCP.NetworkProjectID != "" { + privateZoneName, shouldCreateZone, err := manifests.GetGCPPrivateZoneName(ctx, client, ic, clusterID) + if err != nil { + return err + } + if !shouldCreateZone { + logrus.Debugf("found private zone %s, skipping creation of private zone", privateZoneName) + privateZoneProject, _ := manifests.GetPrivateDNSZoneAndProject(ic) + // The private zone already exists, so we need to add the shared label to the zone. + labels := mergeLabels(ic, clusterID, sharedLabelValue) + if err := client.UpdateDNSPrivateZoneLabels(ctx, ic.Config.ClusterDomain(), privateZoneProject, privateZoneName, labels); err != nil { + return fmt.Errorf("failed to update dns private zone labels: %w", err) + } + return nil + } + privateZoneID = privateZoneName } + logrus.Debugf("creating private zone %s", privateZoneID) // TODO: use the opts for the service to restrict scopes see google.golang.org/api/option.WithScopes dnsService, err := gcpic.GetDNSService(ctx, ic.Config.GCP.ServiceEndpoints) @@ -169,11 +174,11 @@ func createPrivateManagedZone(ctx context.Context, ic *installconfig.InstallConf } managedZone := &dns.ManagedZone{ - Name: defaultPrivateZoneID, + Name: privateZoneID, Description: resourceDescription, DnsName: fmt.Sprintf("%s.", ic.Config.ClusterDomain()), Visibility: "private", - Labels: mergeLabels(ic, clusterID), + Labels: mergeLabels(ic, clusterID, ownedLabelValue), PrivateVisibilityConfig: &dns.ManagedZonePrivateVisibilityConfig{ Networks: []*dns.ManagedZonePrivateVisibilityConfigNetwork{ { @@ -186,7 +191,8 @@ func createPrivateManagedZone(ctx context.Context, ic *installconfig.InstallConf ctx, cancel := context.WithTimeout(ctx, time.Minute*1) defer cancel() - if _, err = dnsService.ManagedZones.Create(ic.Config.GCP.ProjectID, managedZone).Context(ctx).Do(); err != nil { + project, _ := manifests.GetPrivateDNSZoneAndProject(ic) + if _, err = dnsService.ManagedZones.Create(project, managedZone).Context(ctx).Do(); err != nil { return fmt.Errorf("failed to create private managed zone: %w", err) } diff --git a/pkg/infrastructure/gcp/clusterapi/labels.go b/pkg/infrastructure/gcp/clusterapi/labels.go index 488e4e5339a..15b6cd85f9d 100644 --- a/pkg/infrastructure/gcp/clusterapi/labels.go +++ b/pkg/infrastructure/gcp/clusterapi/labels.go @@ -7,9 +7,14 @@ import ( gcpconsts "github.com/openshift/installer/pkg/constants/gcp" ) -func mergeLabels(ic *installconfig.InstallConfig, clusterID string) map[string]string { +const ( + ownedLabelValue = "owned" + sharedLabelValue = "shared" +) + +func mergeLabels(ic *installconfig.InstallConfig, clusterID, clusterLabelValue string) map[string]string { labels := map[string]string{} - labels[fmt.Sprintf(gcpconsts.ClusterIDLabelFmt, clusterID)] = "owned" + labels[fmt.Sprintf(gcpconsts.ClusterIDLabelFmt, clusterID)] = clusterLabelValue for _, label := range ic.Config.GCP.UserLabels { labels[label.Key] = label.Value } diff --git a/pkg/types/gcp/dns.go b/pkg/types/gcp/dns.go new file mode 100644 index 00000000000..90c9aa568ae --- /dev/null +++ b/pkg/types/gcp/dns.go @@ -0,0 +1,20 @@ +package gcp + +// DNSZoneParams is a set of parameters used to find a DNS zone. +type DNSZoneParams struct { + // Name is the name of the DNS zone. When provided, the name will be + // used for the search. When empty any zone matching the other + // parameters will be returned. Note that either `Name` or `BaseDomain` + // must be provided. + Name string + + // Project is the project of the DNS zone. + Project string + + // IsPublic is true if the DNS zone is public. + IsPublic bool + + // BaseDomain is the base domain of the DNS zone. + // Note that either `Name` or `BaseDomain` must be provided. + BaseDomain string +} diff --git a/pkg/types/gcp/metadata.go b/pkg/types/gcp/metadata.go index b496def968d..5043b6d0a8b 100644 --- a/pkg/types/gcp/metadata.go +++ b/pkg/types/gcp/metadata.go @@ -4,9 +4,10 @@ import configv1 "github.com/openshift/api/config/v1" // Metadata contains GCP metadata (e.g. for uninstalling the cluster). type Metadata struct { - Region string `json:"region"` - ProjectID string `json:"projectID"` - NetworkProjectID string `json:"networkProjectID,omitempty"` - PrivateZoneDomain string `json:"privateZoneDomain,omitempty"` - ServiceEndpoints []configv1.GCPServiceEndpoint `json:"serviceEndpoints,omitempty"` + Region string `json:"region"` + ProjectID string `json:"projectID"` + NetworkProjectID string `json:"networkProjectID,omitempty"` + PrivateZoneDomain string `json:"privateZoneDomain,omitempty"` + PrivateZoneProjectID string `json:"privateZoneProjectID,omitempty"` + ServiceEndpoints []configv1.GCPServiceEndpoint `json:"serviceEndpoints,omitempty"` } diff --git a/pkg/types/gcp/platform.go b/pkg/types/gcp/platform.go index 600afbebbfa..d81222c0708 100644 --- a/pkg/types/gcp/platform.go +++ b/pkg/types/gcp/platform.go @@ -7,6 +7,26 @@ import ( "github.com/openshift/installer/pkg/types/dns" ) +// DNS contains the gcp dns zone information for the cluster. +type DNS struct { + // PrivateZone contains the information for a private DNS zone. The Private DNS Zone can + // only be supplied during Shared VPC (XPN) installs. The PrivateZone can exist or be + // created in a second service project; a project other than the one matching projectID + // or networkProjectID. + // +optional + PrivateZone *DNSZone `json:"privateZone,omitempty"` +} + +// DNSZone contains the information about a specific DNS public or private zone. +type DNSZone struct { + // ProjectID is the project where the zone resides. + // +optional + ProjectID string `json:"projectID,omitempty"` + + // Name is the name of the dns-managed zone. + Name string `json:"name"` +} + // Platform stores all the global configuration that all machinesets // use. type Platform struct { @@ -65,6 +85,11 @@ type Platform struct { // There must be only one ServiceEndpoint for a service. // +optional ServiceEndpoints []configv1.GCPServiceEndpoint `json:"serviceEndpoints,omitempty"` + + // DNS contains the dns zone information for the cluster. The DNS information can + // only be supplied during Shared VPC (XPN) installs. + // +optional + DNS *DNS `json:"dns,omitempty"` } // UserLabel is a label to apply to GCP resources created for the cluster. diff --git a/pkg/types/gcp/validation/platform.go b/pkg/types/gcp/validation/platform.go index d2681bd924e..65dacf5b4fc 100644 --- a/pkg/types/gcp/validation/platform.go +++ b/pkg/types/gcp/validation/platform.go @@ -130,6 +130,15 @@ func ValidatePlatform(p *gcp.Platform, fldPath *field.Path, ic *types.InstallCon allErrs = append(allErrs, field.Required(fldPath.Child("network"), "must provide a VPC network when supplying subnets")) } + if p.DNS != nil && p.DNS.PrivateZone != nil { + if p.NetworkProjectID == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("dns").Child("privateZone"), "must provide a network project id when a private dns zone is specified")) + } + if p.DNS.PrivateZone.Name == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("dns").Child("privateZone").Child("name"), "must provide a zone id when a private dns zone is specified")) + } + } + // check if configured userLabels are valid. allErrs = append(allErrs, validateUserLabels(p.UserLabels, fldPath.Child("userLabels"))...) diff --git a/pkg/types/gcp/validation/platform_test.go b/pkg/types/gcp/validation/platform_test.go index a7fb8ac619c..008b8cd8666 100644 --- a/pkg/types/gcp/validation/platform_test.go +++ b/pkg/types/gcp/validation/platform_test.go @@ -155,6 +155,60 @@ func TestValidatePlatform(t *testing.T) { credentialsMode: types.MintCredentialsMode, valid: false, }, + { + name: "GCP missing network project with private zone", + platform: &gcp.Platform{ + Region: "us-east1", + ProjectID: "valid-project", + Network: "valid-vpc", + ComputeSubnet: "valid-compute-subnet", + ControlPlaneSubnet: "valid-cp-subnet", + DNS: &gcp.DNS{ + PrivateZone: &gcp.DNSZone{ + Name: "test-private-zone-name", + }, + }, + }, + credentialsMode: types.PassthroughCredentialsMode, + valid: false, + }, + { + name: "GCP missing Zone with private zone", + platform: &gcp.Platform{ + Region: "us-east1", + NetworkProjectID: "valid-network-project", + ProjectID: "valid-project", + Network: "valid-vpc", + ComputeSubnet: "valid-compute-subnet", + ControlPlaneSubnet: "valid-cp-subnet", + DNS: &gcp.DNS{ + PrivateZone: &gcp.DNSZone{ + ProjectID: "valid-project", + }, + }, + }, + credentialsMode: types.PassthroughCredentialsMode, + valid: false, + }, + { + name: "GCP valid private zone", + platform: &gcp.Platform{ + Region: "us-east1", + NetworkProjectID: "valid-network-project", + ProjectID: "valid-project", + Network: "valid-vpc", + ComputeSubnet: "valid-compute-subnet", + ControlPlaneSubnet: "valid-cp-subnet", + DNS: &gcp.DNS{ + PrivateZone: &gcp.DNSZone{ + ProjectID: "valid-project", + Name: "test-private-zone-name", + }, + }, + }, + credentialsMode: types.PassthroughCredentialsMode, + valid: true, + }, { name: "invalid gcp endpoint blank name", platform: &gcp.Platform{