Skip to content

Commit

Permalink
feat: Add cluster name check for ocicluster webhook (#143)
Browse files Browse the repository at this point in the history
  • Loading branch information
joekr committed Sep 9, 2022
1 parent 78a3a94 commit 67e1fb2
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 17 deletions.
23 changes: 23 additions & 0 deletions api/v1beta1/ocicluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package v1beta1

import (
"fmt"
"regexp"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -37,6 +38,12 @@ var (
_ webhook.Validator = &OCICluster{}
)

const (
// can't use: \/"'[]:|<>+=;,.?*@&, Can't start with underscore. Can't end with period or hyphen.
// not using . in the name to avoid issues when the name is part of DNS name.
clusterNameRegex = `^[a-z0-9][a-z0-9-]{0,42}[a-z0-9]$`
)

// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-ocicluster,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=ociclusters,versions=v1beta1,name=validation.ocicluster.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1
// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-ocicluster,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=ociclusters,versions=v1beta1,name=default.ocicluster.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1

Expand Down Expand Up @@ -115,6 +122,7 @@ func (c *OCICluster) validate(old *OCICluster) field.ErrorList {
}

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

if len(c.Spec.CompartmentId) <= 0 {
allErrs = append(
Expand Down Expand Up @@ -148,3 +156,18 @@ func (c *OCICluster) validate(old *OCICluster) field.ErrorList {

return allErrs
}

// validateClusterName validates the cluster name
func (c *OCICluster) validateClusterName() field.ErrorList {
var allErrs field.ErrorList

if success, _ := regexp.MatchString(clusterNameRegex, c.Name); !success {
allErrs = append(allErrs, field.Invalid(field.NewPath("metadata").Child("Name"), c.Name,
fmt.Sprintf("Cluster Name doesn't match regex %s, can contain only lowercase alphanumeric characters and '-', must start/end with an alphanumeric character",
clusterNameRegex)))
}
if len(allErrs) == 0 {
return nil
}
return allErrs
}
84 changes: 67 additions & 17 deletions api/v1beta1/ocicluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
Role: "not-control-plane",
},
}
goodClusterName := "test-cluster"
badClusterName := "bad.cluster"

tests := []struct {
name string
Expand All @@ -87,7 +89,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
{
name: "shouldn't allow spaces in region",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
Region: "us city 1",
},
Expand All @@ -98,7 +102,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
{
name: "shouldn't allow bad CompartmentId",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
CompartmentId: "badocid",
},
Expand All @@ -109,16 +115,20 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
{
name: "shouldn't allow blank CompartmentId",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
Spec: OCIClusterSpec{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{},
},
errorMgsShouldContain: "compartmentId",
expectErr: true,
},
{
name: "shouldn't allow bad vcn cider",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
Expand All @@ -133,7 +143,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
{
name: "shouldn't allow blank OCIResourceIdentifier",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
CompartmentId: "ocid",
},
Expand All @@ -144,7 +156,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
{
name: "shouldn't allow subnet cidr outside of vcn cidr",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
Expand All @@ -160,7 +174,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
{
name: "should allow empty subnet cidr",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
CompartmentId: "ocid",
OCIResourceIdentifier: "uuid",
Expand All @@ -177,7 +193,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
{
name: "shouldn't allow subnet bad cidr format",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
Expand All @@ -193,7 +211,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
{
name: "shouldn't allow subnet cidr outside of vcn cidr",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
Expand All @@ -209,7 +229,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
{
name: "shouldn't allow subnet role outside of pre-defined roles",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
Expand All @@ -222,10 +244,32 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
errorMgsShouldContain: "subnet role invalid",
expectErr: true,
},
{
name: "shouldn't allow bad cluster names",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{
Name: badClusterName,
},
Spec: OCIClusterSpec{
CompartmentId: "ocid",
OCIResourceIdentifier: "uuid",
NetworkSpec: NetworkSpec{
Vcn: VCN{
CIDR: "10.0.0.0/16",
Subnets: emptySubnetName,
},
},
},
},
errorMgsShouldContain: "Cluster Name doesn't match regex",
expectErr: true,
},
{
name: "should allow empty subnet name",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
CompartmentId: "ocid",
OCIResourceIdentifier: "uuid",
Expand All @@ -242,7 +286,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
{
name: "shouldn't allow bad NSG egress cidr",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
Expand All @@ -264,7 +310,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
{
name: "shouldn't allow bad NSG ingress cidr",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
Expand All @@ -286,7 +334,9 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
{
name: "shouldn't allow bad NSG role",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: goodClusterName,
},
Spec: OCIClusterSpec{
NetworkSpec: NetworkSpec{
Vcn: VCN{
Expand All @@ -304,7 +354,7 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
name: "should allow blank region",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-name",
Name: goodClusterName,
},
Spec: OCIClusterSpec{
Region: "",
Expand All @@ -324,7 +374,7 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
name: "should succeed",
c: &OCICluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-name",
Name: goodClusterName,
},
Spec: OCIClusterSpec{
Region: "us-lexington-1",
Expand Down

0 comments on commit 67e1fb2

Please sign in to comment.