CORS-3691: Migrating Custom DNS ignition editing to a common function

** Migrating Custom DNS	ignition editing to a common function for AWS and GCP
** Add AWS ignition editing for custom DNS. This will extract the dns names
from capa, convert the dns names to ip addresses, add the ip addresses to the config (for later use),
and add the ip addresses to the ignition file.

cmd/openshift-install/create.go

@@ -49,6 +49,7 @@ import (
 	destroybootstrap "github.com/openshift/installer/pkg/destroy/bootstrap"
 	"github.com/openshift/installer/pkg/gather/service"
 	timer "github.com/openshift/installer/pkg/metrics/timer"
+	"github.com/openshift/installer/pkg/types/aws"
 	"github.com/openshift/installer/pkg/types/baremetal"
 	"github.com/openshift/installer/pkg/types/dns"
 	"github.com/openshift/installer/pkg/types/gcp"
@@ -891,6 +892,10 @@ func handleUnreachableAPIServer(ctx context.Context, config *rest.Config) error
 		return fmt.Errorf("failed to fetch %s: %w", installConfig.Name(), err)
 	}
 	switch installConfig.Config.Platform.Name() { //nolint:gocritic
+	case aws.Name:
+		if installConfig.Config.AWS.UserProvisionedDNS != dns.UserProvisionedDNSEnabled {
+			return nil
+		}
 	case gcp.Name:
 		if installConfig.Config.GCP.UserProvisionedDNS != dns.UserProvisionedDNSEnabled {
 			return nil

pkg/asset/lbconfig/lbconfig.go

@@ -6,6 +6,7 @@ import (
 	"net"
 	"os"
 	"path/filepath"
+	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -142,13 +143,14 @@ func (i *Config) ParseDNSDataFromConfig(loadBalancerType LoadBalancerCategory) (
 
 	if internalData, ok := lbData["data"]; ok {
 		if lbStoredData, ok := internalData.(map[string]interface{})[string(loadBalancerType)]; ok {
-			// TODO: make this parse a comma separated string
-			parsedIP := net.ParseIP(lbStoredData.(string))
-			if parsedIP != nil {
-				ipAddresses = append(ipAddresses, parsedIP)
-			} else {
-				// assume the data is a dns entry
-				dnsNames = append(dnsNames, lbStoredData.(string))
+			candidates := strings.Split(lbStoredData.(string), ",")
+			for _, candidate := range candidates {
+				if parsedIP := net.ParseIP(candidate); parsedIP != nil {
+					ipAddresses = append(ipAddresses, parsedIP)
+				} else {
+					// assume the data is a dns entry
+					dnsNames = append(dnsNames, candidate)
+				}
 			}
 		}
 	}

pkg/infrastructure/aws/clusterapi/aws.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"github.com/openshift/installer/pkg/types/dns"
 	"strings"
 	"time"
 
@@ -16,6 +15,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/aws/aws-sdk-go/service/s3/s3manager"
 	"github.com/sirupsen/logrus"
+	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/utils/ptr"
@@ -27,11 +27,13 @@ import (
 	"github.com/openshift/installer/pkg/asset/manifests/capiutils"
 	"github.com/openshift/installer/pkg/infrastructure/clusterapi"
 	awstypes "github.com/openshift/installer/pkg/types/aws"
+	"github.com/openshift/installer/pkg/types/dns"
 )
 
 var (
 	_ clusterapi.Provider           = (*Provider)(nil)
 	_ clusterapi.PreProvider        = (*Provider)(nil)
+	_ clusterapi.IgnitionProvider   = (*Provider)(nil)
 	_ clusterapi.InfraReadyProvider = (*Provider)(nil)
 	_ clusterapi.BootstrapDestroyer = (*Provider)(nil)
 	_ clusterapi.PostDestroyer      = (*Provider)(nil)
@@ -85,6 +87,22 @@ func (*Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionInp
 	return nil
 }
 
+// Ignition edits the ignition contents to add the public and private load balancer ip addresses to the
+// infrastructure CR. The infrastructure CR is updated and added to the ignition files. CAPA creates a
+// bucket for ignition, and this ignition data will be placed in the bucket.
+func (p Provider) Ignition(ctx context.Context, in clusterapi.IgnitionInput) ([]*corev1.Secret, error) {
+	ignBytes, err := editIgnition(ctx, in)
+	if err != nil {
+		return nil, fmt.Errorf("failed to edit bootstrap ignition: %w", err)
+	}
+
+	ignSecrets := []*corev1.Secret{
+		clusterapi.IgnitionSecret(ignBytes, in.InfraID, "bootstrap"),
+		clusterapi.IgnitionSecret(in.MasterIgnData, in.InfraID, "master"),
+	}
+	return ignSecrets, nil
+}
+
 // InfraReady creates private hosted zone and DNS records.
 func (*Provider) InfraReady(ctx context.Context, in clusterapi.InfraReadyInput) error {
 	awsCluster := &capa.AWSCluster{}

pkg/infrastructure/aws/clusterapi/ignition.go

@@ -0,0 +1,72 @@
+package clusterapi
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/sirupsen/logrus"
+	capa "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
+	k8sClient "sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/openshift/installer/pkg/asset/manifests/capiutils"
+	"github.com/openshift/installer/pkg/infrastructure/clusterapi"
+	awstypes "github.com/openshift/installer/pkg/types/aws"
+	"github.com/openshift/installer/pkg/types/dns"
+)
+
+func editIgnition(ctx context.Context, in clusterapi.IgnitionInput) ([]byte, error) {
+	if in.InstallConfig.Config.AWS.UserProvisionedDNS != dns.UserProvisionedDNSEnabled {
+		return in.BootstrapIgnData, nil
+	}
+
+	awsCluster := &capa.AWSCluster{}
+	key := k8sClient.ObjectKey{
+		Name:      in.InfraID,
+		Namespace: capiutils.Namespace,
+	}
+	if err := in.Client.Get(ctx, key, awsCluster); err != nil {
+		return nil, fmt.Errorf("failed to get AWSCluster: %w", err)
+	}
+
+	awsSession, err := in.InstallConfig.AWS.Session(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get aws session: %w", err)
+	}
+
+	// There is no direct access to load balancer IP addresses, so the security groups
+	// are used here to find the network interfaces that correspond to the load balancers.
+	securityGroupIDs := make([]*string, 0, len(awsCluster.Status.Network.SecurityGroups))
+	for _, securityGroup := range awsCluster.Status.Network.SecurityGroups {
+		securityGroupIDs = append(securityGroupIDs, aws.String(securityGroup.ID))
+	}
+	nicInput := ec2.DescribeNetworkInterfacesInput{
+		Filters: []*ec2.Filter{
+			{
+				Name:   aws.String("group-id"),
+				Values: securityGroupIDs,
+			},
+		},
+	}
+	nicOutput, err := ec2.New(awsSession).DescribeNetworkInterfacesWithContext(ctx, &nicInput)
+	if err != nil {
+		return nil, fmt.Errorf("failed to describe network interfaces: %w", err)
+	}
+
+	// The only network interfaces existing at this stage are those from the load balancers.
+	// If this stage is executed after control plane nodes are provisioned, there may be
+	// other network interfaces available.
+	publicIPAddresses := []string{}
+	privateIPAddresses := []string{}
+	for _, nic := range nicOutput.NetworkInterfaces {
+		if nic.Association != nil && nic.Association.PublicIp != nil {
+			logrus.Debugf("found public IP address %s associated with %s", *nic.Association.PublicIp, *nic.Description)
+			publicIPAddresses = append(publicIPAddresses, *nic.Association.PublicIp)
+		} else if nic.PrivateIpAddress != nil {
+			logrus.Debugf("found private IP address %s associated with %s", *nic.PrivateIpAddress, *nic.Description)
+			privateIPAddresses = append(privateIPAddresses, *nic.PrivateIpAddress)
+		}
+	}
+	return clusterapi.EditIgnition(in, awstypes.Name, publicIPAddresses, privateIPAddresses)
+}

pkg/infrastructure/clusterapi/ignition.go

@@ -0,0 +1,125 @@
+package clusterapi
+
+import (
+	"encoding/base64"
+	"encoding/json"
+	"fmt"
+	"strings"
+
+	igntypes "github.com/coreos/ignition/v2/config/v3_2/types"
+	"sigs.k8s.io/yaml"
+
+	configv1 "github.com/openshift/api/config/v1"
+	"github.com/openshift/installer/cmd/openshift-install/command"
+	"github.com/openshift/installer/pkg/asset"
+	"github.com/openshift/installer/pkg/asset/lbconfig"
+	awstypes "github.com/openshift/installer/pkg/types/aws"
+	gcptypes "github.com/openshift/installer/pkg/types/gcp"
+)
+
+const (
+	infrastructureFilepath = "/opt/openshift/manifests/cluster-infrastructure-02-config.yml"
+
+	// replaceable is the string that precedes the encoded data in the ignition data.
+	// The data must be replaced before decoding the string, and the string must be
+	// prepended to the encoded data.
+	replaceable = "data:text/plain;charset=utf-8;base64,"
+)
+
+// EditIgnition attempts to edit the contents of the bootstrap ignition when the user has selected
+// a custom DNS configuration. Find the public and private load balancer addresses and fill in the
+// infrastructure file within the ignition struct.
+func EditIgnition(in IgnitionInput, platform string, publicIPAddresses, privateIPAddresses []string) ([]byte, error) {
+	ignData := &igntypes.Config{}
+	err := json.Unmarshal(in.BootstrapIgnData, ignData)
+	if err != nil {
+		return nil, fmt.Errorf("failed to unmarshal bootstrap ignition: %w", err)
+	}
+
+	err = AddLoadBalancersToInfra(platform, ignData, publicIPAddresses, privateIPAddresses)
+	if err != nil {
+		return nil, fmt.Errorf("failed to add load balancers to ignition config: %w", err)
+	}
+
+	publicIPsSingleStr := strings.Join(publicIPAddresses, ",")
+	privateIPsSingleStr := strings.Join(privateIPAddresses, ",")
+
+	lbConfig, err := lbconfig.GenerateLBConfigOverride(privateIPsSingleStr, publicIPsSingleStr)
+	if err != nil {
+		return nil, err
+	}
+	if err := asset.NewDefaultFileWriter(lbConfig).PersistToFile(command.RootOpts.Dir); err != nil {
+		return nil, fmt.Errorf("failed to save %s to state file: %w", lbConfig.Name(), err)
+	}
+
+	editedIgnBytes, err := json.Marshal(ignData)
+	if err != nil {
+		return nil, fmt.Errorf("failed to convert ignition data to json: %w", err)
+	}
+
+	return editedIgnBytes, nil
+}
+
+// AddLoadBalancersToInfra will load the public and private load balancer information into
+// the infrastructure CR. This will occur after the data has already been inserted into the
+// ignition file.
+func AddLoadBalancersToInfra(platform string, config *igntypes.Config, publicLBs []string, privateLBs []string) error {
+	for i, fileData := range config.Storage.Files {
+		// update the contents of this file
+		if fileData.Path == infrastructureFilepath {
+			contents := config.Storage.Files[i].Contents.Source
+			replaced := strings.Replace(*contents, replaceable, "", 1)
+
+			rawDecodedText, err := base64.StdEncoding.DecodeString(replaced)
+			if err != nil {
+				return fmt.Errorf("failed to decode contents of ignition file: %w", err)
+			}
+
+			infra := &configv1.Infrastructure{}
+			if err := yaml.Unmarshal(rawDecodedText, infra); err != nil {
+				return fmt.Errorf("failed to unmarshal infrastructure: %w", err)
+			}
+
+			// convert the list of strings to a list of IPs
+			apiIntLbs := []configv1.IP{}
+			for _, ip := range privateLBs {
+				apiIntLbs = append(apiIntLbs, configv1.IP(ip))
+			}
+			apiLbs := []configv1.IP{}
+			for _, ip := range publicLBs {
+				apiLbs = append(apiLbs, configv1.IP(ip))
+			}
+			cloudLBInfo := configv1.CloudLoadBalancerIPs{
+				APIIntLoadBalancerIPs: apiIntLbs,
+				APILoadBalancerIPs:    apiLbs,
+			}
+
+			switch platform {
+			case gcptypes.Name:
+				if infra.Status.PlatformStatus.GCP.CloudLoadBalancerConfig.DNSType == configv1.ClusterHostedDNSType {
+					infra.Status.PlatformStatus.GCP.CloudLoadBalancerConfig.ClusterHosted = &cloudLBInfo
+				}
+			case awstypes.Name:
+				if infra.Status.PlatformStatus.AWS.CloudLoadBalancerConfig.DNSType == configv1.ClusterHostedDNSType {
+					infra.Status.PlatformStatus.AWS.CloudLoadBalancerConfig.ClusterHosted = &cloudLBInfo
+				}
+			default:
+				return fmt.Errorf("invalid platform %s", platform)
+			}
+
+			// convert the infrastructure back to an encoded string
+			infraContents, err := yaml.Marshal(infra)
+			if err != nil {
+				return fmt.Errorf("failed to marshal infrastructure: %w", err)
+			}
+
+			encoded := fmt.Sprintf("%s%s", replaceable, base64.StdEncoding.EncodeToString(infraContents))
+			// replace the contents with the edited information
+			config.Storage.Files[i].Contents.Source = &encoded
+
+			break
+		}
+	}
+
+	return nil
+}

pkg/infrastructure/gcp/clusterapi/clusterapi.go

@@ -122,16 +122,11 @@ func (p Provider) Ignition(ctx context.Context, in clusterapi.IgnitionInput) ([]
 		return nil, fmt.Errorf("failed to create bucket %s: %w", bucketName, err)
 	}
 
-	editedIgnitionBytes, err := EditIgnition(ctx, in)
+	ignitionBytes, err := editIgnition(ctx, in)
 	if err != nil {
 		return nil, fmt.Errorf("failed to edit bootstrap ignition: %w", err)
 	}
 
-	ignitionBytes := in.BootstrapIgnData
-	if editedIgnitionBytes != nil {
-		ignitionBytes = editedIgnitionBytes
-	}
-
 	if err := gcp.FillBucket(ctx, bucketHandle, string(ignitionBytes)); err != nil {
 		return nil, fmt.Errorf("ignition failed to fill bucket: %w", err)
 	}

pkg/infrastructure/gcp/clusterapi/ignition.go

@@ -2,21 +2,12 @@ package clusterapi
 
 import (
 	"context"
-	"encoding/base64"
-	"encoding/json"
 	"fmt"
 	"strings"
-	"time"
 
-	igntypes "github.com/coreos/ignition/v2/config/v3_2/types"
 	capg "sigs.k8s.io/cluster-api-provider-gcp/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/yaml"
 
-	configv1 "github.com/openshift/api/config/v1"
-	"github.com/openshift/installer/cmd/openshift-install/command"
-	"github.com/openshift/installer/pkg/asset"
-	"github.com/openshift/installer/pkg/asset/lbconfig"
 	"github.com/openshift/installer/pkg/asset/manifests/capiutils"
 	"github.com/openshift/installer/pkg/infrastructure/clusterapi"
 	"github.com/openshift/installer/pkg/types"
@@ -24,141 +15,51 @@ import (
 	"github.com/openshift/installer/pkg/types/gcp"
 )
 
-const (
-	infrastructureFilepath = "/opt/openshift/manifests/cluster-infrastructure-02-config.yml"
-
-	// replaceable is the string that precedes the encoded data in the ignition data.
-	// The data must be replaced before decoding the string, and the string must be
-	// prepended to the encoded data.
-	replaceable = "data:text/plain;charset=utf-8;base64,"
-)
-
-// EditIgnition attempts to edit the contents of the bootstrap ignition when the user has selected
+// editIgnition attempts to edit the contents of the bootstrap ignition when the user has selected
 // a custom DNS configuration. Find the public and private load balancer addresses and fill in the
 // infrastructure file within the ignition struct.
-func EditIgnition(ctx context.Context, in clusterapi.IgnitionInput) ([]byte, error) {
-	ctx, cancel := context.WithTimeout(ctx, time.Minute*2)
-	defer cancel()
-
-	if in.InstallConfig.Config.GCP.UserProvisionedDNS == dns.UserProvisionedDNSEnabled {
-		gcpCluster := &capg.GCPCluster{}
-		key := client.ObjectKey{
-			Name:      in.InfraID,
-			Namespace: capiutils.Namespace,
-		}
-		if err := in.Client.Get(ctx, key, gcpCluster); err != nil {
-			return nil, fmt.Errorf("failed to get GCP cluster: %w", err)
-		}
-
-		svc, err := NewComputeService()
-		if err != nil {
-			return nil, err
-		}
-
-		project := in.InstallConfig.Config.GCP.ProjectID
-		if in.InstallConfig.Config.GCP.NetworkProjectID != "" {
-			project = in.InstallConfig.Config.GCP.NetworkProjectID
-		}
-
-		computeAddress := ""
-		if in.InstallConfig.Config.Publish == types.ExternalPublishingStrategy {
-			apiIPAddress := *gcpCluster.Status.Network.APIServerAddress
-			addressCut := apiIPAddress[strings.LastIndex(apiIPAddress, "/")+1:]
-			computeAddressObj, err := svc.GlobalAddresses.Get(project, addressCut).Context(ctx).Do()
-			if err != nil {
-				return nil, fmt.Errorf("failed to get global compute address: %w", err)
-			}
-			computeAddress = computeAddressObj.Address
-		}
-
-		apiIntIPAddress := *gcpCluster.Status.Network.APIInternalAddress
-		addressIntCut := apiIntIPAddress[strings.LastIndex(apiIntIPAddress, "/")+1:]
-		computeIntAddress, err := svc.Addresses.Get(project, in.InstallConfig.Config.GCP.Region, addressIntCut).Context(ctx).Do()
-		if err != nil {
-			return nil, fmt.Errorf("failed to get compute address: %w", err)
-		}
-
-		ignData := &igntypes.Config{}
-		err = json.Unmarshal(in.BootstrapIgnData, ignData)
-		if err != nil {
-			return nil, fmt.Errorf("failed to unmarshal bootstrap ignition: %w", err)
-		}
-
-		err = addLoadBalancersToInfra(gcp.Name, ignData, []string{computeAddress}, []string{computeIntAddress.Address})
-		if err != nil {
-			return nil, fmt.Errorf("failed to add load balancers to ignition config: %w", err)
-		}
-
-		lbConfig, err := lbconfig.GenerateLBConfigOverride(computeIntAddress.Address, computeAddress)
-		if err != nil {
-			return nil, err
-		}
-		if err := asset.NewDefaultFileWriter(lbConfig).PersistToFile(command.RootOpts.Dir); err != nil {
-			return nil, fmt.Errorf("failed to save %s to state file: %w", lbConfig.Name(), err)
-		}
-
-		editedIgnBytes, err := json.Marshal(ignData)
-		if err != nil {
-			return nil, fmt.Errorf("failed to convert ignition data to json: %w", err)
-		}
-
-		return editedIgnBytes, nil
+func editIgnition(ctx context.Context, in clusterapi.IgnitionInput) ([]byte, error) {
+	if in.InstallConfig.Config.GCP.UserProvisionedDNS != dns.UserProvisionedDNSEnabled {
+		return in.BootstrapIgnData, nil
 	}
 
-	return nil, nil
-}
-
-// addLoadBalancersToInfra will load the public and private load balancer information into
-// the infrastructure CR. This will occur after the data has already been inserted into the
-// ignition file.
-func addLoadBalancersToInfra(platform string, config *igntypes.Config, publicLBs []string, privateLBs []string) error {
-	for i, fileData := range config.Storage.Files {
-		// update the contents of this file
-		if fileData.Path == infrastructureFilepath {
-			contents := config.Storage.Files[i].Contents.Source
-			replaced := strings.Replace(*contents, replaceable, "", 1)
-
-			rawDecodedText, err := base64.StdEncoding.DecodeString(replaced)
-			if err != nil {
-				return fmt.Errorf("failed to decode contents of ignition file: %w", err)
-			}
-
-			infra := &configv1.Infrastructure{}
-			if err := yaml.Unmarshal(rawDecodedText, infra); err != nil {
-				return fmt.Errorf("failed to unmarshal infrastructure: %w", err)
-			}
-
-			// convert the list of strings to a list of IPs
-			apiIntLbs := []configv1.IP{}
-			for _, ip := range privateLBs {
-				apiIntLbs = append(apiIntLbs, configv1.IP(ip))
-			}
-			apiLbs := []configv1.IP{}
-			for _, ip := range publicLBs {
-				apiLbs = append(apiLbs, configv1.IP(ip))
-			}
-			cloudLBInfo := configv1.CloudLoadBalancerIPs{
-				APIIntLoadBalancerIPs: apiIntLbs,
-				APILoadBalancerIPs:    apiLbs,
-			}
-
-			if infra.Status.PlatformStatus.GCP.CloudLoadBalancerConfig.DNSType == configv1.ClusterHostedDNSType {
-				infra.Status.PlatformStatus.GCP.CloudLoadBalancerConfig.ClusterHosted = &cloudLBInfo
-			}
-
-			// convert the infrastructure back to an encoded string
-			infraContents, err := yaml.Marshal(infra)
-			if err != nil {
-				return fmt.Errorf("failed to marshal infrastructure: %w", err)
-			}
-
-			encoded := fmt.Sprintf("%s%s", replaceable, base64.StdEncoding.EncodeToString(infraContents))
-			// replace the contents with the edited information
-			config.Storage.Files[i].Contents.Source = &encoded
-
-			break
-		}
+	gcpCluster := &capg.GCPCluster{}
+	key := client.ObjectKey{
+		Name:      in.InfraID,
+		Namespace: capiutils.Namespace,
+	}
+	if err := in.Client.Get(ctx, key, gcpCluster); err != nil {
+		return nil, fmt.Errorf("failed to get GCP cluster: %w", err)
 	}
 
-	return nil
+	svc, err := NewComputeService()
+	if err != nil {
+		return nil, err
+	}
+
+	project := in.InstallConfig.Config.GCP.ProjectID
+	if in.InstallConfig.Config.GCP.NetworkProjectID != "" {
+		project = in.InstallConfig.Config.GCP.NetworkProjectID
+	}
+
+	computeAddress := ""
+	if in.InstallConfig.Config.Publish == types.ExternalPublishingStrategy {
+		apiIPAddress := *gcpCluster.Status.Network.APIServerAddress
+		addressCut := apiIPAddress[strings.LastIndex(apiIPAddress, "/")+1:]
+		computeAddressObj, err := svc.GlobalAddresses.Get(project, addressCut).Context(ctx).Do()
+		if err != nil {
+			return nil, fmt.Errorf("failed to get global compute address: %w", err)
+		}
+
+		computeAddress = computeAddressObj.Address
+	}
+
+	apiIntIPAddress := *gcpCluster.Status.Network.APIInternalAddress
+	addressIntCut := apiIntIPAddress[strings.LastIndex(apiIntIPAddress, "/")+1:]
+	computeIntAddress, err := svc.Addresses.Get(project, in.InstallConfig.Config.GCP.Region, addressIntCut).Context(ctx).Do()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get compute address: %w", err)
+	}
+
+	return clusterapi.EditIgnition(in, gcp.Name, []string{computeAddress}, []string{computeIntAddress.Address})
 }