cedar-agent
cedar-agent copied to clipboard
permitio
Add SchemaStore and policy validation
Created the SchemaMemoryStore with get, update and delete operations Implemented creating schema from file Added API endpoints for the three operations Added validation to policy create/update endpoints
Hey @Akamatsu21 - We're taking a look at the PR - but maybe it'll be worth to add some tests here :) Thanks for the contribution !
Hey, @Akamatsu21, do you have an estimate of when you can add tests?
Hello and big apologies, I have been away for the past two weeks on a long holiday. I will have a look at adding some tests, you can expect them by the end of this week. Appreciate your patience on this!
Hello @obsd @RazcoDev, I just pushed some unit tests, looking forward to your feedback.
We currently don't have CI to run the test so we run them manually, please attach a screenshot of the successfully running tests after your changes :)
Correct me if I'm wrong but the PR is missing the DataStore changes - validations
Correct me if I'm wrong but the PR is missing the DataStore changes - validations
Hi @omer9564 thank you so much for the review, I will address your comments today. As mentioned in the discussion the data store validation is the next item on the list, but I wanted to check that the logic thus far has been correct before I attempt it. I am not expecting the PR to be merged before that's finished.
Hey @Akamatsu21 , Overall logic looks very good, besides what I wrote in the comments everything seems ok. Please tag me once you finish with the data store validation and I will re-review everything :)
Hi @omer9564 apologies you had to wait quite long for this fairly small update. I completed all the remaining tasks on my original list, please let me know if this is good to be merged. I am attaching a screenshot of passing tests, this was run on the RustRover IDE with the cargo version 1.76.0 (c84b36747 2024-01-18).
By the way, I merged the main branch into this one in order to make sure any tests added by other people still pass. I appreciate in hindsight that maybe a rebase would have made for a cleaner commit history, hope it's not a big issue.
Hi @omer9564 apologies you had to wait quite long for this fairly small update. I completed all the remaining tasks on my original list, please let me know if this is good to be merged. I am attaching a screenshot of passing tests, this was run on the RustRover IDE with the cargo version 1.76.0 (c84b36747 2024-01-18).
By the way, I merged the main branch into this one in order to make sure any tests added by other people still pass. I appreciate in hindsight that maybe a rebase would have made for a cleaner commit history, hope it's not a big issue.
Thanks for the update, will take a look in the following days :)
Sorry for the delay, had busy couple of weeks. Will hopefully check this in the first week of April.
Looks very good in general,
- left a single comment on how the error handling of the policy store can be simplified with a reference to rust docs
- left a single comment on a missing validation on schema mutation @Akamatsu21 Sorry for taking so long to re-review 😅
No worries, and also sorry for taking a while. I should be able to respond faster now. I added the missing validation, but still think the unpacking of that error type is as good as it's going to be, unless we agree to just overhaul the return types