Applying consistent folder name formatting to Credentials (taiidani#21)

* Fixed folder validation check

* Update formatJobName to dedupe

* Add tests around nested subfolders

* Attempt a "format all the things" strategy

* Add folder property to Job, move to canonical IDs

Co-authored-by: Otterian <eth0linak@gmail.com>

docs/resources/folder.md

@@ -8,19 +8,21 @@ Manages a folder within Jenkins.
 
 ```hcl
 resource jenkins_folder example {
-    name = "folder-name"
+  name = "folder-name"
 }
 
 resource jenkins_folder example_child {
-    name = "${jenkins_folder.example.name}/child-name"
+  name   = "child-name"
+  folder = jenkins_folder.example.id
 }
 ```
 
 ## Argument Reference
 
 The following arguments are supported:
 
-* `name` - (Required) The name of the folder being created. If creating in a subfolder separate folder names with `/`, such as `parent/child`. This name cannot be changed once the folder has been created, and all parent folders must be created in advance.
+* `name` - (Required) The name of the folder being created.
+* `folder` - (Optional) The folder namespace to store the subfolder in. If creating in a nested folder structure you may separate folder names with `/`, such as `parent/child`. This name cannot be changed once the folder has been created, and all parent folders must be created in advance.
 * `description` - (Optional) A block of text describing the folder's purpose.
 * `template` - (Optional) A Jenkins-compatible XML template to describe the folder. You can retrieve an existing folder's XML by appending `/config.xml` to its URL and viewing the source in your browser. The `template` property is rendered using a Golang template that takes the other resource arguments as variables. Do not include the XML prolog in the definition. If `template` is not provided this will default to a "best-guess" folder definition.
 * `permissions` - (Optional) A list of strings containing Jenkins permissions assigments to users and groups for the folder. For example:

docs/resources/job.md

@@ -5,8 +5,13 @@ Manages a job within Jenkins.
 ## Example Usage
 
 ```hcl
+resource jenkins_folder example {
+  name = "folder-name"
+}
+
 resource jenkins_job example {
   name     = "example"
+  folder   = jenkins_folder.example.id
   template = file("${path.module}/job.xml")
 
   parameters = {
@@ -53,7 +58,8 @@ And in `job.xml`:
 
 The following arguments are supported:
 
-* `name` - (Required) The name of the job being created. If creating in a subfolder separate folder names with `/`, such as `parent/child`. This name cannot be changed once the job has been created, and all parent folders must be created in advance.
+* `name` - (Required) The name of the job being created.
+* `folder` - (Optional) The folder namespace to store the job in. If creating in a nested folder structure you may separate folder names with `/`, such as `parent/child`. This name cannot be changed once the folder has been created, and all parent folders must be created in advance.
 * `parameters` - (Optional) A map of string values that are passed into the template for rendering.
 * `template` - (Required) A Jenkins-compatible XML template to describe the job. You can retrieve an existing jobs' XML by appending `/config.xml` to its URL and viewing the source in your browser. The `template` property is rendered using a Golang template that takes the other resource arguments as variables. Do not include the XML prolog in the definition.
 

jenkins/config.go

@@ -3,15 +3,17 @@ package jenkins
 import (
 	"io"
 	"io/ioutil"
+	"strings"
 
 	jenkins "github.com/bndr/gojenkins"
 )
 
 type jenkinsClient interface {
 	CreateJobInFolder(config string, jobName string, parentIDs ...string) (*jenkins.Job, error)
 	Credentials() *jenkins.CredentialsManager
-	DeleteJob(name string) (bool, error)
+	DeleteJobInFolder(name string, parentIDs ...string) (bool, error)
 	GetJob(id string, parentIDs ...string) (*jenkins.Job, error)
+	GetFolder(id string, parents ...string) (*jenkins.Folder, error)
 }
 
 // jenkinsAdapter wraps the Jenkins client, enabling additional functionality
@@ -43,3 +45,9 @@ func (j *jenkinsAdapter) Credentials() *jenkins.CredentialsManager {
 		J: j.Jenkins,
 	}
 }
+
+// DeleteJobInFolder assists in running DeleteJob funcs, as DeleteJob is not folder aware
+// and cannot take a canonical job ID without mishandling it.
+func (j *jenkinsAdapter) DeleteJobInFolder(name string, parentIDs ...string) (bool, error) {
+	return j.DeleteJob(strings.Join(append(parentIDs, name), "/job/"))
+}

jenkins/config_test.go

@@ -9,8 +9,9 @@ import (
 
 type mockJenkinsClient struct {
 	mockCreateJobInFolder func(config string, jobName string, parentIDs ...string) (*jenkins.Job, error)
-	mockDeleteJob         func(name string) (bool, error)
+	mockDeleteJobInFolder func(name string, parentIDs ...string) (bool, error)
 	mockGetJob            func(id string, parentIDs ...string) (*jenkins.Job, error)
+	mockGetFolder         func(id string, parentIDs ...string) (*jenkins.Folder, error)
 }
 
 func (m *mockJenkinsClient) CreateJobInFolder(config string, jobName string, parentIDs ...string) (*jenkins.Job, error) {
@@ -21,14 +22,18 @@ func (m *mockJenkinsClient) Credentials() *jenkins.CredentialsManager {
 	return &jenkins.CredentialsManager{}
 }
 
-func (m *mockJenkinsClient) DeleteJob(name string) (bool, error) {
-	return m.mockDeleteJob(name)
+func (m *mockJenkinsClient) DeleteJobInFolder(name string, parentIDs ...string) (bool, error) {
+	return m.mockDeleteJobInFolder(name, parentIDs...)
 }
 
 func (m *mockJenkinsClient) GetJob(id string, parentIDs ...string) (*jenkins.Job, error) {
 	return m.mockGetJob(id, parentIDs...)
 }
 
+func (m *mockJenkinsClient) GetFolder(id string, parentIDs ...string) (*jenkins.Folder, error) {
+	return m.mockGetFolder(id, parentIDs...)
+}
+
 func TestNewJenkinsClient(t *testing.T) {
 	c := newJenkinsClient(&Config{})
 	if c == nil {

jenkins/resource_jenkins_credential_username.go

@@ -74,13 +74,11 @@ func resourceJenkinsCredentialUsername() *schema.Resource {
 func resourceJenkinsCredentialUsernameCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	client := meta.(jenkinsClient)
 	cm := client.Credentials()
+	cm.Folder = formatFolderName(d.Get("folder").(string))
 
 	// Validate that the folder exists
-	cm.Folder = d.Get("folder").(string)
-	if cm.Folder != "" {
-		if _, err := client.GetJob(formatJobName(cm.Folder)); err != nil {
-			return diag.Errorf("Invalid folder name '%s' specified: %s", cm.Folder, err)
-		}
+	if err := folderExists(client, cm.Folder); err != nil {
+		return diag.FromErr(fmt.Errorf("Invalid folder name '%s' specified: %w", cm.Folder, err))
 	}
 
 	cred := jenkins.UsernameCredentials{
@@ -97,13 +95,13 @@ func resourceJenkinsCredentialUsernameCreate(ctx context.Context, d *schema.Reso
 		return diag.Errorf("Could not create username credentials: %s", err)
 	}
 
-	d.SetId(generateCredentialID(cm.Folder, cred.ID))
+	d.SetId(generateCredentialID(d.Get("folder").(string), cred.ID))
 	return resourceJenkinsCredentialUsernameRead(ctx, d, meta)
 }
 
 func resourceJenkinsCredentialUsernameRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	cm := meta.(jenkinsClient).Credentials()
-	cm.Folder = d.Get("folder").(string)
+	cm.Folder = formatFolderName(d.Get("folder").(string))
 
 	cred := jenkins.UsernameCredentials{}
 	err := cm.GetSingle(
@@ -122,7 +120,7 @@ func resourceJenkinsCredentialUsernameRead(ctx context.Context, d *schema.Resour
 		return diag.Errorf("Could not read username credentials: %s", err)
 	}
 
-	d.SetId(generateCredentialID(cm.Folder, cred.ID))
+	d.SetId(generateCredentialID(d.Get("folder").(string), cred.ID))
 	d.Set("scope", cred.Scope)
 	d.Set("description", cred.Description)
 	d.Set("username", cred.Username)
@@ -134,7 +132,7 @@ func resourceJenkinsCredentialUsernameRead(ctx context.Context, d *schema.Resour
 
 func resourceJenkinsCredentialUsernameUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	cm := meta.(jenkinsClient).Credentials()
-	cm.Folder = d.Get("folder").(string)
+	cm.Folder = formatFolderName(d.Get("folder").(string))
 
 	domain := d.Get("domain").(string)
 	cred := jenkins.UsernameCredentials{
@@ -154,13 +152,13 @@ func resourceJenkinsCredentialUsernameUpdate(ctx context.Context, d *schema.Reso
 		return diag.Errorf("Could not update username credentials: %s", err)
 	}
 
-	d.SetId(generateCredentialID(cm.Folder, cred.ID))
+	d.SetId(generateCredentialID(d.Get("folder").(string), cred.ID))
 	return resourceJenkinsCredentialUsernameRead(ctx, d, meta)
 }
 
 func resourceJenkinsCredentialUsernameDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	cm := meta.(jenkinsClient).Credentials()
-	cm.Folder = d.Get("folder").(string)
+	cm.Folder = formatFolderName(d.Get("folder").(string))
 
 	err := cm.Delete(
 		d.Get("domain").(string),

jenkins/resource_jenkins_credential_username_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	jenkins "github.com/bndr/gojenkins"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 )
@@ -40,9 +41,90 @@ func TestAccJenkinsCredentialUsername_basic(t *testing.T) {
 				  password = "bar"
 				}`),
 				Check: resource.ComposeTestCheckFunc(
+					testAccCheckJenkinsCredentialUsernameExists("jenkins_credential_username.foo", &cred),
 					resource.TestCheckResourceAttr("jenkins_credential_username.foo", "description", "new-description"),
+				),
+			},
+		},
+	})
+}
+
+func TestAccJenkinsCredentialUsername_folder(t *testing.T) {
+	var cred jenkins.UsernameCredentials
+	randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		CheckDestroy: resource.ComposeTestCheckFunc(
+			testAccCheckJenkinsCredentialUsernameDestroy,
+			testAccCheckJenkinsFolderDestroy,
+		),
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(`
+				resource jenkins_folder foo {
+					name = "tf-acc-test-%s"
+					description = "Terraform acceptance testing"
+
+					lifecycle {
+						ignore_changes = [template]
+					}
+				}
+
+				resource jenkins_folder foo_sub {
+					name = "subfolder"
+					folder = jenkins_folder.foo.id
+					description = "Terraform acceptance testing"
+
+					lifecycle {
+						ignore_changes = [template]
+					}
+				}
+
+				resource jenkins_credential_username foo {
+				  name = "test-username"
+				  folder = jenkins_folder.foo_sub.id
+				  username = "foo"
+				  password = "bar"
+				}`, randString),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("jenkins_credential_username.foo", "id", "/job/tf-acc-test-"+randString+"/job/subfolder/test-username"),
+					testAccCheckJenkinsCredentialUsernameExists("jenkins_credential_username.foo", &cred),
+				),
+			},
+			{
+				// Update by adding description
+				Config: fmt.Sprintf(`
+				resource jenkins_folder foo {
+					name = "tf-acc-test-%s"
+					description = "Terraform acceptance testing"
+
+					lifecycle {
+						ignore_changes = [template]
+					}
+				}
+
+				resource jenkins_folder foo_sub {
+					name = "subfolder"
+					folder = jenkins_folder.foo.id
+					description = "Terraform acceptance testing"
+
+					lifecycle {
+						ignore_changes = [template]
+					}
+				}
+
+				resource jenkins_credential_username foo {
+				  name = "test-username"
+				  folder = jenkins_folder.foo_sub.id
+				  description = "new-description"
+				  username = "foo"
+				  password = "bar"
+				}`, randString),
+				Check: resource.ComposeTestCheckFunc(
 					testAccCheckJenkinsCredentialUsernameExists("jenkins_credential_username.foo", &cred),
-					testAccCheckJenkinsCredentialUsernameDescriptionUpdated(&cred, "new-description"),
+					resource.TestCheckResourceAttr("jenkins_credential_username.foo", "description", "new-description"),
 				),
 			},
 		},
