fix(jira-site): fix connection, caching, and session-lifecycle bugs by rantoniuk · Pull Request #1200 · jenkinsci/jira-plugin

rantoniuk · 2026-08-23T12:00:03Z

Related issue

Split out of #1198 for a diff-scoped review. Groups the JiraSite-centered fixes
from that PR, since they land on the same file back-to-back and don't cleanly separate further.

Changes

Several correctness and reliability fixes to JiraSite's connection, caching and session handling:

Timeouts: getHttpClientOptions() never called setConnectionTimeout(), so the configured
connect timeout was always overridden by the library's hardcoded 5s default. It also passed the
read-timeout value into setRequestTimeout(), which the vendored Apache HTTP client never reads,
making the "Read timeout" setting a no-op. Both settings now take effect.
Callback executor lifetime: the executor was static but sized from an instance field, and
ApacheAsyncHttpClient.destroy() calls shutdown() on it — closing one site's client disabled Jira
for every site on the controller. Now one pool per site, with correct double-checked locking.
Session scoping: enhance security of the cached Jira session by scoping it to the credentials
that built it, so a site whose session was created under one set of resolved credentials no longer
keeps serving that same session once a caller resolves to different credentials.
Issue cache correctness: "no session yet" was stored as Optional.empty() and served back as
"this issue does not exist"; the cache was also unbounded and expired after access rather than
after write. Fixed, and bounded (-Dhudson.plugins.jira.JiraSite.issueCache.maxSize, default 1000).
Project list staleness: the project key list was fetched once and kept for the life of the JVM;
every failure mode (lock timeout, RestClientException, interrupt) collapsed into a silent empty
set. Now refreshed on a TTL (-Dhudson.plugins.jira.JiraSite.projectKeys.ttlMillis, default 1h) and
failures are logged instead of swallowed, while still serving the last-known-good list.
Redundant Jira calls: transitioning an issue did three identical GETs of it. Added overloads
that take an already-fetched Issue and reuse it across the transition and status-read calls.
Credential migration: a failed SystemCredentialsProvider.save() during credential migration was
logged and then reported as success, leaving the site referencing a credential that only existed in
memory. Now propagates the failure; JiraSite.readResolve() catches it and loads the site without
credentials rather than failing deserialization outright.

Also switches the callback-executor cache from a raw volatile field to AtomicReference (same
double-checked-locking pattern, but backed by a type static analysis recognizes as thread-safe), and
merges a duplicated Javadoc block left over from the session-scoping change.

Tests

mvn compile, mvn spotless:check, and mvn test all pass standalone on this branch.
JiraSiteTest, JiraSessionTest, JiraRestServiceTest, CredentialsHelperTest, and
ChangingWorkflowTest all green (75 tests), including new coverage for each fix above.
I have updated/added relevant documentation in the docs/ directory
I have verified that the Code Coverage is not lower than before / that all the changes are covered as needed
I have tested my changes with a Jira Cloud / Jira Server

Adds open-PR links to the rows they address, matched against each PR's actual diff/description rather than title alone: jenkinsci#1193 (draft, closes jenkinsci#1178), jenkinsci#1199/jenkinsci#1200 (HTTP stack and JiraSite fixes, jenkinsci#1200 alone covers five separate rows across epics 2-3), jenkinsci#1201, jenkinsci#1202, jenkinsci#1205, jenkinsci#1207 (closes jenkinsci#677), and jenkinsci#1190 (in-progress docs work). Adds one new epic 3 row for a correctness bug (jenkinsci#1208) that had no prior row to attach to. Excludes the Renovate dependency-bump PR (jenkinsci#1211) and the two already-textually-referenced older PRs (jenkinsci#562, jenkinsci#746) as out of scope. Also fixes the sidebar's "Roadmap" link, which pointed at a nonexistent roadmap.md instead of this file.

