.

Signed-off-by: Keran Yang <[email protected]>
KeranYang · Nov 15, 2023 · 3292416 · 3292416
1 parent aab8806
commit 3292416
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 15 deletions.
diff --git a/webhook/validator/isbsvc_test.go b/webhook/validator/isbsvc_test.go
@@ -20,22 +20,34 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	dfv1 "github.com/numaproj/numaflow/pkg/apis/numaflow/v1alpha1"
 )
 
 func TestValidateISBServiceCreate(t *testing.T) {
-	isbsvc := fakeISBSvc()
+	isbsvc := fakeRedisISBSvc()
 	v := NewISBServiceValidator(nil, isbsvc)
 	r := v.ValidateCreate(contextWithLogger(t))
 	assert.True(t, r.Allowed)
 }
 
 func TestValidateISBServiceUpdate(t *testing.T) {
-	isbsvc := fakeISBSvc()
-	t.Run("test ISBSvc spec change", func(t *testing.T) {
-		JetStreamISBSvc := fakeJetStreamISBSvc()
-		newISBSvc := JetStreamISBSvc.DeepCopy()
-		v := NewISBServiceValidator(isbsvc, newISBSvc)
-		r := v.ValidateUpdate(contextWithLogger(t))
-		assert.False(t, r.Allowed)
-	})
+	testCases := []struct {
+		name string
+		old  *dfv1.InterStepBufferService
+		new  *dfv1.InterStepBufferService
+		want bool
+	}{
+		{name: "invalid new ISBSvc spec", old: fakeRedisISBSvc(), new: nil, want: false},
+		{name: "changing ISB Service type is not allowed - redis to jetstream", old: fakeRedisISBSvc(), new: fakeJetStreamISBSvc(), want: false},
+		{name: "changing ISB Service type is not allowed - jetstream to redis", old: fakeJetStreamISBSvc(), new: fakeRedisISBSvc(), want: false},
+		{name: "valid new ISBSvc spec", old: fakeRedisISBSvc(), new: fakeRedisISBSvc(), want: true},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			v := NewISBServiceValidator(tc.old, tc.new)
+			r := v.ValidateUpdate(contextWithLogger(t))
+			assert.Equal(t, tc.want, r.Allowed)
+		})
+	}
 }
diff --git a/webhook/validator/mock_isb_client.go b/webhook/validator/mock_isb_client.go
@@ -32,7 +32,7 @@ type MockInterStepBufferServices struct {
 
 // Get takes name of the interStepBufferService, and returns the corresponding interStepBufferService object, and an error if there is any.
 func (c *MockInterStepBufferServices) Get(ctx context.Context, name string, options v1.GetOptions) (result *v1alpha1.InterStepBufferService, err error) {
-	isbsvc := fakeISBSvc()
+	isbsvc := fakeRedisISBSvc()
 	isbsvc.Status.MarkDeployed()
 	return isbsvc, err
 }

diff --git a/webhook/validator/test_utils.go b/webhook/validator/test_utils.go
@@ -14,12 +14,11 @@ const (
 
 var (
 	fakeK8sClient      = fakeClient.NewSimpleClientset()
-	fakeISBSvcClient   = fake.FakeInterStepBufferServices{}
 	fakePipelineClient = fake.FakePipelines{}
 	fakeNumaClient     = fake.FakeNumaflowV1alpha1{}
 )
 
-func fakeISBSvc() *dfv1.InterStepBufferService {
+func fakeRedisISBSvc() *dfv1.InterStepBufferService {
 	return &dfv1.InterStepBufferService{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: testNamespace,
@@ -38,7 +37,7 @@ func fakeISBSvc() *dfv1.InterStepBufferService {
 func fakeJetStreamISBSvc() *dfv1.InterStepBufferService {
 	return &dfv1.InterStepBufferService{
 		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "test-ns",
+			Namespace: testNamespace,
 			Name:      dfv1.DefaultISBSvcName,
 		},
 		Spec: dfv1.InterStepBufferServiceSpec{
@@ -53,7 +52,7 @@ func fakePipeline() *dfv1.Pipeline {
 	return &dfv1.Pipeline{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "test-pl",
-			Namespace: "test-ns",
+			Namespace: testNamespace,
 		},
 		Spec: dfv1.PipelineSpec{
 			Vertices: []dfv1.AbstractVertex{

diff --git a/webhook/validator/validator_test.go b/webhook/validator/validator_test.go
@@ -34,7 +34,7 @@ func contextWithLogger(t *testing.T) context.Context {
 
 func TestGetValidator(t *testing.T) {
 	t.Run("test get InterStepBufferService validator", func(t *testing.T) {
-		bytes, err := json.Marshal(fakeISBSvc())
+		bytes, err := json.Marshal(fakeRedisISBSvc())
 		assert.NoError(t, err)
 		assert.NotNil(t, bytes)
 		v, err := GetValidator(contextWithLogger(t), fakeK8sClient, &fakeNumaClient, metav1.GroupVersionKind{Group: "numaflow.numaproj.io", Version: "v1alpha1", Kind: "InterStepBufferService"}, nil, bytes)