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.

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

Expand the skaffold init --artifact API to allow specifying artifact context #5000

Merged
merged 5 commits into from
Nov 11, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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 docs/content/en/docs/pipeline-stages/init.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ For multiple artifacts, use `--artifact` multiple times.
```bash
microservices$skaffold init \
MarlonGamez marked this conversation as resolved.
Show resolved Hide resolved
-a '{"builder":"Docker","payload":{"path":"leeroy-app/Dockerfile"},"image":"gcr.io/k8s-skaffold/leeroy-app"}' \
-a '{"builder":"Docker","payload":{"path":"leeroy-web/Dockerfile"},"image":"gcr.io/k8s-skaffold/leeroy-web"}'
-a '{"builder":"Docker","payload":{"path":"leeroy-web/Dockerfile"},"image":"gcr.io/k8s-skaffold/leeroy-web","context":"path/to/context"}'
```

will produce an `skaffold.yaml` config like this
Expand All @@ -178,7 +178,7 @@ build:
- image: gcr.io/k8s-skaffold/leeroy-app
context: leeroy-app
- image: gcr.io/k8s-skaffold/leeroy-web
context: leeroy-web
context: path/to/context
deploy:
kubectl:
manifests:
Expand Down
22 changes: 15 additions & 7 deletions pkg/skaffold/initializer/build/builders_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,11 @@ func TestAutoSelectBuilders(t *testing.T) {

func TestProcessCliArtifacts(t *testing.T) {
tests := []struct {
description string
artifacts []string
shouldErr bool
expectedPairs []BuilderImagePair
description string
artifacts []string
shouldErr bool
expectedPairs []BuilderImagePair
expectedWorkspaces []string
}{
{
description: "Invalid pairs",
Expand Down Expand Up @@ -277,8 +278,8 @@ func TestProcessCliArtifacts(t *testing.T) {
{
description: "Valid",
artifacts: []string{
`{"builder":"Docker","payload":{"path":"/path/to/Dockerfile"},"image":"image1"}`,
`{"builder":"Jib Gradle Plugin","payload":{"path":"/path/to/build.gradle"},"image":"image2"}`,
`{"builder":"Docker","payload":{"path":"/path/to/Dockerfile"},"image":"image1", "context": "path/to/docker/workspace"}`,
`{"builder":"Jib Gradle Plugin","payload":{"path":"/path/to/build.gradle"},"image":"image2", "context":"path/to/jib/workspace"}`,
`{"builder":"Jib Maven Plugin","payload":{"path":"/path/to/pom.xml","project":"project-name","image":"testImage"},"image":"image3"}`,
`{"builder":"Buildpacks","payload":{"path":"/path/to/package.json"},"image":"image4"}`,
},
Expand All @@ -300,14 +301,21 @@ func TestProcessCliArtifacts(t *testing.T) {
ImageName: "image4",
},
},
expectedWorkspaces: []string{
"path/to/docker/workspace",
"path/to/jib/workspace",
"",
"",
},
},
}

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
pairs, err := processCliArtifacts(test.artifacts)
pairs, workspaces, err := processCliArtifacts(test.artifacts)

t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedPairs, pairs)
t.CheckDeepEqual(test.expectedWorkspaces, workspaces)
})
}
}
Expand Down
30 changes: 18 additions & 12 deletions pkg/skaffold/initializer/build/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
type cliBuildInitializer struct {
cliArtifacts []string
builderImagePairs []BuilderImagePair
workspaces []string
builders []InitBuilder
skipBuild bool
enableNewFormat bool
Expand All @@ -49,7 +50,7 @@ func (c *cliBuildInitializer) ProcessImages(images []string) error {

func (c *cliBuildInitializer) BuildConfig() latest.BuildConfig {
return latest.BuildConfig{
Artifacts: Artifacts(c.builderImagePairs),
Artifacts: Artifacts(c.builderImagePairs, c.workspaces),
}
}

Expand All @@ -62,29 +63,32 @@ func (c *cliBuildInitializer) GenerateManifests() (map[GeneratedBuilderImagePair
}

func (c *cliBuildInitializer) processCliArtifacts() error {
pairs, err := processCliArtifacts(c.cliArtifacts)
pairs, workspaces, err := processCliArtifacts(c.cliArtifacts)
if err != nil {
return err
}
c.builderImagePairs = pairs
c.workspaces = workspaces
return nil
}

func processCliArtifacts(cliArtifacts []string) ([]BuilderImagePair, error) {
func processCliArtifacts(cliArtifacts []string) ([]BuilderImagePair, []string, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to not include the workspace in the BuilderImagePair?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer to do this too. The current way that I did things make it more bloated. I'll work on this change.

var pairs []BuilderImagePair
var workspaces []string
for _, artifact := range cliArtifacts {
// Parses JSON in the form of: {"builder":"Name of Builder","payload":{...},"image":"image.name"}.
// Parses JSON in the form of: {"builder":"Name of Builder","payload":{...},"image":"image.name","context":"artifact.context"}.
// The builder field is parsed first to determine the builder type, and the payload is parsed
// afterwards once the type is determined.
MarlonGamez marked this conversation as resolved.
Show resolved Hide resolved
a := struct {
Name string `json:"builder"`
Image string `json:"image"`
Name string `json:"builder"`
Image string `json:"image"`
Workspace string `json:"context"`
}{}
if err := json.Unmarshal([]byte(artifact), &a); err != nil {
// Not JSON, use backwards compatible method
parts := strings.Split(artifact, "=")
if len(parts) != 2 {
return nil, fmt.Errorf("malformed artifact provided: %s", artifact)
return nil, nil, fmt.Errorf("malformed artifact provided: %s", artifact)
}
pairs = append(pairs, BuilderImagePair{
Builder: docker.ArtifactConfig{File: parts[0]},
Expand All @@ -100,7 +104,7 @@ func processCliArtifacts(cliArtifacts []string) ([]BuilderImagePair, error) {
Payload docker.ArtifactConfig `json:"payload"`
}{}
if err := json.Unmarshal([]byte(artifact), &parsed); err != nil {
return nil, err
return nil, nil, err
}
pair := BuilderImagePair{Builder: parsed.Payload, ImageName: a.Image}
pairs = append(pairs, pair)
Expand All @@ -111,7 +115,7 @@ func processCliArtifacts(cliArtifacts []string) ([]BuilderImagePair, error) {
Payload jib.ArtifactConfig `json:"payload"`
}{}
if err := json.Unmarshal([]byte(artifact), &parsed); err != nil {
return nil, err
return nil, nil, err
}
parsed.Payload.BuilderName = a.Name
pair := BuilderImagePair{Builder: parsed.Payload, ImageName: a.Image}
Expand All @@ -122,14 +126,16 @@ func processCliArtifacts(cliArtifacts []string) ([]BuilderImagePair, error) {
Payload buildpacks.ArtifactConfig `json:"payload"`
}{}
if err := json.Unmarshal([]byte(artifact), &parsed); err != nil {
return nil, err
return nil, nil, err
}
pair := BuilderImagePair{Builder: parsed.Payload, ImageName: a.Image}
pairs = append(pairs, pair)

default:
return nil, fmt.Errorf("unknown builder type in CLI artifacts: %q", a.Name)
return nil, nil, fmt.Errorf("unknown builder type in CLI artifacts: %q", a.Name)
}

workspaces = append(workspaces, a.Workspace)
}
return pairs, nil
return pairs, workspaces, nil
}
2 changes: 1 addition & 1 deletion pkg/skaffold/initializer/build/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (d *defaultBuildInitializer) ProcessImages(images []string) error {

func (d *defaultBuildInitializer) BuildConfig() latest.BuildConfig {
return latest.BuildConfig{
Artifacts: Artifacts(d.builderImagePairs),
Artifacts: Artifacts(d.builderImagePairs, nil),
}
}

Expand Down
13 changes: 10 additions & 3 deletions pkg/skaffold/initializer/build/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,23 @@ func findExactlyOneMatchingBuilder(builderConfigs []InitBuilder, image string) i
return matchingConfigIndex
}

func Artifacts(pairs []BuilderImagePair) []*latest.Artifact {
// Artifacts takes builder image pairs and workspaces and creates a list of latest.Artifacts from the data.
func Artifacts(pairs []BuilderImagePair, workspaces []string) []*latest.Artifact {
var artifacts []*latest.Artifact

for _, pair := range pairs {
for i, pair := range pairs {
artifact := &latest.Artifact{
ImageName: pair.ImageName,
ArtifactType: pair.Builder.ArtifactType(),
}

workspace := filepath.Dir(pair.Builder.Path())
var workspace string
if workspaces != nil {
workspace = workspaces[i]
}
if workspace == "" {
workspace = filepath.Dir(pair.Builder.Path())
}
if workspace != "." {
fmt.Fprintf(os.Stdout, "using non standard workspace: %s\n", workspace)
artifact.Workspace = workspace
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/initializer/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (s stubBuildInitializer) PrintAnalysis(io.Writer) error {

func (s stubBuildInitializer) BuildConfig() latest.BuildConfig {
return latest.BuildConfig{
Artifacts: build.Artifacts(s.pairs),
Artifacts: build.Artifacts(s.pairs, nil),
}
}

Expand Down