Skip to content

Commit

Permalink
Apply default values to profiles
Browse files Browse the repository at this point in the history 
Fixes #493

Signed-off-by: David Gageot <[email protected]>
  • Loading branch information
@dgageot
dgageot committed May 11, 2018
1 parent c20f522 commit 87945c0
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 48 deletions.
81 changes: 65 additions & 16 deletions pkg/skaffold/config/profile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ import (

func TestApplyProfiles(t *testing.T) {
tests := []struct {
description string
config SkaffoldConfig
profile string
expectedConfig SkaffoldConfig
shouldErr bool
description string
config SkaffoldConfig
profile string
expected SkaffoldConfig
shouldErr bool
}{
{
description: "unknown profile",
Expand Down Expand Up @@ -61,14 +61,25 @@ func TestApplyProfiles(t *testing.T) {
},
},
},
expectedConfig: SkaffoldConfig{
expected: SkaffoldConfig{
Build: v1alpha2.BuildConfig{
Artifacts: []*v1alpha2.Artifact{
{ImageName: "image"},
{
ImageName: "image",
Workspace: ".",
ArtifactType: v1alpha2.ArtifactType{
DockerArtifact: &v1alpha2.DockerArtifact{
DockerfilePath: "Dockerfile",
},
},
},
},
BuildType: v1alpha2.BuildType{
GoogleCloudBuild: &v1alpha2.GoogleCloudBuild{},
},
TagPolicy: v1alpha2.TagPolicy{
GitTagger: &v1alpha2.GitTagger{},
},
},
Deploy: v1alpha2.DeployConfig{},
},
Expand All @@ -93,12 +104,23 @@ func TestApplyProfiles(t *testing.T) {
},
},
},
expectedConfig: SkaffoldConfig{
expected: SkaffoldConfig{
Build: v1alpha2.BuildConfig{
Artifacts: []*v1alpha2.Artifact{
{ImageName: "image"},
{
ImageName: "image",
Workspace: ".",
ArtifactType: v1alpha2.ArtifactType{
DockerArtifact: &v1alpha2.DockerArtifact{
DockerfilePath: "Dockerfile",
},
},
},
},
TagPolicy: v1alpha2.TagPolicy{ShaTagger: &v1alpha2.ShaTagger{}},
BuildType: v1alpha2.BuildType{
LocalBuild: &v1alpha2.LocalBuild{},
},
},
Deploy: v1alpha2.DeployConfig{},
},
Expand Down Expand Up @@ -126,13 +148,32 @@ func TestApplyProfiles(t *testing.T) {
},
},
},
expectedConfig: SkaffoldConfig{
expected: SkaffoldConfig{
Build: v1alpha2.BuildConfig{
Artifacts: []*v1alpha2.Artifact{
{ImageName: "image"},
{ImageName: "imageProd"},
{
ImageName: "image",
Workspace: ".",
ArtifactType: v1alpha2.ArtifactType{
DockerArtifact: &v1alpha2.DockerArtifact{
DockerfilePath: "Dockerfile",
},
},
},
{
ImageName: "imageProd",
Workspace: ".",
ArtifactType: v1alpha2.ArtifactType{
DockerArtifact: &v1alpha2.DockerArtifact{
DockerfilePath: "Dockerfile",
},
},
},
},
TagPolicy: v1alpha2.TagPolicy{GitTagger: &v1alpha2.GitTagger{}},
BuildType: v1alpha2.BuildType{
LocalBuild: &v1alpha2.LocalBuild{},
},
},
Deploy: v1alpha2.DeployConfig{},
},
Expand All @@ -158,21 +199,29 @@ func TestApplyProfiles(t *testing.T) {
},
},
},
expectedConfig: SkaffoldConfig{
Build: v1alpha2.BuildConfig{},
expected: SkaffoldConfig{
Build: v1alpha2.BuildConfig{
TagPolicy: v1alpha2.TagPolicy{
GitTagger: &v1alpha2.GitTagger{},
},
BuildType: v1alpha2.BuildType{
LocalBuild: &v1alpha2.LocalBuild{},
},
},
Deploy: v1alpha2.DeployConfig{
DeployType: v1alpha2.DeployType{
HelmDeploy: &v1alpha2.HelmDeploy{},
},
},
},
}}
},
}

for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
err := test.config.ApplyProfiles([]string{test.profile})

testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expectedConfig, test.config)
testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, test.config)
})
}
}
72 changes: 40 additions & 32 deletions pkg/skaffold/schema/v1alpha2/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ type SkaffoldConfig struct {
Profiles []Profile `yaml:"profiles,omitempty"`
}

