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

Add Regal for linting Rego #200

Merged
merged 1 commit into from
Oct 16, 2023
Merged

Add Regal for linting Rego #200

merged 1 commit into from
Oct 16, 2023

Conversation

anderseknert
Copy link
Contributor

@anderseknert anderseknert commented Oct 15, 2023

What is this PR About?

Hello, Red Hat friends :)

This PR adds Regal for linting the Rego contained in this repo.

Practically, this PR adds a Regal configuration file adapted to the project, where some of the most reported style issues are ignoed for the time being. Additionally, we're adding a linter job to the CI pipeline to ensure future updates to policy if compliant as well.

A few (hopefully) uncontroversial issues reported have also been fixed:

While the changes are non-intrusive, and should have no impact on evaluation, I naturally wanted to run the tests included in this repo and followed the instructions in TESTING.md. However, even after having installed all of the dependencies listed as required, the test command fails due to a missing oc command. I tried to find that and possibly update the TESTING.md file to include it, but the instructions I followed suggested logging in to a "Red Hat Customer Portal" to get it. If being a customer is required to run the tests, that requirement would be good to add to the docs as well.

This PR adds [Regal](https://github.com/styraInc/regal) for linting the Rego
contained in this repo.

Practically, this PR adds a Regal configuration file adapted to the project,
where some of the most reported style issues are ignoed for the time being.
Additionally, we're adding a linter job to the CI pipeline to ensure future
updates to policy if compliant as well.

A few (hopefully) uncontroversial issues reported have also been fixed:

* [constant-condition](https://docs.styra.com/regal/rules/bugs/constant-condition)
* [use-assignment-operator](https://docs.styra.com/regal/rules/style/use-assignment-operator)
* [use-in-operator](https://docs.styra.com/regal/rules/idiomatic/use-in-operator)
* [use-some-for-output-vars](https://docs.styra.com/regal/rules/idiomatic/use-some-for-output-vars)
* [non-raw-regex-pattern](https://docs.styra.com/regal/rules/idiomatic/non-raw-regex-pattern)

While the changes are non-intrusive, and should have no impact on evaluation,
I naturally wanted to run the tests included in this repo and followed the instructions
in TESTING.md. However, even after having installed all of the dependencies listed as
required, the test command fails due to a missing `oc` command. I tried to find that
and possibly update the TESTING.md file to include it, but the instructions I followed
suggested logging in to a "RedHat Customer Portal" to get it. If being a customer is required
to run the tests, that requirement would be good to add to the docs as well.

Signed-off-by: Anders Eknert <[email protected]>
@garethahealy
Copy link
Contributor

@anderseknert ; thanks for the PR.

Just as an FYI, the rego in https://github.com/redhat-cop/rego-policies/tree/master/policy/lib/konstraint comes from https://github.com/plexsystems/konstraint/tree/main/examples/lib - I am happy to merge this PR but we are not the "owner" of those files.

You can also grab OC from: https://mirror.openshift.com/pub/openshift-v4/x86_64/clients/ocp/

@anderseknert
Copy link
Contributor Author

Thanks @garethahealy! Yep, I did notice that, so I'm planning to submit a PR there too so that future imports from that source will be covered automatically 🙂

Thanks for the link! Adding that to the test instructions would be helpful.

Copy link
Contributor

@garethahealy garethahealy left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@garethahealy garethahealy merged commit abf7801 into redhat-cop:master Oct 16, 2023
5 checks passed
@anderseknert anderseknert deleted the regal branch October 16, 2023 11:00
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