Merge pull request #91743 from spiffxp/fix-kubeconform-link

Merge behavior loading code

test/conformance/behaviors/BUILD

@@ -2,9 +2,16 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
-    srcs = ["types.go"],
+    srcs = [
+        "behaviors.go",
+        "types.go",
+    ],
     importpath = "k8s.io/kubernetes/test/conformance/behaviors",
     visibility = ["//visibility:public"],
+    deps = [
+        "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
+        "//vendor/gopkg.in/yaml.v2:go_default_library",
+    ],
 )
 
 go_test(
@@ -12,9 +19,6 @@ go_test(
     srcs = ["behaviors_test.go"],
     data = glob(["*/*.yaml"]),
     embed = [":go_default_library"],
-    deps = [
-        "//vendor/gopkg.in/yaml.v2:go_default_library",
-    ],
 )
 
 filegroup(

test/conformance/behaviors/behaviors.go

@@ -0,0 +1,78 @@
+/*
+Copyright 2020 The Kubernetes 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 behaviors
+
+import (
+	"fmt"
+	"io/ioutil"
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+
+	"gopkg.in/yaml.v2"
+)
+
+// BehaviorFileList returns a list of eligible behavior files in or under dir
+func BehaviorFileList(dir string) ([]string, error) {
+	var behaviorFiles []string
+
+	r, _ := regexp.Compile(".+.yaml$")
+	err := filepath.Walk(dir,
+		func(path string, info os.FileInfo, err error) error {
+			if err != nil {
+				return err
+			}
+			if r.MatchString(path) {
+				behaviorFiles = append(behaviorFiles, path)
+			}
+			return nil
+		},
+	)
+	return behaviorFiles, err
+}
+
+// LoadSuite loads a Behavior Suite from .yaml file at path
+func LoadSuite(path string) (*Suite, error) {
+	var suite Suite
+	bytes, err := ioutil.ReadFile(path)
+	if err != nil {
+		return nil, fmt.Errorf("error loading suite %s: %v", path, err)
+	}
+	err = yaml.UnmarshalStrict(bytes, &suite)
+	if err != nil {
+		return nil, fmt.Errorf("error loading suite %s: %v", path, err)
+	}
+	return &suite, nil
+}
+
+// ValidateSuite validates that the given suite has no duplicate behavior IDs
+func ValidateSuite(suite *Suite) error {
+	var errs []error
+	behaviorsByID := make(map[string]bool)
+	for _, b := range suite.Behaviors {
+		if _, ok := behaviorsByID[b.ID]; ok {
+			errs = append(errs, fmt.Errorf("Duplicate behavior ID: %s", b.ID))
+		}
+		if !strings.HasPrefix(b.ID, suite.Suite) {
+			errs = append(errs, fmt.Errorf("Invalid behavior ID: %s, must have suite name as prefix: %s", b.ID, suite.Suite))
+		}
+		behaviorsByID[b.ID] = true
+	}
+	return utilerrors.NewAggregate(errs)
+}

test/conformance/behaviors/behaviors_test.go

@@ -17,30 +17,11 @@ limitations under the License.
 package behaviors
 
 import (
-	"io/ioutil"
-	"os"
-	"path/filepath"
-	"regexp"
 	"testing"
-
-	"gopkg.in/yaml.v2"
 )
 
 func TestValidate(t *testing.T) {
-	var behaviorFiles []string
-
-	err := filepath.Walk(".",
-		func(path string, info os.FileInfo, err error) error {
-			if err != nil {
-				t.Errorf("%q", err.Error())
-			}
-
-			r, _ := regexp.Compile(".+.yaml$")
-			if r.MatchString(path) {
-				behaviorFiles = append(behaviorFiles, path)
-			}
-			return nil
-		})
+	behaviorFiles, err := BehaviorFileList(".")
 	if err != nil {
 		t.Errorf("%q", err.Error())
 	}
@@ -51,25 +32,12 @@ func TestValidate(t *testing.T) {
 }
 
 func validateSuite(path string, t *testing.T) {
-	var suite Suite
-	yamlFile, err := ioutil.ReadFile(path)
+	suite, err := LoadSuite(path)
 	if err != nil {
 		t.Errorf("%q", err.Error())
 	}
-	err = yaml.Unmarshal(yamlFile, &suite)
-
+	err = ValidateSuite(suite)
 	if err != nil {
-		t.Errorf("%q", err.Error())
-	}
-
-	behaviorIDList := make(map[string]bool)
-
-	for _, behavior := range suite.Behaviors {
-
-		// Ensure no behavior IDs are duplicated
-		if _, ok := behaviorIDList[behavior.ID]; ok {
-			t.Errorf("Duplicate behavior ID: %s", behavior.ID)
-		}
-		behaviorIDList[behavior.ID] = true
+		t.Errorf("error validating %s: %q", path, err.Error())
 	}
 }

test/conformance/behaviors/sig-network/service-spec.yaml

@@ -1,30 +1,30 @@
 suite: service/spec
 description: Base suite for services
 behaviors:
-- id: service/basic-create/selector
+- id: service/spec/selector/present-during-create
   description: When a Service resource is created with type "ClusterIP", "NodePort", or "LoadBalancer",
     and a selector is specified, an Endpoints object is generated based with the IPs of pods with
     label keys and values matching the selector.
-- id: service/basic-create/no-selector
+- id: service/spec/selector/absent-during-create
   description: When a Service resource is created and a no selector is specified, no changes are made
     to any corresponding Endpoints object.
-- id: service/type/ClusterIP/empty
+- id: service/spec/type/ClusterIP/empty
   description: When the Service type is specified as "ClusterIP" and the clusterIP
     field is empty, a cluster-internal IP address for load-balancing to endpoints is
     allocated.
-- id: service/type/ClusterIP/static
+- id: service/spec/type/ClusterIP/static
   description: When the Service type is specified as "ClusterIP" and the clusterIP
     field is specified as an IP address in the cluster service range, and that IP is
     not already assigned to another service, that IP is be allocated as a cluster-internal
     IP address for load-balancing to endpoints.
-- id: service/type/ClusterIP/None
+- id: service/spec/type/ClusterIP/None
   description: When the Service type is specified as "ClusterIP" and the clusterIP
     field is "None", no virtual IP is allocated and the endpoints are published as a
     set of endpoints rather than a stable IP.
-- id: service/type/NodePort
+- id: service/spec/type/NodePort
   description: When the Service type is specified as "NodePort" , a cluster-internal
     IP address for load-balancing to endpoints is allocated as for type "ClusterIP".
     Additionally, a cluster-wide port is allocated on every node, routing to the clusterIP.
-- id: service/type/ExternalName
+- id: service/spec/type/ExternalName
   description: When the Service type is specified as "ExternalName", the cluster DNS provider
     publishes a CNAME pointing from the service to the specified external name.

test/conformance/behaviors/sig-node/pod-spec.yaml

@@ -42,8 +42,8 @@ behaviors:
   description: When hostIPC is set to false, the Pod MUST NOT use the host's inter-process
     communication namespace.
 - id: pod/spec/label/create
-  decription: Create a Pod with a unique label. Query for the Pod with the label as selector MUST be successful.
+  description: Create a Pod with a unique label. Query for the Pod with the label as selector MUST be successful.
 - id: pod/spec/label/patch
-  decription: A patch request must be able to update the pod to change the value of an existing Label. Query for the Pod with the new value for the label MUST be successful.
+  description: A patch request must be able to update the pod to change the value of an existing Label. Query for the Pod with the new value for the label MUST be successful.
 - id: pod/spec/container/resources
   description: Create a Pod with CPU and Memory request and limits. Pod status MUST have QOSClass set to PodQOSGuaranteed.

test/conformance/kubeconform/link.go

@@ -19,9 +19,6 @@ package main
 import (
 	"fmt"
 	"io/ioutil"
-	"os"
-	"path/filepath"
-	"regexp"
 
 	"gopkg.in/yaml.v2"
 
@@ -29,30 +26,21 @@ import (
 )
 
 func link(o *options) error {
-	var behaviorFiles []string
 	behaviorsMapping := make(map[string][]string)
 	var conformanceDataList []behaviors.ConformanceData
 
-	err := filepath.Walk(o.behaviorsDir,
-		func(path string, info os.FileInfo, err error) error {
-			if err != nil {
-				return err
-			}
-			r, _ := regexp.Compile(".+.yaml$")
-			if r.MatchString(path) {
-				behaviorFiles = append(behaviorFiles, path)
-			}
-			return nil
-		})
+	behaviorFiles, err := behaviors.BehaviorFileList(o.behaviorsDir)
 	if err != nil {
 		return err
 	}
+
 	fmt.Println()
 	fmt.Printf("Using behaviors from these %d files:\n", len(behaviorFiles))
 	for _, f := range behaviorFiles {
 		fmt.Println("    ", f)
 	}
 	fmt.Println()
+
 	if o.listAll {
 		fmt.Println("All behaviors:")
 	} else {
@@ -60,17 +48,14 @@ func link(o *options) error {
 	}
 
 	for _, behaviorFile := range behaviorFiles {
-		var suite behaviors.Suite
-
-		yamlFile, err := ioutil.ReadFile(behaviorFile)
+		suite, err := behaviors.LoadSuite(behaviorFile)
 		if err != nil {
 			return err
 		}
-		err = yaml.UnmarshalStrict(yamlFile, &suite)
+		err = behaviors.ValidateSuite(suite)
 		if err != nil {
-			return err
+			return fmt.Errorf("error validating %s: %q", behaviorFile, err.Error())
 		}
-
 		for _, behavior := range suite.Behaviors {
 			behaviorsMapping[behavior.ID] = nil
 		}
@@ -78,12 +63,12 @@ func link(o *options) error {
 
 	conformanceYaml, err := ioutil.ReadFile(o.testdata)
 	if err != nil {
-		return err
+		return fmt.Errorf("%s: %v", o.testdata, err)
 	}
 
 	err = yaml.Unmarshal(conformanceYaml, &conformanceDataList)
 	if err != nil {
-		return err
+		return fmt.Errorf("%s: %v", o.testdata, err)
 	}
 
 	for _, data := range conformanceDataList {