From eddfc6338ac76d29d7d235cce0abf1c342d7368a Mon Sep 17 00:00:00 2001
From: Kostiantyn Kalynovskyi <kkalynovskyi@mirantis.com>
Date: Mon, 12 Oct 2020 12:00:48 -0500
Subject: [PATCH] Add check for executor ref in phase helper

Relates-To: #365
Closes: #365

Change-Id: I95251a0ef49436d86f6f882bf23a91ceb1b1a952
---
 pkg/api/v1alpha1/phase_types.go                      |  4 +---
 pkg/phase/errors.go                                  | 12 ++++++++++++
 pkg/phase/helper.go                                  |  4 ++++
 pkg/phase/helper_test.go                             | 10 +++++++++-
 .../testdata/valid_site/phases/kustomization.yaml    |  1 +
 .../valid_site/phases/no_executor_phase.yaml         |  7 +++++++
 6 files changed, 34 insertions(+), 4 deletions(-)
 create mode 100644 pkg/phase/testdata/valid_site/phases/no_executor_phase.yaml

diff --git a/pkg/api/v1alpha1/phase_types.go b/pkg/api/v1alpha1/phase_types.go
index 7ed4394cc..9f79373db 100644
--- a/pkg/api/v1alpha1/phase_types.go
+++ b/pkg/api/v1alpha1/phase_types.go
@@ -38,8 +38,6 @@ type PhaseConfig struct {
 // DefaultPhase can be used to safely unmarshal phase object without nil pointers
 func DefaultPhase() *Phase {
 	return &Phase{
-		Config: PhaseConfig{
-			ExecutorRef: &corev1.ObjectReference{},
-		},
+		Config: PhaseConfig{},
 	}
 }
diff --git a/pkg/phase/errors.go b/pkg/phase/errors.go
index ce2573dd0..7a888f14c 100644
--- a/pkg/phase/errors.go
+++ b/pkg/phase/errors.go
@@ -51,3 +51,15 @@ func (e ErrDocumentEntrypointNotDefined) Error() string {
 		e.PhaseName,
 		e.PhaseNamespace)
 }
+
+// ErrExecutorRefNotDefined returned when phase has no entrypoint defined and phase needs it
+type ErrExecutorRefNotDefined struct {
+	PhaseName      string
+	PhaseNamespace string
+}
+
+func (e ErrExecutorRefNotDefined) Error() string {
+	return fmt.Sprintf("Phase name '%s', namespace '%s' must have executorRef field defined in config",
+		e.PhaseName,
+		e.PhaseNamespace)
+}
diff --git a/pkg/phase/helper.go b/pkg/phase/helper.go
index 58cd82148..4296d12f0 100644
--- a/pkg/phase/helper.go
+++ b/pkg/phase/helper.go
@@ -182,6 +182,10 @@ func (helper *Helper) ExecutorDoc(phaseID ifc.ID) (document.Document, error) {
 	}
 	phaseConfig := phaseObj.Config
 
+	if phaseConfig.ExecutorRef == nil {
+		return nil, ErrExecutorRefNotDefined{PhaseName: phaseID.Name, PhaseNamespace: phaseID.Namespace}
+	}
+
 	// Searching executor configuration document referenced in
 	// phase configuration
 	refGVK := phaseConfig.ExecutorRef.GroupVersionKind()
diff --git a/pkg/phase/helper_test.go b/pkg/phase/helper_test.go
index 68026310c..80df8fcc7 100644
--- a/pkg/phase/helper_test.go
+++ b/pkg/phase/helper_test.go
@@ -194,7 +194,7 @@ func TestHelperListPhases(t *testing.T) {
 	}{
 		{
 			name:     "Success phase list",
-			phaseLen: 3,
+			phaseLen: 4,
 			config:   testConfig,
 		},
 		{
@@ -372,6 +372,14 @@ func TestHelperExecutorDoc(t *testing.T) {
 			},
 			errContains: "no such file or directory",
 		},
+		{
+			name:    "Error get phase without executor",
+			config:  testConfig,
+			phaseID: ifc.ID{Name: "no_executor_phase"},
+			errContains: phase.ErrExecutorRefNotDefined{
+				PhaseName: "no_executor_phase",
+			}.Error(),
+		},
 	}
 
 	for _, test := range testCases {
diff --git a/pkg/phase/testdata/valid_site/phases/kustomization.yaml b/pkg/phase/testdata/valid_site/phases/kustomization.yaml
index 977c07f9d..fc7e6fbd1 100644
--- a/pkg/phase/testdata/valid_site/phases/kustomization.yaml
+++ b/pkg/phase/testdata/valid_site/phases/kustomization.yaml
@@ -7,3 +7,4 @@ resources:
   - kubernetes_apply.yaml
   - cluster_map.yaml
   - phase_no_docentrypoint.yaml
+  - no_executor_phase.yaml
diff --git a/pkg/phase/testdata/valid_site/phases/no_executor_phase.yaml b/pkg/phase/testdata/valid_site/phases/no_executor_phase.yaml
new file mode 100644
index 000000000..ecaf14c03
--- /dev/null
+++ b/pkg/phase/testdata/valid_site/phases/no_executor_phase.yaml
@@ -0,0 +1,7 @@
+apiVersion: airshipit.org/v1alpha1
+kind: Phase
+metadata:
+  name: no_executor_phase
+config:
+  documentEntryPoint: valid_site/phases
+