@@ -62,19 +144,11 @@ func testAccCheckJenkinsCredentialUsernameExists(resourceName string, cred *jenk
 			return fmt.Errorf("ID is not set")
 		}
 
-		err := client.Credentials().GetSingle(rs.Primary.Attributes["domain"], rs.Primary.Attributes["name"], cred)
+		manager := client.Credentials()
+		manager.Folder = formatFolderName(rs.Primary.Attributes["folder"])
+		err := manager.GetSingle(rs.Primary.Attributes["domain"], rs.Primary.Attributes["name"], cred)
 		if err != nil {
-			return fmt.Errorf("Unable to retrieve credentials for %s: %w", rs.Primary.ID, err)
-		}
-
-		return nil
-	}
-}
-
-func testAccCheckJenkinsCredentialUsernameDescriptionUpdated(cred *jenkins.UsernameCredentials, description string) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		if cred.Description != description {
-			return fmt.Errorf("Description was not set")
+			return fmt.Errorf("Unable to retrieve credentials for %s - %s: %w", rs.Primary.Attributes["folder"], rs.Primary.Attributes["name"], err)
 		}
 
 		return nil
@@ -90,9 +164,11 @@ func testAccCheckJenkinsCredentialUsernameDestroy(s *terraform.State) error {
 		}
 
 		cred := jenkins.UsernameCredentials{}
