Skip to content

Commit

Permalink
WIP render should validate manifests exist
Browse files Browse the repository at this point in the history
  • Loading branch information
aaron-prindle committed Jun 21, 2021
1 parent 7f754f6 commit 7e18335
Show file tree
Hide file tree
Showing 9 changed files with 294 additions and 206 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 @@ -1018,6 +1018,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 @@ -1086,6 +1087,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
31 changes: 31 additions & 0 deletions pkg/skaffold/parser/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,37 @@ 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 manifest 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
17 changes: 17 additions & 0 deletions pkg/skaffold/schema/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package errors

import (
"fmt"
"path/filepath"

sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
"github.com/GoogleContainerTools/skaffold/proto/v1"
Expand Down Expand Up @@ -200,3 +201,19 @@ func ConfigUnknownAPIVersionErr(version string) error {
},
})
}

// ConfigHasMissingManifestFileErr specifies that a manifest files mentioned in the config could not be found
func ConfigHasMissingManifestFileErr(config, file, cwd string) error {
msg := fmt.Sprintf("skaffold config named %q referenced file %q that could not be found", config, file)
return 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", file, config, filepath.Join(cwd, file)),
},
},
})
}
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
392 changes: 201 additions & 191 deletions proto/enums/enums.pb.go

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions proto/enums/enums.proto
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,8 @@ enum StatusCode {
CONFIG_UNKNOWN_VALIDATOR = 1214;
// The transformer is not allowed in skaffold-managed mode.
CONFIG_UNKNOWN_TRANSFORMER = 1215;
// Manifest file not found
CONFIG_MISSING_MANIFEST_FILE_ERR = 1216;

// Inspect command errors

Expand Down Expand Up @@ -579,6 +581,8 @@ enum SuggestionCode {
CONFIG_ALLOWLIST_VALIDATORS = 708;
// Only the allow listed transformers are acceptable in skaffold-managed mode.
CONFIG_ALLOWLIST_transformers = 709;
// Check mising manifest file section of config and fix as needed.
CONFIG_FIX_MISSING_MANIFEST_FILE = 710;

// `skaffold inspect` command error suggestion codes

Expand Down
2 changes: 2 additions & 0 deletions proto/v1/skaffold.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions proto/v2/skaffold.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 7e18335

Please sign in to comment.