Skip to content

Commit

Permalink
Bastion configvalidator (#527)
Browse files Browse the repository at this point in the history
* checkIfImageExists

GetSecurityGroupWithID

* bastion configvalidator

* testing

add unit test

* change imageID
  • Loading branch information
tedteng authored Sep 5, 2022
1 parent cadc2a1 commit 8e1ef68
Show file tree
Hide file tree
Showing 11 changed files with 434 additions and 54 deletions.
2 changes: 1 addition & 1 deletion pkg/admission/mutator/shoot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions pkg/admission/mutator/shoot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand Down
10 changes: 9 additions & 1 deletion pkg/alicloud/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion pkg/alicloud/client/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
41 changes: 1 addition & 40 deletions pkg/controller/bastion/actuator_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
3 changes: 3 additions & 0 deletions pkg/controller/bastion/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand Down
140 changes: 140 additions & 0 deletions pkg/controller/bastion/configvalidator.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading

0 comments on commit 8e1ef68

Please sign in to comment.