Merge pull request #8359 from patrickdillon/capi-aws-btstrp-del

CORS-2905: capi-aws delete bootstrap ssh rule

Author: openshift-merge-bot[bot]

pkg/asset/manifests/aws/cluster.go

@@ -17,6 +17,11 @@ import (
 	"github.com/openshift/installer/pkg/types"
 )
 
+// BootstrapSSHDescription is the description for the
+// ingress rule that provides SSH access to the bootstrap node
+// & identifies the rule for removal during bootstrap destroy.
+const BootstrapSSHDescription = "Bootstrap SSH Access"
+
 // GenerateClusterAssets generates the manifests for the cluster-api.
 func GenerateClusterAssets(ic *installconfig.InstallConfig, clusterID *installconfig.ClusterID) (*capiutils.GenerateClusterAssetsOutput, error) {
 	manifests := []*asset.RuntimeFile{}
@@ -133,7 +138,7 @@ func GenerateClusterAssets(ic *installconfig.InstallConfig, clusterID *installco
 						SourceSecurityGroupRoles: []capa.SecurityGroupRole{"controlplane", "node"},
 					},
 					{
-						Description: "SSH everyone",
+						Description: BootstrapSSHDescription,
 						Protocol:    capa.SecurityGroupProtocolTCP,
 						FromPort:    22,
 						ToPort:      22,

pkg/infrastructure/aws/clusterapi/aws.go

@@ -4,18 +4,22 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
+	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/aws/session"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/elbv2"
 	"github.com/sirupsen/logrus"
+	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/utils/ptr"
 	capa "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
 	k8sClient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	awsconfig "github.com/openshift/installer/pkg/asset/installconfig/aws"
+	awsmanifest "github.com/openshift/installer/pkg/asset/manifests/aws"
 	"github.com/openshift/installer/pkg/asset/manifests/capiutils"
 	"github.com/openshift/installer/pkg/infrastructure/clusterapi"
 	awstypes "github.com/openshift/installer/pkg/types/aws"
@@ -192,3 +196,77 @@ func getHostedZoneIDForNLB(ctx context.Context, awsSession *session.Session, reg
 
 	return "", errNotFound
 }
+
+// DestroyBootstrap removes aws bootstrap resources not handled
+// by the deletion of the bootstrap machine by the capi controllers.
+func (*Provider) DestroyBootstrap(ctx context.Context, in clusterapi.BootstrapDestroyInput) error {
+	if err := removeSSHRule(ctx, in.Client, in.Metadata.InfraID); err != nil {
+		return fmt.Errorf("failed to remove bootstrap SSH rule: %w", err)
+	}
+	return nil
+}
+
+// removeSSHRule removes the SSH rule for accessing the bootstrap node
+// by removing the rule from the cluster spec and updating the object.
+func removeSSHRule(ctx context.Context, cl k8sClient.Client, infraID string) error {
+	awsCluster := &capa.AWSCluster{}
+	key := k8sClient.ObjectKey{
+		Name:      infraID,
+		Namespace: capiutils.Namespace,
+	}
+	if err := cl.Get(ctx, key, awsCluster); err != nil {
+		return fmt.Errorf("failed to get AWSCluster: %w", err)
+	}
+
+	postBootstrapRules := []capa.IngressRule{}
+	for _, rule := range awsCluster.Spec.NetworkSpec.AdditionalControlPlaneIngressRules {
+		if strings.EqualFold(rule.Description, awsmanifest.BootstrapSSHDescription) {
+			continue
+		}
+		postBootstrapRules = append(postBootstrapRules, rule)
+	}
+
+	awsCluster.Spec.NetworkSpec.AdditionalControlPlaneIngressRules = postBootstrapRules
+
+	if err := cl.Update(ctx, awsCluster); err != nil {
+		return fmt.Errorf("failed to update AWSCluster during bootstrap destroy: %w", err)
+	}
+	logrus.Debug("Updated AWSCluster to remove bootstrap SSH rule")
+
+	timeout := 5 * time.Minute
+	untilTime := time.Now().Add(timeout)
+	timezone, _ := untilTime.Zone()
+	logrus.Infof("Waiting up to %v (until %v %s) for bootstrap SSH rule to be destroyed...", timeout, untilTime.Format(time.Kitchen), timezone)
+	if err := wait.ExponentialBackoffWithContext(ctx, wait.Backoff{
+		Duration: time.Second * 10,
+		Factor:   float64(1.5),
+		Steps:    32,
+		Cap:      timeout,
+	}, func(ctx context.Context) (bool, error) {
+		c := &capa.AWSCluster{}
+		if err := cl.Get(ctx, key, c); err != nil {
+			return false, err
+		}
+		if sg, ok := c.Status.Network.SecurityGroups[capa.SecurityGroupControlPlane]; ok {
+			for _, r := range sg.IngressRules {
+				if r.Description == awsmanifest.BootstrapSSHDescription {
+					return false, nil
+				}
+			}
+			return true, nil
+		}
+		// This shouldn't happen, but if control plane SG is not found, return an error.
+		keys := make([]capa.SecurityGroupRole, 0, len(c.Status.Network.SecurityGroups))
+		for sgr := range c.Status.Network.SecurityGroups {
+			keys = append(keys, sgr)
+		}
+		return false, fmt.Errorf("controlplane not found in cluster security groups: %v", keys)
+	}); err != nil {
+		if wait.Interrupted(err) {
+			return fmt.Errorf("bootstrap ssh rule was not removed within %v: %w", timeout, err)
+		}
+		return fmt.Errorf("unable to remove bootstrap ssh rule: %w", err)
+	}
+
+	return nil
+}

pkg/infrastructure/clusterapi/clusterapi.go

@@ -378,6 +378,17 @@ func (i *InfraProvider) DestroyBootstrap(ctx context.Context, dir string) error
 		}
 	}
 
+	if p, ok := i.impl.(BootstrapDestroyer); ok {
+		bootstrapDestoryInput := BootstrapDestroyInput{
+			Client:   sys.Client(),
+			Metadata: *metadata,
+		}
+
+		if err = p.DestroyBootstrap(ctx, bootstrapDestoryInput); err != nil {
+			return fmt.Errorf("failed during the destroy bootstrap hook: %w", err)
+		}
+	}
+
 	machineName := capiutils.GenerateBoostrapMachineName(metadata.InfraID)
 	machineNamespace := capiutils.Namespace
 	if err := sys.Client().Delete(ctx, &clusterv1.Machine{

pkg/infrastructure/clusterapi/types.go

@@ -10,6 +10,7 @@ import (
 	"github.com/openshift/installer/pkg/asset/machines"
 	"github.com/openshift/installer/pkg/asset/manifests"
 	"github.com/openshift/installer/pkg/asset/rhcos"
+	"github.com/openshift/installer/pkg/types"
 )
 
 // Provider is the base interface that cloud platforms
@@ -87,3 +88,15 @@ type PostProvisionInput struct {
 	InstallConfig *installconfig.InstallConfig
 	InfraID       string
 }
+
+// BootstrapDestroyer allows platform-specific behavior when
+// destroying bootstrap resources.
+type BootstrapDestroyer interface {
+	DestroyBootstrap(ctx context.Context, in BootstrapDestroyInput) error
+}
+
+// BootstrapDestroyInput collects args passed to the DestroyBootstrap hook.
+type BootstrapDestroyInput struct {
+	Client   client.Client
+	Metadata types.ClusterMetadata
+}