Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: REPLACE_ME build pack placeholders should work with modern syntax #6700

Merged
merged 2 commits into from
Feb 7, 2020
Merged

fix: REPLACE_ME build pack placeholders should work with modern syntax #6700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1b2cc1a
fix: REPLACE_ME build pack placeholders should work with modern syntax
abayer Feb 6, 2020
6a1fae6
fix: Move kaniko special-casing to syntax, not jenkinsfile.
abayer Feb 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/cmd/step/create/test_data/step_create_task/packs/jx-syntax-in-buildpack/pipeline.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pipelines:
- command: source /root/.bashrc && flake8
name: flake8
image: maven
dir: /workspace/source
dir: /workspace/source/REPLACE_ME_APP_NAME
release:
pipeline:
agent:
Expand All @@ -25,4 +25,4 @@ pipelines:
- command: source /root/.bashrc && flake8
name: flake8
image: maven
dir: /workspace/source
dir: /workspace/source/REPLACE_ME_APP_NAME
13 changes: 13 additions & 0 deletions pkg/cmd/step/syntax/step_syntax_effective.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,19 @@ func (o *StepSyntaxEffectiveOptions) createPipelineForKind(kind string, lifecycl
}
}

// Replace placeholders in directories.
replacePlaceholderArgs := syntax.StepPlaceholderReplacementArgs{
WorkspaceDir: o.getWorkspaceDir(),
GitName: o.GitInfo.Name,
GitOrg: o.GitInfo.Organisation,
GitHost: o.GitInfo.Host,
ProjectID: o.ProjectID,
DockerRegistry: o.getDockerRegistry(projectConfig),
DockerRegistryOrg: o.GetDockerRegistryOrg(projectConfig, o.GitInfo),
KanikoImage: o.KanikoImage,
UseKaniko: o.UseKaniko,
}
parsed.ReplacePlaceholdersInStepAndStageDirs(replacePlaceholderArgs)
parsed.AddContainerEnvVarsToPipeline(pipelineConfig.Env)

