Merge pull request openshift#8732 from jhixson74/capz_private

CORS-3565: CAPZ private clusters

Author: openshift-merge-bot[bot]

pkg/asset/machines/azure/azuremachines.go

@@ -23,24 +23,37 @@ const (
 	genV2Suffix string = "-gen2"
 )
 
+// MachineInput defines the inputs needed to generate a machine asset.
+type MachineInput struct {
+	Subnet          string
+	Role            string
+	UserDataSecret  string
+	HyperVGen       string
+	UseImageGallery bool
+	Private         bool
+	UserTags        map[string]string
+	Platform        *azure.Platform
+	Pool            *types.MachinePool
+}
+
 // GenerateMachines returns manifests and runtime objects to provision the control plane (including bootstrap, if applicable) nodes using CAPI.
-func GenerateMachines(platform *azure.Platform, pool *types.MachinePool, userDataSecret string, clusterID string, role string, capabilities map[string]string, useImageGallery bool, userTags map[string]string, hyperVGen string, subnet string, resourceGroup string, subscriptionID string) ([]*asset.RuntimeFile, error) {
-	if poolPlatform := pool.Platform.Name(); poolPlatform != azure.Name {
+func GenerateMachines(clusterID, resourceGroup, subscriptionID string, in *MachineInput) ([]*asset.RuntimeFile, error) {
+	if poolPlatform := in.Pool.Platform.Name(); poolPlatform != azure.Name {
 		return nil, fmt.Errorf("non-Azure machine-pool: %q", poolPlatform)
 	}
-	mpool := pool.Platform.Azure
+	mpool := in.Pool.Platform.Azure
 
 	total := int64(1)
-	if pool.Replicas != nil {
-		total = *pool.Replicas
+	if in.Pool.Replicas != nil {
+		total = *in.Pool.Replicas
 	}
 
 	if len(mpool.Zones) == 0 {
 		// if no azs are given we set to []string{""} for convenience over later operations.
 		// It means no-zoned for the machine API
 		mpool.Zones = []string{""}
 	}
-	tags, err := CapzTagsFromUserTags(clusterID, userTags)
+	tags, err := CapzTagsFromUserTags(clusterID, in.UserTags)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create machineapi.TagSpecifications from UserTags: %w", err)
 	}
@@ -64,17 +77,17 @@ func GenerateMachines(platform *azure.Platform, pool *types.MachinePool, userDat
 				ThirdPartyImage: osImage.Plan != azure.ImageNoPurchasePlan,
 			},
 		}
-	case useImageGallery:
+	case in.UseImageGallery:
 		// image gallery names cannot have dashes
 		id := clusterID
-		if hyperVGen == "V2" {
+		if in.HyperVGen == "V2" {
 			id += genV2Suffix
 		}
 		imageID := fmt.Sprintf("/resourceGroups/%s/providers/Microsoft.Compute/galleries/gallery_%s/images/%s/versions/latest", resourceGroup, galleryName, id)
 		image = &capz.Image{ID: &imageID}
 	default:
 		imageID := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/galleries/gallery_%s/images/%s", subscriptionID, resourceGroup, galleryName, clusterID)
-		if hyperVGen == "V2" && platform.CloudName != azure.StackCloud {
+		if in.HyperVGen == "V2" && in.Platform.CloudName != azure.StackCloud {
 			imageID += genV2Suffix
 		}
 		image = &capz.Image{ID: &imageID}
@@ -92,7 +105,7 @@ func GenerateMachines(platform *azure.Platform, pool *types.MachinePool, userDat
 	additionalCapabilities := &capz.AdditionalCapabilities{
 		UltraSSDEnabled: &ultrassd,
 	}
-	if pool.Platform.Azure.DiskEncryptionSet != nil {
+	if in.Pool.Platform.Azure.DiskEncryptionSet != nil {
 		osDisk.ManagedDisk.DiskEncryptionSet = &capz.DiskEncryptionSetParameters{
 			ID: mpool.OSDisk.DiskEncryptionSet.ToID(),
 		}
@@ -120,7 +133,7 @@ func GenerateMachines(platform *azure.Platform, pool *types.MachinePool, userDat
 		zone := mpool.Zones[int(idx)%len(mpool.Zones)]
 		azureMachine := &capz.AzureMachine{
 			ObjectMeta: metav1.ObjectMeta{
-				Name: fmt.Sprintf("%s-%s-%d", clusterID, pool.Name, idx),
+				Name: fmt.Sprintf("%s-%s-%d", clusterID, in.Pool.Name, idx),
 				Labels: map[string]string{
 					"cluster.x-k8s.io/control-plane": "",
 					"cluster.x-k8s.io/cluster-name":  clusterID,
@@ -139,7 +152,7 @@ func GenerateMachines(platform *azure.Platform, pool *types.MachinePool, userDat
 				SecurityProfile:            securityProfile,
 				NetworkInterfaces: []capz.NetworkInterface{
 					{
-						SubnetName:            subnet,
+						SubnetName:            in.Subnet,
 						AcceleratedNetworking: ptr.To(mpool.VMNetworkingType == string(azure.VMnetworkingTypeAccelerated) || mpool.VMNetworkingType == string(azure.AcceleratedNetworkingEnabled)),
 					},
 				},
@@ -167,7 +180,7 @@ func GenerateMachines(platform *azure.Platform, pool *types.MachinePool, userDat
 			Spec: capi.MachineSpec{
 				ClusterName: clusterID,
 				Bootstrap: capi.Bootstrap{
-					DataSecretName: ptr.To(fmt.Sprintf("%s-%s", clusterID, role)),
+					DataSecretName: ptr.To(fmt.Sprintf("%s-%s", clusterID, in.Role)),
 				},
 				InfrastructureRef: v1.ObjectReference{
 					APIVersion: capz.GroupVersion.String(),
@@ -200,7 +213,7 @@ func GenerateMachines(platform *azure.Platform, pool *types.MachinePool, userDat
 			OSDisk:                     osDisk,
 			AdditionalTags:             tags,
 			DisableExtensionOperations: ptr.To(true),
-			AllocatePublicIP:           true,
+			AllocatePublicIP:           !in.Private,
 			AdditionalCapabilities:     additionalCapabilities,
 			SecurityProfile:            securityProfile,
 			Identity:                   capz.VMIdentityUserAssigned,

pkg/asset/machines/clusterapi.go

@@ -277,12 +277,22 @@ func (c *ClusterAPI) Generate(ctx context.Context, dependencies asset.Parents) e
 		if err != nil {
 			return err
 		}
-		// useImageGallery := installConfig.Azure.CloudName != azuretypes.StackCloud
-		useImageGallery := false
-		masterUserDataSecretName := "master-user-data"
-		resourceGroupName := installConfig.Config.Azure.ClusterResourceGroupName(clusterID.InfraID)
 
-		azureMachines, err := azure.GenerateMachines(installConfig.Config.Platform.Azure, &pool, masterUserDataSecretName, clusterID.InfraID, "master", capabilities, useImageGallery, installConfig.Config.Platform.Azure.UserTags, hyperVGen, subnet, resourceGroupName, session.Credentials.SubscriptionID)
+		azureMachines, err := azure.GenerateMachines(clusterID.InfraID,
+			installConfig.Config.Azure.ClusterResourceGroupName(clusterID.InfraID),
+			session.Credentials.SubscriptionID,
+			&azure.MachineInput{
+				Subnet:          subnet,
+				Role:            "master",
+				UserDataSecret:  "master-user-data",
+				HyperVGen:       hyperVGen,
+				UseImageGallery: false,
+				Private:         installConfig.Config.Publish == types.InternalPublishingStrategy,
+				UserTags:        installConfig.Config.Platform.Azure.UserTags,
+				Platform:        installConfig.Config.Platform.Azure,
+				Pool:            &pool,
+			},
+		)
 		if err != nil {
 			return fmt.Errorf("failed to create master machine objects: %w", err)
 		}

pkg/asset/manifests/azure/cluster.go

@@ -47,11 +47,6 @@ func GenerateClusterAssets(installConfig *installconfig.InstallConfig, clusterID
 	computeSubnet := installConfig.Config.Platform.Azure.ComputeSubnetName(clusterID.InfraID)
 	networkSecurityGroup := installConfig.Config.Platform.Azure.NetworkSecurityGroupName(clusterID.InfraID)
 
-	source := "*"
-	if installConfig.Config.Publish == types.InternalPublishingStrategy {
-		source = mainCIDR.String()
-	}
-
 	controlPlaneOutboundLB := &capz.LoadBalancerSpec{
 		Name:             clusterID.InfraID,
 		FrontendIPsCount: to.Ptr(int32(1)),
@@ -60,6 +55,11 @@ func GenerateClusterAssets(installConfig *installconfig.InstallConfig, clusterID
 		controlPlaneOutboundLB = nil
 	}
 
+	source := "*"
+	if installConfig.Config.Publish == types.InternalPublishingStrategy {
+		source = mainCIDR.String()
+	}
+
 	securityGroup := capz.SecurityGroup{
 		Name: networkSecurityGroup,
 		SecurityGroupClass: capz.SecurityGroupClass{

pkg/infrastructure/azure/azure.go

@@ -595,10 +595,11 @@ func (p *Provider) PostProvision(ctx context.Context, in clusterapi.PostProvisio
 			return fmt.Errorf("failed to associate control plane VMs with external load balancer: %w", err)
 		}
 
+		sshRuleName := fmt.Sprintf("%s_ssh_in", in.InfraID)
 		if err = addSecurityGroupRule(ctx, &securityGroupInput{
 			resourceGroupName:    p.ResourceGroupName,
 			securityGroupName:    fmt.Sprintf("%s-nsg", in.InfraID),
-			securityRuleName:     "ssh_in",
+			securityRuleName:     sshRuleName,
 			securityRulePort:     "22",
 			securityRulePriority: 220,
 			networkClientFactory: p.NetworkClientFactory,
@@ -626,7 +627,7 @@ func (p *Provider) PostProvision(ctx context.Context, in clusterapi.PostProvisio
 			resourceGroupName:    p.ResourceGroupName,
 			loadBalancerName:     loadBalancerName,
 			frontendIPConfigID:   frontendIPConfigID,
-			inboundNatRuleName:   "ssh_in",
+			inboundNatRuleName:   sshRuleName,
 			inboundNatRulePort:   22,
 			networkClientFactory: p.NetworkClientFactory,
 		})
@@ -639,7 +640,7 @@ func (p *Provider) PostProvision(ctx context.Context, in clusterapi.PostProvisio
 			bootstrapNicName:     fmt.Sprintf("%s-bootstrap-nic", in.InfraID),
 			frontendIPConfigID:   frontendIPConfigID,
 			inboundNatRuleID:     *inboundNatRule.ID,
-			inboundNatRuleName:   "ssh_in",
+			inboundNatRuleName:   sshRuleName,
 			inboundNatRulePort:   22,
 			networkClientFactory: p.NetworkClientFactory,
 		})
@@ -673,26 +674,41 @@ func (p *Provider) PostDestroy(ctx context.Context, in clusterapi.PostDestroyerI
 		return fmt.Errorf("error creating network client factory: %w", err)
 	}
 
-	// XXX: why is in.Metadata.Azure.ResourceGroupName empty?
-	err = deleteSecurityGroupRule(ctx, &securityGroupInput{
-		resourceGroupName:    fmt.Sprintf("%s-rg", in.Metadata.InfraID),
-		securityGroupName:    fmt.Sprintf("%s-nsg", in.Metadata.InfraID),
-		securityRuleName:     "ssh_in",
-		securityRulePort:     "22",
-		networkClientFactory: networkClientFactory,
-	})
-	if err != nil {
-		return fmt.Errorf("failed to delete security rule: %w", err)
-	}
+	resourceGroupName := fmt.Sprintf("%s-rg", in.Metadata.InfraID)
+	securityGroupName := fmt.Sprintf("%s-nsg", in.Metadata.InfraID)
+	sshRuleName := fmt.Sprintf("%s_ssh_in", in.Metadata.InfraID)
 
-	err = deleteInboundNatRule(ctx, &inboundNatRuleInput{
-		resourceGroupName:    fmt.Sprintf("%s-rg", in.Metadata.InfraID),
-		loadBalancerName:     in.Metadata.InfraID,
-		inboundNatRuleName:   "ssh_in",
-		networkClientFactory: networkClientFactory,
-	})
-	if err != nil {
-		return fmt.Errorf("failed to delete inbound nat rule: %w", err)
+	// See if a security group rule exists with the name ${InfraID}_ssh_in.
+	// If it does, this is a private cluster. If it does not, this is a
+	// public cluster and we need to delete the SSH forward rule and
+	// security group rule.
+	_, err = networkClientFactory.NewSecurityRulesClient().Get(ctx,
+		resourceGroupName,
+		securityGroupName,
+		sshRuleName,
+		nil,
+	)
+	if err == nil {
+		err = deleteSecurityGroupRule(ctx, &securityGroupInput{
+			resourceGroupName:    resourceGroupName,
+			securityGroupName:    securityGroupName,
+			securityRuleName:     sshRuleName,
+			securityRulePort:     "22",
+			networkClientFactory: networkClientFactory,
+		})
+		if err != nil {
+			return fmt.Errorf("failed to delete security rule: %w", err)
+		}
+
+		err = deleteInboundNatRule(ctx, &inboundNatRuleInput{
+			resourceGroupName:    resourceGroupName,
+			loadBalancerName:     in.Metadata.InfraID,
+			inboundNatRuleName:   sshRuleName,
+			networkClientFactory: networkClientFactory,
+		})
+		if err != nil {
+			return fmt.Errorf("failed to delete inbound nat rule: %w", err)
+		}
 	}
 
 	return nil