Merge pull request #15659 from hakman/azure_validate_tags

azure: Verify node identity using VMSS name instead of tags
kubernetes · Jul 18, 2023 · 9781e0a · 9781e0a
2 parents aa584af + c4ec894
commit 9781e0a
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 13 deletions.
diff --git a/pkg/apis/kops/model/features.go b/pkg/apis/kops/model/features.go
@@ -31,6 +31,8 @@ func UseChallengeCallback(cloudProvider kops.CloudProviderID) bool {
 		return true
 	case kops.CloudProviderScaleway:
 		return true
+	case kops.CloudProviderAzure:
+		return true
 	default:
 		return false
 	}

diff --git a/pkg/model/azuremodel/network.go b/pkg/model/azuremodel/network.go
@@ -167,6 +167,17 @@ func (b *NetworkModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 			})
 		}
 	}
+	nsgTask.SecurityRules = append(nsgTask.SecurityRules, &azuretasks.NetworkSecurityRule{
+		Name:                     fi.PtrTo("AllowNodeupChallenge"),
+		Priority:                 fi.PtrTo[int32](220),
+		Access:                   network.SecurityRuleAccessAllow,
+		Direction:                network.SecurityRuleDirectionInbound,
+		Protocol:                 network.SecurityRuleProtocolTCP,
+		SourceAddressPrefix:      fi.PtrTo(b.Cluster.Spec.Networking.NetworkCIDR),
+		SourcePortRange:          fi.PtrTo("*"),
+		DestinationAddressPrefix: fi.PtrTo("*"),
+		DestinationPortRange:     fi.PtrTo(strconv.Itoa(wellknownports.NodeupChallenge)),
+	})
 	var nodePortAccessIPv4, nodePortAccessIPv6 []string
 	for _, cidr := range b.Cluster.Spec.NodePortAccess {
 		switch net.IPFamilyOfCIDRString(cidr) {

diff --git a/upup/pkg/fi/cloudup/azure/verifier.go b/upup/pkg/fi/cloudup/azure/verifier.go
@@ -28,15 +28,16 @@ import (
 	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-05-01/network"
 	"github.com/Azure/go-autorest/autorest/azure/auth"
 	"k8s.io/kops/pkg/bootstrap"
-	"k8s.io/kops/pkg/nodeidentity/azure"
 	"k8s.io/kops/pkg/wellknownports"
 )
 
 type AzureVerifierOptions struct {
+	ClusterName string `json:"clusterName,omitempty"`
 }
 
 type azureVerifier struct {
-	client *client
+	client      *client
+	clusterName string
 }
 
 var _ bootstrap.Verifier = &azureVerifier{}
@@ -47,8 +48,13 @@ func NewAzureVerifier(ctx context.Context, opt *AzureVerifierOptions) (bootstrap
 		return nil, err
 	}
 
+	if opt == nil || opt.ClusterName == "" {
+		return nil, fmt.Errorf("determining cluster name")
+	}
+
 	return &azureVerifier{
-		client: azureClient,
+		client:      azureClient,
+		clusterName: opt.ClusterName,
 	}, nil
 }
 
@@ -65,6 +71,11 @@ func (a azureVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request
 	vmssName := v[1]
 	vmssIndex := v[2]
 
+	if !strings.HasSuffix(vmssName, "."+a.clusterName) {
+		return nil, fmt.Errorf("matching cluster name %q to VMSS %q", a.clusterName, vmssName)
+	}
+	igName := strings.TrimSuffix(vmssName, "."+a.clusterName)
+
 	vm, err := a.client.vmsClient.Get(ctx, a.client.resourceGroup, vmssName, vmssIndex, "")
 	if err != nil {
 		return nil, fmt.Errorf("getting info for VMSS virtual machine %q #%s: %w", vmssName, vmssIndex, err)
@@ -73,11 +84,12 @@ func (a azureVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request
 		return nil, fmt.Errorf("determining VMID for VMSS %q virtual machine #%s", vmssName, vmssIndex)
 	}
 	if vmId != *vm.VMID {
-		return nil, fmt.Errorf("matching VMID %q for VMSS %q virtual machine #%s", vmId, vmssName, vmssIndex)
+		return nil, fmt.Errorf("matching VMID %q to VMSS %q virtual machine #%s", vmId, vmssName, vmssIndex)
 	}
-	if vm.OsProfile == nil || *vm.OsProfile.ComputerName == "" {
+	if vm.OsProfile == nil || vm.OsProfile.ComputerName == nil || *vm.OsProfile.ComputerName == "" {
 		return nil, fmt.Errorf("determining ComputerName for VMSS %q virtual machine #%s", vmssName, vmssIndex)
 	}
+	nodeName := *vm.OsProfile.ComputerName
 
 	ni, err := a.client.nisClient.GetVirtualMachineScaleSetNetworkInterface(ctx, a.client.resourceGroup, vmssName, vmssIndex, vmssName+"-netconfig", "")
 	if err != nil {
@@ -100,17 +112,12 @@ func (a azureVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request
 	}
 
 	result := &bootstrap.VerifyResult{
-		NodeName:          *vm.OsProfile.ComputerName,
+		NodeName:          nodeName,
+		InstanceGroupName: igName,
 		CertificateNames:  addrs,
 		ChallengeEndpoint: challengeEndpoints[0],
 	}
 
-	for key, value := range vm.Tags {
-		if key == azure.InstanceGroupNameTag && value != nil {
-			result.InstanceGroupName = *value
-		}
-	}
-
 	return result, nil
 }
 

diff --git a/upup/pkg/fi/cloudup/template_functions.go b/upup/pkg/fi/cloudup/template_functions.go
@@ -737,7 +737,9 @@ func (tf *TemplateFunctions) KopsControllerConfig() (string, error) {
 			config.Server.Provider.Scaleway = &scaleway.ScalewayVerifierOptions{}
 
 		case kops.CloudProviderAzure:
-			config.Server.Provider.Azure = &azure.AzureVerifierOptions{}
+			config.Server.Provider.Azure = &azure.AzureVerifierOptions{
+				ClusterName: tf.ClusterName(),
+			}
 
 		default:
 			return "", fmt.Errorf("unsupported cloud provider %s", cluster.Spec.GetCloudProvider())