Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-56012

stash-pullrequest-builder-plugin variables not seen in multi-configuration jobs unless listed in safeParameters

    • 1.9

      stash-pullrequest-builder-plugin defines 10 variables, such as `sourceCommitHash`. Those variables are not available in multi-configuration jobs apart from the top-level process.

      Those variable can be made available to the jobs by listing them in hudson.model.ParametersAction.safeParameters on the java command line. That's a workaround for SECURITY-170 that is no longer needed for free-style jobs.

      Even with that workaround, Jenkins spams logs with messages like this:

      WARNING: Skipped parameter `sourceCommitHash` as it is undefined on `TestRepository_pull_request_builder`. Set `-Dhudson.model.ParametersAction.keepUndefinedParameters=true` to allow undefined parameters to be injected as environment variables or `-Dhudson.model.ParametersAction.safeParameters=[comma-separated list]` to whitelist specific parameter names, even though it represents a security breach or `-Dhudson.model.ParametersAction.keepUndefinedParameters=false` to no longer show this message.

      Only multi-configuration jobs appear in those messages. The way to suppress them is to also add -Dhudson.model.ParametersAction.keepUndefinedParameters=false to the java command line.

      My expectations are:

      • The variables defined by stash-pullrequest-builder-plugin should be available to multi-configuration jobs
      • No warnings should be logged about those variables
      • No java command line parameters should be needed to achieve it

          [JENKINS-56012] stash-pullrequest-builder-plugin variables not seen in multi-configuration jobs unless listed in safeParameters

          Is this different than JENKINS-51480 ? I intend to change the build params into variables as suggested in ticket description

          Jakub Bochenski added a comment - Is this different than JENKINS-51480 ? I intend to change the build params into variables as suggested in ticket description

          Pavel Roskin added a comment -

          I don't think it's the same. There is no mention of matrix builds. `-Dhudson.model.ParametersAction...`is too imprecise. In my case, listing all 10 variables exported by the plugin in `-Dhudson.model.ParametersAction.safeParameters` was insufficient to suppress the logs. I had to add `-Dhudson.model.ParametersAction.keepUndefinedParameters=false` in addition to that. I would rather not do it, as it suppresses all warnings about all undefined variables from all plugins. Fortunately I don't have any, but I'd rather have those warnings in the log if I even need to debug another issue with variables.

          I looked what variables are passed to the jobs for individual parameter combinations. `GIT_COMMIT` is one such variable. It must be comping from the git-plugin, so it could probably serve as an example.

          Pavel Roskin added a comment - I don't think it's the same. There is no mention of matrix builds. `-Dhudson.model.ParametersAction...`is too imprecise. In my case, listing all 10 variables exported by the plugin in `-Dhudson.model.ParametersAction.safeParameters` was insufficient to suppress the logs. I had to add `-Dhudson.model.ParametersAction.keepUndefinedParameters=false` in addition to that. I would rather not do it, as it suppresses all warnings about all undefined variables from all plugins. Fortunately I don't have any, but I'd rather have those warnings in the log if I even need to debug another issue with variables. I looked what variables are passed to the jobs for individual parameter combinations. `GIT_COMMIT` is one such variable. It must be comping from the git-plugin, so it could probably serve as an example.

          Pavel Roskin added a comment -

          I tried fixing the issues with the environment. There are multiple issues. One issue that is specific to multi-configuration builds is that StashAditionalParameterEnvironmentContributor#buildEnvironmentFor() uses getCause(StashCause.class). The cause of child builds is the top-level build, not StashCause. That should be considered.

          That said, it is likely that this issue and JENKINS-51480 would be fixed together.

          Pavel Roskin added a comment - I tried fixing the issues with the environment. There are multiple issues. One issue that is specific to multi-configuration builds is that StashAditionalParameterEnvironmentContributor#buildEnvironmentFor() uses getCause(StashCause.class). The cause of child builds is the top-level build, not StashCause. That should be considered. That said, it is likely that this issue and JENKINS-51480 would be fixed together.

          Pavel Roskin added a comment -

          The issue of missing environment variables has been addressed by https://github.com/jenkinsci/stash-pullrequest-builder-plugin/pull/44

          The warnings are still there. For a 2-configuration project with consecutive execution, the second child job generates 150 variables, 15 per variable.

          Pavel Roskin added a comment - The issue of missing environment variables has been addressed by https://github.com/jenkinsci/stash-pullrequest-builder-plugin/pull/44 The warnings are still there. For a 2-configuration project with consecutive execution, the second child job generates 150 variables, 15 per variable.

          Pavel Roskin added a comment -

          It looks like the remaining warnings are a migration issue. They happen when the new plugin (i.e. the one with PR44 merged) loads builds created with the old (pre-PR44) plugin.

          I made Jenkins dump stack after reporting the SECURITY-170 warning, and here's what it shows.

           

          WARNING: Skipped parameter `sourceCommitHash` as it is undefined on `TestRepository_Matrix_PR_Builder`. Set `-Dhudson.model.ParametersAction.keepUndefinedParameters=true` to allow undefined parameters to be injected as environment variables or `-Dhudson.model.ParametersAction.safeParameters=[comma-separated list]` to whitelist specific parameter names, even though it represents a security breach or `-Dhudson.model.ParametersAction.keepUndefinedParameters=false` to no longer show this message.
          java.lang.Exception: Stack trace
          at java.lang.Thread.dumpStack(Thread.java:1336)
          at hudson.model.ParametersAction.filter(ParametersAction.java:333)
          at hudson.model.ParametersAction.getParameters(ParametersAction.java:183)
          at hudson.matrix.MatrixChildParametersAction.onLoad(MatrixChildParametersAction.java:87)
          at hudson.model.Run.onLoad(Run.java:364)
          at hudson.model.RunMap.retrieve(RunMap.java:225)
          at hudson.model.RunMap.retrieve(RunMap.java:57)
          at jenkins.model.lazy.AbstractLazyLoadRunMap.load(AbstractLazyLoadRunMap.java:501)
          at jenkins.model.lazy.AbstractLazyLoadRunMap.load(AbstractLazyLoadRunMap.java:483)
          at jenkins.model.lazy.AbstractLazyLoadRunMap.getByNumber(AbstractLazyLoadRunMap.java:381)
          at jenkins.model.lazy.AbstractLazyLoadRunMap.search(AbstractLazyLoadRunMap.java:346)
          at jenkins.model.lazy.LazyLoadRunMapEntrySet$1.next(LazyLoadRunMapEntrySet.java:74)
          at jenkins.model.lazy.LazyLoadRunMapEntrySet$1.next(LazyLoadRunMapEntrySet.java:63)
          at java.util.AbstractMap$2$1.next(AbstractMap.java:418)
          at hudson.matrix.LinkedLogRotator.perform(LinkedLogRotator.java:69)
          at hudson.model.Job.logRotate(Job.java:468)
          at hudson.model.Run.execute(Run.java:1880)
          at hudson.matrix.MatrixRun.run(MatrixRun.java:153)
          at hudson.model.ResourceController.execute(ResourceController.java:97)
          at hudson.model.Executor.run(Executor.java:429)

           

          Maybe stash-pullrequest-builder-plugin should still use and recognize the parameters? I see that the GitHub pull request builder took a different approach and used a class derived from ParametersAction. I know, that plugin is obsolete, but I'm sure it was getting more attention than stash-pullrequest-builder-plugin.

          Pavel Roskin added a comment - It looks like the remaining warnings are a migration issue. They happen when the new plugin (i.e. the one with PR44 merged) loads builds created with the old (pre-PR44) plugin. I made Jenkins dump stack after reporting the SECURITY-170 warning, and here's what it shows.   WARNING: Skipped parameter `sourceCommitHash` as it is undefined on `TestRepository_Matrix_PR_Builder`. Set `-Dhudson.model.ParametersAction.keepUndefinedParameters=true` to allow undefined parameters to be injected as environment variables or `-Dhudson.model.ParametersAction.safeParameters= [comma-separated list] ` to whitelist specific parameter names, even though it represents a security breach or `-Dhudson.model.ParametersAction.keepUndefinedParameters=false` to no longer show this message. java.lang.Exception: Stack trace at java.lang.Thread.dumpStack(Thread.java:1336) at hudson.model.ParametersAction.filter(ParametersAction.java:333) at hudson.model.ParametersAction.getParameters(ParametersAction.java:183) at hudson.matrix.MatrixChildParametersAction.onLoad(MatrixChildParametersAction.java:87) at hudson.model.Run.onLoad(Run.java:364) at hudson.model.RunMap.retrieve(RunMap.java:225) at hudson.model.RunMap.retrieve(RunMap.java:57) at jenkins.model.lazy.AbstractLazyLoadRunMap.load(AbstractLazyLoadRunMap.java:501) at jenkins.model.lazy.AbstractLazyLoadRunMap.load(AbstractLazyLoadRunMap.java:483) at jenkins.model.lazy.AbstractLazyLoadRunMap.getByNumber(AbstractLazyLoadRunMap.java:381) at jenkins.model.lazy.AbstractLazyLoadRunMap.search(AbstractLazyLoadRunMap.java:346) at jenkins.model.lazy.LazyLoadRunMapEntrySet$1.next(LazyLoadRunMapEntrySet.java:74) at jenkins.model.lazy.LazyLoadRunMapEntrySet$1.next(LazyLoadRunMapEntrySet.java:63) at java.util.AbstractMap$2$1.next(AbstractMap.java:418) at hudson.matrix.LinkedLogRotator.perform(LinkedLogRotator.java:69) at hudson.model.Job.logRotate(Job.java:468) at hudson.model.Run.execute(Run.java:1880) at hudson.matrix.MatrixRun.run(MatrixRun.java:153) at hudson.model.ResourceController.execute(ResourceController.java:97) at hudson.model.Executor.run(Executor.java:429)   Maybe stash-pullrequest-builder-plugin should still use and recognize the parameters? I see that the GitHub pull request builder took a different approach and used a class derived from ParametersAction. I know, that plugin is obsolete, but I'm sure it was getting more attention than stash-pullrequest-builder-plugin.

          Pavel Roskin added a comment -

          The issue is resolved as originally stated. The issue of warnings from historic builds will be reported and addressed separately.

          Pavel Roskin added a comment - The issue is resolved as originally stated. The issue of warnings from historic builds will be reported and addressed separately.

          Pavel Roskin added a comment -

          Pavel Roskin added a comment - Fixed in https://github.com/jenkinsci/stash-pullrequest-builder-plugin/pull/44

            proski Pavel Roskin
            proski Pavel Roskin
            Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: