Extend Generic Container interface

This also moves KRM related logic from executors package to
container package, and creates ClientV1Alpha1 interface that
would allow us to have versioned clients for generic container
executor.

Change-Id: I4b32fd8dd089b9ccea2ed64a805702e6a8705706

manifests/phases/executors.yaml

@@ -54,13 +54,13 @@ metadata:
   name: encrypter
   labels:
     airshipit.org/deploy-k8s: "false"
-kustomizeSinkOutputDir: "target/generator/results/generated"
 spec:
-  container:
+  type: krm
-    image: quay.io/aodinokov/sops:v0.0.3
+  sinkOutputDir: "target/generator/results/generated"
-    envs:
+  image: quay.io/aodinokov/sops:v0.0.3
-    - SOPS_IMPORT_PGP
+  envVars:
-    - SOPS_PGP_FP
+  - SOPS_IMPORT_PGP
+  - SOPS_PGP_FP
 config: |
   apiVersion: v1
   kind: ConfigMap
@@ -183,3 +183,4 @@ spec:
   operationOptions:
     remoteDirect:
       isoURL: REPLACE_ME
+---

pkg/api/v1alpha1/genericcontainer_types.go

@@ -16,7 +16,16 @@ package v1alpha1
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil"
+)
+
+const (
+	// GenericContainerAirshipDockerDriver is the driver name supported by airship container interface
+	// we dont use strong type here for now, to avoid converting to string in the implementation
+	GenericContainerAirshipDockerDriver = "docker"
+	// GenericContainerTypeAirship specifies that airship type container will be used
+	GenericContainerTypeAirship GenericContainerType = "airship"
+	// GenericContainerTypeKrm specifies that kustomize krm function will be used
+	GenericContainerTypeKrm GenericContainerType = "krm"
 )
 
 // +kubebuilder:object:root=true
@@ -25,48 +34,88 @@ import (
 type GenericContainer struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
-	// Executor will write output using kustomize sink if this parameter is specified.
+
-	// Else it will write output to STDOUT.
+	// Holds container configuration
-	// This path relative to current site root.
+	Spec GenericContainerSpec `json:"spec,omitempty"`
-	KustomizeSinkOutputDir string `json:"kustomizeSinkOutputDir,omitempty"`
+
-	// Settings for for a container
-	Spec runtimeutil.FunctionSpec `json:"spec,omitempty"`
 	// Config for the RunFns function in a custom format
 	Config string `json:"config,omitempty"`
 }
 
+// GenericContainerType specify type of the container, there are currently two types:
+// airship - airship will run the container
+// krm - kustomize krm function will run the container
+type GenericContainerType string
+
+// GenericContainerSpec container configuration
+type GenericContainerSpec struct {
+	// Supported types are "airship" and "krm"
+	Type GenericContainerType `json:"type,omitempty"`
+
+	// Ariship container spec
+	Airship AirshipContainerSpec `json:"airship,omitempty"`
+
+	// KRM container function spec
+	KRM KRMContainerSpec `json:"krm,omitempty"`
+
+	// Executor will write output using kustomize sink if this parameter is specified.
+	// Else it will write output to STDOUT.
+	// This path relative to current site root.
+	SinkOutputDir string `json:"sinkOutputDir,omitempty"`
+
+	// HostNetwork defines network specific configuration
+	HostNetwork bool `json:"hostNetwork,omitempty" yaml:"network,omitempty"`
+
+	// Image is the container image to run
+	Image string `json:"image,omitempty" yaml:"image,omitempty"`
+
+	// EnvVars is a slice of env string that will be exposed to container
+	// ["MY_VAR=my-value, "MY_VAR1=my-value1"]
+	// if passed in format ["MY_ENV"] this env variable will be exported the container
+	EnvVars []string `json:"envVars,omitempty"`
+
+	// Mounts are the storage or directories to mount into the container
+	StorageMounts []StorageMount `json:"mounts,omitempty" yaml:"mounts,omitempty"`
+}
+
+// AirshipContainerSpec airship container settings
+type AirshipContainerSpec struct {
+
+	// ContainerRuntime currently supported and default runtime is "docker"
+	ContainerRuntime string `json:"containerRuntime,omitempty"`
+
+	// Cmd to run inside the container, `["/my-command", "arg"]`
+	Cmd []string `json:"cmd,omitempty"`
+
+	// Privileged identifies if the container is to be run in a Privileged mode
+	Privileged bool `json:"pivileged,omitempty"`
+}
+
+// KRMContainerSpec defines a spec for running a function as a container
+// empty for now since it has no extra fields from AirshipContainerSpec
+type KRMContainerSpec struct{}
+
+// StorageMount represents a container's mounted storage option(s)
+// copy from https://github.com/kubernetes-sigs/kustomize to avoid imports in this package
+type StorageMount struct {
+	// Type of mount e.g. bind mount, local volume, etc.
+	MountType string `json:"type,omitempty" yaml:"type,omitempty"`
+
+	// Source for the storage to be mounted.
+	// For named volumes, this is the name of the volume.
+	// For anonymous volumes, this field is omitted (empty string).
+	// For bind mounts, this is the path to the file or directory on the host.
+	Src string `json:"src,omitempty" yaml:"src,omitempty"`
+
+	// The path where the file or directory is mounted in the container.
+	DstPath string `json:"dst,omitempty" yaml:"dst,omitempty"`
+
+	// Mount in ReadWrite mode if it's explicitly configured
+	// See https://docs.docker.com/storage/bind-mounts/#use-a-read-only-bind-mount
+	ReadWriteMode bool `json:"rw,omitempty" yaml:"rw,omitempty"`
+}
+
 // DefaultGenericContainer can be used to safely unmarshal GenericContainer object without nil pointers
 func DefaultGenericContainer() *GenericContainer {
-	return &GenericContainer{
+	return &GenericContainer{}
-		Spec: runtimeutil.FunctionSpec{},
-	}
-}
-
-// DeepCopyInto is copying the receiver, writing into out. in must be non-nil.
-func (in *GenericContainer) DeepCopyInto(out *GenericContainer) {
-	*out = *in
-	out.TypeMeta = in.TypeMeta
-	in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
-
-	out.Spec = in.Spec
-	out.Spec.Container = in.Spec.Container
-	out.Spec.Container.Network = in.Spec.Container.Network
-	if in.Spec.Container.StorageMounts != nil {
-		in, out := &in.Spec.Container.StorageMounts, &out.Spec.Container.StorageMounts
-		*out = make([]runtimeutil.StorageMount, len(*in))
-		copy(*out, *in)
-	}
-	if in.Spec.Container.Env != nil {
-		in, out := &in.Spec.Container.Env, &out.Spec.Container.Env
-		*out = make([]string, len(*in))
-		copy(*out, *in)
-	}
-
-	out.Spec.Starlark = in.Spec.Starlark
-	out.Spec.Exec = in.Spec.Exec
-	if in.Spec.StorageMounts != nil {
-		in, out := &in.Spec.StorageMounts, &out.Spec.StorageMounts
-		*out = make([]runtimeutil.StorageMount, len(*in))
-		copy(*out, *in)
-	}
 }

