Skip to content

Commit cba156b

Browse files
gsquared94gsquared94
committed
add source file and module to config parsing error description
1 parent 36803b4 commit cba156b

File tree

5 files changed

+170
-128
lines changed

5 files changed

+170
-128
lines changed

‎cmd/skaffold/app/cmd/fix.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/spf13/cobra"
2626

2727
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/output"
28+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/parser"
2829
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema"
2930
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
3031
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/validation"
@@ -81,9 +82,12 @@ func fix(out io.Writer, configFile string, toVersion string, overwrite bool) err
8182
// TODO(dgageot): We should be able run validations on any schema version
8283
// but that's not the case. They can only run on the latest version for now.
8384
if toVersion == latestV1.Version {
84-
var cfgs []*latestV1.SkaffoldConfig
85+
var cfgs parser.SkaffoldConfigSet
8586
for _, cfg := range versionedCfgs {
86-
cfgs = append(cfgs, cfg.(*latestV1.SkaffoldConfig))
87+
cfgs = append(cfgs, &parser.SkaffoldConfigEntry{
88+
SkaffoldConfig: cfg.(*latestV1.SkaffoldConfig),
89+
SourceFile: configFile,
90+
IsRootConfig: true})
8791
}
8892
if err := validation.Process(cfgs); err != nil {
8993
return fmt.Errorf("validating upgraded config: %w", err)

‎cmd/skaffold/app/cmd/runner.go

+13-8
Original file line numberDiff line numberDiff line change
@@ -73,16 +73,19 @@ func createNewRunner(out io.Writer, opts config.SkaffoldOptions) (runner.Runner,
7373
}
7474

7575
func runContext(out io.Writer, opts config.SkaffoldOptions) (*runcontext.RunContext, []*latestV1.SkaffoldConfig, error) {
76-
configs, err := withFallbackConfig(out, opts, parser.GetAllConfigs)
76+
cfgSet, err := withFallbackConfig(out, opts, parser.GetConfigSet)
7777
if err != nil {
7878
return nil, nil, err
7979
}
80-
setDefaultDeployer(configs)
80+
setDefaultDeployer(cfgSet)
8181

82-
if err := validation.Process(configs); err != nil {
82+
if err := validation.Process(cfgSet); err != nil {
8383
return nil, nil, fmt.Errorf("invalid skaffold config: %w", err)
8484
}
85-
85+
var configs []*latestV1.SkaffoldConfig
86+
for _, cfg := range cfgSet {
87+
configs = append(configs, cfg.SkaffoldConfig)
88+
}
8689
runCtx, err := runcontext.GetRunContext(opts, configs)
8790
if err != nil {
8891
return nil, nil, fmt.Errorf("getting run context: %w", err)
@@ -96,7 +99,7 @@ func runContext(out io.Writer, opts config.SkaffoldOptions) (*runcontext.RunCont
9699
}
97100

98101
// withFallbackConfig will try to automatically generate a config if root `skaffold.yaml` file does not exist.
99-
func withFallbackConfig(out io.Writer, opts config.SkaffoldOptions, getCfgs func(opts config.SkaffoldOptions) ([]*latestV1.SkaffoldConfig, error)) ([]*latestV1.SkaffoldConfig, error) {
102+
func withFallbackConfig(out io.Writer, opts config.SkaffoldOptions, getCfgs func(opts config.SkaffoldOptions) (parser.SkaffoldConfigSet, error)) (parser.SkaffoldConfigSet, error) {
100103
configs, err := getCfgs(opts)
101104
if err == nil {
102105
return configs, nil
@@ -115,7 +118,9 @@ func withFallbackConfig(out io.Writer, opts config.SkaffoldOptions, getCfgs func
115118

116119
defaults.Set(config)
117120

118-
return []*latestV1.SkaffoldConfig{config}, nil
121+
return parser.SkaffoldConfigSet{
122+
&parser.SkaffoldConfigEntry{SkaffoldConfig: config, IsRootConfig: true},
123+
}, nil
119124
}
120125

121126
return nil, fmt.Errorf("skaffold config file %s not found - check your current working directory, or try running `skaffold init`", opts.ConfigurationFile)
@@ -128,13 +133,13 @@ func withFallbackConfig(out io.Writer, opts config.SkaffoldOptions, getCfgs func
128133
return nil, fmt.Errorf("parsing skaffold config: %w", err)
129134
}
130135

131-
func setDefaultDeployer(configs []*latestV1.SkaffoldConfig) {
136+
func setDefaultDeployer(configs parser.SkaffoldConfigSet) {
132137
// do not set a default deployer in a multi-config application.
133138
if len(configs) > 1 {
134139
return
135140
}
136141
// there always exists at least one config
137-
defaults.SetDefaultDeployer(configs[0])
142+
defaults.SetDefaultDeployer(configs[0].SkaffoldConfig)
138143
}
139144

140145
func warnIfUpdateIsAvailable() {

‎pkg/skaffold/schema/samples_test.go renamed to ‎pkg/skaffold/schema/validation/samples_test.go

+13-12
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package schema
17+
package validation
1818

1919
import (
2020
"bytes"
@@ -24,16 +24,17 @@ import (
2424
"strings"
2525
"testing"
2626

27+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/parser"
28+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema"
2729
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/defaults"
2830
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
29-
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/validation"
3031
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
3132
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/walk"
3233
"github.com/GoogleContainerTools/skaffold/testutil"
3334
)
3435

3536
const (
36-
samplesRoot = "../../../docs/content/en/samples"
37+
samplesRoot = "../../../../docs/content/en/samples"
3738
)
3839

3940
var (
@@ -43,9 +44,9 @@ var (
4344
// Test that every example can be parsed and produces a valid
4445
// Skaffold configuration.
4546
func TestParseExamples(t *testing.T) {
46-
parseConfigFiles(t, "../../../examples")
47-
parseConfigFiles(t, "../../../integration/examples")
48-
parseConfigFiles(t, "../../../integration/testdata/regressions")
47+
parseConfigFiles(t, "../../../../examples")
48+
parseConfigFiles(t, "../../../../integration/examples")
49+
parseConfigFiles(t, "../../../../integration/testdata/regressions")
4950
}
5051

5152
// Samples are skaffold.yaml fragments that are used
@@ -77,17 +78,17 @@ func TestParseSamples(t *testing.T) {
7778

7879
func checkSkaffoldConfig(t *testutil.T, yaml []byte) {
7980
configFile := t.TempFile("skaffold.yaml", yaml)
80-
parsed, err := ParseConfigAndUpgrade(configFile)
81+
parsed, err := schema.ParseConfigAndUpgrade(configFile)
8182
t.CheckNoError(err)
82-
var cfgs []*latestV1.SkaffoldConfig
83+
var cfgs parser.SkaffoldConfigSet
8384
for _, p := range parsed {
84-
cfg := p.(*latestV1.SkaffoldConfig)
85-
err = defaults.Set(cfg)
86-
defaults.SetDefaultDeployer(cfg)
85+
cfg := &parser.SkaffoldConfigEntry{SkaffoldConfig: p.(*latestV1.SkaffoldConfig)}
86+
err = defaults.Set(cfg.SkaffoldConfig)
87+
defaults.SetDefaultDeployer(cfg.SkaffoldConfig)
8788
t.CheckNoError(err)
8889
cfgs = append(cfgs, cfg)
8990
}
90-
err = validation.Process(cfgs)
91+
err = Process(cfgs)
9192
t.CheckNoError(err)
9293
}
9394

‎pkg/skaffold/schema/validation/validation.go

+38-22
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,19 @@ package validation
1818

1919
import (
2020
"context"
21-
"errors"
2221
"fmt"
2322
"reflect"
2423
"regexp"
2524
"strings"
2625
"time"
2726

2827
"github.com/docker/docker/api/types"
28+
"github.com/pkg/errors"
2929

3030
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/misc"
3131
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
3232
sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
33+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/parser"
3334
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
3435
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
3536
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
@@ -44,19 +45,21 @@ var (
4445
)
4546

4647
// Process checks if the Skaffold pipeline is valid and returns all encountered errors as a concatenated string
47-
func Process(configs []*latestV1.SkaffoldConfig) error {
48+
func Process(configs parser.SkaffoldConfigSet) error {
4849
var errs = validateImageNames(configs)
4950
for _, config := range configs {
50-
errs = append(errs, visitStructs(config, validateYamltags)...)
51-
errs = append(errs, validateDockerNetworkMode(config.Build.Artifacts)...)
52-
errs = append(errs, validateCustomDependencies(config.Build.Artifacts)...)
53-
errs = append(errs, validateSyncRules(config.Build.Artifacts)...)
54-
errs = append(errs, validatePortForwardResources(config.PortForward)...)
55-
errs = append(errs, validateJibPluginTypes(config.Build.Artifacts)...)
56-
errs = append(errs, validateLogPrefix(config.Deploy.Logs)...)
57-
errs = append(errs, validateArtifactTypes(config.Build)...)
58-
errs = append(errs, validateTaggingPolicy(config.Build)...)
59-
errs = append(errs, validateCustomTest(config.Test)...)
51+
var cfgErrs []error
52+
cfgErrs = append(cfgErrs, visitStructs(config.SkaffoldConfig, validateYamltags)...)
53+
cfgErrs = append(cfgErrs, validateDockerNetworkMode(config.Build.Artifacts)...)
54+
cfgErrs = append(cfgErrs, validateCustomDependencies(config.Build.Artifacts)...)
55+
cfgErrs = append(cfgErrs, validateSyncRules(config.Build.Artifacts)...)
56+
cfgErrs = append(cfgErrs, validatePortForwardResources(config.PortForward)...)
57+
cfgErrs = append(cfgErrs, validateJibPluginTypes(config.Build.Artifacts)...)
58+
cfgErrs = append(cfgErrs, validateLogPrefix(config.Deploy.Logs)...)
59+
cfgErrs = append(cfgErrs, validateArtifactTypes(config.Build)...)
60+
cfgErrs = append(cfgErrs, validateTaggingPolicy(config.Build)...)
61+
cfgErrs = append(cfgErrs, validateCustomTest(config.Test)...)
62+
errs = append(errs, wrapWithContext(config, cfgErrs...)...)
6063
}
6164
errs = append(errs, validateArtifactDependencies(configs)...)
6265
errs = append(errs, validateSingleKubeContext(configs)...)
@@ -100,35 +103,35 @@ func validateTaggingPolicy(bc latestV1.BuildConfig) (errs []error) {
100103

101104
// validateImageNames makes sure the artifact image names are unique and valid base names,
102105
// without tags nor digests.
103-
func validateImageNames(configs []*latestV1.SkaffoldConfig) (errs []error) {
104-
seen := make(map[string]bool)
106+
func validateImageNames(configs parser.SkaffoldConfigSet) (errs []error) {
107+
seen := make(map[string]string)
105108
for _, c := range configs {
106109
for _, a := range c.Build.Artifacts {
107-
if seen[a.ImageName] {
108-
errs = append(errs, fmt.Errorf("found duplicate images %q: artifact image names must be unique across all configurations", a.ImageName))
110+
if prevSource, found := seen[a.ImageName]; found {
111+
errs = append(errs, fmt.Errorf("duplicate image %q found in sources %s and %s: artifact image names must be unique across all configurations", a.ImageName, prevSource, c.SourceFile))
109112
continue
110113
}
111114

112-
seen[a.ImageName] = true
115+
seen[a.ImageName] = c.SourceFile
113116
parsed, err := docker.ParseReference(a.ImageName)
114117
if err != nil {
115-
errs = append(errs, fmt.Errorf("invalid image %q: %w", a.ImageName, err))
118+
errs = append(errs, wrapWithContext(c, fmt.Errorf("invalid image %q: %w", a.ImageName, err))...)
116119
continue
117120
}
118121

119122
if parsed.Tag != "" {
120-
errs = append(errs, fmt.Errorf("invalid image %q: no tag should be specified. Use taggers instead: https://skaffold.dev/docs/how-tos/taggers/", a.ImageName))
123+
errs = append(errs, wrapWithContext(c, fmt.Errorf("invalid image %q: no tag should be specified. Use taggers instead: https://skaffold.dev/docs/how-tos/taggers/", a.ImageName))...)
121124
}
122125

123126
if parsed.Digest != "" {
124-
errs = append(errs, fmt.Errorf("invalid image %q: no digest should be specified. Use taggers instead: https://skaffold.dev/docs/how-tos/taggers/", a.ImageName))
127+
errs = append(errs, wrapWithContext(c, fmt.Errorf("invalid image %q: no digest should be specified. Use taggers instead: https://skaffold.dev/docs/how-tos/taggers/", a.ImageName))...)
125128
}
126129
}
127130
}
128131
return
129132
}
130133

131-
func validateArtifactDependencies(configs []*latestV1.SkaffoldConfig) (errs []error) {
134+
func validateArtifactDependencies(configs parser.SkaffoldConfigSet) (errs []error) {
132135
var artifacts []*latestV1.Artifact
133136
for _, c := range configs {
134137
artifacts = append(artifacts, c.Build.Artifacts...)
@@ -528,7 +531,7 @@ func validateLogPrefix(lc latestV1.LogsConfig) []error {
528531
return nil
529532
}
530533

531-
func validateSingleKubeContext(configs []*latestV1.SkaffoldConfig) []error {
534+
func validateSingleKubeContext(configs parser.SkaffoldConfigSet) []error {
532535
if len(configs) < 2 {
533536
return nil
534537
}
@@ -565,3 +568,16 @@ func validateCustomTest(tcs []*latestV1.TestCase) (errs []error) {
565568
}
566569
return
567570
}
571+
572+
func wrapWithContext(config *parser.SkaffoldConfigEntry, errs ...error) []error {
573+
var id string
574+
if config.Metadata.Name != "" {
575+
id = fmt.Sprintf("module %q", config.Metadata.Name)
576+
} else {
577+
id = fmt.Sprintf("unnamed config at index %d", config.SourceIndex)
578+
}
579+
for i := range errs {
580+
errs[i] = errors.Wrapf(errs[i], "source: %s, %s", config.SourceFile, id)
581+
}
582+
return errs
583+
}

0 commit comments

Comments
 (0)