Skip to content

Commit

Permalink
fixup! fixup! api: linodemachine: add create validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@cbang-akamai
cbang-akamai committed May 8, 2024
1 parent d652ab8 commit eb1295b
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 20 deletions.
8 changes: 5 additions & 3 deletions api/v1alpha1/linodemachine_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import (
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

. "github.com/linode/cluster-api-provider-linode/clients"
)

var (
Expand Down Expand Up @@ -72,7 +74,7 @@ func (r *LinodeMachine) ValidateCreate() (admission.Warnings, error) {
linodemachinelog.Info("validate create", "name", r.Name)

Check warning on line 74 in api/v1alpha1/linodemachine_webhook.go

View check run for this annotation

Codecov / codecov/patch

api/v1alpha1/linodemachine_webhook.go#L73-L74 

Added lines #L73 - L74 were not covered by tests

client := linodego.NewClient(http.DefaultClient)
return nil, r.validateLinodeMachine(context.TODO(), client)
return nil, r.validateLinodeMachine(context.TODO(), &client)

Check warning on line 77 in api/v1alpha1/linodemachine_webhook.go

View check run for this annotation

Codecov / codecov/patch

api/v1alpha1/linodemachine_webhook.go#L76-L77 

Added lines #L76 - L77 were not covered by tests
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
Expand All @@ -91,7 +93,7 @@ func (r *LinodeMachine) ValidateDelete() (admission.Warnings, error) {
return nil, nil

Check warning on line 93 in api/v1alpha1/linodemachine_webhook.go

View check run for this annotation

Codecov / codecov/patch

api/v1alpha1/linodemachine_webhook.go#L93 

Added line #L93 was not covered by tests
}

func (r *LinodeMachine) validateLinodeMachine(ctx context.Context, client linodego.Client) error {
func (r *LinodeMachine) validateLinodeMachine(ctx context.Context, client LinodeClient) error {
var errs field.ErrorList
if err := r.validateLinodeMachineSpec(ctx, client); err != nil {
errs = slices.Concat(errs, err)
Expand All @@ -105,7 +107,7 @@ func (r *LinodeMachine) validateLinodeMachine(ctx context.Context, client linode
r.Name, errs)
}

func (r *LinodeMachine) validateLinodeMachineSpec(ctx context.Context, client linodego.Client) field.ErrorList {
func (r *LinodeMachine) validateLinodeMachineSpec(ctx context.Context, client LinodeClient) field.ErrorList {
var (
errs field.ErrorList
)
Expand Down
128 changes: 113 additions & 15 deletions api/v1alpha1/linodemachine_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,121 @@ limitations under the License.
package v1alpha1

import (
. "github.com/onsi/ginkgo/v2"
)

var _ = Describe("LinodeMachine Webhook", func() {

Context("When creating LinodeMachine under Validating Webhook", func() {
It("Should deny if a required field is empty", func() {
"context"
"errors"
"math"
"strconv"
"testing"

// TODO(user): Add your logic here
"github.com/linode/linodego"
"github.com/stretchr/testify/assert"
"go.uber.org/mock/gomock"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

})
"github.com/linode/cluster-api-provider-linode/mock"

It("Should admit if all required fields are provided", func() {

// TODO(user): Add your logic here
. "github.com/linode/cluster-api-provider-linode/mock/mocktest"
)

})
})
//nolint:paralleltest // race detected during execution of test
func TestValidateLinodeMachine(t *testing.T) {
var (
machine = LinodeMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "example",
Namespace: "example",
},
Spec: LinodeMachineSpec{
Region: "example",
Type: "example",
},
}
plan = linodego.LinodeType{Disk: math.MaxInt}
plan_zero = linodego.LinodeType{Disk: 0}
disk = InstanceDisk{Size: resource.MustParse("1G")}
disk_zero = InstanceDisk{Size: *resource.NewQuantity(0, resource.BinarySI)}
)

})
NewSuite(t, mock.MockLinodeClient{}).Run(
OneOf(
Path(
Call("valid", func(ctx context.Context, mck Mock) {
mck.LinodeClient.EXPECT().GetRegion(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
mck.LinodeClient.EXPECT().GetType(gomock.Any(), gomock.Any()).Return(&plan, nil).AnyTimes()
}),
Result("success", func(ctx context.Context, mck Mock) {
assert.NoError(t, machine.validateLinodeMachine(ctx, mck.LinodeClient))
}),
Call("valid with disks", func(ctx context.Context, mck Mock) {
mck.LinodeClient.EXPECT().GetRegion(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
mck.LinodeClient.EXPECT().GetType(gomock.Any(), gomock.Any()).Return(&plan, nil).AnyTimes()
}),
Result("success", func(ctx context.Context, mck Mock) {
machine := machine
machine.Spec.OSDisk = &disk
assert.NoError(t, machine.validateLinodeMachine(ctx, mck.LinodeClient))
}),
),
),
OneOf(
Path(Call("invalid region", func(ctx context.Context, mck Mock) {
mck.LinodeClient.EXPECT().GetRegion(gomock.Any(), gomock.Any()).Return(nil, errors.New("invalid region")).AnyTimes()
mck.LinodeClient.EXPECT().GetType(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
})),
Path(Call("invalid type", func(ctx context.Context, mck Mock) {
mck.LinodeClient.EXPECT().GetRegion(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
mck.LinodeClient.EXPECT().GetType(gomock.Any(), gomock.Any()).Return(nil, errors.New("invalid type")).AnyTimes()
})),
),
Result("error", func(ctx context.Context, mck Mock) {
assert.Error(t, machine.validateLinodeMachine(ctx, mck.LinodeClient))
}),
OneOf(
Path(
Call("exceed plan storage", func(ctx context.Context, mck Mock) {
mck.LinodeClient.EXPECT().GetRegion(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
mck.LinodeClient.EXPECT().GetType(gomock.Any(), gomock.Any()).Return(&plan_zero, nil).AnyTimes()
}),
Result("os disk too large", func(ctx context.Context, mck Mock) {
machine := machine
machine.Spec.OSDisk = &disk
assert.ErrorContains(t, machine.validateLinodeMachine(ctx, mck.LinodeClient), strconv.Itoa(plan_zero.Disk))
}),
),
Path(
Call("exceed plan storage", func(ctx context.Context, mck Mock) {
mck.LinodeClient.EXPECT().GetRegion(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
mck.LinodeClient.EXPECT().GetType(gomock.Any(), gomock.Any()).Return(&plan_zero, nil).AnyTimes()
}),
Result("data disk too large", func(ctx context.Context, mck Mock) {
machine := machine
machine.Spec.DataDisks = map[string]*InstanceDisk{"sdb": &disk}
assert.Error(t, machine.validateLinodeMachine(ctx, mck.LinodeClient))
}),
),
Path(
Call("data disk invalid path", func(ctx context.Context, mck Mock) {
mck.LinodeClient.EXPECT().GetRegion(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
mck.LinodeClient.EXPECT().GetType(gomock.Any(), gomock.Any()).Return(&plan, nil).AnyTimes()
}),
Result("error", func(ctx context.Context, mck Mock) {
machine := machine
machine.Spec.DataDisks = map[string]*InstanceDisk{"sda": &disk}
assert.Error(t, machine.validateLinodeMachine(ctx, mck.LinodeClient))
}),
),
Path(
Call("invalid disk size", func(ctx context.Context, mck Mock) {
mck.LinodeClient.EXPECT().GetRegion(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes()
mck.LinodeClient.EXPECT().GetType(gomock.Any(), gomock.Any()).Return(&plan, nil).AnyTimes()
}),
Result("error", func(ctx context.Context, mck Mock) {
machine := machine
machine.Spec.OSDisk = &disk_zero
assert.Error(t, machine.validateLinodeMachine(ctx, mck.LinodeClient))
}),
),
),
)
}
6 changes: 4 additions & 2 deletions api/v1alpha1/webhook_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import (

"github.com/linode/linodego"
"k8s.io/apimachinery/pkg/util/validation/field"

. "github.com/linode/cluster-api-provider-linode/clients"
)

func validateRegion(ctx context.Context, client linodego.Client, id string, path *field.Path) (*linodego.Region, *field.Error) {
func validateRegion(ctx context.Context, client LinodeClient, id string, path *field.Path) (*linodego.Region, *field.Error) {
region, err := client.GetRegion(ctx, id)
if err != nil {
return nil, field.NotFound(path, id)
Expand All @@ -16,7 +18,7 @@ func validateRegion(ctx context.Context, client linodego.Client, id string, path
return region, nil
}

func validateLinodeType(ctx context.Context, client linodego.Client, id string, path *field.Path) (*linodego.LinodeType, *field.Error) {
func validateLinodeType(ctx context.Context, client LinodeClient, id string, path *field.Path) (*linodego.LinodeType, *field.Error) {
plan, err := client.GetType(ctx, id)
if err != nil {
return nil, field.NotFound(path, id)
Expand Down
1 change: 1 addition & 0 deletions clients/clients.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type LinodeInstanceClient interface {
GetImage(ctx context.Context, imageID string) (*linodego.Image, error)
CreateStackscript(ctx context.Context, opts linodego.StackscriptCreateOptions) (*linodego.Stackscript, error)
ListStackscripts(ctx context.Context, opts *linodego.ListOptions) ([]linodego.Stackscript, error)
GetType(ctx context.Context, typeID string) (*linodego.LinodeType, error)
}

// LinodeVPCClient defines the methods that a Linode client must have to interact with Linode's VPC service.
Expand Down
30 changes: 30 additions & 0 deletions mock/client.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit eb1295b

Please sign in to comment.