pkg/api/v1alpha1/zz_generated.deepcopy.go

@@ -23,6 +23,26 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *AirshipContainerSpec) DeepCopyInto(out *AirshipContainerSpec) {
+	*out = *in
+	if in.Cmd != nil {
+		in, out := &in.Cmd, &out.Cmd
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AirshipContainerSpec.
+func (in *AirshipContainerSpec) DeepCopy() *AirshipContainerSpec {
+	if in == nil {
+		return nil
+	}
+	out := new(AirshipContainerSpec)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ApplyConfig) DeepCopyInto(out *ApplyConfig) {
 	*out = *in
@@ -332,6 +352,14 @@ func (in *EphemeralCluster) DeepCopy() *EphemeralCluster {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *GenericContainer) DeepCopyInto(out *GenericContainer) {
+	*out = *in
+	out.TypeMeta = in.TypeMeta
+	in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
+	in.Spec.DeepCopyInto(&out.Spec)
+}
+
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GenericContainer.
 func (in *GenericContainer) DeepCopy() *GenericContainer {
 	if in == nil {
@@ -350,6 +378,33 @@ func (in *GenericContainer) DeepCopyObject() runtime.Object {
 	return nil
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *GenericContainerSpec) DeepCopyInto(out *GenericContainerSpec) {
+	*out = *in
+	in.Airship.DeepCopyInto(&out.Airship)
+	out.KRM = in.KRM
+	if in.EnvVars != nil {
+		in, out := &in.EnvVars, &out.EnvVars
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
+	if in.StorageMounts != nil {
+		in, out := &in.StorageMounts, &out.StorageMounts
+		*out = make([]StorageMount, len(*in))
+		copy(*out, *in)
+	}
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GenericContainerSpec.
+func (in *GenericContainerSpec) DeepCopy() *GenericContainerSpec {
+	if in == nil {
+		return nil
+	}
+	out := new(GenericContainerSpec)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ImageMeta) DeepCopyInto(out *ImageMeta) {
 	*out = *in
@@ -467,6 +522,21 @@ func (in *Isogen) DeepCopy() *Isogen {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *KRMContainerSpec) DeepCopyInto(out *KRMContainerSpec) {
+	*out = *in
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KRMContainerSpec.
+func (in *KRMContainerSpec) DeepCopy() *KRMContainerSpec {
+	if in == nil {
+		return nil
+	}
+	out := new(KRMContainerSpec)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *KubeConfig) DeepCopyInto(out *KubeConfig) {
 	*out = *in
@@ -680,6 +750,21 @@ func (in *ReplacementTransformer) DeepCopyObject() runtime.Object {
 	return nil
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *StorageMount) DeepCopyInto(out *StorageMount) {
+	*out = *in
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StorageMount.
+func (in *StorageMount) DeepCopy() *StorageMount {
+	if in == nil {
+		return nil
+	}
+	out := new(StorageMount)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Templater.
 func (in *Templater) DeepCopy() *Templater {
 	if in == nil {

pkg/container/api.go

@@ -0,0 +1,133 @@
+/*
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+     https://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package container
+
+import (
+	"context"
+	"fmt"
+	"io"
+
+	"sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil"
+	"sigs.k8s.io/kustomize/kyaml/runfn"
+	kyaml "sigs.k8s.io/kustomize/kyaml/yaml"
+	"sigs.k8s.io/yaml"
+
+	"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
+
+	"opendev.org/airship/airshipctl/pkg/errors"
+)
+
+// ClientV1Alpha1 provides airship generic container API
+// TODO add generic mock for this client
+type ClientV1Alpha1 interface {
+	Run() error
+}
+
+// ClientV1Alpha1FactoryFunc used for tests
+type ClientV1Alpha1FactoryFunc func(
+	resultsDir string,
+	input io.Reader,
+	output io.Writer,
+	conf *v1alpha1.GenericContainer) ClientV1Alpha1
+
+type clientV1Alpha1 struct {
+	resultsDir string
+	input      io.Reader
+	output     io.Writer
+	conf       *v1alpha1.GenericContainer
+
+	containerFunc containerFunc
+}
+
+type containerFunc func(ctx context.Context, driver string, url string) (Container, error)
+
+// NewClientV1Alpha1 constructor for ClientV1Alpha1
+func NewClientV1Alpha1(
+	resultsDir string,
+	input io.Reader,
+	output io.Writer,
+	conf *v1alpha1.GenericContainer) ClientV1Alpha1 {
+	return &clientV1Alpha1{
+		resultsDir:    resultsDir,
+		output:        output,
+		input:         input,
+		conf:          conf,
+		containerFunc: NewContainer,
+	}
+}
+
+// Run will peform container run action based on the configuration
+func (c *clientV1Alpha1) Run() error {
+	// set default runtime
+	switch c.conf.Spec.Type {
+	case v1alpha1.GenericContainerTypeAirship, "":
+		return errors.ErrNotImplemented{What: "airship generic container type"}
+	case v1alpha1.GenericContainerTypeKrm:
+		return c.runKRM()
+	default:
+		return fmt.Errorf("uknown generic container type %s", c.conf.Spec.Type)
+	}
+}
+
+func (c *clientV1Alpha1) runKRM() error {
+	mounts := convertKRMMount(c.conf.Spec.StorageMounts)
+	fns := &runfn.RunFns{
+		Network:               c.conf.Spec.HostNetwork,
+		AsCurrentUser:         true,
+		Path:                  c.resultsDir,
+		Input:                 c.input,
+		Output:                c.output,
+		StorageMounts:         mounts,
+		ContinueOnEmptyResult: true,
+	}
+	function, err := kyaml.Parse(c.conf.Config)
+	if err != nil {
+		return err
+	}
+	// Transform GenericContainer.Spec to annotation,
+	// because we need to specify runFns config in annotation
+	spec, err := yaml.Marshal(runtimeutil.FunctionSpec{
+		Container: runtimeutil.ContainerSpec{
+			Image:         c.conf.Spec.Image,
+			Network:       c.conf.Spec.HostNetwork,
+			Env:           c.conf.Spec.EnvVars,
+			StorageMounts: mounts,
+		},
+	})
+	if err != nil {
+		return err
+	}
+	annotation := kyaml.SetAnnotation(runtimeutil.FunctionAnnotationKey, string(spec))
+	_, err = annotation.Filter(function)
+	if err != nil {
+		return err
+	}
+
+	fns.Functions = []*kyaml.RNode{function}
+
+	return fns.Execute()
+}
+
+func convertKRMMount(airMounts []v1alpha1.StorageMount) (fnsMounts []runtimeutil.StorageMount) {
+	for _, mount := range airMounts {
+		fnsMounts = append(fnsMounts, runtimeutil.StorageMount{
+			MountType:     mount.MountType,
+			Src:           mount.Src,
+			DstPath:       mount.DstPath,
+			ReadWriteMode: mount.ReadWriteMode,
+		})
+	}
+	return fnsMounts
+}

pkg/container/api_test.go

@@ -0,0 +1,134 @@
+/*
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+     https://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package container
+
+import (
+	"bytes"
+	"io"
+	"io/ioutil"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
+	"opendev.org/airship/airshipctl/pkg/document"
+	"opendev.org/airship/airshipctl/pkg/phase/ifc"
+)
+
+func bundlePathToInput(t *testing.T, bundlePath string) io.Reader {
+	t.Helper()
+	bundle, err := document.NewBundleByPath(bundlePath)
+	require.NoError(t, err)
+	buf := bytes.NewBuffer([]byte{})
+	err = bundle.Write(buf)
+	require.NoError(t, err)
+	return buf
+}
+
+func TestGenericContainer(t *testing.T) {
+	// TODO add testcase were we mock KRM call, and make sure we put correct input into it
+	tests := []struct {
+		name            string
+		inputBundlePath string
+		outputPath      string
+		expectedErr     string
+
+		output         io.Writer
+		containerAPI   *v1alpha1.GenericContainer
+		execFunc       containerFunc
+		executorConfig ifc.ExecutorConfig
+	}{
+		{
+			name:        "error unknown container type",
+			expectedErr: "uknown generic container type",
+			containerAPI: &v1alpha1.GenericContainer{
+				Spec: v1alpha1.GenericContainerSpec{
+					Type: "unknown",
+				},
+			},
+			execFunc: NewContainer,
+		},
+		{
+			name: "error kyaml cant parse config",
+			containerAPI: &v1alpha1.GenericContainer{
+				Spec: v1alpha1.GenericContainerSpec{
+					Type: v1alpha1.GenericContainerTypeKrm,
+				},
+				Config: "~:~",
+			},
+			execFunc:    NewContainer,
+			expectedErr: "wrong Node Kind",
+		},
+		{
+			name: "error runFns execute",
+			containerAPI: &v1alpha1.GenericContainer{
+				Spec: v1alpha1.GenericContainerSpec{
+					Type: v1alpha1.GenericContainerTypeKrm,
+					StorageMounts: []v1alpha1.StorageMount{
+						{
+							MountType: "bind",
+							Src:       "test",
+							DstPath:   "/mount",
+						},
+					},
+				},
+				Config: `kind: ConfigMap`,
+			},
+			execFunc:    NewContainer,
+			expectedErr: "no such file or directory",
+			outputPath:  "directory doesn't exist",
+		},
+		{
+			name: "basic success airship success written to provided output Writer",
+			containerAPI: &v1alpha1.GenericContainer{
+				Spec: v1alpha1.GenericContainerSpec{
+					Type: v1alpha1.GenericContainerTypeAirship,
+				},
+			},
+			output:      ioutil.Discard,
+			expectedErr: "airship generic container type",
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			input := bundlePathToInput(t, "testdata/single")
+			client := &clientV1Alpha1{
+				input:         input,
+				resultsDir:    tt.outputPath,
+				output:        tt.output,
+				conf:          tt.containerAPI,
+				containerFunc: tt.execFunc,
+			}
+
+			err := client.Run()
+
+			if tt.expectedErr != "" {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tt.expectedErr)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+// Dummy test to keep up with coverage.
+func TestNewClientV1alpha1(t *testing.T) {
+	client := NewClientV1Alpha1("", nil, nil, v1alpha1.DefaultGenericContainer())
+	require.NotNil(t, client)
+}

pkg/phase/executors/container.go

@@ -17,17 +17,11 @@ package executors
 import (
 	"bytes"
 	"io"
-	"log"
 	"os"
 	"path/filepath"
 
-	"sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil"
-	"sigs.k8s.io/kustomize/kyaml/kio"
-	"sigs.k8s.io/kustomize/kyaml/runfn"
-	kyaml "sigs.k8s.io/kustomize/kyaml/yaml"
-	"sigs.k8s.io/yaml"
-
 	"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
+	"opendev.org/airship/airshipctl/pkg/container"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/errors"
 	"opendev.org/airship/airshipctl/pkg/events"
@@ -38,40 +32,42 @@ var _ ifc.Executor = &ContainerExecutor{}
 
 // ContainerExecutor contains resources for generic container executor
 type ContainerExecutor struct {
-	PhaseEntryPointBasePath string
+	ResultsDir string
-	ExecutorBundle          document.Bundle
-	ExecutorDocument        document.Document
 
-	ContConf   *v1alpha1.GenericContainer
+	Container        *v1alpha1.GenericContainer
-	RunFns     runfn.RunFns
+	ClientFunc       container.ClientV1Alpha1FactoryFunc
-	TargetPath string
+	ExecutorBundle   document.Bundle
+	ExecutorDocument document.Document
 }
 
 // NewContainerExecutor creates instance of phase executor
 func NewContainerExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) {
+	// TODO add logic that checks if the path was not defined, and if so, we are fine
+	// and bundle should be either nil or empty, consider ContinueOnEmptyInput option to container client
 	bundle, err := cfg.BundleFactory()
 	if err != nil {
 		return nil, err
 	}
 
-	apiObj := &v1alpha1.GenericContainer{
+	apiObj := v1alpha1.DefaultGenericContainer()
-		Spec: runtimeutil.FunctionSpec{},
-	}
 	err = cfg.ExecutorDocument.ToAPIObject(apiObj, v1alpha1.Scheme)
 	if err != nil {
 		return nil, err
 	}
 
-	return &ContainerExecutor{
+	var resultsDir string
-		PhaseEntryPointBasePath: cfg.Helper.PhaseEntryPointBasePath(),
+	if apiObj.Spec.SinkOutputDir != "" {
-		ExecutorBundle:          bundle,
+		resultsDir = filepath.Join(cfg.Helper.PhaseEntryPointBasePath(), apiObj.Spec.SinkOutputDir)
-		ExecutorDocument:        cfg.ExecutorDocument,
+	}
 
-		ContConf: apiObj,
+	return &ContainerExecutor{
-		RunFns: runfn.RunFns{
+		ResultsDir:       resultsDir,
-			Functions: []*kyaml.RNode{},
+		ExecutorBundle:   bundle,
-		},
+		ExecutorDocument: cfg.ExecutorDocument,
-		TargetPath: cfg.Helper.TargetPath(),
+		// TODO extend tests with proper client, make it interface
+		ClientFunc: container.NewClientV1Alpha1,
+
+		Container: apiObj,
 	}, nil
 }
 
@@ -84,8 +80,22 @@ func (c *ContainerExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) {
 		Message:   "starting generic container",
 	})
 
+	input, err := bundleReader(c.ExecutorBundle)
+	if err != nil {
+		// TODO move bundleFactory here, and make sure that if executorDoc is not defined, we dont fail
+		handleError(evtCh, err)
+		return
+	}
+
+	// TODO this logic is redundant in executor package, move it to pkg/container
+	var output io.Writer
+	if c.ResultsDir == "" {
+		// set output only if the output if resulting directory is not defined
+		output = os.Stdout
+	}
+
+	// TODO check the executor type  when dryrun is set
 	if opts.DryRun {
-		log.Print("generic container will be executed")
 		evtCh <- events.NewEvent().WithGenericContainerEvent(events.GenericContainerEvent{
 			Operation: events.GenericContainerStop,
 			Message:   "DryRun execution finished",
@@ -93,99 +103,22 @@ func (c *ContainerExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) {
 		return
 	}
 
-	if err := c.SetInput(); err != nil {
+	err = c.ClientFunc(c.ResultsDir, input, output, c.Container).Run()
+	if err != nil {
 		handleError(evtCh, err)
 		return
 	}
 
-	if err := c.PrepareFunctions(); err != nil {
-		handleError(evtCh, err)
-		return
-	}
-
-	c.SetMounts()
-
-	var fnsOutputBuffer bytes.Buffer
-
-	if c.ContConf.KustomizeSinkOutputDir != "" {
-		c.RunFns.Output = &fnsOutputBuffer
-	} else {
-		c.RunFns.Output = os.Stdout
-	}
-
-	if err := c.RunFns.Execute(); err != nil {
-		handleError(evtCh, err)
-		return
-	}
-
-	if c.ContConf.KustomizeSinkOutputDir != "" {
-		if err := c.WriteKustomizeSink(&fnsOutputBuffer); err != nil {
-			handleError(evtCh, err)
-			return
-		}
-	}
-
 	evtCh <- events.NewEvent().WithGenericContainerEvent(events.GenericContainerEvent{
 		Operation: events.GenericContainerStop,
 		Message:   "execution of the generic container finished",
 	})
 }
 
-// SetInput sets input for function
+// bundleReader sets input for function
-func (c *ContainerExecutor) SetInput() error {
+func bundleReader(bundle document.Bundle) (io.Reader, error) {
 	buf := &bytes.Buffer{}
-	err := c.ExecutorBundle.Write(buf)
+	return buf, bundle.Write(buf)
-	if err != nil {
-		return err
-	}
-
-	c.RunFns.Input = buf
-	return nil
-}
-
-// PrepareFunctions prepares data for function
-func (c *ContainerExecutor) PrepareFunctions() error {
-	rnode, err := kyaml.Parse(c.ContConf.Config)
-	if err != nil {
-		return err
-	}
-	// Transform GenericContainer.Spec to annotation,
-	// because we need to specify runFns config in annotation
-	spec, err := yaml.Marshal(c.ContConf.Spec)
-	if err != nil {
-		return err
-	}
-	annotation := kyaml.SetAnnotation(runtimeutil.FunctionAnnotationKey, string(spec))
-	_, err = annotation.Filter(rnode)
-	if err != nil {
-		return err
-	}
-
-	c.RunFns.Functions = append(c.RunFns.Functions, rnode)
-
-	return nil
-}
-
-// SetMounts allows to set relative path for storage mounts to prevent security issues
-func (c *ContainerExecutor) SetMounts() {
-	if len(c.ContConf.Spec.Container.StorageMounts) == 0 {
-		return
-	}
-	storageMounts := c.ContConf.Spec.Container.StorageMounts
-	for i, mount := range storageMounts {
-		storageMounts[i].Src = filepath.Join(c.TargetPath, mount.Src)
-	}
-	c.RunFns.StorageMounts = storageMounts
-}
-
-// WriteKustomizeSink writes output to kustomize sink
-func (c *ContainerExecutor) WriteKustomizeSink(fnsOutputBuffer *bytes.Buffer) error {
-	outputDirPath := filepath.Join(c.PhaseEntryPointBasePath, c.ContConf.KustomizeSinkOutputDir)
-	sinkOutputs := []kio.Writer{&kio.LocalPackageWriter{PackagePath: outputDirPath}}
-	err := kio.Pipeline{
-		Inputs:  []kio.Reader{&kio.ByteReader{Reader: fnsOutputBuffer}},
-		Outputs: sinkOutputs}.Execute()
-	return err
 }
 
 // Validate executor configuration and documents

pkg/phase/executors/container_test.go

@@ -2,9 +2,7 @@
  Licensed under the Apache License, Version 2.0 (the "License");
  you may not use this file except in compliance with the License.
  You may obtain a copy of the License at
-
      https://www.apache.org/licenses/LICENSE-2.0
-
  Unless required by applicable law or agreed to in writing, software
  distributed under the License is distributed on an "AS IS" BASIS,
  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -15,277 +13,146 @@
 package executors_test
 
 import (
-	"bytes"
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil"
-	"sigs.k8s.io/kustomize/kyaml/runfn"
-	kyaml "sigs.k8s.io/kustomize/kyaml/yaml"
 
 	"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
+	"opendev.org/airship/airshipctl/pkg/container"
 	"opendev.org/airship/airshipctl/pkg/document"
+	"opendev.org/airship/airshipctl/pkg/events"
 	"opendev.org/airship/airshipctl/pkg/phase/executors"
 	"opendev.org/airship/airshipctl/pkg/phase/ifc"
-	yaml_util "opendev.org/airship/airshipctl/pkg/util/yaml"
 )
 
 const (
 	containerExecutorDoc = `
-apiVersion: airshipit.org/v1alpha1
-kind: GenericContainer
-metadata:
-  name: generic-container
-  labels:
-    airshipit.org/deploy-k8s: "false"
-spec:
-  container:
-      image: quay.io/test/image:v0.0.1
-config: |
   apiVersion: airshipit.org/v1alpha1
-  kind: GenericContainerValues
+  kind: GenericContainer
-  object:
+  metadata:
-    executables:
+    name: builder
-    - name: test
+    labels:
-      cmdline: /tmp/x/script.sh
+      airshipit.org/deploy-k8s: "false"
-      env:
+  spec:
-      - name: var
+    sinkOutputDir: "target/generator/results/generated"
-        value: testval
+    type: krm
-      volumeMounts:
+    image: builder1
-      - name: default
+    mounts:
-        mountPath: /tmp/x
+    - type: bind
-    volumes:
+      src: /home/ubuntu/mounts
-    - name: default
+      dst: /my-mounts
-      secret:
+      rw: true
-        name: test-script
+  config: |
-        defaultMode: 0777`
+    apiVersion: v1
-	//nolint: lll
+    kind: ConfigMap
-	transformedFunction = `apiVersion: airshipit.org/v1alpha1
+    metadata:
-kind: GenericContainerValues
+      name: my-srange-name
-object:
+    data:
-  executables:
+      cmd: encrypt
-  - name: test
+      unencrypted-regex: '^(kind|apiVersion|group|metadata)$'`
-    cmdline: /tmp/x/script.sh
-    env:
-    - name: var
-      value: testval
-    volumeMounts:
-    - name: default
-      mountPath: /tmp/x
-  volumes:
-  - name: default
-    secret:
-      name: test-script
-      defaultMode: 0777
-metadata:
-  annotations:
-    config.kubernetes.io/function: "container:\n  image: quay.io/test/image:v0.0.1\nexec:
-      {}\nstarlark: {}\n"
-`
 	singleExecutorBundlePath = "../../container/testdata/single"
-	firstDocInput            = `---
-apiVersion: v1
-kind: Secret
-metadata:
-  name: test-script
-stringData:
-  script.sh: |
-    #!/bin/sh
-    echo WORKS! $var >&2
-type: Opaque`
-	manyExecutorBundlePath = "../../container/testdata/many"
-	secondDocInput         = `---
-apiVersion: v1
-kind: Secret
-metadata:
-  labels:
-    airshipit.org/ephemeral-node: "true"
-  name: master-0-bmc-secret
-type: Opaque
-`
 )
 
 func TestNewContainerExecutor(t *testing.T) {
 	execDoc, err := document.NewDocumentFromBytes([]byte(containerExecutorDoc))
 	require.NoError(t, err)
-	_, err = executors.NewContainerExecutor(ifc.ExecutorConfig{
+
-		ExecutorDocument: execDoc,
+	t.Run("success new container executor", func(t *testing.T) {
-		BundleFactory:    testBundleFactory(singleExecutorBundlePath),
+		e, err := executors.NewContainerExecutor(ifc.ExecutorConfig{
-		Helper:           makeDefaultHelper(t, "../../container/testdata"),
+			ExecutorDocument: execDoc,
+			BundleFactory:    testBundleFactory(singleExecutorBundlePath),
+			Helper:           makeDefaultHelper(t, "../../container/testdata"),
+		})
+		assert.NoError(t, err)
+		assert.NotNil(t, e)
+	})
+
+	t.Run("error bundle factory", func(t *testing.T) {
+		e, err := executors.NewContainerExecutor(ifc.ExecutorConfig{
+			ExecutorDocument: execDoc,
+			BundleFactory: func() (document.Bundle, error) {
+				return nil, fmt.Errorf("bundle error")
+			},
+			Helper: makeDefaultHelper(t, "../../container/testdata"),
+		})
+		assert.Error(t, err)
+		assert.Nil(t, e)
 	})
-	require.NoError(t, err)
 }
 
-func TestSetInputSingleDocument(t *testing.T) {
+func TestGenericContainer(t *testing.T) {
-	bundle, err := document.NewBundleByPath(singleExecutorBundlePath)
+	tests := []struct {
-	require.NoError(t, err)
-	execDoc, err := document.NewDocumentFromBytes([]byte(containerExecutorDoc))
-	require.NoError(t, err)
-	e := &executors.ContainerExecutor{
-		ExecutorBundle:   bundle,
-		ExecutorDocument: execDoc,
-
-		ContConf: &v1alpha1.GenericContainer{
-			Spec: runtimeutil.FunctionSpec{},
-		},
-		RunFns: runfn.RunFns{
-			Functions: []*kyaml.RNode{},
-		},
-	}
-	err = e.SetInput()
-	require.NoError(t, err)
-
-	// need to use kustomize here, because
-	// it changes order of lines in document
-	doc, err := document.NewDocumentFromBytes([]byte(firstDocInput))
-	require.NoError(t, err)
-	docBytes, err := doc.AsYAML()
-	require.NoError(t, err)
-	buf := &bytes.Buffer{}
-	buf.Write([]byte(yaml_util.DashYamlSeparator))
-	buf.Write(docBytes)
-	buf.Write([]byte(yaml_util.DotYamlSeparator))
-
-	assert.Equal(t, buf, e.RunFns.Input)
-}
-
-func TestSetInputManyDocuments(t *testing.T) {
-	bundle, err := document.NewBundleByPath(manyExecutorBundlePath)
-	require.NoError(t, err)
-	execDoc, err := document.NewDocumentFromBytes([]byte(containerExecutorDoc))
-	require.NoError(t, err)
-	e := &executors.ContainerExecutor{
-		ExecutorBundle:   bundle,
-		ExecutorDocument: execDoc,
-
-		ContConf: &v1alpha1.GenericContainer{
-			Spec: runtimeutil.FunctionSpec{},
-		},
-		RunFns: runfn.RunFns{
-			Functions: []*kyaml.RNode{},
-		},
-	}
-	err = e.SetInput()
-	require.NoError(t, err)
-
-	// need to use kustomize here, because
-	// it changes order of lines in document
-	docSecond, err := document.NewDocumentFromBytes([]byte(secondDocInput))
-	require.NoError(t, err)
-	docSecondBytes, err := docSecond.AsYAML()
-	require.NoError(t, err)
-
-	buf := &bytes.Buffer{}
-	buf.Write([]byte(yaml_util.DashYamlSeparator))
-	buf.Write(docSecondBytes)
-	buf.Write([]byte(yaml_util.DotYamlSeparator))
-
-	docFirst, err := document.NewDocumentFromBytes([]byte(firstDocInput))
-	require.NoError(t, err)
-	docFirstBytes, err := docFirst.AsYAML()
-	require.NoError(t, err)
-	buf.Write([]byte(yaml_util.DashYamlSeparator))
-	buf.Write(docFirstBytes)
-	buf.Write([]byte(yaml_util.DotYamlSeparator))
-
-	assert.Equal(t, buf, e.RunFns.Input)
-}
-
-func TestPrepareFunctions(t *testing.T) {
-	bundle, err := document.NewBundleByPath(singleExecutorBundlePath)
-	require.NoError(t, err)
-	execDoc, err := document.NewDocumentFromBytes([]byte(containerExecutorDoc))
-	require.NoError(t, err)
-	contConf := &v1alpha1.GenericContainer{
-		Spec: runtimeutil.FunctionSpec{},
-	}
-	err = execDoc.ToAPIObject(contConf, v1alpha1.Scheme)
-	require.NoError(t, err)
-	e := &executors.ContainerExecutor{
-		ExecutorBundle:   bundle,
-		ExecutorDocument: execDoc,
-
-		ContConf: contConf,
-		RunFns: runfn.RunFns{
-			Functions: []*kyaml.RNode{},
-		},
-	}
-
-	err = e.PrepareFunctions()
-	require.NoError(t, err)
-	strFuncs, err := e.RunFns.Functions[0].String()
-	require.NoError(t, err)
-
-	assert.Equal(t, transformedFunction, strFuncs)
-}
-
-func TestSetMounts(t *testing.T) {
-	testCases := []struct {
 		name        string
-		targetPath  string
+		outputPath  string
-		in          []runtimeutil.StorageMount
+		expectedErr string
-		expectedOut []runtimeutil.StorageMount
+
+		containerAPI   *v1alpha1.GenericContainer
+		executorConfig ifc.ExecutorConfig
+		runOptions     ifc.RunOptions
+		clientFunc     container.ClientV1Alpha1FactoryFunc
 	}{
 		{
-			name:        "Empty TargetPath and mounts",
+			name:        "error unknown container type",
-			targetPath:  "",
+			expectedErr: "uknown generic container type",
-			in:          nil,
+			containerAPI: &v1alpha1.GenericContainer{
-			expectedOut: nil,
+				Spec: v1alpha1.GenericContainerSpec{
+					Type: "unknown",
+				},
+			},
+			clientFunc: container.NewClientV1Alpha1,
 		},
 		{
-			name:       "Empty TargetPath with Src and DstPath",
+			name: "error kyaml cant parse config",
-			targetPath: "",
+			containerAPI: &v1alpha1.GenericContainer{
-			in: []runtimeutil.StorageMount{
+				Spec: v1alpha1.GenericContainerSpec{
-				{
+					Type: v1alpha1.GenericContainerTypeKrm,
-					MountType: "bind",
-					Src:       "src",
-					DstPath:   "dst",
-				},
-			},
-			expectedOut: []runtimeutil.StorageMount{
-				{
-					MountType: "bind",
-					Src:       "src",
-					DstPath:   "dst",
 				},
+				Config: "~:~",
 			},
+			runOptions:  ifc.RunOptions{},
+			expectedErr: "wrong Node Kind",
+			clientFunc:  container.NewClientV1Alpha1,
 		},
 		{
-			name:       "Not empty TargetPath with Src and DstPath",
+			name:         "success dry run",
-			targetPath: "target_path",
+			containerAPI: &v1alpha1.GenericContainer{},
-			in: []runtimeutil.StorageMount{
+			runOptions:   ifc.RunOptions{DryRun: true},
-				{
-					MountType: "bind",
-					Src:       "src",
-					DstPath:   "dst",
-				},
-			},
-			expectedOut: []runtimeutil.StorageMount{
-				{
-					MountType: "bind",
-					Src:       "target_path/src",
-					DstPath:   "dst",
-				},
-			},
 		},
 	}
 
-	for _, test := range testCases {
+	for _, tt := range tests {
-		tt := test
+		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			c := executors.ContainerExecutor{
+			b, err := document.NewBundleByPath(singleExecutorBundlePath)
-				ContConf: &v1alpha1.GenericContainer{
+			require.NoError(t, err)
-					Spec: runtimeutil.FunctionSpec{
+			container := executors.ContainerExecutor{
-						Container: runtimeutil.ContainerSpec{
+				ResultsDir:     tt.outputPath,
-							StorageMounts: tt.in,
+				ExecutorBundle: b,
-						},
+				Container:      tt.containerAPI,
-					},
+				ClientFunc:     tt.clientFunc,
-				},
+			}
-				TargetPath: tt.targetPath,
+
+			ch := make(chan events.Event)
+			go container.Run(ch, tt.runOptions)
+
+			var actualEvt []events.Event
+			for evt := range ch {
+				actualEvt = append(actualEvt, evt)
+			}
+			require.Greater(t, len(actualEvt), 0)
+
+			if tt.expectedErr != "" {
+				e := actualEvt[len(actualEvt)-1]
+				require.Error(t, e.ErrorEvent.Error)
+				assert.Contains(t, e.ErrorEvent.Error.Error(), tt.expectedErr)
+			} else {
+				e := actualEvt[len(actualEvt)-1]
+				assert.NoError(t, e.ErrorEvent.Error)
+				assert.Equal(t, e.Type, events.GenericContainerType)
+				assert.Equal(t, e.GenericContainerEvent.Operation, events.GenericContainerStop)
 			}
-			c.SetMounts()
-			assert.Equal(t, c.RunFns.StorageMounts, tt.expectedOut)
 		})
 	}
 }

testutil/container/container.go

@@ -31,6 +31,8 @@ type MockContainer struct {
 	MockInspectContainer  func() (container.State, error)
 }
 
+var _ container.Container = &MockContainer{}
+
 // ImagePull Container interface implementation for unit test purposes
 func (mc *MockContainer) ImagePull() error {
 	return mc.MockImagePull()