Skip to content

Commit

Permalink
add exists validation to kubectl manifests specified in skaffold config
Browse files Browse the repository at this point in the history
  • Loading branch information
aaron-prindle committed Jun 28, 2021
1 parent 9cd80d3 commit 590b650
Show file tree
Hide file tree
Showing 15 changed files with 335 additions and 217 deletions.
45 changes: 30 additions & 15 deletions docs/content/en/api/skaffold.swagger.json

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions docs/content/en/docs/references/api/grpc.md
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,7 @@ For Cancelled Error code, use range 800 to 850.<br>
| CONFIG_UNKNOWN_API_VERSION_ERR | 1213 | Config API version not found |
| CONFIG_UNKNOWN_VALIDATOR | 1214 | The validator is not allowed in skaffold-managed mode. |
| CONFIG_UNKNOWN_TRANSFORMER | 1215 | The transformer is not allowed in skaffold-managed mode. |
| CONFIG_MISSING_MANIFEST_FILE_ERR | 1216 | Manifest file not found |
| INSPECT_UNKNOWN_ERR | 1301 | Catch-all `skaffold inspect` command error |
| INSPECT_BUILD_ENV_ALREADY_EXISTS_ERR | 1302 | Trying to add new build environment that already exists |
| INSPECT_BUILD_ENV_INCORRECT_TYPE_ERR | 1303 | Trying to modify build environment that doesn't exist |
Expand Down Expand Up @@ -1087,6 +1088,7 @@ Enum for Suggestion codes
| CONFIG_FIX_API_VERSION | 707 | Fix config API version or upgrade the skaffold binary |
| CONFIG_ALLOWLIST_VALIDATORS | 708 | Only the allow listed validators are acceptable in skaffold-managed mode. |
| CONFIG_ALLOWLIST_transformers | 709 | Only the allow listed transformers are acceptable in skaffold-managed mode. |
| CONFIG_FIX_MISSING_MANIFEST_FILE | 710 | Check mising manifest file section of config and fix as needed. |
| INSPECT_USE_MODIFY_OR_NEW_PROFILE | 800 | Create new build env in a profile instead, or use the 'modify' command |
| INSPECT_USE_ADD_BUILD_ENV | 801 | Check profile selection, or use the 'add' command instead |
| INSPECT_CHECK_INPUT_PROFILE | 802 | Check profile flag value |
Expand Down
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
51 changes: 41 additions & 10 deletions pkg/skaffold/parser/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,16 @@ func GetAllConfigs(opts config.SkaffoldOptions) ([]*latestV1.SkaffoldConfig, err
return cfgs, nil
}

// GetBase returns the correct base address (work dir) for a given config file
func GetBase(isDependency bool, cfgFile string) (string, error) {
if isDependency {
logrus.Tracef("found %s base dir for absolute path substitution within skaffold config %s", filepath.Dir(cfgFile), cfgFile)
return filepath.Dir(cfgFile), nil
}
logrus.Tracef("found cwd as base for absolute path substitution within skaffold config %s", cfgFile)
return util.RealWorkDir()
}

