opa icon indicating copy to clipboard operation
opa copied to clipboard

AST: Include ref to package or rule in serialized annotations

Open anderseknert opened this issue 2 years ago • 3 comments
trafficstars

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.

anderseknert avatar Oct 06 '23 13:10 anderseknert

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?

ashutosh-narkar avatar Oct 06 '23 16:10 ashutosh-narkar

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).

anderseknert avatar Oct 06 '23 16:10 anderseknert

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.

stale[bot] avatar Nov 05 '23 18:11 stale[bot]