From 791672537afcf5c9c59c9da8771ce01c677b1c0b Mon Sep 17 00:00:00 2001 From: Jonathan Tong Date: Mon, 20 Dec 2021 16:54:52 -0500 Subject: [PATCH] Make bastion hosts reconcile/delete async --- api/v1beta1/azurecluster_default.go | 18 +- azure/scope/cluster.go | 41 ++- azure/services/bastionhosts/azurebastion.go | 108 -------- .../bastionhosts/azurebastion_spec.go | 101 ++++++++ azure/services/bastionhosts/bastionhosts.go | 56 +++-- .../bastionhosts/bastionhosts_test.go | 234 +++++------------- azure/services/bastionhosts/client.go | 130 +++++++--- .../mocks_bastionhosts/bastionhosts_mock.go | 103 ++++++-- .../mocks_bastionhosts/client_mock.go | 95 ------- .../bastionhosts/mocks_bastionhosts/doc.go | 2 - azure/types.go | 13 - .../ci/cluster-template-prow-private.yaml | 10 +- .../test/ci/prow-private/kustomization.yaml | 1 + .../test/ci/prow-private/patches/bastion.yaml | 15 ++ .../prow-private/patches/vnet-peerings.yaml | 2 +- test/e2e/azure_privatecluster.go | 1 + test/e2e/common.go | 1 + 17 files changed, 438 insertions(+), 493 deletions(-) delete mode 100644 azure/services/bastionhosts/azurebastion.go create mode 100644 azure/services/bastionhosts/azurebastion_spec.go delete mode 100644 azure/services/bastionhosts/mocks_bastionhosts/client_mock.go create mode 100644 templates/test/ci/prow-private/patches/bastion.yaml diff --git a/api/v1beta1/azurecluster_default.go b/api/v1beta1/azurecluster_default.go index 6d4f8211a43..614b97a79ab 100644 --- a/api/v1beta1/azurecluster_default.go +++ b/api/v1beta1/azurecluster_default.go @@ -309,19 +309,15 @@ func (c *AzureCluster) setBastionDefaults() { c.Spec.BastionSpec.AzureBastion.Name = generateAzureBastionName(c.ObjectMeta.Name) } // Ensure defaults for the Subnet settings. - { - if c.Spec.BastionSpec.AzureBastion.Subnet.Name == "" { - c.Spec.BastionSpec.AzureBastion.Subnet.Name = DefaultAzureBastionSubnetName - } - if len(c.Spec.BastionSpec.AzureBastion.Subnet.CIDRBlocks) == 0 { - c.Spec.BastionSpec.AzureBastion.Subnet.CIDRBlocks = []string{DefaultAzureBastionSubnetCIDR} - } + if c.Spec.BastionSpec.AzureBastion.Subnet.Name == "" { + c.Spec.BastionSpec.AzureBastion.Subnet.Name = DefaultAzureBastionSubnetName + } + if len(c.Spec.BastionSpec.AzureBastion.Subnet.CIDRBlocks) == 0 { + c.Spec.BastionSpec.AzureBastion.Subnet.CIDRBlocks = []string{DefaultAzureBastionSubnetCIDR} } // Ensure defaults for the PublicIP settings. - { - if c.Spec.BastionSpec.AzureBastion.PublicIP.Name == "" { - c.Spec.BastionSpec.AzureBastion.PublicIP.Name = generateAzureBastionPublicIPName(c.ObjectMeta.Name) - } + if c.Spec.BastionSpec.AzureBastion.PublicIP.Name == "" { + c.Spec.BastionSpec.AzureBastion.PublicIP.Name = generateAzureBastionPublicIPName(c.ObjectMeta.Name) } } } diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index 59f1555998a..f514e0d88d6 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -35,6 +35,7 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/bastionhosts" "sigs.k8s.io/cluster-api-provider-azure/azure/services/groups" "sigs.k8s.io/cluster-api-provider-azure/azure/services/loadbalancers" "sigs.k8s.io/cluster-api-provider-azure/azure/services/natgateways" @@ -290,7 +291,7 @@ func (s *ClusterScope) NSGSpecs() []azure.NSGSpec { // SubnetSpecs returns the subnets specs. func (s *ClusterScope) SubnetSpecs() []azure.SubnetSpec { numberOfSubnets := len(s.AzureCluster.Spec.NetworkSpec.Subnets) - if s.AzureCluster.Spec.BastionSpec.AzureBastion != nil { + if s.IsAzureBastionEnabled() { numberOfSubnets++ } @@ -308,7 +309,7 @@ func (s *ClusterScope) SubnetSpecs() []azure.SubnetSpec { subnetSpecs = append(subnetSpecs, subnetSpec) } - if s.AzureCluster.Spec.BastionSpec.AzureBastion != nil { + if s.IsAzureBastionEnabled() { azureBastionSubnet := s.AzureCluster.Spec.BastionSpec.AzureBastion.Subnet subnetSpecs = append(subnetSpecs, azure.SubnetSpec{ Name: azureBastionSubnet.Name, @@ -401,19 +402,33 @@ func (s *ClusterScope) PrivateDNSSpec() *azure.PrivateDNSSpec { return specs } -// BastionSpec returns the bastion spec. -func (s *ClusterScope) BastionSpec() azure.BastionSpec { - var ret azure.BastionSpec - if s.AzureCluster.Spec.BastionSpec.AzureBastion != nil { - ret.AzureBastion = &azure.AzureBastionSpec{ - Name: s.AzureCluster.Spec.BastionSpec.AzureBastion.Name, - SubnetSpec: s.AzureCluster.Spec.BastionSpec.AzureBastion.Subnet, - PublicIPName: s.AzureCluster.Spec.BastionSpec.AzureBastion.PublicIP.Name, - VNetName: s.Vnet().Name, +// IsAzureBastionEnabled returns true if the azure bastion is enabled. +func (s *ClusterScope) IsAzureBastionEnabled() bool { + return s.AzureCluster.Spec.BastionSpec.AzureBastion != nil +} + +// AzureBastion returns the cluster AzureBastion. +func (s *ClusterScope) AzureBastion() *infrav1.AzureBastion { + return s.AzureCluster.Spec.BastionSpec.AzureBastion +} + +// AzureBastionSpec returns the bastion spec. +func (s *ClusterScope) AzureBastionSpec() azure.ResourceSpecGetter { + if s.IsAzureBastionEnabled() { + subnetID := azure.SubnetID(s.SubscriptionID(), s.ResourceGroup(), s.Vnet().Name, s.AzureBastion().Subnet.Name) + publicIPID := azure.PublicIPID(s.SubscriptionID(), s.ResourceGroup(), s.AzureBastion().PublicIP.Name) + + return &bastionhosts.AzureBastionSpec{ + Name: s.AzureBastion().Name, + ResourceGroup: s.ResourceGroup(), + Location: s.Location(), + ClusterName: s.ClusterName(), + SubnetID: subnetID, + PublicIPID: publicIPID, } } - return ret + return nil } // Vnet returns the cluster Vnet. @@ -646,6 +661,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error { infrav1.DisksReadyCondition, infrav1.NATGatewaysReadyCondition, infrav1.LoadBalancersReadyCondition, + infrav1.BastionHostReadyCondition, ), ) @@ -661,6 +677,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error { infrav1.DisksReadyCondition, infrav1.NATGatewaysReadyCondition, infrav1.LoadBalancersReadyCondition, + infrav1.BastionHostReadyCondition, }}) } diff --git a/azure/services/bastionhosts/azurebastion.go b/azure/services/bastionhosts/azurebastion.go deleted file mode 100644 index 6ddc1bbfd1b..00000000000 --- a/azure/services/bastionhosts/azurebastion.go +++ /dev/null @@ -1,108 +0,0 @@ -/* -Copyright 2021 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 bastionhosts - -import ( - "context" - "fmt" - "strings" - - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" - "github.com/Azure/go-autorest/autorest/to" - "github.com/pkg/errors" - "sigs.k8s.io/cluster-api-provider-azure/util/tele" - - infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" - "sigs.k8s.io/cluster-api-provider-azure/azure" - "sigs.k8s.io/cluster-api-provider-azure/azure/converters" -) - -func (s *Service) ensureAzureBastion(ctx context.Context, azureBastionSpec azure.AzureBastionSpec) error { - ctx, log, done := tele.StartSpanWithLogger(ctx, "bastionhosts.Service.ensureAzureBastion") - defer done() - - log.V(2).Info("getting azure bastion public IP", "publicIP", azureBastionSpec.PublicIPName) - publicIP, err := s.publicIPsClient.Get(ctx, s.Scope.ResourceGroup(), azureBastionSpec.PublicIPName) - if err != nil { - return errors.Wrap(err, "failed to get public IP for azure bastion") - } - - log.V(2).Info("getting azure bastion subnet", "subnet", azureBastionSpec.SubnetSpec) - subnet, err := s.subnetsClient.Get(ctx, s.Scope.ResourceGroup(), azureBastionSpec.VNetName, azureBastionSpec.SubnetSpec.Name) - if err != nil { - return errors.Wrap(err, "failed to get subnet for azure bastion") - } - - log.V(2).Info("creating bastion host", "bastion", azureBastionSpec.Name) - bastionHostIPConfigName := fmt.Sprintf("%s-%s", azureBastionSpec.Name, "bastionIP") - err = s.client.CreateOrUpdate( - ctx, - s.Scope.ResourceGroup(), - azureBastionSpec.Name, - network.BastionHost{ - Name: to.StringPtr(azureBastionSpec.Name), - Location: to.StringPtr(s.Scope.Location()), - Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ - ClusterName: s.Scope.ClusterName(), - Lifecycle: infrav1.ResourceLifecycleOwned, - Name: to.StringPtr(azureBastionSpec.Name), - Role: to.StringPtr("Bastion"), - })), - BastionHostPropertiesFormat: &network.BastionHostPropertiesFormat{ - DNSName: to.StringPtr(fmt.Sprintf("%s-bastion", strings.ToLower(azureBastionSpec.Name))), - IPConfigurations: &[]network.BastionHostIPConfiguration{ - { - Name: to.StringPtr(bastionHostIPConfigName), - BastionHostIPConfigurationPropertiesFormat: &network.BastionHostIPConfigurationPropertiesFormat{ - Subnet: &network.SubResource{ - ID: subnet.ID, - }, - PublicIPAddress: &network.SubResource{ - ID: publicIP.ID, - }, - PrivateIPAllocationMethod: network.IPAllocationMethodDynamic, - }, - }, - }, - }, - }, - ) - if err != nil { - return errors.Wrap(err, "cannot create Azure Bastion") - } - - log.V(2).Info("successfully created bastion host", "bastion", azureBastionSpec.Name) - return nil -} - -func (s *Service) ensureAzureBastionDeleted(ctx context.Context, azureBastionSpec azure.AzureBastionSpec) error { - ctx, log, done := tele.StartSpanWithLogger(ctx, "bastionhosts.Service.ensureAzureBastionDeleted") - defer done() - - log.V(2).Info("deleting bastion host", "bastion", azureBastionSpec.Name) - - err := s.client.Delete(ctx, s.Scope.ResourceGroup(), azureBastionSpec.Name) - if err != nil && azure.ResourceNotFound(err) { - // Resource already deleted, all good. - } else if err != nil { - return errors.Wrapf(err, "failed to delete Azure Bastion %s in resource group %s", azureBastionSpec.Name, s.Scope.ResourceGroup()) - } - - log.V(2).Info("successfully deleted bastion host", "bastion", azureBastionSpec.Name) - - return nil -} diff --git a/azure/services/bastionhosts/azurebastion_spec.go b/azure/services/bastionhosts/azurebastion_spec.go new file mode 100644 index 00000000000..b70dda169f7 --- /dev/null +++ b/azure/services/bastionhosts/azurebastion_spec.go @@ -0,0 +1,101 @@ +/* +Copyright 2021 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 bastionhosts + +import ( + "fmt" + "strings" + + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" + "github.com/Azure/go-autorest/autorest/to" + "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/azure/converters" +) + +// AzureBastionSpec defines the specification for azure bastion feature. +type AzureBastionSpec struct { + Name string + ResourceGroup string + Location string + ClusterName string + SubnetID string + PublicIPID string +} + +// AzureBastionSpecInput defines the required inputs to construct an azure bastion spec. +type AzureBastionSpecInput struct { + SubnetName string + PublicIPName string + VNetName string +} + +// ResourceName returns the name of the bastion host. +func (s *AzureBastionSpec) ResourceName() string { + return s.Name +} + +// ResourceGroupName returns the name of the resource group. +func (s *AzureBastionSpec) ResourceGroupName() string { + return s.ResourceGroup +} + +// OwnerResourceName is a no-op for bastion hosts. +func (s *AzureBastionSpec) OwnerResourceName() string { + return "" +} + +// Parameters returns the parameters for the bastion host. +func (s *AzureBastionSpec) Parameters(existing interface{}) (paramteres interface{}, err error) { + if existing != nil { + if _, ok := existing.(network.BastionHost); !ok { + return nil, errors.Errorf("%T is not a network.BastionHost", existing) + } + // bastion host already exists + return nil, nil + } + + bastionHostIPConfigName := fmt.Sprintf("%s-%s", s.Name, "bastionIP") + + return network.BastionHost{ + Name: to.StringPtr(s.Name), + Location: to.StringPtr(s.Location), + Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ + ClusterName: s.ClusterName, + Lifecycle: infrav1.ResourceLifecycleOwned, + Name: to.StringPtr(s.Name), + Role: to.StringPtr("Bastion"), + })), + BastionHostPropertiesFormat: &network.BastionHostPropertiesFormat{ + DNSName: to.StringPtr(fmt.Sprintf("%s-bastion", strings.ToLower(s.Name))), + IPConfigurations: &[]network.BastionHostIPConfiguration{ + { + Name: to.StringPtr(bastionHostIPConfigName), + BastionHostIPConfigurationPropertiesFormat: &network.BastionHostIPConfigurationPropertiesFormat{ + Subnet: &network.SubResource{ + ID: &s.SubnetID, + }, + PublicIPAddress: &network.SubResource{ + ID: &s.PublicIPID, + }, + PrivateIPAllocationMethod: network.IPAllocationMethodDynamic, + }, + }, + }, + }, + }, nil +} diff --git a/azure/services/bastionhosts/bastionhosts.go b/azure/services/bastionhosts/bastionhosts.go index c365ec9cd6b..2922f4f0c80 100644 --- a/azure/services/bastionhosts/bastionhosts.go +++ b/azure/services/bastionhosts/bastionhosts.go @@ -19,35 +19,34 @@ package bastionhosts import ( "context" - "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/publicips" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/subnets" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/async" + "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) +const serviceName = "bastionhosts" + // BastionScope defines the scope interface for a bastion host service. type BastionScope interface { - azure.ClusterDescriber - azure.NetworkDescriber - BastionSpec() azure.BastionSpec + azure.ClusterScoper + azure.AsyncStatusUpdater + AzureBastionSpec() azure.ResourceSpecGetter } // Service provides operations on Azure resources. type Service struct { Scope BastionScope - client - subnetsClient subnets.Client - publicIPsClient publicips.Client + async.Reconciler } // New creates a new service. func New(scope BastionScope) *Service { + client := newClient(scope) return &Service{ - Scope: scope, - client: newClient(scope), - subnetsClient: subnets.NewClient(scope), - publicIPsClient: publicips.NewClient(scope), + Scope: scope, + Reconciler: async.New(scope, client, client), } } @@ -56,15 +55,16 @@ func (s *Service) Reconcile(ctx context.Context) error { ctx, _, done := tele.StartSpanWithLogger(ctx, "bastionhosts.Service.Reconcile") defer done() - azureBastionSpec := s.Scope.BastionSpec().AzureBastion - if azureBastionSpec != nil { - err := s.ensureAzureBastion(ctx, *azureBastionSpec) - if err != nil { - return errors.Wrap(err, "error creating Azure Bastion") - } + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() + + var resultingErr error + if bastionSpec := s.Scope.AzureBastionSpec(); bastionSpec != nil { + _, resultingErr = s.CreateResource(ctx, bastionSpec, serviceName) } - return nil + s.Scope.UpdatePutStatus(infrav1.BastionHostReadyCondition, serviceName, resultingErr) + return resultingErr } // Delete deletes the bastion host with the provided scope. @@ -72,12 +72,14 @@ func (s *Service) Delete(ctx context.Context) error { ctx, _, done := tele.StartSpanWithLogger(ctx, "bastionhosts.Service.Delete") defer done() - azureBastionSpec := s.Scope.BastionSpec().AzureBastion - if azureBastionSpec != nil { - err := s.ensureAzureBastionDeleted(ctx, *azureBastionSpec) - if err != nil { - return errors.Wrap(err, "error deleting Azure Bastion") - } + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() + + var resultingErr error + if bastionSpec := s.Scope.AzureBastionSpec(); bastionSpec != nil { + resultingErr = s.DeleteResource(ctx, bastionSpec, serviceName) } - return nil + + s.Scope.UpdateDeleteStatus(infrav1.BastionHostReadyCondition, serviceName, resultingErr) + return resultingErr } diff --git a/azure/services/bastionhosts/bastionhosts_test.go b/azure/services/bastionhosts/bastionhosts_test.go index b101ab17fb6..24111b5d9b1 100644 --- a/azure/services/bastionhosts/bastionhosts_test.go +++ b/azure/services/bastionhosts/bastionhosts_test.go @@ -21,20 +21,30 @@ import ( "net/http" "testing" - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" "github.com/Azure/go-autorest/autorest" "github.com/golang/mock/gomock" . "github.com/onsi/gomega" "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" - "sigs.k8s.io/cluster-api-provider-azure/azure" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/async/mock_async" mock_bastionhosts "sigs.k8s.io/cluster-api-provider-azure/azure/services/bastionhosts/mocks_bastionhosts" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/publicips/mock_publicips" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/subnets/mock_subnets" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) +var ( + fakeSubnetID = "my-subnet-id" + fakePublicIPID = "my-public-ip-id" + fakeAzureBastionSpec = AzureBastionSpec{ + Name: "my-bastion", + Location: "westus", + ClusterName: "my-cluster", + SubnetID: fakeSubnetID, + PublicIPID: fakePublicIPID, + } + internalError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error") +) + func init() { _ = clusterv1.AddToScheme(scheme.Scheme) } @@ -43,109 +53,32 @@ func TestReconcileBastionHosts(t *testing.T) { testcases := []struct { name string expectedError string - expect func(s *mock_bastionhosts.MockBastionScopeMockRecorder, - m *mock_bastionhosts.MockclientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder) + expect func(s *mock_bastionhosts.MockBastionScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) }{ { - name: "fail to get publicip", - expectedError: "error creating Azure Bastion: failed to get public IP for azure bastion: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, - m *mock_bastionhosts.MockclientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder) { - s.BastionSpec().Return(azure.BastionSpec{ - AzureBastion: &azure.AzureBastionSpec{ - Name: "my-bastion", - VNetName: "my-vnet", - SubnetSpec: v1beta1.SubnetSpec{ - Name: "my-subnet", - }, - PublicIPName: "my-publicip", - }, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - mPublicIP.Get(gomockinternal.AContext(), "my-rg", "my-publicip").Return(network.PublicIPAddress{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + name: "bastion successfully created", + expectedError: "", + expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AzureBastionSpec().Return(&fakeAzureBastionSpec) + r.CreateResource(gomockinternal.AContext(), &fakeAzureBastionSpec, serviceName).Return(nil, nil) + s.UpdatePutStatus(infrav1.BastionHostReadyCondition, serviceName, nil) }, }, { - name: "fail to get subnets", - expectedError: "error creating Azure Bastion: failed to get subnet for azure bastion: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, - m *mock_bastionhosts.MockclientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder) { - s.BastionSpec().Return(azure.BastionSpec{ - AzureBastion: &azure.AzureBastionSpec{ - Name: "my-bastion", - VNetName: "my-vnet", - SubnetSpec: v1beta1.SubnetSpec{ - Name: "my-subnet", - }, - PublicIPName: "my-publicip", - }, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - gomock.InOrder( - mPublicIP.Get(gomockinternal.AContext(), "my-rg", "my-publicip").Return(network.PublicIPAddress{}, nil), - mSubnet.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-subnet"). - Return(network.Subnet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")), - ) + name: "no bastion spec found", + expectedError: "", + expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AzureBastionSpec().Return(nil) + s.UpdatePutStatus(infrav1.BastionHostReadyCondition, serviceName, nil) }, }, { name: "fail to create a bastion", - expectedError: "error creating Azure Bastion: cannot create Azure Bastion: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, - m *mock_bastionhosts.MockclientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder) { - s.BastionSpec().Return(azure.BastionSpec{ - AzureBastion: &azure.AzureBastionSpec{ - Name: "my-bastion", - VNetName: "my-vnet", - SubnetSpec: v1beta1.SubnetSpec{ - Name: "my-subnet", - }, - PublicIPName: "my-publicip", - }, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.Location().AnyTimes().Return("fake-location") - s.ClusterName().AnyTimes().Return("fake-cluster") - gomock.InOrder( - mPublicIP.Get(gomockinternal.AContext(), "my-rg", "my-publicip").Return(network.PublicIPAddress{}, nil), - mSubnet.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil), - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-bastion", gomock.AssignableToTypeOf(network.BastionHost{})).Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")), - ) - }, - }, - { - name: "bastion successfully created", - expectedError: "", - expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, - m *mock_bastionhosts.MockclientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder) { - s.BastionSpec().Return(azure.BastionSpec{ - AzureBastion: &azure.AzureBastionSpec{ - Name: "my-bastion", - VNetName: "my-vnet", - SubnetSpec: v1beta1.SubnetSpec{ - Name: "my-subnet", - }, - PublicIPName: "my-publicip", - }, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.Location().AnyTimes().Return("fake-location") - s.ClusterName().AnyTimes().Return("fake-cluster") - gomock.InOrder( - mPublicIP.Get(gomockinternal.AContext(), "my-rg", "my-publicip").Return(network.PublicIPAddress{}, nil), - mSubnet.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil), - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-bastion", gomock.AssignableToTypeOf(network.BastionHost{})), - ) + expectedError: internalError.Error(), + expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AzureBastionSpec().Return(&fakeAzureBastionSpec) + r.CreateResource(gomockinternal.AContext(), &fakeAzureBastionSpec, serviceName).Return(nil, internalError) + s.UpdatePutStatus(infrav1.BastionHostReadyCondition, serviceName, internalError) }, }, } @@ -158,18 +91,13 @@ func TestReconcileBastionHosts(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() scopeMock := mock_bastionhosts.NewMockBastionScope(mockCtrl) - clientMock := mock_bastionhosts.NewMockclient(mockCtrl) - subnetMock := mock_subnets.NewMockClient(mockCtrl) - publicIPsMock := mock_publicips.NewMockClient(mockCtrl) + asyncMock := mock_async.NewMockReconciler(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), - subnetMock.EXPECT(), publicIPsMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), asyncMock.EXPECT()) s := &Service{ - Scope: scopeMock, - client: clientMock, - subnetsClient: subnetMock, - publicIPsClient: publicIPsMock, + Scope: scopeMock, + Reconciler: asyncMock, } err := s.Reconcile(context.TODO()) @@ -187,75 +115,32 @@ func TestDeleteBastionHost(t *testing.T) { testcases := []struct { name string expectedError string - expect func(s *mock_bastionhosts.MockBastionScopeMockRecorder, - m *mock_bastionhosts.MockclientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder) + expect func(s *mock_bastionhosts.MockBastionScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) }{ { - name: "bastion host deletion fails", - expectedError: "error deleting Azure Bastion: failed to delete Azure Bastion my-bastionhost in resource group my-rg: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, - m *mock_bastionhosts.MockclientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder) { - s.BastionSpec().Return(azure.BastionSpec{ - AzureBastion: &azure.AzureBastionSpec{ - Name: "my-bastionhost", - VNetName: "my-vnet", - SubnetSpec: v1beta1.SubnetSpec{ - Name: "my-subnet", - }, - PublicIPName: "my-publicip", - }, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Delete(gomockinternal.AContext(), "my-rg", "my-bastionhost"). - Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + name: "successfully delete an existing bastion host", + expectedError: "", + expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AzureBastionSpec().Return(&fakeAzureBastionSpec) + r.DeleteResource(gomockinternal.AContext(), &fakeAzureBastionSpec, serviceName).Return(nil) + s.UpdateDeleteStatus(infrav1.BastionHostReadyCondition, serviceName, nil) }, }, { - name: "bastion host already deleted", - expectedError: "", - expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, - m *mock_bastionhosts.MockclientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder) { - s.BastionSpec().Return(azure.BastionSpec{ - AzureBastion: &azure.AzureBastionSpec{ - Name: "my-bastionhost", - VNetName: "my-vnet", - SubnetSpec: v1beta1.SubnetSpec{ - Name: "my-subnet", - }, - PublicIPName: "my-publicip", - }, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Delete(gomockinternal.AContext(), "my-rg", "my-bastionhost"). - Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) + name: "bastion host deletion fails", + expectedError: internalError.Error(), + expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AzureBastionSpec().Return(&fakeAzureBastionSpec) + r.DeleteResource(gomockinternal.AContext(), &fakeAzureBastionSpec, serviceName).Return(internalError) + s.UpdateDeleteStatus(infrav1.BastionHostReadyCondition, serviceName, internalError) }, }, { - name: "successfully delete an existing bastion host", - + name: "no bastion spec found", expectedError: "", - expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, - m *mock_bastionhosts.MockclientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder) { - s.BastionSpec().Return(azure.BastionSpec{ - AzureBastion: &azure.AzureBastionSpec{ - Name: "my-bastionhost", - VNetName: "my-vnet", - SubnetSpec: v1beta1.SubnetSpec{ - Name: "my-subnet", - }, - PublicIPName: "my-publicip", - }, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Delete(gomockinternal.AContext(), "my-rg", "my-bastionhost") + expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AzureBastionSpec().Return(nil) + s.UpdateDeleteStatus(infrav1.BastionHostReadyCondition, serviceName, nil) }, }, } @@ -268,18 +153,13 @@ func TestDeleteBastionHost(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() scopeMock := mock_bastionhosts.NewMockBastionScope(mockCtrl) - clientMock := mock_bastionhosts.NewMockclient(mockCtrl) - subnetMock := mock_subnets.NewMockClient(mockCtrl) - publicIPsMock := mock_publicips.NewMockClient(mockCtrl) + asyncMock := mock_async.NewMockReconciler(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), - subnetMock.EXPECT(), publicIPsMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), asyncMock.EXPECT()) s := &Service{ - Scope: scopeMock, - client: clientMock, - subnetsClient: subnetMock, - publicIPsClient: publicIPsMock, + Scope: scopeMock, + Reconciler: asyncMock, } err := s.Delete(context.TODO()) diff --git a/azure/services/bastionhosts/client.go b/azure/services/bastionhosts/client.go index 5aa8c07f589..192d1fd0d93 100644 --- a/azure/services/bastionhosts/client.go +++ b/azure/services/bastionhosts/client.go @@ -18,28 +18,24 @@ package bastionhosts import ( "context" + "encoding/json" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" "github.com/Azure/go-autorest/autorest" + azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) -// client wraps go-sdk. -type client interface { - Get(context.Context, string, string) (network.BastionHost, error) - CreateOrUpdate(context.Context, string, string, network.BastionHost) error - Delete(context.Context, string, string) error -} - // azureClient contains the Azure go-sdk Client. type azureClient struct { - interfaces network.BastionHostsClient + bastionhosts network.BastionHostsClient } -var _ client = (*azureClient)(nil) - // newClient creates a new VM client from subscription ID. func newClient(auth azure.Authorizer) *azureClient { c := newBastionHostsClient(auth.SubscriptionID(), auth.BaseURI(), auth.Authorizer()) @@ -53,44 +49,114 @@ func newBastionHostsClient(subscriptionID string, baseURI string, authorizer aut return bastionClient } -// Get gets information about the specified bastion host. -func (ac *azureClient) Get(ctx context.Context, resourceGroupName, bastionName string) (network.BastionHost, error) { - ctx, _, done := tele.StartSpanWithLogger(ctx, "bastionhosts.AzureClient.Get") +// Get gets the specified bastion host. +func (ac *azureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "bastionhosts.azureClient.Get") defer done() - return ac.interfaces.Get(ctx, resourceGroupName, bastionName) + return ac.bastionhosts.Get(ctx, spec.ResourceGroupName(), spec.ResourceName()) } -// CreateOrUpdate creates or updates a bastion host. -func (ac *azureClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, bastionName string, bastionHost network.BastionHost) error { - ctx, _, done := tele.StartSpanWithLogger(ctx, "bastionhosts.AzureClient.CreateOrUpdate") +// CreateOrUpdateAsync creates or updates a bastion host asynchronously. +// It sends a PUT request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing +// progress of the operation. +func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "bastionhosts.azureClient.CreateOrUpdateAsync") defer done() - future, err := ac.interfaces.CreateOrUpdate(ctx, resourceGroupName, bastionName, bastionHost) + host, ok := parameters.(network.BastionHost) + if !ok { + return nil, nil, errors.Errorf("%T is not a network.BastionHost", parameters) + } + + createFuture, err := ac.bastionhosts.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), host) if err != nil { - return err + return nil, nil, err } - err = future.WaitForCompletionRef(ctx, ac.interfaces.Client) + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + + err = createFuture.WaitForCompletionRef(ctx, ac.bastionhosts.Client) if err != nil { - return err + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return nil, &createFuture, err } - _, err = future.Result(ac.interfaces) - return err + + result, err = createFuture.Result(ac.bastionhosts) + // if the operation completed, return a nil future + return result, nil, err } -// Delete deletes the specified network interface. -func (ac *azureClient) Delete(ctx context.Context, resourceGroupName, bastionName string) error { - ctx, _, done := tele.StartSpanWithLogger(ctx, "bastionhosts.AzureClient.Delete") +// DeleteAsync deletes a bastion host asynchronously. DeleteAsync sends a DELETE +// request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing +// progress of the operation. +func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "bastionhosts.azureClient.Delete") defer done() - future, err := ac.interfaces.Delete(ctx, resourceGroupName, bastionName) + deleteFuture, err := ac.bastionhosts.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) + if err != nil { + return nil, err + } + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + + err = deleteFuture.WaitForCompletionRef(ctx, ac.bastionhosts.Client) if err != nil { - return err + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return &deleteFuture, err } - err = future.WaitForCompletionRef(ctx, ac.interfaces.Client) + _, err = deleteFuture.Result(ac.bastionhosts) + // if the operation completed, return a nil future. + return nil, err +} + +// IsDone returns true if the long-running operation has completed. +func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "bastionhosts.azureClient.IsDone") + defer done() + + isDone, err = future.DoneWithContext(ctx, ac.bastionhosts) if err != nil { - return err + return false, errors.Wrap(err, "failed checking if the operation was complete") + } + + return isDone, nil +} + +// Result fetches the result of a long-running operation future. +func (ac *azureClient) Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) { + _, _, done := tele.StartSpanWithLogger(ctx, "bastionhosts.azureClient.Result") + defer done() + + if future == nil { + return nil, errors.Errorf("cannot get result from nil future") + } + + switch futureType { + case infrav1.PutFuture: + // Marshal and Unmarshal the future to put it into the correct future type so we can access the Result function. + // Unfortunately the FutureAPI can't be casted directly to BastionHostsCreateOrUpdateFuture because it is a azureautorest.Future, which doesn't implement the Result function. See PR #1686 for discussion on alternatives. + // It was converted back to a generic azureautorest.Future from the CAPZ infrav1.Future type stored in Status: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/main/azure/converters/futures.go#L49. + var createFuture *network.BastionHostsCreateOrUpdateFuture + jsonData, err := future.MarshalJSON() + if err != nil { + return nil, errors.Wrap(err, "failed to marshal future") + } + if err := json.Unmarshal(jsonData, &createFuture); err != nil { + return nil, errors.Wrap(err, "failed to unmarshal future data") + } + return (*createFuture).Result(ac.bastionhosts) + + case infrav1.DeleteFuture: + // Delete does not return a result bastion host + return nil, nil + + default: + return nil, errors.Errorf("unknown future type %q", futureType) } - _, err = future.Result(ac.interfaces) - return err } diff --git a/azure/services/bastionhosts/mocks_bastionhosts/bastionhosts_mock.go b/azure/services/bastionhosts/mocks_bastionhosts/bastionhosts_mock.go index 7c5915acd41..b618546fd1f 100644 --- a/azure/services/bastionhosts/mocks_bastionhosts/bastionhosts_mock.go +++ b/azure/services/bastionhosts/mocks_bastionhosts/bastionhosts_mock.go @@ -27,6 +27,7 @@ import ( gomock "github.com/golang/mock/gomock" v1beta1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" azure "sigs.k8s.io/cluster-api-provider-azure/azure" + v1beta10 "sigs.k8s.io/cluster-api/api/v1beta1" ) // MockBastionScope is a mock of BastionScope interface. @@ -136,32 +137,32 @@ func (mr *MockBastionScopeMockRecorder) AvailabilitySetEnabled() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AvailabilitySetEnabled", reflect.TypeOf((*MockBastionScope)(nil).AvailabilitySetEnabled)) } -// BaseURI mocks base method. -func (m *MockBastionScope) BaseURI() string { +// AzureBastionSpec mocks base method. +func (m *MockBastionScope) AzureBastionSpec() azure.ResourceSpecGetter { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "BaseURI") - ret0, _ := ret[0].(string) + ret := m.ctrl.Call(m, "AzureBastionSpec") + ret0, _ := ret[0].(azure.ResourceSpecGetter) return ret0 } -// BaseURI indicates an expected call of BaseURI. -func (mr *MockBastionScopeMockRecorder) BaseURI() *gomock.Call { +// AzureBastionSpec indicates an expected call of AzureBastionSpec. +func (mr *MockBastionScopeMockRecorder) AzureBastionSpec() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BaseURI", reflect.TypeOf((*MockBastionScope)(nil).BaseURI)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AzureBastionSpec", reflect.TypeOf((*MockBastionScope)(nil).AzureBastionSpec)) } -// BastionSpec mocks base method. -func (m *MockBastionScope) BastionSpec() azure.BastionSpec { +// BaseURI mocks base method. +func (m *MockBastionScope) BaseURI() string { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "BastionSpec") - ret0, _ := ret[0].(azure.BastionSpec) + ret := m.ctrl.Call(m, "BaseURI") + ret0, _ := ret[0].(string) return ret0 } -// BastionSpec indicates an expected call of BastionSpec. -func (mr *MockBastionScopeMockRecorder) BastionSpec() *gomock.Call { +// BaseURI indicates an expected call of BaseURI. +func (mr *MockBastionScopeMockRecorder) BaseURI() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BastionSpec", reflect.TypeOf((*MockBastionScope)(nil).BastionSpec)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BaseURI", reflect.TypeOf((*MockBastionScope)(nil).BaseURI)) } // ClientID mocks base method. @@ -262,6 +263,18 @@ func (mr *MockBastionScopeMockRecorder) ControlPlaneSubnet() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControlPlaneSubnet", reflect.TypeOf((*MockBastionScope)(nil).ControlPlaneSubnet)) } +// DeleteLongRunningOperationState mocks base method. +func (m *MockBastionScope) DeleteLongRunningOperationState(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) +} + +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockBastionScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockBastionScope)(nil).DeleteLongRunningOperationState), arg0, arg1) +} + // FailureDomains mocks base method. func (m *MockBastionScope) FailureDomains() []string { m.ctrl.T.Helper() @@ -276,6 +289,20 @@ func (mr *MockBastionScopeMockRecorder) FailureDomains() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FailureDomains", reflect.TypeOf((*MockBastionScope)(nil).FailureDomains)) } +// GetLongRunningOperationState mocks base method. +func (m *MockBastionScope) GetLongRunningOperationState(arg0, arg1 string) *v1beta1.Future { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1) + ret0, _ := ret[0].(*v1beta1.Future) + return ret0 +} + +// GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. +func (mr *MockBastionScopeMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockBastionScope)(nil).GetLongRunningOperationState), arg0, arg1) +} + // GetPrivateDNSZoneName mocks base method. func (m *MockBastionScope) GetPrivateDNSZoneName() string { m.ctrl.T.Helper() @@ -416,6 +443,18 @@ func (mr *MockBastionScopeMockRecorder) ResourceGroup() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResourceGroup", reflect.TypeOf((*MockBastionScope)(nil).ResourceGroup)) } +// SetLongRunningOperationState mocks base method. +func (m *MockBastionScope) SetLongRunningOperationState(arg0 *v1beta1.Future) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) +} + +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockBastionScopeMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockBastionScope)(nil).SetLongRunningOperationState), arg0) +} + // SetSubnet mocks base method. func (m *MockBastionScope) SetSubnet(arg0 v1beta1.SubnetSpec) { m.ctrl.T.Helper() @@ -484,6 +523,42 @@ func (mr *MockBastionScopeMockRecorder) TenantID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockBastionScope)(nil).TenantID)) } +// UpdateDeleteStatus mocks base method. +func (m *MockBastionScope) UpdateDeleteStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdateDeleteStatus", arg0, arg1, arg2) +} + +// UpdateDeleteStatus indicates an expected call of UpdateDeleteStatus. +func (mr *MockBastionScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockBastionScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) +} + +// UpdatePatchStatus mocks base method. +func (m *MockBastionScope) UpdatePatchStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdatePatchStatus", arg0, arg1, arg2) +} + +// UpdatePatchStatus indicates an expected call of UpdatePatchStatus. +func (mr *MockBastionScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockBastionScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockBastionScope) UpdatePutStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdatePutStatus", arg0, arg1, arg2) +} + +// UpdatePutStatus indicates an expected call of UpdatePutStatus. +func (mr *MockBastionScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockBastionScope)(nil).UpdatePutStatus), arg0, arg1, arg2) +} + // Vnet mocks base method. func (m *MockBastionScope) Vnet() *v1beta1.VnetSpec { m.ctrl.T.Helper() diff --git a/azure/services/bastionhosts/mocks_bastionhosts/client_mock.go b/azure/services/bastionhosts/mocks_bastionhosts/client_mock.go deleted file mode 100644 index 129a464cf1d..00000000000 --- a/azure/services/bastionhosts/mocks_bastionhosts/client_mock.go +++ /dev/null @@ -1,95 +0,0 @@ -/* -Copyright 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. -*/ - -// Code generated by MockGen. DO NOT EDIT. -// Source: ../client.go - -// Package mock_bastionhosts is a generated GoMock package. -package mock_bastionhosts - -import ( - context "context" - reflect "reflect" - - network "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" - gomock "github.com/golang/mock/gomock" -) - -// Mockclient is a mock of client interface. -type Mockclient struct { - ctrl *gomock.Controller - recorder *MockclientMockRecorder -} - -// MockclientMockRecorder is the mock recorder for Mockclient. -type MockclientMockRecorder struct { - mock *Mockclient -} - -// NewMockclient creates a new mock instance. -func NewMockclient(ctrl *gomock.Controller) *Mockclient { - mock := &Mockclient{ctrl: ctrl} - mock.recorder = &MockclientMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *Mockclient) EXPECT() *MockclientMockRecorder { - return m.recorder -} - -// CreateOrUpdate mocks base method. -func (m *Mockclient) CreateOrUpdate(arg0 context.Context, arg1, arg2 string, arg3 network.BastionHost) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdate", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(error) - return ret0 -} - -// CreateOrUpdate indicates an expected call of CreateOrUpdate. -func (mr *MockclientMockRecorder) CreateOrUpdate(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdate", reflect.TypeOf((*Mockclient)(nil).CreateOrUpdate), arg0, arg1, arg2, arg3) -} - -// Delete mocks base method. -func (m *Mockclient) Delete(arg0 context.Context, arg1, arg2 string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Delete", arg0, arg1, arg2) - ret0, _ := ret[0].(error) - return ret0 -} - -// Delete indicates an expected call of Delete. -func (mr *MockclientMockRecorder) Delete(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*Mockclient)(nil).Delete), arg0, arg1, arg2) -} - -// Get mocks base method. -func (m *Mockclient) Get(arg0 context.Context, arg1, arg2 string) (network.BastionHost, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Get", arg0, arg1, arg2) - ret0, _ := ret[0].(network.BastionHost) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Get indicates an expected call of Get. -func (mr *MockclientMockRecorder) Get(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*Mockclient)(nil).Get), arg0, arg1, arg2) -} diff --git a/azure/services/bastionhosts/mocks_bastionhosts/doc.go b/azure/services/bastionhosts/mocks_bastionhosts/doc.go index b9cc5c41382..2a00e78768b 100644 --- a/azure/services/bastionhosts/mocks_bastionhosts/doc.go +++ b/azure/services/bastionhosts/mocks_bastionhosts/doc.go @@ -15,8 +15,6 @@ limitations under the License. */ // Run go generate to regenerate this mock. -//go:generate ../../../../hack/tools/bin/mockgen -destination client_mock.go -package mock_bastionhosts -source ../client.go Client //go:generate ../../../../hack/tools/bin/mockgen -destination bastionhosts_mock.go -package mock_bastionhosts -source ../bastionhosts.go BastionScope -//go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt client_mock.go > _client_mock.go && mv _client_mock.go client_mock.go" //go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt bastionhosts_mock.go > _bastionhosts_mock.go && mv _bastionhosts_mock.go bastionhosts_mock.go" package mock_bastionhosts //nolint diff --git a/azure/types.go b/azure/types.go index fd547fb7f12..0b9db5e6081 100644 --- a/azure/types.go +++ b/azure/types.go @@ -76,19 +76,6 @@ type NSGSpec struct { SecurityRules infrav1.SecurityRules } -// BastionSpec defines the specification for the generic bastion feature. -type BastionSpec struct { - AzureBastion *AzureBastionSpec -} - -// AzureBastionSpec defines the specification for azure bastion feature. -type AzureBastionSpec struct { // nolint - Name string - SubnetSpec infrav1.SubnetSpec - PublicIPName string - VNetName string -} - // ScaleSetSpec defines the specification for a Scale Set. type ScaleSetSpec struct { Name string diff --git a/templates/test/ci/cluster-template-prow-private.yaml b/templates/test/ci/cluster-template-prow-private.yaml index ffdbf70caa6..b1a0c4ec403 100644 --- a/templates/test/ci/cluster-template-prow-private.yaml +++ b/templates/test/ci/cluster-template-prow-private.yaml @@ -30,7 +30,15 @@ spec: creationTimestamp: ${TIMESTAMP} jobName: ${JOB_NAME} bastionSpec: - azureBastion: {} + azureBastion: + name: ${CLUSTER_NAME}-azure-bastion + publicIP: + name: ${CLUSTER_NAME}-azure-bastion-pip + subnet: + cidrBlocks: + - ${AZURE_BASTION_SUBNET_CIDR} + name: AzureBastionSubnet + role: bastion identityRef: apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 kind: AzureClusterIdentity diff --git a/templates/test/ci/prow-private/kustomization.yaml b/templates/test/ci/prow-private/kustomization.yaml index 88619734703..c70e27fd1f2 100644 --- a/templates/test/ci/prow-private/kustomization.yaml +++ b/templates/test/ci/prow-private/kustomization.yaml @@ -8,6 +8,7 @@ patchesStrategicMerge: - ../patches/tags.yaml - ../patches/controller-manager.yaml - ../patches/cluster-cni.yaml + - patches/bastion.yaml - patches/vnet-peerings.yaml configMapGenerator: - name: cni-${CLUSTER_NAME}-calico diff --git a/templates/test/ci/prow-private/patches/bastion.yaml b/templates/test/ci/prow-private/patches/bastion.yaml new file mode 100644 index 00000000000..7ba26a572b9 --- /dev/null +++ b/templates/test/ci/prow-private/patches/bastion.yaml @@ -0,0 +1,15 @@ +apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 +kind: AzureCluster +metadata: + name: ${CLUSTER_NAME} +spec: + bastionSpec: + azureBastion: + name: ${CLUSTER_NAME}-azure-bastion + publicIP: + name: ${CLUSTER_NAME}-azure-bastion-pip + subnet: + cidrBlocks: + - ${AZURE_BASTION_SUBNET_CIDR} + name: AzureBastionSubnet + role: "bastion" diff --git a/templates/test/ci/prow-private/patches/vnet-peerings.yaml b/templates/test/ci/prow-private/patches/vnet-peerings.yaml index cf5101d1d49..7eb9ff1c6ea 100644 --- a/templates/test/ci/prow-private/patches/vnet-peerings.yaml +++ b/templates/test/ci/prow-private/patches/vnet-peerings.yaml @@ -25,4 +25,4 @@ spec: - name: private-node-subnet role: node cidrBlocks: - - ${AZURE_NODE_SUBNET_CIDR} \ No newline at end of file + - ${AZURE_NODE_SUBNET_CIDR} diff --git a/test/e2e/azure_privatecluster.go b/test/e2e/azure_privatecluster.go index 9648b225c81..b1bb9389b76 100644 --- a/test/e2e/azure_privatecluster.go +++ b/test/e2e/azure_privatecluster.go @@ -132,6 +132,7 @@ func AzurePrivateClusterSpec(ctx context.Context, inputGetter func() AzurePrivat Expect(os.Setenv(AzureInternalLBIP, "10.255.0.100")).NotTo(HaveOccurred()) Expect(os.Setenv(AzureCPSubnetCidr, "10.255.0.0/24")).NotTo(HaveOccurred()) Expect(os.Setenv(AzureNodeSubnetCidr, "10.255.1.0/24")).NotTo(HaveOccurred()) + Expect(os.Setenv(AzureBastionSubnetCidr, "10.255.255.224/27")).NotTo(HaveOccurred()) result := &clusterctl.ApplyClusterTemplateAndWaitResult{} clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{ ClusterProxy: publicClusterProxy, diff --git a/test/e2e/common.go b/test/e2e/common.go index f86dabd38b0..ec2f732298c 100644 --- a/test/e2e/common.go +++ b/test/e2e/common.go @@ -54,6 +54,7 @@ const ( AzureCPSubnetCidr = "AZURE_CP_SUBNET_CIDR" AzureVNetCidr = "AZURE_PRIVATE_VNET_CIDR" AzureNodeSubnetCidr = "AZURE_NODE_SUBNET_CIDR" + AzureBastionSubnetCidr = "AZURE_BASTION_SUBNET_CIDR" MultiTenancyIdentityName = "MULTI_TENANCY_IDENTITY_NAME" ClusterIdentityName = "CLUSTER_IDENTITY_NAME" ClusterIdentityNamespace = "CLUSTER_IDENTITY_NAMESPACE"