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

Properly restrict which expressions can be passed to credentials(...) in Declarative

    Details

    • Epic Link:
    • Sprint:
      Declarative - 1.2
    • Similar Issues:

      Description

      Right now, we allow ConstantExpression or GStringExpression as the parameters to an internal function call (i.e., to credentials(...), but that's a little wonky, because you can actually sneak anything you want into the GStringExpression, i.e., "${someMethodCall(...)}" is legal. Now, locking down the GStringExpression contents is a different matter that I'll deal with some day. Probably. But for now, the question is what else should be allowed as a parameter to credentials(...) - there's probably a valid argument for non-block-scoped steps or functions and variables, so perhaps we should add VariableExpression and method calls with the same rules limiting their parameters as credentials(...) itself.

      Needs more thinking. But soon.

        Attachments

          Issue Links

            Activity

            Hide
            abayer Andrew Bayer added a comment -

            cc rsandell, who wrote the CredentialsBindingHandler stuff and understands it better than I do.

            Show
            abayer Andrew Bayer added a comment - cc rsandell , who wrote the CredentialsBindingHandler stuff and understands it better than I do.
            Hide
            abayer Andrew Bayer added a comment -
            Show
            abayer Andrew Bayer added a comment - ah, it's https://github.com/jenkinsci/azure-credentials-plugin/blob/dev/src/main/java/com/microsoft/azure/util/AzureCredentialsBinding.java? Support for MultiBinding subclasses is in https://github.com/jenkinsci/pipeline-model-definition-plugin/blob/master/pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/model/CredentialsBindingHandler.java - I think they'd have to add an extension of org.jenkinsci.plugins.pipeline.modeldefinition.model.CredentialsBindingHandler to their plugin for it to work here.
            Hide
            abayer Andrew Bayer added a comment -

            Ooookay. Then I have no idea what azureServicePrincipal('service-principle') is supposed to be doing?

            Show
            abayer Andrew Bayer added a comment - Ooookay. Then I have no idea what azureServicePrincipal('service-principle') is supposed to be doing?
            Hide
            rtyler R. Tyler Croy added a comment -

            Andrew Bayer, I don't think so

            WorkflowScript: 6: Internal function call parameters must be strings. @ line 6, column 26.
                               UNUSED = credentials(azureServicePrincipal('service-principle'))
            
            Show
            rtyler R. Tyler Croy added a comment - Andrew Bayer , I don't think so WorkflowScript: 6: Internal function call parameters must be strings. @ line 6, column 26. UNUSED = credentials(azureServicePrincipal( 'service-principle' ))
            Hide
            abayer Andrew Bayer added a comment -

            Did you mean credentials(azureServicePrincipal('service-principle'))?

            Show
            abayer Andrew Bayer added a comment - Did you mean credentials(azureServicePrincipal('service-principle')) ?
            Hide
            rtyler R. Tyler Croy added a comment -

            Here's a bit more thorough of an example:

            pipeline {
                agent any
                stages {
                    stage('Derp') {
                        environment {
                            AZ = azureServicePrincipal('service-principle')
                        }
                        steps {
                            echo "principal is: ${AZ} (type: ${AZ.class.toString()})"
                            echo "AZURE_CLIENT_ID should be in the env, but is: ${env.AZURE_CLIENT_ID}"
                        }
                    }
                }
            }
            

            Which, when run, emits:

            Started by user admin
            [Pipeline] node
            Running on master in /var/jenkins_home/workspace/az
            [Pipeline] {
            [Pipeline] stage
            [Pipeline] { (Derp)
            [Pipeline] withEnv
            [Pipeline] {
            [Pipeline] echo
            principal is: @azureServicePrincipal(<anonymous>=service-principle) (type: class java.lang.String)
            [Pipeline] echo
            AZURE_CLIENT_ID should be in the env, but is: null
            [Pipeline] }
            [Pipeline] // withEnv
            [Pipeline] }
            [Pipeline] // stage
            [Pipeline] }
            [Pipeline] // node
            [Pipeline] End of Pipeline
            Finished: SUCCESS
            
            Show
            rtyler R. Tyler Croy added a comment - Here's a bit more thorough of an example: pipeline { agent any stages { stage( 'Derp' ) { environment { AZ = azureServicePrincipal( 'service-principle' ) } steps { echo "principal is: ${AZ} (type: ${AZ. class. toString()})" echo "AZURE_CLIENT_ID should be in the env, but is: ${env.AZURE_CLIENT_ID}" } } } } Which, when run, emits: Started by user admin [Pipeline] node Running on master in / var /jenkins_home/workspace/az [Pipeline] { [Pipeline] stage [Pipeline] { (Derp) [Pipeline] withEnv [Pipeline] { [Pipeline] echo principal is: @azureServicePrincipal(<anonymous>=service-principle) (type: class java.lang. String ) [Pipeline] echo AZURE_CLIENT_ID should be in the env, but is: null [Pipeline] } [Pipeline] // withEnv [Pipeline] } [Pipeline] // stage [Pipeline] } [Pipeline] // node [Pipeline] End of Pipeline Finished: SUCCESS
            Hide
            rtyler R. Tyler Croy added a comment -

            Here is an example in Scripted Pipeline which I think should be supported in Declarative for a custom credential type.

            Show
            rtyler R. Tyler Croy added a comment - Here is an example in Scripted Pipeline which I think should be supported in Declarative for a custom credential type.

              People

              • Assignee:
                abayer Andrew Bayer
                Reporter:
                abayer Andrew Bayer
              • Votes:
                1 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated: