Add new selector by ObjectReference

This selector will allow us to improve internal logic in places
where we create selectors by ObjectReference objects and reuse
the code.

Change-Id: I4e4808bfdffc4446e9df255e2ed0b7b8f47d135c
Signed-off-by: Ruslan Aliev <raliev@mirantis.com>

pkg/document/selectors.go

@@ -18,9 +18,9 @@ import (
 	"fmt"
 	"strings"
 
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime"
-
 	"sigs.k8s.io/kustomize/api/types"
 	"sigs.k8s.io/kustomize/kyaml/resid"
 )
@@ -104,6 +104,15 @@ func (s Selector) ByObject(obj runtime.Object, scheme *runtime.Scheme) (Selector
 	return result, nil
 }
 
+// ByObjectReference select by ObjectReference
+func (s Selector) ByObjectReference(objRef *corev1.ObjectReference) Selector {
+	refGVK := objRef.GroupVersionKind()
+	return NewSelector().
+		ByGvk(refGVK.Group, refGVK.Version, refGVK.Kind).
+		ByName(objRef.Name).
+		ByNamespace(objRef.Namespace)
+}
+
 // String is a convenience function which dumps all relevant information about a Selector in the following format:
 // [Key1=Value1, Key2=Value2, ...]
 func (s Selector) String() string {

pkg/document/selectors_test.go

@@ -208,3 +208,41 @@ func TestSelectorToObject(t *testing.T) {
 		})
 	}
 }
+
+func TestSelectorByObjRef(t *testing.T) {
+	tests := []struct {
+		name        string
+		objRef      *k8sv1.ObjectReference
+		expectedSel document.Selector
+	}{
+		{
+			name: "Selector with GVK, name and namespace",
+			objRef: &k8sv1.ObjectReference{
+				Kind:       "TestKind",
+				Name:       "TestName",
+				APIVersion: "api.version/v1",
+				Namespace:  "TestNamespace",
+			},
+			expectedSel: document.Selector{
+				Selector: types.Selector{
+					ResId: resid.ResId{
+						Gvk: resid.Gvk{
+							Group:   "api.version",
+							Version: "v1",
+							Kind:    "TestKind",
+						},
+						Name:      "TestName",
+						Namespace: "TestNamespace",
+					},
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		tt := test
+		t.Run(tt.name, func(t *testing.T) {
+			actualSel := document.NewSelector().ByObjectReference(tt.objRef)
+			assert.Equal(t, tt.expectedSel, actualSel)
+		})
+	}
+}

pkg/phase/client.go

@@ -63,24 +63,10 @@ type phase struct {
 	processor events.EventProcessor
 }
 
-func (p *phase) defaultBundleFactory() document.BundleFactoryFunc {
-	return document.BundleFactoryFromDocRoot(p.DocumentRoot)
-}
-
-func (p *phase) defaultDocFactory() document.DocFactoryFunc {
-	return func() (document.Document, error) {
-		return p.helper.ExecutorDoc(ifc.ID{Name: p.apiObj.Name, Namespace: p.apiObj.Namespace})
-	}
-}
-
 // Executor returns executor interface associated with the phase
 func (p *phase) Executor() (ifc.Executor, error) {
-	return p.executor(p.defaultDocFactory(), p.defaultBundleFactory())
-}
-
-func (p *phase) executor(docFactory document.DocFactoryFunc,
-	bundleFactory document.BundleFactoryFunc) (ifc.Executor, error) {
-	executorDoc, err := docFactory()
+	executorDoc, err := p.helper.PhaseConfigBundle().SelectOne(
+		document.NewSelector().ByObjectReference(p.apiObj.Config.ExecutorRef))
 	if err != nil {
 		return nil, err
 	}
@@ -112,7 +98,7 @@ func (p *phase) executor(docFactory document.DocFactoryFunc,
 	return executorFactory(
 		ifc.ExecutorConfig{
 			ClusterMap:        cMap,
-			BundleFactory:     bundleFactory,
+			BundleFactory:     document.BundleFactoryFromDocRoot(p.DocumentRoot),
 			PhaseName:         p.apiObj.Name,
 			KubeConfig:        kubeconf,
 			ExecutorDocument:  executorDoc,

pkg/phase/client_test.go

@@ -233,13 +233,6 @@ func TestPhaseValidate(t *testing.T) {
 			registryFunc: fakeRegistry,
 			errContains:  "executor identified by 'airshipit.org/v1alpha1, Kind=SomeExecutor' is not found",
 		},
-		{
-			name:         "Error no executor",
-			configFunc:   testConfig,
-			phaseID:      ifc.ID{Name: "no_executor_phase"},
-			registryFunc: fakeRegistry,
-			errContains:  "Phase name 'no_executor_phase', namespace '' must have executorRef field defined in config",
-		},
 		{
 			name:       "Error executor validate",
 			configFunc: testConfig,

pkg/phase/executors/container.go

@@ -182,12 +182,7 @@ func (c *ContainerExecutor) Render(w io.Writer, o ifc.RenderOptions) error {
 func (c *ContainerExecutor) setConfig() error {
 	if c.Container.ConfigRef != nil {
 		log.Debugf("Config reference is specified, looking for the object in config ref: '%v'", c.Container.ConfigRef)
-		gvk := c.Container.ConfigRef.GroupVersionKind()
-		selector := document.NewSelector().
-			ByName(c.Container.ConfigRef.Name).
-			ByNamespace(c.Container.ConfigRef.Namespace).
-			ByGvk(gvk.Group, gvk.Version, gvk.Kind)
-		doc, err := c.Options.PhaseConfigBundle.SelectOne(selector)
+		doc, err := c.Options.PhaseConfigBundle.SelectOne(document.NewSelector().ByObjectReference(c.Container.ConfigRef))
 		if err != nil {
 			return err
 		}

pkg/phase/helper.go

@@ -110,6 +110,13 @@ func (helper *Helper) Phase(phaseID ifc.ID) (*v1alpha1.Phase, error) {
 	if err = doc.ToAPIObject(phase, v1alpha1.Scheme); err != nil {
 		return nil, err
 	}
+	// Phase must contain an executor
+	if phase.Config.ExecutorRef == nil {
+		return nil, errors.ErrExecutorRefNotDefined{
+			PhaseName:      phase.Name,
+			PhaseNamespace: phase.Namespace,
+		}
+	}
 	return phase, nil
 }
 
@@ -271,19 +278,9 @@ func (helper *Helper) ExecutorDoc(phaseID ifc.ID) (document.Document, error) {
 		return nil, err
 	}
 
-	phaseConfig := phaseObj.Config
-	if phaseConfig.ExecutorRef == nil {
-		return nil, errors.ErrExecutorRefNotDefined{PhaseName: phaseID.Name, PhaseNamespace: phaseID.Namespace}
-	}
-
 	// Searching executor configuration document referenced in
 	// phase configuration
-	refGVK := phaseConfig.ExecutorRef.GroupVersionKind()
-	selector := document.NewSelector().
-		ByGvk(refGVK.Group, refGVK.Version, refGVK.Kind).
-		ByName(phaseConfig.ExecutorRef.Name).
-		ByNamespace(phaseConfig.ExecutorRef.Namespace)
-	return helper.phaseConfigBundle.SelectOne(selector)
+	return helper.phaseConfigBundle.SelectOne(document.NewSelector().ByObjectReference(phaseObj.Config.ExecutorRef))
 }
 
 // TargetPath returns manifest root

pkg/phase/helper_test.go

@@ -75,6 +75,12 @@ func TestHelperPhase(t *testing.T) {
 			phaseID:     ifc.ID{Name: "some_name"},
 			errContains: "found no documents",
 		},
+		{
+			name:        "Error no executor",
+			config:      testConfig,
+			phaseID:     ifc.ID{Name: "no_executor_phase"},
+			errContains: "Phase name 'no_executor_phase', namespace '' must have executorRef field defined in config",
+		},
 		{
 			name: "Error bundle path doesn't exist",
 			config: func(t *testing.T) *config.Config {
@@ -95,6 +101,7 @@ func TestHelperPhase(t *testing.T) {
 				require.Error(t, err)
 				return
 			}
+			require.NoError(t, err)
 			require.NotNil(t, helper)
 			actualPhase, actualErr := helper.Phase(tt.phaseID)
 			if tt.errContains != "" {

pkg/phase/testdata/valid_site_with_doc_prefix/phases/sample.yaml

@@ -3,4 +3,8 @@ kind: Phase
 metadata:
   name: sample
 config:
+  executorRef:
+    apiVersion: airshipit.org/v1alpha1
+    kind: SomeExecutor
+    name: executor-name
   documentEntryPoint: entrypoint