iremcaginyurtturk commented on PR #1999: URL: https://github.com/apache/iceberg-go/pull/1999#issuecomment-5597162750
All four review findings addressed: **1. (major) Fix pinned by no test** — `cca7c83` adds `TestSigV4SignsWithPropsCredentials`: it stands up an httptest server, passes creds only via `WithAdditionalProps`, and asserts the captured `Authorization` header contains the props access key (`Credential=AKIDEXAMPLEPROPS/`). Verified by mutation — deleting `cfg.Credentials = creds` makes it fail (`no EC2 IMDS role found`). **2. (major) Partial `s3.*` pair signs as a different principal** — `cca7c83`: `staticCredsFromProps` now returns an error when exactly one of `s3.access-key-id` / `s3.secret-access-key` is set, naming the required properties. Fully-empty still falls through to the default chain. `TestStaticCredsFromProps` updated to assert the error cases instead of enshrining the old behavior. **3. (minor) Undocumented precedence** — `48164f5`: documented the `WithAwsConfig` > `s3.*` props > default-chain order on `WithSigV4` (godoc) and in the `rest.sigv4-enabled` row of `website/src/configuration.md`. **4. (minor) Server `/v1/config` override can select the signing identity** — confirming intent: this is acceptable. The REST control plane is a trusted input per SECURITY-THREAT-MODEL.md and no secret reaches a new audience (the server already knows any creds it injected). If you'd prefer the signing identity be pinned to client-supplied properties regardless of server overrides, I'm happy to capture the client creds before `fetchConfig` folds in the overrides — just let me know. Full `catalog/rest` suite passes; `go vet` clean. -- 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]
