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

ItemGroupMixin#createProject() does not call Jenkins#checkGoodName()

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Minor Minor
    • core
    • Jenkins 2.164.2
      Folders plugin 6.9
    • Jenkins 2.237

      String nested = "one/two"
      
      // Creates a single folder named "one/two" with / embedded in the name
      createProject(Folder.class, nested)
      
      // Looks for a subfolder named "two" inside a folder named "one"
      getItemByFullName(nested)
      

      As a result, this code fails

      import com.cloudbees.hudson.plugins.folder.Folder
      
      String folder = "nested/folders"
      
      create_folder_if_missing(folder)
      create_folder_if_missing(folder)
      
      void create_folder_if_missing(String folder) {
        def item = Jenkins.instance.getItemByFullName(folder)
      
        if (item == null) {
          println(folder + " not found. Creating.")
          Jenkins.instance.createProject(Folder.class, folder)
          println("Created " + folder)
        }
        else {
          println(folder + " found. Skipping.")
        }
      }
      

      with the result

      nested/folders not found. Creating.
      Created nested/folders
      nested/folders not found. Creating.
      java.lang.IllegalArgumentException: nested/folders already exists
      	at hudson.model.Items.verifyItemDoesNotAlreadyExist(Items.java:641)
      

      I think createProject(Folder.class, nested) should change its behavior to look for slashes and create subfolders, rather than allowing a slash in a folder name. At this point it's impossible to search for a folder with a slash in the name using getItemByFullName() function.

          [JENKINS-61956] ItemGroupMixin#createProject() does not call Jenkins#checkGoodName()

          Daniel Beck added a comment -

          Is this a regression?

          Tried it back to 1.620, a 5 years old release, doesn't work.

          println Jenkins.instance.getItemByFullName("foo/bar")
          Jenkins.instance.createProject(com.cloudbees.hudson.plugins.folder.Folder.class, "foo/bar")
          println Jenkins.instance.getItemByFullName("foo/bar")
          

          Output is

          null
          null
          

          So no regression here for any sensible definition of the term. Nobody cares what happened >5 years ago.

          Daniel Beck added a comment - Is this a regression? Tried it back to 1.620, a 5 years old release, doesn't work. println Jenkins.instance.getItemByFullName("foo/bar") Jenkins.instance.createProject(com.cloudbees.hudson.plugins.folder.Folder.class, "foo/bar") println Jenkins.instance.getItemByFullName("foo/bar") Output is null null So no regression here for any sensible definition of the term. Nobody cares what happened >5 years ago.

          Calvin Park added a comment -

          Thank you for checking. Thoughts on making createProject handle subfolders?

          Calvin Park added a comment - Thank you for checking. Thoughts on making createProject handle subfolders?

          Daniel Beck added a comment -

          It's like 5 lines of code to write a utility function. Don't be so lazy.

          Daniel Beck added a comment - It's like 5 lines of code to write a utility function. Don't be so lazy.

          Calvin Park added a comment -

          This is what I've got so far - far from a 5 line function. I'd appreciate your comments on how I can improve it

          #jinja2: lstrip_blocks: True
          #!/usr/bin/env groovy
          import com.cloudbees.hudson.plugins.folder.Folder
          import org.apache.commons.lang3.StringUtils;
          
          // Create a folder in a parent folder
          Folder create_folder(def parent_folder, String folder_name) {
              // Actual create method call
              parent_folder.createProject(Folder.class, folder_name)
          
              // Below is unfortunately needed because createProject() doesn't return the
              // newly created folder. Therefore need to query the full path again to fetch.
              Jenkins jenkins = Jenkins.instance
              String folder_path
              if (parent_folder == jenkins) {
                  folder_path = folder_name
              } else {
                  folder_path = parent_folder.fullName + "/" + folder_name
              }
          
              return jenkins.getItemByFullName(folder_path)
          }
          
          // Create a folder and its parent folders
          Folder create_folder_recursively(String folder_path) {
              // If the folder already exists, return
              Folder jenkins_folder = Jenkins.instance.getItemByFullName(folder_path)
              if (jenkins_folder)
                  return jenkins_folder
          
              String folder_name, parent_path
              def parent_folder
          
              // If the folder is top level, set parent to Jenkins itself
              if (!folder_path.contains("/")) {
                  parent_folder = Jenkins.instance
                  folder_name = folder_path
              }
              // If the folder is a sub-folder, recurse to create/fetch the parent folder
              else {
                  folder_name = StringUtils.substringAfterLast(folder_path, "/")
                  parent_path = StringUtils.substringBeforeLast(folder_path, "/")
          
                  parent_folder = create_folder_recursively(parent_path)
              }
          
              // Create the folder in the parent folder
              return create_folder(parent_folder, folder_name)
          }
          
          // Collect the names of all jobs and credentials
          ArrayList<String> names = new ArrayList<String>();
          {% for pipeline in pipelines %}
          names.add("{{ pipeline.name }}")
          {% endfor %}
          {% for cred in credential_store %}
          names.add("{{ cred.id }}")
          {% endfor %}
          
          // Filter to collect the names of all folders
          ArrayList<String> folders = new ArrayList<String>();
          for (String name : names) {
              if (name.contains("/")) {
                  folders.add(StringUtils.substringBeforeLast(name, "/"))
              }
          }
          
          // Remove duplicates
          folders.unique().sort()
          
          // Create folders and sub-folders
          for (String folder_path : folders)
              create_folder_recursively(folder_path)
          
          

          Calvin Park added a comment - This is what I've got so far - far from a 5 line function. I'd appreciate your comments on how I can improve it #jinja2: lstrip_blocks: True #!/usr/bin/env groovy import com.cloudbees.hudson.plugins.folder.Folder import org.apache.commons.lang3.StringUtils; // Create a folder in a parent folder Folder create_folder(def parent_folder, String folder_name) { // Actual create method call parent_folder.createProject(Folder.class, folder_name) // Below is unfortunately needed because createProject() doesn't return the // newly created folder. Therefore need to query the full path again to fetch. Jenkins jenkins = Jenkins.instance String folder_path if (parent_folder == jenkins) { folder_path = folder_name } else { folder_path = parent_folder.fullName + "/" + folder_name } return jenkins.getItemByFullName(folder_path) } // Create a folder and its parent folders Folder create_folder_recursively( String folder_path) { // If the folder already exists, return Folder jenkins_folder = Jenkins.instance.getItemByFullName(folder_path) if (jenkins_folder) return jenkins_folder String folder_name, parent_path def parent_folder // If the folder is top level, set parent to Jenkins itself if (!folder_path.contains( "/" )) { parent_folder = Jenkins.instance folder_name = folder_path } // If the folder is a sub-folder, recurse to create/fetch the parent folder else { folder_name = StringUtils.substringAfterLast(folder_path, "/" ) parent_path = StringUtils.substringBeforeLast(folder_path, "/" ) parent_folder = create_folder_recursively(parent_path) } // Create the folder in the parent folder return create_folder(parent_folder, folder_name) } // Collect the names of all jobs and credentials ArrayList< String > names = new ArrayList< String >(); {% for pipeline in pipelines %} names.add( "{{ pipeline.name }}" ) {% endfor %} {% for cred in credential_store %} names.add( "{{ cred.id }}" ) {% endfor %} // Filter to collect the names of all folders ArrayList< String > folders = new ArrayList< String >(); for ( String name : names) { if (name.contains( "/" )) { folders.add(StringUtils.substringBeforeLast(name, "/" )) } } // Remove duplicates folders.unique().sort() // Create folders and sub-folders for ( String folder_path : folders) create_folder_recursively(folder_path)

          Calvin Park added a comment -

          and no insults please. I'm trying to improve Jenkins with my limited knowledge.

          Calvin Park added a comment - and no insults please. I'm trying to improve Jenkins with my limited knowledge.

          Calvin Park added a comment -

          Looked into it more and came up with this.

          // Create a folder and its parent folders
          // mkdir -p folder_path
          Folder create_folder_and_parent_folders(String folder_path) {
              // If the folder already exists, return
              Folder folder = Jenkins.instance.getItemByFullName(folder_path)
              if (folder)
                  return folder
          
              String folder_name, parent_path
              def parent_folder
          
              // If the folder is top level, set parent to Jenkins itself
              if (!folder_path.contains("/")) {
                  parent_folder = Jenkins.instance
                  folder_name = folder_path
              }
              // If the folder is a sub-folder, recurse to create/fetch the parent folder
              else {
                  parent_path = StringUtils.substringBeforeLast(folder_path, "/")
                  parent_folder = create_folder_and_parent_folders(parent_path)
                  folder_name = StringUtils.substringAfterLast(folder_path, "/")
              }
          
              // Create the folder in the parent folder
              return parent_folder.createProject(Folder.class, folder_name)
          } 

          Calvin Park added a comment - Looked into it more and came up with this. // Create a folder and its parent folders // mkdir -p folder_path Folder create_folder_and_parent_folders( String folder_path) { // If the folder already exists, return Folder folder = Jenkins.instance.getItemByFullName(folder_path) if (folder) return folder String folder_name, parent_path def parent_folder // If the folder is top level, set parent to Jenkins itself if (!folder_path.contains( "/" )) { parent_folder = Jenkins.instance folder_name = folder_path } // If the folder is a sub-folder, recurse to create/fetch the parent folder else { parent_path = StringUtils.substringBeforeLast(folder_path, "/" ) parent_folder = create_folder_and_parent_folders(parent_path) folder_name = StringUtils.substringAfterLast(folder_path, "/" ) } // Create the folder in the parent folder return parent_folder.createProject(Folder.class, folder_name) }

          Calvin Park added a comment -

          Calvin Park added a comment - Opened  https://github.com/jenkinsci/jenkins/pull/4684

          Calvin Park added a comment - - edited

          danielbeck Please change the title of this ticket to ItemGroupMixin#createProject() does not call Jenkins#checkGoodName()

          Calvin Park added a comment - - edited danielbeck Please change the title of this ticket to ItemGroupMixin#createProject() does not call Jenkins#checkGoodName()

          Daniel Beck added a comment -

          calvinpark Done! Note that you can just change it yourself

          Daniel Beck added a comment - calvinpark Done! Note that you can just change it yourself

          Calvin Park added a comment -

          Ah sorry, I didn't realize the name itself was clickable. Thank you!

          Calvin Park added a comment - Ah sorry, I didn't realize the name itself was clickable. Thank you!

            Unassigned Unassigned
            calvinpark Calvin Park
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: