Implement maxTries parameter by lasselindqvist · Pull Request #1 · apache/maven-jarsigner-plugin

lasselindqvist · 2018-12-19T19:20:19Z

Implements maxTries discussed in MJARSIGNER-41. This change is based on the patch provided in the issue in Jira.

maxTries lets the plugin try signing multiple times if first tries fail. Useful when used with a Time Stamp Authority. Default is 1 so backwards compatibility is fully kept.

bdt-stru · 2019-03-08T08:47:32Z

i do need this enhancement because tsa timeouts on me quite often.
the retrydelay parameter is definitly mandatory too with default value of 0. someone will probably need it.

turns out im the guy who also needs the delay. tsa.starfieldtech.com currently starts timeouting after many requests. having dozens of jars to sign it stops long before it is even close to finishing.

bdt-stru · 2019-03-08T10:36:59Z

@lasselindqvist thanks for sharing your changes. that made it easier for me to also add the retryDelay parameter.
#3

lasselindqvist · 2019-03-19T20:19:53Z

@rfscholte, any chance of getting this merged?

rfscholte · 2019-03-25T19:45:16Z

I need to catch up after a week of absence due to conferences. But the first thing I notice is a missing test. Probably easiest to solve when mocking jarSigner.

maxTries lets the plugin try signing multiple times if first tries fail. Useful when used with a Time Stamp Authority. Default is 1 so backwards compatibility is fully kept.

lasselindqvist · 2019-03-29T17:41:01Z

I added a unit test for the new method and fixed Checkstyle errors while at it. "mvn install" passes.

lasselindqvist · 2019-08-26T16:51:10Z

@rfscholte, possible to merge this? I have been using a forked version for a while and can confirm it works.

Tibor17 · 2019-08-26T20:42:17Z

