-
Notifications
You must be signed in to change notification settings - Fork 4
Refinement of Output for Mequal and Automated Policy Bundle Documentatation Generation #9
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
Refinement of Output for Mequal and Automated Policy Bundle Documentatation Generation #9
Conversation
…ccessed by policies
…est json structure
container_files/Containerfile
Outdated
&& mkdir ./bundle | ||
|
||
RUN adduser -u 1000 -U mequal -m | ||
|
||
USER root | ||
COPY ./policy/ ./policy/ | ||
COPY ./hack/ ./hack/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think of hack
as scripts to support building and expect to run locally, these are ran as part of the container image build, we should perhaps re-purpose local-opa-run.sh
to be ran at container build time, rather than maintain two seperate approaches to building the policy bundles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're absolutely right about this. I'll move the scripts that the image depends on to a scripts
folder, and in hack
I can create scripts to prep the development environment by running those scripts locally. It currently relies on building and running the image for development and testing, since inside the image it generates that metadata object to be used.
With something like dev-setup.sh, we generate the metadata object and put it into where it would be in the directory, so we can then use OPA locally to do our development. This script can also include downloading the helper functions to be used, so we can remove them from the code at least and let it live in another repo. (currently each policy repo has them copied over as a temporary solution, which is not good at all)
Both can then be marked by gitignore so they don't end up making it into the source code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separated them into separate scripts folder
hack/generate_bundle_info.py
Outdated
@@ -0,0 +1,222 @@ | |||
# Generated with assistance from a large language model trained by Google. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add #!/bin/env python
and set executable, I'm not sure about the current header, it doesn't give much information or attribution about what was used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I can do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done as #!/usr/bin/env python3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to address the LLM comment: Gemini by default isn't really attaching any info to the code it generates, and I just asked how to credit it. And it said to do this way since versioning may differ in the background if a specific model etc is specified. So I just kept it broad like this
@@ -0,0 +1,4 @@ | |||
{ | |||
"revision": "v1.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't need fixing just yet as the container image contains the git sha but I created a PR for a temporary solution to having this also set in the policy manifest as I think we will forget to update this and it may catch us out if we distribute policies by alternative methods - mertbugrabicak#2 but feel free to ignore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes, this was something I wanted to do but forgot! We can definitely inject it during image build.
For the mequal bundle versioning, we can use our git sha, and for the external policies we define in our config json, I can create a revision field where we can concretely define what sha we are fetching the external policy from, and then inject those shas to their respective manifests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will take in your changes and work from there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged your changes, will now expand it to include external policies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will leave this for now, to think of a better solution if possible for the external policy revisions
policy/main/eval.rego
Outdated
import rego.v1 | ||
|
||
# Main evaluation rule to generate the desired "evaluation" output object | ||
evaluation := { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opa fmt
changes this but as far as I can see there are no functional or syntax changes, only formatting, given we want to use regal in future I don't think we need to apply the changes but feel free
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I should be able to do this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged fine
In this commit: * Added Gemini generated script to use current git revision as .manifest version * Call this script before building policy bundle in container
In this commit: * Allow overrides to the manifest revision * Update readme for building * Update github ci to override manifest revision
In this commit: * `opa fmt policy --v1-compatible -w`
…ve helpers and get externally
|
@jonathanchristison if you can have another look tomorrow, hopefully it should be mergeable now. it's it a lot better shape |
There is also a TODO to adapt the local opa run script to our latest bundle structure, i might also work on that tomorrow, but that can also wait |
@mertbugrabicak looks good to me and some nice additions with local scripts and reduced runtime image size 👍 |
First off sorry for the gigantic PR!
The output we eventually wanted to get required doing some data processing before runtime, where we use OPA to generate objects containing the policy annotations in the source code, map them to a clean JSON, and then include them in the Mequal bundle for policies to make use of.
So the main changes: