Skip to content

Commit

Permalink
chore(tekton): Switch env to be corev1.EnvVar (jenkins-x#4332)
Browse files Browse the repository at this point in the history 
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 <andrew.bayer@gmail.com>
  • Loading branch information
@abayer @jenkins-x-bot
abayer authored and jenkins-x-bot committed Jun 19, 2019
1 parent 9a088e3 commit fc95231
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 87 deletions.
4 changes: 3 additions & 1 deletion Makefile.codegen
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
82 changes: 27 additions & 55 deletions pkg/tekton/syntax/pipeline.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand Down Expand Up @@ -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"`
Expand All @@ -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.
Expand Down Expand Up @@ -501,19 +495,19 @@ 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
}

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
}
Expand Down Expand Up @@ -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,
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/tekton/syntax/pipeline_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1743,7 +1743,7 @@ func TestParsedPipelineHelpers(t *testing.T) {
Unit: syntax.TimeoutUnitSeconds,
},
},
Env: []syntax.EnvVar{
Env: []corev1.EnvVar{
{
Name: "ANIMAL",
Value: "MONKEY",
Expand Down Expand Up @@ -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",
Expand Down
6 changes: 3 additions & 3 deletions pkg/tekton/syntax/syntax_helpers_test/test_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down Expand Up @@ -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,
})
Expand Down Expand Up @@ -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,
})
Expand Down
46 changes: 20 additions & 26 deletions pkg/tekton/syntax/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit fc95231

Please sign in to comment.