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

Assertion on withEnv fails when Env Var value contains equals (=) sign #388

Closed
stefano-m opened this issue Jul 8, 2021 · 2 comments · Fixed by #468
Closed

Assertion on withEnv fails when Env Var value contains equals (=) sign #388

stefano-m opened this issue Jul 8, 2021 · 2 comments · Fixed by #468

Comments

@stefano-m
Copy link

Hello, and thank you for working on JenkinsPipelineUnit.

When environment variables containing equals signs in their values are used in withEnv, the BasePipelineTest check fails erroneously because it splits the variable "too much".

A use case is passing tags or other parameters to the AWS CLI:

withEnv([
'TAGS=Key=Owner,Value=blueTeam',
'ATTR_DEFS=AttributeName=Artist,AttributeType=S AttributeName=SongTitle,AttributeType=S',
'KEY_SCHEMA=AttributeName=Artist,KeyType=HASH AttributeName=SongTitle,KeyType=RANGE',
'THROUGHPUT=ReadCapacityUnits=5,WriteCapacityUnits=5'
]) {
    sh '''
      aws dynamodb create-table \
    --table-name MusicCollection \
    --attribute-definitions $ATTR_DEFS \
    --key-schema $KEY_SCHEMA \
    --provisioned-throughput $THROUGHPUT \
    --tags $TAGS
       '''
}

However, the above causes an assertion failure in the BasePipelineTest because the code uses item.split('=') which splits along all = resulting in an array of more than 2 elements. Unfortunately, this is breaking some tests on pipelines that otherwise work fine.

def item = it.split('=')
assert item.size() == 2, "withEnv list does not look right: ${list.toString()}"

IIUC something like

def item = it.split('=', 2)

should do the correct thing and split along the first = only.

Thank you.

@stefano-m
Copy link
Author

At the moment this is working for me:

      helper.registerAllowedMethod(
            'withEnv',          // see https://github.com/jenkinsci/JenkinsPipelineUnit/issues/388
            [List, Closure],
            { List list, Closure c ->
                def stashedEnv = [:]
                try {
                    stashedEnv.putAll(binding.getVariable('env') as Map)
                } catch (MissingPropertyException e) {
                    // 'env' not set yet?
                }

                list.each {
                    def item = it.split('=', 2)
                    assert item.size() == 2, "Variable ${it} in withEnv (${list}) cannot be split in NAME/VALUE pair"
                    addEnvVar(item[0], item[1])
                }

                try {
                    c.delegate = binding
                    helper.callClosure(c)
                } finally {
                    // Restore original contents of 'env'
                    binding.setVariable('env', stashedEnv)
                }
            }
        )

@juanpablo-santos
Copy link

Another usual use-case that's also hit by this bug is when setting a custom MAVEN_OPTS using withEnv:

withEnv( [ 'MAVEN_OPTS=-Djansi.force=true' ] ) {
...

suggested patch works, but that forces us to overwrite the complete registerAllowedMethods() with the aforementioned change to avoid the exception thrown otherwise, which is unfortunate.

Brenne added a commit to Brenne/JenkinsPipelineUnit that referenced this issue Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants