Skip to content

Commit

Permalink
feat: update OCICluster webhook validations
Browse files Browse the repository at this point in the history
  • Loading branch information
@joekr
joekr committed Apr 28, 2022
1 parent 72c2995 commit 184feed
Show file tree
Hide file tree
Showing 3 changed files with 335 additions and 21 deletions.
20 changes: 17 additions & 3 deletions api/v1beta1/ocicluster_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (c *OCICluster) ValidateCreate() error {

var allErrs field.ErrorList

allErrs = append(allErrs, c.validate()...)
allErrs = append(allErrs, c.validate(nil)...)

if len(allErrs) == 0 {
return nil
Expand Down Expand Up @@ -93,7 +93,7 @@ func (c *OCICluster) ValidateUpdate(old runtime.Object) error {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "ociResourceIdentifier"), c.Spec.OCIResourceIdentifier, "field is immutable"))
}

allErrs = append(allErrs, c.validate()...)
allErrs = append(allErrs, c.validate(oldCluster)...)

if len(allErrs) == 0 {
return nil
Expand All @@ -102,9 +102,17 @@ func (c *OCICluster) ValidateUpdate(old runtime.Object) error {
return apierrors.NewInvalid(c.GroupVersionKind().GroupKind(), c.Name, allErrs)
}

func (c *OCICluster) validate() field.ErrorList {
func (c *OCICluster) validate(old *OCICluster) field.ErrorList {
var allErrs field.ErrorList

var oldNetworkSpec NetworkSpec
if old != nil {
oldNetworkSpec = old.Spec.NetworkSpec
}

allErrs = append(allErrs, c.validateClusterName()...)
allErrs = append(allErrs, validateNetworkSpec(c.Spec.NetworkSpec, oldNetworkSpec, field.NewPath("spec").Child("networkSpec"))...)

if len(c.Spec.CompartmentId) <= 0 {
allErrs = append(
allErrs,
Expand All @@ -125,6 +133,12 @@ func (c *OCICluster) validate() field.ErrorList {
field.Invalid(field.NewPath("spec", "ociResourceIdentifier"), c.Spec.OCIResourceIdentifier, "field is required"))
}

if len(c.Spec.Region) > 0 && !validRegion(c.Spec.Region) {
allErrs = append(
allErrs,
field.Invalid(field.NewPath("spec", "region"), c.Spec.Region, "field is invalid. See https://docs.oracle.com/en-us/iaas/Content/General/Concepts/regions.htm"))
}

if len(allErrs) == 0 {
return nil
}
Expand Down
203 changes: 186 additions & 17 deletions api/v1beta1/ocicluster_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package v1beta1

import (
"strings"
"testing"

"github.com/onsi/gomega"
Expand All @@ -28,12 +29,58 @@ import (
)

func TestOCICluster_ValidateCreate(t *testing.T) {
goodSubnets := []*Subnet{
&Subnet{
Name: "test-subnet",
CIDR: "10.0.0.0/16",
},
}
badSubnetCidr := []*Subnet{
&Subnet{
Name: "test-subnet",
CIDR: "10.1.0.0/16",
},
}
badSubnetCidrFormat := []*Subnet{
&Subnet{
Name: "test-subnet",
CIDR: "no-a-cidr",
},
}
dupSubnetNames := []*Subnet{
&Subnet{
Name: "dup-name",
CIDR: "10.0.0.0/16",
},
&Subnet{
Name: "dup-name",
CIDR: "10.0.0.0/16",
},
}
badSubnetName := []*Subnet{
&Subnet{
Name: "",
CIDR: "10.0.0.0/16",
},
}

tests := []struct {
name string
c *OCICluster
expectErr bool
name string
c *OCICluster
errorMgsShouldContain string
expectErr bool
}{
{
name: "shouldn't allow spaces in region",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
Spec: OCIClusterSpec{
Region: "us city 1",
},
},
errorMgsShouldContain: "region",
expectErr: true,
},
{
name: "shouldn't allow bad CompartmentId",
c: &OCICluster{
Expand All @@ -42,15 +89,32 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
CompartmentId: "badocid",
},
},
expectErr: true,
errorMgsShouldContain: "compartmentId",
expectErr: true,
},
{
name: "shouldn't allow blank CompartmentId",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
Spec: OCIClusterSpec{},
},
expectErr: true,
errorMgsShouldContain: "compartmentId",
expectErr: true,
},
{
name: "shouldn't allow bad vcn cider",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
CIDR: "not-a-cidr",
},
},
},
},
errorMgsShouldContain: "invalid CIDR format",
expectErr: true,
},
{
name: "shouldn't allow blank OCIResourceIdentifier",
Expand All @@ -60,15 +124,89 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
CompartmentId: "ocid",
},
},
expectErr: true,
errorMgsShouldContain: "ociResourceIdentifier",
expectErr: true,
},
{
name: "should succeed",
name: "shouldn't allow subnet cidr outside of vcn cidr",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
CIDR: "10.0.0.0/16",
Subnets: badSubnetCidr,
},
},
},
},
errorMgsShouldContain: "cidr",
expectErr: true,
},
{
name: "shouldn't allow subnet bad cidr format",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
CIDR: "10.0.0.0/16",
Subnets: badSubnetCidrFormat,
},
},
},
},
errorMgsShouldContain: "invalid CIDR format",
expectErr: true,
},
{
name: "shouldn't allow subnet cidr outside of vcn cidr",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
CIDR: "10.0.0.0/16",
Subnets: dupSubnetNames,
},
},
},
},
errorMgsShouldContain: "networkSpec.subnets: Duplicate value",
expectErr: true,
},
{
name: "shouldn't allow invalid subnet name",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
CIDR: "10.0.0.0/16",
Subnets: badSubnetName,
},
},
},
},
errorMgsShouldContain: "subnet name invalid",
expectErr: true,
},
{
name: "should succeed",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-name",
},
Spec: OCIClusterSpec{
Region: "us-lexington-1",
CompartmentId: "ocid",
OCIResourceIdentifier: "uuid",
NetworkSpec: NetworkSpec{
Vcn: VCN{
CIDR: "10.0.0.0/16",
Subnets: goodSubnets,
},
},
},
},
expectErr: false,
Expand All @@ -79,7 +217,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
g := gomega.NewWithT(t)

if test.expectErr {
g.Expect(test.c.ValidateCreate()).NotTo(gomega.Succeed())
err := test.c.ValidateCreate()
g.Expect(err).NotTo(gomega.Succeed())
g.Expect(strings.Contains(err.Error(), test.errorMgsShouldContain)).To(gomega.BeTrue())
} else {
g.Expect(test.c.ValidateCreate()).To(gomega.Succeed())
}
Expand All @@ -88,11 +228,19 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
}

func TestOCICluster_ValidateUpdate(t *testing.T) {
goodSubnets := []*Subnet{
&Subnet{
Name: "test-subnet",
CIDR: "10.0.0.0/16",
},
}

tests := []struct {
name string
c *OCICluster
old *OCICluster
expectErr bool
name string
c *OCICluster
old *OCICluster
errorField string
expectErr bool
}{
{
name: "shouldn't region change",
Expand All @@ -108,7 +256,8 @@ func TestOCICluster_ValidateUpdate(t *testing.T) {
Region: "old-region",
},
},
expectErr: true,
errorField: "region",
expectErr: true,
},
{
name: "shouldn't change OCIResourceIdentifier",
Expand All @@ -127,23 +276,40 @@ func TestOCICluster_ValidateUpdate(t *testing.T) {
OCIResourceIdentifier: "uuid-2",
},
},
expectErr: true,
errorField: "ociResourceIdentifier",
expectErr: true,
},
{
name: "should succeed",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
},
Spec: OCIClusterSpec{
CompartmentId: "ocid",
Region: "old-region",
OCIResourceIdentifier: "uuid",
NetworkSpec: NetworkSpec{
Vcn: VCN{
CIDR: "10.0.0.0/16",
Subnets: goodSubnets,
},
},
},
},
old: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
},
Spec: OCIClusterSpec{
Region: "old-region",
OCIResourceIdentifier: "uuid",
NetworkSpec: NetworkSpec{
Vcn: VCN{
CIDR: "10.0.0.0/16",
Subnets: goodSubnets,
},
},
},
},
expectErr: false,
Expand All @@ -155,7 +321,10 @@ func TestOCICluster_ValidateUpdate(t *testing.T) {
g := gomega.NewWithT(t)

if test.expectErr {
g.Expect(test.c.ValidateUpdate(test.old)).NotTo(gomega.Succeed())
//g.Expect(test.c.ValidateUpdate(test.old)).NotTo(gomega.Succeed())
err := test.c.ValidateUpdate(test.old)
g.Expect(err).NotTo(gomega.Succeed())
g.Expect(strings.Contains(err.Error(), test.errorField)).To(gomega.BeTrue())
} else {
g.Expect(test.c.ValidateUpdate(test.old)).To(gomega.Succeed())
}
Expand Down

0 comments on commit 184feed

Please sign in to comment.