Skip to content

fix: close workload identity delegate after auth cleanup failure - #883

Open
sylvesterkaczmarek wants to merge 2 commits into
openai:mainfrom
sylvesterkaczmarek:fix/workload-identity-close-cleanup
Open

fix: close workload identity delegate after auth cleanup failure#883
sylvesterkaczmarek wants to merge 2 commits into
openai:mainfrom
sylvesterkaczmarek:fix/workload-identity-close-cleanup

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown

Summary

Ensure WorkloadIdentityHttpClient.close() always attempts to close its delegate even when workload-identity authentication cleanup throws.

Fixes #882.

Problem

The wrapper currently closes resources sequentially:

override fun close() {
    workloadIdentityAuth?.close()
    delegate.close()
}

A custom SubjectTokenProvider can implement AutoCloseable, and WorkloadIdentityAuth.close() delegates to it. If that cleanup throws, delegate.close() is skipped entirely.

That can leave the underlying HTTP transport, connection pool, executor, or other delegate-owned resources open.

The sibling AuthenticatingHttpClient already handles the same ownership problem correctly by attempting both closes and preserving failures through suppressed exceptions.

Fix

Mirror that failure-preserving close pattern:

  • attempt workload-identity cleanup first;
  • if it fails, still attempt delegate.close();
  • propagate the auth failure as the primary exception;
  • attach a distinct delegate-close failure as a suppressed exception;
  • preserve normal delegate-close propagation when auth cleanup succeeds.

Regression coverage

Extended WorkloadIdentityHttpClientTest to verify:

  • normal close closes both auth and delegate resources;
  • auth cleanup failure still closes the delegate and propagates the original auth failure;
  • when both close operations fail, the delegate failure is retained as a suppressed exception.

Validation

  • branch is based directly on current upstream main at cf942a40074291290634321ad9fe21e514030b4c;
  • branch is 0 commits behind upstream;
  • production diff is 12 additions / 1 deletion in WorkloadIdentityHttpClient.kt;
  • test style and failure semantics mirror the existing AuthenticatingHttpClient close coverage.

Full repository validation is left to GitHub Actions.

Risk

Low. Successful close behavior is unchanged. The only behavioral change is that delegate cleanup is no longer skipped when workload-identity cleanup fails.

@sylvesterkaczmarek
sylvesterkaczmarek requested a review from a team as a code owner August 18, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WorkloadIdentityHttpClient can leak its delegate when auth cleanup throws

1 participant