Some slight refactoring and adding back Unmarshal logic to preserve o…

…rder of releases (#533)

cmd/root.go

@@ -207,7 +207,7 @@ var convertCmd = &cobra.Command{
 		return validateArgs(cmd, args)
 	},
 	Run: func(cmd *cobra.Command, args []string) {
-		newCourse, err := course.OpenCourseV2(courseFile, courseSchema)
+		newCourse, err := course.OpenCourseFile(courseFile, courseSchema)
 		if err != nil {
 			color.Red(err.Error())
 			os.Exit(1)

end_to_end_testing/tests/24_no_context.yaml

@@ -27,3 +27,7 @@ testcases:
       reckoner template {{.course}} -o chart-with-no-context
     assertions:
     - result.code ShouldEqual 0
+  - name: 24 - cleanup
+    steps:
+      - script: |
+          kubectl delete ns {{.namespace}}

pkg/course/course.go

@@ -138,6 +138,8 @@ type Release struct {
 
 // ReleaseV1 represents a helm release and all of its configuration from v1 schema
 type ReleaseV1 struct {
+	// Name is the name of the release
+	Name string `yaml:"name" json:"name"`
 	// Namespace is the namespace that this release should be placed in
 	Namespace string `yaml:"namespace,omitempty" json:"namespace,omitempty"`
 	// NamespaceMgmt is a set of labels and annotations to be added to the namespace for this release
@@ -183,10 +185,14 @@ type FileV1 struct {
 		// Default is the default namespace config for this course
 		Default *NamespaceConfig `yaml:"default" json:"default"`
 	} `yaml:"namespace_management" json:"namespace_management"`
-	// Charts is the map of releases
-	Charts map[string]ReleaseV1 `yaml:"charts" json:"charts"`
+	// Charts is the list of releases. In the actual file this will be a map, but we must convert to a list to preserve order.
+	// This conversion is done in the ChartsListV1 UnmarshalYAML function.
+	Charts ChartsListV1 `yaml:"charts" json:"charts"`
 }
 
+// ChartsListV1 is a list of releases which we convert from a map of releases to preserve order
+type ChartsListV1 []ReleaseV1
+
 // FileV1Unmarshal is a helper type that allows us to have a custom unmarshal function for the FileV2 struct
 type FileV1Unmarshal FileV1
 
@@ -218,10 +224,11 @@ func convertV1toV2(fileName string) (*FileV2, error) {
 	newFile.NamespaceMgmt = oldFile.NamespaceMgmt
 	newFile.DefaultRepository = oldFile.DefaultRepository
 	newFile.Repositories = oldFile.Repositories
+	newFile.Releases = make([]*Release, len(oldFile.Charts))
 	newFile.Hooks = oldFile.Hooks
 	newFile.MinimumVersions = oldFile.MinimumVersions
 
-	for releaseName, release := range oldFile.Charts {
+	for releaseIndex, release := range oldFile.Charts {
 		repositoryName, ok := release.Repository.(string)
 		// The repository is not in the format repository: string. Need to handle that
 		if !ok {
@@ -238,7 +245,7 @@ func convertV1toV2(fileName string) (*FileV2, error) {
 			if addRepo.Git != "" {
 				klog.V(3).Infof("detected a git-based inline repository. Attempting to convert to repository in header")
 
-				repositoryName = fmt.Sprintf("%s-git-repository", releaseName)
+				repositoryName = fmt.Sprintf("%s-git-repository", release.Name)
 				newFile.Repositories[repositoryName] = Repository{
 					Git:  addRepo.Git,
 					Path: addRepo.Path,
@@ -249,32 +256,25 @@ func convertV1toV2(fileName string) (*FileV2, error) {
 				repositoryName = addRepo.Name
 			}
 		}
-		newFile.Releases = append(newFile.Releases, &Release{
-			Name:          releaseName,
+		newFile.Releases[releaseIndex] = &Release{
+			Name:          release.Name,
 			Namespace:     release.Namespace,
 			NamespaceMgmt: release.NamespaceMgmt,
 			Repository:    repositoryName,
 			Chart:         release.Chart,
 			Version:       release.Version,
 			Values:        release.Values,
 			Hooks:         release.Hooks,
-		})
+		}
 	}
 	return newFile, nil
 }
 
-// OpenCourseV2 opens a v2 schema course file
-func OpenCourseV2(fileName string, schema []byte) (*FileV2, error) {
-	courseFile := &FileV2{}
-	data, err := ioutil.ReadFile(fileName)
+func OpenCourseFile(fileName string, schema []byte) (*FileV2, error) {
+	courseFile, err := OpenCourseV2(fileName)
 	if err != nil {
 		return nil, err
 	}
-	err = yaml.Unmarshal(data, courseFile)
-	if err != nil {
-		klog.V(3).Infof("failed to unmarshal file: %s", err.Error())
-		return nil, SchemaValidationError
-	}
 	if courseFile.SchemaVersion != "v2" {
 		klog.V(2).Infof("did not detect v2 course file - trying conversion from v1")
 		fileV2, errConvert := convertV1toV2(fileName)
@@ -307,6 +307,21 @@ func OpenCourseV2(fileName string, schema []byte) (*FileV2, error) {
 	return courseFile, nil
 }
 
+// OpenCourseV2 opens a v2 schema course file
+func OpenCourseV2(fileName string) (*FileV2, error) {
+	courseFile := &FileV2{}
+	data, err := ioutil.ReadFile(fileName)
+	if err != nil {
+		return nil, err
+	}
+	err = yaml.Unmarshal(data, courseFile)
+	if err != nil {
+		klog.V(3).Infof("failed to unmarshal file: %s", err.Error())
+		return nil, SchemaValidationError
+	}
+	return courseFile, nil
+}
+
 // OpenCourseV1 opens a v1 schema course file
 func OpenCourseV1(fileName string) (*FileV1, error) {
 	courseFile := &FileV1{}
@@ -316,11 +331,22 @@ func OpenCourseV1(fileName string) (*FileV1, error) {
 	}
 	err = yaml.Unmarshal(data, courseFile)
 	if err != nil {
-		return nil, err
+		klog.V(3).Infof("failed to unmarshal file: %s", err.Error())
+		return nil, SchemaValidationError
 	}
 	return courseFile, nil
 }
 
+// SetGitPaths allows the caller to set both the clone path and the chart subpath for a release.
+func (r *Release) SetGitPaths(clonePath, subPath string) error {
+	if r.GitClonePath != nil {
+		return fmt.Errorf("cannot set GitClonePath on release %s - it is already set to %s", r.Name, *r.GitClonePath)
+	}
+	r.GitClonePath = &clonePath
+	r.GitChartSubPath = &subPath
+	return nil
+}
+
 // UnmarshalYAML implements the yaml.Unmarshaler interface for FileV1. This allows us to do environment variable parsing
 // and changing behavior for boolean parsing such that non-quoted `yes`, `no`, `on`, `off` become booleans.
 func (f *FileV1) UnmarshalYAML(value *yaml.Node) error {
@@ -351,6 +377,24 @@ func (f *FileV2) UnmarshalYAML(value *yaml.Node) error {
 	return nil
 }
 
+// UnmarshalYAML implements the yaml.Unmarshaler interface to customize how we Unmarshal this particular field of the FileV1 struct
+func (cl *ChartsListV1) UnmarshalYAML(value *yaml.Node) error {
+	if value.Kind != yaml.MappingNode {
+		return fmt.Errorf("ChartsList must contain YAML mapping, has %v", value.Kind)
+	}
+	*cl = make([]ReleaseV1, len(value.Content)/2)
+	for i := 0; i < len(value.Content); i += 2 {
+		var res = &(*cl)[i/2]
+		if err := value.Content[i].Decode(&res.Name); err != nil {
+			return err
+		}
+		if err := value.Content[i+1].Decode(&res); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func decodeYamlWithEnv(value *yaml.Node) error {
 	v := *value
 	for i := 0; i < len(v.Content); i += 2 {
@@ -515,16 +559,6 @@ func (f *FileV2) validateJsonSchema(schemaData []byte) error {
 	return nil
 }
 
-// SetGitPaths allows the caller to set both the clone path and the chart subpath for a release.
-func (r *Release) SetGitPaths(clonePath, subPath string) error {
-	if r.GitClonePath != nil {
-		return fmt.Errorf("cannot set GitClonePath on release %s - it is already set to %s", r.Name, *r.GitClonePath)
-	}
-	r.GitClonePath = &clonePath
-	r.GitChartSubPath = &subPath
-	return nil
-}
-
 func parseEnv(data string) (string, error) {
 	dataWithEnv := os.Expand(data, envMapper)
 	if strings.Contains(dataWithEnv, "_ENV_NOT_SET_") {

pkg/course/testdata/convert1.yaml

@@ -10,7 +10,7 @@ repositories:
 charts:
   basic:
     chart: somechart
-    repository: 
+    repository:
       name: helm-repo
     version: "2.0.0"
     values:
@@ -23,4 +23,4 @@ charts:
     version: main
   standard:
     chart: "basic"
-    repository: "helm-repo"
+    repository: "helm-repo"

pkg/reckoner/client.go

@@ -56,7 +56,7 @@ var clientset *kubernetes.Clientset
 // If getClient is true, attempts to get a Kubernetes client from config
 func NewClient(fileName, version string, plotAll bool, releases []string, kubeClient bool, dryRun bool, createNamespaces bool, schema []byte, continueOnError bool) (*Client, error) {
 	// Get the course file
-	courseFile, err := course.OpenCourseV2(fileName, schema)
+	courseFile, err := course.OpenCourseFile(fileName, schema)
 	if err != nil {
 		return nil, err
 	}