Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add display_name support to jenkins_folder resource #53

Merged
merged 5 commits into from
Jul 23, 2021

Conversation

zahiar
Copy link
Collaborator

@zahiar zahiar commented Jul 10, 2021

Dependencies

N/A

What Is Changing

An optional display_name argument can now be set so that you can specify a custom name to display in the Jenkins UI for the folder.

Test Steps

  • Have you updated the docs/ folder to match your change?
  • Have you exercised your change using the examples/ docker compose setup?
make test
go test -cover ./...
?       github.com/taiidani/terraform-provider-jenkins  [no test files]
ok      github.com/taiidani/terraform-provider-jenkins/jenkins  0.332s  coverage: 18.6% of statements

I tested the provider in my local Jenkins setup.

resource "jenkins_folder" "project" {
  name = "my-super-duper-project"
  display_name = "My Super Duper Project"
}

After applying, the change was present in the UI :)

Screenshot 2021-07-10 at 17 49 53

And when viewing the folder config page, I can see both name & display name values matching what I set in my Terraform code earlier.
Screenshot 2021-07-10 at 17 50 12

An optional `display_name` argument can now be set so that you can specify a custom name to display in the Jenkins UI for the folder.
@zahiar
Copy link
Collaborator Author

zahiar commented Jul 10, 2021

@taiidani The errors in the acceptance tests are down to the Terraform version being used which is generating a Terraform state file of a higher version.

    data_source_jenkins_credential_username_test.go:14: Step 1/1 error: unsupported state format version: expected "0.1", got "0.2"
    testing_new.go:56: Error retrieving state, there may be dangling resources: unsupported state format version: expected "0.1", got "0.2"
--- FAIL: TestAccJenkinsCredentialUsernameDataSource_basic (0.64s)

So locally, I switched to Terraform 0.14.11 and all tests pass.

$ tfswitch
✔ 0.14.11
Downloading https://releases.hashicorp.com/terraform/0.14.11/terraform_0.14.11_darwin_amd64.zip to terraform_0.14.11_darwin_amd64.zip
Downloading ...
34597577 bytes downloaded.
Switched terraform to version "0.14.11" 

$ make testacc
Building jenkins
[+] Building 1.7s (6/6) FINISHED                                                                                                                                                                                        
 => [internal] load build definition from Dockerfile                                                                                                                                                               0.0s
 => => transferring dockerfile: 37B                                                                                                                                                                                0.0s
 => [internal] load .dockerignore                                                                                                                                                                                  0.0s
 => => transferring context: 2B                                                                                                                                                                                    0.0s
 => [internal] load metadata for docker.io/jenkins/jenkins:lts                                                                                                                                                     1.6s
 => [1/2] FROM docker.io/jenkins/jenkins:lts@sha256:f430bcb70ab64af5e96a13427ee2e3c8ef1c1cba3688c6341ee2486a91deba2a                                                                                               0.0s
 => CACHED [2/2] RUN /usr/local/bin/install-plugins.sh hashicorp-vault-plugin cloudbees-folder pipeline-model-definition git matrix-auth                                                                           0.0s
 => exporting to image                                                                                                                                                                                             0.0s
 => => exporting layers                                                                                                                                                                                            0.0s
 => => writing image sha256:10fb7ad65fd3cd666aeb55ca17d622bc3205e363043822decc992234a5d4638d                                                                                                                       0.0s
 => => naming to docker.io/library/example_jenkins                                                                                                                                                                 0.0s

Use 'docker scan' to run Snyk tests against images to find vulnerabilities and learn how to fix them
Recreating jenkins-provider-acc ... done
Waiting for Jenkins to start...
Waiting for Jenkins to start...
Waiting for Jenkins to start...
Waiting for Jenkins to start...
Waiting for Jenkins to start...
TF_ACC=1 JENKINS_URL="http://localhost:8080" JENKINS_USERNAME="admin" JENKINS_PASSWORD="admin" go test -v -cover ./...
?       github.com/taiidani/terraform-provider-jenkins  [no test files]
=== RUN   TestNewJenkinsClient
--- PASS: TestNewJenkinsClient (0.00s)
=== RUN   TestJenkinsAdapter_Credentials
--- PASS: TestJenkinsAdapter_Credentials (0.00s)
=== RUN   TestAccJenkinsCredentialUsernameDataSource_basic
--- PASS: TestAccJenkinsCredentialUsernameDataSource_basic (3.93s)
=== RUN   TestAccJenkinsCredentialUsernameDataSource_nested
--- PASS: TestAccJenkinsCredentialUsernameDataSource_nested (1.94s)
=== RUN   TestAccJenkinsCredentialVaultAppRoleDataSource_basic
--- PASS: TestAccJenkinsCredentialVaultAppRoleDataSource_basic (1.69s)
=== RUN   TestAccJenkinsCredentialVaultAppRoleDataSource_nested
--- PASS: TestAccJenkinsCredentialVaultAppRoleDataSource_nested (1.85s)
=== RUN   TestAccJenkinsFolderDataSource_basic
--- PASS: TestAccJenkinsFolderDataSource_basic (1.71s)
=== RUN   TestAccJenkinsFolderDataSource_nested
--- PASS: TestAccJenkinsFolderDataSource_nested (1.88s)
=== RUN   TestAccJenkinsJobDataSource_basic
--- PASS: TestAccJenkinsJobDataSource_basic (1.92s)
=== RUN   TestAccJenkinsJobDataSource_nested
--- PASS: TestAccJenkinsJobDataSource_nested (1.80s)
=== RUN   Test_parseFolder
=== RUN   Test_parseFolder/success
=== RUN   Test_parseFolder/error-invalid-xml
--- PASS: Test_parseFolder (0.00s)
    --- PASS: Test_parseFolder/success (0.00s)
    --- PASS: Test_parseFolder/error-invalid-xml (0.00s)
