Skip to content

Introduce err code for user error in devfile #196

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

Merged
merged 2 commits into from
Jan 8, 2024
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
3 changes: 2 additions & 1 deletion pkg/devfile/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package devfile
import (
"github.com/devfile/api/v2/pkg/validation/variables"
"github.com/devfile/library/v2/pkg/devfile/parser"
errPkg "github.com/devfile/library/v2/pkg/devfile/parser/errors"
"github.com/devfile/library/v2/pkg/devfile/validate"
)

Expand Down Expand Up @@ -121,7 +122,7 @@ func ParseDevfileAndValidate(args parser.ParserArgs) (d parser.DevfileObj, varWa
// generic validation on devfile content
err = validate.ValidateDevfileData(d.Data)
if err != nil {
return d, varWarning, err
return d, varWarning, &errPkg.NonCompliantDevfile{Err: err.Error()}
}

return d, varWarning, err
Expand Down
8 changes: 4 additions & 4 deletions pkg/devfile/parser/context/apiVersion.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"strings"

"github.com/devfile/library/v2/pkg/devfile/parser/data"
"github.com/pkg/errors"
errPkg "github.com/devfile/library/v2/pkg/devfile/parser/errors"
"k8s.io/klog"
)

Expand All @@ -32,7 +32,7 @@ func (d *DevfileCtx) SetDevfileAPIVersion() error {
var r map[string]interface{}
err := json.Unmarshal(d.rawContent, &r)
if err != nil {
return errors.Wrapf(err, "failed to decode devfile json")
return &errPkg.NonCompliantDevfile{Err: err.Error()}
}

// Get "schemaVersion" value from map for devfile V2
Expand All @@ -47,10 +47,10 @@ func (d *DevfileCtx) SetDevfileAPIVersion() error {
if okSchema {
// SchemaVersion cannot be empty
if schemaVersion.(string) == "" {
return fmt.Errorf("schemaVersion in devfile: %s cannot be empty", devfilePath)
return &errPkg.NonCompliantDevfile{Err: fmt.Sprintf("schemaVersion in devfile: %s cannot be empty", devfilePath)}
}
} else {
return fmt.Errorf("schemaVersion not present in devfile: %s", devfilePath)
return &errPkg.NonCompliantDevfile{Err: fmt.Sprintf("schemaVersion not present in devfile: %s", devfilePath)}
}

// Successful
Expand Down
13 changes: 11 additions & 2 deletions pkg/devfile/parser/context/apiVersion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"fmt"
"reflect"
"testing"

errPkg "github.com/devfile/library/v2/pkg/devfile/parser/errors"
)

func TestSetDevfileAPIVersion(t *testing.T) {
Expand All @@ -29,6 +31,7 @@ func TestSetDevfileAPIVersion(t *testing.T) {
concreteSchema = `{"schemaVersion": "2.2.0-latest"}`
emptyJson = "{}"
emptySchemaVersionJson = `{"schemaVersion": ""}`
badJson = `{"name": "Joe", "age": null]`
devfilePath = "/testpath/devfile.yaml"
devfileURL = "http://server/devfile.yaml"
)
Expand Down Expand Up @@ -56,13 +59,19 @@ func TestSetDevfileAPIVersion(t *testing.T) {
name: "schemaVersion not present",
devfileCtx: DevfileCtx{rawContent: []byte(emptyJson), absPath: devfilePath},
want: "",
wantErr: fmt.Errorf("schemaVersion not present in devfile: %s", devfilePath),
wantErr: &errPkg.NonCompliantDevfile{Err: fmt.Sprintf("schemaVersion not present in devfile: %s", devfilePath)},
},
{
name: "schemaVersion empty",
devfileCtx: DevfileCtx{rawContent: []byte(emptySchemaVersionJson), url: devfileURL},
want: "",
wantErr: fmt.Errorf("schemaVersion in devfile: %s cannot be empty", devfileURL),
wantErr: &errPkg.NonCompliantDevfile{Err: fmt.Sprintf("schemaVersion in devfile: %s cannot be empty", devfileURL)},
},
{
name: "unmarshal error",
devfileCtx: DevfileCtx{rawContent: []byte(badJson), url: devfileURL},
want: "",
wantErr: &errPkg.NonCompliantDevfile{Err: "invalid character ']' after object key:value pair"},
},
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/devfile/parser/context/content.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"bytes"
"unicode"

errPkg "github.com/devfile/library/v2/pkg/devfile/parser/errors"
parserUtil "github.com/devfile/library/v2/pkg/devfile/parser/util"
"github.com/devfile/library/v2/pkg/util"
"github.com/pkg/errors"
Expand All @@ -42,7 +43,7 @@ func YAMLToJSON(data []byte) ([]byte, error) {
// Is YAML, convert to JSON
data, err := yaml.YAMLToJSON(data)
if err != nil {
return data, errors.Wrapf(err, "failed to convert devfile yaml to json")
return data, &errPkg.NonCompliantDevfile{Err: err.Error()}
}

// Successful
Expand Down
2 changes: 1 addition & 1 deletion pkg/devfile/parser/context/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
)

func TestPopulateFromBytes(t *testing.T) {
failedToConvertYamlErr := "failed to convert devfile yaml to json: yaml: mapping values are not allowed in this context"
failedToConvertYamlErr := "mapping values are not allowed in this context"

tests := []struct {
name string
Expand Down
5 changes: 3 additions & 2 deletions pkg/devfile/parser/context/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"

"github.com/devfile/library/v2/pkg/devfile/parser/data"
errPkg "github.com/devfile/library/v2/pkg/devfile/parser/errors"
"github.com/pkg/errors"
"github.com/xeipuuv/gojsonschema"
"k8s.io/klog"
Expand All @@ -30,7 +31,7 @@ func (d *DevfileCtx) SetDevfileJSONSchema() error {
// Check if json schema is present for the given apiVersion
jsonSchema, err := data.GetDevfileJSONSchema(d.apiVersion)
if err != nil {
return err
return &errPkg.NonCompliantDevfile{Err: err.Error()}
}
d.jsonSchema = jsonSchema
return nil
Expand All @@ -54,7 +55,7 @@ func (d *DevfileCtx) ValidateDevfileSchema() error {
for _, desc := range result.Errors() {
errMsg = errMsg + fmt.Sprintf("- %s\n", desc)
}
return fmt.Errorf(errMsg)
return &errPkg.NonCompliantDevfile{Err: errMsg}
}

// Sucessful
Expand Down
31 changes: 31 additions & 0 deletions pkg/devfile/parser/errors/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//
// Copyright 2024 Red Hat, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package errors

import "fmt"

// NonCompliantDevfile returns an error if devfile parsing failed due to Non-Compliant Devfile
type NonCompliantDevfile struct {
Err string
}

func (e *NonCompliantDevfile) Error() string {
errMsg := "error parsing devfile because of non-compliant data"
if e.Err != "" {
errMsg = fmt.Sprintf("%s due to %v", errMsg, e.Err)
}
return errMsg
}
29 changes: 15 additions & 14 deletions pkg/devfile/parser/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
devfileCtx "github.com/devfile/library/v2/pkg/devfile/parser/context"
"github.com/devfile/library/v2/pkg/devfile/parser/data"
"github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common"
errPkg "github.com/devfile/library/v2/pkg/devfile/parser/errors"
"github.com/devfile/library/v2/pkg/util"
registryLibrary "github.com/devfile/registry-support/registry-library/library"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -65,7 +66,7 @@ func parseDevfile(d DevfileObj, resolveCtx *resolutionContextTree, tool resolver
// Unmarshal devfile content into devfile struct
err = json.Unmarshal(d.Ctx.GetDevfileContent(), &d.Data)
if err != nil {
return d, errors.Wrapf(err, "failed to decode devfile content")
return d, &errPkg.NonCompliantDevfile{Err: err.Error()}
}

if flattenedDevfile {
Expand Down Expand Up @@ -156,14 +157,14 @@ func ParseDevfile(args ParserArgs) (d DevfileObj, err error) {
if args.Data != nil {
d.Ctx, err = devfileCtx.NewByteContentDevfileCtx(args.Data)
if err != nil {
return d, errors.Wrap(err, "failed to set devfile content from bytes")
return d, err
}
} else if args.Path != "" {
d.Ctx = devfileCtx.NewDevfileCtx(args.Path)
} else if args.URL != "" {
d.Ctx = devfileCtx.NewURLDevfileCtx(args.URL)
} else {
return d, errors.Wrap(err, "the devfile source is not provided")
return d, fmt.Errorf("the devfile source is not provided")
}

if args.Token != "" {
Expand Down Expand Up @@ -196,7 +197,7 @@ func ParseDevfile(args ParserArgs) (d DevfileObj, err error) {

d, err = populateAndParseDevfile(d, &resolutionContextTree{}, tool, flattenedDevfile)
if err != nil {
return d, errors.Wrap(err, "failed to populateAndParseDevfile")
return d, err
}

setBooleanDefaults := true
Expand Down Expand Up @@ -249,7 +250,7 @@ type resolverTools struct {
func populateAndParseDevfile(d DevfileObj, resolveCtx *resolutionContextTree, tool resolverTools, flattenedDevfile bool) (DevfileObj, error) {
var err error
if err = resolveCtx.hasCycle(); err != nil {
return DevfileObj{}, err
return DevfileObj{}, &errPkg.NonCompliantDevfile{Err: err.Error()}
}
// Fill the fields of DevfileCtx struct
if d.Ctx.GetURL() != "" {
Expand Down Expand Up @@ -330,7 +331,7 @@ func parseParentAndPlugin(d DevfileObj, resolveCtx *resolutionContextTree, tool
case parent.Kubernetes != nil:
parentDevfileObj, err = parseFromKubeCRD(parent.ImportReference, resolveCtx, tool)
default:
return fmt.Errorf("devfile parent does not define any resources")
err = &errPkg.NonCompliantDevfile{Err: "devfile parent does not define any resources"}
}
if err != nil {
return err
Expand All @@ -347,7 +348,7 @@ func parseParentAndPlugin(d DevfileObj, resolveCtx *resolutionContextTree, tool
}

if parentDevfileVerson.GreaterThan(mainDevfileVersion) {
return fmt.Errorf("the parent devfile version from %v is greater than the child devfile version from %v", resolveImportReference(parent.ImportReference), resolveImportReference(resolveCtx.importReference))
return &errPkg.NonCompliantDevfile{Err: fmt.Sprintf("the parent devfile version from %v is greater than the child devfile version from %v", resolveImportReference(parent.ImportReference), resolveImportReference(resolveCtx.importReference))}
}
}
parentWorkspaceContent := parentDevfileObj.Data.GetDevfileWorkspaceSpecContent()
Expand Down Expand Up @@ -392,7 +393,7 @@ func parseParentAndPlugin(d DevfileObj, resolveCtx *resolutionContextTree, tool
case plugin.Kubernetes != nil:
pluginDevfileObj, err = parseFromKubeCRD(plugin.ImportReference, resolveCtx, tool)
default:
return fmt.Errorf("plugin %s does not define any resources", component.Name)
err = &errPkg.NonCompliantDevfile{Err: fmt.Sprintf("plugin %s does not define any resources", component.Name)}
}
if err != nil {
return err
Expand All @@ -408,7 +409,7 @@ func parseParentAndPlugin(d DevfileObj, resolveCtx *resolutionContextTree, tool
return fmt.Errorf("fail to parse version of plugin devfile from: %v", resolveImportReference(component.Plugin.ImportReference))
}
if pluginDevfileVerson.GreaterThan(mainDevfileVersion) {
return fmt.Errorf("the plugin devfile version from %v is greater than the child devfile version from %v", resolveImportReference(component.Plugin.ImportReference), resolveImportReference(resolveCtx.importReference))
return &errPkg.NonCompliantDevfile{Err: fmt.Sprintf("the plugin devfile version from %v is greater than the child devfile version from %v", resolveImportReference(component.Plugin.ImportReference), resolveImportReference(resolveCtx.importReference))}
}
}
pluginWorkspaceContent := pluginDevfileObj.Data.GetDevfileWorkspaceSpecContent()
Expand Down Expand Up @@ -462,7 +463,7 @@ func parseFromURI(importReference v1.ImportReference, curDevfileCtx devfileCtx.D
newUri = path.Join(path.Dir(curDevfileCtx.GetAbsPath()), uri)
d.Ctx = devfileCtx.NewDevfileCtx(newUri)
if util.ValidateFile(newUri) != nil {
return DevfileObj{}, fmt.Errorf("the provided path is not a valid filepath %s", newUri)
return DevfileObj{}, &errPkg.NonCompliantDevfile{Err: fmt.Sprintf("the provided path is not a valid filepath %s", newUri)}
}
srcDir := path.Dir(newUri)
destDir := path.Dir(curDevfileCtx.GetAbsPath())
Expand Down Expand Up @@ -520,7 +521,7 @@ func parseFromRegistry(importReference v1.ImportReference, resolveCtx *resolutio
}
d.Ctx, err = devfileCtx.NewByteContentDevfileCtx(devfileContent)
if err != nil {
return d, errors.Wrap(err, "failed to set devfile content from bytes")
return d, err
}
newResolveCtx := resolveCtx.appendNode(importReference)

Expand Down Expand Up @@ -551,15 +552,15 @@ func parseFromRegistry(importReference v1.ImportReference, resolveCtx *resolutio
}
}
} else {
return DevfileObj{}, fmt.Errorf("failed to fetch from registry, registry URL is not provided")
return DevfileObj{}, &errPkg.NonCompliantDevfile{Err: "failed to fetch from registry, registry URL is not provided"}
}

return DevfileObj{}, fmt.Errorf("failed to get id: %s from registry URLs provided", id)
}

func getDevfileFromRegistry(id, registryURL, version string, httpTimeout *int) ([]byte, error) {
if !strings.HasPrefix(registryURL, "http://") && !strings.HasPrefix(registryURL, "https://") {
return nil, fmt.Errorf("the provided registryURL: %s is not a valid URL", registryURL)
return nil, &errPkg.NonCompliantDevfile{Err: fmt.Sprintf("the provided registryURL: %s is not a valid URL", registryURL)}
}
param := util.HTTPRequestParams{
URL: fmt.Sprintf("%s/devfiles/%s/%s", registryURL, id, version),
Expand Down Expand Up @@ -594,7 +595,7 @@ func getResourcesFromRegistry(id, registryURL, destDir string) error {
func parseFromKubeCRD(importReference v1.ImportReference, resolveCtx *resolutionContextTree, tool resolverTools) (d DevfileObj, err error) {

if tool.k8sClient == nil || tool.context == nil {
return DevfileObj{}, fmt.Errorf("Kubernetes client and context are required to parse from Kubernetes CRD")
return DevfileObj{}, fmt.Errorf("kubernetes client and context are required to parse from Kubernetes CRD")
}
namespace := importReference.Kubernetes.Namespace

Expand Down
Loading