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

Different error handling needed when Job is triggered by user or by Timer if no file is provided #54

Open
ValdarXY opened this issue May 28, 2021 · 9 comments
Labels
bug Something isn't working

Comments

@ValdarXY
Copy link

ValdarXY commented May 28, 2021

Version report

Jenkins and plugins versions report:

Jenkins: 2.277.4
OS: Linux - 4.9.0-15-amd64
---
external-monitor-job:1.7
ace-editor:1.1
matrix-project:1.18
antisamy-markup-formatter:2.1
workflow-step-api:2.23
bootstrap4-api:4.6.0-3
PrioritySorter:4.0.0
analysis-model-api:10.2.5
authentication-tokens:1.4
workflow-multibranch:2.23
display-url-api:2.3.4
timestamper:1.13
command-launcher:1.6
build-monitor-plugin:1.12+build.201809061734
script-security:1.76
lockable-resources:2.10
resource-disposer:0.15
ssh-credentials:1.18.1
htmlpublisher:1.25
role-strategy:3.1.1
data-tables-api:1.10.23-3
git-server:1.9
build-timeout:1.20
pipeline-stage-view:2.19
variant:1.4
scm-api:2.6.4
jjwt-api:0.11.2-9.c8b45b8bb173
xunit:3.0.2
workflow-api:2.42
branch-api:2.6.4
cloudbees-folder:6.15
workflow-job:2.40
okhttp-api:3.14.9
claim:2.18.2
pam-auth:1.6
publish-over:0.22
pipeline-rest-api:2.19
pipeline-stage-step:2.5
test-results-analyzer:0.3.5
ssh-slaves:1.31.5
extended-read-permission:3.2
subversion:2.14.1
favorite:2.3.3
xml-job-to-job-dsl:0.1.13
apache-httpcomponents-client-4-api:4.5.13-1.0
git:4.7.1
junit:1.49
jquery3-api:3.6.0-1
pipeline-input-step:2.12
email-ext:2.82
font-awesome-api:5.15.3-2
snakeyaml-api:1.27.0
mapdb-api:1.0.9.0
pipeline-model-definition:1.8.4
structs:1.23
gradle:1.36
pipeline-model-api:1.8.4
workflow-aggregator:2.6
jackson2-api:2.12.3
token-macro:2.15
pubsub-light:1.14
checks-api:1.7.0
git-client:3.7.1
job-dsl:1.77
credentials-binding:1.24
echarts-api:5.1.0-2
workflow-support:3.8
jdk-tool:1.5
workflow-durable-task-step:2.38
popper-api:1.16.1-2
pipeline-stage-tags-metadata:1.8.4
ws-cleanup:0.39
github-api:1.123
windows-slaves:1.8
workflow-cps:2.91
plugin-util-api:2.2.0
credentials:2.4.1
ansicolor:0.7.5
docker-workflow:1.26
dtkit-api:3.0.0
jquery-detached:1.2.1
file-operations:1.11
trilead-api:1.0.13
forensics-api:1.0.0
matrix-auth:2.6.6
plain-credentials:1.7
badge:1.8
workflow-scm-step:2.12
mailer:1.34
pipeline-model-extensions:1.8.4
pipeline-utility-steps:2.7.1
copyartifact:1.46
jobConfigHistory:2.27
handlebars:3.0.8
durable-task:1.36
promoted-builds:3.9.1
docker-commons:1.17
pipeline-graph-analysis:1.10
jquery:1.12.4-1
pipeline-milestone-step:1.3.2
pipeline-github-lib:1.0
javadoc:1.6
bouncycastle-api:2.20
pipeline-build-step:2.13
warnings-ng:9.1.0
ant:1.11
github:1.33.1
workflow-basic-steps:2.23
greenballs:1.15.1
github-branch-source:2.10.4
parameterized-scheduler:1.0
jsch:0.1.55.2
ldap:2.6
text-file-operations:1.3.2
workflow-cps-global-lib:2.19
jenkins-design-language:1.24.6
build-failure-analyzer:2.0.0
publish-over-cifs:0.16
momentjs:1.1.1
file-parameters:99.102.vbc6a133bcbbb
  • What Operating System are you using (both controller, and any agents involved in the problem)?
Linux, no agents involved

Reproduction steps

pipeline {
  agent any
  parameters {
    stashedFile 'file.zip'
  }
  triggers {
	cron('0 * * * *')
  }
  stages {
    stage('unstash file') {
      steps {
		  cleanWs()
          catchError(buildResult: 'SUCCESS', stageResult: 'SUCCESS') { //needed when TimeTriggered or by SCM because unstashing fails
    		unstash 'file.zip'
          }
      }
    }
    stage('analyze file')
	{
        environment {
            DIR = "Bin"
        }
        when {
            expression { sh(returnStatus:true, script: '[ -s file.zip ]' ) == 0 } //neeeded when triggered manually because unstashing works, but the file has a size of zero bytes
        }
		stages {
			stage('unpack file') {
				steps {
					unzip zipFile: 'file.zip', dir: "$DIR"
				}
			}
		}
	}
  }
}

Results

Expected result:

I'd expect to do the same error handling, no matter whether the pipeline was started manually or by another trigger

Actual result:

unstashing fails when the pipeline is TimerTriggered or by SCM
unstashing works and the file exists with a size of 0 when triggering the pipeline manually

