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

AST: Include ref to package or rule in serialized annotations #6280

Closed
anderseknert opened this issue Oct 6, 2023 · 4 comments
Closed

AST: Include ref to package or rule in serialized annotations #6280

anderseknert opened this issue Oct 6, 2023 · 4 comments

Comments

@anderseknert
Copy link
Member

The serialized AST contains a number rules, and it contains a number of annotations (if present). This is good, and tools using the AST, like Regal may use this to e.g. check that annotations contain some expected attributes. What we however really miss, is knowing what an annotation annotates. The AST provides no information to connect an annotation to a package or rule, so we can't easily write rules that say "rule allow must be an entrypoint", or "all rules must have a description", and so on.

Policy

# METADATA
# description: do some foo things
package foo

# METADATA
# description: do some bar things
bar {
	true
}

AST

{
  "package": {
    "path": [
      {
        "type": "var",
        "value": "data"
      },
      {
        "type": "string",
        "value": "foo"
      }
    ]
  },
  "annotations": [
    {
      "description": "do some foo things",
      "scope": "package"
    },
    {
      "description": "do some bar things",
      "scope": "rule"
    }
  ],
  "rules": [
    {
      "body": [
        {
          "index": 0,
          "terms": {
            "type": "boolean",
            "value": true
          }
        }
      ],
      "head": {
        "name": "bar",
        "value": {
          "type": "boolean",
          "value": true
        },
        "ref": [
          {
            "type": "var",
            "value": "bar"
          }
        ]
      }
    }
  ]
}

Parsing with locations included could allow trying to "map" the location of an annotation to the location of a package or rule, but this would be error-prone. Instead, I suggest that any annotation provide a ref to the package or rule it annotates:

  "annotations": [
    {
      "description": "do some foo things",
      "scope": "package"
      "ref": [
        {"type": "var", "value": "data"},
        {"type": "string", "value": "foo"}
      ]
    },
    {
      "description": "do some bar things",
      "scope": "rule",
      "ref": [
        {"type": "var", "value": "data"},
        {"type": "string", "value": "foo"},
        {"type": "string", "value": "bar"},
      ]
    }
  ],

The ref could also be called path I guess, as that's what we say in rego.metadata.chain(), but I don't really have an opinion on that.

@ashutosh-narkar
Copy link
Member

Parsing with locations included could allow trying to "map" the location of an annotation to the location of a package or rule, but this would be error-prone.

Due to compiler rewriting the policy or something else?

@anderseknert
Copy link
Member Author

Sorry, @ashutosh-narkar, I wasn't clear — I meant it is cumbersome and error-prone to do this in external tools, not in OPA. Adding to that, anything that must be done in external tooling will necessarily have to be done in all external tooling, while doing it in OPA requires doing it only once (and all downstream consumers benefit).

Copy link

stale bot commented Nov 5, 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.

@anderseknert
Copy link
Member Author

This was solved for rules in #6771, and adding the same for packages is tracked in #6796. No reason to keep this issue open. Still very much want this for packages 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

2 participants