* docs(adr): add modernisation architecture decision records Record the load-bearing decisions behind the modernisation programme, in MADR format, plus an index and template pointer. 0002 Jira Cloud detection and search API routing (#747) — thread an explicit isCloudVersion through our own client construction rather than shadowing UriUtil; the fix works on the already-pinned JRJC 6.0.2 and the detection bug is still unfixed in 8.0.0. 0003 Future of the vendored Atlassian HTTP client — keep and fix in 3.x because the fork carries hudson.ProxyConfiguration support upstream lacks; target apache-httpcomponents-client-5-api in 4.0. 0004 Continuous delivery and version numbering — JEP-229 CD with a manually controlled 3.x prefix and an on-demand trigger, addressing both objections raised on the issue. 0005 Staged deprecation removal — fixes in 3.x, removals batched in 4.0 so users can take the Cloud fix without taking API breakage. 0006 HTTP client lifecycle — why a client is built per request. This is the shipped fix for JENKINS-60536 (9e3b451), not a regression; reverting it reintroduces a build hang, as has already happened. Refs #747, #468, #541 * docs: require doc updates per PR and reference ADRs Every PR that changes user-visible behaviour now needs a matching Markdown change under docs/ in the same PR, keeping the relevant Declarative Pipeline example correct and runnable. New and updated examples use Declarative form; the older scripted snippets get converted as they are touched. Also point CONTRIBUTING.md and AGENTS.md at .github/adr/, which was previously undiscoverable, with a note to check it before "fixing" code that only looks wrong. * docs(adr): relocate ADRs from .github/adr to docs/adr Move the ADR set into the docsify site so they're readable at jenkinsci.github.io/jira-plugin instead of only in the GitHub file browser, and fix all ADR-to-ADR cross-links and the index table for docsify's link resolution (relative to the docs/ root, not the current file's directory) — every sibling reference was broken before this, 404ing from both GitHub's blob view and the docsify site. * docs: overhaul docsify site and contributor docs * docs: add draft modernisation epics and issue backlog Derived from the modernisation plan and ADRs 0002-0006, for manual review before anything is created on GitHub. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: fold System Properties and Maintainers into parent pages Merges docs/system-properties.md into configuration.md (as a "System Properties" section) and docs/maintainers.md into CONTRIBUTING.md (as its "For Maintainers" section), removing both standalone pages and their sidebar entries so related content lives on one page instead of being split across a link. * docs: rewrite features page with a TOC and Pipeline examples Rewrites docs/features.md into a complete, source-verified feature inventory (including JiraCreateIssueNotifier and JiraIssueMigrator, previously undocumented) with a table of contents and a minimal declarative Pipeline example per feature, merging in and retiring docs/usage-examples.md. Points readers at the Jenkins.io Pipeline steps reference as the canonical, auto-generated parameter reference, since it's extracted from this plugin's own source on every release. Updates docs/README.md and AGENTS.md's doc-update instructions to match the new page structure. * docs: add epic 7, Pipeline step parity, targeted at 4.0 Moves the two Pipeline-parity rows buried in epic 3 into a new epic 7, retargeted from 3.x to 4.0: closing the freestyle-only gap (JiraCreateIssueNotifier, JiraIssueMigrator, JiraEnvironmentVariableBuilder) against the Jenkins.io Pipeline Steps Reference and docs/features.md's "What is not yet supported in Pipeline" section, following the same Builder+SimpleBuildStep+@symbol pattern epic 6 already uses to replace JiraVersionCreator/JiraReleaseVersionUpdater. * docs: link open upstream PRs to their epic rows Adds open-PR links to the rows they address, matched against each PR's actual diff/description rather than title alone: #1193 (draft, closes #1178), #1199/#1200 (HTTP stack and JiraSite fixes, #1200 alone covers five separate rows across epics 2-3), #1201, #1202, #1205, #1207 (closes #677), and #1190 (in-progress docs work). Adds one new epic 3 row for a correctness bug (#1208) that had no prior row to attach to. Excludes the Renovate dependency-bump PR (#1211) and the two already-textually-referenced older PRs (#562, #746) as out of scope. Also fixes the sidebar's "Roadmap" link, which pointed at a nonexistent roadmap.md instead of this file. * chore(docs): reorg * docs(roadmap): link every ADR reference to its file * chore: update AGENTS.md --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

JiraSite.getHttpClientOptions() never called setConnectionTimeout(), so the configured connect timeout was always overridden by the library's hardcoded 5s default. It also passed the read-timeout value into setRequestTimeout(), which the vendored Apache HTTP client never reads, making the "Read timeout" setting a no-op; the socket/read timeout it does honor was fed the connect-timeout value instead. Wire timeout -> connection timeout and readTimeout -> socket timeout so both settings take effect. Fixes #215 (cherry picked from commit ad05f98)

JiraSite.executorService was static but sized from the instance field threadExecutorNumber, so whichever site built it first fixed the pool size for the whole controller and every other site's "Thread Executor Size" setting became decorative. The null check sat outside the synchronized block with no re-check inside it, so two threads could both build a pool and orphan one with its threads still running. The damaging part was ownership: the single shared pool was handed to every site's HTTP client as the callback executor, and ApacheAsyncHttpClient.destroy() calls shutdown() on it. Closing one site's client therefore terminated the pool for every site on the controller, and since the field was never reset, every later callback was rejected until Jenkins restarted. Make it a per-site field with correct double-checked locking, rebuild it if it is found shut down, and release it from destroy() - which also stops "Validate Settings" leaking a thread pool per click, since that path builds a throwaway site and destroys it. Drops the LI_LAZY_INIT_STATIC suppression this bug was hiding behind. (cherry picked from commit b02d3b6)

…ials JiraSite caches one Jira session per site, but credentials are resolved per Item - CredentialsProvider.lookupCredentials walks the folder ancestry, so a folder-scoped credential can shadow the global one of the same id. The cached session did not track which credentials it was built from, and getSession() only looked at its Item argument on the very first call. Remember which credentials the cached session was built from and rebuild it whenever the requesting item resolves to different ones. Sessions are still reused for every item resolving to the same credentials, so the common case costs nothing.

getIssue() wrote Optional.empty() into the issue cache whenever there was no session yet, so "we could not reach Jira" was stored - and then served - as "this issue does not exist". Genuine 404s and timeouts, by contrast, throw out of the loader and are never cached, so the caching was exactly the wrong way round. The cache made it worse in two ways: it was unbounded, so every issue key ever matched by the changelog annotator stayed resident, and it expired after *access* rather than after write, so the annotator re-reading a key on every page render kept renewing the poisoned entry instead of letting the two minutes elapse. Check for the session before consulting the cache, bound the cache, and expire entries relative to when they were written. The maximum size can be overridden with -Dhudson.plugins.jira.JiraSite.issueCache.maxSize, matching the existing ioThreadCount tunable. (cherry picked from commit 195fc29)

getProjectKeys() fetched the list once and kept it for the lifetime of the JVM - its own FIXME said so - meaning a newly created Jira project stayed invisible until Jenkins restarted. All three ways the fetch could fail then collapsed into the same silent Collections.emptySet(): - tryLock(3s) returning false fell straight through, logging nothing - RestClientException was caught and discarded, with the exception unused - InterruptedException restored the flag and logged nothing An empty set is not neutral here. JiraChangeLogAnnotator uses it to decide whether an issue belongs to a known project, so every issue link on every changelog page silently disappeared, and nothing in the log said why. Give the list a one-hour TTL (overridable with -Dhudson.plugins.jira.JiraSite.projectKeys.ttlMillis), log each bail-out with a deferred supplier, and keep serving the previous list when a refresh fails instead of reporting "no projects". JiraSession memoises the same list one layer down, so it is now told to drop its copy before each refetch. (cherry picked from commit d6f2857)

Transitioning an issue took three identical GETs of that issue: getAvailableActions(key) fetched it to read its transitions link, progressWorkflowAction(key, id) fetched it again a moment later to hand to the transition call, and a third fetch afterwards reports the resulting status. On a JQL matching 200 issues that is 600 issue fetches where 400 were pure waste, and none of them go through JiraSite's issue cache. Add overloads that take an already-fetched Issue, and have progressMatchingIssues fetch once and reuse it for both calls. The key-taking overloads stay and delegate, since they are public API and JiraCreateIssueNotifier uses them. The issues from the JQL search cannot be reused for this: the search asks for seven named fields and does not include the transitions link, so getTransitions would have nothing to follow. That is called out on the new overload so nobody optimises the remaining fetch away. The duplicate, result-discarding getStatusById() call this issue also named was already removed in 25c62ce. (cherry picked from commit 9b733b0)

migrateCredentials() added the new credential to the in-memory store, caught the IOException from save(), logged a warning, and then returned the same value it returns on success. Callers cannot tell the two apart, so JiraSite stored the id of a credential that was never written to disk. That is worse than it sounds, because the legacy userName and password fields are transient: once the site is saved again they are gone. The site keeps working until the controller restarts and is then permanently broken, with a weeks-old warning as the only clue. Roll the in-memory addition back and propagate. FormException is already on the signature, on both callers and on readResolve, so nothing else changes shape. JiraSite.readResolve catches it and loads the site without credentials rather than letting deserialization fail - aborting there would take the whole global configuration down. (cherry picked from commit 161bef4)

…utor volatile alone is not treated as a thread-safe type by static analysis for a double-checked-locking cache; AtomicReference keeps the same happens-before guarantees while satisfying that check. Also merges a duplicated Javadoc block left over from the session-scoping change, and makes the workflow-transition log message lazily evaluated.

sonarqubecloud · 2026-09-06T22:26:23Z

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
82.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

rantoniuk requested a review from a team as a code owner August 23, 2026 12:00

rantoniuk added the bug label Aug 23, 2026

rantoniuk added this to the 3.x milestone Aug 23, 2026

This was referenced Aug 23, 2026

fix: resolve every bug-labelled issue in the 3.x milestone #1198

Closed

fix(http): apply connect and read timeouts to Jira HTTP client #1197

Closed

rantoniuk added 8 commits September 7, 2026 00:13

rantoniuk force-pushed the fix/jira-site-session-hardening branch from 93c2cd9 to bfe16ae Compare September 6, 2026 22:13

Navigation Menu

Uh oh!

fix(jira-site): fix connection, caching, and session-lifecycle bugs - #1200

fix(jira-site): fix connection, caching, and session-lifecycle bugs#1200
rantoniuk wants to merge 8 commits into
jenkinsci:masterfrom
rantoniuk:fix/jira-site-session-hardening

rantoniuk commented Aug 23, 2026

sonarqubecloud Bot commented Sep 6, 2026

This branch has not been deployed

Labels

1 participant