@ValdarXY ValdarXY added the bug Something isn't working label May 28, 2021
@jglick
Copy link
Member

jglick commented Jun 2, 2021

Will have to look into it when I get a chance. Did you try the withFileParameter step?

@ValdarXY
Copy link
Author

ValdarXY commented Jun 11, 2021

Yes, the behaviour is also inconsistent when using withFileParameter
Manually triggered: an empty file is accessible; no error is thrown, even with allowNoFile: false
TimerTriggered:

  • using allowNoFile: true: The environment variable specified in the withFileParameter step is not set
  • using allowNoFile: false: an error is thrown by the withFileParameter step

nevertheless, your hint to withFileParameter was very useful because now i can check whether a file was provided in a when condition:

pipeline {
  agent any
  parameters {
    stashedFile(name: 'ZIP_FILE', description: "File to unzip")
  }
  triggers {
	cron('* * * * *')
  }
  stages {
    stage('test')
    {
        steps {
            catchError(buildResult: 'SUCCESS', stageResult: 'SUCCESS') { 
                //Fine when triggered manually without a file, fails when TimerTriggered without a file (because ZIP_FILE variable is not set)
                withFileParameter(name:'ZIP_FILE', allowNoFile: true) { sh 'echo doing stuff with $ZIP_FILE' }
            }
            catchError(buildResult: 'SUCCESS', stageResult: 'SUCCESS') { 
                //Fine when triggered manually without a file, fails when TimerTriggered without a file (because withFileParameter throws an error)
                withFileParameter(name:'ZIP_FILE') { sh 'echo doing stuff with $ZIP_FILE' }
            }
        }
    }
    
    //my intended use
    stage('check file existence')
	{
        environment {
            DIR = "Bin"
        }
        when {
            expression { 
                withFileParameter(name:'ZIP_FILE', allowNoFile: true) {
                    //two checks needed: variable has to be set and filesisze may not be zero
                    sh(returnStatus:true, script: '[ ! -z "$ZIP_FILE" ] && [ -s $ZIP_FILE ]' ) == 0 }
            }
        }
		stages {
			stage('unpack file') {
				steps {
				    withFileParameter(name:'ZIP_FILE') {
					    unzip zipFile: ZIP_FILE, dir: DIR
				    }
				}
			}
		}
	}
  }
}

@ValdarXY
Copy link
Author

note: I'm still testing with file-parameters:99.102.vbc6a133bcbbb

@Angelin01
Copy link

Angelin01 commented Jun 16, 2021

I'd like to add to this issue, since it is quite similar. Automatic triggers that don't supply any file can fail when trying to access the parameter by name, through the global env or params variables. A snippet like this:

parameters {
    base64File(
        name: 'MY_JSON',
        description: 'My JSON file'
    )
}

If accessed like this (similar to how you can access other parameters):

withFileParameter('MY_JSON', allowNoFile: true) {
    myFunc(env.MY_JSON)
}

will fail with an error ERROR: No parameter named MY_JSON, where as with any other "missing" parameter that value will be null.

Edit: It seems it is the withFileParameter function that is erroring out, not the env access. I've updated the snippet accordingly.

One quick solution is to allow a "default" value? Probably supplied with a base64 string (allowing empty would be nice). In our case, we have a job that's triggered automatically by another job, but if triggered manually, the user can supply the OPTIONAL file to update the current environment.

If you want to ask why we are accessing it through env: it's complicated and I unfortunately can't go into much detail.

@jglick
Copy link
Member

jglick commented Mar 11, 2022

Related to #26.

@jglick
Copy link
Member

jglick commented Mar 11, 2022

Also related to #22: you will get a *_FILENAME environment variable if the user via GUI actually selected a file to upload. If they did not, apparently the HTML form is posted as if it were an upload of an empty file with no name—hence the anomalous behavior. We could treat an empty file upload like a missing parameter, but this seems sure to break some legitimate use case.

@jglick
Copy link
Member

jglick commented Mar 11, 2022

Allow both kinds of file parameters to specify a default value (empty or some Base64 content) seems like a reasonable improvement that would obviate the need for error handling under the assumption that

  • it is legitimate to run a build with no specified file (for example on a schedule)
  • there is no need to differentiate the case that a build was explicitly triggered with a file that happens to match the default (e.g., empty) from the case of a build that did not specify a file

There are several different ways to pass file parameters (at least GUI upload, HTTP POST, CLI, build step) so offering a default value would help minimize the variation and need for documentation regarding missing values.

@ValdarXY
Copy link
Author

I found a solution for this issue: we can check reliably for the *_FILENAME-variable. I'm fine with that.

pipeline {
    agent any
    parameters {
        stashedFile 'ZIPFILE'
    }
    triggers {
        cron('0 * * * *')
    }
    stages {
        stage('file handling')
        {
            environment {
                DIR = "Bin"
            }
            when {
                expression { env.ZIPFILE_FILENAME } 
            }
            stages {
                stage('unpack file') {
                    steps {
                        withFileParameter('ZIPFILE') {
                            unzip zipFile: 'ZIPFILE', dir: "$DIR"
                        }
                    }
                }
            }
        }
    }
}

@jglick
Copy link
Member

jglick commented Jan 13, 2023

That seems like a workaround, not a solution.

@jglick jglick reopened this Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants