sungwy commented on code in PR #15850:
URL: https://github.com/apache/iceberg/pull/15850#discussion_r3070635063
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3522,6 +3522,11 @@ components:
If remote signing for a specific storage provider is enabled, clients
must respect the following configurations when creating a remote signer client:
- `signer.endpoint`: the remote signer endpoint. Required. Can either
be a relative path (to be resolved against `signer.uri`) or an absolute URI.
- `signer.uri`: the base URI to resolve `signer.endpoint` against.
Optional. Only meaningful if `signer.endpoint` is a relative path. Defaults to
the catalog's base URI if not set.
+ - `signer.properties.*`: additional properties to be passed through
to the signer endpoint in remote sign
+ requests. Optional. If such properties are present, signer clients
MUST pass them through to the signer
Review Comment:
I think @singhpk234 raises a valid concern around backward compatibility
with older clients that dont recognize this property. That said, I agree with
@adutra that it would be helpful to decouple that concern from the normative
behavior expected of newer clients that do understand it.
I think it would be helpful to base the decision between `MUST` and `SHOULD`
purely on the semantic role these properties play in the signing protocol. If
they are intended as opaque catalog provided signer context that clients are
expected to forward to the signer endpoint, then I think `MUST` is appropriate.
If they are only optional hints, then `SHOULD` may be more appropriate. I’d
personally like to better understand how these properties are expected to be
used in practice, as that would help me form an opinion here.
Backward compatibility is still a very important concern, but I think it is
best to think of it as a separate concern. Older clients may omit these
properties entirely, and servers can choose how to handle that case. It could
decide to recompute the required data, process the request without it, or
reject the request according to server policy.
And that decision may also depend on how a given deployment has rolled out
support for this behavior across its REST clients and engines. Keeping the
normative requirement separate from backward compatibility concerns gives
implementers more flexibility in how they adopt this property within their own
ecosystem, while allowing the spec to evolve cleanly.
If it's helpful, we could also make that server's compatibility expectation
explicit in the spec as well.
--
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]
