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

[lsp] Implement rulerefs in rego #849

Merged
merged 3 commits into from
Jun 19, 2024

Conversation

charlieegan3
Copy link
Member

This implements the rule_refs provider in Rego. This involves using a shared inmem rego store to allow the parsed modules for all workspace files to be accessible when running the new Rego-based policy provider.

Copy link
Member

@anderseknert anderseknert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work! Some questions, but OK to merge now and discuss more later.

some file, parsed in data.workspace.parsed

package_name := concat(".", [path.value |
some i, path in parsed["package"].path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need i here

And yes, we shouldn't have to care: #60

@@ -398,7 +401,15 @@
},
"context": {
"description": "extra attributes provided in the specific evaluation context",
"type": "object"
"type": "object",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should just create a new schema for this entrypoint if we're not going to use the AST from there

@@ -35,8 +35,8 @@ var regalRules = func() bundle.Bundle {
// NewPolicy creates a new Policy provider. This provider is distinctly different from the other providers
// as it acts like the entrypoint for all Rego-based providers, and not a single provider "function" like
// the Go providers do.
func NewPolicy() *Policy {
pq, err := prepareQuery("completions := data.regal.lsp.completion.items")
func NewPolicy(store storage.Store) *Policy {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

@@ -22,27 +23,58 @@ import (
// updateParse updates the module cache with the latest parse result for a given URI,
// if the module cannot be parsed, the parse errors are saved as diagnostics for the
// URI instead.
func updateParse(ctx context.Context, cache *cache.Cache, uri string) (bool, error) {
content, ok := cache.GetFileContents(uri)
func updateParse(ctx context.Context, cache *cache.Cache, store storage.Store, fileURI string) (bool, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we'll have to make sure never to reference this storage from regular linting policies, as this isn't going to be available in this context?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it's only going to be available in the lsp.

This update has the locals provider also use the workspace contents

Signed-off-by: Charlie Egan <[email protected]>
@charlieegan3
Copy link
Member Author

Thanks for the review, will patch up when I'm back on tomorrow morning but I've also added 694ea52
which does a refactor to migrate the locals provider to source the ast from the store too. Lmk if that looks good too and we can get this in asap.

@charlieegan3 charlieegan3 merged commit 7bf1c93 into StyraInc:main Jun 19, 2024
3 checks passed
@charlieegan3 charlieegan3 deleted the rule-refs-rego branch June 19, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants