Merge pull request #4982 from XiShanYongYe-Chang/add-verify-dependencies-response

add verify for customized dependencies response

pkg/resourceinterpreter/customized/declarative/configurable.go

@@ -30,6 +30,7 @@ import (
 	"github.com/karmada-io/karmada/pkg/resourceinterpreter/customized/declarative/configmanager"
 	"github.com/karmada-io/karmada/pkg/resourceinterpreter/customized/declarative/luavm"
 	"github.com/karmada-io/karmada/pkg/util/fedinformer/genericmanager"
+	"github.com/karmada-io/karmada/pkg/util/interpreter/validation"
 )
 
 // ConfigurableInterpreter interprets resources with resource interpreter customizations.
@@ -178,6 +179,10 @@ func (c *ConfigurableInterpreter) GetDependencies(object *unstructured.Unstructu
 				object.GroupVersionKind(), object.GetNamespace(), object.GetName(), err)
 			return
 		}
+		err = validation.VerifyDependencies(references)
+		if err != nil {
+			return
+		}
 		refs.Insert(references...)
 	}
 	dependencies = refs.UnsortedList()

pkg/resourceinterpreter/customized/webhook/request/resourceinterpretercontext.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/uuid"
 
 	configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1"
+	"github.com/karmada-io/karmada/pkg/util/interpreter/validation"
 )
 
 // CreateResourceInterpreterContext returns the unique request uid, the ResourceInterpreterContext object to send the webhook,
@@ -114,6 +115,10 @@ func verifyResourceInterpreterContext(operation configv1alpha1.InterpreterOperat
 		res.ReplicaRequirements = response.ReplicaRequirements
 		return res, nil
 	case configv1alpha1.InterpreterOperationInterpretDependency:
+		err := validation.VerifyDependencies(response.Dependencies)
+		if err != nil {
+			return nil, err
+		}
 		res.Dependencies = response.Dependencies
 		return res, nil
 	case configv1alpha1.InterpreterOperationPrune, configv1alpha1.InterpreterOperationReviseReplica,

pkg/resourceinterpreter/default/native/default.go

@@ -26,6 +26,7 @@ import (
 
 	configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1"
 	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
+	"github.com/karmada-io/karmada/pkg/util/interpreter/validation"
 )
 
 // DefaultInterpreter contains all default operation interpreter factory
@@ -136,7 +137,12 @@ func (e *DefaultInterpreter) GetDependencies(object *unstructured.Unstructured)
 	if !exist {
 		return dependencies, fmt.Errorf("default interpreter for operation %s not found", configv1alpha1.InterpreterOperationInterpretDependency)
 	}
-	return handler(object)
+
+	dependencies, err = handler(object)
+	if err != nil {
+		return
+	}
+	return dependencies, validation.VerifyDependencies(dependencies)
 }
 
 // ReflectStatus returns the status of the object.

pkg/resourceinterpreter/default/thirdparty/thirdparty.go

@@ -29,6 +29,7 @@ import (
 	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
 	"github.com/karmada-io/karmada/pkg/resourceinterpreter/customized/declarative/configmanager"
 	"github.com/karmada-io/karmada/pkg/resourceinterpreter/customized/declarative/luavm"
+	"github.com/karmada-io/karmada/pkg/util/interpreter/validation"
 )
 
 // ConfigurableInterpreter interprets resources with third party resource interpreter.
@@ -164,6 +165,10 @@ func (p *ConfigurableInterpreter) GetDependencies(object *unstructured.Unstructu
 				object.GroupVersionKind(), object.GetNamespace(), object.GetName(), err)
 			return
 		}
+		err = validation.VerifyDependencies(references)
+		if err != nil {
+			return
+		}
 		refs.Insert(references...)
 	}
 	dependencies = refs.UnsortedList()

pkg/util/interpreter/validation/validation.go

@@ -0,0 +1,40 @@
+/*
+Copyright 2024 The Karmada Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package validation
+
+import (
+	"errors"
+
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+
+	configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1"
+)
+
+// VerifyDependencies verifies dependencies.
+func VerifyDependencies(dependencies []configv1alpha1.DependentObjectReference) error {
+	var errs []error
+	for _, dependency := range dependencies {
+		if len(dependency.APIVersion) == 0 || len(dependency.Kind) == 0 {
+			errs = append(errs, errors.New("dependency missing required apiVersion or kind"))
+			continue
+		}
+		if len(dependency.Name) == 0 && dependency.LabelSelector == nil {
+			errs = append(errs, errors.New("dependency can not leave name and labelSelector all empty"))
+		}
+	}
+	return utilerrors.NewAggregate(errs)
+}

pkg/util/interpreter/validation/validation_test.go

@@ -0,0 +1,73 @@
+/*
+Copyright 2024 The Karmada Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package validation
+
+import (
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1"
+)
+
+func TestVerifyDependencies(t *testing.T) {
+	type args struct {
+		dependencies []configv1alpha1.DependentObjectReference
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{
+			name: "normal case",
+			args: args{dependencies: []configv1alpha1.DependentObjectReference{
+				{APIVersion: "v1", Kind: "Foo", Name: "test"},
+				{APIVersion: "v2", Kind: "Hu", Namespace: "default", LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"bar": "foo"}}},
+			}},
+			wantErr: false,
+		},
+		{
+			name: "empty apiVersion",
+			args: args{dependencies: []configv1alpha1.DependentObjectReference{
+				{Kind: "Foo", Name: "test"},
+			}},
+			wantErr: true,
+		},
+		{
+			name: "empty kind",
+			args: args{dependencies: []configv1alpha1.DependentObjectReference{
+				{APIVersion: "v1", Name: "test"},
+			}},
+			wantErr: true,
+		},
+		{
+			name: "empty Name and LabelSelector at the same time",
+			args: args{dependencies: []configv1alpha1.DependentObjectReference{
+				{APIVersion: "v1", Kind: "Foo"},
+			}},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := VerifyDependencies(tt.args.dependencies); (err != nil) != tt.wantErr {
+				t.Errorf("VerifyDependencies() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}