Skip to content

[JEP-222] Add experimental WebSocket support + Jetty update in Winstone + Remoting 4.0 - #1

Open
Chi-Song-Owen wants to merge 298 commits into
masterfrom
websocket
Open

[JEP-222] Add experimental WebSocket support + Jetty update in Winstone + Remoting 4.0#1
Chi-Song-Owen wants to merge 298 commits into
masterfrom
websocket

Conversation

@Chi-Song-Owen

Copy link
Copy Markdown

See JEP-222. Downstream of jenkinsci/winstone#79 + jenkinsci/remoting#357 + jenkinsci/jenkins-test-harness#183.

Proposed Changelog Entries

oleg-nenashev and others added 30 commits November 12, 2019 15:06
[JENKINS-59631] Fix styling issue in build history table
* Try to execute flyweight tasks on the master

* Fix offline behavior, add tests

* Remove obsolete TODOs, also check #isAcceptingTasks
[JENKINS-59793] Avoid hanging jobs with faulty SubTasks
The jenkins-slave.xml file has been copied to the windows-slave-installer-module,
but a unused (old) copy is still present in the jenkins core.
…pane of the Plugin Manager (#4298)

* refs #JENKINS-59665 Fixed sorting for uninstall column of installed in plugin manager

* refs #JENKINS-59665 removed test attribute
* Hide password form fields by default

* Trying to bypass enforced autocompletion by just having a test field at first

Something like onfocus didn't work, you'd tab through form elements
and unless you filled in the user name, changing the form field to
password would cause it to autocomplete.

It looks like, at least in Mac/Firefox, going from plain text to password
in the 'oninput' event handler works. The plain text is revealed neither
with typing nor pasting.

* Update core/src/main/resources/lib/form/password.jelly

Co-Authored-By: daniel-beck <daniel-beck@users.noreply.github.com>

* Use previously defined value

* Make new password form fields opt-out

* Add support for redacting form secrets in new password field

* Have a password value pre-set in Jelly

* Fix method

* Fix test by clicking on the button to change the password

* Forgot period separator between class and property name
[JENKINS-59508] Sidebar links show full label on hover
Switch from maven-jenkins-dev-plugin to upstream jetty-maven-plugin
Add StandardOpenOption.CREATE flag to create FileChannelWriter to avoid full fs flush and 5sec log operation on creating empty file with CephFS as a storage
* JENKINS-6722 Show slave error status on Dashboard

* Update executors.properties
…unds on the plugin side (#3935)

* JENKINS-56553 Make proxy config compatible with JCASC

* Use DataBoundSetters as other fields are optional

* Fix Jelly for ProxyConfiguration

* Use default field name now that field matches

* Fix validate proxy

* Change field type

* After review

* Update core/src/main/java/jenkins/model/Jenkins.java

Co-Authored-By: timja <t.jacomb@kainos.com>

* Update core/src/main/java/jenkins/model/Jenkins.java

Co-Authored-By: timja <t.jacomb@kainos.com>

* Update core/src/main/java/hudson/ProxyConfiguration.java

Co-Authored-By: timja <t.jacomb@kainos.com>

* Update core/src/main/java/jenkins/model/Jenkins.java

Co-Authored-By: timja <t.jacomb@kainos.com>

* Update core/src/main/java/jenkins/model/Jenkins.java

Co-Authored-By: timja <t.jacomb@kainos.com>

* Update core/src/main/java/hudson/ProxyConfiguration.java

Co-Authored-By: Oleg Nenashev <o.v.nenashev@gmail.com>

* Tweak javadoc

* Align fields

* Update core/src/main/java/hudson/ProxyConfiguration.java

Co-Authored-By: Adrien Lecharpentier <adrien.lecharpentier@gmail.com>
Document workaround for Stapler data binding bug in repeatableProperty
…ling events (#4345)

* Rejuvenate log levels

* clean up

* Reverting the plugin download

Co-Authored-By: Oleg Nenashev <o.v.nenashev@gmail.com>
jsoref and others added 29 commits January 10, 2020 12:26
… Fix JavaScript error in Plugin Manager when optional dependency metadata cannot be retrieved (#3904)

* [JENKINS-56173] Uncaught TypeError: Cannot read property 'jenkinsPluginMetadata' of undefined

* [JENKINS-56173] missing dependencies covers both not installed and not enabled

* [JENKINS-56152] i18n: DirectlyDependentPlugins.title / IndirectlyDependentPlugins.title

* [JENKINS-56152] replace `version X` with `(X)`

* [JENKINS-56152] simplify messages; replace `version X` with `(X)`; use higher instead of later

* [JENKINS-56173] Uncaught Exception: TypeError: Cannot read property 'get' of undefined at _createResponder

* Update PluginWrapperTest

* Switch to descriptive class for plugin version
[JENKINS-42658] - Fix NPE in Agent API when the agent is offline (e.g. retrieving agent version or OS description)
Inlining test-pom into test and removing test-jdk8
…IZABLE, ES_COMPARING_STRINGS_WITH_EQ) (#4379)

* fixed Spotbugs issues (SE_COMPARATOR_SHOULD_BE_SERIALIZABLE, ES_COMPARING_STRINGS_WITH_EQ)

* replaced wildcard import with specific imports

* added missing import

* fixed spotbugs issues

* removed accidently added import
…stages (#4207)

* [JENKINS-59412] Clarify that build history does not include pipeline stages

* Clarify that pipelines are merely an example of tasks that may not be listed
Simplify CoreUpdateMonitor/message.properties: changelog.url → jenkins-version.properties
* Add the maintainer checklist to the pull request template

* Add checklist items for LTS

* Add "Proposed changelog entries" to the pull request template

* Pull request template: `Internal: ` prefix is now added automatically by the Core Changelog generator

jenkinsci/core-changelog-generator#6

* Apply suggestions from review by @MarkEWaite

Co-Authored-By: Mark Waite <mark.earl.waite@gmail.com>

Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Upload draft Changelog YAMLs to GitHub Release assets
…or-yaml

Revert "Upload draft Changelog YAMLs to GitHub Release assets"
[JENKINS-60740] - Rely on GitHub Actions to generate the release draft YAMLs
Release Drafter Fix: Properly inherit the parent config
[JENKINS-60716] Retain causes of a LogRotator failure for diagnosis
* Spotbugs: Fixed comparing strings with equals instead of ==

* Spotbugs: Do not return null in @nonnull marked method, instead return empty string.

* Added Suppress for Spotbugs

* Apply suggestions from code review

Co-Authored-By: Oleg Nenashev <o.v.nenashev@gmail.com>

* fixed javadoc

Co-authored-by: Oleg Nenashev <o.v.nenashev@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fa2b19339a

ℹ️ 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".

Comment on lines +91 to +95
state.fireAfterProperties(ImmutableMap.of(
// TODO or just pass all request headers?
JnlpConnectionState.CLIENT_NAME_KEY, agent,
JnlpConnectionState.SECRET_KEY, secret
));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Propagate the reconnect cookie into channel state

When an agent reconnects before its old channel has been cleared, DefaultJnlpSlaveReceiver permits replacement only if the event's COOKIE_KEY matches the cookie stored on that channel. This properties map omits the cookie, while line 100 merely returns a newly generated value in the response, so beforeChannel never stores it and a later reconnect is rejected as “already connected.” Pass the selected/generated cookie through fireAfterProperties and return that same value to the agent.

Useful? React with 👍 / 👎.

Comment on lines +119 to +125
Computer.threadPoolForRemoting.submit(() -> {
LOGGER.fine(() -> "setting up channel for " + agent);
state.fireBeforeChannel(new ChannelBuilder(agent, Computer.threadPoolForRemoting));
state.fireAfterChannel(state.getChannelBuilder().build(new Transport()));
LOGGER.fine(() -> "set up channel for " + agent);
return null;
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Initialize the transport before returning from open

Under remoting-executor saturation or simply a fast peer, opened() returns after only submitting channel setup, allowing Jetty to deliver a binary frame or close callback before Transport.setup assigns receiver. Both callbacks then dereference a null receiver, dropping an otherwise valid agent connection. Complete receiver initialization synchronously before returning, or buffer callbacks until the submitted setup finishes.

Useful? React with 👍 / 👎.

Comment on lines +309 to +312
class CLIEndpoint extends Endpoint {
@Override
protected void onExit(int code) {
this.exit = code;
finished();
}
public void onOpen(Session session, EndpointConfig config) {}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Complete the CLI wait when the WebSocket closes

If an established WebSocket closes before an EXIT frame arrives—for example during a Jenkins restart, proxy timeout, or server-side command failure—this endpoint's inherited onClose/onError methods do nothing. Consequently ClientSideImpl.handleClose() is never called and connection.exit() waits indefinitely. Override the closure/error callbacks and notify the connection so the CLI terminates rather than hanging.

Useful? React with 👍 / 👎.

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.