Skip to content

Commit

Permalink
[v3] Add proto for render error code and suggestions. (#5953)
Browse files Browse the repository at this point in the history
  • Loading branch information
yuwenma authored Jun 11, 2021
1 parent f8642e6 commit 59edbda
Show file tree
Hide file tree
Showing 10 changed files with 498 additions and 224 deletions.
137 changes: 120 additions & 17 deletions docs/content/en/api/skaffold.swagger.json

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions docs/content/en/docs/references/api/grpc.md
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,7 @@ Enum indicating deploy tools used
| HELM | 1 | Helm Deployer |
| KUSTOMIZE | 2 | Kustomize Deployer |
| KUBECTL | 3 | Kubectl Deployer |
| KPT | 4 | kpt Deployer |



Expand Down Expand Up @@ -878,6 +879,7 @@ For Cancelled Error code, use range 800 to 850.<br>
| OK | 0 | A default status code for events that do not have an associated phase. Typically seen with the DevEndEvent event on success. |
| STATUSCHECK_SUCCESS | 200 | Status Check Success |
| BUILD_SUCCESS | 201 | Build Success |
| RENDER_SUCCESS | 204 | Render Success |
| DEPLOY_SUCCESS | 202 | Deploy Success |
| TEST_SUCCESS | 203 | Test Success |
| BUILD_PUSH_ACCESS_DENIED | 101 | Build error due to push access denied |
Expand Down Expand Up @@ -984,6 +986,7 @@ For Cancelled Error code, use range 800 to 850.<br>
| DEPLOY_PARSE_MANIFEST_IMAGES_ERR | 1021 | Error getting images from a kubernetes manifest. |
| DEPLOY_HELM_CREATE_NS_NOT_AVAILABLE | 1022 | Helm config `createNamespace` not available |
| DEPLOY_CLUSTER_INTERNAL_SYSTEM_ERR | 1023 | Kubernetes cluster reported an internal system error |
| DEPLOY_KPTFILE_INIT_ERR | 1024 | The Kptfile cannot be created via `kpt live init`. |
| TEST_USER_CONFIG_ERR | 1101 | Error expanding paths |
| TEST_CST_USER_ERR | 1102 | Error running container-structure-test |
| TEST_IMG_PULL_ERR | 1103 | Unable to docker pull image |
Expand All @@ -997,6 +1000,9 @@ For Cancelled Error code, use range 800 to 850.<br>
| TEST_CUSTOM_DEPENDENCIES_CMD_ERR | 1112 | Error getting dependencies from command |
| TEST_CUSTOM_DEPENDENCIES_UNMARSHALL_ERR | 1113 | Unmarshalling dependency output error |
| TEST_CUSTOM_CMD_RETRIEVE_ERR | 1114 | Error retrieving the command |
| RENDER_KPTFILE_INIT_ERR | 1501 | Render errors The Kptfile cannot be created via `kpt pkg init`. |
| RENDER_KPTFILE_INVALID_YAML_ERR | 1401 | The Kptfile is not a valid yaml file |
| RENDER_KPTFILE_INVALID_SCHEMA_ERR | 1402 | The Kptfile is not a valid API schema |
| CONFIG_FILE_PARSING_ERR | 1201 | Catch-all configuration file parsing error |
| CONFIG_FILE_NOT_FOUND_ERR | 1202 | Main configuration file not found |
| CONFIG_DEPENDENCY_NOT_FOUND_ERR | 1203 | Dependency configuration file not found |
Expand All @@ -1010,6 +1016,8 @@ For Cancelled Error code, use range 800 to 850.<br>
| CONFIG_MULTI_IMPORT_PROFILE_CONFLICT_ERR | 1211 | Same config imported at least twice with different set of profiles |
| CONFIG_PROFILES_NOT_FOUND_ERR | 1212 | Profile selection did not match known profile names |
| 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. |
| 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 @@ -1066,6 +1074,8 @@ Enum for Suggestion codes
| UNPAUSE_MINIKUBE | 502 | Minikube is paused: use `minikube unpause` |
| RUN_DOCKER_PULL | 551 | Run Docker pull for the image with v1 manifest and try again. |
| SET_RENDER_FLAG_OFFLINE_FALSE | 600 | Rerun with correct offline flag value. |
| KPTFILE_MANUAL_INIT | 601 | Manually run `kpt pkg init` or `kpt live init` |
| KPTFILE_CHECK_YAML | 602 | Check if the Kptfile is correct. |
| CONFIG_CHECK_FILE_PATH | 700 | Check configuration file path |
| CONFIG_CHECK_DEPENDENCY_DEFINITION | 701 | Check dependency config definition |
| CONFIG_CHANGE_NAMES | 702 | Change config name to avoid duplicates |
Expand All @@ -1074,6 +1084,8 @@ Enum for Suggestion codes
| CONFIG_CHECK_DEPENDENCY_PROFILES_SELECTION | 705 | Check active profile selection for dependency config |
| CONFIG_CHECK_PROFILE_SELECTION | 706 | Check profile selection flag |
| 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. |
| 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
32 changes: 26 additions & 6 deletions pkg/skaffold/render/renderer/renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ import (
"github.com/sirupsen/logrus"
"gopkg.in/yaml.v2"

sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/render/generate"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/render/kptfile"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/render/validate"
latestV2 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v2"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/proto/v1"
)

const (
Expand Down Expand Up @@ -71,6 +73,7 @@ func NewSkaffoldRenderer(config *latestV2.RenderConfig, workingDir string) (Rend
} else {
validator, _ = validate.NewValidator([]latestV2.Validator{})
}

return &SkaffoldRenderer{Generator: *generator, Validator: *validator, workingDir: workingDir, hydrationDir: hydrationDir}, nil
}

Expand All @@ -97,9 +100,17 @@ func (r *SkaffoldRenderer) prepareHydrationDir(ctx context.Context) error {
if _, err := os.Stat(kptFilePath); os.IsNotExist(err) {
cmd := exec.CommandContext(ctx, "kpt", "pkg", "init", r.hydrationDir)
if _, err := util.RunCmdOut(cmd); err != nil {
// TODO: user error. need manual init
return fmt.Errorf("unable to initialize kpt directory in %v, please manually run `kpt pkg init %v`",
kptFilePath, kptFilePath)
return sErrors.NewError(err,
proto.ActionableErr{
Message: fmt.Sprintf("unable to initialize Kptfile in %v", r.hydrationDir),
ErrCode: proto.StatusCode_RENDER_KPTFILE_INIT_ERR,
Suggestions: []*proto.Suggestion{
{
SuggestionCode: proto.SuggestionCode_KPTFILE_MANUAL_INIT,
Action: fmt.Sprintf("please manually run `kpt pkg init %v`", r.hydrationDir),
},
},
})
}
}
return nil
Expand Down Expand Up @@ -135,9 +146,18 @@ func (r *SkaffoldRenderer) Render(ctx context.Context, out io.Writer, builds []g
defer file.Close()
kfConfig := &kptfile.KptFile{}
if err := yaml.NewDecoder(file).Decode(&kfConfig); err != nil {
// TODO: user error.
return fmt.Errorf("unable to parse %v: %w, please check if the kptfile is updated to new apiVersion > v1alpha2",
kptFilePath, err)
return sErrors.NewError(err,
proto.ActionableErr{
Message: fmt.Sprintf("unable to parse Kptfile in %v", r.hydrationDir),
ErrCode: proto.StatusCode_RENDER_KPTFILE_INVALID_YAML_ERR,
Suggestions: []*proto.Suggestion{
{
SuggestionCode: proto.SuggestionCode_KPTFILE_CHECK_YAML,
Action: fmt.Sprintf("please check if the Kptfile is correct and " +
"the `apiVersion` is greater than `v1alpha2`"),
},
},
})
}
if kfConfig.Pipeline == nil {
kfConfig.Pipeline = &kptfile.Pipeline{}
Expand Down
16 changes: 16 additions & 0 deletions pkg/skaffold/render/renderer/renderer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package renderer
import (
"bytes"
"context"
"errors"
"fmt"
"path/filepath"
"testing"
Expand Down Expand Up @@ -149,3 +150,18 @@ pipeline:
})
}
}
func TestRender_UserErr(t *testing.T) {
testutil.Run(t, "", func(t *testutil.T) {
r, err := NewSkaffoldRenderer(&latestV2.RenderConfig{
Generate: &latestV2.Generate{Manifests: []string{"pod.yaml"}},
Validate: &[]latestV2.Validator{{Name: "kubeval"}},
}, "")
t.CheckNoError(err)
fakeCmd := testutil.CmdRunOutErr(fmt.Sprintf("kpt pkg init %v", DefaultHydrationDir), "",
errors.New("fake err"))
t.Override(&util.DefaultExecCommand, fakeCmd)
err = r.Render(context.Background(), &bytes.Buffer{}, []graph.Artifact{{ImageName: "leeroy-web",
Tag: "leeroy-web:v1"}})
t.CheckContains("please manually run `kpt pkg init", err.Error())
})
}
31 changes: 24 additions & 7 deletions pkg/skaffold/render/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,40 @@ package validate
import (
"fmt"

sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/render/kptfile"
latestV2 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v2"
"github.com/GoogleContainerTools/skaffold/proto/v1"
)

var validatorWhitelist = map[string]kptfile.Function{
"kubeval": {Image: "gcr.io/kpt-fn/kubeval:v0.1"},
// TODO: Add conftest validator in kpt catalog.
}
var (
AllowlistedValidators = []string{"kubeval"}
validatorAllowlist = map[string]kptfile.Function{
"kubeval": {Image: "gcr.io/kpt-fn/kubeval:v0.1"},
// TODO: Add conftest validator in kpt catalog.
}
)

// NewValidator instantiates a Validator object.
func NewValidator(config []latestV2.Validator) (*Validator, error) {
var fns []kptfile.Function
for _, c := range config {
fn, ok := validatorWhitelist[c.Name]
fn, ok := validatorAllowlist[c.Name]
if !ok {
// TODO: kpt user error
return nil, fmt.Errorf("unsupported validator %v", c.Name)
// TODO: Add links to explain "skaffold-managed mode" and "kpt-managed mode".
return nil, sErrors.NewErrorWithStatusCode(
proto.ActionableErr{
Message: fmt.Sprintf("unsupported validator %q", c.Name),
ErrCode: proto.StatusCode_CONFIG_UNKNOWN_VALIDATOR,
Suggestions: []*proto.Suggestion{
{
SuggestionCode: proto.SuggestionCode_CONFIG_ALLOWLIST_VALIDATORS,
Action: fmt.Sprintf(
"please only use the following validators in skaffold-managed mode: %v. "+
"to use custom validators, please use kpt-managed mode.", AllowlistedValidators),
},
},
})
}
fns = append(fns, fn)
}
Expand Down
23 changes: 11 additions & 12 deletions pkg/skaffold/render/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,36 +22,35 @@ import (
"github.com/GoogleContainerTools/skaffold/testutil"
)

func TestValidatorInit(t *testing.T) {
func TestNewValidator(t *testing.T) {
tests := []struct {
description string
config []latestV2.Validator
shouldErr bool
}{
{
description: "no validation",
config: []latestV2.Validator{},
shouldErr: false,
},
{
description: "kubeval validator",
config: []latestV2.Validator{
{Name: "kubeval"},
},
shouldErr: false,
},
{
description: "invalid validator",
config: []latestV2.Validator{
{Name: "bad-validator"},
},
shouldErr: true,
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
_, err := NewValidator(test.config)
t.CheckError(test.shouldErr, err)
t.CheckNoError(err)
})
}
}

func TestNewValidator_Error(t *testing.T) {
testutil.Run(t, "", func(t *testutil.T) {
_, err := NewValidator([]latestV2.Validator{
{Name: "bad-validator"},
})
t.CheckContains(`unsupported validator "bad-validator". please only use the`, err.Error())
})
}
Loading

0 comments on commit 59edbda

Please sign in to comment.