From fc95231f13a77b149cd1c1db7d9930e71814ba78 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Wed, 19 Jun 2019 13:22:05 -0400 Subject: [PATCH] chore(tekton): Switch env to be corev1.EnvVar (#4332) I honestly can't remember why I used our own `EnvVar` rather than `corev1.EnvVar`, and it's kinda confusing, so let's just switch. Signed-off-by: Andrew Bayer --- Makefile.codegen | 4 +- pkg/tekton/syntax/pipeline.go | 82 ++++++------------- pkg/tekton/syntax/pipeline_test.go | 4 +- .../syntax_helpers_test/test_helpers.go | 6 +- pkg/tekton/syntax/zz_generated.deepcopy.go | 46 +++++------ 5 files changed, 55 insertions(+), 87 deletions(-) diff --git a/Makefile.codegen b/Makefile.codegen index 7981c0c542..ad459fa952 100644 --- a/Makefile.codegen +++ b/Makefile.codegen @@ -40,11 +40,13 @@ go-generate: @echo "Generating Mocks using pegomock" $(GO) generate ./... -generate-client: codegen-clientset fmt ## Generate the client +generate-client: codegen-clientset codegen-config fmt ## Generate the client codegen-clientset: build-codegen ## Generate the k8s types and clients @echo "Generating Kubernetes Clients for pkg/apis in pkg/client for jenkins.io:v1" ./build/$(CODE_GEN_BIN_NAME) --generator-version $(CLIENTSET_GENERATOR_VERSION) clientset --output-package=pkg/client --input-package=pkg/apis --group-with-version=jenkins.io:v1 + +codegen-config: build-codegen ## Generate the deepcopy for the config, jenkinsfile, and tekton/syntax packages ./build/$(CODE_GEN_BIN_NAME) --generator-version $(CLIENTSET_GENERATOR_VERSION) clientset --generator deepcopy --output-package=pkg --input-package=pkg --group-with-version=config: --group-with-version=jenkinsfile: --group-with-version=tekton:syntax # Generated docs are not checked in diff --git a/pkg/tekton/syntax/pipeline.go b/pkg/tekton/syntax/pipeline.go index 7b8f048dd9..3ce1fd186b 100644 --- a/pkg/tekton/syntax/pipeline.go +++ b/pkg/tekton/syntax/pipeline.go @@ -36,15 +36,15 @@ const ( // ParsedPipeline is the internal representation of the Pipeline, used to validate and create CRDs type ParsedPipeline struct { - Agent *Agent `json:"agent,omitempty"` - Env []EnvVar `json:"env,omitempty"` - Options *RootOptions `json:"options,omitempty"` - Stages []Stage `json:"stages"` - Post []Post `json:"post,omitempty"` - WorkingDir *string `json:"dir,omitempty"` + Agent *Agent `json:"agent,omitempty"` + Env []corev1.EnvVar `json:"env,omitempty"` + Options *RootOptions `json:"options,omitempty"` + Stages []Stage `json:"stages"` + Post []Post `json:"post,omitempty"` + WorkingDir *string `json:"dir,omitempty"` // Replaced by Env, retained for backwards compatibility - Environment []EnvVar `json:"environment,omitempty"` + Environment []corev1.EnvVar `json:"environment,omitempty"` } // Agent defines where the pipeline, stage, or step should run. @@ -58,12 +58,6 @@ type Agent struct { Dir string `json:"dir,omitempty"` } -// EnvVar is a key/value pair defining an environment variable -type EnvVar struct { - Name string `json:"name"` - Value string `json:"value"` -} - // TimeoutUnit is used for calculating timeout duration type TimeoutUnit string @@ -175,7 +169,7 @@ type Step struct { Image string `json:"image,omitempty"` // env allows defining per-step environment variables - Env []EnvVar `json:"env,omitempty"` + Env []corev1.EnvVar `json:"env,omitempty"` // Legacy fields from jenkinsfile.PipelineStep before it was eliminated. Comment string `json:"comment,omitempty"` @@ -201,18 +195,18 @@ type Loop struct { // Stage is a unit of work in a pipeline, corresponding either to a Task or a set of Tasks to be run sequentially or in // parallel with common configuration. type Stage struct { - Name string `json:"name"` - Agent *Agent `json:"agent,omitempty"` - Env []EnvVar `json:"env,omitempty"` - Options *StageOptions `json:"options,omitempty"` - Steps []Step `json:"steps,omitempty"` - Stages []Stage `json:"stages,omitempty"` - Parallel []Stage `json:"parallel,omitempty"` - Post []Post `json:"post,omitempty"` - WorkingDir *string `json:"dir,omitempty"` + Name string `json:"name"` + Agent *Agent `json:"agent,omitempty"` + Env []corev1.EnvVar `json:"env,omitempty"` + Options *StageOptions `json:"options,omitempty"` + Steps []Step `json:"steps,omitempty"` + Stages []Stage `json:"stages,omitempty"` + Parallel []Stage `json:"parallel,omitempty"` + Post []Post `json:"post,omitempty"` + WorkingDir *string `json:"dir,omitempty"` // Replaced by Env, retained for backwards compatibility - Environment []EnvVar `json:"environment,omitempty"` + Environment []corev1.EnvVar `json:"environment,omitempty"` } // PostCondition is used to specify under what condition a post action should be executed. @@ -501,7 +495,7 @@ func MangleToRfc1035Label(body string, suffix string) string { } // GetEnv gets the environment for the ParsedPipeline, returning Env first and Environment if Env isn't populated. -func (j *ParsedPipeline) GetEnv() []EnvVar { +func (j *ParsedPipeline) GetEnv() []corev1.EnvVar { if j != nil { if len(j.Env) > 0 { return j.Env @@ -509,11 +503,11 @@ func (j *ParsedPipeline) GetEnv() []EnvVar { return j.Environment } - return []EnvVar{} + return []corev1.EnvVar{} } // GetEnv gets the environment for the Stage, returning Env first and Environment if Env isn't populated. -func (s *Stage) GetEnv() []EnvVar { +func (s *Stage) GetEnv() []corev1.EnvVar { if len(s.Env) > 0 { return s.Env } @@ -955,28 +949,16 @@ func EnvMapToSlice(envMap map[string]corev1.EnvVar) []corev1.EnvVar { return env } -func toContainerEnvVars(origEnv []EnvVar) []corev1.EnvVar { - env := make([]corev1.EnvVar, 0, len(origEnv)) - for _, e := range origEnv { - env = append(env, corev1.EnvVar{ - Name: e.Name, - Value: e.Value, - }) - } - - return env -} - // AddContainerEnvVarsToPipeline allows for adding a slice of container environment variables directly to the // pipeline, if they're not already defined. func (j *ParsedPipeline) AddContainerEnvVarsToPipeline(origEnv []corev1.EnvVar) { if len(origEnv) > 0 { - envMap := make(map[string]EnvVar) + envMap := make(map[string]corev1.EnvVar) // Add the container env vars first. for _, e := range origEnv { if e.ValueFrom == nil { - envMap[e.Name] = EnvVar{ + envMap[e.Name] = corev1.EnvVar{ Name: e.Name, Value: e.Value, } @@ -988,7 +970,7 @@ func (j *ParsedPipeline) AddContainerEnvVarsToPipeline(origEnv []corev1.EnvVar) envMap[e.Name] = e } - env := make([]EnvVar, 0, len(envMap)) + env := make([]corev1.EnvVar, 0, len(envMap)) // Avoid nondeterministic results by sorting the keys and appending vars in that order. var envVars []string @@ -1022,16 +1004,6 @@ func scopedEnv(newEnv []corev1.EnvVar, parentEnv []corev1.EnvVar) []corev1.EnvVa return EnvMapToSlice(envMap) } -func (j *ParsedPipeline) toStepEnvVars() []corev1.EnvVar { - envMap := make(map[string]corev1.EnvVar) - - for _, e := range j.GetEnv() { - envMap[e.Name] = corev1.EnvVar{Name: e.Name, Value: e.Value} - } - - return EnvMapToSlice(envMap) -} - type transformedStage struct { Stage Stage // Only one of Sequential, Parallel, and Task is non-empty @@ -1182,7 +1154,7 @@ func stageToTask(s Stage, pipelineIdentifier string, buildIdentifier string, nam stageContainer = merged } - env := scopedEnv(toContainerEnvVars(s.GetEnv()), parentEnv) + env := scopedEnv(s.GetEnv(), parentEnv) agent := s.Agent.DeepCopy() @@ -1440,7 +1412,7 @@ func generateSteps(step Step, inheritedAgent, sourceDir string, baseWorkingDir * c.Stdin = false c.TTY = false - c.Env = scopedEnv(toContainerEnvVars(step.Env), scopedEnv(env, c.Env)) + c.Env = scopedEnv(step.Env, scopedEnv(env, c.Env)) steps = append(steps, *c) } else if step.Loop != nil { @@ -1533,7 +1505,7 @@ func (j *ParsedPipeline) GenerateCRDs(pipelineIdentifier string, buildIdentifier var tasks []*tektonv1alpha1.Task - baseEnv := j.toStepEnvVars() + baseEnv := j.GetEnv() for i, s := range j.Stages { isLastStage := i == len(j.Stages)-1 diff --git a/pkg/tekton/syntax/pipeline_test.go b/pkg/tekton/syntax/pipeline_test.go index 0b8193e65f..4113313f68 100644 --- a/pkg/tekton/syntax/pipeline_test.go +++ b/pkg/tekton/syntax/pipeline_test.go @@ -1743,7 +1743,7 @@ func TestParsedPipelineHelpers(t *testing.T) { Unit: syntax.TimeoutUnitSeconds, }, }, - Env: []syntax.EnvVar{ + Env: []corev1.EnvVar{ { Name: "ANIMAL", Value: "MONKEY", @@ -1816,7 +1816,7 @@ func TestParsedPipelineHelpers(t *testing.T) { Image: "some-other-image", }, }}, - Env: []syntax.EnvVar{ + Env: []corev1.EnvVar{ { Name: "STAGE_VAR_ONE", Value: "some value", diff --git a/pkg/tekton/syntax/syntax_helpers_test/test_helpers.go b/pkg/tekton/syntax/syntax_helpers_test/test_helpers.go index a3a3404748..0d74230442 100644 --- a/pkg/tekton/syntax/syntax_helpers_test/test_helpers.go +++ b/pkg/tekton/syntax/syntax_helpers_test/test_helpers.go @@ -261,7 +261,7 @@ func PipelineOptionsRetry(count int8) PipelineOptionsOp { // PipelineEnvVar add an environment variable, with specified name and value, to the pipeline. func PipelineEnvVar(name, value string) PipelineOp { return func(parsed *syntax.ParsedPipeline) { - parsed.Env = append(parsed.GetEnv(), syntax.EnvVar{ + parsed.Env = append(parsed.GetEnv(), corev1.EnvVar{ Name: name, Value: value, }) @@ -382,7 +382,7 @@ func StageOptionsUnstash(name, dir string) StageOptionsOp { // StageEnvVar add an environment variable, with specified name and value, to the stage. func StageEnvVar(name, value string) StageOp { return func(stage *syntax.Stage) { - stage.Env = append(stage.GetEnv(), syntax.EnvVar{ + stage.Env = append(stage.GetEnv(), corev1.EnvVar{ Name: name, Value: value, }) @@ -481,7 +481,7 @@ func StepLoop(variable string, values []string, ops ...LoopOp) StepOp { // StepEnvVar add an environment variable, with specified name and value, to the step. func StepEnvVar(name, value string) StepOp { return func(step *syntax.Step) { - step.Env = append(step.Env, syntax.EnvVar{ + step.Env = append(step.Env, corev1.EnvVar{ Name: name, Value: value, }) diff --git a/pkg/tekton/syntax/zz_generated.deepcopy.go b/pkg/tekton/syntax/zz_generated.deepcopy.go index 413000e2a8..06b6fb77e5 100644 --- a/pkg/tekton/syntax/zz_generated.deepcopy.go +++ b/pkg/tekton/syntax/zz_generated.deepcopy.go @@ -24,22 +24,6 @@ func (in *Agent) DeepCopy() *Agent { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *EnvVar) DeepCopyInto(out *EnvVar) { - *out = *in - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EnvVar. -func (in *EnvVar) DeepCopy() *EnvVar { - if in == nil { - return nil - } - out := new(EnvVar) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Loop) DeepCopyInto(out *Loop) { *out = *in @@ -78,8 +62,10 @@ func (in *ParsedPipeline) DeepCopyInto(out *ParsedPipeline) { } if in.Env != nil { in, out := &in.Env, &out.Env - *out = make([]EnvVar, len(*in)) - copy(*out, *in) + *out = make([]v1.EnvVar, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.Options != nil { in, out := &in.Options, &out.Options @@ -107,8 +93,10 @@ func (in *ParsedPipeline) DeepCopyInto(out *ParsedPipeline) { } if in.Environment != nil { in, out := &in.Environment, &out.Environment - *out = make([]EnvVar, len(*in)) - copy(*out, *in) + *out = make([]v1.EnvVar, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -247,8 +235,10 @@ func (in *Stage) DeepCopyInto(out *Stage) { } if in.Env != nil { in, out := &in.Env, &out.Env - *out = make([]EnvVar, len(*in)) - copy(*out, *in) + *out = make([]v1.EnvVar, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.Options != nil { in, out := &in.Options, &out.Options @@ -290,8 +280,10 @@ func (in *Stage) DeepCopyInto(out *Stage) { } if in.Environment != nil { in, out := &in.Environment, &out.Environment - *out = make([]EnvVar, len(*in)) - copy(*out, *in) + *out = make([]v1.EnvVar, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -385,8 +377,10 @@ func (in *Step) DeepCopyInto(out *Step) { } if in.Env != nil { in, out := &in.Env, &out.Env - *out = make([]EnvVar, len(*in)) - copy(*out, *in) + *out = make([]v1.EnvVar, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.Steps != nil { in, out := &in.Steps, &out.Steps