replace multi yaml file parsing via yaml separator (#6170)

go.mod

@@ -50,6 +50,7 @@ require (
 	gopkg.in/square/go-jose.v2 v2.6.0
 	gopkg.in/yaml.v2 v2.4.0
 	gopkg.in/yaml.v3 v3.0.1
+	gotest.tools v2.2.0+incompatible
 	k8s.io/api v0.26.2
 	k8s.io/apimachinery v0.26.2
 	k8s.io/apiserver v0.26.2

pkg/api/v1alpha1/cloudstackmachineconfig.go

@@ -2,11 +2,12 @@ package v1alpha1
 
 import (
 	"fmt"
-	"os"
-	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/yaml"
+
+	"github.com/aws/eks-anywhere/pkg/utils/file"
+	yamlutil "github.com/aws/eks-anywhere/pkg/utils/yaml"
 )
 
 // DefaultCloudStackUser is the default CloudStackMachingConfig username.
@@ -66,23 +67,32 @@ func (c *CloudStackMachineConfigGenerate) Name() string {
 
 func GetCloudStackMachineConfigs(fileName string) (map[string]*CloudStackMachineConfig, error) {
 	configs := make(map[string]*CloudStackMachineConfig)
-	content, err := os.ReadFile(fileName)
+
+	r, err := file.ReadFile(fileName)
+	if err != nil {
+		return nil, err
+	}
+
+	resources, err := yamlutil.SplitDocuments(r)
 	if err != nil {
-		return nil, fmt.Errorf("unable to read file due to: %v", err)
+		return nil, err
 	}
-	for _, c := range strings.Split(string(content), YamlSeparator) {
+
+	for _, d := range resources {
 		var config CloudStackMachineConfig
-		if err = yaml.UnmarshalStrict([]byte(c), &config); err == nil {
+		if err = yaml.UnmarshalStrict(d, &config); err == nil {
 			if config.Kind == CloudStackMachineConfigKind {
 				configs[config.Name] = &config
 				continue
 			}
 		}
-		_ = yaml.Unmarshal([]byte(c), &config) // this is to check if there is a bad spec in the file
+
+		_ = yaml.Unmarshal(d, &config) // this is to check if there is a bad spec in the file
 		if config.Kind == CloudStackMachineConfigKind {
 			return nil, fmt.Errorf("unable to unmarshall content from file due to: %v", err)
 		}
 	}
+
 	if len(configs) == 0 {
 		return nil, fmt.Errorf("unable to find kind %v in file", CloudStackMachineConfigKind)
 	}

pkg/api/v1alpha1/cloudstackmachineconfig_test.go

@@ -1,6 +1,7 @@
 package v1alpha1
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -58,6 +59,12 @@ func TestGetCloudStackMachineConfigs(t *testing.T) {
 			wantCloudStackMachineConfigs: nil,
 			wantErr:                      true,
 		},
+		{
+			testName:                     "non-splitable manifest",
+			fileName:                     "testdata/invalid_manifest.yaml",
+			wantCloudStackMachineConfigs: nil,
+			wantErr:                      true,
+		},
 		{
 			testName: "valid 1.19",
 			fileName: "testdata/cluster_1_19_cloudstack.yaml",
@@ -253,9 +260,13 @@ func TestGetCloudStackMachineConfigs(t *testing.T) {
 			wantErr:                      true,
 		},
 	}
-	for _, tt := range tests {
+	for i, tt := range tests {
+		if i != 2 {
+			continue
+		}
 		t.Run(tt.testName, func(t *testing.T) {
 			got, err := GetCloudStackMachineConfigs(tt.fileName)
+			fmt.Println(err)
 			if (err != nil) != tt.wantErr {
 				t.Fatalf("GetCloudStackMachineConfigs() error = %v, wantErr %v", err, tt.wantErr)
 			}

pkg/api/v1alpha1/cluster.go

@@ -1,9 +1,12 @@
 package v1alpha1
 
 import (
+	"bufio"
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"net"
 	"net/url"
 	"os"
@@ -15,6 +18,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/validation"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	yamlutil "k8s.io/apimachinery/pkg/util/yaml"
 	"sigs.k8s.io/yaml"
 
 	"github.com/aws/eks-anywhere/pkg/constants"
@@ -26,7 +30,6 @@ import (
 // constants defined for cluster.go.
 const (
 	ClusterKind              = "Cluster"
-	YamlSeparator            = "\n---\n"
 	RegistryMirrorCAKey      = "EKSA_REGISTRY_MIRROR_CA"
 	podSubnetNodeMaskMaxDiff = 16
 )
@@ -264,14 +267,24 @@ type kindObject struct {
 // ParseClusterConfigFromContent unmarshalls an API object implementing the KindAccessor interface
 // from a multiobject yaml content. It doesn't set defaults nor validates the object.
 func ParseClusterConfigFromContent(content []byte, clusterConfig KindAccessor) error {
-	for _, c := range strings.Split(string(content), YamlSeparator) {
+	r := yamlutil.NewYAMLReader(bufio.NewReader(bytes.NewReader(content)))
+	for {
+		d, err := r.Read()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			return err
+		}
+
 		k := &kindObject{}
-		if err := yaml.Unmarshal([]byte(c), k); err != nil {
+
+		if err := yaml.Unmarshal(d, k); err != nil {
 			return err
 		}
 
 		if k.Kind == clusterConfig.ExpectedKind() {
-			return yaml.UnmarshalStrict([]byte(c), clusterConfig)
+			return yaml.UnmarshalStrict(d, clusterConfig)
 		}
 	}
 

pkg/api/v1alpha1/cluster_test.go

@@ -3,11 +3,15 @@ package v1alpha1
 import (
 	"errors"
 	"fmt"
+	//nolint: staticcheck
+	"io/ioutil"
 	"reflect"
 	"strings"
 	"testing"
 
 	. "github.com/onsi/gomega"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -1402,6 +1406,43 @@ func TestParseClusterConfig(t *testing.T) {
 	}
 }
 
+func Test_ParseClusterConfigFromContent(t *testing.T) {
+	tests := []struct {
+		name          string
+		fileName      string
+		clusterConfig KindAccessor
+		expectedError error
+	}{
+		{
+			name:          "Good cluster config parse",
+			fileName:      "testdata/cluster_vsphere.yaml",
+			clusterConfig: &Cluster{},
+			expectedError: nil,
+		},
+		{
+			name:          "non-splitable manifest",
+			fileName:      "testdata/invalid_manifest.yaml",
+			clusterConfig: &Cluster{},
+			expectedError: errors.New("invalid Yaml document separator: \\nkey: value\\ninvalid_separator\\n"),
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			content, err := ioutil.ReadFile(test.fileName)
+			require.NoError(t, err)
+
+			err = ParseClusterConfigFromContent(content, test.clusterConfig)
+
+			if test.expectedError != nil {
+				assert.Equal(t, test.expectedError.Error(), err.Error())
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
 func TestCluster_PauseReconcile(t *testing.T) {
 	tests := []struct {
 		name  string

pkg/api/v1alpha1/nutanixmachineconfig.go

@@ -2,12 +2,13 @@ package v1alpha1
 
 import (
 	"fmt"
-	"os"
-	"strings"
 
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/yaml"
+
+	"github.com/aws/eks-anywhere/pkg/utils/file"
+	yamlutil "github.com/aws/eks-anywhere/pkg/utils/yaml"
 )
 
 // NutanixIdentifierType is an enumeration of different resource identifier types.
@@ -122,28 +123,39 @@ func (c *NutanixMachineConfigGenerate) Name() string {
 
 func GetNutanixMachineConfigs(fileName string) (map[string]*NutanixMachineConfig, error) {
 	configs := make(map[string]*NutanixMachineConfig)
-	content, err := os.ReadFile(fileName)
+
+	r, err := file.ReadFile(fileName)
+	if err != nil {
+		return nil, err
+	}
+
+	resources, err := yamlutil.SplitDocuments(r)
 	if err != nil {
-		return nil, fmt.Errorf("unable to read file due to: %v", err)
+		return nil, err
 	}
-	for _, c := range strings.Split(string(content), YamlSeparator) {
+
+	for _, d := range resources {
 		config := NutanixMachineConfig{
 			TypeMeta: metav1.TypeMeta{},
 			ObjectMeta: metav1.ObjectMeta{
 				Annotations: map[string]string{},
 			},
 		}
-		if err = yaml.UnmarshalStrict([]byte(c), &config); err == nil {
+
+		if err = yaml.UnmarshalStrict(d, &config); err == nil {
 			if config.Kind == NutanixMachineConfigKind {
 				configs[config.Name] = &config
 				continue
 			}
 		}
-		_ = yaml.Unmarshal([]byte(c), &config) // this is to check if there is a bad spec in the file
+
+		_ = yaml.Unmarshal(d, &config) // this is to check if there is a bad spec in the file
 		if config.Kind == NutanixMachineConfigKind {
 			return nil, fmt.Errorf("unable to unmarshall content from file due to: %v", err)
 		}
+
 	}
+
 	if len(configs) == 0 {
 		return nil, fmt.Errorf("unable to find kind %v in file", NutanixMachineConfigKind)
 	}

pkg/api/v1alpha1/nutanixmachineconfig_test.go

@@ -153,15 +153,31 @@ func TestGetNutanixMachineConfigsValidConfig(t *testing.T) {
 				assert.NotNil(t, y)
 			},
 		},
+		{
+			name:        "invalid-manifest",
+			fileName:    "testdata/invalid_manifest.yaml",
+			machineConf: nil,
+			assertions: func(t *testing.T, machineConf *NutanixMachineConfig) {
+				m := machineConf.Marshallable()
+				require.NotNil(t, m)
+				y, err := yaml.Marshal(m)
+				assert.NoError(t, err)
+				assert.NotNil(t, y)
+			},
+		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			conf, err := GetNutanixMachineConfigs(test.fileName)
-			assert.NoError(t, err)
-			require.NotNil(t, conf)
-			assert.True(t, reflect.DeepEqual(test.machineConf, conf))
-			test.assertions(t, conf[machineConfName])
+			if test.machineConf != nil {
+				require.NoError(t, err)
+				require.NotNil(t, conf)
+				assert.True(t, reflect.DeepEqual(test.machineConf, conf))
+				test.assertions(t, conf[machineConfName])
+			} else {
+				assert.Error(t, err)
+			}
 		})
 	}
 }

pkg/api/v1alpha1/testdata/invalid_manifest.yaml

@@ -0,0 +1 @@
+---\nkey: value\ninvalid_separator\n

pkg/api/v1alpha1/testdata/tinkerbell_cluster_with_duplicate_mchine_config_fields.yaml

@@ -0,0 +1,45 @@
+apiVersion: anywhere.eks.amazonaws.com/v1alpha1
+kind: Cluster
+metadata:
+  name: single-node
+spec:
+  clusterNetwork:
+    cniConfig:
+      cilium: {}
+    pods:
+      cidrBlocks:
+      - 192.168.0.0/16
+    services:
+      cidrBlocks:
+      - 10.96.0.0/12
+  controlPlaneConfiguration:
+    count: 1
+    endpoint:
+      host: "10.80.8.90"
+    machineGroupRef:
+      kind: TinkerbellMachineConfig
+      name: single-node-cp
+  datacenterRef:
+    kind: TinkerbellDatacenterConfig
+    name: single-node
+  kubernetesVersion: "1.23"
+  managementCluster:
+    name: single-node
+---
+apiVersion: anywhere.eks.amazonaws.com/v1alpha1
+kind: TinkerbellDatacenterConfig
+metadata:
+  name: single-node
+spec:
+  tinkerbellIP: "10.80.8.91"
+---
+apiVersion: anywhere.eks.amazonaws.com/v1alpha1
+kind: TinkerbellMachineConfig
+metadata:
+  name: single-node-cp
+  name: another-cp
+spec:
+  hardwareSelector:
+    type: cp
+  osFamily: bottlerocket
+  templateRef: {}