[JENKINS-71077] Allow overriding codePointLimit to allow reading larger yaml files #206

pascal-hofmann · 2023-04-14T16:43:01Z

Since the upgrade to SnakeYAML 1.32 it is not possible to read YAML files greater than 3MB. This PR adds the new, optional attribute codePointLimit to readYaml. This makes it possible to work around this limitation.

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
~~Link to relevant issues in GitHub or Jira~~
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue

MarkEWaite · 2023-04-17T00:17:11Z

Thanks for the pull request. Also reported as JENKINS-71077. I've invited the person that reported the issue to test drive this pull request and report their results.

MarkEWaite · 2023-04-17T00:27:44Z

@pascal-hofmann it would be nice if the pull request title were updated to include [JENKINS-71077] as the start the the title so that the changelog generated by release drafter will include a link to the Jira issue.

pascal-hofmann · 2023-04-17T01:51:17Z

Hi @MarkEWaite,
I've updated the commit message.

Thanks for looking at this PR so quickly. ❤️

Cheers,
Pascal

sbollap1 · 2023-04-17T17:05:39Z

I was able to verify that pipeline-utility-steps:2.15.2-rc651.15f371774808 version of the plugin worked for me after making the necessary changes to my readYaml call, thanks a lot for the help
ex: Data = readYaml file: "mylargeyamlfile.yaml", maxAliasesForCollections: 999, codePointLimit: 104857600

MarkEWaite · 2023-04-17T17:07:37Z

I was able to verify that pipeline-utility-steps:2.15.2-rc651.15f371774808 version of the plugin worked for me

Thanks very much. Could you also check that the Pipeline syntax snippet generator is well behaved when generating sample syntax for the improved readYaml step?

sbollap1 · 2023-04-17T17:46:10Z

I was able to verify that pipeline-utility-steps:2.15.2-rc651.15f371774808 version of the plugin worked for me

Thanks very much. Could you also check that the Pipeline syntax snippet generator is well behaved when generating sample syntax for the improved readYaml step?
I am not seeing the parameters or anything useful being generated, for other sample step, I am seeing the file option at the least, however, on the other versions of jenkins(s) and plugin version also I am seeing same behavior, so its fine.

MarkEWaite · 2023-04-17T20:21:25Z

Thanks for checking. I didn't realize that was a special step that does not have full support in the Pipeline syntax snippet generator. Could you confirm that the online help displayed when the '?' is clicked is reasonable.

sbollap1 · 2023-04-17T20:48:29Z

Thanks for checking. I didn't realize that was a special step that does not have full support in the Pipeline syntax snippet generator. Could you confirm that the online help displayed when the '?' is clicked is reasonable.

yes, that looks accurate and matches with the version I have

MarkEWaite · 2023-04-17T20:50:27Z

That's great. Thanks. I think this has been well tested. Now it is left to the maintainers to review, merge, and release.

pascal-hofmann · 2023-04-17T20:51:53Z

FYI: I just added a missing space to the help text. (Limit for incoming data in bytes.Default…)

vgrutsenko · 2023-04-17T20:59:18Z

