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

Option to disallow `properties` step on specific pipeline job

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      So I just really discovered the the `properties` step... I can see a bunch of reasons to use it in multibranch and regular pipelines... but wow... that can really wreck a job configuration that you don't want people to mess with, huh?

      For example, we're using the gitlab-plugin. The setup for that isn't the most painful thing, but it also isn't super simple – either way, its pretty important.

      It's actually really bad that a developer can, without realizing what they are doing, add a properties step to a jenkinsfile and break the integration point.  Sure, we have code review to catch that sort of thing. But that doesn't really help if:

      • Naive developer adds `properties` step
      • Sets up merge request
      • Build automatically triggered
      • Build removes gitlab integration
      • All other merge requests blocked until project is reconfigured
      • (ノಥ,_」ಥ)ノ彡┻━┻

      It would be really great if there was a way to say that the properties step is not allowed for a given job. This feature would need to exist for all pipeline jobs and not just multibranch pipeline.

        Attachments

          Activity

          crussell52 Chris Russell created issue -
          Hide
          jglick Jesse Glick added a comment -

          Sounds like some misbehavior of the Gitlab plugin offhand. Not familiar with it.

          Show
          jglick Jesse Glick added a comment - Sounds like some misbehavior of the Gitlab plugin offhand. Not familiar with it.
          jglick Jesse Glick made changes -
          Field Original Value New Value
          Component/s gitlab-plugin [ 19326 ]
          Component/s workflow-multibranch-plugin [ 21465 ]
          Labels multibranch
          Hide
          owenmehegan Owen Mehegan added a comment -

          It's not misbehavior of the plugin, it's basically https://issues.jenkins-ci.org/browse/JENKINS-44848. The issue is that we (the plugin maintainers) understood that a properties step was the only way to refer to a global Jenkins property, such as 1 of possibly >1 different GitLab hosts, in a Pipeline job. Chris Russell is just expressing surprise that using a properties step overrides all previously-used properties steps. I think that is documented well enough in Jenkins. What I'm wondering, as the gitlab-plugin maintainer, is, is there a different way we should be doing this? I pinged Andrew Bayer about this (https://github.com/jenkinsci/gitlab-plugin/issues/475) but I'm not surprised that he hasn't had a chance to respond yet.

          Show
          owenmehegan Owen Mehegan added a comment - It's not misbehavior of the plugin, it's basically https://issues.jenkins-ci.org/browse/JENKINS-44848 . The issue is that we (the plugin maintainers) understood that a properties step was the only way to refer to a global Jenkins property, such as 1 of possibly >1 different GitLab hosts, in a Pipeline job. Chris Russell is just expressing surprise that using a properties step overrides all previously-used properties steps. I think that is documented well enough in Jenkins. What I'm wondering, as the gitlab-plugin maintainer, is, is there a different way we should be doing this? I pinged Andrew Bayer about this ( https://github.com/jenkinsci/gitlab-plugin/issues/475)  but I'm not surprised that he hasn't had a chance to respond yet.
          owenmehegan Owen Mehegan made changes -
          Assignee Owen Mehegan [ owenmehegan ]
          owenmehegan Owen Mehegan made changes -
          Component/s pipeline [ 21692 ]
          Hide
          owenmehegan Owen Mehegan added a comment -

          Adding pipeline component because ultimately this sounds like gitlab-plugin may need to change its integration with that, if we are to resolve this issue.

          Show
          owenmehegan Owen Mehegan added a comment - Adding pipeline component because ultimately this sounds like gitlab-plugin may need to change its integration with that, if we are to resolve this issue.
          Hide
          abayer Andrew Bayer added a comment -

          Oh yeaaaaaah. So, yeah, properties overrides any JobProperty set by any previous invocation of the properties step, either in that run or in a previous run. Until recently, it would override any JobProperty set from the UI or some other mechanism as well, but as of workflow-multibranch 2.16, it's now smart enough to only nuke things that have been set by properties. But it will still overwrite a previous properties call and there isn't a lot we can do about that: the only alternative I came up with (and it was a crappy alternative) was to have separate addProperty and removeProperty steps, so you'd have to explicitly remove a JobProperty via removeProperty if you no longer wanted it...it was a bad idea.

          An alternative for you could be to use a JobProperty behind the scenes but only interact with it through a gitlab-plugin-provided step. Since you'd be setting the JobProperty outside of the properties step, it wouldn't get touched by any other iterations of the properties step.

          Show
          abayer Andrew Bayer added a comment - Oh yeaaaaaah. So, yeah, properties overrides any JobProperty set by any previous invocation of the properties step, either in that run or in a previous run. Until recently, it would override any JobProperty set from the UI or some other mechanism as well, but as of workflow-multibranch 2.16, it's now smart enough to only nuke things that have been set by properties . But it will still overwrite a previous properties call and there isn't a lot we can do about that: the only alternative I came up with (and it was a crappy alternative) was to have separate addProperty and removeProperty steps, so you'd have to explicitly remove a JobProperty via removeProperty if you no longer wanted it...it was a bad idea. An alternative for you could be to use a JobProperty behind the scenes but only interact with it through a gitlab-plugin-provided step. Since you'd be setting the JobProperty outside of the properties step, it wouldn't get touched by any other iterations of the properties step.
          Hide
          owenmehegan Owen Mehegan added a comment -

          Andrew Bayer I get it, thanks for the explanation! I'll see if we can explore that idea of making our own step to handle this. 

          Show
          owenmehegan Owen Mehegan added a comment - Andrew Bayer I get it, thanks for the explanation! I'll see if we can explore that idea of making our own step to handle this. 
          owenmehegan Owen Mehegan made changes -
          Resolution Fixed [ 1 ]
          Status Open [ 1 ] Closed [ 6 ]
          Hide
          jglick Jesse Glick added a comment -

          “Fixed” resolution refers to a specific bug for which a software fix was delivered.

          Show
          jglick Jesse Glick added a comment - “Fixed” resolution refers to a specific bug for which a software fix was delivered.
          jglick Jesse Glick made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          Hide
          jglick Jesse Glick added a comment -

          …whereas this was not. In fact I have no idea why the Gitlab plugin would be using a JobProperty to begin with. I suspect you are doing something the wrong way. Read https://github.com/jenkinsci/scm-api-plugin/blob/master/docs/implementation.adoc and ask on the dev list.

          Show
          jglick Jesse Glick added a comment - …whereas this was not. In fact I have no idea why the Gitlab plugin would be using a JobProperty to begin with. I suspect you are doing something the wrong way. Read https://github.com/jenkinsci/scm-api-plugin/blob/master/docs/implementation.adoc  and ask on the dev list.
          jglick Jesse Glick made changes -
          Resolution Incomplete [ 4 ]
          Status Reopened [ 4 ] Resolved [ 5 ]
          Hide
          crussell52 Chris Russell added a comment -

          FWIW, my original frustration was the fact that the properties step overwrote properties which I had manually set up (e.g. job parameters) which were important to our gitlab integration workflow.

          Meaning a Jenkinsfile author could undo my purposely configured parameters, even though they did not have permissions to configure the job. It looks like this was addressed in JENKINS-44848 and that my original concern has been addressed:

          Until recently, it would override any JobProperty }}set from the UI or some other mechanism as well, but as of {{workflow-multibranch 2.16, it's now smart enough to only nuke things that have been set by properties

          Owen Mehegan, I hope my naive explanation did not send you on a wild goose chase. I am unfamiliar with the issues currently being discussed, so I am going to unsubscribe from this ticket. Please bring me back in, if you need more info from me.

          Show
          crussell52 Chris Russell added a comment - FWIW, my original frustration was the fact that the properties  step overwrote properties which I had manually set up (e.g. job parameters) which were important to our gitlab integration workflow. Meaning a Jenkinsfile author could undo my purposely configured parameters, even though they did not have permissions to configure the job. It looks like this was addressed in JENKINS-44848 and that my original concern has been addressed: Until recently, it would override any  JobProperty }}set from the UI or some other mechanism as well, but as of {{workflow-multibranch  2.16, it's now smart enough to only nuke things that have been set by  properties .  Owen Mehegan , I hope my naive explanation did not send you on a wild goose chase. I am unfamiliar with the issues currently being discussed, so I am going to unsubscribe from this ticket. Please bring me back in, if you need more info from me.

            People

            Assignee:
            owenmehegan Owen Mehegan
            Reporter:
            crussell52 Chris Russell
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: