diff --git a/pkg/controller/bastion/actuator.go b/pkg/controller/bastion/actuator.go index 506c965d5..f2a85ed72 100644 --- a/pkg/controller/bastion/actuator.go +++ b/pkg/controller/bastion/actuator.go @@ -183,19 +183,19 @@ func getPublicIP(ctx context.Context, factory azureclient.Factory, opt *Options) return ip, nil } -func getSubnet(ctx context.Context, factory azureclient.Factory, opt *Options) (*network.Subnet, error) { +func getSubnet(ctx context.Context, factory azureclient.Factory, vNet, subnetWork string, opt *Options) (*network.Subnet, error) { subnetClient, err := factory.Subnet(ctx, opt.SecretReference) if err != nil { return nil, err } - subnet, err := subnetClient.Get(ctx, opt.ResourceGroupName, opt.VirtualNetwork, opt.Subnetwork, "") + subnet, err := subnetClient.Get(ctx, opt.ResourceGroupName, vNet, subnetWork, "") if err != nil { return nil, err } if subnet == nil { - logger.Info("subnet not found,", "subnet_name", opt.Subnetwork) + logger.Info("subnet not found,", "subnet_name", subnetWork) return nil, nil } diff --git a/pkg/controller/bastion/actuator_delete.go b/pkg/controller/bastion/actuator_delete.go index 7a0d4e309..292d41e8b 100644 --- a/pkg/controller/bastion/actuator_delete.go +++ b/pkg/controller/bastion/actuator_delete.go @@ -35,7 +35,12 @@ func (a *actuator) Delete(ctx context.Context, bastion *extensionsv1alpha1.Basti var factory = azureclient.NewAzureClientFactory(a.client) - opt, err := DetermineOptions(bastion, cluster) + infrastructureStatus, err := getInfrastructureStatus(ctx, a, cluster) + if err != nil { + return err + } + + opt, err := DetermineOptions(bastion, cluster, infrastructureStatus.ResourceGroup.Name) if err != nil { return err } diff --git a/pkg/controller/bastion/actuator_reconcile.go b/pkg/controller/bastion/actuator_reconcile.go index 453ac2d54..f765720c6 100644 --- a/pkg/controller/bastion/actuator_reconcile.go +++ b/pkg/controller/bastion/actuator_reconcile.go @@ -21,12 +21,15 @@ import ( "net" "time" + "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure" + "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure/helper" azureclient "github.com/gardener/gardener-extension-provider-azure/pkg/azure/client" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-05-01/network" "github.com/gardener/gardener/extensions/pkg/controller" extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" ctrlerror "github.com/gardener/gardener/pkg/controllerutils/reconciler" + "github.com/gardener/gardener/pkg/extensions" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -52,7 +55,12 @@ func (a *actuator) Reconcile(ctx context.Context, bastion *extensionsv1alpha1.Ba factory = azureclient.NewAzureClientFactory(a.client) ) - opt, err := DetermineOptions(bastion, cluster) + infrastructureStatus, err := getInfrastructureStatus(ctx, a, cluster) + if err != nil { + return err + } + + opt, err := DetermineOptions(bastion, cluster, infrastructureStatus.ResourceGroup.Name) if err != nil { return err } @@ -62,7 +70,15 @@ func (a *actuator) Reconcile(ctx context.Context, bastion *extensionsv1alpha1.Ba return err } - nic, err := ensureNic(ctx, factory, opt, publicIP) + if infrastructureStatus.Networks.Layout != "SingleSubnet" { + return fmt.Errorf("unsupported network layout %s", infrastructureStatus.Networks.Layout) + } + + if infrastructureStatus.Networks.VNet.Name == "" || len(infrastructureStatus.Networks.Subnets) == 0 { + return errors.New("virtual network name and subnet must be set") + } + + nic, err := ensureNic(ctx, factory, opt, infrastructureStatus.Networks.VNet.Name, infrastructureStatus.Networks.Subnets[0].Name, publicIP) if err != nil { return err } @@ -110,6 +126,28 @@ func (a *actuator) Reconcile(ctx context.Context, bastion *extensionsv1alpha1.Ba return a.client.Status().Patch(ctx, bastion, patch) } +func getInfrastructureStatus(ctx context.Context, a *actuator, cluster *extensions.Cluster) (*azure.InfrastructureStatus, error) { + var infrastructureStatus *azure.InfrastructureStatus + worker := &extensionsv1alpha1.Worker{} + err := a.client.Get(ctx, client.ObjectKey{Namespace: cluster.ObjectMeta.Name, Name: cluster.Shoot.Name}, worker) + if err != nil { + return nil, err + } + + if worker == nil || worker.Spec.InfrastructureProviderStatus == nil { + return nil, errors.New("infrastructure provider status must be not empty for worker") + } + + if infrastructureStatus, err = helper.InfrastructureStatusFromRaw(worker.Spec.InfrastructureProviderStatus); err != nil { + return nil, err + } + + if infrastructureStatus.ResourceGroup.Name == "" { + return nil, errors.New("resource group name must be not empty for infrastructure provider status") + } + return infrastructureStatus, nil +} + func getPrivateIPv4Address(nic *network.Interface) (string, error) { if nic.IPConfigurations == nil { return "", fmt.Errorf("nic.IPConfigurations %s is nil", *nic.ID) @@ -257,7 +295,7 @@ func ensureComputeInstance(ctx context.Context, logger logr.Logger, bastion *ext return nil } -func ensureNic(ctx context.Context, factory azureclient.Factory, opt *Options, publicIP *network.PublicIPAddress) (*network.Interface, error) { +func ensureNic(ctx context.Context, factory azureclient.Factory, opt *Options, vNet, subnetWork string, publicIP *network.PublicIPAddress) (*network.Interface, error) { nic, err := getNic(ctx, factory, opt) if err != nil { return nil, err @@ -271,7 +309,7 @@ func ensureNic(ctx context.Context, factory azureclient.Factory, opt *Options, p logger.Info("create new bastion compute instance nic") - subnet, err := getSubnet(ctx, factory, opt) + subnet, err := getSubnet(ctx, factory, vNet, subnetWork, opt) if err != nil { return nil, err } @@ -388,7 +426,7 @@ func addOrReplaceNsgRulesDefinition(existingRules *[]network.SecurityRule, desir // filter rules intended to be replaced for _, existentRule := range *existingRules { - if ruleExist(existentRule.Name, desiredRules) { + if RuleExist(existentRule.Name, desiredRules) { continue } result = append(result, existentRule) @@ -403,7 +441,8 @@ func addOrReplaceNsgRulesDefinition(existingRules *[]network.SecurityRule, desir *existingRules = result } -func ruleExist(ruleName *string, rules *[]network.SecurityRule) bool { +// RuleExist checks if the rule with the given name is present in the list of rules. +func RuleExist(ruleName *string, rules *[]network.SecurityRule) bool { if ruleName == nil { return false } diff --git a/pkg/controller/bastion/bastion_test.go b/pkg/controller/bastion/bastion_test.go index df28dbafa..18742c14b 100644 --- a/pkg/controller/bastion/bastion_test.go +++ b/pkg/controller/bastion/bastion_test.go @@ -161,13 +161,11 @@ var _ = Describe("Bastion test", func() { Describe("Determine options", func() { It("should return options", func() { - options, err := DetermineOptions(bastion, cluster) + options, err := DetermineOptions(bastion, cluster, "cluster1") Expect(err).To(Not(HaveOccurred())) Expect(options.BastionInstanceName).To(Equal("cluster1-bastionName1-bastion-1cdc8")) - Expect(options.Subnetwork).To(Equal("cluster1-nodes")) Expect(options.BastionPublicIPName).To(Equal("cluster1-bastionName1-bastion-1cdc8-public-ip")) - Expect(options.VirtualNetwork).To(Equal("cluster1")) Expect(options.SecretReference).To(Equal(corev1.SecretReference{ Namespace: "cluster1", Name: "cloudprovider", diff --git a/pkg/controller/bastion/options.go b/pkg/controller/bastion/options.go index 73c07b3b7..d4b8e6e42 100644 --- a/pkg/controller/bastion/options.go +++ b/pkg/controller/bastion/options.go @@ -33,7 +33,7 @@ const maxLengthForBaseName = 33 // Options contains provider-related information required for setting up // a bastion instance. This struct combines precomputed values like the // bastion instance name with the IDs of pre-existing cloud provider -// resources, like the nic name, subnet name etc. +// resources, like the nic name etc. type Options struct { BastionInstanceName string BastionPublicIPName string @@ -45,8 +45,6 @@ type Options struct { NicName string NicID string DiskName string - Subnetwork string - VirtualNetwork string SecretReference corev1.SecretReference WorkersCIDR string CIDRs []string @@ -55,7 +53,7 @@ type Options struct { // DetermineOptions determines the information that are required to reconcile a Bastion on Azure. This // function does not create any IaaS resources. -func DetermineOptions(bastion *extensionsv1alpha1.Bastion, cluster *controller.Cluster) (*Options, error) { +func DetermineOptions(bastion *extensionsv1alpha1.Bastion, cluster *controller.Cluster, resourceGroup string) (*Options, error) { clusterName := cluster.ObjectMeta.Name baseResourceName, err := generateBastionBaseResourceName(clusterName, bastion.Name) if err != nil { @@ -84,15 +82,13 @@ func DetermineOptions(bastion *extensionsv1alpha1.Bastion, cluster *controller.C return &Options{ BastionInstanceName: baseResourceName, - Subnetwork: nodesResourceName(clusterName), BastionPublicIPName: publicIPResourceName(baseResourceName), - VirtualNetwork: clusterName, SecretReference: secretReference, CIDRs: cidrs, WorkersCIDR: workersCidr, DiskName: DiskResourceName(baseResourceName), Location: cluster.Shoot.Spec.Region, - ResourceGroupName: cluster.ObjectMeta.Name, + ResourceGroupName: resourceGroup, NicName: NicResourceName(baseResourceName), Tags: tags, SecurityGroupName: NSGName(clusterName), diff --git a/test/integration/bastion/bastion_test.go b/test/integration/bastion/bastion_test.go index 3172ef14f..a413b23f8 100644 --- a/test/integration/bastion/bastion_test.go +++ b/test/integration/bastion/bastion_test.go @@ -30,6 +30,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/manager" + apisazure "github.com/gardener/gardener-extension-provider-azure/pkg/apis/azure" + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-03-01/compute" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-05-01/network" "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-05-01/resources" @@ -51,6 +53,7 @@ import ( networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -151,16 +154,19 @@ var ( logger *logrus.Entry extensionscluster *extensionsv1alpha1.Cluster + worker *extensionsv1alpha1.Worker + bastion *extensionsv1alpha1.Bastion controllercluster *controller.Cluster options *bastionctrl.Options - bastion *extensionsv1alpha1.Bastion secret *corev1.Secret - testEnv *envtest.Environment - c client.Client - mgrCancel context.CancelFunc - clientSet *azureClientSet - name string + testEnv *envtest.Environment + c client.Client + mgrCancel context.CancelFunc + clientSet *azureClientSet + name string + vNetName string + subnetName string ) var _ = BeforeSuite(func() { @@ -168,6 +174,8 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) name = fmt.Sprintf("azure-bastion-it--%s", randString) + vNetName = name + subnetName = vNetName + "-nodes" myPublicIP, err = getMyPublicIPWithMask() Expect(err).NotTo(HaveOccurred()) @@ -193,6 +201,7 @@ var _ = BeforeSuite(func() { Paths: []string{ filepath.Join(repoRoot, "example", "20-crd-extensions.gardener.cloud_clusters.yaml"), filepath.Join(repoRoot, "example", "20-crd-extensions.gardener.cloud_bastions.yaml"), + filepath.Join(repoRoot, "example", "20-crd-extensions.gardener.cloud_workers.yaml"), }, }, } @@ -227,6 +236,7 @@ var _ = BeforeSuite(func() { extensionscluster, controllercluster = createClusters(name) bastion, options = createBastion(controllercluster, name) + worker = createWorker(name) secret = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -259,7 +269,7 @@ var _ = Describe("Bastion tests", func() { It("should successfully create and delete", func() { resourceGroupName := name - vNetName := name + securityGroupName := name + "-workers" By("setup Infrastructure") @@ -271,7 +281,7 @@ var _ = Describe("Bastion tests", func() { Expect(err).NotTo(HaveOccurred()) By("setup Virtual Network") - err = prepareNewVNet(ctx, logger, clientSet, resourceGroupName, vNetName, *region, VNetCIDR, sg) + err = prepareNewVNet(ctx, logger, clientSet, resourceGroupName, vNetName, subnetName, *region, VNetCIDR, sg) Expect(err).NotTo(HaveOccurred()) framework.AddCleanupAction(func() { @@ -280,9 +290,9 @@ var _ = Describe("Bastion tests", func() { }) By("create namespace for test execution") - setupEnvironmentObjects(ctx, c, namespace(name), secret, extensionscluster) + setupEnvironmentObjects(ctx, c, namespace(name), secret, extensionscluster, worker) framework.AddCleanupAction(func() { - teardownShootEnvironment(ctx, c, namespace(name), secret, extensionscluster) + teardownShootEnvironment(ctx, c, namespace(name), secret, extensionscluster, worker) }) By("setup bastion") @@ -413,9 +423,9 @@ func prepareSecurityGroup(ctx context.Context, logger *logrus.Entry, resourceGro return future.Result(az.securityGroups) } -func prepareNewVNet(ctx context.Context, logger *logrus.Entry, az *azureClientSet, groupName, vNetName, location, cidr string, nsg network.SecurityGroup) error { - logger.Infof("generating new VNet: %s/%s", groupName, vNetName) - vNetFuture, err := az.vnet.CreateOrUpdate(ctx, groupName, vNetName, network.VirtualNetwork{ +func prepareNewVNet(ctx context.Context, logger *logrus.Entry, az *azureClientSet, resourceGroupName, vNetName, subnetName, location, cidr string, nsg network.SecurityGroup) error { + logger.Infof("generating new resource Group/VNet/subnetName: %s/%s/%s", resourceGroupName, vNetName, subnetName) + vNetFuture, err := az.vnet.CreateOrUpdate(ctx, resourceGroupName, vNetName, network.VirtualNetwork{ VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ AddressSpace: &network.AddressSpace{ AddressPrefixes: &[]string{ @@ -424,7 +434,7 @@ func prepareNewVNet(ctx context.Context, logger *logrus.Entry, az *azureClientSe }, Subnets: &[]network.Subnet{ { - Name: to.StringPtr(groupName + "-nodes"), + Name: to.StringPtr(subnetName), SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ AddressPrefix: to.StringPtr(cidr), NetworkSecurityGroup: &nsg, @@ -477,17 +487,19 @@ func namespace(name string) *corev1.Namespace { } } -func setupEnvironmentObjects(ctx context.Context, c client.Client, namespace *corev1.Namespace, secret *corev1.Secret, cluster *extensionsv1alpha1.Cluster) { +func setupEnvironmentObjects(ctx context.Context, c client.Client, namespace *corev1.Namespace, secret *corev1.Secret, cluster *extensionsv1alpha1.Cluster, worker *extensionsv1alpha1.Worker) { Expect(c.Create(ctx, namespace)).To(Succeed()) Expect(c.Create(ctx, cluster)).To(Succeed()) Expect(c.Create(ctx, secret)).To(Succeed()) - + Expect(c.Create(ctx, worker)).To(Succeed()) } -func teardownShootEnvironment(ctx context.Context, c client.Client, namespace *corev1.Namespace, secret *corev1.Secret, cluster *extensionsv1alpha1.Cluster) { +func teardownShootEnvironment(ctx context.Context, c client.Client, namespace *corev1.Namespace, secret *corev1.Secret, cluster *extensionsv1alpha1.Cluster, worker *extensionsv1alpha1.Worker) { + Expect(client.IgnoreNotFound(c.Delete(ctx, worker))).To(Succeed()) Expect(client.IgnoreNotFound(c.Delete(ctx, secret))).To(Succeed()) Expect(client.IgnoreNotFound(c.Delete(ctx, cluster))).To(Succeed()) Expect(client.IgnoreNotFound(c.Delete(ctx, namespace))).To(Succeed()) + } func createBastion(cluster *controller.Cluster, name string) (*extensionsv1alpha1.Bastion, *bastionctrl.Options) { @@ -509,12 +521,44 @@ func createBastion(cluster *controller.Cluster, name string) (*extensionsv1alpha }, } - options, err := bastionctrl.DetermineOptions(bastion, cluster) + options, err := bastionctrl.DetermineOptions(bastion, cluster, name) Expect(err).NotTo(HaveOccurred()) return bastion, options } +func createWorker(name string) *extensionsv1alpha1.Worker { + return &extensionsv1alpha1.Worker{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: name, + }, + Spec: extensionsv1alpha1.WorkerSpec{ + DefaultSpec: extensionsv1alpha1.DefaultSpec{ + Type: azure.Type, + }, + InfrastructureProviderStatus: &runtime.RawExtension{ + Object: &apisazure.InfrastructureStatus{ + ResourceGroup: apisazure.ResourceGroup{ + Name: name, + }, + Networks: apisazure.NetworkStatus{ + Layout: apisazure.NetworkLayout("SingleSubnet"), + VNet: apisazure.VNetStatus{Name: vNetName}, + Subnets: []apisazure.Subnet{ + { + Purpose: apisazure.PurposeNodes, + Name: subnetName, + }, + }, + }, + }, + }, + Pools: []extensionsv1alpha1.WorkerPool{}, + }, + } +} + func createInfrastructureConfig() *azurev1alpha1.InfrastructureConfig { return &azurev1alpha1.InfrastructureConfig{ TypeMeta: metav1.TypeMeta{ @@ -533,6 +577,10 @@ func createShoot(infrastructureConfig []byte) *gardencorev1beta1.Shoot { APIVersion: "core.gardener.cloud/v1beta1", Kind: "Shoot", }, + + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, Spec: gardencorev1beta1.ShootSpec{ Region: *region, SecretBindingName: v1beta1constants.SecretNameCloudProvider, @@ -626,40 +674,22 @@ func verifyDeletion(ctx context.Context, az *azureClientSet, options *bastionctr } func checkSecurityRuleDoesNotExist(ctx context.Context, az *azureClientSet, options *bastionctrl.Options, securityRuleName string) { - _, err := az.securityRules.Get(ctx, options.ResourceGroupName, options.SecurityGroupName, securityRuleName) + //does not have authorization to performsecurityRules get due to global rule. use security group to check it. + sg, err := az.securityGroups.Get(ctx, options.ResourceGroupName, options.SecurityGroupName, "") + Expect(len(*sg.SecurityRules)).To(Equal(0)) Expect(ignoreAzureNotFoundError(err)).To(Succeed()) } -func checkSecurityRuleslExists(ctx context.Context, az *azureClientSet, options *bastionctrl.Options, securityRuleName string) { - nsgRule, err := az.securityRules.Get(ctx, options.ResourceGroupName, options.SecurityGroupName, securityRuleName) - Expect(err).NotTo(HaveOccurred()) - Expect(*nsgRule.Name).To(Equal(securityRuleName)) -} - func verifyCreation(ctx context.Context, az *azureClientSet, options *bastionctrl.Options) { - By("checkNSGExists") - // bastion NSG - Check Ingress / Egress firewalls created - checkSecurityRuleslExists(ctx, az, options, bastionctrl.NSGIngressAllowSSHResourceNameIPv4(options.BastionInstanceName)) - checkSecurityRuleslExists(ctx, az, options, bastionctrl.NSGEgressDenyAllResourceName(options.BastionInstanceName)) - checkSecurityRuleslExists(ctx, az, options, bastionctrl.NSGEgressAllowOnlyResourceName(options.BastionInstanceName)) - - By("checking NSG-allow-ssh rule SSHPortOpen,Public Source Ranges") - result, err := az.securityRules.Get(ctx, options.ResourceGroupName, options.SecurityGroupName, bastionctrl.NSGIngressAllowSSHResourceNameIPv4(options.BastionInstanceName)) + By("RuleExist") + //does not have authorization to performsecurityRules get due to global rule. use security group to check it. + sg, err := az.securityGroups.Get(ctx, options.ResourceGroupName, options.SecurityGroupName, "") Expect(err).NotTo(HaveOccurred()) - Expect(*result.DestinationPortRange).To(Equal("22")) - Expect(*result.SourceAddressPrefixes).To(Equal([]string{myPublicIP})) - Expect(result.SourceAddressPrefix).To(BeNil()) - By("checking Firewall-deny-all rule") - result, err = az.securityRules.Get(ctx, options.ResourceGroupName, options.SecurityGroupName, bastionctrl.NSGEgressDenyAllResourceName(options.BastionInstanceName)) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Protocol).To(Equal(network.SecurityRuleProtocolAsterisk)) - Expect(result.DestinationAddressPrefix).To(Equal(to.StringPtr("*"))) - - By("checking Firewall-egress-worker rule") - result, err = az.securityRules.Get(ctx, options.ResourceGroupName, options.SecurityGroupName, bastionctrl.NSGEgressAllowOnlyResourceName(options.BastionInstanceName)) - Expect(err).NotTo(HaveOccurred()) - Expect(*result.DestinationAddressPrefix).To(Equal(options.WorkersCIDR)) + // bastion NSG - Check Ingress / Egress firewalls created + bastionctrl.RuleExist(pointer.StringPtr(bastionctrl.NSGIngressAllowSSHResourceNameIPv4(options.BastionInstanceName)), sg.SecurityRules) + bastionctrl.RuleExist(pointer.StringPtr(bastionctrl.NSGEgressDenyAllResourceName(options.BastionInstanceName)), sg.SecurityRules) + bastionctrl.RuleExist(pointer.StringPtr(bastionctrl.NSGEgressAllowOnlyResourceName(options.BastionInstanceName)), sg.SecurityRules) By("checking bastion instance") //bastion instance