src/main/java/org/jenkinsci/plugins/pipeline/utility/steps/conf/ReadYamlStep.java

 */
 public class ReadYamlStep extends AbstractFileOrTextStep {

+	private int codePointLimit = -1;


would it be possible to add a static field/method to set the default value for this setting as its done for the MAX_ALIASES_FOR_COLLECTIONS? then old pipelines can keep using old step signature and stay backward compatible.

I‘ll add this tomorrow. 👍

thank you so much!

I've just added this. Is it ok like this?

Looks good to me, thanks!
I tested this on local jenkins instance with couple of nodes - works fine.

I added MAX_CODE_POINT_LIMIT. With this, the setters/getters also make sense, because they are required for validation / the help page / tests.

pascal-hofmann · 2023-04-19T18:54:54Z

Is there an issue with the CI job or is the long runtime OK?

sbollap1 · 2023-04-20T21:34:47Z

I can see the build is successfull but there are some junut checks that failed. https://ci.jenkins.io/job/Plugins/job/pipeline-utility-steps-plugin/view/change-requests/job/PR-206/5/console however, not sure if this is what's blocking it from moving forward.

rsandell · 2023-04-21T14:20:57Z

src/main/java/org/jenkinsci/plugins/pipeline/utility/steps/conf/ReadYamlStep.java

+	public static int setDefaultCodePointLimit(int defaultCodePointLimit) {
+		DEFAULT_CODE_POINT_LIMIT = defaultCodePointLimit;
+		return DEFAULT_CODE_POINT_LIMIT;
+	}
+	public static int getDefaultCodePointLimit() {
+		return DEFAULT_CODE_POINT_LIMIT;
+	}


This is very redundant. No need for a setter/getter for a pseudo constant.

Suggested change

public static int setDefaultCodePointLimit(int defaultCodePointLimit) {

DEFAULT_CODE_POINT_LIMIT = defaultCodePointLimit;

return DEFAULT_CODE_POINT_LIMIT;

}

public static int getDefaultCodePointLimit() {

return DEFAULT_CODE_POINT_LIMIT;

}

rsandell · 2023-04-21T14:21:31Z

src/main/java/org/jenkinsci/plugins/pipeline/utility/steps/conf/ReadYamlStep.java

+	public static final String DEFAULT_CODE_POINT_LIMIT_PROPERTY = ReadYamlStep.class.getName() + ".DEFAULT_CODE_POINT_LIMIT";
+
+	@SuppressFBWarnings(value={"MS_SHOULD_BE_FINAL"}, justification="Non final so that an admin can adjust the value through the groovy script console without restarting the instance.")
+	private static /*almost final*/ int DEFAULT_CODE_POINT_LIMIT = setDefaultCodePointLimit(Integer.getInteger(DEFAULT_CODE_POINT_LIMIT_PROPERTY, -1));


This is very redundant. No need for a setter/getter for a pseudo constant.

Suggested change

private static /*almost final*/ int DEFAULT_CODE_POINT_LIMIT = setDefaultCodePointLimit(Integer.getInteger(DEFAULT_CODE_POINT_LIMIT_PROPERTY, -1));

private static /*almost final*/ int DEFAULT_CODE_POINT_LIMIT = Integer.getInteger(DEFAULT_CODE_POINT_LIMIT_PROPERTY, -1);

rsandell · 2023-04-21T14:24:54Z

src/main/java/org/jenkinsci/plugins/pipeline/utility/steps/conf/ReadYamlStep.java

+
+	@DataBoundSetter
+	public void setCodePointLimit(final int codePointLimit) {
+		this.codePointLimit = codePointLimit;


This is very similar to masAliasForCollection. So you need to add similar protections as in setMaxAliasesForCollections(...) or a potential nefarious user could sink a Jenkins controller by reading in a couple of GB of data until the instance runs our of memory.

Whoever sets this limit can also just execute random groovy/shell code and sink the instance this way.

No because the pipeline is running in the sandbox.
https://www.jenkins.io/doc/developer/security/misc/#groovy-scripting

Besides, it is needed for users to not shoot themselves in the foot. They don't know how much memory the Jenkins controller has available, but the administrator should.

There is known limitation in pipeline-utility-steps plugin which is blocking us from running deployment pipelines. which is being fixed by jenkinsci/pipeline-utility-steps-plugin#206 until this pull request is merged, this PS is using specific version of the plugin from this pull request. quay.io plugin removed as this is not being used and has a vulnerability warning displayed Change-Id: If9f14b6d65504eea25dc44456a54e2dfa1c4deb5

icep87 · 2023-05-29T06:27:42Z

Will this manage to make it into the next release?

pascal-hofmann · 2023-05-29T07:02:38Z

Hi Piotr,
I did not have time to update my PR according to the review comments yet. I try to squeeze this in this week.

Cheers,
Pascal

pascal-hofmann · 2023-06-12T19:07:04Z

Hey everyone,
I finally managed to work on this again. Could you do another review?

Thanks for your patience.

Cheers,
Pascal

Edit: I just realized there were some conflicts, so I rebased on latest master.

…er yaml files

pascal-hofmann · 2023-06-16T12:35:09Z

@rsandell Can you have another look at this PR?

vgrutsenko · 2023-07-05T15:59:06Z

Thanks @rsandell for merging this. How soon can we expect a new version of the plugin?

rsandell · 2023-07-06T11:55:01Z

Oh, sorry!
I merged this while I was on vacation and forgot to make the release when I came back. Doing the release now.

vgrutsenko · 2023-07-06T14:55:27Z

Perfect, thanks!

Kanav14 · 2023-07-14T18:07:22Z

I was able to verify that pipeline-utility-steps:2.15.2-rc651.15f371774808 version of the plugin worked for me after making the necessary changes to my readYaml call, thanks a lot for the help ex: Data = readYaml file: "mylargeyamlfile.yaml", maxAliasesForCollections: 999, codePointLimit: 104857600

This did not work for me when reading yaml as a variable like below:

readYaml text: newVar, codePointLimit: 52428800

Kanav14 · 2023-07-14T18:13:27Z

, codePointLimit=52428800} for org.jenkinsci.plugins.pipeline.utility.steps.conf.ReadYamlStep: java.lang.reflect.InvocationTargetException

error message

rsandell · 2023-08-21T15:14:41Z

Don't use = for named parameter values, use name: value i.e. , codePointLimit: 52428800}

sblatnick · 2024-01-03T23:21:35Z

We're using 2.16.0 and this is failing from within a shared library class:

def yaml = script.readYaml file: results, codePointLimit: 134217728

With:

16:08:12  java.lang.IllegalArgumentException: Could not instantiate {file=scanner-results.yaml, codePointLimit=134217728} for org.jenkinsci.plugins.pipeline.utility.steps.conf.ReadYamlStep: java.lang.reflect.InvocationTargetException

Any ideas what might be wrong?

rsandell · 2024-03-18T09:51:50Z

Can you get the stacktrace of the InvocationTargetException? It's probably in the system log.

sblatnick · 2024-03-18T16:26:30Z

This is the same issue as JENKINS-72556

Unfortunately, I don't have access to the jenkins system logs, and we found a work-around by using python and only extracting what we need from the yaml.

I don't understand why this wasn't working for us, but unless someone else is seeing this issue, feel free to close it.

cpotter302 · 2024-07-03T10:13:57Z

We're using 2.16.0 and this is failing from within a shared library class:

def yaml = script.readYaml file: results, codePointLimit: 134217728

With:

16:08:12  java.lang.IllegalArgumentException: Could not instantiate {file=scanner-results.yaml, codePointLimit=134217728} for org.jenkinsci.plugins.pipeline.utility.steps.conf.ReadYamlStep: java.lang.reflect.InvocationTargetException

Any ideas what might be wrong?

Having the same issues here. System log is empty unfortunately

rsandell · 2024-08-20T13:18:13Z

Without knowing the exception that caused the InvocationTargetException it will be hard.
Maybe the codePointLimit: 134217728 is too large i.e. larger than what MAX_CODE_POINT_LIMIT is set to.

pascal-hofmann force-pushed the feature/add-codepointlimit branch from 56082ff to c49f637 Compare April 14, 2023 16:50

pascal-hofmann force-pushed the feature/add-codepointlimit branch from c49f637 to 15f3717 Compare April 17, 2023 01:47

pascal-hofmann changed the title ~~Allow overriding codePointLimit to allow reading larger yaml files~~ [JENKINS-71077] Allow overriding codePointLimit to allow reading larger yaml files Apr 17, 2023

pascal-hofmann force-pushed the feature/add-codepointlimit branch from 15f3717 to 1542eae Compare April 17, 2023 20:51

vgrutsenko suggested changes Apr 17, 2023

View reviewed changes

pascal-hofmann force-pushed the feature/add-codepointlimit branch from 1542eae to 9db10ea Compare April 18, 2023 08:57

pascal-hofmann requested a review from vgrutsenko April 18, 2023 08:58

vgrutsenko approved these changes Apr 18, 2023

View reviewed changes

sbollap1 approved these changes Apr 18, 2023

View reviewed changes

MarkEWaite approved these changes Apr 18, 2023

View reviewed changes

rsandell requested changes Apr 21, 2023

View reviewed changes

rsandell added the enhancement label Apr 21, 2023

pascal-hofmann force-pushed the feature/add-codepointlimit branch from 9db10ea to 898ca71 Compare May 19, 2023 11:49

pascal-hofmann force-pushed the feature/add-codepointlimit branch 2 times, most recently from 09cafb6 to 5773678 Compare June 12, 2023 19:04

[JENKINS-71077] Allow overriding codePointLimit to allow reading larg…

78e3fac

…er yaml files

pascal-hofmann force-pushed the feature/add-codepointlimit branch from 5773678 to 78e3fac Compare June 12, 2023 19:12

pascal-hofmann requested a review from rsandell June 14, 2023 12:38

rsandell approved these changes Jun 19, 2023

View reviewed changes

rsandell merged commit b750cd9 into jenkinsci:master Jun 19, 2023

pascal-hofmann deleted the feature/add-codepointlimit branch June 20, 2023 08:56

	private static /almost final/ int DEFAULT_CODE_POINT_LIMIT = setDefaultCodePointLimit(Integer.getInteger(DEFAULT_CODE_POINT_LIMIT_PROPERTY, -1));
	private static /almost final/ int DEFAULT_CODE_POINT_LIMIT = Integer.getInteger(DEFAULT_CODE_POINT_LIMIT_PROPERTY, -1);

[JENKINS-71077] Allow overriding codePointLimit to allow reading larger yaml files #206

[JENKINS-71077] Allow overriding codePointLimit to allow reading larger yaml files #206

Uh oh!

Conversation

pascal-hofmann commented Apr 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarkEWaite commented Apr 17, 2023

Uh oh!

MarkEWaite commented Apr 17, 2023

Uh oh!

pascal-hofmann commented Apr 17, 2023

Uh oh!

sbollap1 commented Apr 17, 2023

Uh oh!

MarkEWaite commented Apr 17, 2023

Uh oh!

sbollap1 commented Apr 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarkEWaite commented Apr 17, 2023

Uh oh!

sbollap1 commented Apr 17, 2023

Uh oh!

MarkEWaite commented Apr 17, 2023

Uh oh!

pascal-hofmann commented Apr 17, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pascal-hofmann commented Apr 19, 2023

Uh oh!

sbollap1 commented Apr 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pascal-hofmann Apr 21, 2023 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

icep87 commented May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pascal-hofmann commented May 29, 2023

Uh oh!

pascal-hofmann commented Jun 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pascal-hofmann commented Jun 16, 2023

Uh oh!

vgrutsenko commented Jul 5, 2023

Uh oh!

rsandell commented Jul 6, 2023

Uh oh!

vgrutsenko commented Jul 6, 2023

Uh oh!

Kanav14 commented Jul 14, 2023

Uh oh!

Kanav14 commented Jul 14, 2023

Uh oh!

rsandell commented Aug 21, 2023

pascal-hofmann commented Apr 14, 2023 •

edited

Loading

sbollap1 commented Apr 17, 2023 •

edited

Loading

sbollap1 commented Apr 20, 2023 •

edited

Loading

pascal-hofmann Apr 21, 2023 •

edited

Loading

icep87 commented May 29, 2023 •

edited

Loading

pascal-hofmann commented Jun 12, 2023 •

edited

Loading