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.

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

CORS-2271: IBMCloud: Add DNS Service support - installconfig #6255

Merged
merged 2 commits into from
Aug 30, 2022
+1,097 −57
Merged

CORS-2271: IBMCloud: Add DNS Service support - installconfig #6255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
edad6a5
IBMCloud: Add DNS Service support - installconfig
cjschaef Aug 17, 2022
13a20a7
IBMCloud: Add DNS Service support - vendor
cjschaef Aug 29, 2022
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
126 changes: 106 additions & 20 deletions pkg/asset/installconfig/ibmcloud/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,20 @@ import (
"fmt"
"net/http"
"os"
"strings"
"time"

"github.com/IBM/go-sdk-core/v5/core"
"github.com/IBM/networking-go-sdk/dnsrecordsv1"
"github.com/IBM/networking-go-sdk/dnszonesv1"
"github.com/IBM/networking-go-sdk/zonesv1"
"github.com/IBM/platform-services-go-sdk/iamidentityv1"
"github.com/IBM/platform-services-go-sdk/resourcecontrollerv2"
"github.com/IBM/platform-services-go-sdk/resourcemanagerv2"
"github.com/IBM/vpc-go-sdk/vpcv1"
"github.com/pkg/errors"

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

//go:generate mockgen -source=./client.go -destination=./mock/ibmcloudclient_generated.go -package=mock
Expand All @@ -23,11 +27,12 @@ import (
type API interface {
GetAuthenticatorAPIKeyDetails(ctx context.Context) (*iamidentityv1.APIKey, error)
GetCISInstance(ctx context.Context, crnstr string) (*resourcecontrollerv2.ResourceInstance, error)
GetDNSInstance(ctx context.Context, crnstr string) (*resourcecontrollerv2.ResourceInstance, error)
GetDedicatedHostByName(ctx context.Context, name string, region string) (*vpcv1.DedicatedHost, error)
GetDedicatedHostProfiles(ctx context.Context, region string) ([]vpcv1.DedicatedHostProfile, error)
GetDNSRecordsByName(ctx context.Context, crnstr string, zoneID string, recordName string) ([]dnsrecordsv1.DnsrecordDetails, error)
GetDNSZoneIDByName(ctx context.Context, name string) (string, error)
GetDNSZones(ctx context.Context) ([]DNSZoneResponse, error)
GetDNSZoneIDByName(ctx context.Context, name string, publish types.PublishingStrategy) (string, error)
GetDNSZones(ctx context.Context, publish types.PublishingStrategy) ([]DNSZoneResponse, error)
GetEncryptionKey(ctx context.Context, keyCRN string) (*EncryptionKeyResponse, error)
GetResourceGroups(ctx context.Context) ([]resourcemanagerv2.ResourceGroup, error)
GetResourceGroup(ctx context.Context, nameOrID string) (*resourcemanagerv2.ResourceGroup, error)
Expand All @@ -49,8 +54,20 @@ type Client struct {
vpcAPI *vpcv1.VpcV1
}

// cisServiceID is the Cloud Internet Services' catalog service ID.
const cisServiceID = "75874a60-cb12-11e7-948e-37ac098eb1b9"
// InstanceType is the IBM Cloud network services type being used
type InstanceType string

const (
// CISInstanceType is a Cloud Internet Services InstanceType
CISInstanceType InstanceType = "CIS"
// DNSInstanceType is a DNS Services InstanceType
DNSInstanceType InstanceType = "DNS"

// cisServiceID is the Cloud Internet Services' catalog service ID.
cisServiceID = "75874a60-cb12-11e7-948e-37ac098eb1b9"
// dnsServiceID is the DNS Services' catalog service ID.
dnsServiceID = "b4ed8a30-936f-11e9-b289-1d079699cbe5"
)

// VPCResourceNotFoundError represents an error for a VPC resoruce that is not found.
type VPCResourceNotFoundError struct{}
Expand All @@ -68,15 +85,19 @@ type DNSZoneResponse struct {
// ID is the zone's ID.
ID string

// CISInstanceCRN is the IBM Cloud Resource Name for the CIS instance where
// InstanceID is the IBM Cloud Resource ID for the service instance where
// the DNS zone is managed.
CISInstanceCRN string
InstanceID string

// CISInstanceName is the display name of the CIS instance where the DNS zone
// InstanceCRN is the IBM Cloud Resource CRN for the service instance where
// the DNS zone is managed.
InstanceCRN string

// InstanceName is the display name of the service instance where the DNS zone
// is managed.
CISInstanceName string
InstanceName string

// ResourceGroupID is the resource group ID of the CIS instance.
// ResourceGroupID is the resource group ID of the service instance.
ResourceGroupID string
}

Expand Down Expand Up @@ -138,20 +159,30 @@ func (c *Client) GetAuthenticatorAPIKeyDetails(ctx context.Context) (*iamidentit
return details, nil
}

// GetCISInstance gets a specific Cloud Internet Services instance by its CRN.
func (c *Client) GetCISInstance(ctx context.Context, crnstr string) (*resourcecontrollerv2.ResourceInstance, error) {
// getInstance gets a specific DNS or CIS instance by its CRN.
func (c *Client) getInstance(ctx context.Context, crnstr string, iType InstanceType) (*resourcecontrollerv2.ResourceInstance, error) {
_, cancel := context.WithTimeout(ctx, 1*time.Minute)
defer cancel()

options := c.controllerAPI.NewGetResourceInstanceOptions(crnstr)
resourceInstance, _, err := c.controllerAPI.GetResourceInstance(options)
if err != nil {
return nil, errors.Wrap(err, "failed to get cis instances")
return nil, errors.Wrapf(err, "failed to get %s instances", iType)
}

return resourceInstance, nil
}

// GetCISInstance gets a specific Cloud Internet Services by its CRN.
func (c *Client) GetCISInstance(ctx context.Context, crnstr string) (*resourcecontrollerv2.ResourceInstance, error) {
return c.getInstance(ctx, crnstr, CISInstanceType)
}

// GetDNSInstance gets a specific DNS Services instance by its CRN.
func (c *Client) GetDNSInstance(ctx context.Context, crnstr string) (*resourcecontrollerv2.ResourceInstance, error) {
return c.getInstance(ctx, crnstr, DNSInstanceType)
}

// GetDedicatedHostByName gets dedicated host by name.
func (c *Client) GetDedicatedHostByName(ctx context.Context, name string, region string) (*vpcv1.DedicatedHost, error) {
err := c.SetVPCServiceURLForRegion(ctx, region)
Expand Down Expand Up @@ -218,10 +249,9 @@ func (c *Client) GetDNSRecordsByName(ctx context.Context, crnstr string, zoneID
return records.Result, nil
}

// GetDNSZoneIDByName gets the CIS zone ID from its domain name.
func (c *Client) GetDNSZoneIDByName(ctx context.Context, name string) (string, error) {

zones, err := c.GetDNSZones(ctx)
// GetDNSZoneIDByName gets the DNS (Internal) or CIS zone ID from its domain name.
func (c *Client) GetDNSZoneIDByName(ctx context.Context, name string, publish types.PublishingStrategy) (string, error) {
zones, err := c.GetDNSZones(ctx, publish)
if err != nil {
return "", err
}
Expand All @@ -235,11 +265,66 @@ func (c *Client) GetDNSZoneIDByName(ctx context.Context, name string) (string, e
return "", fmt.Errorf("DNS zone %q not found", name)
}

// GetDNSZones returns all of the active DNS zones managed by CIS.
func (c *Client) GetDNSZones(ctx context.Context) ([]DNSZoneResponse, error) {
// GetDNSZones returns all of the active DNS zones managed by DNS or CIS.
func (c *Client) GetDNSZones(ctx context.Context, publish types.PublishingStrategy) ([]DNSZoneResponse, error) {
_, cancel := context.WithTimeout(ctx, 1*time.Minute)
defer cancel()

if publish == types.InternalPublishingStrategy {
return c.getDNSDNSZones(ctx)
}
return c.getCISDNSZones(ctx)
}

func (c *Client) getDNSDNSZones(ctx context.Context) ([]DNSZoneResponse, error) {
options := c.controllerAPI.NewListResourceInstancesOptions()
options.SetResourceID(dnsServiceID)

listResourceInstancesResponse, _, err := c.controllerAPI.ListResourceInstances(options)
if err != nil {
return nil, errors.Wrap(err, "failed to get dns instance")
}

var allZones []DNSZoneResponse
for _, instance := range listResourceInstancesResponse.Resources {
authenticator, err := NewIamAuthenticator(c.APIKey)
if err != nil {
return nil, err
}
dnsZoneService, err := dnszonesv1.NewDnsZonesV1(&dnszonesv1.DnsZonesV1Options{
Authenticator: authenticator,
})
if err != nil {
return nil, errors.Wrap(err, "failed to list DNS zones")
}

options := dnsZoneService.NewListDnszonesOptions(*instance.GUID)
result, _, err := dnsZoneService.ListDnszones(options)
if result == nil {
return nil, err
}

for _, zone := range result.Dnszones {
stateLower := strings.ToLower(*zone.State)
// DNS Zones can be 'pending_network_add' (without a permitted network, added during TF)
if stateLower == dnszonesv1.Dnszone_State_Active || stateLower == dnszonesv1.Dnszone_State_PendingNetworkAdd {
zoneStruct := DNSZoneResponse{
Name: *zone.Name,
ID: *zone.ID,
InstanceID: *instance.GUID,
InstanceCRN: *instance.CRN,
InstanceName: *instance.Name,
ResourceGroupID: *instance.ResourceGroupID,
}
allZones = append(allZones, zoneStruct)
}
}
}

return allZones, nil
}

func (c *Client) getCISDNSZones(ctx context.Context) ([]DNSZoneResponse, error) {
options := c.controllerAPI.NewListResourceInstancesOptions()
options.SetResourceID(cisServiceID)

Expand Down Expand Up @@ -275,8 +360,9 @@ func (c *Client) GetDNSZones(ctx context.Context) ([]DNSZoneResponse, error) {
zoneStruct := DNSZoneResponse{
Name: *zone.Name,
ID: *zone.ID,
CISInstanceCRN: *instance.CRN,
CISInstanceName: *instance.Name,
InstanceID: *instance.GUID,
InstanceCRN: *instance.CRN,
InstanceName: *instance.Name,
ResourceGroupID: *instance.ResourceGroupID,
}
allZones = append(allZones, zoneStruct)
Expand Down
14 changes: 10 additions & 4 deletions pkg/asset/installconfig/ibmcloud/dns.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ import (
survey "github.com/AlecAivazis/survey/v2"
"github.com/AlecAivazis/survey/v2/core"
"github.com/pkg/errors"

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

// Zone represents a DNS Zone
type Zone struct {
Name string
CISInstanceCRN string
ID string
InstanceCRN string
ResourceGroupID string
}

Expand All @@ -27,7 +30,9 @@ func GetDNSZone() (*Zone, error) {
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
defer cancel()

publicZones, err := client.GetDNSZones(ctx)
// IBM Cloud defaults to External (CIS) publish strategy during domain query
// TODO(cjschaef): Consider also offering Internal (DNS) based domains as well
publicZones, err := client.GetDNSZones(ctx, types.ExternalPublishingStrategy)
if err != nil {
return nil, errors.Wrap(err, "could not retrieve base domains")
}
Expand All @@ -38,10 +43,11 @@ func GetDNSZone() (*Zone, error) {
var options []string
var optionToZoneMap = make(map[string]*Zone, len(publicZones))
for _, zone := range publicZones {
option := fmt.Sprintf("%s (%s)", zone.Name, zone.CISInstanceName)
option := fmt.Sprintf("%s (%s)", zone.Name, zone.InstanceName)
optionToZoneMap[option] = &Zone{
Name: zone.Name,
CISInstanceCRN: zone.CISInstanceCRN,
ID: zone.ID,
InstanceCRN: zone.InstanceCRN,
ResourceGroupID: zone.ResourceGroupID,
}
options = append(options, option)
Expand Down
52 changes: 50 additions & 2 deletions pkg/asset/installconfig/ibmcloud/metadata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"sync"

"github.com/IBM/go-sdk-core/v5/core"

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

// Metadata holds additional metadata for InstallConfig resources that
Expand All @@ -22,11 +24,18 @@ type Metadata struct {
client *Client
computeSubnets map[string]Subnet
controlPlaneSubnets map[string]Subnet
dnsInstance *DNSInstance

mutex sync.Mutex
clientMutex sync.Mutex
}

// DNSInstance holds information for a DNS Services instance
type DNSInstance struct {
ID string
CRN string
}

// NewMetadata initializes a new Metadata object.
func NewMetadata(baseDomain string, region string, controlPlaneSubnets []string, computeSubnets []string) *Metadata {
return &Metadata{
Expand Down Expand Up @@ -71,14 +80,14 @@ func (m *Metadata) CISInstanceCRN(ctx context.Context) (string, error) {
return "", err
}

zones, err := client.GetDNSZones(ctx)
zones, err := client.GetDNSZones(ctx, types.ExternalPublishingStrategy)
if err != nil {
return "", err
}

for _, z := range zones {
if z.Name == m.BaseDomain {
m.SetCISInstanceCRN(z.CISInstanceCRN)
m.SetCISInstanceCRN(z.InstanceCRN)
return m.cisInstanceCRN, nil
}
}
Expand All @@ -92,6 +101,45 @@ func (m *Metadata) SetCISInstanceCRN(crn string) {
m.cisInstanceCRN = crn
}

// DNSInstance returns a DNSInstance holding information about the DNS Services instance
// managing the DNS zone for the base domain.
func (m *Metadata) DNSInstance(ctx context.Context) (*DNSInstance, error) {
if m.dnsInstance != nil {
return m.dnsInstance, nil
}
Comment on lines +107 to +109
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to allow access of this variable outside the protection of the mutex?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that is acceptable, since if the value isn't set, it get locked by the mutex after this check, setting it, or waiting for another thread to set it and then rechecking if it is set.

I thought this method was potentially best given the last time I ran into this when fixing the deadlock bug, per this review comment
#6241 (review)

Not opposed to making a change, but just wasn't sure which path is safer/best.


m.mutex.Lock()
defer m.mutex.Unlock()

// Prevent multiple attempts to retrieve (set) the dnsInstance if it hasn't been set (multiple threads reach mutex concurrently)
if m.dnsInstance == nil {
client, err := m.Client()
if err != nil {
return nil, err
}

zones, err := client.GetDNSZones(ctx, types.InternalPublishingStrategy)
if err != nil {
return nil, err
}

for _, z := range zones {
if z.Name == m.BaseDomain {
if z.InstanceID == "" || z.InstanceCRN == "" {
return nil, fmt.Errorf("dnsInstance has unknown ID/CRN: %q - %q", z.InstanceID, z.InstanceCRN)
}
m.dnsInstance = &DNSInstance{
ID: z.InstanceID,
CRN: z.InstanceCRN,
}
return m.dnsInstance, nil
}
}
return nil, fmt.Errorf("dnsInstance unknown due to DNS zone %q not found", m.BaseDomain)
}
return m.dnsInstance, nil
}

// ComputeSubnets gets the Subnet details for compute subnets
func (m *Metadata) ComputeSubnets(ctx context.Context) (map[string]Subnet, error) {
m.mutex.Lock()
Expand Down
Loading