From d78cbe96a1bc17246e7cc8c5cae52033a1812412 Mon Sep 17 00:00:00 2001 From: Kostiantyn Kalynovskyi Date: Mon, 8 Feb 2021 15:43:30 +0000 Subject: [PATCH] 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 | 13 +- pkg/api/v1alpha1/genericcontainer_types.go | 127 +++++--- pkg/api/v1alpha1/zz_generated.deepcopy.go | 85 ++++++ pkg/container/api.go | 133 ++++++++ pkg/container/api_test.go | 134 ++++++++ pkg/phase/executors/container.go | 149 +++------ pkg/phase/executors/container_test.go | 339 +++++++-------------- testutil/container/container.go | 2 + 8 files changed, 593 insertions(+), 389 deletions(-) create mode 100644 pkg/container/api.go create mode 100644 pkg/container/api_test.go diff --git a/manifests/phases/executors.yaml b/manifests/phases/executors.yaml index b50e31dad..3bd2be05f 100644 --- a/manifests/phases/executors.yaml +++ b/manifests/phases/executors.yaml @@ -54,13 +54,13 @@ metadata: name: encrypter labels: airshipit.org/deploy-k8s: "false" -kustomizeSinkOutputDir: "target/generator/results/generated" spec: - container: - image: quay.io/aodinokov/sops:v0.0.3 - envs: - - SOPS_IMPORT_PGP - - SOPS_PGP_FP + type: krm + sinkOutputDir: "target/generator/results/generated" + image: quay.io/aodinokov/sops:v0.0.3 + envVars: + - SOPS_IMPORT_PGP + - SOPS_PGP_FP config: | apiVersion: v1 kind: ConfigMap @@ -183,3 +183,4 @@ spec: operationOptions: remoteDirect: isoURL: REPLACE_ME +--- diff --git a/pkg/api/v1alpha1/genericcontainer_types.go b/pkg/api/v1alpha1/genericcontainer_types.go index b0c5716cb..67fa8d3f5 100644 --- a/pkg/api/v1alpha1/genericcontainer_types.go +++ b/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. - // This path relative to current site root. - KustomizeSinkOutputDir string `json:"kustomizeSinkOutputDir,omitempty"` - // Settings for for a container - Spec runtimeutil.FunctionSpec `json:"spec,omitempty"` + + // Holds container configuration + Spec GenericContainerSpec `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{ - 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) - } + return &GenericContainer{} } diff --git a/pkg/api/v1alpha1/zz_generated.deepcopy.go b/pkg/api/v1alpha1/zz_generated.deepcopy.go index a90d47f4f..8b62d2691 100644 --- a/pkg/api/v1alpha1/zz_generated.deepcopy.go +++ b/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 { diff --git a/pkg/container/api.go b/pkg/container/api.go new file mode 100644 index 000000000..6a24e114e --- /dev/null +++ b/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 +} diff --git a/pkg/container/api_test.go b/pkg/container/api_test.go new file mode 100644 index 000000000..de171b292 --- /dev/null +++ b/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) +} diff --git a/pkg/phase/executors/container.go b/pkg/phase/executors/container.go index 07219d554..0f3c6cbb5 100644 --- a/pkg/phase/executors/container.go +++ b/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 - ExecutorBundle document.Bundle - ExecutorDocument document.Document + ResultsDir string - ContConf *v1alpha1.GenericContainer - RunFns runfn.RunFns - TargetPath string + Container *v1alpha1.GenericContainer + ClientFunc container.ClientV1Alpha1FactoryFunc + 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{ - Spec: runtimeutil.FunctionSpec{}, - } + apiObj := v1alpha1.DefaultGenericContainer() err = cfg.ExecutorDocument.ToAPIObject(apiObj, v1alpha1.Scheme) if err != nil { return nil, err } - return &ContainerExecutor{ - PhaseEntryPointBasePath: cfg.Helper.PhaseEntryPointBasePath(), - ExecutorBundle: bundle, - ExecutorDocument: cfg.ExecutorDocument, + var resultsDir string + if apiObj.Spec.SinkOutputDir != "" { + resultsDir = filepath.Join(cfg.Helper.PhaseEntryPointBasePath(), apiObj.Spec.SinkOutputDir) + } - ContConf: apiObj, - RunFns: runfn.RunFns{ - Functions: []*kyaml.RNode{}, - }, - TargetPath: cfg.Helper.TargetPath(), + return &ContainerExecutor{ + ResultsDir: resultsDir, + ExecutorBundle: bundle, + ExecutorDocument: cfg.ExecutorDocument, + // 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 -func (c *ContainerExecutor) SetInput() error { +// bundleReader sets input for function +func bundleReader(bundle document.Bundle) (io.Reader, error) { buf := &bytes.Buffer{} - err := c.ExecutorBundle.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 + return buf, bundle.Write(buf) } // Validate executor configuration and documents diff --git a/pkg/phase/executors/container_test.go b/pkg/phase/executors/container_test.go index ce06d7df5..087435a1d 100644 --- a/pkg/phase/executors/container_test.go +++ b/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 - object: - executables: - - name: test - 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` - //nolint: lll - transformedFunction = `apiVersion: airshipit.org/v1alpha1 -kind: GenericContainerValues -object: - executables: - - name: test - 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" -` + kind: GenericContainer + metadata: + name: builder + labels: + airshipit.org/deploy-k8s: "false" + spec: + sinkOutputDir: "target/generator/results/generated" + type: krm + image: builder1 + mounts: + - type: bind + src: /home/ubuntu/mounts + dst: /my-mounts + rw: true + config: | + apiVersion: v1 + kind: ConfigMap + metadata: + name: my-srange-name + data: + cmd: encrypt + unencrypted-regex: '^(kind|apiVersion|group|metadata)$'` 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, - BundleFactory: testBundleFactory(singleExecutorBundlePath), - Helper: makeDefaultHelper(t, "../../container/testdata"), + + t.Run("success new container executor", func(t *testing.T) { + e, err := executors.NewContainerExecutor(ifc.ExecutorConfig{ + 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) { - bundle, err := document.NewBundleByPath(singleExecutorBundlePath) - 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 { +func TestGenericContainer(t *testing.T) { + tests := []struct { name string - targetPath string - in []runtimeutil.StorageMount - expectedOut []runtimeutil.StorageMount + outputPath string + expectedErr string + + containerAPI *v1alpha1.GenericContainer + executorConfig ifc.ExecutorConfig + runOptions ifc.RunOptions + clientFunc container.ClientV1Alpha1FactoryFunc }{ { - name: "Empty TargetPath and mounts", - targetPath: "", - in: nil, - expectedOut: nil, + name: "error unknown container type", + expectedErr: "uknown generic container type", + containerAPI: &v1alpha1.GenericContainer{ + Spec: v1alpha1.GenericContainerSpec{ + Type: "unknown", + }, + }, + clientFunc: container.NewClientV1Alpha1, }, { - name: "Empty TargetPath with Src and DstPath", - targetPath: "", - in: []runtimeutil.StorageMount{ - { - MountType: "bind", - Src: "src", - DstPath: "dst", - }, - }, - expectedOut: []runtimeutil.StorageMount{ - { - MountType: "bind", - Src: "src", - DstPath: "dst", + name: "error kyaml cant parse config", + containerAPI: &v1alpha1.GenericContainer{ + Spec: v1alpha1.GenericContainerSpec{ + Type: v1alpha1.GenericContainerTypeKrm, }, + Config: "~:~", }, + runOptions: ifc.RunOptions{}, + expectedErr: "wrong Node Kind", + clientFunc: container.NewClientV1Alpha1, }, { - name: "Not empty TargetPath with Src and DstPath", - targetPath: "target_path", - in: []runtimeutil.StorageMount{ - { - MountType: "bind", - Src: "src", - DstPath: "dst", - }, - }, - expectedOut: []runtimeutil.StorageMount{ - { - MountType: "bind", - Src: "target_path/src", - DstPath: "dst", - }, - }, + name: "success dry run", + containerAPI: &v1alpha1.GenericContainer{}, + runOptions: ifc.RunOptions{DryRun: true}, }, } - for _, test := range testCases { - tt := test + for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { - c := executors.ContainerExecutor{ - ContConf: &v1alpha1.GenericContainer{ - Spec: runtimeutil.FunctionSpec{ - Container: runtimeutil.ContainerSpec{ - StorageMounts: tt.in, - }, - }, - }, - TargetPath: tt.targetPath, + b, err := document.NewBundleByPath(singleExecutorBundlePath) + require.NoError(t, err) + container := executors.ContainerExecutor{ + ResultsDir: tt.outputPath, + ExecutorBundle: b, + Container: tt.containerAPI, + ClientFunc: tt.clientFunc, + } + + 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) }) } } diff --git a/testutil/container/container.go b/testutil/container/container.go index 06ade22b4..8ae3b7416 100755 --- a/testutil/container/container.go +++ b/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()