Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v3] Add proto for render error code and suggestions. #5953

Merged
merged 1 commit into from
Jun 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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