yuqi1129 commented on PR #12850: URL: https://github.com/apache/gravitino/pull/12850#issuecomment-5522243005
Overall, the validation and fail-closed per-dataset authorization direction is sound, but I do not think this is ready to merge until the multi-metalake context issue is fixed. Two contracts also need to be explicit: - Inputs and outputs currently use the same CAN_ACCESS_METADATA expression. That is reasonable if the requirement is only metadata visibility; if lineage integrity requires the reporter to represent reads and writes, use direction-specific privileges or a dedicated lineage-ingest privilege. - OpenLineage namespace is generic, while this code treats it as a Gravitino metalake and defaults a missing datasetType facet to TABLE. Please document that only Gravitino-normalized identifiers are authorizable and define the behavior for unsupported or external dataset types. Please also update docs/open-api/lineage.yaml with the new 201, 400, and 403 behavior. Test coverage should include role-based authorization across two metalakes, including reversed input and output order; failure at every input and output position; exact resolved identifiers for every supported type; malformed target versus denial versus internal-error status mapping; authorization-enabled and disabled HTTP paths; and verification that no rejected event is dispatched. The current cross-metalake success test stubs isOwner(any()) and therefore misses the role-context bug. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
