Reject ObjectReferences missing key elements. (knative#151)

Fixes: knative#150

tracker/enqueue.go

@@ -17,6 +17,7 @@ limitations under the License.
 package tracker
 
 import (
+ "fmt"
  "sync"
  "time"
 
@@ -61,6 +62,24 @@ type set map[string]time.Time
 
 // Track implements Interface.
 func (i *impl) Track(ref corev1.ObjectReference, obj interface{}) error {
+ var missingFields []string
+ if ref.APIVersion == "" {
+ missingFields = append(missingFields, "APIVersion")
+ }
+ if ref.Kind == "" {
+ missingFields = append(missingFields, "Kind")
+ }
+ if ref.Namespace == "" {
+ missingFields = append(missingFields, "Namespace")
+ }
+ if ref.Name == "" {
+ missingFields = append(missingFields, "Name")
+ }
+ if len(missingFields) > 0 {
+ return fmt.Errorf("expected ObjectReference to specify the following missing fields: %v",
+ missingFields)
+ }
+
  key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(obj)
  if err != nil {
  return err

tracker/enqueue_test.go

@@ -17,9 +17,11 @@ limitations under the License.
 package tracker
 
 import (
+ "strings"
  "testing"
  "time"
 
+ corev1 "k8s.io/api/core/v1"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
  . "github.com/knative/pkg/testing"
@@ -28,7 +30,7 @@ import (
 // Ensure our resource satisfies the interface.
 var _ accessor = (*Resource)(nil)
 
-func TestFoo(t *testing.T) {
+func TestHappyPaths(t *testing.T) {
  calls := 0
  f := func(key string) {
  calls = calls + 1
@@ -123,3 +125,78 @@ func TestFoo(t *testing.T) {
  }
  })
 }
+
+func TestBadObjectReferences(t *testing.T) {
+ trk := New(func(key string) {}, 10*time.Millisecond)
+ thing1 := &Resource{
+ TypeMeta: metav1.TypeMeta{
+ APIVersion: "ref.knative.dev/v1alpha1",
+ Kind: "Thing1",
+ },
+ ObjectMeta: metav1.ObjectMeta{
+ Namespace: "ns",
+ Name: "foo",
+ },
+ }
+
+ tests := []struct {
+ name string
+ objRef corev1.ObjectReference
+ substring string
+ }{{
+ name: "Missing APIVersion",
+ objRef: corev1.ObjectReference{
+ // APIVersion: "build.knative.dev/v1alpha1",
+ Kind: "Build",
+ Namespace: "default",
+ Name: "kaniko",
+ },
+ substring: "APIVersion",
+ }, {
+ name: "Missing Kind",
+ objRef: corev1.ObjectReference{
+ APIVersion: "build.knative.dev/v1alpha1",
+ // Kind: "Build",
+ Namespace: "default",
+ Name: "kaniko",
+ },
+ substring: "Kind",
+ }, {
+ name: "Missing Namespace",
+ objRef: corev1.ObjectReference{
+ APIVersion: "build.knative.dev/v1alpha1",
+ Kind: "Build",
+ // Namespace: "default",
+ Name: "kaniko",
+ },
+ substring: "Namespace",
+ }, {
+ name: "Missing Name",
+ objRef: corev1.ObjectReference{
+ APIVersion: "build.knative.dev/v1alpha1",
+ Kind: "Build",
+ Namespace: "default",
+ // Name: "kaniko",
+ },
+ substring: "Name",
+ }, {
+ name: "Missing All",
+ objRef: corev1.ObjectReference{
+ // APIVersion: "build.knative.dev/v1alpha1",
+ // Kind: "Build",
+ // Namespace: "default",
+ // Name: "kaniko",
+ },
+ substring: "APIVersion Kind Namespace Name",
+ }}
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ if err := trk.Track(test.objRef, thing1); err == nil {
+ t.Error("Track() = nil, wanted error")
+ } else if !strings.Contains(err.Error(), test.substring) {
+ t.Errorf("Track() = %v, wanted substring: %s", err, test.substring)
+ }
+ })
+ }
+}