Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bastion configvalidator #527

Merged
merged 4 commits into from
Sep 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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