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

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

    XMLWordPrintable

Details

    • Bug
    • Status: Resolved (View Workflow)
    • Minor
    • Resolution: Fixed
    • core
    • Jenkins 2.164.2
      Folders plugin 6.9
    • Jenkins 2.237

    Description

      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.

      Attachments

        Issue Links

          Activity

            danielbeck Daniel Beck added a comment - - edited

            Looks like createProject is missing a call to Jenkins#checkGoodName but that's about it.

            While it would be more convenient to have a "createByFullName" that splits at the last /, finds the folder for the first part and inserts the item named the second part, that's a quick utility method.

            danielbeck Daniel Beck added a comment - - edited Looks like  createProject is missing a call to Jenkins#checkGoodName but that's about it. While it would be more convenient to have a "createByFullName" that splits at the last / , finds the folder for the first part and inserts the item named the second part, that's a quick utility method.
            calvinpark Calvin Park added a comment -

            Is this a regression? Our folder creation script had been working for a few years and now it fails with this problem. I'm not sure what changed since we rarely upgrade master

            calvinpark Calvin Park added a comment - Is this a regression? Our folder creation script had been working for a few years and now it fails with this problem. I'm not sure what changed since we rarely upgrade master
            danielbeck 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.

            danielbeck 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.
            calvinpark Calvin Park added a comment -

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

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

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

            danielbeck Daniel Beck added a comment - It's like 5 lines of code to write a utility function. Don't be so lazy.
            calvinpark 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)
            
            
            calvinpark 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)
            calvinpark Calvin Park added a comment -

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

            calvinpark Calvin Park added a comment - and no insults please. I'm trying to improve Jenkins with my limited knowledge.
            calvinpark 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)
            } 
            calvinpark 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) }
            calvinpark Calvin Park added a comment - Opened  https://github.com/jenkinsci/jenkins/pull/4684
            calvinpark Calvin Park added a comment - - edited

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

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

            calvinpark Done! Note that you can just change it yourself

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

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

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

            People

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

              Dates

                Created:
                Updated:
                Resolved: