From 49b7abb4ca428c44163175c7441a02644e05b4e0 Mon Sep 17 00:00:00 2001 From: Oliver Nocon <33484802+OliverNocon@users.noreply.github.com> Date: Thu, 2 Jul 2020 12:08:56 +0200 Subject: [PATCH] Fix stashing issue with binrary execution (#1749) Co-authored-by: Daniel Kurzynski --- pkg/config/stepmeta.go | 45 ++++++++++++++------------ pkg/config/stepmeta_test.go | 25 +++++++------- test/groovy/PiperExecuteBinTest.groovy | 19 +++++++++++ vars/piperExecuteBin.groovy | 26 ++++++++------- 4 files changed, 73 insertions(+), 42 deletions(-) diff --git a/pkg/config/stepmeta.go b/pkg/config/stepmeta.go index 979bd0ea9..4d82a06ee 100644 --- a/pkg/config/stepmeta.go +++ b/pkg/config/stepmeta.go @@ -198,16 +198,19 @@ func (m *StepData) GetParameterFilters() StepFilters { // GetContextParameterFilters retrieves all scope dependent parameter filters func (m *StepData) GetContextParameterFilters() StepFilters { var filters StepFilters + contextFilters := []string{} for _, secret := range m.Spec.Inputs.Secrets { - filters.All = append(filters.All, secret.Name) - filters.General = append(filters.General, secret.Name) - filters.Steps = append(filters.Steps, secret.Name) - filters.Stages = append(filters.Stages, secret.Name) - filters.Parameters = append(filters.Parameters, secret.Name) - filters.Env = append(filters.Env, secret.Name) + contextFilters = append(contextFilters, secret.Name) } - containerFilters := []string{} + if len(m.Spec.Inputs.Resources) > 0 { + for _, res := range m.Spec.Inputs.Resources { + if res.Type == "stash" { + contextFilters = append(contextFilters, "stashContent") + break + } + } + } if len(m.Spec.Containers) > 0 { parameterKeys := []string{"containerCommand", "containerShell", "dockerEnvVars", "dockerImage", "dockerOptions", "dockerPullImage", "dockerVolumeBind", "dockerWorkspace"} for _, container := range m.Spec.Containers { @@ -219,19 +222,21 @@ func (m *StepData) GetContextParameterFilters() StepFilters { } } // ToDo: append dependentParam.Value & dependentParam.Name only according to correct parameter scope and not generally - containerFilters = append(containerFilters, parameterKeys...) + contextFilters = append(contextFilters, parameterKeys...) } if len(m.Spec.Sidecars) > 0 { //ToDo: support fallback for "dockerName" configuration property -> via aliasing? - containerFilters = append(containerFilters, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}...) + contextFilters = append(contextFilters, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}...) //ToDo: add condition param.Value and param.Name to filter as for Containers } - if len(containerFilters) > 0 { - filters.All = append(filters.All, containerFilters...) - filters.General = append(filters.General, containerFilters...) - filters.Steps = append(filters.Steps, containerFilters...) - filters.Stages = append(filters.Stages, containerFilters...) - filters.Parameters = append(filters.Parameters, containerFilters...) + if len(contextFilters) > 0 { + filters.All = append(filters.All, contextFilters...) + filters.General = append(filters.General, contextFilters...) + filters.Steps = append(filters.Steps, contextFilters...) + filters.Stages = append(filters.Stages, contextFilters...) + filters.Parameters = append(filters.Parameters, contextFilters...) + filters.Env = append(filters.Env, contextFilters...) + } return filters } @@ -268,7 +273,7 @@ func (m *StepData) GetContextDefaults(stepName string) (io.ReadCloser, error) { } p["containerName"] = container.Name p["containerShell"] = container.Shell - p["dockerEnvVars"] = envVarsAsStringSlice(container.EnvVars) + p["dockerEnvVars"] = envVarsAsMap(container.EnvVars) p["dockerImage"] = container.Image p["dockerName"] = container.Name p["dockerPullImage"] = container.ImagePullPolicy != "Never" @@ -286,7 +291,7 @@ func (m *StepData) GetContextDefaults(stepName string) (io.ReadCloser, error) { if len(m.Spec.Sidecars[0].Command) > 0 { root["sidecarCommand"] = m.Spec.Sidecars[0].Command[0] } - root["sidecarEnvVars"] = envVarsAsStringSlice(m.Spec.Sidecars[0].EnvVars) + root["sidecarEnvVars"] = envVarsAsMap(m.Spec.Sidecars[0].EnvVars) root["sidecarImage"] = m.Spec.Sidecars[0].Image root["sidecarName"] = m.Spec.Sidecars[0].Name root["sidecarPullImage"] = m.Spec.Sidecars[0].ImagePullPolicy != "Never" @@ -364,10 +369,10 @@ func (m *StepData) GetResourceParameters(path, name string) map[string]interface return resourceParams } -func envVarsAsStringSlice(envVars []EnvVar) []string { - e := []string{} +func envVarsAsMap(envVars []EnvVar) map[string]string { + e := map[string]string{} for _, v := range envVars { - e = append(e, fmt.Sprintf("%v=%v", v.Name, v.Value)) + e[v.Name] = v.Value } return e } diff --git a/pkg/config/stepmeta_test.go b/pkg/config/stepmeta_test.go index c82ea0e00..3c3087b26 100644 --- a/pkg/config/stepmeta_test.go +++ b/pkg/config/stepmeta_test.go @@ -230,6 +230,9 @@ func TestGetContextParameterFilters(t *testing.T) { {Name: "testSecret1", Type: "jenkins"}, {Name: "testSecret2", Type: "jenkins"}, }, + Resources: []StepResources{ + {Name: "buildDescriptor", Type: "stash"}, + }, }, }, } @@ -255,14 +258,14 @@ func TestGetContextParameterFilters(t *testing.T) { }, } - t.Run("Secrets", func(t *testing.T) { + t.Run("Secrets and stashes", func(t *testing.T) { filters := metadata1.GetContextParameterFilters() - assert.Equal(t, []string{"testSecret1", "testSecret2"}, filters.All, "incorrect filter All") - assert.Equal(t, []string{"testSecret1", "testSecret2"}, filters.General, "incorrect filter General") - assert.Equal(t, []string{"testSecret1", "testSecret2"}, filters.Steps, "incorrect filter Steps") - assert.Equal(t, []string{"testSecret1", "testSecret2"}, filters.Stages, "incorrect filter Stages") - assert.Equal(t, []string{"testSecret1", "testSecret2"}, filters.Parameters, "incorrect filter Parameters") - assert.Equal(t, []string{"testSecret1", "testSecret2"}, filters.Env, "incorrect filter Env") + assert.Equal(t, []string{"testSecret1", "testSecret2", "stashContent"}, filters.All, "incorrect filter All") + assert.Equal(t, []string{"testSecret1", "testSecret2", "stashContent"}, filters.General, "incorrect filter General") + assert.Equal(t, []string{"testSecret1", "testSecret2", "stashContent"}, filters.Steps, "incorrect filter Steps") + assert.Equal(t, []string{"testSecret1", "testSecret2", "stashContent"}, filters.Stages, "incorrect filter Stages") + assert.Equal(t, []string{"testSecret1", "testSecret2", "stashContent"}, filters.Parameters, "incorrect filter Parameters") + assert.Equal(t, []string{"testSecret1", "testSecret2", "stashContent"}, filters.Env, "incorrect filter Env") }) t.Run("Containers", func(t *testing.T) { @@ -272,7 +275,7 @@ func TestGetContextParameterFilters(t *testing.T) { assert.Equal(t, []string{"containerCommand", "containerShell", "dockerEnvVars", "dockerImage", "dockerOptions", "dockerPullImage", "dockerVolumeBind", "dockerWorkspace", "pip", "scanType"}, filters.Steps, "incorrect filter Steps") assert.Equal(t, []string{"containerCommand", "containerShell", "dockerEnvVars", "dockerImage", "dockerOptions", "dockerPullImage", "dockerVolumeBind", "dockerWorkspace", "pip", "scanType"}, filters.Stages, "incorrect filter Stages") assert.Equal(t, []string{"containerCommand", "containerShell", "dockerEnvVars", "dockerImage", "dockerOptions", "dockerPullImage", "dockerVolumeBind", "dockerWorkspace", "pip", "scanType"}, filters.Parameters, "incorrect filter Parameters") - assert.NotEqual(t, []string{"containerCommand", "containerShell", "dockerEnvVars", "dockerImage", "dockerOptions", "dockerPullImage", "dockerVolumeBind", "dockerWorkspace", "pip", "scanType"}, filters.Env, "incorrect filter Env") + assert.Equal(t, []string{"containerCommand", "containerShell", "dockerEnvVars", "dockerImage", "dockerOptions", "dockerPullImage", "dockerVolumeBind", "dockerWorkspace", "pip", "scanType"}, filters.Env, "incorrect filter Env") }) t.Run("Sidecars", func(t *testing.T) { @@ -282,7 +285,7 @@ func TestGetContextParameterFilters(t *testing.T) { assert.Equal(t, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}, filters.Steps, "incorrect filter Steps") assert.Equal(t, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}, filters.Stages, "incorrect filter Stages") assert.Equal(t, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}, filters.Parameters, "incorrect filter Parameters") - assert.NotEqual(t, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}, filters.Env, "incorrect filter Env") + assert.Equal(t, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}, filters.Env, "incorrect filter Env") }) } @@ -393,7 +396,7 @@ func TestGetContextDefaults(t *testing.T) { assert.Equal(t, "test/command", d.Defaults[0].Steps["testStep"]["containerCommand"], "containerCommand default not available") assert.Equal(t, "testcontainer", d.Defaults[0].Steps["testStep"]["containerName"], "containerName default not available") assert.Equal(t, "/bin/bash", d.Defaults[0].Steps["testStep"]["containerShell"], "containerShell default not available") - assert.Equal(t, []interface{}{"env1=val1", "env2=val2"}, d.Defaults[0].Steps["testStep"]["dockerEnvVars"], "dockerEnvVars default not available") + assert.Equal(t, map[string]interface{}{"env1": "val1", "env2": "val2"}, d.Defaults[0].Steps["testStep"]["dockerEnvVars"], "dockerEnvVars default not available") assert.Equal(t, "testImage:tag", d.Defaults[0].Steps["testStep"]["dockerImage"], "dockerImage default not available") assert.Equal(t, "testcontainer", d.Defaults[0].Steps["testStep"]["dockerName"], "dockerName default not available") assert.Equal(t, true, d.Defaults[0].Steps["testStep"]["dockerPullImage"], "dockerPullImage default not available") @@ -402,7 +405,7 @@ func TestGetContextDefaults(t *testing.T) { //assert.Equal(t, []interface{}{"mn1:mp1", "mn2:mp2"}, d.Defaults[0].Steps["testStep"]["dockerVolumeBind"], "dockerVolumeBind default not available") assert.Equal(t, "/sidecar/command", d.Defaults[0].Steps["testStep"]["sidecarCommand"], "sidecarCommand default not available") - assert.Equal(t, []interface{}{"env3=val3", "env4=val4"}, d.Defaults[0].Steps["testStep"]["sidecarEnvVars"], "sidecarEnvVars default not available") + assert.Equal(t, map[string]interface{}{"env3": "val3", "env4": "val4"}, d.Defaults[0].Steps["testStep"]["sidecarEnvVars"], "sidecarEnvVars default not available") assert.Equal(t, "testSidecarImage:tag", d.Defaults[0].Steps["testStep"]["sidecarImage"], "sidecarImage default not available") assert.Equal(t, "testsidecar", d.Defaults[0].Steps["testStep"]["sidecarName"], "sidecarName default not available") assert.Equal(t, false, d.Defaults[0].Steps["testStep"]["sidecarPullImage"], "sidecarPullImage default not available") diff --git a/test/groovy/PiperExecuteBinTest.groovy b/test/groovy/PiperExecuteBinTest.groovy index f43d43816..ce4df4b46 100644 --- a/test/groovy/PiperExecuteBinTest.groovy +++ b/test/groovy/PiperExecuteBinTest.groovy @@ -147,6 +147,7 @@ class PiperExecuteBinTest extends BasePiperTest { assertThat(credentials[2], allOf(hasEntry('credentialsId', 'credUsernamePassword'), hasEntry('usernameVariable', 'PIPER_user') , hasEntry('passwordVariable', 'PIPER_password'))) assertThat(dockerExecuteRule.dockerParams.dockerImage, is('my.Registry/my/image:latest')) + assertThat(dockerExecuteRule.dockerParams.stashContent, is([])) assertThat(artifacts[0], allOf(hasEntry('artifacts', '1234.pdf'), hasEntry('allowEmptyArchive', false))) } @@ -359,4 +360,22 @@ class PiperExecuteBinTest extends BasePiperTest { assertThat(unstableCalled, is(true)) } + + @Test + void testProperStashHandling() { + shellCallRule.setReturnValue('./piper getConfig --contextConfig --stepMetadata \'.pipeline/tmp/metadata/test.yaml\'', '{"dockerImage":"test","stashContent":["buildDescriptor"]}') + + stepRule.step.piperExecuteBin( + [ + juStabUtils: utils, + jenkinsUtilsStub: jenkinsUtils, + script: nullScript + ], + 'testStep', + 'metadata/test.yaml', + [] + ) + + assertThat(dockerExecuteRule.dockerParams.stashContent, is(["buildDescriptor", "pipelineConfigAndTests"])) + } } diff --git a/vars/piperExecuteBin.groovy b/vars/piperExecuteBin.groovy index d59c22e31..523dcc08b 100644 --- a/vars/piperExecuteBin.groovy +++ b/vars/piperExecuteBin.groovy @@ -22,9 +22,11 @@ void call(Map parameters = [:], String stepName, String metadataFile, List crede Script script = checkScript(this, parameters) ?: this def jenkinsUtils = parameters.jenkinsUtilsStub ?: new JenkinsUtils() + def utils = parameters.juStabUtils ?: new Utils() + String piperGoPath = parameters.piperGoPath ?: './piper' - prepareExecution(script, parameters) + prepareExecution(script, utils, parameters) prepareMetadataResource(script, metadataFile) Map stepParameters = prepareStepParameters(parameters) @@ -45,6 +47,14 @@ void call(Map parameters = [:], String stepName, String metadataFile, List crede echo "Context Config: ${config}" } + // prepare stashes + // first eliminate empty stahes + config.stashContent = utils.unstashAll(config.stashContent) + // then make sure that commonPipelineEnvironment, config, ... is also available when step stashing is active + if (config.stashContent?.size() > 0) { + config.stashContent.add('pipelineConfigAndTests') + } + if (parameters.stashNoDefaultExcludes) { // Merge this parameter which is only relevant in Jenkins context // (for dockerExecuteOnKubernetes step) and go binary doesn't know about @@ -64,8 +74,7 @@ void call(Map parameters = [:], String stepName, String metadataFile, List crede } } -static void prepareExecution(Script script, Map parameters = [:]) { - def utils = parameters.juStabUtils ?: new Utils() +static void prepareExecution(Script script, Utils utils, Map parameters = [:]) { def piperGoUtils = parameters.piperGoUtils ?: new PiperGoUtils(script, utils) piperGoUtils.unstashPiperBin() utils.unstash('pipelineConfigAndTests') @@ -123,14 +132,9 @@ static String getCustomConfigArg(def script) { void dockerWrapper(script, config, body) { if (config.dockerImage) { - dockerExecute( - script: script, - dockerImage: config.dockerImage, - dockerWorkspace: config.dockerWorkspace, - dockerOptions: config.dockerOptions, - stashNoDefaultExcludes : config.stashNoDefaultExcludes, - //ToDo: add additional dockerExecute parameters - ) { + Map dockerExecuteParameters = [:].plus(config) + dockerExecuteParameters.script = script + dockerExecute(dockerExecuteParameters) { body() } } else {