feat: integrate X.509 workload identity with OkHttp clients - #948
Conversation
Castiron custom code✅ No new custom-code files detected. 53 mixed files remain; 0 existing customizations changed. Compared 53 existing customizations unchanged
13 more in the full report. A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 33110056820 --repo openai/openai-java \
--name castiron-custom-code-33110056820-1 --dir /tmp/castiron-custom-code-33110056820-1
git apply --stat /tmp/castiron-custom-code-33110056820-1/custom-code.patch
cat /tmp/castiron-custom-code-33110056820-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 24089bd23b830aeee724472a60f8927493c1fd6b c5ac1a190b7ffe223d51f96f6cc41bd089b5a06f
python3 scripts/castiron/custom_code_report.py report \
--base 24089bd23b830aeee724472a60f8927493c1fd6b \
--head c5ac1a190b7ffe223d51f96f6cc41bd089b5a06f --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-c5ac1a190b7f
cat /tmp/castiron-custom-code-c5ac1a190b7f/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Reviewed a74fec31ee9f786c61923862585373d1f3164810 across all seven changed files and the authentication, retry, lifecycle, and TLS call paths. No additional nonduplicate findings. Please land the already-open transport-origin work in #946 and rebase this integration on it before approval.
Validation was static source review; I did not run the Java or live X.509 suites.
a74fec3 to
93c3f40
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93c3f409c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Re-reviewed 93c3f409c40fea3120ab835baf33c5c17e677250 across all seven changed files and the surrounding authentication, retry, copy, lifecycle, and TLS paths. The prerequisite #946 is now merged and included.
The existing withOptions transport/origin finding remains valid after an independent source-level check. Please address that existing thread before approval; I have not added a duplicate inline comment. No additional nonduplicate findings.
Validation was static source review only; I did not execute tests or live X.509 requests.
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Re-reviewed the complete seven-file PR and the material fix in c5ac1a1. The prior withOptions finding is addressed: synchronous and asynchronous authentication now validate the resolved destination before acquiring a token or attaching Authorization, independently of a replacement transport. I also inspected the public clone regressions and the surrounding fixed-alias TLS, cache/refresh, rejected-token retry, copy, and resource-lifecycle paths. No remaining actionable findings.
Validation was static source review; I did not execute tests or live X.509 requests.
Summary
X509WorkloadIdentityconfiguration for an identity-provider ID, service-account ID, existing fixed-aliasX509Transport, and optional token-refresh buffer.Security and scope
Verification
WorkloadIdentityAuthTest,WorkloadIdentityHttpClientTest, andClientOptionsTestcompatibility coverage.