-		err := client.Credentials().GetSingle(rs.Primary.Meta["domain"].(string), rs.Primary.Meta["name"].(string), &cred)
+		manager := client.Credentials()
+		manager.Folder = formatFolderName(rs.Primary.Meta["folder"].(string))
+		err := manager.GetSingle(rs.Primary.Meta["domain"].(string), rs.Primary.Meta["name"].(string), &cred)
 		if err == nil {
-			return fmt.Errorf("Credentials %s still exists", rs.Primary.ID)
+			return fmt.Errorf("Credentials still exists: %s - %s", rs.Primary.Attributes["folder"], rs.Primary.Attributes["name"])
 		}
 	}
 

jenkins/resource_jenkins_folder.go

@@ -27,10 +27,18 @@ func resourceJenkinsFolder() *schema.Resource {
 		DeleteContext: resourceJenkinsJobDelete,
 		Schema: map[string]*schema.Schema{
 			"name": {
-				Type:        schema.TypeString,
-				Description: "The unique name of the JenkinsCI folder. Subfolders may be specified as foldername/name.",
-				Required:    true,
-				ForceNew:    true,
+				Type:             schema.TypeString,
+				Description:      "The unique name of the JenkinsCI folder.",
+				Required:         true,
+				ForceNew:         true,
+				ValidateDiagFunc: validateJobName,
+			},
+			"folder": {
+				Type:             schema.TypeString,
+				Description:      "The folder namespace that the folder will be added to as a subfolder.",
+				Optional:         true,
+				ForceNew:         true,
+				ValidateDiagFunc: validateFolderName,
 			},
 			"description": {
 				Type:        schema.TypeString,

jenkins/resource_jenkins_folder_test.go

@@ -18,7 +18,47 @@ func TestAccJenkinsFolder_basic(t *testing.T) {
 		CheckDestroy: testAccCheckJenkinsFolderDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccJenkinsFolderConfig(randString),
+				Config: fmt.Sprintf(`
+				resource jenkins_folder foo {
+				  name = "tf-acc-test-%s"
+				  description = "Terraform acceptance tests %s"
+				}`, randString, randString),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("jenkins_folder.foo", "id", "/job/tf-acc-test-"+randString),
+					resource.TestCheckResourceAttr("jenkins_folder.foo", "name", "tf-acc-test-"+randString),
+				),
+			},
+		},
+	})
+}
+
+func TestAccJenkinsFolder_nested(t *testing.T) {
+	randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckJenkinsFolderDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(`
+				resource jenkins_folder foo {
+					name = "tf-acc-test-%s"
+					description = "Terraform acceptance tests %s"
+				}
+
+				resource jenkins_folder sub {
+					name = "subfolder"
+					folder = jenkins_folder.foo.id
+					description = "Terraform acceptance tests ${jenkins_folder.foo.name}"
+				}`, randString, randString),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("jenkins_folder.foo", "id", "/job/tf-acc-test-"+randString),
+					resource.TestCheckResourceAttr("jenkins_folder.foo", "name", "tf-acc-test-"+randString),
+					resource.TestCheckResourceAttr("jenkins_folder.sub", "id", "/job/tf-acc-test-"+randString+"/job/subfolder"),
+					resource.TestCheckResourceAttr("jenkins_folder.sub", "name", "subfolder"),
+					resource.TestCheckResourceAttr("jenkins_folder.sub", "folder", "/job/tf-acc-test-"+randString),
+				),
 			},
 		},
 	})
@@ -40,12 +80,3 @@ func testAccCheckJenkinsFolderDestroy(s *terraform.State) error {
 
 	return nil
 }
-
-func testAccJenkinsFolderConfig(randString string) string {
-	return fmt.Sprintf(`
-resource jenkins_folder foo {
-  name = "tf-acc-test-%s"
-  description = "Terraform acceptance tests %s"
-}
-`, randString, randString)
-}