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

Open Policy Agent engine evaluation per tenant #170

Merged
merged 32 commits into from
Mar 21, 2024

Conversation

wcalderipe
Copy link
Collaborator

@wcalderipe wcalderipe commented Mar 15, 2024

Note

  • Engine signing is currently a proof of concept. It's unclear how it will evolve with remote signing, so I'd rather not modify it at this point.
  • Building OPA targeting WASM involves a process with many steps that interact with the OS and its file system. The current implementation addresses the happy path and a few obvious errors. Therefore, error handling for edge cases is not yet implemented, and I expect these to be addressed as we encounter issues in the building process.
    • The whole implementation is in a single place (see wasm-build.util.ts).

Changelog

  • Added a core Engine interface along with an Open Policy Agent implementation of it, effectively hiding OPA as an implementation detail. Everything outside the apps/policy-engine/src/open-policy-agent directory SHOULD NOT have knowledge of OPA and Rego.
  • Added temporary code in the BootstrapService to add a development tenant pointing to the devtool stores.
  • Added a resource directory for the server to access files from the disk.
    • Added a RESOURCE_PATH environment variable because we can't depend on __dirname to resolve the path. Its value changes based on how the application is built. With webpack, it minimizes, and __dirname always points to the lowest level of the directory tree. By contrast, in tests, the directory tree is preserved because files are transformed on the fly.
    • Added the resource directory as a NestJS assets directory in project.json.
  • Changed the Rego core directory to apps/policy-engine/src/resource.
  • Removed most of the legacy OPA code.
  • Changed the tenant module location to the engine module due to numerous circular dependency issues in the DI container.
    • Note: it felt like a good example of unclear module boundaries, and I'd rather roll it back and see how it evolves than trying to address problems we SHOULD NOT have.

@wcalderipe wcalderipe self-assigned this Mar 15, 2024
transactionRequest?: TransactionRequest
principal: CredentialEntity
resource?: { uid: string }
approvals: CredentialEntity[]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@samteb why are the approvals always required? Is it because the principal always approves the request, so it'll always have at least one?

Copy link
Collaborator Author

@wcalderipe wcalderipe Mar 19, 2024

Choose a reason for hiding this comment

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

I feel we have a misaligned somewhere because the Evaluation Request specifies the approvals as optional.

export type EvaluationRequest = {
  authentication: JwtString
  request: Request
  approvals?: JwtString[]
  transfers?: HistoricalTransfer[]
  prices?: Prices
  feeds?: Feed<unknown>[]
}

@wcalderipe wcalderipe changed the title Load engine instance per tenant Open Policy Agent engine evaluation per tenant Mar 20, 2024
@wcalderipe wcalderipe marked this pull request as ready for review March 20, 2024 15:46
Check encryption configuration at the bootstrap.
@wcalderipe wcalderipe force-pushed the feature/policy-dataset-compilation branch from 4dad927 to 9fbfc98 Compare March 21, 2024 11:02
@wcalderipe wcalderipe merged commit 286ce66 into main Mar 21, 2024
2 checks passed
mattschoch pushed a commit that referenced this pull request Jun 19, 2024
Note:
- Engine signing is currently a proof of concept. It's unclear how it will evolve with remote signing, so I'd rather not modify it at this point.
- Building OPA targeting WASM involves a process with many steps that interact with the OS and its file system. The current implementation addresses the happy path and a few obvious errors. Therefore, error handling for edge cases is not yet implemented, and I expect these to be addressed as we encounter issues in the building process.
  - The whole implementation is in a single place (see `wasm-build.util.ts`). 

Changelog:
- Added a core Engine interface along with an Open Policy Agent implementation of it, effectively hiding OPA as an implementation detail. Everything outside the `apps/policy-engine/src/open-policy-agent` directory SHOULD NOT have knowledge of OPA and Rego.
- Added temporary code in the BootstrapService to add a development tenant pointing to the devtool stores.
- Added a `resource` directory for the server to access files from the disk.
  - Added a `RESOURCE_PATH` environment variable because we can't depend on `__dirname` to resolve the path. Its value changes based on how the application is built. With webpack, it minimizes, and `__dirname` always points to the lowest level of the directory tree. By contrast, in tests, the directory tree is preserved because files are transformed on the fly.
  - Added the resource directory as a NestJS assets directory in `project.json`.
- Changed the Rego core directory to `apps/policy-engine/src/resource`.
- Removed most of the legacy OPA code.
- Changed the tenant module location to the engine module due to numerous circular dependency issues in the DI container. 
  - Note: it felt like a good example of unclear module boundaries, and I'd rather roll it back and see how it evolves than trying to address problems we SHOULD NOT have.

Co-authored-by: samuel <[email protected]>
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.

1 participant