c-thiel commented on code in PR #10722:
URL: https://github.com/apache/iceberg/pull/10722#discussion_r1706467120
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2747,6 +2747,81 @@ components:
uuid:
type: string
+ ADLSCredentials:
+ type: object
+ allOf:
+ - $ref: '#/components/schemas/Credentials'
+ required:
+ - type
+ properties:
+ type:
+ type: string
+ enum: [ "adls" ]
+ account-name:
+ type: string
+ account-key:
+ type: string
+ sas-token:
+ type: string
+ expires-at-ms:
+ type: integer
+ format: int64
+
+ GCSCredentials:
+ type: object
+ allOf:
+ - $ref: '#/components/schemas/Credentials'
+ required:
+ - type
+ - token
+ - expires-at-ms
+ properties:
+ type:
+ type: string
+ enum: [ "gcs" ]
+ token:
+ type: string
+ expires-at-ms:
+ type: integer
+ format: int64
+
+ S3Credentials:
+ type: object
+ allOf:
+ - $ref: '#/components/schemas/Credentials'
+ required:
+ - type
+ - access-key-id
+ - secret-access-key
+ - session-token
+ - expires-at-ms
+ properties:
+ type:
+ type: string
+ enum: [ "s3" ]
+ access-key-id:
+ type: string
+ secret-access-key:
+ type: string
+ session-token:
+ type: string
+ expires-at-ms:
+ type: integer
+ format: int64
+
+ Credentials:
+ type: object
+ discriminator:
+ propertyName: type
+ mapping:
+ adls: '#/components/schemas/ADLSCredentials'
+ gcs: '#/components/schemas/GCSCredentials'
+ s3: '#/components/schemas/S3Credentials'
+ oneOf:
Review Comment:
I think it would make sense to add S3RemoteSigning as another form of
Configuration.
Currently it is initiated via "s3.remote-signing-enabled" config attribute
for Table endpoints with the "s3.signer.uri" only being passed at the initial
config endpoint. It's strictly speaking not a Credential, but it serves the
same purpose and makes sense to standardize.
The selection of S3 Configuration can then be made by the server based on
what is specified via the `X-Iceberg-Access-Delegation` header.
https://github.com/apache/iceberg/blob/257b1d7b18f638b5925de32bcd9bbcbe5a4416c2/open-api/rest-catalog-open-api.yaml#L1493
One problem in almost all client implementations today is that this header
is not implemented according to spec - i.e. even clients supporting both forms
of S3 access only supply a single attribute and not a list of supported
methods. This is a problem as the client can't know if an STS Endpoint is
available or not for this specific S3 storage. Thus clients should supply all
supported forms and leave the decision to the server. I think it would make
sense making this more specific in the spec.
Currently with tabular.io, we get a 400 error if the client specifies
`remote-signing,vended-credentials` as `X-Iceberg-Access-Delegation` - which
shows that no client follows the desired behavior.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
