Fixed TODO about error definitions

Use SelectOne to replace Select and additional checks.

Change-Id: I40b4780d62fa4b3a0e01cc54ef7dbe4b32a4763d

pkg/bootstrap/cloudinit/cloud-init.go

@@ -32,19 +32,10 @@ func GetCloudData(docBundle document.Bundle) (userData []byte, netConf []byte, e
 func getUserData(docBundle document.Bundle) ([]byte, error) {
 	// find the user-data document
 	selector := document.NewEphemeralCloudDataSelector()
-	docs, err := docBundle.Select(selector)
+	userDataDoc, err := docBundle.SelectOne(selector)
 	if err != nil {
 		return nil, err
 	}
-	var userDataDoc document.Document = &document.Factory{}
-	switch numDocsFound := len(docs); {
-	case numDocsFound == 0:
-		return nil, document.ErrDocNotFound{Selector: selector}
-	case numDocsFound > 1:
-		return nil, document.ErrMultipleDocsFound{Selector: selector}
-	case numDocsFound == 1:
-		userDataDoc = docs[0]
-	}
 
 	// finally, try and retrieve the data we want from the document
 	userData, err := decodeData(userDataDoc, userDataKey)
@@ -58,44 +49,24 @@ func getUserData(docBundle document.Bundle) ([]byte, error) {
 func getNetworkData(docBundle document.Bundle) ([]byte, error) {
 	// find the baremetal host indicated as the ephemeral node
 	selector := document.NewEphemeralBMHSelector()
-	docs, err := docBundle.Select(selector)
+	d, err := docBundle.SelectOne(selector)
 	if err != nil {
 		return nil, err
 	}
 
-	var bmhDoc document.Document = &document.Factory{}
-	switch numDocsFound := len(docs); {
-	case numDocsFound == 0:
-		return nil, document.ErrDocNotFound{Selector: selector}
-	case numDocsFound > 1:
-		return nil, document.ErrMultipleDocsFound{Selector: selector}
-	case numDocsFound == 1:
-		bmhDoc = docs[0]
-	}
-
 	// try and find these documents in our bundle
-	selector, err = document.NewNetworkDataSelector(bmhDoc)
+	selector, err = document.NewNetworkDataSelector(d)
 	if err != nil {
 		return nil, err
 	}
-	docs, err = docBundle.Select(selector)
+	d, err = docBundle.SelectOne(selector)
 
 	if err != nil {
 		return nil, err
 	}
 
-	var networkDataDoc document.Document = &document.Factory{}
-	switch numDocsFound := len(docs); {
-	case numDocsFound == 0:
-		return nil, document.ErrDocNotFound{Selector: selector}
-	case numDocsFound > 1:
-		return nil, document.ErrMultipleDocsFound{Selector: selector}
-	case numDocsFound == 1:
-		networkDataDoc = docs[0]
-	}
-
 	// finally, try and retrieve the data we want from the document
-	netData, err := decodeData(networkDataDoc, networkDataKey)
+	netData, err := decodeData(d, networkDataKey)
 	if err != nil {
 		return nil, err
 	}

pkg/bootstrap/cloudinit/cloud-init_test.go

@@ -40,7 +40,7 @@ func TestGetCloudData(t *testing.T) {
 			labelFilter:      "test=ephemeralduplicate",
 			expectedUserData: nil,
 			expectedNetData:  nil,
-			expectedErr: document.ErrMultipleDocsFound{
+			expectedErr: document.ErrMultiDocsFound{
 				Selector: document.NewSelector().
 					ByLabel(document.EphemeralHostSelector).
 					ByKind("BareMetalHost"),

pkg/cluster/status_test.go

@@ -147,11 +147,8 @@ func TestGetStatusForResource(t *testing.T) {
 			testStatusMap, err := cluster.NewStatusMap(bundle)
 			require.NoError(t, err)
 
-			// TODO(howell): Replace with with SelectOne when it becomes available
+			doc, err := bundle.SelectOne(tt.selector)
-			docs, err := bundle.Select(tt.selector)
 			require.NoError(t, err)
-			require.Len(t, docs, 1)
-			doc := docs[0]
 
 			actualStatus, err := testStatusMap.GetStatusForResource(tt.testClient, doc)
 			if tt.err != nil {

pkg/document/bundle.go

@@ -2,7 +2,6 @@ package document
 
 import (
 	"errors"
-	"fmt"
 	"io"
 
 	"sigs.k8s.io/kustomize/v3/k8sdeps/kunstruct"
@@ -178,25 +177,9 @@ func (b *BundleFactory) GetAllDocuments() ([]Document, error) {
 	return docSet, nil
 }
 
-// GetByName finds a document by name, error if more than one document found
+// GetByName finds a document by name
-// or if no documents found
 func (b *BundleFactory) GetByName(name string) (Document, error) {
-	resSet := make([]*resource.Resource, 0, len(b.ResMap.Resources()))
+	return b.SelectOne(NewSelector().ByName(name))
-	for _, res := range b.ResMap.Resources() {
-		if res.GetName() == name {
-			resSet = append(resSet, res)
-		}
-	}
-	// alanmeadows(TODO): improve this and other error potentials by
-	// by adding strongly typed errors
-	switch found := len(resSet); {
-	case found == 0:
-		return &Factory{}, fmt.Errorf("no documents found with name %s", name)
-	case found > 1:
-		return &Factory{}, fmt.Errorf("more than one document found with name %s", name)
-	default:
-		return NewDocument(resSet[0])
-	}
 }
 
 // Select offers an interface to pass a Selector, built on top of kustomize Selector
@@ -239,7 +222,7 @@ func (b *BundleFactory) SelectOne(selector Selector) (Document, error) {
 	case numDocsFound == 0:
 		return nil, ErrDocNotFound{Selector: selector}
 	case numDocsFound > 1:
-		return nil, ErrMultipleDocsFound{Selector: selector}
+		return nil, ErrMultiDocsFound{Selector: selector}
 	}
 	return docSet[0], nil
 }

pkg/document/errors.go

@@ -4,11 +4,16 @@ import (
 	"fmt"
 )
 
-// ErrDocNotFound returned if desired document not found
+// ErrDocNotFound returned if desired document not found by selector
 type ErrDocNotFound struct {
 	Selector Selector
 }
 
+// ErrMultiDocsFound returned if multiple documents were found by selector
+type ErrMultiDocsFound struct {
+	Selector Selector
+}
+
 // ErrDocumentDataKeyNotFound returned if desired key within a document not found
 type ErrDocumentDataKeyNotFound struct {
 	DocName string
@@ -22,23 +27,18 @@ type ErrDocumentMalformed struct {
 	Message string
 }
 
-// ErrMultipleDocsFound returned if desired document not found
+func (e ErrDocNotFound) Error() string {
-type ErrMultipleDocsFound struct {
+	return fmt.Sprintf("document filtered by selector %v found no documents", e.Selector)
-	Selector Selector
 }
 
-func (e ErrDocNotFound) Error() string {
+func (e ErrMultiDocsFound) Error() string {
-	return fmt.Sprintf("Document filtered by selector %v found no documents", e.Selector)
+	return fmt.Sprintf("document filtered by selector %v found more than one document", e.Selector)
 }
 
 func (e ErrDocumentDataKeyNotFound) Error() string {
-	return fmt.Sprintf("Document %q cannot retrieve data key %q", e.DocName, e.Key)
+	return fmt.Sprintf("document %q cannot retrieve data key %q", e.DocName, e.Key)
 }
 
 func (e ErrDocumentMalformed) Error() string {
-	return fmt.Sprintf("Document %q is malformed: %q", e.DocName, e.Message)
+	return fmt.Sprintf("document %q is malformed: %q", e.DocName, e.Message)
-}
-
-func (e ErrMultipleDocsFound) Error() string {
-	return fmt.Sprintf("Document filtered by selector %v found more than one document", e.Selector)
 }