Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-67000] Restore setting of null security manager #507

Merged
merged 1 commit into from
Feb 7, 2022

Conversation

jglick
Copy link
Member

@jglick jglick commented Feb 4, 2022

https://issues.jenkins.io/browse/JENKINS-67000

Reverts #481 pending a more nuanced fix.

@jglick jglick added the bug For changelog: Fixes a bug. label Feb 4, 2022
@jglick jglick force-pushed the SecurityManager-JENKINS-67000 branch from 55922bb to 5a8a2c3 Compare February 4, 2022 17:41
@basil
Copy link
Member

basil commented Feb 4, 2022

As a temporary hotfix, OK.

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

Tbh once we drop Java 8 we can just revert this I think

@jglick
Copy link
Member Author

jglick commented Feb 4, 2022

once we drop Java 8 we can just revert this

Only if we remove JWS support as well. You can run IcedTea on Java 11.

@jglick
Copy link
Member Author

jglick commented Feb 4, 2022

Will leave it to someone who knows how to use the release bot to merge & release this. (Ironically, I think we could use regular JEP-229 for this component if it were not for the signature, which is only used by…JavaWebStart.)

@timja
Copy link
Member

timja commented Feb 4, 2022

once we drop Java 8 we can just revert this

Only if we remove JWS support as well. You can run IcedTea on Java 11.

I doubt people are going to the hassle of setting that up just for this

@timja
Copy link
Member

timja commented Feb 4, 2022

@MarkEWaite
Copy link
Contributor

@jeffret-b when you have reviewed this and decide to merge it, let me know and I can assist with the release machinery.

@jglick
Copy link
Member Author

jglick commented Feb 4, 2022

we don't support it on Java 11 even with iced tea

jenkinsci/jenkins#3766 indeed, which seems incorrect, not least because it is checking the version of Java running on the controller which is irrelevant in this context.

🤷 if we do not care to support IcedTea/OpenWebStart users on Java 11+, fine with me but ought to be a conscious decision mentioned in @MarkEWaite’s jenkinsci/jep#382. At the same time we should consider removing the JAR signature here, which (AFAIK) serves no function in non-JWS usages of remoting.jar and just complicates infrastructure; we can delete the GUI mode from remoting; and we can remove slave-installer and its implementations: https://github.com/jenkinsci/jep/blob/master/jep/230/README.adoc#marking-slave-installer-as-detached

@jeffret-b
Copy link
Contributor

We really should remove the Launch button and all support for WebStart. They cause more confusion and bad behavior than they help. I've tried a couple of times in the past, but encountered some opposition based on unknown usage of the feature. The low level of interest in this regression indicates that this would be an overall gain and would negatively impact few people. We definitely should not carry WS forward into Java 11, even though it's possible. There's also no reason why we need to wait for Java 8 support termination before we take this step.

Copy link
Contributor

@jeffret-b jeffret-b left a comment

Choose a reason for hiding this comment

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

Since this was an accidental regression, it's fine to restore this operation.

Ultimately, we really should remove it.

@jeffret-b
Copy link
Contributor

Let's give this one the 24 hour waiting period for any additional comments or concerns.

@jeffret-b jeffret-b merged commit 4274c17 into jenkinsci:master Feb 7, 2022
@jeffret-b
Copy link
Contributor

@MarkEWaite , can you perform a release when you have a chance?

@jglick jglick deleted the SecurityManager-JENKINS-67000 branch February 7, 2022 14:26
@MarkEWaite
Copy link
Contributor

MarkEWaite commented Feb 7, 2022

@MarkEWaite , can you perform a release when you have a chance?

Release build 4.12 is complete and available on artifactory.

@MarkEWaite
Copy link
Contributor

@jeffret-b could you publish the changelog from GitHub releases?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For changelog: Fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants