Fixes panic when inventory is empty

pkg/kptfile/kptfileutil/util.go

@@ -15,9 +15,11 @@
 package kptfileutil
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/GoogleContainerTools/kpt/pkg/kptfile"
 	"sigs.k8s.io/kustomize/kyaml/errors"
@@ -110,3 +112,24 @@ func ReadFileStrict(pkgPath string) (kptfile.KptFile, error) {
 	}
 	return kf, nil
 }
+
+// ValidateInventory returns true and a nil error if the passed inventory
+// is valid; otherwise, false and the reason the inventory is not valid
+// is returned. A valid inventory must have a non-empty namespace, name,
+// and id.
+func ValidateInventory(inv *kptfile.Inventory) (bool, error) {
+	if inv == nil {
+		return false, fmt.Errorf("kptfile missing inventory section")
+	}
+	// Validate the name, namespace, and inventory id
+	if strings.TrimSpace(inv.Name) == "" {
+		return false, fmt.Errorf("kptfile inventory empty name")
+	}
+	if strings.TrimSpace(inv.Namespace) == "" {
+		return false, fmt.Errorf("kptfile inventory empty namespace")
+	}
+	if strings.TrimSpace(inv.InventoryID) == "" {
+		return false, fmt.Errorf("kptfile inventory missing inventoryID")
+	}
+	return true, nil
+}

pkg/kptfile/kptfileutil/util_test.go

@@ -0,0 +1,56 @@
+// Copyright 2020 Google LLC
+//
+// 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 kptfileutil
+
+import (
+	"testing"
+
+	"github.com/GoogleContainerTools/kpt/pkg/kptfile"
+)
+
+// TestValidateInventory tests the ValidateInventory function.
+func TestValidateInventory(t *testing.T) {
+	// nil inventory should not validate
+	isValid, err := ValidateInventory(nil)
+	if isValid || err == nil {
+		t.Errorf("nil inventory should not validate")
+	}
+	// Empty inventory should not validate
+	inv := &kptfile.Inventory{}
+	isValid, err = ValidateInventory(inv)
+	if isValid || err == nil {
+		t.Errorf("empty inventory should not validate")
+	}
+	// Empty inventory parameters strings should not validate
+	inv = &kptfile.Inventory{
+		Namespace:   "",
+		Name:        "",
+		InventoryID: "",
+	}
+	isValid, err = ValidateInventory(inv)
+	if isValid || err == nil {
+		t.Errorf("empty inventory parameters strings should not validate")
+	}
+	// Inventory with non-empty namespace, name, and id should validate.
+	inv = &kptfile.Inventory{
+		Namespace:   "test-namespace",
+		Name:        "test-name",
+		InventoryID: "test-id",
+	}
+	isValid, err = ValidateInventory(inv)
+	if !isValid || err != nil {
+		t.Errorf("inventory with non-empty namespace, name, and id should validate")
+	}
+}

pkg/live/rgpath.go

@@ -5,7 +5,6 @@ package live
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/GoogleContainerTools/kpt/pkg/kptfile"
 	"github.com/GoogleContainerTools/kpt/pkg/kptfile/kptfileutil"
@@ -31,41 +30,32 @@ func (p *ResourceGroupPathManifestReader) Read() ([]*unstructured.Unstructured,
 	if err != nil {
 		return []*unstructured.Unstructured{}, err
 	}
+	klog.V(4).Infof("path Read() %d resources", len(objs))
 	// Read the Kptfile in the top directory to get the inventory
 	// parameters to create the ResourceGroup inventory object.
 	kf, err := kptfileutil.ReadFile(p.pathReader.Path)
+	if err != nil {
+		klog.V(4).Infof("unable to parse Kptfile for ResourceGroup inventory: %s", err)
+		return objs, nil
+	}
+	inv := kf.Inventory
+	invObj, err := generateInventoryObj(inv)
 	if err == nil {
-		inv := kf.Inventory
 		klog.V(4).Infof("from Kptfile generating ResourceGroup inventory object %s/%s/%s",
 			inv.Namespace, inv.Name, inv.InventoryID)
-		invObj, err := generateInventoryObj(inv)
-		if err == nil {
-			objs = append(objs, invObj)
-		} else {
-			klog.V(4).Infof("unable to generate ResourceGroup inventory: %s", err)
-		}
+		objs = append(objs, invObj)
 	} else {
-		klog.V(4).Infof("unable to parse Kpfile for ResourceGroup inventory: %s", err)
+		klog.V(4).Infof("unable to generate ResourceGroup inventory: %s", err)
 	}
-	klog.V(4).Infof("path Read() generated %d resources", len(objs))
 	return objs, nil
 }
 
 // generateInventoryObj returns the ResourceGroupInventory object using the
 // passed information.
 func generateInventoryObj(inv *kptfile.Inventory) (*unstructured.Unstructured, error) {
-	// Validate the parameters
-	name := strings.TrimSpace(inv.Name)
-	if name == "" {
-		return nil, fmt.Errorf("kptfile inventory empty name")
-	}
-	namespace := strings.TrimSpace(inv.Namespace)
-	if namespace == "" {
-		return nil, fmt.Errorf("kptfile inventory empty namespace")
-	}
-	id := strings.TrimSpace(inv.InventoryID)
-	if id == "" {
-		return nil, fmt.Errorf("kptfile inventory missing inventoryID")
+	// First, ensure the Kptfile inventory section is valid.
+	if isValid, err := kptfileutil.ValidateInventory(inv); !isValid {
+		return nil, err
 	}
 	// Create and return ResourceGroup custom resource as inventory object.
 	groupVersion := fmt.Sprintf("%s/%s", ResourceGroupGVK.Group, ResourceGroupGVK.Version)
@@ -74,10 +64,10 @@ func generateInventoryObj(inv *kptfile.Inventory) (*unstructured.Unstructured, e
 			"apiVersion": groupVersion,
 			"kind":       ResourceGroupGVK.Kind,
 			"metadata": map[string]interface{}{
-				"name":      name,
-				"namespace": namespace,
+				"name":      inv.Name,
+				"namespace": inv.Namespace,
 				"labels": map[string]interface{}{
-					common.InventoryLabel: id,
+					common.InventoryLabel: inv.InventoryID,
 				},
 			},
 			"spec": map[string]interface{}{
@@ -89,7 +79,7 @@ func generateInventoryObj(inv *kptfile.Inventory) (*unstructured.Unstructured, e
 	if labels == nil {
 		labels = make(map[string]string)
 	}
-	labels[common.InventoryLabel] = id
+	labels[common.InventoryLabel] = inv.InventoryID
 	inventoryObj.SetLabels(labels)
 	inventoryObj.SetAnnotations(inv.Annotations)
 	return inventoryObj, nil