diff --git a/pkg/admission/mutator/shoot.go b/pkg/admission/mutator/shoot.go index 4c54902e7..3ed7fa515 100644 --- a/pkg/admission/mutator/shoot.go +++ b/pkg/admission/mutator/shoot.go @@ -190,7 +190,7 @@ func (s *shootMutator) isOwnedbyAliCloud(ctx context.Context, shoot *corev1beta1 if err != nil { return false, err } - if exist, err := shootECSClient.CheckIfImageExists(ctx, imageId); err != nil { + if exist, err := shootECSClient.CheckIfImageExists(imageId); err != nil { return false, err } else if exist { return shootECSClient.CheckIfImageOwnedByAliCloud(imageId) diff --git a/pkg/admission/mutator/shoot_test.go b/pkg/admission/mutator/shoot_test.go index 6aae71889..e22c38db4 100644 --- a/pkg/admission/mutator/shoot_test.go +++ b/pkg/admission/mutator/shoot_test.go @@ -245,7 +245,7 @@ var _ = Describe("Mutating Shoot", func() { ), alicloudClientFactory.EXPECT().NewECSClient(regionId, accessKeyID, accessKeySecret).Return(ecsClient, nil), - ecsClient.EXPECT().CheckIfImageExists(ctx, imageId).Return(false, nil), + ecsClient.EXPECT().CheckIfImageExists(imageId).Return(false, nil), ) err := mutator.Mutate(ctx, newShoot, nil) Expect(err).NotTo(HaveOccurred()) @@ -290,7 +290,7 @@ var _ = Describe("Mutating Shoot", func() { ), alicloudClientFactory.EXPECT().NewECSClient(regionId, accessKeyID, accessKeySecret).Return(ecsClient, nil), - ecsClient.EXPECT().CheckIfImageExists(ctx, imageId).Return(false, nil), + ecsClient.EXPECT().CheckIfImageExists(imageId).Return(false, nil), //ecsClient.EXPECT().CheckIfImageOwnedByAliCloud(imageId).Return(false, nil) ) err := mutator.Mutate(ctx, newShoot, nil) @@ -321,7 +321,7 @@ var _ = Describe("Mutating Shoot", func() { ), alicloudClientFactory.EXPECT().NewECSClient(regionId, accessKeyID, accessKeySecret).Return(ecsClient, nil), - ecsClient.EXPECT().CheckIfImageExists(ctx, imageId).Return(true, nil), + ecsClient.EXPECT().CheckIfImageExists(imageId).Return(true, nil), ecsClient.EXPECT().CheckIfImageOwnedByAliCloud(imageId).Return(true, nil), ) err := mutator.Mutate(ctx, newShoot, nil) @@ -368,7 +368,7 @@ var _ = Describe("Mutating Shoot", func() { ), alicloudClientFactory.EXPECT().NewECSClient(regionId, accessKeyID, accessKeySecret).Return(ecsClient, nil), - ecsClient.EXPECT().CheckIfImageExists(ctx, imageId).Return(false, nil), + ecsClient.EXPECT().CheckIfImageExists(imageId).Return(false, nil), ) err := mutator.Mutate(ctx, newShoot, oldShoot) Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/alicloud/client/client.go b/pkg/alicloud/client/client.go index 58ac64847..a78d401be 100644 --- a/pkg/alicloud/client/client.go +++ b/pkg/alicloud/client/client.go @@ -210,7 +210,7 @@ func (f *clientFactory) NewECSClient(region, accessKeyID, accessKeySecret string } // CheckIfImageExists checks whether given imageID can be accessed by the client. -func (c *ecsClient) CheckIfImageExists(ctx context.Context, imageID string) (bool, error) { +func (c *ecsClient) CheckIfImageExists(imageID string) (bool, error) { request := ecs.CreateDescribeImagesRequest() request.ImageId = imageID request.SetScheme("HTTPS") @@ -229,6 +229,14 @@ func (c *ecsClient) GetSecurityGroup(name string) (*ecs.DescribeSecurityGroupsRe return c.DescribeSecurityGroups(request) } +// GetSecurityGroup return security group metadata by security group name +func (c *ecsClient) GetSecurityGroupWithID(id string) (*ecs.DescribeSecurityGroupsResponse, error) { + request := ecs.CreateDescribeSecurityGroupsRequest() + request.SetScheme("HTTPS") + request.SecurityGroupId = id + return c.DescribeSecurityGroups(request) +} + // GetInstances return instance metadata by instance name func (c *ecsClient) GetInstances(name string) (*ecs.DescribeInstancesResponse, error) { request := ecs.CreateDescribeInstancesRequest() diff --git a/pkg/alicloud/client/types.go b/pkg/alicloud/client/types.go index e4225edda..feb3be5df 100644 --- a/pkg/alicloud/client/types.go +++ b/pkg/alicloud/client/types.go @@ -58,10 +58,11 @@ type ecsClient struct { // ECS is an interface which declares ECS related methods. type ECS interface { - CheckIfImageExists(ctx context.Context, imageID string) (bool, error) + CheckIfImageExists(imageID string) (bool, error) CheckIfImageOwnedByAliCloud(imageID string) (bool, error) ShareImageToAccount(ctx context.Context, regionID, imageID, accountID string) error GetSecurityGroup(name string) (*ecs.DescribeSecurityGroupsResponse, error) + GetSecurityGroupWithID(id string) (*ecs.DescribeSecurityGroupsResponse, error) DescribeSecurityGroups(request *ecs.DescribeSecurityGroupsRequest) (*ecs.DescribeSecurityGroupsResponse, error) DescribeSecurityGroupAttribute(request *ecs.DescribeSecurityGroupAttributeRequest) (*ecs.DescribeSecurityGroupAttributeResponse, error) DescribeKeyPairs(request *ecs.DescribeKeyPairsRequest) (*ecs.DescribeKeyPairsResponse, error) diff --git a/pkg/controller/bastion/actuator_reconcile.go b/pkg/controller/bastion/actuator_reconcile.go index e16d52e53..a0116d8f3 100644 --- a/pkg/controller/bastion/actuator_reconcile.go +++ b/pkg/controller/bastion/actuator_reconcile.go @@ -22,14 +22,11 @@ import ( "github.com/gardener/gardener-extension-provider-alicloud/pkg/alicloud" aliclient "github.com/gardener/gardener-extension-provider-alicloud/pkg/alicloud/client" - alicloudapi "github.com/gardener/gardener-extension-provider-alicloud/pkg/apis/alicloud" - "github.com/gardener/gardener-extension-provider-alicloud/pkg/apis/alicloud/helper" "github.com/aliyun/alibaba-cloud-sdk-go/services/ecs" "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" "k8s.io/apimachinery/pkg/api/equality" @@ -57,7 +54,7 @@ func (a *actuator) Reconcile(ctx context.Context, log logr.Logger, bastion *exte return err } - infrastructureStatus, err := getInfrastructureStatus(ctx, a, cluster) + infrastructureStatus, err := getInfrastructureStatus(ctx, a.client, cluster) if err != nil { return err } @@ -451,39 +448,3 @@ func egressRuleEqual(a ecs.AuthorizeSecurityGroupEgressRequest, b ecs.Permission return true } - -func getInfrastructureStatus(ctx context.Context, a *actuator, cluster *extensions.Cluster) (*alicloudapi.InfrastructureStatus, error) { - var infrastructureStatus *alicloudapi.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.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.VPC.ID == "" { - return nil, errors.New("vpc id must be not empty for infrastructure provider status") - } - - if len(infrastructureStatus.VPC.VSwitches) == 0 || infrastructureStatus.VPC.VSwitches[0].ID == "" || infrastructureStatus.VPC.VSwitches[0].Zone == "" { - return nil, errors.New("vswitches id must be not empty for infrastructure provider status") - } - - if len(infrastructureStatus.MachineImages) == 0 || infrastructureStatus.MachineImages[0].ID == "" { - return nil, errors.New("machineImages id must be not empty for infrastructure provider status") - } - - // The assumption is that the shoot only has one security group - if len(infrastructureStatus.VPC.SecurityGroups) == 0 || infrastructureStatus.VPC.SecurityGroups[0].ID == "" { - return nil, errors.New("shoot securityGroups id must be not empty for infrastructure provider status") - } - - return infrastructureStatus, nil -} diff --git a/pkg/controller/bastion/add.go b/pkg/controller/bastion/add.go index 9d4c37083..ad7c366db 100644 --- a/pkg/controller/bastion/add.go +++ b/pkg/controller/bastion/add.go @@ -16,6 +16,8 @@ package bastion import ( "github.com/gardener/gardener-extension-provider-alicloud/pkg/alicloud" + aliclient "github.com/gardener/gardener-extension-provider-alicloud/pkg/alicloud/client" + "github.com/gardener/gardener/extensions/pkg/controller/bastion" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -39,6 +41,7 @@ type AddOptions struct { func AddToManagerWithOptions(mgr manager.Manager, opts AddOptions) error { return bastion.Add(mgr, bastion.AddArgs{ Actuator: newActuator(), + ConfigValidator: NewConfigValidator(aliclient.NewClientFactory()), ControllerOptions: opts.Controller, Predicates: bastion.DefaultPredicates(opts.IgnoreOperationAnnotation), Type: alicloud.Type, diff --git a/pkg/controller/bastion/configvalidator.go b/pkg/controller/bastion/configvalidator.go new file mode 100644 index 000000000..f5d974291 --- /dev/null +++ b/pkg/controller/bastion/configvalidator.go @@ -0,0 +1,140 @@ +package bastion + +import ( + "context" + "errors" + "fmt" + + "github.com/gardener/gardener-extension-provider-alicloud/pkg/alicloud" + aliclient "github.com/gardener/gardener-extension-provider-alicloud/pkg/alicloud/client" + alicloudapi "github.com/gardener/gardener-extension-provider-alicloud/pkg/apis/alicloud" + "github.com/gardener/gardener-extension-provider-alicloud/pkg/apis/alicloud/helper" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/gardener/gardener/extensions/pkg/controller/bastion" + "github.com/gardener/gardener/extensions/pkg/controller/common" + v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" + extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + "github.com/gardener/gardener/pkg/extensions" +) + +// configValidator implements ConfigValidator for AliCloud bastion resources. +type configValidator struct { + common.ClientContext + aliClientFactory aliclient.ClientFactory +} + +// NewConfigValidator creates a new ConfigValidator. +func NewConfigValidator(aliClientFactory aliclient.ClientFactory) bastion.ConfigValidator { + return &configValidator{ + aliClientFactory: aliClientFactory, + } +} + +// Validate validates the provider config of the given bastion resource with the cloud provider. +func (c *configValidator) Validate(ctx context.Context, bastion *extensionsv1alpha1.Bastion, cluster *extensions.Cluster) field.ErrorList { + allErrs := field.ErrorList{} + + // Get value from infrastructure status + infrastructureStatus, err := getInfrastructureStatus(ctx, c.Client(), cluster) + if err != nil { + allErrs = append(allErrs, field.InternalError(nil, err)) + return allErrs + } + + secretReference := &corev1.SecretReference{ + Namespace: cluster.ObjectMeta.Name, + Name: v1beta1constants.SecretNameCloudProvider, + } + + credentials, err := alicloud.ReadCredentialsFromSecretRef(ctx, c.Client(), secretReference) + if err != nil { + allErrs = append(allErrs, field.InternalError(nil, err)) + return allErrs + } + + // Create alicloud ECS client + aliCloudECSClient, err := c.aliClientFactory.NewECSClient(cluster.Shoot.Spec.Region, credentials.AccessKeyID, credentials.AccessKeySecret) + if err != nil { + allErrs = append(allErrs, field.InternalError(nil, err)) + return allErrs + } + + aliCloudVPCClient, err := c.aliClientFactory.NewVPCClient(cluster.Shoot.Spec.Region, credentials.AccessKeyID, credentials.AccessKeySecret) + if err != nil { + allErrs = append(allErrs, field.InternalError(nil, err)) + return allErrs + } + + // Validate infrastructureStatus value + allErrs = append(allErrs, c.validateInfrastructureStatus(ctx, aliCloudECSClient, aliCloudVPCClient, infrastructureStatus)...) + return allErrs +} + +func getInfrastructureStatus(ctx context.Context, c client.Client, cluster *extensions.Cluster) (*alicloudapi.InfrastructureStatus, error) { + var infrastructureStatus *alicloudapi.InfrastructureStatus + worker := &extensionsv1alpha1.Worker{} + err := c.Get(ctx, client.ObjectKey{Namespace: cluster.ObjectMeta.Name, Name: cluster.Shoot.Name}, worker) + if err != nil { + return nil, err + } + + if 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.VPC.ID == "" { + return nil, errors.New("vpc id must be not empty for infrastructure provider status") + } + + if len(infrastructureStatus.VPC.VSwitches) == 0 || infrastructureStatus.VPC.VSwitches[0].ID == "" || infrastructureStatus.VPC.VSwitches[0].Zone == "" { + return nil, errors.New("vswitches id must be not empty for infrastructure provider status") + } + + if len(infrastructureStatus.MachineImages) == 0 || infrastructureStatus.MachineImages[0].ID == "" { + return nil, errors.New("machineImages id must be not empty for infrastructure provider status") + } + + // The assumption is that the shoot only has one security group + if len(infrastructureStatus.VPC.SecurityGroups) == 0 || infrastructureStatus.VPC.SecurityGroups[0].ID == "" { + return nil, errors.New("shoot securityGroups id must be not empty for infrastructure provider status") + } + + return infrastructureStatus, nil +} + +func (c *configValidator) validateInfrastructureStatus(ctx context.Context, aliCloudECSClient aliclient.ECS, aliCloudVPCClient aliclient.VPC, infrastructureStatus *alicloudapi.InfrastructureStatus) field.ErrorList { + allErrs := field.ErrorList{} + + vpc, err := aliCloudVPCClient.GetVPCWithID(ctx, infrastructureStatus.VPC.ID) + if err != nil || len(vpc) == 0 { + allErrs = append(allErrs, field.InternalError(field.NewPath("vpc"), fmt.Errorf("could not get vpc %s from alicloud provider: %w", infrastructureStatus.VPC.ID, err))) + return allErrs + } + + vSwitch, err := aliCloudVPCClient.GetVSwitchesInfoByID(infrastructureStatus.VPC.VSwitches[0].ID) + if err != nil || vSwitch.ZoneID == "" { + allErrs = append(allErrs, field.InternalError(field.NewPath("vswitches"), fmt.Errorf("could not get vswitches %s from alicloud provider: %w", infrastructureStatus.VPC.VSwitches[0].ID, err))) + return allErrs + } + + machineImages, err := aliCloudECSClient.CheckIfImageExists(infrastructureStatus.MachineImages[0].ID) + if err != nil || !machineImages { + allErrs = append(allErrs, field.InternalError(field.NewPath("machineImages"), fmt.Errorf("could not get machineImages %s from alicloud provider: %w", infrastructureStatus.MachineImages[0].ID, err))) + return allErrs + } + + shootSecurityGroupId, err := aliCloudECSClient.GetSecurityGroupWithID(infrastructureStatus.VPC.SecurityGroups[0].ID) + if err != nil || len(shootSecurityGroupId.SecurityGroups.SecurityGroup) == 0 || shootSecurityGroupId.SecurityGroups.SecurityGroup[0].SecurityGroupId == "" { + allErrs = append(allErrs, field.InternalError(field.NewPath("securityGroup"), fmt.Errorf("could not get shoot security group %s from alicloud provider: %w", infrastructureStatus.VPC.SecurityGroups[0].ID, err))) + return allErrs + } + + return allErrs +} diff --git a/pkg/controller/bastion/configvalidator_test.go b/pkg/controller/bastion/configvalidator_test.go new file mode 100644 index 000000000..6d1e4aea5 --- /dev/null +++ b/pkg/controller/bastion/configvalidator_test.go @@ -0,0 +1,252 @@ +package bastion + +import ( + "context" + "encoding/json" + + "github.com/gardener/gardener-extension-provider-alicloud/pkg/alicloud" + aliclient "github.com/gardener/gardener-extension-provider-alicloud/pkg/alicloud/client" + apialicloud "github.com/gardener/gardener-extension-provider-alicloud/pkg/apis/alicloud" + mockalicloudclient "github.com/gardener/gardener-extension-provider-alicloud/pkg/mock/provider-alicloud/alicloud/client" + + ecs "github.com/aliyun/alibaba-cloud-sdk-go/services/ecs" + "github.com/aliyun/alibaba-cloud-sdk-go/services/vpc" + "github.com/gardener/gardener/extensions/pkg/controller/bastion" + corev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" + extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1" + "github.com/gardener/gardener/pkg/extensions" + mockclient "github.com/gardener/gardener/pkg/mock/controller-runtime/client" + . "github.com/gardener/gardener/pkg/utils/test/matchers" + "github.com/golang/mock/gomock" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + gstruct "github.com/onsi/gomega/gstruct" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/runtime/inject" +) + +const ( + name = "foo" + namespace = "shoot--foobar--alicloud" + accessKeyID = "accessKeyID" + accessKeySecret = "accessKeySecret" + region = "region" + id = "id" +) + +var _ = Describe("ConfigValidator", func() { + var ( + ctrl *gomock.Controller + c *mockclient.MockClient + alicloudClientFactory *mockalicloudclient.MockClientFactory + ecsClient *mockalicloudclient.MockECS + vpcClient *mockalicloudclient.MockVPC + ctx context.Context + worker *extensionsv1alpha1.Worker + cv bastion.ConfigValidator + bastion *extensionsv1alpha1.Bastion + cluster *extensions.Cluster + cloudProfile *corev1beta1.CloudProfile + secretBinding *corev1beta1.SecretBinding + secret *corev1.Secret + ) + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + defer ctrl.Finish() + c = mockclient.NewMockClient(ctrl) + alicloudClientFactory = mockalicloudclient.NewMockClientFactory(ctrl) + ecsClient = mockalicloudclient.NewMockECS(ctrl) + vpcClient = mockalicloudclient.NewMockVPC(ctrl) + ctx = context.TODO() + + cv = NewConfigValidator(alicloudClientFactory) + err := cv.(inject.Client).InjectClient(c) + Expect(err).NotTo(HaveOccurred()) + + bastion = &extensionsv1alpha1.Bastion{} + cluster = &extensions.Cluster{} + + secretBinding = &corev1beta1.SecretBinding{ + SecretRef: corev1.SecretReference{ + Name: name, + Namespace: namespace, + }, + } + + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + alicloud.AccessKeyID: []byte(accessKeyID), + alicloud.AccessKeySecret: []byte(accessKeySecret), + }, + } + + infraStatus := &apialicloud.InfrastructureStatus{ + VPC: apialicloud.VPCStatus{ + ID: id, + VSwitches: []apialicloud.VSwitch{ + { + ID: id, + Zone: "zone", + }, + }, + SecurityGroups: []apialicloud.SecurityGroup{ + {ID: id}, + }, + }, + MachineImages: []apialicloud.MachineImage{{ + ID: id, + }, + }, + } + + worker = &extensionsv1alpha1.Worker{ + Spec: extensionsv1alpha1.WorkerSpec{ + InfrastructureProviderStatus: &runtime.RawExtension{ + Raw: encode(infraStatus), + }, + }, + } + + }) + + AfterEach(func() { + ctrl.Finish() + }) + + Describe("#Validate", func() { + BeforeEach(func() { + cluster = createClusters() + key := client.ObjectKey{Namespace: cluster.ObjectMeta.Name, Name: cluster.Shoot.Name} + + c.EXPECT().Get(ctx, key, gomock.AssignableToTypeOf(&extensionsv1alpha1.Worker{})).DoAndReturn( + func(_ context.Context, namespacedName client.ObjectKey, obj *extensionsv1alpha1.Worker) error { + worker.DeepCopyInto(obj) + return nil + }) + c.EXPECT().Get(ctx, client.ObjectKey{Namespace: cluster.ObjectMeta.Name, Name: v1beta1constants.SecretNameCloudProvider}, gomock.AssignableToTypeOf(&corev1beta1.CloudProfile{})).DoAndReturn(clientGet(cloudProfile)) + c.EXPECT().Get(ctx, key, gomock.AssignableToTypeOf(&corev1beta1.SecretBinding{})).DoAndReturn(clientGet(secretBinding)) + c.EXPECT().Get(ctx, key, gomock.AssignableToTypeOf(&corev1.Secret{})).DoAndReturn(clientGet(secret)) + alicloudClientFactory.EXPECT().NewECSClient(region, accessKeyID, accessKeySecret).Return(ecsClient, nil) + alicloudClientFactory.EXPECT().NewVPCClient(region, accessKeyID, accessKeySecret).Return(vpcClient, nil) + }) + + It("should succeed if there are infrastructureStatus passed", func() { + vpcClient.EXPECT().GetVPCWithID(ctx, id).Return([]vpc.Vpc{{VpcId: id}}, nil) + vpcClient.EXPECT().GetVSwitchesInfoByID(id).Return(&aliclient.VSwitchInfo{ZoneID: "zoneid"}, nil) + ecsClient.EXPECT().CheckIfImageExists(id).Return(true, nil) + ecsClient.EXPECT().GetSecurityGroupWithID(id).Return(&ecs.DescribeSecurityGroupsResponse{ + SecurityGroups: ecs.SecurityGroups{ + SecurityGroup: []ecs.SecurityGroup{ + {SecurityGroupId: id}, + }, + }, + }, nil) + errorList := cv.Validate(ctx, bastion, cluster) + Expect(errorList).To(BeEmpty()) + }) + + It("should fail with InternalError if getting vpc failed", func() { + vpcClient.EXPECT().GetVPCWithID(ctx, id).Return(nil, nil) + errorList := cv.Validate(ctx, bastion, cluster) + Expect(errorList).To(ConsistOfFields( + gstruct.Fields{ + "Type": Equal(field.ErrorTypeInternal), + "Field": Equal("vpc"), + "Detail": Equal("could not get vpc id from alicloud provider: %!w()"), + })) + }) + + It("should fail with InternalError if getting vSwitch failed", func() { + vpcClient.EXPECT().GetVPCWithID(ctx, id).Return([]vpc.Vpc{{VpcId: id}}, nil) + vpcClient.EXPECT().GetVSwitchesInfoByID(id).Return(&aliclient.VSwitchInfo{ZoneID: ""}, nil) + errorList := cv.Validate(ctx, bastion, cluster) + Expect(errorList).To(ConsistOfFields( + gstruct.Fields{ + "Type": Equal(field.ErrorTypeInternal), + "Field": Equal("vswitches"), + "Detail": Equal("could not get vswitches id from alicloud provider: %!w()"), + })) + }) + + It("should fail with InternalError if getting machineImages id failed", func() { + vpcClient.EXPECT().GetVPCWithID(ctx, id).Return([]vpc.Vpc{{VpcId: id}}, nil) + vpcClient.EXPECT().GetVSwitchesInfoByID(id).Return(&aliclient.VSwitchInfo{ZoneID: "zoneid"}, nil) + ecsClient.EXPECT().CheckIfImageExists(id).Return(false, nil) + errorList := cv.Validate(ctx, bastion, cluster) + Expect(errorList).To(ConsistOfFields( + gstruct.Fields{ + "Type": Equal(field.ErrorTypeInternal), + "Field": Equal("machineImages"), + "Detail": Equal("could not get machineImages id from alicloud provider: %!w()"), + })) + }) + + It("should fail with InternalError if getting securityGroup id failed", func() { + vpcClient.EXPECT().GetVPCWithID(ctx, id).Return([]vpc.Vpc{{VpcId: id}}, nil) + vpcClient.EXPECT().GetVSwitchesInfoByID(id).Return(&aliclient.VSwitchInfo{ZoneID: "zoneid"}, nil) + ecsClient.EXPECT().CheckIfImageExists(id).Return(true, nil) + ecsClient.EXPECT().GetSecurityGroupWithID(id).Return(&ecs.DescribeSecurityGroupsResponse{ + SecurityGroups: ecs.SecurityGroups{ + SecurityGroup: []ecs.SecurityGroup{ + {SecurityGroupId: ""}, + }, + }, + }, nil) + errorList := cv.Validate(ctx, bastion, cluster) + Expect(errorList).To(ConsistOfFields( + gstruct.Fields{ + "Type": Equal(field.ErrorTypeInternal), + "Field": Equal("securityGroup"), + "Detail": Equal("could not get shoot security group id from alicloud provider: %!w()"), + })) + }) + + }) + +}) + +func encode(obj runtime.Object) []byte { + data, _ := json.Marshal(obj) + return data +} + +func createClusters() *extensions.Cluster { + return &extensions.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Shoot: &corev1beta1.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: v1beta1constants.SecretNameCloudProvider, + }, + Spec: corev1beta1.ShootSpec{ + Region: region, + }, + }, + } +} + +func clientGet(result client.Object) interface{} { + return func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + switch obj.(type) { + case *corev1.Secret: + *obj.(*corev1.Secret) = *result.(*corev1.Secret) + case *corev1beta1.CloudProfile: + *obj.(*corev1beta1.CloudProfile) = *result.(*corev1beta1.CloudProfile) + case *corev1beta1.SecretBinding: + *obj.(*corev1beta1.SecretBinding) = *result.(*corev1beta1.SecretBinding) + } + return nil + } +} diff --git a/pkg/controller/infrastructure/actuator.go b/pkg/controller/infrastructure/actuator.go index 236b0a960..7c2951441 100644 --- a/pkg/controller/infrastructure/actuator.go +++ b/pkg/controller/infrastructure/actuator.go @@ -436,7 +436,7 @@ func (a *actuator) ensureEncryptedImageForShootProviderAccount( return nil, err } - if exist, err := shootECSClient.CheckIfImageExists(ctx, imageID); err != nil { + if exist, err := shootECSClient.CheckIfImageExists(imageID); err != nil { return nil, err } else if exist { // Check if image is provided by AliCloud (OwnerAlias is System). @@ -502,7 +502,7 @@ func (a *actuator) makeImageVisibleForShoot(ctx context.Context, log logr.Logger return nil } - exists, err := shootECSClient.CheckIfImageExists(ctx, imageID) + exists, err := shootECSClient.CheckIfImageExists(imageID) if err != nil { return err } diff --git a/pkg/mock/provider-alicloud/alicloud/client/mocks.go b/pkg/mock/provider-alicloud/alicloud/client/mocks.go index a51d682e8..616c02236 100644 --- a/pkg/mock/provider-alicloud/alicloud/client/mocks.go +++ b/pkg/mock/provider-alicloud/alicloud/client/mocks.go @@ -215,18 +215,18 @@ func (mr *MockECSMockRecorder) AllocatePublicIp(arg0 interface{}) *gomock.Call { } // CheckIfImageExists mocks base method. -func (m *MockECS) CheckIfImageExists(arg0 context.Context, arg1 string) (bool, error) { +func (m *MockECS) CheckIfImageExists(arg0 string) (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CheckIfImageExists", arg0, arg1) + ret := m.ctrl.Call(m, "CheckIfImageExists", arg0) ret0, _ := ret[0].(bool) ret1, _ := ret[1].(error) return ret0, ret1 } // CheckIfImageExists indicates an expected call of CheckIfImageExists. -func (mr *MockECSMockRecorder) CheckIfImageExists(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockECSMockRecorder) CheckIfImageExists(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckIfImageExists", reflect.TypeOf((*MockECS)(nil).CheckIfImageExists), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckIfImageExists", reflect.TypeOf((*MockECS)(nil).CheckIfImageExists), arg0) } // CheckIfImageOwnedByAliCloud mocks base method. @@ -434,6 +434,21 @@ func (mr *MockECSMockRecorder) GetSecurityGroup(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecurityGroup", reflect.TypeOf((*MockECS)(nil).GetSecurityGroup), arg0) } +// GetSecurityGroupWithID mocks base method. +func (m *MockECS) GetSecurityGroupWithID(arg0 string) (*ecs.DescribeSecurityGroupsResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSecurityGroupWithID", arg0) + ret0, _ := ret[0].(*ecs.DescribeSecurityGroupsResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSecurityGroupWithID indicates an expected call of GetSecurityGroupWithID. +func (mr *MockECSMockRecorder) GetSecurityGroupWithID(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecurityGroupWithID", reflect.TypeOf((*MockECS)(nil).GetSecurityGroupWithID), arg0) +} + // RevokeEgressRule mocks base method. func (m *MockECS) RevokeEgressRule(arg0 *ecs.RevokeSecurityGroupEgressRequest) error { m.ctrl.T.Helper() diff --git a/test/integration/bastion/bastion_test.go b/test/integration/bastion/bastion_test.go index 9c3398246..c45279fa4 100644 --- a/test/integration/bastion/bastion_test.go +++ b/test/integration/bastion/bastion_test.go @@ -67,7 +67,7 @@ const ( natGatewayCIDR = "10.250.128.0/21" // Enhanced NatGateway need bind with VSwitch, natGatewayCIDR is used for this VSwitch podCIDR = "100.96.0.0/11" securityGroupSuffix = "-sg" - imageID = "m-gw8iwwd4iiln01dj646s" + imageID = "m-gw8c603eae9ygxgt2ig6" ) var myPublicIP = ""