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

scm (git) command does not fail correctly when it cannot look up credentials

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed (View Workflow)
    • Priority: Minor
    • Resolution: Fixed
    • Component/s: git-plugin
    • Labels:
    • Environment:
      jenkins 1.609.3
      java8
      credentials-binding 1.23
      credentials 1.5
      git-client 1.19.0
      git 2.4.0
      workflow 1.11-SNAPSHOT (a24f1309) (required for deleteDir)
    • Similar Issues:

      Description

      If you specify an scm to use a specific credential in workflow if that credential can not be found I expect a error saying so - not a generic error from git.

      Steps to reproduce

      1. create a global credential "SSH Username with private key" in Jenkins configured for "Global Access" and provide it an id of "github" (and set up this key on github)
      2. create a workflow with the following syntax

      node() {
              deleteDir() // ensure workspace is fully clean
              checkout([$class: 'GitSCM', userRemoteConfigs: [[credentialsId: 'github', url: 'git@github.com:jenkinsci/jenkins.git']], branches: [[name: '*/master']], doGenerateSubmoduleConfigurations: false, extensions: [[$class: 'CleanBeforeCheckout'], [$class: 'RelativeTargetDirectory', relativeTargetDir: 'jenkins']], submoduleCfg: []])
            echo "hello there"
          }
      }
      

      3. run the workflow and verify it can clone correctly
      4. change the credential from "Global Access" to "System Access"

      expected results

      the workflow should fail saying can not locate credential with id "githib"

      Actual Results

      workflow fails with the following:

      Cloning the remote Git repository
      Cloning repository git@github.com:jenkinsci/jenkins.git
       > git init /home/jenkins/slave/workspace/automated_release/jenkinsci/jenkins # timeout=10
      Fetching upstream changes from git@github.com:jenkinsci/jenkins.git
       > git --version # timeout=10
       > git -c core.askpass=true fetch --tags --progress git@github.com:jenkinsci/jenkins.git +refs/heads/*:refs/remotes/origin/*
      ERROR: Error cloning remote repo 'origin'
      hudson.plugins.git.GitException: Command "git -c core.askpass=true fetch --tags --progress git@github.com:jenkinsci/jenkins.git +refs/heads/*:refs/remotes/origin/*" returned status code 128:
      stdout: 
      stderr: Permission denied (publickey).
      fatal: Could not read from remote repository.
      
      Please make sure you have the correct access rights
      and the repository exists.
      
      	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:1640)
      	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandWithCredentials(CliGitAPIImpl.java:1388)
      	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.access$300(CliGitAPIImpl.java:62)
      	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$1.execute(CliGitAPIImpl.java:313)
      	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$2.execute(CliGitAPIImpl.java:505)
      	at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$1.call(RemoteGitImpl.java:152)
      	at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler$1.call(RemoteGitImpl.java:145)
      	at hudson.remoting.UserRequest.perform(UserRequest.java:121)
      	at hudson.remoting.UserRequest.perform(UserRequest.java:49)
      	at hudson.remoting.Request$2.run(Request.java:326)
      	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68)
      	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
      	at java.lang.Thread.run(Thread.java:745)
      	at ......remote call to linnux-slave(Native Method)
      	at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1413)
      	at hudson.remoting.UserResponse.retrieve(UserRequest.java:221)
      	at hudson.remoting.Channel.call(Channel.java:778)
      	at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler.execute(RemoteGitImpl.java:145)
      	at sun.reflect.GeneratedMethodAccessor376.invoke(Unknown Source)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      	at java.lang.reflect.Method.invoke(Method.java:497)
      	at org.jenkinsci.plugins.gitclient.RemoteGitImpl$CommandInvocationHandler.invoke(RemoteGitImpl.java:131)
      	at com.sun.proxy.$Proxy70.execute(Unknown Source)
      	at hudson.plugins.git.GitSCM.retrieveChanges(GitSCM.java:1003)
      	at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1043)
      	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:109)
      	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep$StepExecutionImpl.run(SCMStep.java:83)
      	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep$StepExecutionImpl.run(SCMStep.java:73)
      	at org.jenkinsci.plugins.workflow.steps.AbstractSynchronousNonBlockingStepExecution$1$1.call(AbstractSynchronousNonBlockingStepExecution.java:49)
      	at hudson.security.ACL.impersonate(ACL.java:213)
      	at org.jenkinsci.plugins.workflow.steps.AbstractSynchronousNonBlockingStepExecution$1.run(AbstractSynchronousNonBlockingStepExecution.java:47)
      	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
      	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
      	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
      	at java.lang.Thread.run(Thread.java:745)
      

      This error message does not actually tell you what is wrong.

        Attachments

          Activity

          teilo James Nord created issue -
          teilo James Nord made changes -
          Field Original Value New Value
          Environment jenkins 1.609.3
          java8
          credentials-binding 1.23
          credentials 1.5
          git-client 1.19.0
          git 2.4.0
          jenkins 1.609.3
          java8
          credentials-binding 1.23
          credentials 1.5
          git-client 1.19.0
          git 2.4.0
          workflow 1.11-SNAPSHOT (a24f1309) (required for deleteDir)
          Hide
          teilo James Nord added a comment - - edited

          Note: requires a snapshot build of workflow for deleteDir.
          Can be reproduced with 1.10 of workflow if deleteDir is replaced with "sh 'rm -fr jenkins'" if running on Linux but for windows something like "bat 'del /s jenkins && rmdir s/ jenkins'"

          Show
          teilo James Nord added a comment - - edited Note: requires a snapshot build of workflow for deleteDir. Can be reproduced with 1.10 of workflow if deleteDir is replaced with " sh 'rm -fr jenkins' " if running on Linux but for windows something like " bat 'del /s jenkins && rmdir s/ jenkins' "
          markewaite Mark Waite made changes -
          Assignee Mark Waite [ markewaite ]
          Hide
          rsandell rsandell added a comment -

          It seems like the credentials lookup @ https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/GitSCM.java#L717 is wrongly assuming that if null is returned then there are no credentials specified. Maybe could be simply fixed by an extra check on line 725 if any credential ids where specified and if so throw an exception. Same assumption seems to exist in AbstractGitSCMSource.

          Show
          rsandell rsandell added a comment - It seems like the credentials lookup @ https://github.com/jenkinsci/git-plugin/blob/master/src/main/java/hudson/plugins/git/GitSCM.java#L717 is wrongly assuming that if null is returned then there are no credentials specified. Maybe could be simply fixed by an extra check on line 725 if any credential ids where specified and if so throw an exception. Same assumption seems to exist in AbstractGitSCMSource.
          Hide
          teilo James Nord added a comment -

          Stephen Connolly can you comment on rsandell's comment above?

          Show
          teilo James Nord added a comment - Stephen Connolly can you comment on rsandell 's comment above?
          Hide
          stephenconnolly Stephen Connolly added a comment -

          So this all gets "fun" really fast.

          The first thing is that we need to ensure that we are respecting the build authentication. If somebody uses the Authorize Project plugin (or equivalent) then the build will not be running as ACL.SYSTEM.

          So the rule should be:

          • Look up the credentials as Jenkins.getAuthentication()
          • If none found and Jenkins.getAuthentication() has CredentialsProvider.USE_ITEM then look up the credentials as ACL.SYSTEM

          That's all fine and mostly ok...

          Now what happens when the job is configured to be parameterized and the credentials are provided from a parameter?

          In that case git's credentialId will be something like "${CREDENTIAL_PARAM}"

          When faced with such a credentialId, we need to add the User who triggered the build into the mix... the sequence then becomes (for an id that contains ${ and }:

          • Resolve the ID in the context of the current build by expanding the token macros
          • If the user who triggered the build has CredentialsProvider.USE_OWN then search for the resolved ID as the user who triggered the build.
          • If none found and the user who triggered the build has CredentialsProvider.USE_ITEM then search for the resolved ID as ACL.SYSTEM
          • If none found then look up the credentials as Jenkins.getAuthentication()
          • If none found and Jenkins.getAuthentication() has CredentialsProvider.USE_ITEM then look up the credentials as ACL.SYSTEM

          The good news is that CredentialsProvider.findCredentialsById(..., Run, ...) does all that logic for you... so as long as you are using that method, the credential lookup should be correct.

          OK, so how does that affect whether the build should continue or fail...

          The credential parameter can be configured to allow selection of no credentials as a valid option.

          Thus the job configuration might anticipate that it is valid to not select credentials (I can think of a job where you do a dry run of the release if no credentials to commit back are provided)...

          So I think that if the credentialsId is non-empty and we do not find any credentials... that may not be a problem... it may be by design... any solution to this issue needs to address whether it makes sense to let people configure a job that can work in the absence of credentials if they have configured credentials... but paying attention to the potential mix that credentials parameters introduces

          Show
          stephenconnolly Stephen Connolly added a comment - So this all gets "fun" really fast. The first thing is that we need to ensure that we are respecting the build authentication. If somebody uses the Authorize Project plugin (or equivalent) then the build will not be running as ACL.SYSTEM . So the rule should be: Look up the credentials as Jenkins.getAuthentication() If none found and Jenkins.getAuthentication() has CredentialsProvider.USE_ITEM then look up the credentials as ACL.SYSTEM That's all fine and mostly ok... Now what happens when the job is configured to be parameterized and the credentials are provided from a parameter? In that case git's credentialId will be something like "${CREDENTIAL_PARAM}" When faced with such a credentialId, we need to add the User who triggered the build into the mix... the sequence then becomes (for an id that contains ${ and } : Resolve the ID in the context of the current build by expanding the token macros If the user who triggered the build has CredentialsProvider.USE_OWN then search for the resolved ID as the user who triggered the build. If none found and the user who triggered the build has CredentialsProvider.USE_ITEM then search for the resolved ID as ACL.SYSTEM If none found then look up the credentials as Jenkins.getAuthentication() If none found and Jenkins.getAuthentication() has CredentialsProvider.USE_ITEM then look up the credentials as ACL.SYSTEM The good news is that CredentialsProvider.findCredentialsById(..., Run, ...) does all that logic for you... so as long as you are using that method, the credential lookup should be correct. OK, so how does that affect whether the build should continue or fail... The credential parameter can be configured to allow selection of no credentials as a valid option. Thus the job configuration might anticipate that it is valid to not select credentials (I can think of a job where you do a dry run of the release if no credentials to commit back are provided)... So I think that if the credentialsId is non-empty and we do not find any credentials... that may not be a problem... it may be by design... any solution to this issue needs to address whether it makes sense to let people configure a job that can work in the absence of credentials if they have configured credentials... but paying attention to the potential mix that credentials parameters introduces
          Hide
          jglick Jesse Glick added a comment -

          Do not see any reason to believe this is a Workflow-specific issue.

          Show
          jglick Jesse Glick added a comment - Do not see any reason to believe this is a Workflow-specific issue.
          jglick Jesse Glick made changes -
          Component/s workflow-plugin [ 18820 ]
          Assignee Mark Waite [ markewaite ]
          Labels diagnostics ux diagnostics ux workflow
          markewaite Mark Waite made changes -
          Assignee Mark Waite [ markewaite ]
          markewaite Mark Waite made changes -
          Labels diagnostics ux workflow diagnostics ux
          Hide
          jglick Jesse Glick added a comment -

          Not sure I followed all of Stephen Connolly’s explanation there, but at a minimum I would expect that if the Git command actually failed (especially if we can determine that the failure was due to authentication—straightforward I think for JGit, not so much for CLI Git—and a credentialsId was specified, we should print a special diagnostic.

          Alternately, whenever credentialsId was specified but we do not wind up passing any actual Credentials, print a warning to the log.

          Show
          jglick Jesse Glick added a comment - Not sure I followed all of Stephen Connolly ’s explanation there, but at a minimum I would expect that if the Git command actually failed (especially if we can determine that the failure was due to authentication—straightforward I think for JGit, not so much for CLI Git— and a credentialsId was specified, we should print a special diagnostic. Alternately, whenever credentialsId was specified but we do not wind up passing any actual Credentials , print a warning to the log.
          Hide
          teilo James Nord added a comment -

          Jesse Glick the workflow bit is that you can misstype credentials. with other job types the UI makes sure the credentials are correct. (until they are removed from the system..)

          Show
          teilo James Nord added a comment - Jesse Glick the workflow bit is that you can misstype credentials. with other job types the UI makes sure the credentials are correct. (until they are removed from the system..)
          Hide
          markewaite Mark Waite added a comment - - edited

          I like the idea of a diagnostic for those two cases. I'm not sure how easy it will be to detect the cases, but I agree that they are excellent cases to detect. I think it would also be good to add an assertion to the existing git client plugin CredentialsTest that none of the credentials test cases caused those warnings to be reported to the log.

          Show
          markewaite Mark Waite added a comment - - edited I like the idea of a diagnostic for those two cases. I'm not sure how easy it will be to detect the cases, but I agree that they are excellent cases to detect. I think it would also be good to add an assertion to the existing git client plugin CredentialsTest that none of the credentials test cases caused those warnings to be reported to the log.
          Hide
          jglick Jesse Glick added a comment -

          with other job types the UI makes sure the credentials are correct

          Job DSL, Yaml Project, Python Jenkins, …

          I'm not sure how easy it will be to detect the cases

          So the simplest possible change, which I think would be good enough: always print a message before the checkout.

          • If there is no credentialsId specified, say No credentials specified.
          • If there is one specified, but no Credentials can be found for it, say Warning: credentials with ID abcd-0123 could not be found.
          • If the Credentials can be found, say Using credentials: bob/**** or whatever CredentialsName says.
          Show
          jglick Jesse Glick added a comment - with other job types the UI makes sure the credentials are correct Job DSL, Yaml Project, Python Jenkins, … I'm not sure how easy it will be to detect the cases So the simplest possible change, which I think would be good enough: always print a message before the checkout. If there is no credentialsId specified, say No credentials specified . If there is one specified, but no Credentials can be found for it, say Warning : credentials with ID abcd-0123 could not be found . If the Credentials can be found, say Using credentials: bob/**** or whatever CredentialsName says.
          rtyler R. Tyler Croy made changes -
          Workflow JNJira [ 165626 ] JNJira + In-Review [ 182060 ]
          cloudbees CloudBees Inc. made changes -
          Remote Link This issue links to "CloudBees Internal OSS-317 (Web Link)" [ 18915 ]
          jtaboada Jose Blas Camacho Taboada made changes -
          Assignee Jose Blas Camacho Taboada [ jtaboada ]
          jtaboada Jose Blas Camacho Taboada made changes -
          Remote Link This issue links to "Github PR 653 (Web Link)" [ 22159 ]
          jtaboada Jose Blas Camacho Taboada made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          jtaboada Jose Blas Camacho Taboada made changes -
          Status In Progress [ 3 ] In Review [ 10005 ]
          jtaboada Jose Blas Camacho Taboada made changes -
          Remote Link This issue links to "Github PR to stable-3.0 (Web Link)" [ 22182 ]
          jtaboada Jose Blas Camacho Taboada made changes -
          Remote Link This issue links to "Github PR to stable-3.9 (Web Link)" [ 22183 ]
          jtaboada Jose Blas Camacho Taboada made changes -
          Remote Link This issue links to "Github PR to stable-3.0 (Web Link)" [ 22182 ]
          jtaboada Jose Blas Camacho Taboada made changes -
          Resolution Fixed [ 1 ]
          Status In Review [ 10005 ] Resolved [ 5 ]
          Hide
          jtaboada Jose Blas Camacho Taboada added a comment -

          merged at stable-3.9

          Show
          jtaboada Jose Blas Camacho Taboada added a comment - merged at stable-3.9
          jtaboada Jose Blas Camacho Taboada made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            Assignee:
            jtaboada Jose Blas Camacho Taboada
            Reporter:
            teilo James Nord
            Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: