From 83d14d4343acf98f52577a489a6b278f25c99608 Mon Sep 17 00:00:00 2001 From: Ciprian Hacman Date: Wed, 12 Jul 2023 21:24:52 +0300 Subject: [PATCH 1/5] azure: Add support for dns=none --- cmd/kops-controller/main.go | 7 + cmd/kops-controller/pkg/config/options.go | 2 + nodeup/pkg/model/bootstrap_client.go | 7 + pkg/apis/kops/model/features.go | 2 +- pkg/apis/kops/validation/validation.go | 10 -- pkg/model/azuremodel/network.go | 31 +++- upup/pkg/fi/cloudup/apply_cluster.go | 22 +-- upup/pkg/fi/cloudup/azure/authenticator.go | 104 +++++++++++++ upup/pkg/fi/cloudup/azure/loadbalancer.go | 9 ++ upup/pkg/fi/cloudup/azure/verifier.go | 145 ++++++++++++++++++ .../pkg/fi/cloudup/azuretasks/loadbalancer.go | 61 +++++++- upup/pkg/fi/cloudup/azuretasks/testing.go | 10 ++ upup/pkg/fi/cloudup/dns_test.go | 78 ---------- upup/pkg/fi/cloudup/new_cluster.go | 2 +- upup/pkg/fi/cloudup/template_functions.go | 4 + upup/pkg/fi/nodeup/command.go | 7 + 16 files changed, 386 insertions(+), 115 deletions(-) create mode 100644 upup/pkg/fi/cloudup/azure/authenticator.go create mode 100644 upup/pkg/fi/cloudup/azure/verifier.go diff --git a/cmd/kops-controller/main.go b/cmd/kops-controller/main.go index 94b00e3342109..6b2a80bd25aef 100644 --- a/cmd/kops-controller/main.go +++ b/cmd/kops-controller/main.go @@ -42,6 +42,7 @@ import ( nodeidentityos "k8s.io/kops/pkg/nodeidentity/openstack" nodeidentityscw "k8s.io/kops/pkg/nodeidentity/scaleway" "k8s.io/kops/upup/pkg/fi/cloudup/awsup" + "k8s.io/kops/upup/pkg/fi/cloudup/azure" "k8s.io/kops/upup/pkg/fi/cloudup/do" "k8s.io/kops/upup/pkg/fi/cloudup/gce/tpm/gcetpmverifier" "k8s.io/kops/upup/pkg/fi/cloudup/hetzner" @@ -153,6 +154,12 @@ func main() { setupLog.Error(err, "unable to create verifier") os.Exit(1) } + } else if opt.Server.Provider.Azure != nil { + verifier, err = azure.NewAzureVerifier(ctx, opt.Server.Provider.Azure) + if err != nil { + setupLog.Error(err, "unable to create verifier") + os.Exit(1) + } } else { klog.Fatalf("server cloud provider config not provided") } diff --git a/cmd/kops-controller/pkg/config/options.go b/cmd/kops-controller/pkg/config/options.go index defc08124d248..7527a765f44fd 100644 --- a/cmd/kops-controller/pkg/config/options.go +++ b/cmd/kops-controller/pkg/config/options.go @@ -18,6 +18,7 @@ package config import ( "k8s.io/kops/upup/pkg/fi/cloudup/awsup" + "k8s.io/kops/upup/pkg/fi/cloudup/azure" "k8s.io/kops/upup/pkg/fi/cloudup/do" gcetpm "k8s.io/kops/upup/pkg/fi/cloudup/gce/tpm" "k8s.io/kops/upup/pkg/fi/cloudup/hetzner" @@ -73,6 +74,7 @@ type ServerProviderOptions struct { OpenStack *openstack.OpenStackVerifierOptions `json:"openstack,omitempty"` DigitalOcean *do.DigitalOceanVerifierOptions `json:"do,omitempty"` Scaleway *scaleway.ScalewayVerifierOptions `json:"scaleway,omitempty"` + Azure *azure.AzureVerifierOptions `json:"azure,omitempty"` } // DiscoveryOptions configures our support for discovery, particularly gossip DNS (i.e. k8s.local) diff --git a/nodeup/pkg/model/bootstrap_client.go b/nodeup/pkg/model/bootstrap_client.go index abc678277724c..29d01b22d7438 100644 --- a/nodeup/pkg/model/bootstrap_client.go +++ b/nodeup/pkg/model/bootstrap_client.go @@ -29,6 +29,7 @@ import ( "k8s.io/kops/pkg/wellknownports" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/awsup" + "k8s.io/kops/upup/pkg/fi/cloudup/azure" "k8s.io/kops/upup/pkg/fi/cloudup/do" "k8s.io/kops/upup/pkg/fi/cloudup/gce/gcediscovery" "k8s.io/kops/upup/pkg/fi/cloudup/gce/tpm/gcetpmsigner" @@ -93,6 +94,12 @@ func (b BootstrapClientBuilder) Build(c *fi.NodeupModelBuilderContext) error { return err } authenticator = a + case kops.CloudProviderAzure: + a, err := azure.NewAzureAuthenticator() + if err != nil { + return err + } + authenticator = a default: return fmt.Errorf("unsupported cloud provider for authenticator %q", b.CloudProvider()) diff --git a/pkg/apis/kops/model/features.go b/pkg/apis/kops/model/features.go index 6829b59fe8466..15882767020a5 100644 --- a/pkg/apis/kops/model/features.go +++ b/pkg/apis/kops/model/features.go @@ -24,7 +24,7 @@ import ( // UseKopsControllerForNodeBootstrap is true if nodeup should use kops-controller for bootstrapping. func UseKopsControllerForNodeBootstrap(cloudProvider kops.CloudProviderID) bool { - return cloudProvider != kops.CloudProviderAzure + return true } // UseChallengeCallback is true if we should use a callback challenge during node provisioning with kops-controller. diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index ef300727ac54a..7bd96336d628b 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -521,17 +521,7 @@ func validateTopology(c *kops.Cluster, topology *kops.TopologySpec, fieldPath *f } if topology.DNS != "" { - cloud := c.Spec.GetCloudProvider() allErrs = append(allErrs, IsValidValue(fieldPath.Child("dns", "type"), &topology.DNS, kops.SupportedDnsTypes)...) - - if topology.DNS == kops.DNSTypeNone { - switch cloud { - case kops.CloudProviderOpenstack, kops.CloudProviderHetzner, kops.CloudProviderAWS, kops.CloudProviderGCE, kops.CloudProviderDO, kops.CloudProviderScaleway: - // ok - default: - allErrs = append(allErrs, field.Invalid(fieldPath.Child("dns", "type"), topology.DNS, fmt.Sprintf("not supported for %q", c.Spec.GetCloudProvider()))) - } - } } return allErrs diff --git a/pkg/model/azuremodel/network.go b/pkg/model/azuremodel/network.go index 2062f17741307..041b8d6740e04 100644 --- a/pkg/model/azuremodel/network.go +++ b/pkg/model/azuremodel/network.go @@ -18,8 +18,10 @@ package azuremodel import ( "fmt" + "strconv" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-05-01/network" + "k8s.io/kops/pkg/wellknownports" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/azuretasks" "k8s.io/utils/net" @@ -109,7 +111,7 @@ func (b *NetworkModelBuilder) Build(c *fi.CloudupModelBuilderContext) error { SourceAddressPrefixes: &k8sAccessIPv4, SourcePortRange: fi.PtrTo("*"), DestinationAddressPrefix: fi.PtrTo("*"), - DestinationPortRange: fi.PtrTo("443"), + DestinationPortRange: fi.PtrTo(strconv.Itoa(wellknownports.KubeAPIServer)), }) } if len(k8sAccessIPv6) > 0 { @@ -122,7 +124,32 @@ func (b *NetworkModelBuilder) Build(c *fi.CloudupModelBuilderContext) error { SourceAddressPrefixes: &k8sAccessIPv6, SourcePortRange: fi.PtrTo("*"), DestinationAddressPrefix: fi.PtrTo("*"), - DestinationPortRange: fi.PtrTo("443"), + DestinationPortRange: fi.PtrTo(strconv.Itoa(wellknownports.KubeAPIServer)), + }) + } + if b.Cluster.UsesNoneDNS() { + // TODO: Limit access to necessary source address prefixes instead of "0.0.0.0/0" and "::/0" + nsgTask.SecurityRules = append(nsgTask.SecurityRules, &azuretasks.NetworkSecurityRule{ + Name: fi.PtrTo("AllowKopsController"), + Priority: fi.PtrTo[int32](210), + Access: network.SecurityRuleAccessAllow, + Direction: network.SecurityRuleDirectionInbound, + Protocol: network.SecurityRuleProtocolTCP, + SourceAddressPrefix: fi.PtrTo("0.0.0.0/0"), + SourcePortRange: fi.PtrTo("*"), + DestinationAddressPrefix: fi.PtrTo("*"), + DestinationPortRange: fi.PtrTo(strconv.Itoa(wellknownports.KopsControllerPort)), + }) + nsgTask.SecurityRules = append(nsgTask.SecurityRules, &azuretasks.NetworkSecurityRule{ + Name: fi.PtrTo("AllowKopsController_v6"), + Priority: fi.PtrTo[int32](211), + Access: network.SecurityRuleAccessAllow, + Direction: network.SecurityRuleDirectionInbound, + Protocol: network.SecurityRuleProtocolTCP, + SourceAddressPrefix: fi.PtrTo("::/0"), + SourcePortRange: fi.PtrTo("*"), + DestinationAddressPrefix: fi.PtrTo("*"), + DestinationPortRange: fi.PtrTo(strconv.Itoa(wellknownports.KopsControllerPort)), }) } var nodePortAccessIPv4, nodePortAccessIPv6 []string diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go index cc4a63a026ee7..3752e145b7897 100644 --- a/upup/pkg/fi/cloudup/apply_cluster.go +++ b/upup/pkg/fi/cloudup/apply_cluster.go @@ -1446,13 +1446,7 @@ func (n *nodeUpConfigBuilder) BuildConfig(ig *kops.InstanceGroup, apiserverAddit } } - case kops.CloudProviderDO, kops.CloudProviderScaleway: - // Use any IP address that is found (including public ones) - for _, additionalIP := range apiserverAdditionalIPs { - controlPlaneIPs = append(controlPlaneIPs, additionalIP) - } - - case kops.CloudProviderGCE: + case kops.CloudProviderDO, kops.CloudProviderScaleway, kops.CloudProviderGCE, kops.CloudProviderAzure: // Use any IP address that is found (including public ones) for _, additionalIP := range apiserverAdditionalIPs { controlPlaneIPs = append(controlPlaneIPs, additionalIP) @@ -1460,19 +1454,7 @@ func (n *nodeUpConfigBuilder) BuildConfig(ig *kops.InstanceGroup, apiserverAddit } if cluster.UsesNoneDNS() { - switch cluster.Spec.GetCloudProvider() { - case kops.CloudProviderAWS, kops.CloudProviderHetzner, kops.CloudProviderOpenstack: - bootConfig.APIServerIPs = controlPlaneIPs - - case kops.CloudProviderDO, kops.CloudProviderScaleway: - bootConfig.APIServerIPs = controlPlaneIPs - - case kops.CloudProviderGCE: - bootConfig.APIServerIPs = controlPlaneIPs - - default: - return nil, nil, fmt.Errorf("'none' DNS topology is not supported for cloud %q", cluster.Spec.GetCloudProvider()) - } + bootConfig.APIServerIPs = controlPlaneIPs } else { // If we do have a fixed IP, we use it (on some clouds, initially) switch cluster.Spec.GetCloudProvider() { diff --git a/upup/pkg/fi/cloudup/azure/authenticator.go b/upup/pkg/fi/cloudup/azure/authenticator.go new file mode 100644 index 0000000000000..52e56987badb0 --- /dev/null +++ b/upup/pkg/fi/cloudup/azure/authenticator.go @@ -0,0 +1,104 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package azure + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + + "k8s.io/kops/pkg/bootstrap" +) + +const AzureAuthenticationTokenPrefix = "x-azure-id " + +type azureAuthenticator struct { +} + +var _ bootstrap.Authenticator = &azureAuthenticator{} + +func NewAzureAuthenticator() (bootstrap.Authenticator, error) { + return &azureAuthenticator{}, nil +} + +func (h *azureAuthenticator) CreateToken(body []byte) (string, error) { + m, err := queryInstanceMetadata() + if err != nil { + return "", fmt.Errorf("querying instance metadata: %w", err) + } + + // The fully qualified VMSS VM resource ID format is: + // /subscriptions/SUBSCRIPTION_ID/resourceGroups/RESOURCE_GROUP_NAME/providers/Microsoft.Compute/virtualMachineScaleSets/VMSS_NAME/virtualMachines/VMSS_INDEX + r := strings.Split(m.Compute.ResourceID, "/") + if len(r) != 11 || r[7] != "virtualMachineScaleSets" || r[9] != "virtualMachines" { + return "", fmt.Errorf("unexpected resource ID format: %q", m.Compute.ResourceID) + } + + vmssName := r[8] + vmssIndex := r[10] + + return AzureAuthenticationTokenPrefix + vmssName + " " + vmssIndex, nil +} + +type instanceComputeMetadata struct { + ResourceGroupName string `json:"resourceGroupName"` + ResourceID string `json:"resourceId"` + SubscriptionID string `json:"subscriptionId"` +} + +type instanceMetadata struct { + Compute *instanceComputeMetadata `json:"compute"` +} + +// queryInstanceMetadata queries Azure Instance Metadata Service (IMDS) +// https://learn.microsoft.com/en-us/azure/virtual-machines/instance-metadata-service?tabs=linux +func queryInstanceMetadata() (*instanceMetadata, error) { + transport := &http.Transport{Proxy: nil} + + client := http.Client{Transport: transport} + + req, err := http.NewRequest("GET", "http://169.254.169.254/metadata/instance", nil) + if err != nil { + return nil, fmt.Errorf("creating a new request: %w", err) + } + req.Header.Add("Metadata", "True") + + q := req.URL.Query() + q.Add("format", "json") + q.Add("api-version", "2021-02-01") + req.URL.RawQuery = q.Encode() + + resp, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("sending request to the instance metadata server: %w", err) + } + + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("reading a response from the metadata server: %w", err) + } + metadata := &instanceMetadata{} + err = json.Unmarshal(body, metadata) + if err != nil { + return nil, fmt.Errorf("unmarshalling instance metadata: %w", err) + } + + return metadata, nil +} diff --git a/upup/pkg/fi/cloudup/azure/loadbalancer.go b/upup/pkg/fi/cloudup/azure/loadbalancer.go index 8019f3e7dd9da..7ee8e5b3f279c 100644 --- a/upup/pkg/fi/cloudup/azure/loadbalancer.go +++ b/upup/pkg/fi/cloudup/azure/loadbalancer.go @@ -28,6 +28,7 @@ import ( type LoadBalancersClient interface { CreateOrUpdate(ctx context.Context, resourceGroupName, loadBalancerName string, parameters network.LoadBalancer) error List(ctx context.Context, resourceGroupName string) ([]network.LoadBalancer, error) + Get(ctx context.Context, resourceGroupName string, loadBalancerName string) (*network.LoadBalancer, error) Delete(ctx context.Context, resourceGroupName, loadBalancerName string) error } @@ -53,6 +54,14 @@ func (c *loadBalancersClientImpl) List(ctx context.Context, resourceGroupName st return l, nil } +func (c *loadBalancersClientImpl) Get(ctx context.Context, resourceGroupName string, loadBalancerName string) (*network.LoadBalancer, error) { + l, err := c.c.Get(ctx, resourceGroupName, loadBalancerName, "frontendIpConfigurations/publicIpAddress") + if err != nil { + return nil, err + } + return &l, nil +} + func (c *loadBalancersClientImpl) Delete(ctx context.Context, resourceGroupName, loadBalancerName string) error { future, err := c.c.Delete(ctx, resourceGroupName, loadBalancerName) if err != nil { diff --git a/upup/pkg/fi/cloudup/azure/verifier.go b/upup/pkg/fi/cloudup/azure/verifier.go new file mode 100644 index 0000000000000..3b275e04ef593 --- /dev/null +++ b/upup/pkg/fi/cloudup/azure/verifier.go @@ -0,0 +1,145 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package azure + +import ( + "context" + "fmt" + "net" + "net/http" + "strconv" + "strings" + + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute" + "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 { +} + +type azureVerifier struct { + client *client +} + +var _ bootstrap.Verifier = &azureVerifier{} + +func NewAzureVerifier(ctx context.Context, opt *AzureVerifierOptions) (bootstrap.Verifier, error) { + azureClient, err := newClient() + if err != nil { + return nil, err + } + + return &azureVerifier{ + client: azureClient, + }, nil +} + +func (a azureVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request, token string, body []byte, useInstanceIDForNodeName bool) (*bootstrap.VerifyResult, error) { + if !strings.HasPrefix(token, AzureAuthenticationTokenPrefix) { + return nil, fmt.Errorf("incorrect authorization type") + } + + v := strings.Split(strings.TrimPrefix(token, AzureAuthenticationTokenPrefix), " ") + if len(v) != 2 { + return nil, fmt.Errorf("incorrect token format") + } + vmssName := v[0] + vmssIndex := v[1] + + 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) + } + if vm.OsProfile == nil || *vm.OsProfile.ComputerName == "" { + return nil, fmt.Errorf("determining ComputerName for VMSS %q virtual machine #%s", vmssName, vmssIndex) + } + + ni, err := a.client.nisClient.GetVirtualMachineScaleSetNetworkInterface(ctx, a.client.resourceGroup, vmssName, vmssIndex, vmssName+"-netconfig", "") + if err != nil { + return nil, fmt.Errorf("getting info for VMSS network interface %q #%s: %w", vmssName, vmssIndex, err) + } + + var addrs []string + var challengeEndpoints []string + for _, ipc := range *ni.IPConfigurations { + if ipc.PrivateIPAddress != nil { + addrs = append(addrs, *ipc.PrivateIPAddress) + challengeEndpoints = append(challengeEndpoints, net.JoinHostPort(*ipc.PrivateIPAddress, strconv.Itoa(wellknownports.NodeupChallenge))) + } + } + if len(addrs) == 0 { + return nil, fmt.Errorf("determining challenge endpoint for VMSS %q virtual machine #%s", vmssName, vmssIndex) + } + if len(challengeEndpoints) == 0 { + return nil, fmt.Errorf("determining challenge endpoint for VMSS %q virtual machine #%s", vmssName, vmssIndex) + } + + result := &bootstrap.VerifyResult{ + NodeName: *vm.OsProfile.ComputerName, + CertificateNames: addrs, + ChallengeEndpoint: challengeEndpoints[0], + } + + for key, value := range vm.Tags { + if key == azure.InstanceGroupNameTag && value != nil { + result.InstanceGroupName = *value + } + } + + return result, nil +} + +// client is an Azure client. +type client struct { + resourceGroup string + nisClient *network.InterfacesClient + vmsClient *compute.VirtualMachineScaleSetVMsClient +} + +// newClient returns a new Client. +func newClient() (*client, error) { + m, err := queryInstanceMetadata() + if err != nil || m == nil { + return nil, fmt.Errorf("getting instance metadata: %w", err) + } + if m.Compute == nil || m.Compute.ResourceGroupName == "" { + return nil, fmt.Errorf("empty resource group name") + } + if m.Compute == nil || m.Compute.SubscriptionID == "" { + return nil, fmt.Errorf("empty subscription name") + } + + authorizer, err := auth.NewAuthorizerFromEnvironment() + if err != nil { + return nil, fmt.Errorf("creating authorizer: %w", err) + } + + nisClient := network.NewInterfacesClient(m.Compute.SubscriptionID) + nisClient.Authorizer = authorizer + vmsClient := compute.NewVirtualMachineScaleSetVMsClient(m.Compute.SubscriptionID) + vmsClient.Authorizer = authorizer + + return &client{ + resourceGroup: m.Compute.ResourceGroupName, + nisClient: &nisClient, + vmsClient: &vmsClient, + }, nil +} diff --git a/upup/pkg/fi/cloudup/azuretasks/loadbalancer.go b/upup/pkg/fi/cloudup/azuretasks/loadbalancer.go index 320927dc2809e..bda71e0396344 100644 --- a/upup/pkg/fi/cloudup/azuretasks/loadbalancer.go +++ b/upup/pkg/fi/cloudup/azuretasks/loadbalancer.go @@ -19,10 +19,12 @@ package azuretasks import ( "context" "fmt" + "strings" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-05-01/network" "github.com/Azure/go-autorest/autorest/to" "k8s.io/klog/v2" + "k8s.io/kops/pkg/wellknownports" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/azure" ) @@ -58,6 +60,29 @@ func (lb *LoadBalancer) IsForAPIServer() bool { return lb.ForAPIServer } +func (lb *LoadBalancer) FindAddresses(c *fi.CloudupContext) ([]string, error) { + cloud := c.T.Cloud.(azure.AzureCloud) + loadbalancer, err := cloud.LoadBalancer().Get(context.TODO(), *lb.ResourceGroup.Name, *lb.Name) + if err != nil && !strings.Contains(err.Error(), "NotFound") { + return nil, err + } + + if loadbalancer != nil && loadbalancer.FrontendIPConfigurations != nil && len(*loadbalancer.FrontendIPConfigurations) > 0 { + var addresses []string + for _, fipc := range *loadbalancer.FrontendIPConfigurations { + if fipc.PrivateIPAddress != nil { + addresses = append(addresses, *fipc.PrivateIPAddress) + } + if fipc.PublicIPAddress != nil && fipc.PublicIPAddress.IPAddress != nil { + addresses = append(addresses, *fipc.PublicIPAddress.IPAddress) + } + } + return addresses, nil + } + + return nil, nil +} + // Find discovers the LoadBalancer in the cloud provider func (lb *LoadBalancer) Find(c *fi.CloudupContext) (*LoadBalancer, error) { cloud := c.T.Cloud.(azure.AzureCloud) @@ -151,6 +176,7 @@ func (*LoadBalancer) RenderAzure(t *azure.AzureAPITarget, a, e, changes *LoadBal ID: to.StringPtr(fmt.Sprintf("/%s/virtualNetworks/%s/subnets/%s", idPrefix, *e.Subnet.VirtualNetwork.Name, *e.Subnet.Name)), } } + // TODO: Move hardcoded values to the model lb := network.LoadBalancer{ Location: to.StringPtr(t.Cloud.Region()), Sku: &network.LoadBalancerSku{ @@ -173,7 +199,16 @@ func (*LoadBalancer) RenderAzure(t *azure.AzureAPITarget, a, e, changes *LoadBal Name: to.StringPtr("Health-TCP-443"), ProbePropertiesFormat: &network.ProbePropertiesFormat{ Protocol: network.ProbeProtocolTCP, - Port: to.Int32Ptr(443), + Port: to.Int32Ptr(wellknownports.KubeAPIServer), + IntervalInSeconds: to.Int32Ptr(15), + NumberOfProbes: to.Int32Ptr(4), + }, + }, + { + Name: to.StringPtr("Health-TCP-3988"), + ProbePropertiesFormat: &network.ProbePropertiesFormat{ + Protocol: network.ProbeProtocolTCP, + Port: to.Int32Ptr(wellknownports.KopsControllerPort), IntervalInSeconds: to.Int32Ptr(15), NumberOfProbes: to.Int32Ptr(4), }, @@ -184,8 +219,8 @@ func (*LoadBalancer) RenderAzure(t *azure.AzureAPITarget, a, e, changes *LoadBal Name: to.StringPtr("TCP-443"), LoadBalancingRulePropertiesFormat: &network.LoadBalancingRulePropertiesFormat{ Protocol: network.TransportProtocolTCP, - FrontendPort: to.Int32Ptr(443), - BackendPort: to.Int32Ptr(443), + FrontendPort: to.Int32Ptr(wellknownports.KubeAPIServer), + BackendPort: to.Int32Ptr(wellknownports.KubeAPIServer), IdleTimeoutInMinutes: to.Int32Ptr(4), EnableFloatingIP: to.BoolPtr(false), LoadDistribution: network.LoadDistributionDefault, @@ -200,6 +235,26 @@ func (*LoadBalancer) RenderAzure(t *azure.AzureAPITarget, a, e, changes *LoadBal }, }, }, + { + Name: to.StringPtr("TCP-3988"), + LoadBalancingRulePropertiesFormat: &network.LoadBalancingRulePropertiesFormat{ + Protocol: network.TransportProtocolTCP, + FrontendPort: to.Int32Ptr(wellknownports.KopsControllerPort), + BackendPort: to.Int32Ptr(wellknownports.KopsControllerPort), + IdleTimeoutInMinutes: to.Int32Ptr(4), + EnableFloatingIP: to.BoolPtr(false), + LoadDistribution: network.LoadDistributionDefault, + FrontendIPConfiguration: &network.SubResource{ + ID: to.StringPtr(fmt.Sprintf("/%s/loadbalancers/%s/frontendIPConfigurations/%s", idPrefix, *e.Name, *to.StringPtr("LoadBalancerFrontEnd"))), + }, + BackendAddressPool: &network.SubResource{ + ID: to.StringPtr(fmt.Sprintf("/%s/loadbalancers/%s/backendAddressPools/%s", idPrefix, *e.Name, *to.StringPtr("LoadBalancerBackEnd"))), + }, + Probe: &network.SubResource{ + ID: to.StringPtr(fmt.Sprintf("/%s/loadbalancers/%s/probes/%s", idPrefix, *e.Name, *to.StringPtr("Health-TCP-3988"))), + }, + }, + }, }, }, Tags: e.Tags, diff --git a/upup/pkg/fi/cloudup/azuretasks/testing.go b/upup/pkg/fi/cloudup/azuretasks/testing.go index dd2ce904a2186..0ceb2a6e8b27c 100644 --- a/upup/pkg/fi/cloudup/azuretasks/testing.go +++ b/upup/pkg/fi/cloudup/azuretasks/testing.go @@ -576,6 +576,16 @@ func (c *MockLoadBalancersClient) List(ctx context.Context, resourceGroupName st return l, nil } +// Get returns a loadbalancer. +func (c *MockLoadBalancersClient) Get(ctx context.Context, resourceGroupName string, loadBalancerName string) (*network.LoadBalancer, error) { + for _, lb := range c.LBs { + if *lb.Name == loadBalancerName { + return nil, nil + } + } + return nil, nil +} + // Delete deletes a specified loadbalancer. func (c *MockLoadBalancersClient) Delete(ctx context.Context, scope, lbName string) error { // Ignore scope for simplicity. diff --git a/upup/pkg/fi/cloudup/dns_test.go b/upup/pkg/fi/cloudup/dns_test.go index 92ada0fafba33..2df0088d8af6a 100644 --- a/upup/pkg/fi/cloudup/dns_test.go +++ b/upup/pkg/fi/cloudup/dns_test.go @@ -30,84 +30,6 @@ func TestPrecreateDNSNames(t *testing.T) { cluster *kops.Cluster expected []recordKey }{ - { - cluster: &kops.Cluster{ - Spec: kops.ClusterSpec{ - CloudProvider: kops.CloudProviderSpec{ - Azure: &kops.AzureSpec{}, - }, - }, - }, - expected: []recordKey{ - {"api.cluster1.example.com", rrstype.A}, - {"api.internal.cluster1.example.com", rrstype.A}, - }, - }, - { - cluster: &kops.Cluster{ - Spec: kops.ClusterSpec{ - CloudProvider: kops.CloudProviderSpec{ - Azure: &kops.AzureSpec{}, - }, - Networking: kops.NetworkingSpec{ - NonMasqueradeCIDR: "::/0", - }, - }, - }, - expected: []recordKey{ - {"api.cluster1.example.com", rrstype.A}, - {"api.cluster1.example.com", rrstype.AAAA}, - {"api.internal.cluster1.example.com", rrstype.AAAA}, - }, - }, - { - cluster: &kops.Cluster{ - Spec: kops.ClusterSpec{ - API: kops.APISpec{ - LoadBalancer: &kops.LoadBalancerAccessSpec{}, - }, - CloudProvider: kops.CloudProviderSpec{ - Azure: &kops.AzureSpec{}, - }, - }, - }, - expected: []recordKey{ - {"api.internal.cluster1.example.com", rrstype.A}, - }, - }, - { - cluster: &kops.Cluster{ - Spec: kops.ClusterSpec{ - API: kops.APISpec{ - LoadBalancer: &kops.LoadBalancerAccessSpec{}, - }, - CloudProvider: kops.CloudProviderSpec{ - Azure: &kops.AzureSpec{}, - }, - Networking: kops.NetworkingSpec{ - NonMasqueradeCIDR: "::/0", - }, - }, - }, - expected: []recordKey{ - {"api.internal.cluster1.example.com", rrstype.AAAA}, - }, - }, - { - cluster: &kops.Cluster{ - Spec: kops.ClusterSpec{ - API: kops.APISpec{ - LoadBalancer: &kops.LoadBalancerAccessSpec{ - UseForInternalAPI: true, - }, - }, - CloudProvider: kops.CloudProviderSpec{ - Azure: &kops.AzureSpec{}, - }, - }, - }, - expected: nil, - }, { cluster: &kops.Cluster{ Spec: kops.ClusterSpec{ diff --git a/upup/pkg/fi/cloudup/new_cluster.go b/upup/pkg/fi/cloudup/new_cluster.go index 5db0c6d1db4f9..f80d3815e514f 100644 --- a/upup/pkg/fi/cloudup/new_cluster.go +++ b/upup/pkg/fi/cloudup/new_cluster.go @@ -1321,7 +1321,7 @@ func setupDNSTopology(opt *NewClusterOptions, cluster *api.Cluster) error { switch strings.ToLower(opt.DNSType) { case "": switch cluster.Spec.GetCloudProvider() { - case api.CloudProviderHetzner, api.CloudProviderDO: + case api.CloudProviderHetzner, api.CloudProviderDO, api.CloudProviderAzure: // Use dns=none if not specified cluster.Spec.Networking.Topology.DNS = api.DNSTypeNone default: diff --git a/upup/pkg/fi/cloudup/template_functions.go b/upup/pkg/fi/cloudup/template_functions.go index 33e8c8ead6a2f..f79dbcbfccc08 100644 --- a/upup/pkg/fi/cloudup/template_functions.go +++ b/upup/pkg/fi/cloudup/template_functions.go @@ -62,6 +62,7 @@ import ( "k8s.io/kops/pkg/wellknownports" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/awsup" + "k8s.io/kops/upup/pkg/fi/cloudup/azure" "k8s.io/kops/upup/pkg/fi/cloudup/do" "k8s.io/kops/upup/pkg/fi/cloudup/gce" gcetpm "k8s.io/kops/upup/pkg/fi/cloudup/gce/tpm" @@ -739,6 +740,9 @@ func (tf *TemplateFunctions) KopsControllerConfig() (string, error) { case kops.CloudProviderScaleway: config.Server.Provider.Scaleway = &scaleway.ScalewayVerifierOptions{} + case kops.CloudProviderAzure: + config.Server.Provider.Azure = &azure.AzureVerifierOptions{} + default: return "", fmt.Errorf("unsupported cloud provider %s", cluster.Spec.GetCloudProvider()) } diff --git a/upup/pkg/fi/nodeup/command.go b/upup/pkg/fi/nodeup/command.go index 31fe13c83ece2..f3eb8194f8108 100644 --- a/upup/pkg/fi/nodeup/command.go +++ b/upup/pkg/fi/nodeup/command.go @@ -54,6 +54,7 @@ import ( "k8s.io/kops/pkg/wellknownports" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/awsup" + "k8s.io/kops/upup/pkg/fi/cloudup/azure" "k8s.io/kops/upup/pkg/fi/cloudup/do" "k8s.io/kops/upup/pkg/fi/cloudup/gce/gcediscovery" "k8s.io/kops/upup/pkg/fi/cloudup/gce/tpm/gcetpmsigner" @@ -776,6 +777,12 @@ func getNodeConfigFromServers(ctx context.Context, bootConfig *nodeup.BootConfig return nil, err } authenticator = a + case api.CloudProviderAzure: + a, err := azure.NewAzureAuthenticator() + if err != nil { + return nil, err + } + authenticator = a default: return nil, fmt.Errorf("unsupported cloud provider for node configuration %s", bootConfig.CloudProvider) } From ecbcd7a66c08a1aed1abab8495d88e825829c022 Mon Sep 17 00:00:00 2001 From: Ciprian Hacman Date: Fri, 14 Jul 2023 07:23:17 +0300 Subject: [PATCH 2/5] aws: Add more DNS pre-creation tests --- upup/pkg/fi/cloudup/dns_test.go | 54 +++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/upup/pkg/fi/cloudup/dns_test.go b/upup/pkg/fi/cloudup/dns_test.go index 2df0088d8af6a..9151289d4ba71 100644 --- a/upup/pkg/fi/cloudup/dns_test.go +++ b/upup/pkg/fi/cloudup/dns_test.go @@ -30,13 +30,64 @@ func TestPrecreateDNSNames(t *testing.T) { cluster *kops.Cluster expected []recordKey }{ + { + cluster: &kops.Cluster{ + Spec: kops.ClusterSpec{ + API: kops.APISpec{ + LoadBalancer: &kops.LoadBalancerAccessSpec{}, + }, + CloudProvider: kops.CloudProviderSpec{ + AWS: &kops.AWSSpec{}, + }, + }, + }, + expected: []recordKey{ + {"api.internal.cluster1.example.com", rrstype.A}, + {"kops-controller.internal.cluster1.example.com", rrstype.A}, + }, + }, + { + cluster: &kops.Cluster{ + Spec: kops.ClusterSpec{ + API: kops.APISpec{ + LoadBalancer: &kops.LoadBalancerAccessSpec{}, + }, + CloudProvider: kops.CloudProviderSpec{ + AWS: &kops.AWSSpec{}, + }, + Networking: kops.NetworkingSpec{ + NonMasqueradeCIDR: "::/0", + }, + }, + }, + expected: []recordKey{ + {"api.internal.cluster1.example.com", rrstype.AAAA}, + {"kops-controller.internal.cluster1.example.com", rrstype.AAAA}, + }, + }, + { + cluster: &kops.Cluster{ + Spec: kops.ClusterSpec{ + API: kops.APISpec{ + LoadBalancer: &kops.LoadBalancerAccessSpec{ + UseForInternalAPI: true, + }, + }, + CloudProvider: kops.CloudProviderSpec{ + AWS: &kops.AWSSpec{}, + }, + }, + }, + expected: []recordKey{ + {"kops-controller.internal.cluster1.example.com", rrstype.A}, + }, + }, { cluster: &kops.Cluster{ Spec: kops.ClusterSpec{ CloudProvider: kops.CloudProviderSpec{ AWS: &kops.AWSSpec{}, }, - KubernetesVersion: "1.22.0", }, }, expected: []recordKey{ @@ -51,7 +102,6 @@ func TestPrecreateDNSNames(t *testing.T) { CloudProvider: kops.CloudProviderSpec{ AWS: &kops.AWSSpec{}, }, - KubernetesVersion: "1.22.0", Networking: kops.NetworkingSpec{ NonMasqueradeCIDR: "::/0", }, From 15b44bad52bc8c13cab86b5a873c9a439e185513 Mon Sep 17 00:00:00 2001 From: Ciprian Hacman Date: Fri, 14 Jul 2023 13:46:26 +0300 Subject: [PATCH 3/5] azure: Remove permissions for nodes when dns=none --- pkg/model/azuremodel/vmscaleset.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/pkg/model/azuremodel/vmscaleset.go b/pkg/model/azuremodel/vmscaleset.go index 7f62b013ca07b..684b79babc8f5 100644 --- a/pkg/model/azuremodel/vmscaleset.go +++ b/pkg/model/azuremodel/vmscaleset.go @@ -49,17 +49,19 @@ func (b *VMScaleSetModelBuilder) Build(c *fi.CloudupModelBuilderContext) error { } c.AddTask(vmss) - // Create tasks for assigning built-in roles to VM Scale Sets. - // See https://docs.microsoft.com/en-us/azure/role-based-access-control/built-in-roles - // for the ID definitions. - roleDefIDs := map[string]string{ - // Owner - "owner": "8e3af657-a8ff-443c-a75c-2fe8c4bcb635", - // Storage Blob Data Contributor - "blob": "ba92f5b4-2d11-453d-a403-e96b0029c9fe", - } - for k, roleDefID := range roleDefIDs { - c.AddTask(b.buildRoleAssignmentTask(vmss, k, roleDefID)) + if ig.IsControlPlane() || b.Cluster.UsesLegacyGossip() { + // Create tasks for assigning built-in roles to VM Scale Sets. + // See https://docs.microsoft.com/en-us/azure/role-based-access-control/built-in-roles + // for the ID definitions. + roleDefIDs := map[string]string{ + // Owner + "owner": "8e3af657-a8ff-443c-a75c-2fe8c4bcb635", + // Storage Blob Data Contributor + "blob": "ba92f5b4-2d11-453d-a403-e96b0029c9fe", + } + for k, roleDefID := range roleDefIDs { + c.AddTask(b.buildRoleAssignmentTask(vmss, k, roleDefID)) + } } } From 576ef5ea480bb4a3144c8a59c8c0059e7a7de965 Mon Sep 17 00:00:00 2001 From: Ciprian Hacman Date: Sat, 15 Jul 2023 09:14:58 +0300 Subject: [PATCH 4/5] azure: Verify VM ID when registering nodes --- upup/pkg/fi/cloudup/azure/authenticator.go | 9 +++++++-- upup/pkg/fi/cloudup/azure/verifier.go | 13 ++++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/upup/pkg/fi/cloudup/azure/authenticator.go b/upup/pkg/fi/cloudup/azure/authenticator.go index 52e56987badb0..132de76d91e2a 100644 --- a/upup/pkg/fi/cloudup/azure/authenticator.go +++ b/upup/pkg/fi/cloudup/azure/authenticator.go @@ -43,23 +43,28 @@ func (h *azureAuthenticator) CreateToken(body []byte) (string, error) { return "", fmt.Errorf("querying instance metadata: %w", err) } + vmId := m.Compute.VMID + if vmId == "" { + return "", fmt.Errorf("missing virtual machine ID") + } + // The fully qualified VMSS VM resource ID format is: // /subscriptions/SUBSCRIPTION_ID/resourceGroups/RESOURCE_GROUP_NAME/providers/Microsoft.Compute/virtualMachineScaleSets/VMSS_NAME/virtualMachines/VMSS_INDEX r := strings.Split(m.Compute.ResourceID, "/") if len(r) != 11 || r[7] != "virtualMachineScaleSets" || r[9] != "virtualMachines" { return "", fmt.Errorf("unexpected resource ID format: %q", m.Compute.ResourceID) } - vmssName := r[8] vmssIndex := r[10] - return AzureAuthenticationTokenPrefix + vmssName + " " + vmssIndex, nil + return AzureAuthenticationTokenPrefix + vmId + " " + vmssName + " " + vmssIndex, nil } type instanceComputeMetadata struct { ResourceGroupName string `json:"resourceGroupName"` ResourceID string `json:"resourceId"` SubscriptionID string `json:"subscriptionId"` + VMID string `json:"vmId"` } type instanceMetadata struct { diff --git a/upup/pkg/fi/cloudup/azure/verifier.go b/upup/pkg/fi/cloudup/azure/verifier.go index 3b275e04ef593..3ce31bd7b6f77 100644 --- a/upup/pkg/fi/cloudup/azure/verifier.go +++ b/upup/pkg/fi/cloudup/azure/verifier.go @@ -58,16 +58,23 @@ func (a azureVerifier) VerifyToken(ctx context.Context, rawRequest *http.Request } v := strings.Split(strings.TrimPrefix(token, AzureAuthenticationTokenPrefix), " ") - if len(v) != 2 { + if len(v) != 3 { return nil, fmt.Errorf("incorrect token format") } - vmssName := v[0] - vmssIndex := v[1] + vmId := v[0] + vmssName := v[1] + vmssIndex := v[2] 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) } + if vm.VMID == nil { + 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) + } if vm.OsProfile == nil || *vm.OsProfile.ComputerName == "" { return nil, fmt.Errorf("determining ComputerName for VMSS %q virtual machine #%s", vmssName, vmssIndex) } From 80944323f35bb2ba56600b3e0a4bda2f933e099c Mon Sep 17 00:00:00 2001 From: Ciprian Hacman Date: Sat, 15 Jul 2023 18:41:39 +0300 Subject: [PATCH 5/5] azure: Allow full load balancer access only when public --- pkg/model/azuremodel/network.go | 61 ++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/pkg/model/azuremodel/network.go b/pkg/model/azuremodel/network.go index 041b8d6740e04..c0b8a19528aa6 100644 --- a/pkg/model/azuremodel/network.go +++ b/pkg/model/azuremodel/network.go @@ -21,6 +21,7 @@ import ( "strconv" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-05-01/network" + "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/wellknownports" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/azuretasks" @@ -128,29 +129,43 @@ func (b *NetworkModelBuilder) Build(c *fi.CloudupModelBuilderContext) error { }) } if b.Cluster.UsesNoneDNS() { - // TODO: Limit access to necessary source address prefixes instead of "0.0.0.0/0" and "::/0" - nsgTask.SecurityRules = append(nsgTask.SecurityRules, &azuretasks.NetworkSecurityRule{ - Name: fi.PtrTo("AllowKopsController"), - Priority: fi.PtrTo[int32](210), - Access: network.SecurityRuleAccessAllow, - Direction: network.SecurityRuleDirectionInbound, - Protocol: network.SecurityRuleProtocolTCP, - SourceAddressPrefix: fi.PtrTo("0.0.0.0/0"), - SourcePortRange: fi.PtrTo("*"), - DestinationAddressPrefix: fi.PtrTo("*"), - DestinationPortRange: fi.PtrTo(strconv.Itoa(wellknownports.KopsControllerPort)), - }) - nsgTask.SecurityRules = append(nsgTask.SecurityRules, &azuretasks.NetworkSecurityRule{ - Name: fi.PtrTo("AllowKopsController_v6"), - Priority: fi.PtrTo[int32](211), - Access: network.SecurityRuleAccessAllow, - Direction: network.SecurityRuleDirectionInbound, - Protocol: network.SecurityRuleProtocolTCP, - SourceAddressPrefix: fi.PtrTo("::/0"), - SourcePortRange: fi.PtrTo("*"), - DestinationAddressPrefix: fi.PtrTo("*"), - DestinationPortRange: fi.PtrTo(strconv.Itoa(wellknownports.KopsControllerPort)), - }) + if b.Cluster.Spec.API.LoadBalancer != nil && b.Cluster.Spec.API.LoadBalancer.Type == kops.LoadBalancerTypeInternal { + nsgTask.SecurityRules = append(nsgTask.SecurityRules, &azuretasks.NetworkSecurityRule{ + Name: fi.PtrTo("AllowKopsController"), + Priority: fi.PtrTo[int32](210), + 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.KopsControllerPort)), + }) + } else { + // TODO: Limit access to necessary source address prefixes instead of "0.0.0.0/0" and "::/0" + nsgTask.SecurityRules = append(nsgTask.SecurityRules, &azuretasks.NetworkSecurityRule{ + Name: fi.PtrTo("AllowKopsController"), + Priority: fi.PtrTo[int32](210), + Access: network.SecurityRuleAccessAllow, + Direction: network.SecurityRuleDirectionInbound, + Protocol: network.SecurityRuleProtocolTCP, + SourceAddressPrefix: fi.PtrTo("0.0.0.0/0"), + SourcePortRange: fi.PtrTo("*"), + DestinationAddressPrefix: fi.PtrTo("*"), + DestinationPortRange: fi.PtrTo(strconv.Itoa(wellknownports.KopsControllerPort)), + }) + nsgTask.SecurityRules = append(nsgTask.SecurityRules, &azuretasks.NetworkSecurityRule{ + Name: fi.PtrTo("AllowKopsController_v6"), + Priority: fi.PtrTo[int32](211), + Access: network.SecurityRuleAccessAllow, + Direction: network.SecurityRuleDirectionInbound, + Protocol: network.SecurityRuleProtocolTCP, + SourceAddressPrefix: fi.PtrTo("::/0"), + SourcePortRange: fi.PtrTo("*"), + DestinationAddressPrefix: fi.PtrTo("*"), + DestinationPortRange: fi.PtrTo(strconv.Itoa(wellknownports.KopsControllerPort)), + }) + } } var nodePortAccessIPv4, nodePortAccessIPv6 []string for _, cidr := range b.Cluster.Spec.NodePortAccess {