Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

GCP: Remove the BYOH code #6771

Merged
merged 1 commit into from Jan 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 1 addition & 5 deletions data/data/gcp/cluster/dns/base.tf
Expand Up @@ -7,7 +7,6 @@ resource "google_dns_managed_zone" "int" {
description = local.description
dns_name = "${var.cluster_domain}."
visibility = "private"
project = var.private_zone_project

private_visibility_config {
networks {
Expand All @@ -19,13 +18,12 @@ resource "google_dns_managed_zone" "int" {
}

resource "google_dns_record_set" "api_external" {
count = var.public_endpoints && var.create_public_zone_records ? 1 : 0
count = var.public_endpoints ? 1 : 0

name = "api.${var.cluster_domain}."
type = "A"
ttl = "60"
managed_zone = var.public_zone_name
project = var.public_zone_project
rrdatas = [var.api_external_lb_ip]
}

Expand All @@ -34,7 +32,6 @@ resource "google_dns_record_set" "api_internal" {
type = "A"
ttl = "60"
managed_zone = google_dns_managed_zone.int.name
project = var.private_zone_project
rrdatas = [var.api_internal_lb_ip]
}

Expand All @@ -43,6 +40,5 @@ resource "google_dns_record_set" "api_external_internal_zone" {
type = "A"
ttl = "60"
managed_zone = google_dns_managed_zone.int.name
project = var.private_zone_project
rrdatas = [var.api_internal_lb_ip]
}
14 changes: 0 additions & 14 deletions data/data/gcp/cluster/dns/variables.tf
Expand Up @@ -33,17 +33,3 @@ variable "public_endpoints" {
description = "If the cluster should have externally accessible resources."
}

variable "create_public_zone_records" {
type = bool
description = "Create records for the public managed zone."
}

variable "public_zone_project" {
type = string
description = "Project where the public managed zone will exist."
}

variable "private_zone_project" {
type = string
description = "Project where the private managed zone will exist."
}
17 changes: 7 additions & 10 deletions data/data/gcp/cluster/main.tf
Expand Up @@ -73,16 +73,13 @@ module "network" {
module "dns" {
source = "./dns"

cluster_id = var.cluster_id
private_zone_project = var.gcp_private_zone_project
create_public_zone_records = var.gcp_create_public_zone_records
public_zone_project = var.gcp_public_zone_project
public_zone_name = var.gcp_public_zone_name
network = module.network.network
cluster_domain = var.cluster_domain
api_external_lb_ip = module.network.cluster_public_ip
api_internal_lb_ip = module.network.cluster_ip
public_endpoints = local.public_endpoints
cluster_id = var.cluster_id
public_zone_name = var.gcp_public_zone_name
network = module.network.network
cluster_domain = var.cluster_domain
api_external_lb_ip = module.network.cluster_public_ip
api_internal_lb_ip = module.network.cluster_ip
public_endpoints = local.public_endpoints
}

resource "google_compute_image" "cluster" {
Expand Down
18 changes: 0 additions & 18 deletions data/data/gcp/variables-gcp.tf
Expand Up @@ -146,24 +146,6 @@ variable "gcp_create_firewall_rules" {
description = "Create the cluster's network firewall rules."
}

variable "gcp_create_public_zone_records" {
type = bool
default = true
description = "Create records for the public managed zone."
}

variable "gcp_public_zone_project" {
type = string
default = ""
description = "Project where the public managed zone will exist."
}

variable "gcp_private_zone_project" {
type = string
default = ""
description = "Project where the private managed zone will exist."
}

variable "gcp_master_secure_boot" {
type = string
description = "Verify the digital signature of all boot components."
Expand Down
30 changes: 0 additions & 30 deletions data/data/install.openshift.io_installconfigs.yaml
Expand Up @@ -2334,40 +2334,10 @@ spec:
description: NetworkProjectID specifies which project the network
and subnets exist in when they are not in the main ProjectID.
type: string
privateDNSZone:
description: PrivateDNSZone Technology Preview. PrivateDNSZone
contains the zone ID and project where the Private DNS zone
records will be created.
properties:
id:
description: ID Technology Preview. ID or name of the zone.
type: string
project:
description: ProjectID Technology Preview. When the ProjectID
is provided, the zone will exist in this project. When the
ProjectID is empty, the ProjectID defaults to the Service
Project (GCP.ProjectID).
type: string
type: object
projectID:
description: ProjectID is the the project that will be used for
the cluster.
type: string
publicDNSZone:
description: PublicDNSZone Technology Preview. PublicDNSZone contains
the zone ID and project where the Public DNS zone records will
be created.
properties:
id:
description: ID Technology Preview. ID or name of the zone.
type: string
project:
description: ProjectID Technology Preview. When the ProjectID
is provided, the zone will exist in this project. When the
ProjectID is empty, the ProjectID defaults to the Service
Project (GCP.ProjectID).
type: string
type: object
region:
description: Region specifies the GCP region where the cluster
will be created.
Expand Down
53 changes: 14 additions & 39 deletions pkg/asset/cluster/tfvars.go
Expand Up @@ -449,32 +449,10 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
}
preexistingnetwork := installConfig.Config.GCP.Network != ""

// Setup defaults for public dns zone
createPublicZoneRecords := true
publicZoneName := ""
publicZoneProject := installConfig.Config.GCP.ProjectID
if installConfig.Config.GCP.PublicDNSZone != nil && installConfig.Config.GCP.PublicDNSZone.ProjectID != "" {
publicZoneProject = installConfig.Config.GCP.PublicDNSZone.ProjectID
}

switch {
case installConfig.Config.Publish != types.ExternalPublishingStrategy:
// Do not create public records when not publishing externally.
createPublicZoneRecords = false
case installConfig.Config.GCP.PublicDNSZone != nil && installConfig.Config.GCP.PublicDNSZone.ID != "":
publicZoneName = installConfig.Config.GCP.PublicDNSZone.ID
default:
// Search the project for a dns zone with the specified base domain.
publicZone, err := gcpconfig.GetPublicZone(ctx, publicZoneProject, installConfig.Config.BaseDomain)
if err != nil {
return errors.Wrapf(err, "failed to get GCP public zone")
}
publicZoneName = publicZone.Name
}

privateZoneProject := ""
if installConfig.Config.GCP.PrivateDNSZone != nil && installConfig.Config.GCP.PrivateDNSZone.ProjectID != "" {
privateZoneProject = installConfig.Config.GCP.PrivateDNSZone.ProjectID
// Search the project for a dns zone with the specified base domain.
publicZone, err := gcpconfig.GetPublicZone(ctx, installConfig.Config.GCP.ProjectID, installConfig.Config.BaseDomain)
if err != nil {
return errors.Wrapf(err, "failed to get GCP public zone")
}

archName := coreosarch.RpmArch(string(installConfig.Config.ControlPlane.Architecture))
Expand All @@ -497,19 +475,16 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
imageURL := fmt.Sprintf("https://storage.googleapis.com/rhcos/rhcos/%s.tar.gz", img.Name)
data, err := gcptfvars.TFVars(
gcptfvars.TFVarsSources{
Auth: auth,
MasterConfigs: masterConfigs,
WorkerConfigs: workerConfigs,
CreateFirewallRules: createFirewallRules,
CreatePublicZoneRecords: createPublicZoneRecords,
ImageURI: imageURL,
ImageLicenses: installConfig.Config.GCP.Licenses,
InstanceServiceAccount: instanceServiceAccount,
PreexistingNetwork: preexistingnetwork,
PrivateZoneProject: privateZoneProject,
PublicZoneName: publicZoneName,
PublicZoneProject: publicZoneProject,
PublishStrategy: installConfig.Config.Publish,
Auth: auth,
MasterConfigs: masterConfigs,
WorkerConfigs: workerConfigs,
CreateFirewallRules: createFirewallRules,
ImageURI: imageURL,
ImageLicenses: installConfig.Config.GCP.Licenses,
InstanceServiceAccount: instanceServiceAccount,
PreexistingNetwork: preexistingnetwork,
PublicZoneName: publicZone.Name,
PublishStrategy: installConfig.Config.Publish,
},
)
if err != nil {
Expand Down
102 changes: 11 additions & 91 deletions pkg/asset/installconfig/gcp/validation.go
Expand Up @@ -4,18 +4,17 @@ import (
"context"
"fmt"
"net"
"net/http"
"strings"

"github.com/pkg/errors"
"github.com/sirupsen/logrus"
compute "google.golang.org/api/compute/v1"
"google.golang.org/api/dns/v1"
"google.golang.org/api/googleapi"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation/field"

"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/gcp"
"github.com/openshift/installer/pkg/validate"
)

Expand Down Expand Up @@ -46,8 +45,6 @@ func Validate(client API, ic *types.InstallConfig) error {
allErrs = append(allErrs, validateNetworkProject(client, ic, field.NewPath("platform").Child("gcp"))...)
allErrs = append(allErrs, validateRegion(client, ic, field.NewPath("platform").Child("gcp"))...)
allErrs = append(allErrs, validateNetworks(client, ic, field.NewPath("platform").Child("gcp"))...)
allErrs = append(allErrs, validateZoneProjects(client, ic, field.NewPath("platform").Child("gcp"))...)
allErrs = append(allErrs, validateManagedZones(client, ic, field.NewPath("platform").Child("gcp"))...)
allErrs = append(allErrs, validateInstanceTypes(client, ic)...)
allErrs = append(allErrs, validateCredentialMode(client, ic)...)

Expand Down Expand Up @@ -117,79 +114,6 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
return allErrs
}

// validateZoneProjects will validate the public and private zone projects when provided
func validateZoneProjects(client API, ic *types.InstallConfig, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

projects, err := client.GetProjects(context.TODO())
if err != nil {
return append(allErrs, field.InternalError(fieldPath.Child("project"), err))
}

// If the PublicZoneProject is empty, the value will default to ProjectID, and it won't be checked here
if ic.GCP.PublicDNSZone != nil && ic.GCP.PublicDNSZone.ProjectID != "" {
if _, found := projects[ic.GCP.PublicDNSZone.ProjectID]; !found {
allErrs = append(allErrs, field.Invalid(fieldPath.Child("PublicDNSZone").Child("ProjectID"), ic.GCP.PublicDNSZone.ProjectID, "invalid public zone project"))
}
}

if ic.GCP.PrivateDNSZone != nil && ic.GCP.PrivateDNSZone.ProjectID != "" {
if _, found := projects[ic.GCP.PrivateDNSZone.ProjectID]; !found {
allErrs = append(allErrs, field.Invalid(fieldPath.Child("PrivateDNSZone").Child("ProjectID"), ic.GCP.PrivateDNSZone.ProjectID, "invalid private zone project"))
}
}

return allErrs
}

// findProject finds the correct project to use during installation. If the project id is
// provided in the zone use the project id, otherwise use the default project.
func findProject(zone *gcp.DNSZone, defaultProject string) string {
if zone != nil && zone.ProjectID != "" {
return zone.ProjectID
}
return defaultProject
}

// findDNSZone finds a zone in a project. If a project is provided in the zone, the project
// is used otherwise the default project is used.
func findDNSZone(client API, zone *gcp.DNSZone, project string) (*dns.ManagedZone, error) {
returnedZone, err := client.GetDNSZoneByName(context.TODO(), project, zone.ID)
if err != nil {
return nil, err
}
return returnedZone, nil
}

// validateManagedZones will validate the public managed zones if they exist.
func validateManagedZones(client API, ic *types.InstallConfig, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if ic.GCP.PublicDNSZone != nil && ic.GCP.PublicDNSZone.ID != "" {
project := findProject(ic.GCP.PublicDNSZone, ic.GCP.ProjectID)
returnedZone, err := findDNSZone(client, ic.Platform.GCP.PublicDNSZone, project)
if err != nil {
switch {
case IsNotFound(err):
allErrs = append(allErrs, field.NotFound(field.NewPath("baseDomain"), errors.Wrapf(err, "dns zone (%s/%s)", project, ic.BaseDomain).Error()))
case IsForbidden(err):
errMsg := errors.Wrapf(err, "unable to fetch public dns zone information: %s", ic.BaseDomain).Error()
allErrs = append(allErrs, field.Invalid(fieldPath.Child("publicManagedZone"), ic.GCP.PublicDNSZone.ID, errMsg))
default:
allErrs = append(allErrs, field.InternalError(field.NewPath("baseDomain"), err))
}
} else {
// verify that the managed zone exists in the BaseDomain - Trim both values just in case
if !strings.EqualFold(strings.TrimSuffix(returnedZone.DnsName, "."), strings.TrimSuffix(ic.BaseDomain, ".")) {
errMsg := fmt.Sprintf("publicDNSZone does not exist in baseDomain %s", ic.BaseDomain)
allErrs = append(allErrs, field.Invalid(fieldPath.Child("publicDNSZone").Child("id"), ic.Platform.GCP.PublicDNSZone.ID, errMsg))
}
}
}

return allErrs
}

// ValidatePreExistingPublicDNS ensure no pre-existing DNS record exists in the public
// 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
Expand All @@ -201,30 +125,26 @@ func ValidatePreExistingPublicDNS(client API, ic *types.InstallConfig) error {
}

record := fmt.Sprintf("api.%s.", strings.TrimSuffix(ic.ClusterDomain(), "."))
project := findProject(ic.GCP.PublicDNSZone, ic.GCP.ProjectID)
zoneName := ""

if ic.GCP.PublicDNSZone != nil && ic.GCP.PublicDNSZone.ID != "" {
zoneName = ic.GCP.PublicDNSZone.ID
}

if zoneName == "" {
zone, err := client.GetPublicDNSZone(context.TODO(), project, ic.BaseDomain)
if err != nil {
return errors.Wrapf(err, "failed to find public zone in project %s", project)
zone, err := client.GetPublicDNSZone(context.TODO(), ic.Platform.GCP.ProjectID, ic.BaseDomain)
if err != nil {
var gErr *googleapi.Error
if errors.As(err, &gErr) {
if gErr.Code == http.StatusNotFound {
return field.NotFound(field.NewPath("baseDomain"), fmt.Sprintf("DNS Zone (%s/%s)", ic.Platform.GCP.ProjectID, ic.BaseDomain))
}
}

zoneName = zone.Name
return field.InternalError(field.NewPath("baseDomain"), err)
}

rrSets, err := client.GetRecordSets(context.TODO(), project, zoneName)
rrSets, err := client.GetRecordSets(context.TODO(), ic.GCP.ProjectID, zone.Name)
if err != nil {
return field.InternalError(field.NewPath("baseDomain"), err)
}

for _, r := range rrSets {
if strings.EqualFold(r.Name, record) {
errMsg := fmt.Sprintf("record %s already exists in DNS Zone (%s/%s) and might be in use by another cluster, please remove it to continue", record, project, zoneName)
errMsg := fmt.Sprintf("record %s already exists in DNS Zone (%s/%s) and might be in use by another cluster, please remove it to continue", record, ic.GCP.ProjectID, zone.Name)
return field.Invalid(field.NewPath("metadata", "name"), ic.ObjectMeta.Name, errMsg)
}
}
Expand Down