Entity validation using schema fragments with external entity types - #116
Entity validation using schema fragments with external entity types#116john-h-kastner-aws wants to merge 1 commit into
Conversation
Signed-off-by: jkastner <[email protected]>
|
|
||
| Following the same basic design, there are some alternative syntax options to consider. | ||
|
|
||
| * We could use a different keyword: `extern`, shorter, also used by C and Rust; `import`, perhaps wrongly gives the impression that we're loading in an entity definition from somewhere; `use`, similar to `import`. |
There was a problem hiding this comment.
I like the use of external or extern rather than use / import, which to me implies that we have access to the definition of the imported entity. extern makes it clear this is just a symbol that is declared externally.
There was a problem hiding this comment.
I also like external rather than use/import for these same reasons. The keyword import would make me think of expansion of the entity definition in the fragment that has the import in it.
|
|
||
| When combining multiple fragments, we will attempt to link external entity type declarations with concrete definitions. Entity types are linked when their names match precisely (accounting for any containing namespace), but may never be linked with a builtin or common type definition even if the names are the same. An entity type may be declared as external any number of times, but it may only have one concrete definition across all schema fragments. If there is a definition, then that entity type becomes concrete in the resulting fragment, and is treated exactly the same as any other entity type. When there is no concrete definition the entity type remains external in the resulting fragment, but we do not report an error. | ||
|
|
||
| If every external entity type in a fragment has a corresponding definition, then the fragment is fully concrete. We can construct a complete `Schema` from it and use it exactly as if there were no external entity type declarations. In this situation, external declarations act as a helpful tool for schema authors to explicitly specify the dependencies between different schema fragments, but could be omitted entirely. |
There was a problem hiding this comment.
nit: it says fully concrete but later refers to complete, I prefer complete to talk about a complete or closed schema with no unresolved external references.
| ## Alternatives | ||
|
|
||
| * All undefined type names are assumed to be entity types. This isn't actually an alternative, but it's worth looking at why we can't do this. Schema fragments already have a meaning in which an undefined name may legitimately refer to a common type or action defined in another fragment. Assuming that every unresolved reference is an entity type would silently reinterpret those references and change the meaning of existing fragments, so a fragment could no longer be composed with the definitions it was actually written against. We might also frame this as a soundness issue — an entity that validated against a fragment could fail to validate against the complete schema. | ||
| * Add syntax for tagging the kind of type reference when it's used. E.g., `other: B::<entity>`. If `B` is defined as an entity, then it resolves to `B`, otherwise `B` is assumed to be external. This could also be useful for resolving ambiguity when a schema defines `B` as a common type and an entity type. I don't like this option because it doesn't explicitly say that `B` is external, just that it's an entity. We also wouldn't be able to detect simple typos like `Usr` instead of `User` before building the complete schema. |
There was a problem hiding this comment.
I didn't understand that part, is <entity> a tag that would indicate what kind of type B is?
|
|
||
| To validate a `User` entity we would check that all elements of `my_actions` are one of the declared instances of `Shared::Action`. | ||
|
|
||
| We could not validate a request which applies to an external action. |
There was a problem hiding this comment.
Beyond this limitation, are there other reasons we should not consider an extension to actions?
|
|
||
| Say we have an application managing documents for users. Documents and users are stored and managed separately, so we also want to validate their corresponding Cedar entities independently. In this model, each schema fragment typically defines a single entity type and its relationships. We can create a schema defining the document entity, but, for each document, there is a user who owns that document, so our document schema must reference the user entity type. | ||
|
|
||
| While the current Cedar schema specification allows us to define the `Document` and `User` entities in separate documents (schema fragments), it does not allow us to validate either type of entity until we have composed a complete schema, even when much of the information in the schema is irrelevant. In fact, we may need to retrieve and load a schema fragment when we don't need any information from that fragment just to satisfy the requirement that all entity types referenced in the complete schema are also defined. In a distributed system, loading this information can lead to unnecessary latency when validating entity data. |
There was a problem hiding this comment.
This RFC will be extremely useful for a feature we are building in AVP.
|
|
||
| Following the same basic design, there are some alternative syntax options to consider. | ||
|
|
||
| * We could use a different keyword: `extern`, shorter, also used by C and Rust; `import`, perhaps wrongly gives the impression that we're loading in an entity definition from somewhere; `use`, similar to `import`. |
There was a problem hiding this comment.
I also like external rather than use/import for these same reasons. The keyword import would make me think of expansion of the entity definition in the fragment that has the import in it.
| entity Document in Folder, Application; | ||
| ``` | ||
|
|
||
| Now that `Application` is declared and specified to be a valid parent of `Document`, the entity `Document::"file.txt"` validates as expected. This new fragment is semantically equivalent to the original fragment once it has been composed to form a complete schema. This workaround however makes customers replicate parts of their entity hierarchy in every fragment that needs it. A change to `Application` or `Folder` to allow for more parent entity types would require a corresponding change to the `Document` schema before being reflected in `Document` entity validation. |
There was a problem hiding this comment.
This could be a confusing customer experience as different orgs with separate IAM permissions may manage group memberships separately from a team that owns definitions of leaf nodes in the hierarchy. Replicating parts of the hierarchy would require cross-org/team communication.
| I propose adding `external` as a new Cedar schema keyword. The keyword appears before the `entity` keyword, avoiding any ambiguity between the keyword and entity type names. In this example, we declare that `B` and `C` are entities, so we can resolve the entity type references in the definition of `A`. Note that the external entity declarations can be combined in the same way as regular entity declarations. | ||
|
|
||
| ``` | ||
| external entity B, C; |
| ``` | ||
| external entity Application; | ||
| external entity Folder in Application; | ||
| entity Document in Folder; |
There was a problem hiding this comment.
This is easier to understand compared to in-line version, IMO.
Rendered
Checklist
text/SUMMARY.mdin numerical order