Skip to content

CORS-2280: IBMCloud: Add TF support for private DNS #6282

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-robot merged 1 commit into from
Sep 7, 2022
Merged

CORS-2280: IBMCloud: Add TF support for private DNS #6282

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
2 changes: 1 addition & 1 deletion data/data/ibmcloud/bootstrap/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ resource "ibm_is_security_group" "bootstrap" {
resource "ibm_is_security_group_rule" "bootstrap_ssh_inbound" {
group = ibm_is_security_group.bootstrap.id
direction = "inbound"
remote = local.public_endpoints ? "0.0.0.0/0" : var.control_plane_security_group_id_list.0.id
remote = local.public_endpoints ? "0.0.0.0/0" : var.control_plane_security_group_id_list[0]
Copy link
Contributor

@barbacbd barbacbd Sep 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any other instance of this variable changed. Is this missing id or did the type change?

Copy link
Member Author

@cjschaef cjschaef Sep 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually a bug fix, for a bug that was never noticed until we started testing Internal, which hasn't been supported until this and the previous CORS-2280 PR for Golang changes.

For instance, using this PR with a revert of the line above, Terraform fails since the control_plane_security_group_id_list is a list of Id's (strings) already, not SecurityGroup data sources.

# go/src/github.com/openshift/installer/bin/openshift-install version
go/src/github.com/openshift/installer/bin/openshift-install unreleased-master-6395-g75c6ab06c034cfc841b672b936a6a750930606d7
built from commit 75c6ab06c034cfc841b672b936a6a750930606d7
release image registry.ci.openshift.org/origin/release:4.12
release architecture amd64

# git -C go/src/github.com/openshift/installer/ diff cjschaef/ibmcloud_private_terraform..75c6ab06c034cfc841b672b936a6a750930606d7
diff --git a/data/data/ibmcloud/bootstrap/main.tf b/data/data/ibmcloud/bootstrap/main.tf
index 4471b3325..2f0aaea6a 100644
--- a/data/data/ibmcloud/bootstrap/main.tf
+++ b/data/data/ibmcloud/bootstrap/main.tf
@@ -67,7 +67,7 @@ resource "ibm_is_security_group" "bootstrap" {
 resource "ibm_is_security_group_rule" "bootstrap_ssh_inbound" {
   group     = ibm_is_security_group.bootstrap.id
   direction = "inbound"
-  remote    = local.public_endpoints ? "0.0.0.0/0" : var.control_plane_security_group_id_list[0]
+  remote    = local.public_endpoints ? "0.0.0.0/0" : var.control_plane_security_group_id_list.0.id
   tcp {
     port_min = 22
     port_max = 22


# go/src/github.com/openshift/installer/bin/openshift-install create cluster --dir clusters/private-sg-id-list-1/
WARNING Found override for release image. Please be warned, this is not advised 
INFO Consuming Worker Machines from target directory 
INFO Consuming OpenShift Install (Manifests) from target directory 
INFO Consuming Master Machines from target directory 
INFO Consuming Common Manifests from target directory 
INFO Consuming Openshift Manifests from target directory 
INFO Obtaining RHCOS image file from 'https://rhcos.mirror.openshift.com/art/storage/releases/rhcos-4.12/412.86.202208101039-0/x86_64/rhcos-412.86.202208101039-0-ibmcloud.x86_64.qcow2.gz?sha256=09b599849b945bdd405b18765225160f50e07ca205fe9787f70f188c8a96f293' 
INFO The file was found in cache: /root/.cache/openshift-installer/image_cache/rhcos-412.86.202208101039-0-ibmcloud.x86_64.qcow2. Reusing... 
INFO Creating infrastructure resources...         
ERROR                                              
ERROR Error: Unsupported attribute                 
ERROR                                              
ERROR   on main.tf line 70, in resource "ibm_is_security_group_rule" "bootstrap_ssh_inbound": 
ERROR   70:   remote    = local.public_endpoints ? "0.0.0.0/0" : var.control_plane_security_group_id_list.0.id 
ERROR     ├────────────────                        
ERROR     │ var.control_plane_security_group_id_list[0] is "r018-a45f4ff4-b452-44a7-bc12-72bc0e424644" 
ERROR                                              
ERROR Can't access attributes on a primitive-typed value (string). 
ERROR failed to fetch Cluster: failed to generate asset "Cluster": failure applying terraform for "bootstrap" stage: failed to create cluster: failed to apply Terraform: exit status 1 
ERROR                                              
ERROR Error: Unsupported attribute                 
ERROR                                              
ERROR   on main.tf line 70, in resource "ibm_is_security_group_rule" "bootstrap_ssh_inbound": 
ERROR   70:   remote    = local.public_endpoints ? "0.0.0.0/0" : var.control_plane_security_group_id_list.0.id 
ERROR     ├────────────────                        
ERROR     │ var.control_plane_security_group_id_list[0] is "r018-a45f4ff4-b452-44a7-bc12-72bc0e424644" 
ERROR                                              
ERROR Can't access attributes on a primitive-typed value (string).

tcp {
port_min = 22
port_max = 22
Expand Down
10 changes: 8 additions & 2 deletions data/data/ibmcloud/network/cis/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
############################################

data "ibm_cis_domain" "base_domain" {
count = var.is_external ? 1 : 0

cis_id = var.cis_id
domain = var.base_domain
}
Expand All @@ -12,17 +14,21 @@ data "ibm_cis_domain" "base_domain" {
############################################

resource "ibm_cis_dns_record" "kubernetes_api" {
count = var.is_external ? 1 : 0

cis_id = var.cis_id
domain_id = data.ibm_cis_domain.base_domain.id
domain_id = data.ibm_cis_domain.base_domain[0].id
type = "CNAME"
name = "api.${var.cluster_domain}"
content = var.lb_kubernetes_api_public_hostname != "" ? var.lb_kubernetes_api_public_hostname : var.lb_kubernetes_api_private_hostname
ttl = 60
}

resource "ibm_cis_dns_record" "kubernetes_api_internal" {
count = var.is_external ? 1 : 0

cis_id = var.cis_id
domain_id = data.ibm_cis_domain.base_domain.id
domain_id = data.ibm_cis_domain.base_domain[0].id
type = "CNAME"
name = "api-int.${var.cluster_domain}"
content = var.lb_kubernetes_api_private_hostname
Expand Down
4 changes: 4 additions & 0 deletions data/data/ibmcloud/network/cis/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ variable "cluster_domain" {
type = string
}

variable "is_external" {
type = bool
}

variable "lb_kubernetes_api_public_hostname" {
type = string
}
Expand Down
13 changes: 13 additions & 0 deletions data/data/ibmcloud/network/dns/common.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
locals {
dns_zone_id = var.is_external ? "" : data.ibm_dns_zones.zones[0].dns_zones[index(data.ibm_dns_zones.zones[0].dns_zones[*].name, var.base_domain)].zone_id
}

############################################
# DNS Zone
############################################

data "ibm_dns_zones" "zones" {
count = var.is_external ? 0 : 1

instance_id = var.dns_id
}
27 changes: 27 additions & 0 deletions data/data/ibmcloud/network/dns/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
############################################
# DNS permitted networks
############################################

resource "ibm_dns_permitted_network" "vpc" {
count = var.is_external ? 0 : 1

instance_id = var.dns_id
zone_id = local.dns_zone_id
vpc_crn = var.vpc_crn
type = "vpc"
}

############################################
# DNS records (CNAME)
############################################

resource "ibm_dns_resource_record" "kubernetes_api_private" {
count = var.is_external ? 0 : 1

instance_id = var.dns_id
zone_id = local.dns_zone_id
type = "CNAME"
name = "api-int.${var.cluster_domain}"
rdata = var.lb_kubernetes_api_private_hostname
ttl = "60"
}
27 changes: 27 additions & 0 deletions data/data/ibmcloud/network/dns/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
############################################
# DNS module variables
############################################

variable "dns_id" {
type = string
}

variable "vpc_crn" {
type = string
}

variable "base_domain" {
type = string
}

variable "cluster_domain" {
type = string
}

variable "is_external" {
type = bool
}

variable "lb_kubernetes_api_private_hostname" {
type = string
}
18 changes: 18 additions & 0 deletions data/data/ibmcloud/network/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,29 @@ module "cis" {
cis_id = var.ibmcloud_cis_crn
base_domain = var.base_domain
cluster_domain = var.cluster_domain
is_external = local.public_endpoints
Copy link
Contributor

@barbacbd barbacbd Sep 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider passing this in as a boolean to the ibm-variables where the logic is moved to tfvars in the installer.

Copy link
Member Author

@cjschaef cjschaef Sep 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The boolean is based on PublishStrategy currently, and just passed around after that.
If it makes sense to add boolean version of the PublishStrategy (basically) to tfvars that gets used in place of that Internel/External string, I can do that in a follow up PR.


lb_kubernetes_api_public_hostname = module.vpc.lb_kubernetes_api_public_hostname
lb_kubernetes_api_private_hostname = module.vpc.lb_kubernetes_api_private_hostname
}

############################################
# DNS module
############################################

module "dns" {
source = "./dns"
depends_on = [module.vpc]

dns_id = var.ibmcloud_dns_id
vpc_crn = module.vpc.vpc_crn
base_domain = var.base_domain
cluster_domain = var.cluster_domain
is_external = local.public_endpoints
Copy link
Contributor

@barbacbd barbacbd Sep 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider making is_external more descriptive.

Copy link
Member Author

@cjschaef cjschaef Sep 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll see about following up this PR with some more descriptive


lb_kubernetes_api_private_hostname = module.vpc.lb_kubernetes_api_private_hostname
}

############################################
# Dedicated Host module
############################################
Expand Down
5 changes: 4 additions & 1 deletion data/data/ibmcloud/network/vpc/common.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ locals {
# Common locals
prefix = var.cluster_id
zones_all = distinct(concat(var.zones_master, var.zones_worker))
vpc_id = var.preexisting_vpc ? data.ibm_is_vpc.vpc[0].id : ibm_is_vpc.vpc[0].id

# VPC locals
vpc_id = var.preexisting_vpc ? data.ibm_is_vpc.vpc[0].id : ibm_is_vpc.vpc[0].id
vpc_crn = var.preexisting_vpc ? data.ibm_is_vpc.vpc[0].crn : ibm_is_vpc.vpc[0].crn

# LB locals
port_kubernetes_api = 6443
Expand Down
4 changes: 4 additions & 0 deletions data/data/ibmcloud/network/vpc/outputs.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,7 @@ output "lb_pool_machine_config_id" {
output "vpc_id" {
value = local.vpc_id
}

output "vpc_crn" {
value = local.vpc_crn
}
7 changes: 7 additions & 0 deletions data/data/ibmcloud/variables-ibmcloud.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ variable "ibmcloud_bootstrap_instance_type" {
variable "ibmcloud_cis_crn" {
type = string
description = "The CRN of CIS instance to use."
default = ""
}

variable "ibmcloud_dns_id" {
type = string
description = "The ID of DNS Service instance to use."
default = ""
}

variable "ibmcloud_region" {
Expand Down
8 changes: 8 additions & 0 deletions pkg/asset/cluster/ibmcloud/ibmcloud.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ import (
func Metadata(infraID string, config *types.InstallConfig, meta *icibmcloud.Metadata) *ibmcloud.Metadata {
accountID, _ := meta.AccountID(context.TODO())
cisCrn, _ := meta.CISInstanceCRN(context.TODO())
dnsInstance, _ := meta.DNSInstance(context.TODO())

var dnsInstanceID string
if dnsInstance != nil {
dnsInstanceID = dnsInstance.ID
}

subnets := []string{}
controlPlaneSubnets, _ := meta.ControlPlaneSubnets(context.TODO())
for id := range controlPlaneSubnets {
Expand All @@ -30,6 +37,7 @@ func Metadata(infraID string, config *types.InstallConfig, meta *icibmcloud.Meta
AccountID: accountID,
BaseDomain: config.BaseDomain,
CISInstanceCRN: cisCrn,
DNSInstanceID: dnsInstanceID,
Region: config.Platform.IBMCloud.Region,
ResourceGroupName: config.Platform.IBMCloud.ClusterResourceGroupName(infraID),
Subnets: subnets,
Expand Down
24 changes: 19 additions & 5 deletions pkg/asset/cluster/tfvars.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,16 +526,30 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
}
}

// Get CISInstanceCRN from InstallConfig metadata
crn, err := installConfig.IBMCloud.CISInstanceCRN(ctx)
if err != nil {
return err
var cisCRN, dnsID string

if installConfig.Config.Publish == types.InternalPublishingStrategy {
// Get DNSInstanceCRN from InstallConfig metadata
dnsInstance, err := installConfig.IBMCloud.DNSInstance(ctx)
if err != nil {
return err
}
if dnsInstance != nil {
dnsID = dnsInstance.ID
}
} else {
// Get CISInstanceCRN from InstallConfig metadata
cisCRN, err = installConfig.IBMCloud.CISInstanceCRN(ctx)
if err != nil {
return err
}
}

data, err = ibmcloudtfvars.TFVars(
ibmcloudtfvars.TFVarsSources{
Auth: auth,
CISInstanceCRN: crn,
CISInstanceCRN: cisCRN,
DNSInstanceID: dnsID,
ImageURL: string(*rhcosImage),
MasterConfigs: masterConfigs,
MasterDedicatedHosts: masterDedicatedHosts,
Expand Down
3 changes: 3 additions & 0 deletions pkg/tfvars/ibmcloud/ibmcloud.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type config struct {
Region string `json:"ibmcloud_region,omitempty"`
BootstrapInstanceType string `json:"ibmcloud_bootstrap_instance_type,omitempty"`
CISInstanceCRN string `json:"ibmcloud_cis_crn,omitempty"`
DNSInstanceID string `json:"ibmcloud_dns_id,omitempty"`
ExtraTags []string `json:"ibmcloud_extra_tags,omitempty"`
MasterAvailabilityZones []string `json:"ibmcloud_master_availability_zones"`
WorkerAvailabilityZones []string `json:"ibmcloud_worker_availability_zones"`
Expand All @@ -45,6 +46,7 @@ type config struct {
type TFVarsSources struct {
Auth Auth
CISInstanceCRN string
DNSInstanceID string
ImageURL string
MasterConfigs []*ibmcloudprovider.IBMCloudMachineProviderSpec
MasterDedicatedHosts []DedicatedHost
Expand Down Expand Up @@ -90,6 +92,7 @@ func TFVars(sources TFVarsSources) ([]byte, error) {
Auth: sources.Auth,
BootstrapInstanceType: masterConfig.Profile,
CISInstanceCRN: sources.CISInstanceCRN,
DNSInstanceID: sources.DNSInstanceID,
ImageFilePath: cachedImage,
MasterAvailabilityZones: masterAvailabilityZones,
MasterDedicatedHosts: sources.MasterDedicatedHosts,
Expand Down
3 changes: 2 additions & 1 deletion pkg/types/ibmcloud/metadata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ package ibmcloud
type Metadata struct {
AccountID string `json:"accountID"`
BaseDomain string `json:"baseDomain"`
CISInstanceCRN string `json:"cisInstanceCRN"`
CISInstanceCRN string `json:"cisInstanceCRN,omitempty"`
DNSInstanceID string `json:"dnsInstanceID,omitempty"`
Region string `json:"region,omitempty"`
ResourceGroupName string `json:"resourceGroupName,omitempty"`
VPC string `json:"vpc,omitempty"`
Expand Down