if pipelineConfig.ContainerOptions != nil {
Expand Down
32 changes: 15 additions & 17 deletions pkg/cmd/step/syntax/step_syntax_effective_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,26 +463,26 @@ func TestCreateCanonicalPipeline(t *testing.T) {
Pipeline: sht.ParsedPipeline(
sht.PipelineAgent("go"),
sht.PipelineStage("from-build-pack",
sht.StageStep(sht.StepCmd("make build"), sht.StepName("build")),
sht.StageStep(sht.StepCmd("jx step helm build"), sht.StepName("helm-build")),
sht.StageStep(sht.StepCmd("make build"), sht.StepName("build"), sht.StepDir("/workspace/source")),
sht.StageStep(sht.StepCmd("jx step helm build"), sht.StepName("helm-build"), sht.StepDir("/workspace/source")),
),
),
},
PullRequest: &jenkinsfile.PipelineLifecycles{
Pipeline: sht.ParsedPipeline(
sht.PipelineAgent("go"),
sht.PipelineStage("from-build-pack",
sht.StageStep(sht.StepCmd("make build"), sht.StepName("build")),
sht.StageStep(sht.StepCmd("jx step helm build"), sht.StepName("helm-build")),
sht.StageStep(sht.StepCmd("make build"), sht.StepName("build"), sht.StepDir("/workspace/source")),
sht.StageStep(sht.StepCmd("jx step helm build"), sht.StepName("helm-build"), sht.StepDir("/workspace/source")),
),
),
},
Release: &jenkinsfile.PipelineLifecycles{
Pipeline: sht.ParsedPipeline(
sht.PipelineAgent("go"),
sht.PipelineStage("from-build-pack",
sht.StageStep(sht.StepCmd("make build"), sht.StepName("build")),
sht.StageStep(sht.StepCmd("jx step helm build"), sht.StepName("helm-build")),
sht.StageStep(sht.StepCmd("make build"), sht.StepName("build"), sht.StepDir("/workspace/source")),
sht.StageStep(sht.StepCmd("jx step helm build"), sht.StepName("helm-build"), sht.StepDir("/workspace/source")),
),
),
},
Expand Down Expand Up @@ -515,8 +515,8 @@ func TestCreateCanonicalPipeline(t *testing.T) {
sht.PipelineEnvVar("FRUIT", "BANANA"),
sht.PipelineEnvVar("GIT_AUTHOR_NAME", "somebodyelse"),
sht.PipelineStage("from-build-pack",
sht.StageStep(sht.StepCmd("make build"), sht.StepName("build")),
sht.StageStep(sht.StepCmd("jx step helm build"), sht.StepName("helm-build")),
sht.StageStep(sht.StepCmd("make build"), sht.StepName("build"), sht.StepDir("/workspace/source")),
sht.StageStep(sht.StepCmd("jx step helm build"), sht.StepName("helm-build"), sht.StepDir("/workspace/source")),
),
),
},
Expand All @@ -526,8 +526,8 @@ func TestCreateCanonicalPipeline(t *testing.T) {
sht.PipelineEnvVar("FRUIT", "BANANA"),
sht.PipelineEnvVar("GIT_AUTHOR_NAME", "somebodyelse"),
sht.PipelineStage("from-build-pack",
sht.StageStep(sht.StepCmd("make build"), sht.StepName("build")),
sht.StageStep(sht.StepCmd("jx step helm build"), sht.StepName("helm-build")),
sht.StageStep(sht.StepCmd("make build"), sht.StepName("build"), sht.StepDir("/workspace/source")),
sht.StageStep(sht.StepCmd("jx step helm build"), sht.StepName("helm-build"), sht.StepDir("/workspace/source")),
),
),
},
Expand All @@ -537,8 +537,8 @@ func TestCreateCanonicalPipeline(t *testing.T) {
sht.PipelineEnvVar("FRUIT", "BANANA"),
sht.PipelineEnvVar("GIT_AUTHOR_NAME", "somebodyelse"),
sht.PipelineStage("from-build-pack",
sht.StageStep(sht.StepCmd("make build"), sht.StepName("build")),
sht.StageStep(sht.StepCmd("jx step helm build"), sht.StepName("helm-build")),
sht.StageStep(sht.StepCmd("make build"), sht.StepName("build"), sht.StepDir("/workspace/source")),
sht.StageStep(sht.StepCmd("jx step helm build"), sht.StepName("helm-build"), sht.StepDir("/workspace/source")),
),
),
},
Expand Down Expand Up @@ -831,17 +831,17 @@ func TestCreateCanonicalPipeline(t *testing.T) {
sht.PipelineAgent("maven"),
sht.PipelineStage("build",
sht.StageStep(sht.StepCmd("source /root/.bashrc && flake8"), sht.StepImage("maven"),
sht.StepDir("/workspace/source"), sht.StepName("flake8")),
sht.StepDir("/workspace/source/jx-demo-qs"), sht.StepName("flake8")),
),
),
},
Release: &jenkinsfile.PipelineLifecycles{
Pipeline: sht.ParsedPipeline(
sht.PipelineAgent("maven"),
sht.PipelineStage("build",
sht.StageStep(sht.StepCmd("source /root/.bashrc && flake8"), sht.StepImage("maven"), sht.StepDir("/workspace/source"),
sht.StageStep(sht.StepCmd("source /root/.bashrc && flake8"), sht.StepImage("maven"), sht.StepDir("/workspace/source/jx-demo-qs"),
sht.StepName("flake8")),
sht.StageStep(sht.StepCmd("echo hi there"), sht.StepName("hi-there")),
sht.StageStep(sht.StepCmd("echo hi there"), sht.StepName("hi-there"), sht.StepDir("/workspace/source")),
),
),
},
Expand Down Expand Up @@ -932,8 +932,6 @@ func TestCreateCanonicalPipeline(t *testing.T) {
t.Fatalf("Error creating canonical pipeline: %s", err)
}
if d, _ := kmp.SafeDiff(tt.expected, newConfig, cmpopts.IgnoreFields(corev1.ResourceRequirements{}, "Requests")); d != "" {
cy, _ := yaml.Marshal(newConfig)
t.Logf("NEW CONFIG: %s", cy)
t.Errorf("Generated canonical pipeline does not match expected:\n%s", d)
}
})
Expand Down
58 changes: 1 addition & 57 deletions pkg/jenkinsfile/pipeline.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"os"
"path/filepath"
"reflect"
"regexp"
"strconv"
"strings"
"text/template"
Expand Down Expand Up @@ -58,10 +57,6 @@ var (

// CreateStepModes the step creation modes
CreateStepModes = []string{CreateStepModePre, CreateStepModePost, CreateStepModeReplace}

ipAddressRegistryRegex = regexp.MustCompile(`\d+\.\d+\.\d+\.\d+.\d+(:\d+)?`)

commandIsSkaffoldRegex = regexp.MustCompile(`export VERSION=.*? && skaffold build.*`)
)

