dimas-b commented on code in PR #3826:
URL: https://github.com/apache/polaris/pull/3826#discussion_r2835523960
##########
spec/polaris-catalog-apis/generic-tables-api.yaml:
##########
@@ -256,6 +262,53 @@ components:
items:
$ref:
'../iceberg-rest-catalog-open-api.yaml#/components/schemas/TableIdentifier'
+
+ StorageAccessCredential:
+ type: object
+ required:
+ - prefix
+ - config
+ properties:
+ prefix:
+ type: string
+ description: Indicates a storage location prefix where the
credential is relevant. Clients should choose the most
+ specific prefix (by selecting the longest prefix) if several
credentials of the same type are available.
+ config:
+ type: object
+ description: |
+ Credential configurations for AWS S3, GCP GCS, and Azure ADLS are
supported. The following outlines
Review Comment:
I'd like to further generalize "AWS S3" to "AWS S3 and compatible systems",
please.
##########
spec/polaris-catalog-apis/generic-tables-api.yaml:
##########
@@ -256,6 +262,55 @@ components:
items:
$ref:
'../iceberg-rest-catalog-open-api.yaml#/components/schemas/TableIdentifier'
+
+ StorageAccessCredential:
+ type: object
+ required:
+ - prefix
+ - config
+ properties:
+ prefix:
+ type: string
+ description: Indicates a storage location prefix where the
credential is relevant. Clients should choose the most
+ specific prefix (by selecting the longest prefix) if several
credentials of the same type are available.
+ config:
+ type: object
+ description: |
+ Credential configurations for AWS S3, GCP GCS, and Azure ADLS are
supported. The following outlines
+ the currently supported configuration options:
+
+ ## AWS Configurations
+
+ The following configurations should be respected when working with
tables stored in AWS S3
+ - `s3.access-key-id`: id for credentials that provide access to
the data in S3
+ - `s3.secret-access-key`: secret for credentials that provide
access to data in S3
Review Comment:
Actually, I think adding new properties requires a Polaris Server change
anyway. If that is a given, additionally updating object types in Open API is
not that hard, IMHO. I wonder what other people think. I'll post this to the
`dev` ML for awareness.
##########
spec/polaris-catalog-apis/generic-tables-api.yaml:
##########
@@ -256,6 +262,53 @@ components:
items:
$ref:
'../iceberg-rest-catalog-open-api.yaml#/components/schemas/TableIdentifier'
+
+ StorageAccessCredential:
+ type: object
+ required:
+ - prefix
+ - config
+ properties:
+ prefix:
+ type: string
+ description: Indicates a storage location prefix where the
credential is relevant. Clients should choose the most
+ specific prefix (by selecting the longest prefix) if several
credentials of the same type are available.
+ config:
+ type: object
+ description: |
+ Credential configurations for AWS S3, GCP GCS, and Azure ADLS are
supported. The following outlines
+ the currently supported configuration options:
+
+ ## S3 Configurations
+
+ The following configurations should be respected when working with
tables stored in AWS S3
+ - `s3.access-key-id`: id for credentials that provide access to
the data in S3
+ - `s3.secret-access-key`: secret for credentials that provide
access to data in S3
+ - `s3.session-token`: if present, this value should be used for
as the session token
+ - `s3.session-token-expires-at-ms`: the time the aws session
token expires, in milliseconds
+ Extra properties:
+ - `client.region`: region to configure client for making
requests to AWS
Review Comment:
As discussed in a previous comment about `endpoint`, `client.region` and
`client.refresh-credentials-endpoint` are not themselves credentials. Reporting
them under `StorageAccessCredential` is confusing and may lead to obscure
assumptions on the client side.
I think it is fine to expose these properties to clients, but I believe we
ought to structure the API in a way to clearly separate sensitive data from
general configuration.
--
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]
