smaheshwar-pltr commented on PR #13225:
URL: https://github.com/apache/iceberg/pull/13225#issuecomment-5642253575
@singhpk234 @szlta apologies if I’ve been unclear. I definitely agree it
would be good to get something into this release. But I don't understand what's
being proposed, it seems very non-trivial to me and not quite acceptable to
just decide this without any spec change in.
Let me be concrete:
- What triggers rejection: advertising storage vending, receiving
credentials, or detecting that the delegated credential set is incomplete? Are
we supposed to throw, or discard returned credentials and use configured
credentials?
- Recall that storage credentials can arrive through both
`storage-credentials` and `LoadTableResponse.config`. That config also contains
general REST stuff, planning and ordinary FileIO settings, and custom
implementations can define their own properties. Do we prohibit any
configuration entirely (this is extreme), maintain lists of ignorised
credential properties, provider-specific validation? How do _catalog-level
defaults and overrides_ fit into this?
- The restriction also needs to account for remote signing and credentials
returned by scan planning. Again, this is non-trivial.
- What about failure timing matters too. For example, credentials returned
by a create/register operation are inspected after the server may already have
completed that operation.
I’m struggling to understand the value of this PR with this restrictions and
no KMS vending being available to lift them (there's no KMS vending spec in).
But I'd argue that hybrid configuration is valuable when KMS authorization is
managed externally to the catalog. (I'd also argue that external KMS
authorization is not problematic enough to introduce such intrusive
enforcements^ but I'm not going to block on this).
I think I'm misunderstanding something because I can't imagine that just
deciding all this now in this PR for the next release is good - if there's a
straightforward implementation, please feel free to implement it on top of this
PR, or supersede this PR if that’s easier! Very happy for that. (My point here
is that I'm just not understanding unfortunately)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