// Pipelines contains all the different kinds of pipeline for different branches
Expand Down Expand Up @@ -814,20 +809,6 @@ func (c *PipelineConfig) createPipelineSteps(step *syntax.Step, prefixPath strin
if step.Dir != "" {
dir = step.Dir
}
// Replace the Go buildpack path with the correct location for Tekton builds.
dir = strings.Replace(dir, "/home/jenkins/go/src/REPLACE_ME_GIT_PROVIDER/REPLACE_ME_ORG/REPLACE_ME_APP_NAME", args.WorkspaceDir, -1)

dir = strings.Replace(dir, util.PlaceHolderAppName, args.GitName, -1)
dir = strings.Replace(dir, util.PlaceHolderOrg, args.GitOrg, -1)
dir = strings.Replace(dir, util.PlaceHolderGitProvider, strings.ToLower(args.GitHost), -1)
dir = strings.Replace(dir, util.PlaceHolderDockerRegistryOrg, args.DockerRegistryOrg, -1)

if strings.HasPrefix(dir, "./") {
dir = args.WorkspaceDir + strings.TrimPrefix(dir, ".")
}
if !filepath.IsAbs(dir) {
dir = filepath.Join(args.WorkspaceDir, dir)
}

if step.GetCommand() != "" {
if containerName == "" {
Expand Down Expand Up @@ -855,9 +836,7 @@ func (c *PipelineConfig) createPipelineSteps(step *syntax.Step, prefixPath strin

s.Dir = dir

modifyStep := c.modifyStep(s, dir, args.DockerRegistry, args.DockerRegistryOrg, args.GitName, args.ProjectID, args.KanikoImage, args.UseKaniko)

steps = append(steps, modifyStep)
steps = append(steps, s)
} else if step.Loop != nil {
// Just copy in the loop step without altering it.
// TODO: We don't get magic around image resolution etc, but we avoid naming collisions that result otherwise.
Expand Down Expand Up @@ -889,41 +868,6 @@ func replaceCommandText(step *syntax.Step) string {
return answer
}

// modifyStep allows a container step to be modified to do something different
func (c *PipelineConfig) modifyStep(parsedStep syntax.Step, workspaceDir, dockerRegistry, dockerRegistryOrg, appName, projectID, kanikoImage string, useKaniko bool) syntax.Step {
if useKaniko {
if strings.HasPrefix(parsedStep.GetCommand(), "skaffold build") ||
(len(parsedStep.Arguments) > 0 && strings.HasPrefix(strings.Join(parsedStep.Arguments[1:], " "), "skaffold build")) ||
commandIsSkaffoldRegex.MatchString(parsedStep.GetCommand()) {

sourceDir := workspaceDir
dockerfile := filepath.Join(sourceDir, "Dockerfile")
localRepo := dockerRegistry
destination := dockerRegistry + "/" + dockerRegistryOrg + "/" + appName

args := []string{"--cache=true", "--cache-dir=/workspace",
"--context=" + sourceDir,
"--dockerfile=" + dockerfile,
"--destination=" + destination + ":${inputs.params.version}",
"--cache-repo=" + localRepo + "/" + projectID + "/cache",
}
if localRepo != "gcr.io" {
args = append(args, "--skip-tls-verify-registry="+localRepo)
}

if ipAddressRegistryRegex.MatchString(localRepo) {
args = append(args, "--insecure")
}

parsedStep.Command = "/kaniko/executor"
parsedStep.Arguments = args

parsedStep.Image = kanikoImage
}
}
return parsedStep
}

// createStageForBuildPack generates the Task for a build pack
func (c *PipelineConfig) createStageForBuildPack(args CreatePipelineArguments) (*syntax.Stage, int, error) {
if args.Lifecycles == nil {
Expand Down
126 changes: 126 additions & 0 deletions pkg/tekton/syntax/pipeline.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ const (
braceMatchingRegex = "(\\$(\\{(?P<var>inputs\\.params\\.[_a-zA-Z][_a-zA-Z0-9.-]*)\\}))"
)

var (
ipAddressRegistryRegex = regexp.MustCompile(`\d+\.\d+\.\d+\.\d+.\d+(:\d+)?`)

commandIsSkaffoldRegex = regexp.MustCompile(`export VERSION=.*? && skaffold build.*`)
)

// ParsedPipeline is the internal representation of the Pipeline, used to validate and create CRDs
type ParsedPipeline struct {
Agent *Agent `json:"agent,omitempty"`
Expand Down Expand Up @@ -1091,6 +1097,126 @@ func (j *ParsedPipeline) GetPossibleAffinityPolicy(name string) *corev1.Affinity
return nil
}

// StepPlaceholderReplacementArgs specifies the arguments required for replacing placeholders in build pack directories.
type StepPlaceholderReplacementArgs struct {
WorkspaceDir string
GitName string
GitOrg string
GitHost string
DockerRegistry string
DockerRegistryOrg string
ProjectID string
KanikoImage string
UseKaniko bool
}

// ReplacePlaceholdersInStepAndStageDirs traverses this pipeline's stages and any nested stages for any steps (and any nested steps)
// within the stages, and replaces "REPLACE_ME_..." placeholders in those steps' directories.
func (j *ParsedPipeline) ReplacePlaceholdersInStepAndStageDirs(args StepPlaceholderReplacementArgs) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported method ParsedPipeline.ReplacePlaceholdersInStepAndStageDirs should have comment or be unexported

var stages []Stage
for _, s := range j.Stages {
s.replacePlaceholdersInStage(args)
stages = append(stages, s)
}
j.Stages = stages
}

func (s *Stage) replacePlaceholdersInStage(args StepPlaceholderReplacementArgs) {
var steps []Step
var stages []Stage
var parallel []Stage
for _, step := range s.Steps {
step.replacePlaceholdersInStep(args)
steps = append(steps, step)
}
for _, nested := range s.Stages {
nested.replacePlaceholdersInStage(args)
stages = append(stages, nested)
}
for _, p := range s.Parallel {
p.replacePlaceholdersInStage(args)
parallel = append(parallel, p)
}
s.Steps = steps
s.Stages = stages
s.Parallel = parallel
}

func (s *Step) replacePlaceholdersInStep(args StepPlaceholderReplacementArgs) {
if s.GetCommand() != "" {
s.modifyStep(args)
dir := args.WorkspaceDir

if s.Dir != "" {
dir = s.Dir
}
// Replace the Go buildpack path with the correct location for Tekton builds.
dir = strings.Replace(dir, "/home/jenkins/go/src/REPLACE_ME_GIT_PROVIDER/REPLACE_ME_ORG/REPLACE_ME_APP_NAME", args.WorkspaceDir, -1)

dir = strings.Replace(dir, util.PlaceHolderAppName, args.GitName, -1)
dir = strings.Replace(dir, util.PlaceHolderOrg, args.GitOrg, -1)
dir = strings.Replace(dir, util.PlaceHolderGitProvider, strings.ToLower(args.GitHost), -1)
dir = strings.Replace(dir, util.PlaceHolderDockerRegistryOrg, args.DockerRegistryOrg, -1)

if strings.HasPrefix(dir, "./") {
dir = args.WorkspaceDir + strings.TrimPrefix(dir, ".")
}
if !filepath.IsAbs(dir) {
dir = filepath.Join(args.WorkspaceDir, dir)
}

s.Dir = dir
}
var steps []*Step
for _, nested := range s.Steps {
nested.replacePlaceholdersInStep(args)
steps = append(steps, nested)
}
s.Steps = steps
if s.Loop != nil {
var loopSteps []Step
for _, nested := range s.Loop.Steps {
nested.replacePlaceholdersInStep(args)
loopSteps = append(loopSteps, nested)
}
s.Loop.Steps = loopSteps
}
}

// modifyStep allows a container step to be modified to do something different
func (s *Step) modifyStep(params StepPlaceholderReplacementArgs) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gee, I was not aware of how tightly the buildpack setup is tied in here. So changes to build packs might have very unexpected side effects. Either when we try to change them or when a user tries to customize.

I realize though that we are just moving code here.

Would it potentially not better to move this somehow out and let the user make a choice, by for example choosing a specific build-pack w/ or w/o kaniko? I seem to recall @garethjevans saying that it would be nice to get this out, but that it would not be so easy?

if params.UseKaniko {
if strings.HasPrefix(s.GetCommand(), "skaffold build") ||
(len(s.Arguments) > 0 && strings.HasPrefix(strings.Join(s.Arguments[1:], " "), "skaffold build")) ||
commandIsSkaffoldRegex.MatchString(s.GetCommand()) {

sourceDir := params.WorkspaceDir
dockerfile := filepath.Join(sourceDir, "Dockerfile")
localRepo := params.DockerRegistry
destination := params.DockerRegistry + "/" + params.DockerRegistryOrg + "/" + params.GitName

args := []string{"--cache=true", "--cache-dir=/workspace",
"--context=" + sourceDir,
"--dockerfile=" + dockerfile,
"--destination=" + destination + ":${inputs.params.version}",
"--cache-repo=" + localRepo + "/" + params.ProjectID + "/cache",
}
if localRepo != "gcr.io" {
args = append(args, "--skip-tls-verify-registry="+localRepo)
}

if ipAddressRegistryRegex.MatchString(localRepo) {
args = append(args, "--insecure")
}

s.Command = "/kaniko/executor"
s.Arguments = args

s.Image = params.KanikoImage
}
}
}

// 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) {
Expand Down
16 changes: 16 additions & 0 deletions pkg/tekton/syntax/zz_generated.deepcopy.go
View file
Open in desktop

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