From 6d6ea7ac04cc9810ef58dba094a56c16681396a8 Mon Sep 17 00:00:00 2001
From: Michael Heap <m@michaelheap.com>
Date: Mon, 4 May 2020 13:40:11 +0100
Subject: [PATCH] Implement UnmarshalYAML for ActionRunsUsing (#223)

In #222 I added case normalisation to ReadAction() to ensure that Docker and docker are interpreted the same way.

I realised that this was being done at the wrong level and required multiple type conversions. By implementing `func (a ActionRunsUsing) UnmarshalYAML` we can lowercase the string as it's being unserialized

This has an added benefit that any time this type is hydrated the `runs.using` key will be lowercased, rather than relying on `ReadAction()`
---
 pkg/model/action.go        | 25 +++++++++++++++++++++----
 pkg/runner/step_context.go |  2 +-
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/pkg/model/action.go b/pkg/model/action.go
index e464e7c..a994427 100644
--- a/pkg/model/action.go
+++ b/pkg/model/action.go
@@ -1,6 +1,7 @@
 package model
 
 import (
+	"fmt"
 	"io"
 	"strings"
 
@@ -10,6 +11,26 @@ import (
 // ActionRunsUsing is the type of runner for the action
 type ActionRunsUsing string
 
+func (a *ActionRunsUsing) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	var using string
+	if err := unmarshal(&using); err != nil {
+		return err
+	}
+
+	// Force input to lowercase for case insensitive comparison
+	format := ActionRunsUsing(strings.ToLower(using))
+	switch format {
+	case ActionRunsUsingNode12, ActionRunsUsingDocker:
+		*a = format
+	default:
+		return fmt.Errorf(fmt.Sprintf("The runs.using key in action.yml must be one of: %v, got %s", []string{
+			ActionRunsUsingDocker,
+			ActionRunsUsingNode12,
+		}, format))
+	}
+	return nil
+}
+
 const (
 	// ActionRunsUsingNode12 for running with node12
 	ActionRunsUsingNode12 = "node12"
@@ -54,9 +75,5 @@ type Output struct {
 func ReadAction(in io.Reader) (*Action, error) {
 	a := new(Action)
 	err := yaml.NewDecoder(in).Decode(a)
-
-	// Normalise Runs.Using to lowercase so that Docker and docker are
-	// equivalent when evaluating a step context
-	a.Runs.Using = ActionRunsUsing(strings.ToLower(string(a.Runs.Using)))
 	return a, err
 }
diff --git a/pkg/runner/step_context.go b/pkg/runner/step_context.go
index 0af6411..3da774e 100644
--- a/pkg/runner/step_context.go
+++ b/pkg/runner/step_context.go
@@ -307,7 +307,7 @@ func (sc *StepContext) runAction(actionDir string, actionPath string) common.Exe
 				stepContainer.Remove().IfBool(!rc.Config.ReuseContainers),
 			)(ctx)
 		default:
-			return fmt.Errorf(fmt.Sprintf("The runs.using key in action.yml must be one of: %v, got %s", []string{
+			return fmt.Errorf(fmt.Sprintf("The runs.using key must be one of: %v, got %s", []string{
 				model.ActionRunsUsingDocker,
 				model.ActionRunsUsingNode12,
 			}, action.Runs.Using))