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

Empty yaml documents should be ignored when importing lists #3464

Open
uhthomas opened this issue Sep 25, 2024 · 6 comments
Open

Empty yaml documents should be ignored when importing lists #3464

uhthomas opened this issue Sep 25, 2024 · 6 comments

Comments

@uhthomas
Copy link
Contributor

What version of CUE are you using (cue version)?

$ cue version
cue version 0.9.2

go version go1.22.5
      -buildmode pie
       -compiler gc
       -trimpath true
  DefaultGODEBUG httplaxcontentlength=1,httpmuxgo121=1,tls10server=1,tlsrsakex=1,tlsunsafeekm=1
     CGO_ENABLED 1
          GOARCH amd64
            GOOS linux
         GOAMD64 v1
cue.lang.version v0.9.2

Does this issue reproduce with the latest stable release?

Yes.

What did you do?

Import a list of yaml documents and transform it with -l.

❯ cue import -l "strings.ToLower(kind)" --list abc.yaml
error evaluating label strings.ToLower(kind): reference "kind" not found
abc.yaml
---
kind: Deployment
---
---
kind: Deployment

What did you expect to see?

It would be nice if this just didn't add anything to the list instead of failing. I use cue import a lot to import Helm charts and often, because Helm is just a bad text templating engine, it will include empty yaml documents. I therefore have to find the empty documents myself and run the import again.

What did you see instead?

error evaluating label strings.ToLower(kind): reference "kind" not found

@uhthomas uhthomas added NeedsInvestigation Triage Requires triage/attention labels Sep 25, 2024
@mvdan
Copy link
Member

mvdan commented Sep 26, 2024

This seems reasonable. Do you want to send a patch with a test?

@mvdan mvdan removed the Triage Requires triage/attention label Sep 26, 2024
@haoqixu
Copy link
Contributor

haoqixu commented Sep 30, 2024

I would like to take a look at this issue

@haoqixu
Copy link
Contributor

haoqixu commented Oct 1, 2024

The YAML decoder decodes an empty YAML as null, making it indistinguishable from an actual null. Would it be reasonable to ignore null when importing lists?

@mvdan
Copy link
Member

mvdan commented Oct 1, 2024

@haoqixu yes, I think that's fine. The YAML spec strongly hints that an empty document is equivalent to a null value.

@uhthomas
Copy link
Contributor Author

uhthomas commented Oct 1, 2024

What is the expected behavior when a document is not empty? Is it possible to handle missing keys gracefully for the general case? Say:

❯ cue import -l "strings.ToLower(metadata.namespace)" --list abc.yaml
---
metadata:
    namespace: some-namespace
---
metadata:
    name: some-name # missing namespace
---
metadata:
    namespace: some-namespace

@mvdan
Copy link
Member

mvdan commented Oct 1, 2024

If multiple behaviors are wanted depending on the use case, we could always rethink the flag slightly so that --list is equivalent to --list=all, and add another mode like --list=nonnull or --list=nonempty. Hopefully the added UX complexity is not needed though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants