Add policy definitions and attachments to request context, change pol… #141
Conversation
…icy executor to use them from context instead of directly from repo
| @@ -3,6 +3,7 @@ export interface ResourceGroup { | |||
| upstreams: Upstream[]; | |||
| upstreamClientCredentials: UpstreamClientCredentials[]; | |||
| policies: Policy[]; | |||
| policyAttachments?: PolicyAttachments; | |||
There was a problem hiding this comment.
Leave a comment about this being a "runtime-only" property? Since the rest are assumed to always exist
There was a problem hiding this comment.
Come to think of it, it will be populated on the first call to fetchLatest same as the others, and will always exist (empty object if there are no attachments at all, same as the other fields here).
All of them are actually runtime-only properties representing what is stored in the underlying repository, with the only different between the attachments and the other properties being that the other properties are all stored in one file and the attachments are stored in other files.
However, when using this interface we don't really care that it is stored in other files than the other properties, so I'm actually leaning towards removing the ? here and treating it like any other property.
One key difference is that the contents of this property are calculated based on the policies property and not created/updated directly by a user, but it only happens once when we save the policy and then saved this way in the repository, so I would still say we should treat it like the other properties here (but I will add a comment to highlight this difference)
| upstreams: [], | ||
| upstreamClientCredentials: [], | ||
| policies: [], | ||
| policyAttachments: {}, |
There was a problem hiding this comment.
Shouldn't policyAttachments not be here?
There was a problem hiding this comment.
It is technically there because of the way we currently manage the bucket mocking, but I'll remove it to be in line with the tests in update-resources.spec (since this is not something we really test here)
| @@ -17,13 +18,29 @@ export function mockResourceBucket(initialValue: ResourceGroup, initialPolicyFil | |||
| .reply(200, (_, body) => { | |||
| value.current = JSON.parse(body as string) as ResourceGroup; | |||
| }) | |||
| .get(new RegExp(`/${bucketName!}\?.*prefix=${encodeURIComponent(policiesKeyPrefix!)}.*`)) | |||
There was a problem hiding this comment.
Perhaps a comment or some constants could make this more readable, because right now it's a bit hard :X
tomeresk
force-pushed
the
authz_move_policy_details_to_context
branch
from
9729e3c
to
2e656a9
Compare
* Authorization - fully implemented registry part (#133) This includes: Create/update policy resource Attachments support for policy resource (with support for writing the attachment to both s3 and fs repositories) Opa policy type implementation, including compiling rego code to wasm and adding that to the policy as an attachment * Authorization gateway - basic features (#138) implemented full flow with basic features Implement local policy attachment caching for all resource repositories * Add policy definitions and attachments to request context, change pol… (#141) * Add policy definitions and attachments to request context, change policy executor to use them from context instead of directly from repo * PR comments * allow jwt in param injection (policy authorization can use it through args) (#144) * Support for policy query (#143) * Policy directive - accept only a single policy (#146) * change policy directive to accept only a single policy * Refactored PolicyExecutor API to only expose static methods Co-authored-by: Tomer Eskenazi <tomeresk@gmail.com>
#141) * Add policy definitions and attachments to request context, change policy executor to use them from context instead of directly from repo * PR comments
…icy executor to use them from context instead of directly from repo