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

fix(cli): argo lint with strict should report case-sensitive errors. Fixes #13006 #13250

Merged
merged 3 commits into from
Jul 18, 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
41 changes: 41 additions & 0 deletions cmd/argo/commands/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,45 @@ spec:

assert.False(t, fatal, "should not have exited")
})

workflowCaseSensitivePath := filepath.Join(subdir, "workflowCaseSensitive.yaml")
err = os.WriteFile(workflowCaseSensitivePath, []byte(`
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: hello-world-
spec:
entrypoInt: whalesay
templates:
- name: whalesay
container:
image: docker/whalesay
command: [ cowsay ]
args: [ "hello world" ]
resources:
limits:
memory: 32Mi
cpu: 100m
`), 0644)
require.NoError(t, err)

t.Run("linting a workflow with case sensitive fields and strict enabled", func(t *testing.T) {
defer func() { logrus.StandardLogger().ExitFunc = nil }()
var fatal bool
logrus.StandardLogger().ExitFunc = func(int) { fatal = true }

runLint(context.Background(), []string{workflowCaseSensitivePath}, true, nil, "pretty", true)

assert.True(t, fatal, "should have exited")
})

t.Run("linting a workflow with case sensitive fields and strict disabled", func(t *testing.T) {
defer func() { logrus.StandardLogger().ExitFunc = nil }()
var fatal bool
logrus.StandardLogger().ExitFunc = func(int) { fatal = true }

runLint(context.Background(), []string{workflowCaseSensitivePath}, true, nil, "pretty", false)

assert.False(t, fatal, "should not have exited")
})
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ require (
k8s.io/component-helpers v0.26.15 // indirect
k8s.io/metrics v0.26.15 // indirect
moul.io/http2curl/v2 v2.3.0 // indirect
sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect
sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2
sigs.k8s.io/kustomize/api v0.12.1 // indirect
sigs.k8s.io/kustomize/kustomize/v4 v4.5.7 // indirect
sigs.k8s.io/kustomize/kyaml v0.13.9 // indirect
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1111,7 +1111,7 @@ func (s *CLISuite) TestTemplateCommands() {
s.Given().RunCli([]string{"template", "lint", "testdata/workflow-templates"}, func(t *testing.T, output string, err error) {
assert.Error(t, err)
assert.Contains(t, output, "invalid-workflowtemplate.yaml")
assert.Contains(t, output, `unknown field "entrypoints"`)
assert.Contains(t, output, `unknown field "spec.entrypoints"`)
assert.Contains(t, output, "linting errors found!")
})
})
Expand Down
16 changes: 15 additions & 1 deletion workflow/common/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
log "github.com/sirupsen/logrus"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
kjson "sigs.k8s.io/json"
"sigs.k8s.io/yaml"

wf "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow"
Expand Down Expand Up @@ -96,7 +98,19 @@ func toWorkflowTypeYAML(body []byte, kind string, strict bool) (metav1.Object, e
func toWorkflowTypeJSON(body []byte, kind string, strict bool) (metav1.Object, error) {
v := objectForKind(kind)
if strict {
return v, jsonpkg.UnmarshalStrict(body, v)
var strictErrs []error
strictJSONErrs, err := kjson.UnmarshalStrict(body, v)
if err != nil {
// fatal decoding error, not due to strictness
return v, err
}
strictErrs = append(strictErrs, strictJSONErrs...)

if len(strictErrs) > 0 {
// return the successfully decoded object along with the strict errors
return v, runtime.NewStrictDecodingError(strictErrs)
}
return v, err
}

return v, jsonpkg.Unmarshal(body, v)
Expand Down
Loading