Skip to content

Commit

Permalink
Make sure upgraded configs have default values applied
Browse files Browse the repository at this point in the history
Signed-off-by: David Gageot <david@gageot.net>
  • Loading branch information
dgageot committed Nov 29, 2018
1 parent b0e748d commit 70fd325
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 17 deletions.
4 changes: 2 additions & 2 deletions cmd/skaffold/app/cmd/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NewCmdFix(out io.Writer) *cobra.Command {
}

func runFix(out io.Writer, configFile string, overwrite bool) error {
cfg, err := schema.ParseConfig(configFile, false, false)
cfg, err := schema.ParseConfig(configFile, false)
if err != nil {
return err
}
Expand All @@ -54,7 +54,7 @@ func runFix(out io.Writer, configFile string, overwrite bool) error {
return nil
}

cfg, err = schema.ParseConfig(configFile, false, true)
cfg, err = schema.ParseConfig(configFile, true)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/skaffold/app/cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func doInit(out io.Writer) error {

for _, file := range potentialConfigs {
if !force {
config, err := schema.ParseConfig(file, true, false)
config, err := schema.ParseConfig(file, false)
if err == nil && config != nil {
return fmt.Errorf("pre-existing %s found", file)
}
Expand Down
6 changes: 5 additions & 1 deletion cmd/skaffold/app/cmd/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,15 @@ import (

// newRunner creates a SkaffoldRunner and returns the SkaffoldPipeline associated with it.
func newRunner(opts *config.SkaffoldOptions) (*runner.SkaffoldRunner, *latest.SkaffoldPipeline, error) {
parsed, err := schema.ParseConfig(opts.ConfigurationFile, true, true)
parsed, err := schema.ParseConfig(opts.ConfigurationFile, true)
if err != nil {
return nil, nil, errors.Wrap(err, "parsing skaffold config")
}

if err := parsed.SetDefaultValues(); err != nil {
return nil, nil, errors.Wrap(err, "setting default values")
}

config := parsed.(*latest.SkaffoldPipeline)
err = schema.ApplyProfiles(config, opts.Profiles)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/schema/v1alpha1/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (config *SkaffoldPipeline) Upgrade() (util.VersionedConfig, error) {

var newHelmDeploy *next.HelmDeploy
if config.Deploy.DeployType.HelmDeploy != nil {
newReleases := make([]next.HelmRelease, 0)
var newReleases []next.HelmRelease
for _, release := range config.Deploy.DeployType.HelmDeploy.Releases {
newReleases = append(newReleases, next.HelmRelease{
Name: release.Name,
Expand All @@ -70,7 +70,7 @@ func (config *SkaffoldPipeline) Upgrade() (util.VersionedConfig, error) {
}
var newKubectlDeploy *next.KubectlDeploy
if config.Deploy.DeployType.KubectlDeploy != nil {
newManifests := make([]string, 0)
var newManifests []string
logrus.Warn("Ignoring manifest parameters when transforming v1alpha1 config; check kubernetes yaml before running skaffold")
for _, manifest := range config.Deploy.DeployType.KubectlDeploy.Manifests {
newManifests = append(newManifests, manifest.Paths...)
Expand All @@ -80,7 +80,7 @@ func (config *SkaffoldPipeline) Upgrade() (util.VersionedConfig, error) {
}
}

var newArtifacts = make([]*next.Artifact, 0)
var newArtifacts []*next.Artifact
for _, artifact := range config.Build.Artifacts {
newArtifacts = append(newArtifacts, &next.Artifact{
ImageName: artifact.ImageName,
Expand Down
2 changes: 0 additions & 2 deletions pkg/skaffold/schema/v1alpha1/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ build:
TagPolicy: next.TagPolicy{
GitTagger: &next.GitTagger{},
},
Artifacts: []*next.Artifact{},
},
},
},
Expand All @@ -62,7 +61,6 @@ build:
TagPolicy: next.TagPolicy{
ShaTagger: &next.ShaTagger{},
},
Artifacts: []*next.Artifact{},
},
},
},
Expand Down
8 changes: 1 addition & 7 deletions pkg/skaffold/schema/versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (v *versions) Find(apiVersion string) (func() util.VersionedConfig, bool) {
}

// ParseConfig reads a configuration file.
func ParseConfig(filename string, applyDefaults bool, upgrade bool) (util.VersionedConfig, error) {
func ParseConfig(filename string, upgrade bool) (util.VersionedConfig, error) {
buf, err := misc.ReadConfiguration(filename)
if err != nil {
return nil, errors.Wrap(err, "read skaffold config")
Expand All @@ -88,12 +88,6 @@ func ParseConfig(filename string, applyDefaults bool, upgrade bool) (util.Versio
return nil, errors.Wrap(err, "unable to parse config")
}

if applyDefaults {
if err := cfg.SetDefaultValues(); err != nil {
return nil, errors.Wrap(err, "setting default values")
}
}

if err := yamltags.ProcessStruct(cfg); err != nil {
return nil, errors.Wrap(err, "invalid config")
}
Expand Down
7 changes: 6 additions & 1 deletion pkg/skaffold/schema/versions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,12 @@ func TestParseConfig(t *testing.T) {
yaml := fmt.Sprintf("apiVersion: %s\nkind: Config\n%s", test.apiVersion, test.config)
tmp.Write("skaffold.yaml", yaml)

cfg, err := ParseConfig(tmp.Path("skaffold.yaml"), true, true)
cfg, err := ParseConfig(tmp.Path("skaffold.yaml"), true)
if cfg != nil {
if err := cfg.SetDefaultValues(); err != nil {
t.Fatal("unable to set default values")
}
}

testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, cfg)
})
Expand Down

0 comments on commit 70fd325

Please sign in to comment.