func (config *SkaffoldConfig) GetVersion() string {
return config.APIVersion
func (c *SkaffoldConfig) GetVersion() string {
return c.APIVersion
}

// BuildConfig contains all the configuration for the build steps
Expand Down Expand Up @@ -156,17 +156,38 @@ type BazelArtifact struct {
BuildTarget string `yaml:"target"`
}

func defaultToLocalBuild(config *SkaffoldConfig) {
if config.Build.BuildType != (BuildType{}) {
// Parse reads a SkaffoldConfig from yaml.
func (c *SkaffoldConfig) Parse(contents []byte, useDefaults bool) error {
if err := yaml.Unmarshal(contents, c); err != nil {
return err
}

if useDefaults {
c.setDefaultValues()
}

return nil
}

func (c *SkaffoldConfig) setDefaultValues() {
c.defaultToLocalBuild()
c.defaultToDockerArtifacts()
c.setDefaultTagger()
c.setDefaultDockerfiles()
c.setDefaultWorkspaces()
}

func (c *SkaffoldConfig) defaultToLocalBuild() {
if c.Build.BuildType != (BuildType{}) {
return
}

logrus.Debugf("Defaulting build type to local build")
config.Build.BuildType.LocalBuild = &LocalBuild{}
c.Build.BuildType.LocalBuild = &LocalBuild{}
}

func defaultToDockerArtifacts(config *SkaffoldConfig) {
for _, artifact := range config.Build.Artifacts {
func (c *SkaffoldConfig) defaultToDockerArtifacts() {
for _, artifact := range c.Build.Artifacts {
if artifact.ArtifactType != (ArtifactType{}) {
continue
}
Expand All @@ -177,24 +198,24 @@ func defaultToDockerArtifacts(config *SkaffoldConfig) {
}
}

func setDefaultTagger(cfg *SkaffoldConfig) {
if cfg.Build.TagPolicy != (TagPolicy{}) {
func (c *SkaffoldConfig) setDefaultTagger() {
if c.Build.TagPolicy != (TagPolicy{}) {
return
}

cfg.Build.TagPolicy = TagPolicy{GitTagger: &GitTagger{}}
c.Build.TagPolicy = TagPolicy{GitTagger: &GitTagger{}}
}

func setDefaultDockerfiles(config *SkaffoldConfig) {
for _, artifact := range config.Build.Artifacts {
func (c *SkaffoldConfig) setDefaultDockerfiles() {
for _, artifact := range c.Build.Artifacts {
if artifact.DockerArtifact != nil && artifact.DockerArtifact.DockerfilePath == "" {
artifact.DockerArtifact.DockerfilePath = constants.DefaultDockerfilePath
}
}
}

func setDefaultWorkspaces(config *SkaffoldConfig) {
for _, artifact := range config.Build.Artifacts {
func (c *SkaffoldConfig) setDefaultWorkspaces() {
for _, artifact := range c.Build.Artifacts {
if artifact.Workspace == "" {
artifact.Workspace = "."
}
Expand All @@ -203,23 +224,24 @@ func setDefaultWorkspaces(config *SkaffoldConfig) {

// ApplyProfiles returns configuration modified by the application
// of a list of profiles.
func (config *SkaffoldConfig) ApplyProfiles(profiles []string) error {
func (c *SkaffoldConfig) ApplyProfiles(profiles []string) error {
var err error

byName := profilesByName(config.Profiles)
byName := profilesByName(c.Profiles)
for _, name := range profiles {
profile, present := byName[name]
if !present {
return fmt.Errorf("couldn't find profile %s", name)
}

err = applyProfile(config, profile)
err = applyProfile(c, profile)
if err != nil {
return errors.Wrapf(err, "applying profile %s", name)
}
}

config.Profiles = nil
c.setDefaultValues()
c.Profiles = nil

return nil
}
Expand All @@ -242,17 +264,3 @@ func profilesByName(profiles []Profile) map[string]Profile {
}
return byName
}

func (config *SkaffoldConfig) Parse(contents []byte, useDefaults bool) error {
if err := yaml.Unmarshal(contents, config); err != nil {
return err
}
if useDefaults {
defaultToLocalBuild(config)
defaultToDockerArtifacts(config)
setDefaultTagger(config)
setDefaultDockerfiles(config)
setDefaultWorkspaces(config)
}
return nil
}

0 comments on commit 87945c0

Please sign in to comment.