fix: return error instead of log.Fatalf() (#5625)

* fix: return error instead of log.Fatalf()

* chore: add meaningful message to error output

* chore: add meaningful message to fatal function

api/resource/factory.go

@@ -41,28 +41,26 @@ func (rf *Factory) Hasher() ifc.KustHasher {
 }
 
 // FromMap returns a new instance of Resource.
-func (rf *Factory) FromMap(m map[string]interface{}) *Resource {
+func (rf *Factory) FromMap(m map[string]interface{}) (*Resource, error) {
 	res, err := rf.FromMapAndOption(m, nil)
 	if err != nil {
-		// TODO: return err instead of log.
-		log.Fatalf("failed to create resource from map: %v", err)
+		return nil, fmt.Errorf("failed to create resource from map: %w", err)
 	}
-	return res
+	return res, nil
 }
 
 // FromMapWithName returns a new instance with the given "original" name.
-func (rf *Factory) FromMapWithName(n string, m map[string]interface{}) *Resource {
+func (rf *Factory) FromMapWithName(n string, m map[string]interface{}) (*Resource, error) {
 	return rf.FromMapWithNamespaceAndName(resid.DefaultNamespace, n, m)
 }
 
 // FromMapWithNamespaceAndName returns a new instance with the given "original" namespace.
-func (rf *Factory) FromMapWithNamespaceAndName(ns string, n string, m map[string]interface{}) *Resource {
+func (rf *Factory) FromMapWithNamespaceAndName(ns string, n string, m map[string]interface{}) (*Resource, error) {
 	r, err := rf.FromMapAndOption(m, nil)
 	if err != nil {
-		// TODO: return err instead of log.
-		log.Fatalf("failed to create resource from map: %v", err)
+		return nil, fmt.Errorf("failed to create resource from map: %w", err)
 	}
-	return r.setPreviousId(ns, n, r.GetKind())
+	return r.setPreviousId(ns, n, r.GetKind()), nil
 }
 
 // FromMapAndOption returns a new instance of Resource with given options.

api/resource/factory_test.go

@@ -274,24 +274,32 @@ kind: List
 			},
 		},
 	}
-	testDeploymentA := factory.FromMap(
+	deploymentA := "deployment-a"
+	testDeploymentA, errA := factory.FromMap(
 		map[string]interface{}{
 			"apiVersion": "apps/v1",
 			"kind":       "Deployment",
 			"metadata": map[string]interface{}{
-				"name": "deployment-a",
+				"name": deploymentA,
 			},
 			"spec": testDeploymentSpec,
 		})
-	testDeploymentB := factory.FromMap(
+	if errA != nil {
+		t.Fatalf("failed to create new instance with %v: %v", deploymentA, errA)
+	}
+	deploymentB := "deployment-b"
+	testDeploymentB, errB := factory.FromMap(
 		map[string]interface{}{
 			"apiVersion": "apps/v1",
 			"kind":       "Deployment",
 			"metadata": map[string]interface{}{
-				"name": "deployment-b",
+				"name": deploymentB,
 			},
 			"spec": testDeploymentSpec,
 		})
+	if errB != nil {
+		t.Fatalf("failed to create new instance with %v: %v", deploymentB, errB)
+	}
 
 	fSys := filesys.MakeFsInMemory()
 	fSys.WriteFile(string(patchGood1), []byte(patch1))
@@ -308,6 +316,16 @@ kind: List
 		t.Fatal(err)
 	}
 
+	td, err := createTestDeployment()
+	if err != nil {
+		t.Fatalf("failed to create test deployment: %v", err)
+	}
+
+	tc, err := createTestConfigMap()
+	if err != nil {
+		t.Fatalf("failed to create test config: %v", err)
+	}
+
 	tests := map[string]struct {
 		input       []types.PatchStrategicMerge
 		expectedOut []*Resource
@@ -315,7 +333,7 @@ kind: List
 	}{
 		"happy": {
 			input:       []types.PatchStrategicMerge{patchGood1, patchGood2},
-			expectedOut: []*Resource{testDeployment, testConfigMap},
+			expectedOut: []*Resource{td, tc},
 			expectedErr: false,
 		},
 		"badFileName": {
@@ -330,7 +348,7 @@ kind: List
 		},
 		"listOfPatches": {
 			input:       []types.PatchStrategicMerge{patchList},
-			expectedOut: []*Resource{testDeployment, testConfigMap},
+			expectedOut: []*Resource{td, tc},
 			expectedErr: false,
 		},
 		"listWithAnchorReference": {

api/resource/idset_test.go

@@ -12,21 +12,45 @@ import (
 
 func TestIdSet_Empty(t *testing.T) {
 	s := MakeIdSet([]*Resource{})
+	td, err := createTestDeployment()
+	if err != nil {
+		t.Fatalf("Failed to create test deployment: %v", err)
+	}
+	tc, err := createTestConfigMap()
+	if err != nil {
+		t.Fatalf("Failed to create test config: %v", err)
+	}
 	assert.Equal(t, 0, s.Size())
-	assert.False(t, s.Contains(testDeployment.CurId()))
-	assert.False(t, s.Contains(testConfigMap.CurId()))
+	assert.False(t, s.Contains(td.CurId()))
+	assert.False(t, s.Contains(tc.CurId()))
 }
 
 func TestIdSet_One(t *testing.T) {
-	s := MakeIdSet([]*Resource{testDeployment})
+	td, err := createTestDeployment()
+	if err != nil {
+		t.Fatalf("failed to create test deployment: %v", err)
+	}
+	tc, err := createTestConfigMap()
+	if err != nil {
+		t.Fatalf("failed to create test config: %v", err)
+	}
+	s := MakeIdSet([]*Resource{td})
 	assert.Equal(t, 1, s.Size())
-	assert.True(t, s.Contains(testDeployment.CurId()))
-	assert.False(t, s.Contains(testConfigMap.CurId()))
+	assert.True(t, s.Contains(td.CurId()))
+	assert.False(t, s.Contains(tc.CurId()))
 }
 
 func TestIdSet_Two(t *testing.T) {
-	s := MakeIdSet([]*Resource{testDeployment, testConfigMap})
+	td, err := createTestDeployment()
+	if err != nil {
+		t.Fatalf("failed to create test Deployment: %v", err)
+	}
+	tc, err := createTestConfigMap()
+	if err != nil {
+		t.Fatalf("failed to create test Config: %v", err)
+	}
+	s := MakeIdSet([]*Resource{td, tc})
 	assert.Equal(t, 2, s.Size())
-	assert.True(t, s.Contains(testDeployment.CurId()))
-	assert.True(t, s.Contains(testConfigMap.CurId()))
+	assert.True(t, s.Contains(td.CurId()))
+	assert.True(t, s.Contains(tc.CurId()))
 }

api/resource/resource_test.go

@@ -20,27 +20,39 @@ import (
 
 var factory = provider.NewDefaultDepProvider().GetResourceFactory()
 
-var testConfigMap = factory.FromMap(
-	map[string]interface{}{
-		"apiVersion": "v1",
-		"kind":       "ConfigMap",
-		"metadata": map[string]interface{}{
-			"name":      "winnie",
-			"namespace": "hundred-acre-wood",
-		},
-	})
+func createTestConfigMap() (*Resource, error) {
+	res, err := factory.FromMap(
+		map[string]interface{}{
+			"apiVersion": "v1",
+			"kind":       "ConfigMap",
+			"metadata": map[string]interface{}{
+				"name":      "winnie",
+				"namespace": "hundred-acre-wood",
+			},
+		})
+	if err != nil {
+		return nil, fmt.Errorf("failed to create test config: %w", err)
+	}
+	return res, nil
+}
 
 //nolint:gosec
 const configMapAsString = `{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"winnie","namespace":"hundred-acre-wood"}}`
 
-var testDeployment = factory.FromMap(
-	map[string]interface{}{
-		"apiVersion": "apps/v1",
-		"kind":       "Deployment",
-		"metadata": map[string]interface{}{
-			"name": "pooh",
-		},
-	})
+func createTestDeployment() (*Resource, error) {
+	res, err := factory.FromMap(
+		map[string]interface{}{
+			"apiVersion": "apps/v1",
+			"kind":       "Deployment",
+			"metadata": map[string]interface{}{
+				"name": "pooh",
+			},
+		})
+	if err != nil {
+		return nil, fmt.Errorf("failed to create Deployment: %w", err)
+	}
+	return res, nil
+}
 
 const deploymentAsString = `{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"name":"pooh"}}`
 
@@ -50,9 +62,13 @@ kind: Deployment
 metadata:
   name: pooh
 `
-	yaml, err := testDeployment.AsYAML()
+	td, err := createTestDeployment()
 	if err != nil {
-		t.Fatal(err)
+		t.Fatalf("failed to create test deployment: %s", err)
+	}
+	yaml, err := td.AsYAML()
+	if err != nil {
+		t.Fatalf("failed to get yaml: %s", err)
 	}
 	if string(yaml) != expected {
 		t.Fatalf("--- expected\n%s\n--- got\n%s\n", expected, string(yaml))
@@ -60,16 +76,24 @@ metadata:
 }
 
 func TestResourceString(t *testing.T) {
+	td, err := createTestDeployment()
+	if err != nil {
+		t.Fatalf("failed to create test deployment: %v", err)
+	}
+	tc, err := createTestConfigMap()
+	if err != nil {
+		t.Fatalf("failed to create test config: %v", err)
+	}
 	tests := []struct {
 		in *Resource
 		s  string
 	}{
 		{
-			in: testConfigMap,
+			in: tc,
 			s:  configMapAsString,
 		},
 		{
-			in: testDeployment,
+			in: td,
 			s:  deploymentAsString,
 		},
 	}
@@ -79,18 +103,26 @@ func TestResourceString(t *testing.T) {
 }
 
 func TestResourceId(t *testing.T) {
+	td, err := createTestDeployment()
+	if err != nil {
+		t.Fatalf("failed to create test deployment: %v", err)
+	}
+	tc, err := createTestConfigMap()
+	if err != nil {
+		t.Fatalf("failed to create test config: %v", err)
+	}
 	tests := []struct {
 		in *Resource
 		id resid.ResId
 	}{
 		{
-			in: testConfigMap,
+			in: tc,
 			id: resid.NewResIdWithNamespace(
 				resid.NewGvk("", "v1", "ConfigMap"),
 				"winnie", "hundred-acre-wood"),
 		},
 		{
-			in: testDeployment,
+			in: td,
 			id: resid.NewResId(
 				resid.NewGvk("apps", "v1", "Deployment"), "pooh"),
 		},
@@ -103,7 +135,7 @@ func TestResourceId(t *testing.T) {
 }
 
 func TestDeepCopy(t *testing.T) {
-	r := factory.FromMap(
+	r, err := factory.FromMap(
 		map[string]interface{}{
 			"apiVersion": "apps/v1",
 			"kind":       "Deployment",
@@ -111,6 +143,9 @@ func TestDeepCopy(t *testing.T) {
 				"name": "pooh",
 			},
 		})
+	if err != nil {
+		t.Fatalf("failed to create test config: %v", err)
+	}
 	r.AppendRefBy(resid.NewResId(resid.Gvk{Group: "somegroup", Kind: "MyKind"}, "random"))
 
 	var1 := types.Var{