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

YAML on/off as keys cannot be queried correctly #5754

Open
scnewma opened this issue Mar 13, 2023 · 12 comments
Open

YAML on/off as keys cannot be queried correctly #5754

scnewma opened this issue Mar 13, 2023 · 12 comments

Comments

@scnewma
Copy link

scnewma commented Mar 13, 2023

Short description

When using a YAML file as a data input, object keys that are YAML keywords are not converted correctly to JSON. A common example is on which is commonly used in Github Actions YAML. When loaded as data, the on object key is converted to true instead.

opa version: 0.50.0

Steps To Reproduce

data.yaml

on: push

example.rego

package example

import future.keywords.if

default is_push := false

is_push if input.on == "push"

Evaluate a basic OPA query to check if on is equal to "push":

❯ opa eval --data data.yaml 'is_push := data.on == "push"'
{}

Expected behavior

Able to query for the on field and get data back.

Additional context

This happens because on is converted to true in the key, as can be seen with this query:

❯ opa eval --data data.yaml 'data'
{
  "result": [
    {
      "expressions": [
        {
          "value": {
            "true": "push"
          },
          "text": "data",
          "location": {
            "row": 1,
            "col": 1
          }
        }
      ]
    }
  ]
}

This happens with off keyword as well (it's converted to false):

data.yaml

off: some-value
❯ opa eval --data data.yaml 'data'
{
  "result": [
    {
      "expressions": [
        {
          "value": {
            "false": "some-value"
          },
          "text": "data",
          "location": {
            "row": 1,
            "col": 1
          }
        }
      ]
    }
  ]
}
@scnewma scnewma added the bug label Mar 13, 2023
@anderseknert
Copy link
Member

Thanks for reporting this, @scnewma! YAML truly is a cursed format.

@anderseknert
Copy link
Member

Some digging into this, and OPA even mentioned in this issue from a year ago: ghodss/yaml#77
The library we're using seems to be abandonded, and the fork which apparently fixes this doesn't look super active either. Not a great situation, but might be worth making the switch still.

@scnewma
Copy link
Author

scnewma commented Mar 15, 2023

Good find! After some more poking, I found that the Kubernetes project has their own fork of ghodss/yaml as well here: https://github.com/kubernetes-sigs/yaml. It looks like they weren't able to simply upgrade from go-yaml v2 to v3, though from a quick glance it's hard to tell if that's because of something Kubernetes specific vs changes to the library.

@anderseknert
Copy link
Member

Interesting. Thanks @scnewma! It'd be useful to try that out as an alternative to our current version.

@scnewma
Copy link
Author

scnewma commented Mar 16, 2023

Forgot to add more details. I tried that and because that library has not updated to go-yaml v3 it still has the on/off bug 😞 . I believe the only option may be the fork that you mentioned in your first link.

@anderseknert
Copy link
Member

Thanks @scnewma! Sounds like it'd be worth investigating that option then 👍

@anderseknert
Copy link
Member

Related 🤦 https://ruudvanasseldonk.com/2023/01/11/the-yaml-document-from-hell

@stale
Copy link

stale bot commented Apr 26, 2023

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

@stale stale bot added the inactive label Apr 26, 2023
@ViacheslavKudinov
Copy link

Hi,
any updates here?
Got same issue with yaml like this one (GitHub reusable workflow):

on:
  workflow_call:
    inputs:
      java-version:
        description: "The java-version input supports an exact version or a version range using SemVer notation: major versions: 8, 11, 16, 17. more specific versions: 17.0, 11.0, 11.0.4, 8.0.232, 8.0.282+8 early access (EA) versions: 15-ea, 15.0.0-ea, 15.0.0-ea.2, 15.0.0+2-ea"
        required: true
        type: string
      java_distribution:
        description: 'Currently, the following distributions are supported: `"temurin"` (Eclipse Temurin), `"zulu"` (Zulu OpenJDK), `"adopt"` (Adopt OpenJDK Hotspot), `"adopt-openj9"` (Adopt OpenJDK OpenJ9).'
        required: false
        default: "temurin"
        type: string

@stale stale bot removed the inactive label Jun 7, 2023
@jalseth
Copy link
Member

jalseth commented Jun 8, 2023

You can force YAML to treat the key as a string by quoting it. Ex:

"on":
  foo: bar

->

{
  "on": {
    "foo": "bar"
  }
}

@ViacheslavKudinov
Copy link

Hi @jalseth
yes, it works on GitHub, even looks weird.
I think OPA will work correctly with quotation.
Thanks for the idea.

@stale
Copy link

stale bot commented Jul 9, 2023

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.

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

4 participants