// GetConfigSet returns the list of all skaffold configurations parsed from the target config file in addition to all resolved dependency configs as a `SkaffoldConfigSet`.
// This struct additionally contains the file location that each skaffold configuration is parsed from.
func GetConfigSet(opts config.SkaffoldOptions) (SkaffoldConfigSet, error) {
Expand Down Expand Up @@ -129,6 +139,36 @@ func getConfigs(cfgOpts configOpts, opts config.SkaffoldOptions, r *record) (Ska
seen[cfgName] = true
}

// // validate that manifest files referenced in config exist
// for _, cfg := range parsed {
// if cfg.(*latestV1.SkaffoldConfig).Deploy.KubectlDeploy == nil {
// break
// }
// manifests := cfg.(*latestV1.SkaffoldConfig).Deploy.KubectlDeploy.Manifests
// // TODO(6050) validate for other deploy types - helm, kpt, etc.
// for _, pattern := range manifests {
// if util.IsURL(pattern) {
// continue
// }
// // handle absolute manifest paths
// base := ""
// if !filepath.IsAbs(pattern) {
// base, err = getBase(cfgOpts)
// if err != nil {
// return nil, err
// }
// }

// expanded, err := filepath.Glob(filepath.Join(base, pattern))
// if err != nil {
// return nil, err
// }
// if len(expanded) == 0 {
// return nil, sErrors.ConfigHasMissingManifestFileErr(cfgOpts.file, pattern, base)
// }
// }
// }

var configs SkaffoldConfigSet
for i, cfg := range parsed {
config := cfg.(*latestV1.SkaffoldConfig)
Expand Down Expand Up @@ -176,7 +216,7 @@ func processEachConfig(config *latestV1.SkaffoldConfig, cfgOpts configOpts, opts
// if `opts.MakePathsAbsolute` is set, use that as condition to decide on making file paths absolute for all configs or none at all.
// This is used when the parsed config is marshalled out (for commands like `skaffold diagnose` or `skaffold inspect`), we want to retain the original relative paths in the output files.
if isMakePathsAbsoluteSet(opts) || (opts.MakePathsAbsolute == nil && cfgOpts.isDependency) {
base, err := getBase(cfgOpts)
base, err := GetBase(cfgOpts.isDependency, cfgOpts.file)
if err != nil {
return nil, sErrors.ConfigSetAbsFilePathsErr(config.Metadata.Name, cfgOpts.file, err)
}
Expand Down Expand Up @@ -359,15 +399,6 @@ func isMakePathsAbsoluteSet(opts config.SkaffoldOptions) bool {
return opts.MakePathsAbsolute != nil && (*opts.MakePathsAbsolute)
}

func getBase(cfgOpts configOpts) (string, error) {
if cfgOpts.isDependency {
logrus.Tracef("found %s base dir for absolute path substitution within skaffold config %s", filepath.Dir(cfgOpts.file), cfgOpts.file)
return filepath.Dir(cfgOpts.file), nil
}
logrus.Tracef("found cwd as base for absolute path substitution within skaffold config %s", cfgOpts.file)
return util.RealWorkDir()
}

func isRemoteConfig(file string, opts config.SkaffoldOptions) (bool, error) {
dir, err := git.GetRepoCacheDir(opts)
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions pkg/skaffold/schema/errors/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ func TestErrors(t *testing.T) {
errCode: proto.StatusCode_CONFIG_MULTI_IMPORT_PROFILE_CONFLICT_ERR,
suggestionCode: proto.SuggestionCode_CONFIG_CHECK_DEPENDENCY_PROFILES_SELECTION,
},
{
err: ConfigHasMissingManifestFileErr("", "", ""),
errCode: proto.StatusCode_CONFIG_MISSING_MANIFEST_FILE_ERR,
suggestionCode: proto.SuggestionCode_CONFIG_FIX_MISSING_MANIFEST_FILE,
},
}

for i, test := range tests {
Expand Down
48 changes: 48 additions & 0 deletions pkg/skaffold/schema/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package validation
import (
"context"
"fmt"
"path/filepath"
"reflect"
"regexp"
"strings"
Expand Down Expand Up @@ -63,6 +64,8 @@ func Process(configs parser.SkaffoldConfigSet) error {
}
errs = append(errs, validateArtifactDependencies(configs)...)
errs = append(errs, validateSingleKubeContext(configs)...)
// TODO(6050) validate for other deploy types - helm, kpt, etc.
errs = append(errs, validateKubectlManifests(configs)...)
if len(errs) == 0 {
return nil
}
Expand Down Expand Up @@ -581,3 +584,48 @@ func wrapWithContext(config *parser.SkaffoldConfigEntry, errs ...error) []error
}
return errs
}

// validateKubectlManifests
// - validates that kubectl manifest files specified in the skaffold config exist
func validateKubectlManifests(configs parser.SkaffoldConfigSet) (errs []error) {
for _, c := range configs {
if c.IsRemote {
continue
}
// validate that manifest files referenced in config exist
for _, pattern := range c.Deploy.KubectlDeploy.Manifests {
if util.IsURL(pattern) {
continue
}
// handle absolute manifest paths
base := ""
var err error
if !filepath.IsAbs(pattern) {
base, err = parser.GetBase(!c.IsRootConfig, c.SourceFile)
if err != nil {
errs = append(errs, err)
}
}

expanded, err := filepath.Glob(filepath.Join(base, pattern))
if err != nil {
errs = append(errs, err)
}
if len(expanded) == 0 {
msg := fmt.Sprintf("skaffold config named %q referenced file %q that could not be found", c.SourceFile, pattern)
errs = append(errs, sErrors.NewError(fmt.Errorf(msg),
proto.ActionableErr{
Message: msg,
ErrCode: proto.StatusCode_CONFIG_MISSING_MANIFEST_FILE_ERR,
Suggestions: []*proto.Suggestion{
{
SuggestionCode: proto.SuggestionCode_CONFIG_FIX_MISSING_MANIFEST_FILE,
Action: fmt.Sprintf("Verify that file %q referenced in config %q exists at %q and the path and naming are correct", pattern, c.SourceFile, filepath.Join(base, pattern)),
},
},
}))
}
}
}
return
}
Loading

0 comments on commit 590b650

Please sign in to comment.