=== RUN   Test_folder_Render
=== RUN   Test_folder_Render/success
--- PASS: Test_folder_Render (0.00s)
    --- PASS: Test_folder_Render/success (0.00s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestAccJenkinsCredentialSecretFile_basic
--- PASS: TestAccJenkinsCredentialSecretFile_basic (2.74s)
=== RUN   TestAccJenkinsCredentialSecretFile_folder
--- PASS: TestAccJenkinsCredentialSecretFile_folder (3.04s)
=== RUN   TestAccJenkinsCredentialSecretText_basic
--- PASS: TestAccJenkinsCredentialSecretText_basic (2.80s)
=== RUN   TestAccJenkinsCredentialSecretText_folder
--- PASS: TestAccJenkinsCredentialSecretText_folder (3.08s)
=== RUN   TestAccJenkinsCredentialSSH_basic
--- PASS: TestAccJenkinsCredentialSSH_basic (3.81s)
=== RUN   TestAccJenkinsCredentialSSH_folder
--- PASS: TestAccJenkinsCredentialSSH_folder (4.36s)
=== RUN   TestAccJenkinsCredentialUsername_basic
--- PASS: TestAccJenkinsCredentialUsername_basic (2.66s)
=== RUN   TestAccJenkinsCredentialUsername_folder
--- PASS: TestAccJenkinsCredentialUsername_folder (3.03s)
=== RUN   TestAccJenkinsCredentialVaultAppRole_basic
--- PASS: TestAccJenkinsCredentialVaultAppRole_basic (2.68s)
=== RUN   TestAccJenkinsCredentialVaultAppRole_folder
--- PASS: TestAccJenkinsCredentialVaultAppRole_folder (3.03s)
=== RUN   TestAccJenkinsFolder_basic
--- PASS: TestAccJenkinsFolder_basic (1.57s)
=== RUN   TestAccJenkinsFolder_nested
--- PASS: TestAccJenkinsFolder_nested (1.68s)
=== RUN   TestAccJenkinsJob_basic
--- PASS: TestAccJenkinsJob_basic (1.64s)
=== RUN   TestAccJenkinsJob_nested
--- PASS: TestAccJenkinsJob_nested (1.72s)
=== RUN   Test_resourceJenkinsJobDelete
=== RUN   Test_resourceJenkinsJobDelete/success
=== RUN   Test_resourceJenkinsJobDelete/error
--- PASS: Test_resourceJenkinsJobDelete (0.00s)
    --- PASS: Test_resourceJenkinsJobDelete/success (0.00s)
    --- PASS: Test_resourceJenkinsJobDelete/error (0.00s)
=== RUN   Test_resourceJenkinsJobRead
=== RUN   Test_resourceJenkinsJobRead/missing-job
=== RUN   Test_resourceJenkinsJobRead/error-job
--- PASS: Test_resourceJenkinsJobRead (0.00s)
    --- PASS: Test_resourceJenkinsJobRead/missing-job (0.00s)
    --- PASS: Test_resourceJenkinsJobRead/error-job (0.00s)
=== RUN   TestRenderTemplate
--- PASS: TestRenderTemplate (0.00s)
=== RUN   TestRenderTemplateInvalid
--- PASS: TestRenderTemplateInvalid (0.00s)
=== RUN   TestFormatFolderName
--- PASS: TestFormatFolderName (0.00s)
=== RUN   TestFormatFolderID
--- PASS: TestFormatFolderID (0.00s)
=== RUN   TestParseCanonicalJobID
--- PASS: TestParseCanonicalJobID (0.00s)
=== RUN   TestTemplateDiff
--- PASS: TestTemplateDiff (0.00s)
=== RUN   TestGenerateCredentialID
--- PASS: TestGenerateCredentialID (0.00s)
=== RUN   TestValidateJobName
--- PASS: TestValidateJobName (0.00s)
=== RUN   TestValidateFolderName
--- PASS: TestValidateFolderName (0.00s)
=== RUN   TestValidateCredentialScope
--- PASS: TestValidateCredentialScope (0.00s)
PASS
coverage: 67.2% of statements
ok      github.com/taiidani/terraform-provider-jenkins/jenkins  54.936s coverage: 67.2% of statements
Stopping jenkins-provider-acc ... done
Removing jenkins-provider-acc ... done
Removing network example_default

@zahiar
Copy link
Collaborator Author

zahiar commented Jul 10, 2021

@taiidani Looks like we may need to bump github.com/hashicorp/terraform-json to a higher version to be able to parse higher revision Terraform state files, kinda like the terratest project who hit the same issue: gruntwork-io/terratest#944

@zahiar
Copy link
Collaborator Author

zahiar commented Jul 14, 2021

@taiidani Ah so the other dependabot PR for bumping the terraform-plugin-sdk will solve this issue as that brings in the updated terraform-json
#52

Also, if you are open to it, I don't mind helping you to maintain this project :)

@taiidani taiidani self-requested a review July 18, 2021 16:11
@taiidani taiidani added this to the 0.8 milestone Jul 18, 2021
@codecov
Copy link

codecov bot commented Jul 18, 2021

Codecov Report

Merging #53 (5743ed9) into main (3a471c6) will increase coverage by 28.10%.
The diff coverage is 78.57%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main      #53       +/-   ##
===========================================
+ Coverage   53.29%   81.39%   +28.10%     
===========================================
  Files          17       17               
  Lines        1244     1258       +14     
===========================================
+ Hits          663     1024      +361     
+ Misses        577      184      -393     
- Partials        4       50       +46     
Flag Coverage Δ
acceptance 81.39% <78.57%> (?)
unit 53.49% <71.42%> (+0.20%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
jenkins/folder.go 100.00% <ø> (ø)
jenkins/resource_jenkins_folder.go 60.64% <66.66%> (+20.91%) ⬆️
jenkins/data_source_jenkins_folder.go 100.00% <100.00%> (+9.25%) ⬆️
...ns/data_source_jenkins_credential_vault_approle.go 100.00% <0.00%> (+11.11%) ⬆️
jenkins/data_source_jenkins_credential_username.go 100.00% <0.00%> (+12.50%) ⬆️
jenkins/provider.go 89.65% <0.00%> (+13.79%) ⬆️
jenkins/resource_jenkins_job.go 71.27% <0.00%> (+14.89%) ⬆️
jenkins/data_source_jenkins_job.go 100.00% <0.00%> (+18.51%) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a471c6...5743ed9. Read the comment docs.

@taiidani
Copy link
Owner

Thanks for your contribution! You were correct about the Dependabot PR needing to be merged, the tests are passing now.

During my own testing though I found a problem where the lack of a display_name does not fall back upon the name property. If users leave the [optional] field blank then their folders will look like this:

image

We probably want to default to name if display_name is not provided (e.g. exclude <displayName> from the XML), otherwise there won't be clickable hyperlinks for the folder lists!

Tested by:

make
cd example/
docker-compose up -d
terraform init && terraform apply
open http://localhost:8080

taiidani and others added 2 commits July 18, 2021 10:15
If it's not set, then we omit it from the XML payload we generate and submit to the Jenkins API, and Jenkins defaults to using `name`.

And added an additional test case so we cover both setting and not setting `display_name`.
@zahiar
Copy link
Collaborator Author

zahiar commented Jul 18, 2021

Thanks for catching that! I've pushed up a fix and added an additional test case, so we, well I can't get caught out by this again 😁

Copy link
Owner

@taiidani taiidani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you!

@taiidani taiidani merged commit 184ba2b into taiidani:main Jul 23, 2021
@zahiar zahiar deleted the add-folder-display-name branch August 9, 2021 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants