fix(cli): handle multi-resource yaml in offline lint. Fixes #12137#13531
Conversation
…oproj#12137 Signed-off-by: Wang, Wayne <Wayne.Wang@fmr.com>
…-issue-on-multi-documents-yaml-file
Signed-off-by: Wang, Wayne <Wayne.Wang@fmr.com>
693db02 to
70c5e90
Compare
|
@miltalex do you want to try reviewing this? Since you've worked on |
|
I'm curious if this also fixes #10608? |
I would be happy to review it |
miltalex
left a comment
There was a problem hiding this comment.
Nice work, thank you very much for submitting the fix. I have a small stylistic suggestion otherwise LGTM
Signed-off-by: Wang, Wayne <Wayne.Wang@fmr.com>
Signed-off-by: Wang, Wayne <Wayne.Wang@fmr.com>
|
/retest |
…-issue-on-multi-documents-yaml-file
Signed-off-by: Wang, Wayne <Wayne.Wang@fmr.com>
899730b to
0c5b6aa
Compare
Signed-off-by: Wang, Wayne <Wayne.Wang@fmr.com>
651d412 to
3bb0c67
Compare
| templateRef: | ||
| name: hello-world-template-local-arg-2 | ||
| template: hello-world | ||
| `), 0644) |
There was a problem hiding this comment.
This should be rewritten for #13610, but that should probably be a separate PR for the whole file. Your current style matches the existing style of this file, so I think this is fine for now
| cwftmpl := new(wfv1.ClusterWorkflowTemplate) | ||
| if err := yaml.Unmarshal(bytes, &cwftmpl); err != nil { | ||
| return fmt.Errorf("failed to unmarshal file %s as a ClusterWorkflowTemplate: %w", path, err) | ||
| for _, pr := range common.ParseObjects(bytes, false) { |
There was a problem hiding this comment.
I see this is copied from lint.go, so similarly I'd say this is fine for now, but a more clear name would be better in both places, like parsed instead of pr (which makes one think of "pull request")
| for _, pr := range common.ParseObjects(bytes, false) { | |
| for _, parsed := range common.ParseObjects(bytes, false) { |
| if obj == nil { | ||
| continue // could not parse to kubernetes object |
There was a problem hiding this comment.
This too is copied from lint.go, although I would think both places should have an error in this case rather than a silent continue.
| namespace: wftmpl.Namespace, | ||
| workflowTemplates: map[string]*wfv1.WorkflowTemplate{}, | ||
| objName := obj.GetName() | ||
| namespace := obj.GetNamespace() |
There was a problem hiding this comment.
same here that this is copied from lint.go, but the naming could be consistent with objName as objNs in both places
| namespace := obj.GetNamespace() | |
| objNs := obj.GetNamespace() |
Fixes #12137
Fixes #10608
Motivation
If there are multiple templates in one yaml file. The
argo lint --offlinecan only get the first template.Because the
yaml.Unmarshalfunction is used to parse a single YAML document.Modifications
Read yaml file with
common.ParseObjectsin offline mode.Verification
e2e test.
argo lint --offline test.yamlAdd unittest case.