Merge pull request #27 from PDOK/wr/url-immutability

improved url immutability checks

Author: Shalucik

pkg/validation/url.go

@@ -3,6 +3,7 @@ package validation
 import (
 	"errors"
 	"fmt"
+	"github.com/pdok/smooth-operator/model"
 	"net/url"
 
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -27,14 +28,13 @@ func ValidateBaseURL(baseURL string) error {
 	return nil
 }
 
-type BaseUrlProvider interface {
-	GetBaseUrl() string
-}
-
-func CheckBaseUrlImmutability(oldProvider BaseUrlProvider, newProvider BaseUrlProvider, allErrs *field.ErrorList) {
-	if oldProvider.GetBaseUrl() != newProvider.GetBaseUrl() {
+func CheckUrlImmutability(oldURL, newURL model.URL, allErrs *field.ErrorList, path *field.Path) {
+	if oldURL.URL == nil && newURL.URL == nil {
+		return
+	}
+	if (oldURL.URL == nil && newURL.URL != nil) || (oldURL.URL != nil && newURL.URL == nil) || (*oldURL.URL != *newURL.URL) {
 		*allErrs = append(*allErrs, field.Forbidden(
-			field.NewPath("spec").Child("service").Child("baseURL"),
+			path,
 			"is immutable",
 		))
 	}

pkg/validation/url_test.go

@@ -0,0 +1,75 @@
+package validation
+
+import (
+	"github.com/pdok/smooth-operator/model"
+	"github.com/stretchr/testify/assert"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"testing"
+)
+
+func TestCheckUrlImmutability(t *testing.T) {
+	URL1, _ := model.ParseURL("http://test.com/test")
+	URL2, _ := model.ParseURL("http://test.com/test")
+	URL3, _ := model.ParseURL("http://test.com/other")
+	type args struct {
+		oldURL model.URL
+		newURL model.URL
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{
+			name: "the same URLs",
+			args: args{
+				oldURL: model.URL{URL: URL1},
+				newURL: model.URL{URL: URL2},
+			},
+			wantErr: false,
+		},
+		{
+			name: "different URLs",
+			args: args{
+				oldURL: model.URL{URL: URL1},
+				newURL: model.URL{URL: URL3},
+			},
+			wantErr: true,
+		},
+		{
+			name: "nil URLs",
+			args: args{
+				oldURL: model.URL{},
+				newURL: model.URL{},
+			},
+			wantErr: false,
+		},
+		{
+			name: "old nil, new URL",
+			args: args{
+				oldURL: model.URL{},
+				newURL: model.URL{URL: URL3},
+			},
+			wantErr: true,
+		},
+		{
+			name: "old URL, new nil",
+			args: args{
+				oldURL: model.URL{URL: URL1},
+				newURL: model.URL{URL: URL3},
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			allErrs := field.ErrorList{}
+			CheckUrlImmutability(tt.args.oldURL, tt.args.newURL, &allErrs, nil)
+			if tt.wantErr {
+				assert.NotEmpty(t, allErrs)
+			} else {
+				assert.Empty(t, allErrs)
+			}
+		})
+	}
+}