From efc4399e1778c0404552a0b8c99e231f41dfb6ac Mon Sep 17 00:00:00 2001 From: Kostiantyn Kalynovskyi Date: Thu, 11 Feb 2021 00:21:47 +0000 Subject: [PATCH] Allow container config to be referenced as objects Now GenericContainer input config can be referenced as another object inside the config bundle (with phase and executor objects). Change-Id: Iff35e0844b1e9ce4beb72d939e229410208dcb16 --- pkg/api/v1alpha1/genericcontainer_types.go | 10 +++- pkg/api/v1alpha1/zz_generated.deepcopy.go | 5 ++ pkg/phase/executors/baremetal_manager_test.go | 10 ++-- pkg/phase/executors/clusterctl_test.go | 8 +-- pkg/phase/executors/common_test.go | 8 ++- pkg/phase/executors/container.go | 37 ++++++++++++- pkg/phase/executors/container_test.go | 53 +++++++++++++++++-- pkg/phase/executors/k8s_applier_test.go | 10 ++-- 8 files changed, 117 insertions(+), 24 deletions(-) diff --git a/pkg/api/v1alpha1/genericcontainer_types.go b/pkg/api/v1alpha1/genericcontainer_types.go index 67fa8d3f5..4008a6cc6 100644 --- a/pkg/api/v1alpha1/genericcontainer_types.go +++ b/pkg/api/v1alpha1/genericcontainer_types.go @@ -15,6 +15,7 @@ package v1alpha1 import ( + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -38,8 +39,15 @@ type GenericContainer struct { // Holds container configuration Spec GenericContainerSpec `json:"spec,omitempty"` - // Config for the RunFns function in a custom format + // Config will be passed to stdin of the container togather with other objects + // more information on easy ways to consume the config can be found here + // https://googlecontainertools.github.io/kpt/guides/producer/functions/golang/ Config string `json:"config,omitempty"` + // Reference is a reference to a configuration object, that must reside in the same + // bundle as this GenericContainer object, if specified, Config string will be + // ignored and referenced object in ConfigRef will be used into the Config string + // instead and passed further into the container stdin + ConfigRef *v1.ObjectReference `json:"configRef,omitempty"` } // GenericContainerType specify type of the container, there are currently two types: diff --git a/pkg/api/v1alpha1/zz_generated.deepcopy.go b/pkg/api/v1alpha1/zz_generated.deepcopy.go index 8b62d2691..744145839 100644 --- a/pkg/api/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/api/v1alpha1/zz_generated.deepcopy.go @@ -358,6 +358,11 @@ func (in *GenericContainer) DeepCopyInto(out *GenericContainer) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) + if in.ConfigRef != nil { + in, out := &in.ConfigRef, &out.ConfigRef + *out = new(v1.ObjectReference) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GenericContainer. diff --git a/pkg/phase/executors/baremetal_manager_test.go b/pkg/phase/executors/baremetal_manager_test.go index 480697dfd..8b59fdcb2 100644 --- a/pkg/phase/executors/baremetal_manager_test.go +++ b/pkg/phase/executors/baremetal_manager_test.go @@ -50,7 +50,7 @@ func TestNewBMHExecutor(t *testing.T) { executor, err := executors.NewBaremetalExecutor(ifc.ExecutorConfig{ ExecutorDocument: execDoc, BundleFactory: testBundleFactory(singleExecutorBundlePath), - Helper: makeDefaultHelper(t, "../testdata"), + Helper: makeDefaultHelper(t, "../testdata", defaultMetadataPath), }) assert.NoError(t, err) assert.NotNil(t, executor) @@ -64,7 +64,7 @@ func TestNewBMHExecutor(t *testing.T) { executor, actualErr := executors.NewBaremetalExecutor(ifc.ExecutorConfig{ ExecutorDocument: execDoc, BundleFactory: testBundleFactory(singleExecutorBundlePath), - Helper: makeDefaultHelper(t, "../testdata"), + Helper: makeDefaultHelper(t, "../testdata", defaultMetadataPath), }) assert.Equal(t, exepectedErr, actualErr) assert.Nil(t, executor) @@ -121,7 +121,7 @@ func TestBMHExecutorRun(t *testing.T) { executor, err := executors.NewBaremetalExecutor(ifc.ExecutorConfig{ ExecutorDocument: tt.execDoc, BundleFactory: testBundleFactory(singleExecutorBundlePath), - Helper: makeDefaultHelper(t, "../testdata/"), + Helper: makeDefaultHelper(t, "../testdata/", defaultMetadataPath), }) require.NoError(t, err) require.NotNil(t, executor) @@ -180,7 +180,7 @@ func TestBMHValidate(t *testing.T) { executor, err := executors.NewBaremetalExecutor(ifc.ExecutorConfig{ ExecutorDocument: tt.execDoc, BundleFactory: testBundleFactory(singleExecutorBundlePath), - Helper: makeDefaultHelper(t, "../testdata/"), + Helper: makeDefaultHelper(t, "../testdata/", defaultMetadataPath), }) require.NoError(t, err) require.NotNil(t, executor) @@ -202,7 +202,7 @@ func TestBMHManagerRender(t *testing.T) { executor, err := executors.NewBaremetalExecutor(ifc.ExecutorConfig{ ExecutorDocument: execDoc, BundleFactory: testBundleFactory(singleExecutorBundlePath), - Helper: makeDefaultHelper(t, "../testdata"), + Helper: makeDefaultHelper(t, "../testdata", defaultMetadataPath), }) require.NoError(t, err) require.NotNil(t, executor) diff --git a/pkg/phase/executors/clusterctl_test.go b/pkg/phase/executors/clusterctl_test.go index 70b136b0a..5c97864eb 100755 --- a/pkg/phase/executors/clusterctl_test.go +++ b/pkg/phase/executors/clusterctl_test.go @@ -77,7 +77,7 @@ func TestNewExecutor(t *testing.T) { }{ { name: "New Clusterctl Executor", - helper: makeDefaultHelper(t, "../../clusterctl/client/testdata"), + helper: makeDefaultHelper(t, "../../clusterctl/client/testdata", defaultMetadataPath), }, } for _, test := range testCases { @@ -165,7 +165,7 @@ func TestExecutorRun(t *testing.T) { executor, err := executors.NewClusterctlExecutor( ifc.ExecutorConfig{ ExecutorDocument: tt.cfgDoc, - Helper: makeDefaultHelper(t, "../../clusterctl/client/testdata"), + Helper: makeDefaultHelper(t, "../../clusterctl/client/testdata", defaultMetadataPath), KubeConfig: kubeCfg, ClusterMap: tt.clusterMap, }) @@ -196,7 +196,7 @@ func TestExecutorValidate(t *testing.T) { executor, err := executors.NewClusterctlExecutor( ifc.ExecutorConfig{ ExecutorDocument: sampleCfgDoc, - Helper: makeDefaultHelper(t, "../../clusterctl/client/testdata"), + Helper: makeDefaultHelper(t, "../../clusterctl/client/testdata", defaultMetadataPath), }) require.NoError(t, err) expectedErr := airerrors.ErrNotImplemented{} @@ -209,7 +209,7 @@ func TestExecutorRender(t *testing.T) { executor, err := executors.NewClusterctlExecutor( ifc.ExecutorConfig{ ExecutorDocument: sampleCfgDoc, - Helper: makeDefaultHelper(t, "../../clusterctl/client/testdata"), + Helper: makeDefaultHelper(t, "../../clusterctl/client/testdata", defaultMetadataPath), }) require.NoError(t, err) actualOut := &bytes.Buffer{} diff --git a/pkg/phase/executors/common_test.go b/pkg/phase/executors/common_test.go index cf7b1f1f2..e1a82e5f2 100755 --- a/pkg/phase/executors/common_test.go +++ b/pkg/phase/executors/common_test.go @@ -29,6 +29,10 @@ import ( "opendev.org/airship/airshipctl/pkg/phase/ifc" ) +const ( + defaultMetadataPath = "metadata.yaml" +) + func TestRegisterExecutor(t *testing.T) { testCases := []struct { name string @@ -100,11 +104,11 @@ func TestRegisterExecutor(t *testing.T) { } } -func makeDefaultHelper(t *testing.T, targetPath string) ifc.Helper { +func makeDefaultHelper(t *testing.T, targetPath, metaPath string) ifc.Helper { t.Helper() cfg := config.NewConfig() cfg.Manifests[config.AirshipDefaultManifest].TargetPath = targetPath - cfg.Manifests[config.AirshipDefaultManifest].MetadataPath = "metadata.yaml" + cfg.Manifests[config.AirshipDefaultManifest].MetadataPath = metaPath cfg.Manifests[config.AirshipDefaultManifest].Repositories[config.DefaultTestPhaseRepo].URLString = "" cfg.SetLoadedConfigPath(".") helper, err := phase.NewHelper(cfg) diff --git a/pkg/phase/executors/container.go b/pkg/phase/executors/container.go index 0f3c6cbb5..73d5ccdff 100644 --- a/pkg/phase/executors/container.go +++ b/pkg/phase/executors/container.go @@ -25,6 +25,7 @@ import ( "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/errors" "opendev.org/airship/airshipctl/pkg/events" + "opendev.org/airship/airshipctl/pkg/log" "opendev.org/airship/airshipctl/pkg/phase/ifc" ) @@ -38,6 +39,7 @@ type ContainerExecutor struct { ClientFunc container.ClientV1Alpha1FactoryFunc ExecutorBundle document.Bundle ExecutorDocument document.Document + Helper ifc.Helper } // NewContainerExecutor creates instance of phase executor @@ -66,8 +68,8 @@ func NewContainerExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) { ExecutorDocument: cfg.ExecutorDocument, // TODO extend tests with proper client, make it interface ClientFunc: container.NewClientV1Alpha1, - - Container: apiObj, + Helper: cfg.Helper, + Container: apiObj, }, nil } @@ -93,6 +95,10 @@ func (c *ContainerExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) { // set output only if the output if resulting directory is not defined output = os.Stdout } + if err = c.setConfig(); err != nil { + handleError(evtCh, err) + return + } // TODO check the executor type when dryrun is set if opts.DryRun { @@ -130,3 +136,30 @@ func (c *ContainerExecutor) Validate() error { func (c *ContainerExecutor) Render(_ io.Writer, _ ifc.RenderOptions) error { return errors.ErrNotImplemented{} } + +func (c *ContainerExecutor) setConfig() error { + if c.Container.ConfigRef != nil { + log.Printf("Config reference is specified, looking for the object in config ref: '%v'", c.Container.ConfigRef) + log.Printf("using bundle root %s", c.Helper.PhaseBundleRoot()) + bundle, err := document.NewBundleByPath(c.Helper.PhaseBundleRoot()) + if err != nil { + return err + } + 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 := bundle.SelectOne(selector) + if err != nil { + return err + } + config, err := doc.AsYAML() + if err != nil { + return err + } + c.Container.Config = string(config) + return nil + } + return nil +} diff --git a/pkg/phase/executors/container_test.go b/pkg/phase/executors/container_test.go index 087435a1d..bb21dbe41 100644 --- a/pkg/phase/executors/container_test.go +++ b/pkg/phase/executors/container_test.go @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/container" @@ -52,6 +53,7 @@ const ( data: cmd: encrypt unencrypted-regex: '^(kind|apiVersion|group|metadata)$'` + singleExecutorBundlePath = "../../container/testdata/single" ) @@ -63,7 +65,7 @@ func TestNewContainerExecutor(t *testing.T) { e, err := executors.NewContainerExecutor(ifc.ExecutorConfig{ ExecutorDocument: execDoc, BundleFactory: testBundleFactory(singleExecutorBundlePath), - Helper: makeDefaultHelper(t, "../../container/testdata"), + Helper: makeDefaultHelper(t, "../../container/testdata", "metadata.yaml"), }) assert.NoError(t, err) assert.NotNil(t, e) @@ -75,7 +77,7 @@ func TestNewContainerExecutor(t *testing.T) { BundleFactory: func() (document.Bundle, error) { return nil, fmt.Errorf("bundle error") }, - Helper: makeDefaultHelper(t, "../../container/testdata"), + Helper: makeDefaultHelper(t, "../../container/testdata", "metadata.yaml"), }) assert.Error(t, err) assert.Nil(t, e) @@ -84,9 +86,11 @@ func TestNewContainerExecutor(t *testing.T) { func TestGenericContainer(t *testing.T) { tests := []struct { - name string - outputPath string - expectedErr string + name string + outputPath string + expectedErr string + targetPath string + resultConfig string containerAPI *v1alpha1.GenericContainer executorConfig ifc.ExecutorConfig @@ -102,6 +106,7 @@ func TestGenericContainer(t *testing.T) { }, }, clientFunc: container.NewClientV1Alpha1, + targetPath: singleExecutorBundlePath, }, { name: "error kyaml cant parse config", @@ -114,11 +119,47 @@ func TestGenericContainer(t *testing.T) { runOptions: ifc.RunOptions{}, expectedErr: "wrong Node Kind", clientFunc: container.NewClientV1Alpha1, + targetPath: singleExecutorBundlePath, + }, + { + name: "error no object referenced in config", + containerAPI: &v1alpha1.GenericContainer{ + ConfigRef: &v1.ObjectReference{ + Kind: "no such kind", + Name: "no such name", + }, + }, + runOptions: ifc.RunOptions{DryRun: true}, + expectedErr: "found no documents", + targetPath: singleExecutorBundlePath, }, { name: "success dry run", containerAPI: &v1alpha1.GenericContainer{}, runOptions: ifc.RunOptions{DryRun: true}, + targetPath: singleExecutorBundlePath, + }, + { + name: "success referenced config present", + containerAPI: &v1alpha1.GenericContainer{ + ConfigRef: &v1.ObjectReference{ + Kind: "Secret", + Name: "test-script", + APIVersion: "v1", + }, + }, + runOptions: ifc.RunOptions{DryRun: true}, + targetPath: singleExecutorBundlePath, + resultConfig: `apiVersion: v1 +kind: Secret +metadata: + name: test-script +stringData: + script.sh: | + #!/bin/sh + echo WORKS! $var >&2 +type: Opaque +`, }, } @@ -132,6 +173,7 @@ func TestGenericContainer(t *testing.T) { ExecutorBundle: b, Container: tt.containerAPI, ClientFunc: tt.clientFunc, + Helper: makeDefaultHelper(t, tt.targetPath, "../metadata.yaml"), } ch := make(chan events.Event) @@ -152,6 +194,7 @@ func TestGenericContainer(t *testing.T) { assert.NoError(t, e.ErrorEvent.Error) assert.Equal(t, e.Type, events.GenericContainerType) assert.Equal(t, e.GenericContainerEvent.Operation, events.GenericContainerStop) + assert.Equal(t, tt.resultConfig, container.Container.Config) } }) } diff --git a/pkg/phase/executors/k8s_applier_test.go b/pkg/phase/executors/k8s_applier_test.go index 2f7a25a3a..614d79ac4 100644 --- a/pkg/phase/executors/k8s_applier_test.go +++ b/pkg/phase/executors/k8s_applier_test.go @@ -94,7 +94,7 @@ func TestNewKubeApplierExecutor(t *testing.T) { name: "valid executor", cfgDoc: ValidExecutorDoc, kubeconf: testKubeconfig(testValidKubeconfig), - helper: makeDefaultHelper(t, "../../k8s/applier/testdata"), + helper: makeDefaultHelper(t, "../../k8s/applier/testdata", defaultMetadataPath), bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"), }, { @@ -107,7 +107,7 @@ metadata: labels: cli-utils.sigs.k8s.io/inventory-id: "some id"`, expectedErr: "wrong config document", - helper: makeDefaultHelper(t, "../../k8s/applier/testdata"), + helper: makeDefaultHelper(t, "../../k8s/applier/testdata", defaultMetadataPath), bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"), }, @@ -116,7 +116,7 @@ metadata: cfgDoc: ValidExecutorDoc, expectedErr: "no such file or directory", kubeconf: testKubeconfig(testValidKubeconfig), - helper: makeDefaultHelper(t, "../../k8s/applier/testdata"), + helper: makeDefaultHelper(t, "../../k8s/applier/testdata", defaultMetadataPath), bundleFactory: testBundleFactory("does not exist"), }, } @@ -165,7 +165,7 @@ func TestKubeApplierExecutorRun(t *testing.T) { { name: "cant read kubeconfig error", containsErr: "no such file or directory", - helper: makeDefaultHelper(t, "../../k8s/applier/testdata"), + helper: makeDefaultHelper(t, "../../k8s/applier/testdata", defaultMetadataPath), bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"), kubeconf: testKubeconfig(`invalid kubeconfig`), execDoc: executorDoc(t, ValidExecutorDocNamespaced), @@ -179,7 +179,7 @@ func TestKubeApplierExecutorRun(t *testing.T) { { name: "error cluster not defined", containsErr: "cluster is not defined in in cluster map", - helper: makeDefaultHelper(t, "../../k8s/applier/testdata"), + helper: makeDefaultHelper(t, "../../k8s/applier/testdata", defaultMetadataPath), bundleFactory: testBundleFactory("../../k8s/applier/testdata/source_bundle"), kubeconf: testKubeconfig(testValidKubeconfig), execDoc: executorDoc(t, ValidExecutorDocNamespaced),