+
+    @Test
+    public void testSignSuccessOnFirst() throws MojoExecutionException, JavaToolException {
+        AbstractJarsignerMojo mojo = Mockito.mock(AbstractJarsignerMojo.class);


@lasselindqvist Excellent!
This is the first unit test. One advice, pls use Mockito injection in JUnit4+ and keep using our codestyle with spaces and brackets on new line. This may help you to configure Eclipse or IDEA.
https://maven.apache.org/developers/conventions/code.html
Configuration of Ides:
https://maven.apache.org/developers/maven-idea-codestyle.xml
https://maven.apache.org/developers/maven-eclipse-codestyle.xml

I'll try to find time to change the formatting this weekend.

Tibor17 · 2019-08-26T20:46:07Z

+        result1.setExitCode( 1 );
+        JavaToolResult result2 = new JavaToolResult();
+        result2.setExitCode( 1 );
+        Mockito.when( jarSigner.execute( request )).thenReturn( result1 ).thenReturn( result2 );


you can avoid chaining in one line and use multiple lines:

when( ... ) .thenReturn( ... ) .thenReturn( ... );

This way you can avoid too long lines which are broken in Github and it is more readable because your eyes see all important structure/parts of the story sentence in one place: when then... and the arguments are the second important things.

slachiewicz

need more tests - sometimes build fails https://builds.apache.org/view/M-R/view/Maven/job/maven-box/job/maven-jarsigner-plugin/job/MJARSIGNER-41/

slachiewicz · 2020-01-19T13:03:21Z

maybe we should combine this change with #3

lasselindqvist · 2020-01-19T14:12:02Z

The failures seem to be unrelated to this:

AbstractJarsignerMojo.java:667 | Normal | TODO | remove the part with ToolchainManager lookup once we depend on
-- | -- | -- | --
AbstractJarsignerMojo.java:666 | High | FIXME | tchemit-20123-11-13, need to find out how to do this...

if it is indeed the "new" tasks that break the build. The tests pass fine.

jcompagner · 2023-01-31T15:23:51Z

why is this not really picked up?

Our build system now and then breaks constantly because of those TSA servers. i need to manually switch between then (adjusting the mvn settings file) to go to another TSA server now and then, and then that one can have other problems..

we really need what other sign tools already have implemented not just a retry count but also that you can have multiply tsa urls..
so we should be able todo:

<jarsigner.tsa>http://timestamp.sectigo.com,http://rfc3161timestamp.globalsign.com/advanced,http://timestamp.digicert.com</jarsigner.tsa>

then if the first fails it retries it with the second one if that one fails it tries it with the 3rd.
If that one fails it waits for X seconds (retryDelay) and then does that whole thing again according to the maxTries

i think only then we can get a reliable build, now it is just a hit and miss.

jcompagner · 2023-02-01T16:25:38Z

i made a patch for this on top of this jarsigner plugin:

Servoy@bcb2b6a

now it tries all the timestamp urls you give the jarsigner.tsa (separated by ,) and if all of them fail, it will retry them again with a delay that you can set.

schedin · 2023-12-10T13:33:58Z

Since the multiple tsa URL suggestion by @jcompagner was a bit hidden (in the comment of this pull request) I have written a dedicated ticket for it: https://issues.apache.org/jira/projects/MJARSIGNER/issues/MJARSIGNER-74.

jcompagner · 2023-12-11T09:49:04Z

that multi tsa is more important then just a retry, that is because if a tsa service is not working then it can be not working for quite some time (not minutes but more hours or half days....)

thats why we use now a patch version of the jarsigner:

Servoy@bcb2b6a

that does retries but first over all tsa urls that are given (comma separated list)
so my jar signer property is something like:

<jarsigner.tsa>http://timestamp.sectigo.com,http://rfc3161timestamp.globalsign.com/advanced,http://timestamp.digicert.com</jarsigner.tsa>

and then with:

<jarsigner.tsaMaxRetry>3</jarsigner.tsaMaxRetry>
<jarsigner.tsaRetryDelay>5</jarsigner.tsaRetryDelay>

but those last 2 are not that important it could even have retry once, because if the first tsa fails then retrying it over the other 2 will almost all the time work fine

elharo · 2023-12-11T13:19:03Z

superseded by #13

schedin · 2023-12-11T13:25:06Z

@jcompagner: You have a good point. Since this pull request is now closed (because most of the feature request was implemented in #13), do you think the existing ticket https://issues.apache.org/jira/projects/MJARSIGNER/issues/MJARSIGNER-74 (that links to the patch) is enough to track this?

Since I did not have direct need for multiple TSA URLs I did not implement it.

jcompagner · 2023-12-11T13:31:19Z

as i said, for us multiply TSA is way more important then retries, it just need to roll over the TSA once

Because last week Thursday, 1 or 2 projects builds where broken (that where using a ant sign script instead of mvn) and that ant sign script doesn't have that multiply tsa support
and i had to change:

http://timestamp.sectigo.com

to

http://sha256timestamp.ws.symantec.com/sha256/timestamp

because starting in the night until the whole morning the sectigo one failed, so that was many ours..

bdt-stru mentioned this pull request Mar 8, 2019

adds maxTries and retryDelay #3

Closed

lasselindqvist force-pushed the maxTries branch from 1df500a to 7b10ed5 Compare March 29, 2019 17:29

Implement maxTries parameter

67ddfef

maxTries lets the plugin try signing multiple times if first tries fail. Useful when used with a Time Stamp Authority. Default is 1 so backwards compatibility is fully kept.

lasselindqvist force-pushed the maxTries branch from 7b10ed5 to 67ddfef Compare March 29, 2019 17:37

Tibor17 reviewed Aug 26, 2019

View reviewed changes

slachiewicz approved these changes Jan 19, 2020

View reviewed changes

slachiewicz requested changes Jan 19, 2020

View reviewed changes

This was referenced Dec 4, 2023

Internal code review of schedin implementation of MJARSIGNER-41 for maxTries and retryDelay schedin/maven-jarsigner-plugin#2

Merged

[MJARSIGNER-41] Retry and delay feature when signing #13

Merged

elharo closed this Dec 11, 2023

Conversation

lasselindqvist commented Dec 19, 2018

Uh oh!

bdt-stru commented Mar 8, 2019

Uh oh!

bdt-stru commented Mar 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lasselindqvist commented Mar 19, 2019

Uh oh!

rfscholte commented Mar 25, 2019

Uh oh!

lasselindqvist commented Mar 29, 2019

Uh oh!

lasselindqvist commented Aug 26, 2019

Uh oh!

Tibor17 Aug 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lasselindqvist Sep 28, 2019

Choose a reason for hiding this comment

Uh oh!

Tibor17 Aug 26, 2019

Choose a reason for hiding this comment

Uh oh!

Tibor17 Aug 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slachiewicz left a comment

Choose a reason for hiding this comment

Uh oh!

slachiewicz commented Jan 19, 2020

Uh oh!

lasselindqvist commented Jan 19, 2020

Uh oh!

jcompagner commented Jan 31, 2023

Uh oh!

jcompagner commented Feb 1, 2023

Uh oh!

schedin commented Dec 10, 2023

Uh oh!

jcompagner commented Dec 11, 2023

Uh oh!

elharo commented Dec 11, 2023

Uh oh!

schedin commented Dec 11, 2023

Uh oh!

jcompagner commented Dec 11, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

bdt-stru commented Mar 8, 2019 •

edited

Loading

Tibor17 Aug 26, 2019 •

edited

Loading

Tibor17 Aug 26, 2019 •

edited

Loading