From 02160c6c79943e8ab2358bda4ab0899ef4e23817 Mon Sep 17 00:00:00 2001 From: Evan Read Date: Thu, 4 Apr 2019 00:55:35 +0000 Subject: Merge branch 'docs-google_secure_ldap' into 'master' Add documentation for Google Secure LDAP See merge request gitlab-org/gitlab-ce!26064 (cherry picked from commit 13ace389787f21b847ca09eccc194c3b08a7ea86) 1a0856ea Add documentation for Google Secure LDAP --- doc/administration/auth/google_secure_ldap.md | 207 +++++++++++++++++++++ .../auth/img/google_secure_ldap_add_step_1.png | Bin 0 -> 28849 bytes .../auth/img/google_secure_ldap_add_step_2.png | Bin 0 -> 82115 bytes .../img/google_secure_ldap_client_settings.png | Bin 0 -> 63959 bytes doc/administration/auth/ldap.md | 8 + 5 files changed, 215 insertions(+) create mode 100644 doc/administration/auth/google_secure_ldap.md create mode 100644 doc/administration/auth/img/google_secure_ldap_add_step_1.png create mode 100644 doc/administration/auth/img/google_secure_ldap_add_step_2.png create mode 100644 doc/administration/auth/img/google_secure_ldap_client_settings.png diff --git a/doc/administration/auth/google_secure_ldap.md b/doc/administration/auth/google_secure_ldap.md new file mode 100644 index 00000000000..65a51fc4aa0 --- /dev/null +++ b/doc/administration/auth/google_secure_ldap.md @@ -0,0 +1,207 @@ +# Google Secure LDAP **[CORE ONLY]** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/46391) in GitLab 11.9. + +[Google Cloud Identity](https://cloud.google.com/identity/) provides a Secure +LDAP service that can be configured with GitLab for authentication and group sync. + +Secure LDAP requires a slightly different configuration than standard LDAP servers. +The steps below cover: + +- Configuring the Secure LDAP Client in the Google Admin console. +- Required GitLab configuration. + +## Configuring Google LDAP client + +1. Navigate to https://admin.google.com and sign in as a GSuite domain administrator. + +1. Go to **Apps > LDAP > Add Client**. + +1. Provide an `LDAP client name` and an optional `Description`. Any descriptive + values are acceptable. For example, the name could be 'GitLab' and the + description could be 'GitLab LDAP Client'. Click the **Continue** button. + + ![Add LDAP Client Step 1](img/google_secure_ldap_add_step_1.png) + +1. Set **Access Permission** according to your needs. You must choose either + 'Entire domain (GitLab)' or 'Selected organizational units' for both 'Verify user + credentials' and 'Read user information'. Select 'Add LDAP Client' + + TIP: **Tip:** If you plan to use GitLab [LDAP Group Sync](https://docs.gitlab.com/ee/administration/auth/ldap-ee.html#group-sync) + , turn on 'Read group information'. + + ![Add LDAP Client Step 2](img/google_secure_ldap_add_step_2.png) + +1. Download the generated certificate. This is required for GitLab to + communicate with the Google Secure LDAP service. Save the downloaded certificates + for later use. After downloading, click the **Continue to Client Details** button. + +1. Expand the **Service Status** section and turn the LDAP client 'ON for everyone'. + After selecting 'Save', click on the 'Service Status' bar again to collapse + and return to the rest of the settings. + +1. Expand the **Authentication** section and choose 'Generate New Credentials'. + Copy/note these credentials for later use. After selecting 'Close', click + on the 'Authentication' bar again to collapse and return to the rest of the settings. + +Now the Google Secure LDAP Client configuration is finished. The screenshot below +shows an example of the final settings. Continue on to configure GitLab. + +![LDAP Client Settings](img/google_secure_ldap_client_settings.png) + +## Configuring GitLab + +Edit GitLab configuration, inserting the access credentials and certificate +obtained earlier. + +The following are the configuration keys that need to be modified using the +values obtained during the LDAP client configuration earlier: + +- `bind_dn`: The access credentials username +- `password`: The access credentials password +- `cert`: The `.crt` file text from the downloaded certificate bundle +- `key`: The `.key` file text from the downloaded certificate bundle + +**For Omnibus installations** + +1. Edit `/etc/gitlab/gitlab.rb`: + + ```ruby + gitlab_rails['ldap_enabled'] = true + gitlab_rails['ldap_servers'] = YAML.load <<-EOS # remember to close this block with 'EOS' below + main: # 'main' is the GitLab 'provider ID' of this LDAP server + label: 'Google Secure LDAP' + + host: 'ldap.google.com' + port: 636 + uid: 'uid' + bind_dn: 'DizzyHorse' + password: 'd6V5H8nhMUW9AuDP25abXeLd' + encryption: 'simple_tls' + verify_certificates: true + + tls_options: + cert: | + -----BEGIN CERTIFICATE----- + MIIDbDCCAlSgAwIBAgIGAWlzxiIfMA0GCSqGSIb3DQEBCwUAMHcxFDASBgNVBAoTC0dvb2dsZSBJ + bmMuMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MRQwEgYDVQQDEwtMREFQIENsaWVudDEPMA0GA1UE + CxMGR1N1aXRlMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTAeFw0xOTAzMTIyMTE5 + MThaFw0yMjAzMTEyMTE5MThaMHcxFDASBgNVBAoTC0dvb2dsZSBJbmMuMRYwFAYDVQQHEw1Nb3Vu + dGFpbiBWaWV3MRQwEgYDVQQDEwtMREFQIENsaWVudDEPMA0GA1UECxMGR1N1aXRlMQswCQYDVQQG + EwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB + ALOTy4aC38dyjESk6N8fRsKk8DN23ZX/GaNFL5OUmmA1KWzrvVC881OzNdtGm3vNOIxr9clteEG/ + tQwsmsJvQT5U+GkBt+tGKF/zm7zueHUYqTP7Pg5pxAnAei90qkIRFi17ulObyRHPYv1BbCt8pxNB + 4fG/gAXkFbCNxwh1eiQXXRTfruasCZ4/mHfX7MVm8JmWU9uAVIOLW+DSWOFhrDQduJdGBXJOyC2r + Gqoeg9+tkBmNH/jjxpnEkFW8q7io9DdOUqqNgoidA1h9vpKTs3084sy2DOgUvKN9uXWx14uxIyYU + Y1DnDy0wczcsuRt7l+EgtCEgpsLiLJQbKW+JS1UCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAf60J + yazhbHkDKIH2gFxfm7QLhhnqsmafvl4WP7JqZt0u0KdnvbDPfokdkM87yfbKJU1MTI86M36wEC+1 + P6bzklKz7kXbzAD4GggksAzxsEE64OWHC+Y64Tkxq2NiZTw/76POkcg9StiIXjG0ZcebHub9+Ux/ + rTncip92nDuvgEM7lbPFKRIS/YMhLCk09B/U0F6XLsf1yYjyf5miUTDikPkov23b/YGfpc8kh6hq + 1kqdi6a1cYPP34eAhtRhMqcZU9qezpJF6s9EeN/3YFfKzLODFSsVToBRAdZgGHzj//SAtLyQTD4n + KCSvK1UmaMxNaZyTHg8JnMf0ZuRpv26iSg== + -----END CERTIFICATE----- + + key: | + -----BEGIN PRIVATE KEY----- + MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCzk8uGgt/HcoxEpOjfH0bCpPAz + dt2V/xmjRS+TlJpgNSls671QvPNTszXbRpt7zTiMa/XJbXhBv7UMLJrCb0E+VPhpAbfrRihf85u8 + 7nh1GKkz+z4OacQJwHovdKpCERYte7pTm8kRz2L9QWwrfKcTQeHxv4AF5BWwjccIdXokF10U367m + rAmeP5h31+zFZvCZllPbgFSDi1vg0ljhYaw0HbiXRgVyTsgtqxqqHoPfrZAZjR/448aZxJBVvKu4 + qPQ3TlKqjYKInQNYfb6Sk7N9POLMtgzoFLyjfbl1sdeLsSMmFGNQ5w8tMHM3LLkbe5fhILQhIKbC + 4iyUGylviUtVAgMBAAECggEAIPb0CQy0RJoX+q/lGbRVmnyJpYDf+115WNnl+mrwjdGkeZyqw4v0 + BPzkWYzUFP1esJRO6buBNFybQRFdFW0z5lvVv/zzRKq71aVUBPInxaMRyHuJ8D5lIL8nDtgVOwyE + 7DOGyDtURUMzMjdUwoTe7K+O6QBU4X/1pVPZYgmissYSMmt68LiP8k0p601F4+r5xOi/QEy44aVp + aOJZBUOisKB8BmUXZqmQ4Cy05vU9Xi1rLyzkn9s7fxnZ+JO6Sd1r0Thm1mE0yuPgxkDBh/b4f3/2 + GsQNKKKCiij/6TfkjnBi8ZvWR44LnKpu760g/K7psVNrKwqJG6C/8RAcgISWQQKBgQDop7BaKGhK + 1QMJJ/vnlyYFTucfGLn6bM//pzTys5Gop0tpcfX/Hf6a6Dd+zBhmC3tBmhr80XOX/PiyAIbc0lOI + 31rafZuD/oVx5mlIySWX35EqS14LXmdVs/5vOhsInNgNiE+EPFf1L9YZgG/zA7OUBmqtTeYIPDVC + 7ViJcydItQKBgQDFmK0H0IA6W4opGQo+zQKhefooqZ+RDk9IIZMPOAtnvOM7y3rSVrfsSjzYVuMS + w/RP/vs7rwhaZejnCZ8/7uIqwg4sdUBRzZYR3PRNFeheW+BPZvb+2keRCGzOs7xkbF1mu54qtYTa + HZGZj1OsD83AoMwVLcdLDgO1kw32dkS8IQKBgFRdgoifAHqqVah7VFB9se7Y1tyi5cXWsXI+Wufr + j9U9nQ4GojK52LqpnH4hWnOelDqMvF6TQTyLIk/B+yWWK26Ft/dk9wDdSdystd8L+dLh4k0Y+Whb + +lLMq2YABw+PeJUnqdYE38xsZVHoDjBsVjFGRmbDybeQxauYT7PACy3FAoGBAK2+k9bdNQMbXp7I + j8OszHVkJdz/WXlY1cmdDAxDwXOUGVKIlxTAf7TbiijILZ5gg0Cb+hj+zR9/oI0WXtr+mAv02jWp + W8cSOLS4TnBBpTLjIpdu+BwbnvYeLF6MmEjNKEufCXKQbaLEgTQ/XNlchBSuzwSIXkbWqdhM1+gx + EjtBAoGARAdMIiDMPWIIZg3nNnFebbmtBP0qiBsYohQZ+6i/8s/vautEHBEN6Q0brIU/goo+nTHc + t9VaOkzjCmAJSLPUanuBC8pdYgLu5J20NXUZLD9AE/2bBT3OpezKcdYeI2jqoc1qlWHlNtVtdqQ2 + AcZSFJQjdg5BTyvdEDhaYUKGdRw= + -----END PRIVATE KEY----- + EOS + ``` + +1. Save the file and [reconfigure] GitLab for the changes to take effect. + +--- + +**For installations from source** + +1. Edit `config/gitlab.yml`: + + ```yaml + ldap: + enabled: true + servers: + main: # 'main' is the GitLab 'provider ID' of this LDAP server + label: 'Google Secure LDAP' + + host: 'ldap.google.com' + port: 636 + uid: 'uid' + bind_dn: 'DizzyHorse' + password: 'd6V5H8nhMUW9AuDP25abXeLd' + encryption: 'simple_tls' + verify_certificates: true + + tls_options: + cert: | + -----BEGIN CERTIFICATE----- + MIIDbDCCAlSgAwIBAgIGAWlzxiIfMA0GCSqGSIb3DQEBCwUAMHcxFDASBgNVBAoTC0dvb2dsZSBJ + bmMuMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MRQwEgYDVQQDEwtMREFQIENsaWVudDEPMA0GA1UE + CxMGR1N1aXRlMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTAeFw0xOTAzMTIyMTE5 + MThaFw0yMjAzMTEyMTE5MThaMHcxFDASBgNVBAoTC0dvb2dsZSBJbmMuMRYwFAYDVQQHEw1Nb3Vu + dGFpbiBWaWV3MRQwEgYDVQQDEwtMREFQIENsaWVudDEPMA0GA1UECxMGR1N1aXRlMQswCQYDVQQG + EwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB + ALOTy4aC38dyjESk6N8fRsKk8DN23ZX/GaNFL5OUmmA1KWzrvVC881OzNdtGm3vNOIxr9clteEG/ + tQwsmsJvQT5U+GkBt+tGKF/zm7zueHUYqTP7Pg5pxAnAei90qkIRFi17ulObyRHPYv1BbCt8pxNB + 4fG/gAXkFbCNxwh1eiQXXRTfruasCZ4/mHfX7MVm8JmWU9uAVIOLW+DSWOFhrDQduJdGBXJOyC2r + Gqoeg9+tkBmNH/jjxpnEkFW8q7io9DdOUqqNgoidA1h9vpKTs3084sy2DOgUvKN9uXWx14uxIyYU + Y1DnDy0wczcsuRt7l+EgtCEgpsLiLJQbKW+JS1UCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAf60J + yazhbHkDKIH2gFxfm7QLhhnqsmafvl4WP7JqZt0u0KdnvbDPfokdkM87yfbKJU1MTI86M36wEC+1 + P6bzklKz7kXbzAD4GggksAzxsEE64OWHC+Y64Tkxq2NiZTw/76POkcg9StiIXjG0ZcebHub9+Ux/ + rTncip92nDuvgEM7lbPFKRIS/YMhLCk09B/U0F6XLsf1yYjyf5miUTDikPkov23b/YGfpc8kh6hq + 1kqdi6a1cYPP34eAhtRhMqcZU9qezpJF6s9EeN/3YFfKzLODFSsVToBRAdZgGHzj//SAtLyQTD4n + KCSvK1UmaMxNaZyTHg8JnMf0ZuRpv26iSg== + -----END CERTIFICATE----- + + key: | + -----BEGIN PRIVATE KEY----- + MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCzk8uGgt/HcoxEpOjfH0bCpPAz + dt2V/xmjRS+TlJpgNSls671QvPNTszXbRpt7zTiMa/XJbXhBv7UMLJrCb0E+VPhpAbfrRihf85u8 + 7nh1GKkz+z4OacQJwHovdKpCERYte7pTm8kRz2L9QWwrfKcTQeHxv4AF5BWwjccIdXokF10U367m + rAmeP5h31+zFZvCZllPbgFSDi1vg0ljhYaw0HbiXRgVyTsgtqxqqHoPfrZAZjR/448aZxJBVvKu4 + qPQ3TlKqjYKInQNYfb6Sk7N9POLMtgzoFLyjfbl1sdeLsSMmFGNQ5w8tMHM3LLkbe5fhILQhIKbC + 4iyUGylviUtVAgMBAAECggEAIPb0CQy0RJoX+q/lGbRVmnyJpYDf+115WNnl+mrwjdGkeZyqw4v0 + BPzkWYzUFP1esJRO6buBNFybQRFdFW0z5lvVv/zzRKq71aVUBPInxaMRyHuJ8D5lIL8nDtgVOwyE + 7DOGyDtURUMzMjdUwoTe7K+O6QBU4X/1pVPZYgmissYSMmt68LiP8k0p601F4+r5xOi/QEy44aVp + aOJZBUOisKB8BmUXZqmQ4Cy05vU9Xi1rLyzkn9s7fxnZ+JO6Sd1r0Thm1mE0yuPgxkDBh/b4f3/2 + GsQNKKKCiij/6TfkjnBi8ZvWR44LnKpu760g/K7psVNrKwqJG6C/8RAcgISWQQKBgQDop7BaKGhK + 1QMJJ/vnlyYFTucfGLn6bM//pzTys5Gop0tpcfX/Hf6a6Dd+zBhmC3tBmhr80XOX/PiyAIbc0lOI + 31rafZuD/oVx5mlIySWX35EqS14LXmdVs/5vOhsInNgNiE+EPFf1L9YZgG/zA7OUBmqtTeYIPDVC + 7ViJcydItQKBgQDFmK0H0IA6W4opGQo+zQKhefooqZ+RDk9IIZMPOAtnvOM7y3rSVrfsSjzYVuMS + w/RP/vs7rwhaZejnCZ8/7uIqwg4sdUBRzZYR3PRNFeheW+BPZvb+2keRCGzOs7xkbF1mu54qtYTa + HZGZj1OsD83AoMwVLcdLDgO1kw32dkS8IQKBgFRdgoifAHqqVah7VFB9se7Y1tyi5cXWsXI+Wufr + j9U9nQ4GojK52LqpnH4hWnOelDqMvF6TQTyLIk/B+yWWK26Ft/dk9wDdSdystd8L+dLh4k0Y+Whb + +lLMq2YABw+PeJUnqdYE38xsZVHoDjBsVjFGRmbDybeQxauYT7PACy3FAoGBAK2+k9bdNQMbXp7I + j8OszHVkJdz/WXlY1cmdDAxDwXOUGVKIlxTAf7TbiijILZ5gg0Cb+hj+zR9/oI0WXtr+mAv02jWp + W8cSOLS4TnBBpTLjIpdu+BwbnvYeLF6MmEjNKEufCXKQbaLEgTQ/XNlchBSuzwSIXkbWqdhM1+gx + EjtBAoGARAdMIiDMPWIIZg3nNnFebbmtBP0qiBsYohQZ+6i/8s/vautEHBEN6Q0brIU/goo+nTHc + t9VaOkzjCmAJSLPUanuBC8pdYgLu5J20NXUZLD9AE/2bBT3OpezKcdYeI2jqoc1qlWHlNtVtdqQ2 + AcZSFJQjdg5BTyvdEDhaYUKGdRw= + -----END PRIVATE KEY----- + ``` + +1. Save the file and [restart] GitLab for the changes to take effect. + + +[reconfigure]: ../restart_gitlab.md#omnibus-gitlab-reconfigure +[restart]: ../restart_gitlab.md#installations-from-source diff --git a/doc/administration/auth/img/google_secure_ldap_add_step_1.png b/doc/administration/auth/img/google_secure_ldap_add_step_1.png new file mode 100644 index 00000000000..fd254443d75 Binary files /dev/null and b/doc/administration/auth/img/google_secure_ldap_add_step_1.png differ diff --git a/doc/administration/auth/img/google_secure_ldap_add_step_2.png b/doc/administration/auth/img/google_secure_ldap_add_step_2.png new file mode 100644 index 00000000000..611a21ae03c Binary files /dev/null and b/doc/administration/auth/img/google_secure_ldap_add_step_2.png differ diff --git a/doc/administration/auth/img/google_secure_ldap_client_settings.png b/doc/administration/auth/img/google_secure_ldap_client_settings.png new file mode 100644 index 00000000000..3c0b3f3d4bd Binary files /dev/null and b/doc/administration/auth/img/google_secure_ldap_client_settings.png differ diff --git a/doc/administration/auth/ldap.md b/doc/administration/auth/ldap.md index 440c2b1285a..2d057dc7509 100644 --- a/doc/administration/auth/ldap.md +++ b/doc/administration/auth/ldap.md @@ -48,6 +48,14 @@ LDAP-enabled users can always authenticate with Git using their GitLab username or email and LDAP password, even if password authentication for Git is disabled in the application settings. +## Google Secure LDAP **[CORE ONLY]** + +> Introduced in GitLab 11.9. + +[Google Cloud Identity](https://cloud.google.com/identity/) provides a Secure +LDAP service that can be configured with GitLab for authentication and group sync. +See [Google Secure LDAP](google_secure_ldap.md) for detailed configuration instructions. + ## Configuration NOTE: **Note**: -- cgit v1.2.1 From c044bfe84aa69af65dd2923ecd8e49573974f354 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 4 Apr 2019 14:54:25 +0000 Subject: Merge branch 'id-mr-list-when-filtered-by-approvers-only' into 'master' Consider array params on rendering MR list on dashboard See merge request gitlab-org/gitlab-ce!26623 (cherry picked from commit 7926384ff32b9ad8833dcfffc9bb87d036c4bd21) b3f5413a Consider array params on rendering MR list on dashboard --- app/controllers/dashboard_controller.rb | 5 +++- app/finders/issuable_finder.rb | 1 - .../concerns/issuable_collections_spec.rb | 4 +-- spec/controllers/dashboard_controller_spec.rb | 33 ++++++++++++++++++++++ spec/features/dashboard/merge_requests_spec.rb | 25 ++++++++++++++++ 5 files changed, 64 insertions(+), 4 deletions(-) diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 75329b05a6f..1a97b39d3ae 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -46,7 +46,10 @@ class DashboardController < Dashboard::ApplicationController end def check_filters_presence! - @no_filters_set = finder_type.scalar_params.none? { |k| params.key?(k) } + no_scalar_filters_set = finder_type.scalar_params.none? { |k| params.key?(k) } + no_array_filters_set = finder_type.array_params.none? { |k, _| params.key?(k) } + + @no_filters_set = no_scalar_filters_set && no_array_filters_set return unless @no_filters_set diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 072d07e0ed2..fa434a3b4e8 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -53,7 +53,6 @@ class IssuableFinder assignee_username author_id author_username - label_name milestone_title my_reaction_emoji search diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb index 8580900215c..a82b66361ca 100644 --- a/spec/controllers/concerns/issuable_collections_spec.rb +++ b/spec/controllers/concerns/issuable_collections_spec.rb @@ -117,7 +117,7 @@ describe IssuableCollections do due_date: '2017-01-01', group_id: '3', iids: '4', - label_name: 'foo', + label_name: ['foo'], milestone_title: 'bar', my_reaction_emoji: 'thumbsup', non_archived: 'true', @@ -142,7 +142,7 @@ describe IssuableCollections do 'author_id' => '2', 'author_username' => 'user2', 'confidential' => true, - 'label_name' => 'foo', + 'label_name' => ['foo'], 'milestone_title' => 'bar', 'my_reaction_emoji' => 'thumbsup', 'due_date' => '2017-01-01', diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index c857a78d5e8..b039ec2906c 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -23,4 +23,37 @@ describe DashboardController do it_behaves_like 'authenticates sessionless user', :issues, :atom, author_id: User.first it_behaves_like 'authenticates sessionless user', :issues_calendar, :ics + + describe "#check_filters_presence!" do + let(:user) { create(:user) } + + before do + sign_in(user) + get :merge_requests, params: params + end + + context "no filters" do + let(:params) { {} } + + it 'sets @no_filters_set to false' do + expect(assigns[:no_filters_set]).to eq(true) + end + end + + context "scalar filters" do + let(:params) { { author_id: user.id } } + + it 'sets @no_filters_set to false' do + expect(assigns[:no_filters_set]).to eq(false) + end + end + + context "array filters" do + let(:params) { { label_name: ['bug'] } } + + it 'sets @no_filters_set to false' do + expect(assigns[:no_filters_set]).to eq(false) + end + end + end end diff --git a/spec/features/dashboard/merge_requests_spec.rb b/spec/features/dashboard/merge_requests_spec.rb index 9ffa75aee47..4965770605a 100644 --- a/spec/features/dashboard/merge_requests_spec.rb +++ b/spec/features/dashboard/merge_requests_spec.rb @@ -44,6 +44,8 @@ describe 'Dashboard Merge Requests' do end context 'merge requests exist' do + let(:label) { create(:label) } + let!(:assigned_merge_request) do create(:merge_request, assignee: current_user, @@ -72,6 +74,14 @@ describe 'Dashboard Merge Requests' do target_project: public_project, source_project: forked_project) end + let!(:labeled_merge_request) do + create(:labeled_merge_request, + source_branch: 'labeled', + labels: [label], + author: current_user, + source_project: project) + end + let!(:other_merge_request) do create(:merge_request, source_branch: 'fix', @@ -90,6 +100,7 @@ describe 'Dashboard Merge Requests' do expect(page).not_to have_content(authored_merge_request.title) expect(page).not_to have_content(authored_merge_request_from_fork.title) expect(page).not_to have_content(other_merge_request.title) + expect(page).not_to have_content(labeled_merge_request.title) end it 'shows authored merge requests', :js do @@ -98,7 +109,21 @@ describe 'Dashboard Merge Requests' do expect(page).to have_content(authored_merge_request.title) expect(page).to have_content(authored_merge_request_from_fork.title) + expect(page).to have_content(labeled_merge_request.title) + + expect(page).not_to have_content(assigned_merge_request.title) + expect(page).not_to have_content(assigned_merge_request_from_fork.title) + expect(page).not_to have_content(other_merge_request.title) + end + + it 'shows labeled merge requests', :js do + reset_filters + input_filtered_search("label:#{label.name}") + expect(page).to have_content(labeled_merge_request.title) + + expect(page).not_to have_content(authored_merge_request.title) + expect(page).not_to have_content(authored_merge_request_from_fork.title) expect(page).not_to have_content(assigned_merge_request.title) expect(page).not_to have_content(assigned_merge_request_from_fork.title) expect(page).not_to have_content(other_merge_request.title) -- cgit v1.2.1 From 52778632c1fbd55400faa1e8308280cd727d8153 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Mon, 1 Apr 2019 15:13:23 +0000 Subject: Merge branch 'fj-59547-fix-has-commits' into 'master' Fix MergeRequest#has_commits? nil comparison Closes #59547 See merge request gitlab-org/gitlab-ce!26828 (cherry picked from commit b224efe767e4f8e24b51b87753f55bf6d3129f68) 6645b825 Fix MergeRequest#commits_count nil comparison --- app/models/merge_request.rb | 2 +- spec/models/merge_request_spec.rb | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index acf80addc6a..06a86a1f36f 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1301,7 +1301,7 @@ class MergeRequest < ActiveRecord::Base end def has_commits? - merge_request_diff && commits_count > 0 + merge_request_diff && commits_count.to_i > 0 end def has_no_commits? diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 07cb4c9c1e3..87db9e574cf 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2613,14 +2613,21 @@ describe MergeRequest do end describe '#has_commits?' do - before do + it 'returns true when merge request diff has commits' do allow(subject.merge_request_diff).to receive(:commits_count) .and_return(2) - end - it 'returns true when merge request diff has commits' do expect(subject.has_commits?).to be_truthy end + + context 'when commits_count is nil' do + it 'returns false' do + allow(subject.merge_request_diff).to receive(:commits_count) + .and_return(nil) + + expect(subject.has_commits?).to be_falsey + end + end end describe '#has_no_commits?' do -- cgit v1.2.1 From 3fec9fd3f546fc080fab2d58de580edf30798726 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 4 Apr 2019 15:00:56 +0000 Subject: Merge branch 'allow-to-use-untrusted-ruby-syntax' into 'master' Allow to use untrusted ruby syntax See merge request gitlab-org/gitlab-ce!26905 (cherry picked from commit 7dcc3003119666c75a35c27d73ffb297c696fcc8) 9bc4fbf1 Allow to use untrusted Regexp via feature flag --- .../allow-to-use-untrusted-ruby-syntax.yml | 5 ++ doc/ci/yaml/README.md | 21 +++++++ lib/gitlab/ci/build/policy/refs.rb | 2 +- lib/gitlab/ci/config/entry/policy.rb | 4 +- lib/gitlab/config/entry/validators.rb | 16 ++++- lib/gitlab/untrusted_regexp/ruby_syntax.rb | 39 +++++++++---- spec/lib/gitlab/ci/build/policy/refs_spec.rb | 26 +++++++++ spec/lib/gitlab/ci/config/entry/policy_spec.rb | 67 +++++++++++++++++++++ .../gitlab/untrusted_regexp/ruby_syntax_spec.rb | 68 ++++++++++++++++++---- 9 files changed, 223 insertions(+), 25 deletions(-) create mode 100644 changelogs/unreleased/allow-to-use-untrusted-ruby-syntax.yml diff --git a/changelogs/unreleased/allow-to-use-untrusted-ruby-syntax.yml b/changelogs/unreleased/allow-to-use-untrusted-ruby-syntax.yml new file mode 100644 index 00000000000..731c9c10b00 --- /dev/null +++ b/changelogs/unreleased/allow-to-use-untrusted-ruby-syntax.yml @@ -0,0 +1,5 @@ +--- +title: Allow to use untrusted Regexp via feature flag +merge_request: 26905 +author: +type: deprecated diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 816d12a8dd4..bc498939522 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -397,6 +397,27 @@ job: only: ['branches', 'tags'] ``` +### Supported `only`/`except` regexp syntax + +CAUTION: **Warning:** +This is a breaking change that was introduced with GitLab 11.9.4. + +In GitLab 11.9.4, GitLab begun internally converting regexp used +in `only` and `except` parameters to [RE2](https://github.com/google/re2/wiki/Syntax). + +This means that only subset of features provided by [Ruby Regexp](https://ruby-doc.org/core/Regexp.html) +is supported. [RE2](https://github.com/google/re2/wiki/Syntax) limits the set of features +provided due to computational complexity, which means some features became unavailable in GitLab 11.9.4. +For example, negative lookaheads. + +For GitLab versions from 11.9.7 and up to GitLab 12.0, GitLab provides a feature flag that can be +enabled by administrators that allows users to use unsafe regexp syntax. This brings compatibility +with previously allowed syntax version and allows users to gracefully migrate to the new syntax. + +```ruby +Feature.enable(:allow_unsafe_ruby_regexp) +``` + ### `only`/`except` (advanced) > - `refs` and `kubernetes` policies introduced in GitLab 10.0. diff --git a/lib/gitlab/ci/build/policy/refs.rb b/lib/gitlab/ci/build/policy/refs.rb index 360424bec11..c3005303fd8 100644 --- a/lib/gitlab/ci/build/policy/refs.rb +++ b/lib/gitlab/ci/build/policy/refs.rb @@ -35,7 +35,7 @@ module Gitlab # patterns can be matched only when branch or tag is used # the pattern matching does not work for merge requests pipelines if pipeline.branch? || pipeline.tag? - if regexp = Gitlab::UntrustedRegexp::RubySyntax.fabricate(pattern) + if regexp = Gitlab::UntrustedRegexp::RubySyntax.fabricate(pattern, fallback: true) regexp.match?(pipeline.ref) else pattern == pipeline.ref diff --git a/lib/gitlab/ci/config/entry/policy.rb b/lib/gitlab/ci/config/entry/policy.rb index adc3660d950..7b14218d3ea 100644 --- a/lib/gitlab/ci/config/entry/policy.rb +++ b/lib/gitlab/ci/config/entry/policy.rb @@ -17,7 +17,7 @@ module Gitlab include ::Gitlab::Config::Entry::Validatable validations do - validates :config, array_of_strings_or_regexps: true + validates :config, array_of_strings_or_regexps_with_fallback: true end def value @@ -38,7 +38,7 @@ module Gitlab validate :variables_expressions_syntax with_options allow_nil: true do - validates :refs, array_of_strings_or_regexps: true + validates :refs, array_of_strings_or_regexps_with_fallback: true validates :kubernetes, allowed_values: %w[active] validates :variables, array_of_strings: true validates :changes, array_of_strings: true diff --git a/lib/gitlab/config/entry/validators.rb b/lib/gitlab/config/entry/validators.rb index d348e11b753..9b2ace382ef 100644 --- a/lib/gitlab/config/entry/validators.rb +++ b/lib/gitlab/config/entry/validators.rb @@ -118,6 +118,12 @@ module Gitlab end end + protected + + def fallback + false + end + private def matches_syntax?(value) @@ -126,7 +132,7 @@ module Gitlab def validate_regexp(value) matches_syntax?(value) && - Gitlab::UntrustedRegexp::RubySyntax.valid?(value) + Gitlab::UntrustedRegexp::RubySyntax.valid?(value, fallback: fallback) end end @@ -151,6 +157,14 @@ module Gitlab end end + class ArrayOfStringsOrRegexpsWithFallbackValidator < ArrayOfStringsOrRegexpsValidator + protected + + def fallback + true + end + end + class ArrayOfStringsOrStringValidator < RegexpValidator def validate_each(record, attribute, value) unless validate_array_of_strings_or_string(value) diff --git a/lib/gitlab/untrusted_regexp/ruby_syntax.rb b/lib/gitlab/untrusted_regexp/ruby_syntax.rb index 91f300f97d0..6adf119aa75 100644 --- a/lib/gitlab/untrusted_regexp/ruby_syntax.rb +++ b/lib/gitlab/untrusted_regexp/ruby_syntax.rb @@ -6,7 +6,7 @@ module Gitlab # and converts that to RE2 representation: # // class RubySyntax - PATTERN = %r{^/(?.+)/(?[ismU]*)$}.freeze + PATTERN = %r{^/(?.*)/(?[ismU]*)$}.freeze # Checks if pattern matches a regexp pattern # but does not enforce it's validity @@ -16,28 +16,47 @@ module Gitlab # The regexp can match the pattern `/.../`, but may not be fabricatable: # it can be invalid or incomplete: `/match ( string/` - def self.valid?(pattern) - !!self.fabricate(pattern) + def self.valid?(pattern, fallback: false) + !!self.fabricate(pattern, fallback: fallback) end - def self.fabricate(pattern) - self.fabricate!(pattern) + def self.fabricate(pattern, fallback: false) + self.fabricate!(pattern, fallback: fallback) rescue RegexpError nil end - def self.fabricate!(pattern) + def self.fabricate!(pattern, fallback: false) raise RegexpError, 'Pattern is not string!' unless pattern.is_a?(String) matches = pattern.match(PATTERN) raise RegexpError, 'Invalid regular expression!' if matches.nil? - expression = matches[:regexp] - flags = matches[:flags] - expression.prepend("(?#{flags})") if flags.present? + begin + create_untrusted_regexp(matches[:regexp], matches[:flags]) + rescue RegexpError + raise unless fallback && + Feature.enabled?(:allow_unsafe_ruby_regexp, default_enabled: false) - UntrustedRegexp.new(expression, multiline: false) + create_ruby_regexp(matches[:regexp], matches[:flags]) + end end + + def self.create_untrusted_regexp(pattern, flags) + pattern.prepend("(?#{flags})") if flags.present? + + UntrustedRegexp.new(pattern, multiline: false) + end + private_class_method :create_untrusted_regexp + + def self.create_ruby_regexp(pattern, flags) + options = 0 + options += Regexp::IGNORECASE if flags&.include?('i') + options += Regexp::MULTILINE if flags&.include?('m') + + Regexp.new(pattern, options) + end + private_class_method :create_ruby_regexp end end end diff --git a/spec/lib/gitlab/ci/build/policy/refs_spec.rb b/spec/lib/gitlab/ci/build/policy/refs_spec.rb index ec0450643c3..22ca681cfd3 100644 --- a/spec/lib/gitlab/ci/build/policy/refs_spec.rb +++ b/spec/lib/gitlab/ci/build/policy/refs_spec.rb @@ -101,6 +101,32 @@ describe Gitlab::Ci::Build::Policy::Refs do expect(described_class.new(['/fix-.*/'])) .not_to be_satisfied_by(pipeline) end + + context 'when unsafe regexp is used' do + let(:subject) { described_class.new(['/^(?!master).+/']) } + + context 'when allow_unsafe_ruby_regexp is disabled' do + before do + stub_feature_flags(allow_unsafe_ruby_regexp: false) + end + + it 'ignores invalid regexp' do + expect(subject) + .not_to be_satisfied_by(pipeline) + end + end + + context 'when allow_unsafe_ruby_regexp is enabled' do + before do + stub_feature_flags(allow_unsafe_ruby_regexp: true) + end + + it 'is satisfied by regexp' do + expect(subject) + .to be_satisfied_by(pipeline) + end + end + end end context 'malicious regexp' do diff --git a/spec/lib/gitlab/ci/config/entry/policy_spec.rb b/spec/lib/gitlab/ci/config/entry/policy_spec.rb index 1c987e13a9a..fba5671594d 100644 --- a/spec/lib/gitlab/ci/config/entry/policy_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/policy_spec.rb @@ -1,4 +1,5 @@ require 'fast_spec_helper' +require 'support/helpers/stub_feature_flags' require_dependency 'active_model' describe Gitlab::Ci::Config::Entry::Policy do @@ -33,6 +34,44 @@ describe Gitlab::Ci::Config::Entry::Policy do end end + context 'when config is an empty regexp' do + let(:config) { ['//'] } + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + end + + context 'when using unsafe regexp' do + include StubFeatureFlags + + let(:config) { ['/^(?!master).+/'] } + + subject { described_class.new([regexp]) } + + context 'when allow_unsafe_ruby_regexp is disabled' do + before do + stub_feature_flags(allow_unsafe_ruby_regexp: false) + end + + it 'is not valid' do + expect(entry).not_to be_valid + end + end + + context 'when allow_unsafe_ruby_regexp is enabled' do + before do + stub_feature_flags(allow_unsafe_ruby_regexp: true) + end + + it 'is valid' do + expect(entry).to be_valid + end + end + end + context 'when config is a special keyword' do let(:config) { %w[tags triggers branches] } @@ -67,6 +106,34 @@ describe Gitlab::Ci::Config::Entry::Policy do end end + context 'when using unsafe regexp' do + include StubFeatureFlags + + let(:config) { { refs: ['/^(?!master).+/'] } } + + subject { described_class.new([regexp]) } + + context 'when allow_unsafe_ruby_regexp is disabled' do + before do + stub_feature_flags(allow_unsafe_ruby_regexp: false) + end + + it 'is not valid' do + expect(entry).not_to be_valid + end + end + + context 'when allow_unsafe_ruby_regexp is enabled' do + before do + stub_feature_flags(allow_unsafe_ruby_regexp: true) + end + + it 'is valid' do + expect(entry).to be_valid + end + end + end + context 'when specifying kubernetes policy' do let(:config) { { kubernetes: 'active' } } diff --git a/spec/lib/gitlab/untrusted_regexp/ruby_syntax_spec.rb b/spec/lib/gitlab/untrusted_regexp/ruby_syntax_spec.rb index 005d41580de..f1882e03581 100644 --- a/spec/lib/gitlab/untrusted_regexp/ruby_syntax_spec.rb +++ b/spec/lib/gitlab/untrusted_regexp/ruby_syntax_spec.rb @@ -1,5 +1,6 @@ require 'fast_spec_helper' require 'support/shared_examples/malicious_regexp_shared_examples' +require 'support/helpers/stub_feature_flags' describe Gitlab::UntrustedRegexp::RubySyntax do describe '.matches_syntax?' do @@ -33,6 +34,12 @@ describe Gitlab::UntrustedRegexp::RubySyntax do end end + context 'when regexp is empty' do + it 'fabricates regexp correctly' do + expect(described_class.fabricate('//')).not_to be_nil + end + end + context 'when regexp is a raw pattern' do it 'returns error' do expect(described_class.fabricate('some .* thing')).to be_nil @@ -41,24 +48,63 @@ describe Gitlab::UntrustedRegexp::RubySyntax do end describe '.fabricate!' do - context 'when regexp is using /regexp/ scheme with flags' do - it 'fabricates regexp with a single flag' do - regexp = described_class.fabricate!('/something/i') + context 'safe regexp is used' do + context 'when regexp is using /regexp/ scheme with flags' do + it 'fabricates regexp with a single flag' do + regexp = described_class.fabricate!('/something/i') + + expect(regexp).to eq Gitlab::UntrustedRegexp.new('(?i)something') + expect(regexp.scan('SOMETHING')).to be_one + end - expect(regexp).to eq Gitlab::UntrustedRegexp.new('(?i)something') - expect(regexp.scan('SOMETHING')).to be_one + it 'fabricates regexp with multiple flags' do + regexp = described_class.fabricate!('/something/im') + + expect(regexp).to eq Gitlab::UntrustedRegexp.new('(?im)something') + end + + it 'fabricates regexp without flags' do + regexp = described_class.fabricate!('/something/') + + expect(regexp).to eq Gitlab::UntrustedRegexp.new('something') + end end + end - it 'fabricates regexp with multiple flags' do - regexp = described_class.fabricate!('/something/im') + context 'when unsafe regexp is used' do + include StubFeatureFlags - expect(regexp).to eq Gitlab::UntrustedRegexp.new('(?im)something') + before do + stub_feature_flags(allow_unsafe_ruby_regexp: true) + + allow(Gitlab::UntrustedRegexp).to receive(:new).and_raise(RegexpError) end - it 'fabricates regexp without flags' do - regexp = described_class.fabricate!('/something/') + context 'when no fallback is enabled' do + it 'raises an exception' do + expect { described_class.fabricate!('/something/') } + .to raise_error(RegexpError) + end + end + + context 'when fallback is used' do + it 'fabricates regexp with a single flag' do + regexp = described_class.fabricate!('/something/i', fallback: true) + + expect(regexp).to eq Regexp.new('something', Regexp::IGNORECASE) + end + + it 'fabricates regexp with multiple flags' do + regexp = described_class.fabricate!('/something/im', fallback: true) + + expect(regexp).to eq Regexp.new('something', Regexp::IGNORECASE | Regexp::MULTILINE) + end + + it 'fabricates regexp without flags' do + regexp = described_class.fabricate!('/something/', fallback: true) - expect(regexp).to eq Gitlab::UntrustedRegexp.new('something') + expect(regexp).to eq Regexp.new('something') + end end end -- cgit v1.2.1 From bd8f532d80e5b220f552a492cb0beca7e6b725d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Mon, 8 Apr 2019 16:20:23 +0000 Subject: Merge branch 'fix-pull-request-importer' into 'master' Improve performance of PR import See merge request gitlab-org/gitlab-ce!27121 (cherry picked from commit f15caf0109998308e7f960baaa541d73be8bcacb) f3ad51f8 Improve performance of PR import --- .../unreleased/fix-pull-request-importer.yml | 5 ++ lib/gitlab/import/merge_request_helpers.rb | 2 +- .../gitlab/import/merge_request_helpers_spec.rb | 73 ++++++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/fix-pull-request-importer.yml create mode 100644 spec/lib/gitlab/import/merge_request_helpers_spec.rb diff --git a/changelogs/unreleased/fix-pull-request-importer.yml b/changelogs/unreleased/fix-pull-request-importer.yml new file mode 100644 index 00000000000..5f642a0710b --- /dev/null +++ b/changelogs/unreleased/fix-pull-request-importer.yml @@ -0,0 +1,5 @@ +--- +title: Improve performance of PR import +merge_request: 27121 +author: +type: performance diff --git a/lib/gitlab/import/merge_request_helpers.rb b/lib/gitlab/import/merge_request_helpers.rb index b3fe1fc0685..4bc39868389 100644 --- a/lib/gitlab/import/merge_request_helpers.rb +++ b/lib/gitlab/import/merge_request_helpers.rb @@ -22,7 +22,7 @@ module Gitlab # additional work that is strictly necessary. merge_request_id = insert_and_return_id(attributes, project.merge_requests) - merge_request = project.merge_requests.reload.find(merge_request_id) + merge_request = project.merge_requests.reset.find(merge_request_id) [merge_request, false] end diff --git a/spec/lib/gitlab/import/merge_request_helpers_spec.rb b/spec/lib/gitlab/import/merge_request_helpers_spec.rb new file mode 100644 index 00000000000..cc0f2baf905 --- /dev/null +++ b/spec/lib/gitlab/import/merge_request_helpers_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Import::MergeRequestHelpers, type: :helper do + set(:project) { create(:project, :repository) } + set(:user) { create(:user) } + + describe '.create_merge_request_without_hooks' do + let(:iid) { 42 } + + let(:attributes) do + { + iid: iid, + title: 'My Pull Request', + description: 'This is my pull request', + source_project_id: project.id, + target_project_id: project.id, + source_branch: 'master-42', + target_branch: 'master', + state: :merged, + author_id: user.id, + assignee_id: user.id + } + end + + subject { helper.create_merge_request_without_hooks(project, attributes, iid) } + + context 'when merge request does not exist' do + it 'returns a new object' do + expect(subject.first).not_to be_nil + expect(subject.second).to eq(false) + end + + it 'does load all existing objects' do + 5.times do |iid| + MergeRequest.create!( + attributes.merge(iid: iid, source_branch: iid.to_s)) + end + + # does ensure that we only load object twice + # 1. by #insert_and_return_id + # 2. by project.merge_requests.find + expect_any_instance_of(MergeRequest).to receive(:attributes) + .twice.times.and_call_original + + expect(subject.first).not_to be_nil + expect(subject.second).to eq(false) + end + end + + context 'when merge request does exist' do + before do + MergeRequest.create!(attributes) + end + + it 'returns an existing object' do + expect(subject.first).not_to be_nil + expect(subject.second).to eq(true) + end + end + + context 'when project is deleted' do + before do + project.delete + end + + it 'returns an existing object' do + expect(subject.first).to be_nil + end + end + end +end -- cgit v1.2.1 From e476b49d5ab2c02328787c211e832e886e3cde1d Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Wed, 10 Apr 2019 21:11:55 +0000 Subject: Merge branch 'sh-disable-diff-instrumentation' into 'master' Disable method instrumentation for diffs Closes #52898 See merge request gitlab-org/gitlab-ce!27235 (cherry picked from commit b9e6e72501173c29a30ee1b4a2afb9ee9d3b1117) b397ad8a Disable method instrumentation for diffs --- changelogs/unreleased/sh-disable-diff-instrumentation.yml | 5 +++++ config/initializers/zz_metrics.rb | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/sh-disable-diff-instrumentation.yml diff --git a/changelogs/unreleased/sh-disable-diff-instrumentation.yml b/changelogs/unreleased/sh-disable-diff-instrumentation.yml new file mode 100644 index 00000000000..55f4c2a8510 --- /dev/null +++ b/changelogs/unreleased/sh-disable-diff-instrumentation.yml @@ -0,0 +1,5 @@ +--- +title: Disable method instrumentation for diffs +merge_request: 27235 +author: +type: performance diff --git a/config/initializers/zz_metrics.rb b/config/initializers/zz_metrics.rb index 151cad3ef9a..5aa6f73c5c5 100644 --- a/config/initializers/zz_metrics.rb +++ b/config/initializers/zz_metrics.rb @@ -30,7 +30,6 @@ def instrument_classes(instrumentation) # are included. %w(app services [^concerns]**) => %w(app services), %w(lib gitlab conflicts) => ['lib'], - %w(lib gitlab diff) => ['lib'], %w(lib gitlab email message) => ['lib'], %w(lib gitlab checks) => ['lib'] } -- cgit v1.2.1 From 05dbf8e048a8e0b98e1dce95e714bd54a3955f0d Mon Sep 17 00:00:00 2001 From: John Jarvis Date: Thu, 11 Apr 2019 12:46:53 +0000 Subject: Merge branch 'restore-hipchat-11-9' into '11-9-stable' [11.9] Revert "Remove HipChat integration from GitLab" See merge request gitlab-org/gitlab-ce!27257 --- .rubocop_todo.yml | 1 + Gemfile | 3 + Gemfile.lock | 4 + app/models/project.rb | 1 + app/models/project_services/hipchat_service.rb | 311 ++++++++++++++++ app/models/service.rb | 1 + changelogs/unreleased/restore-hipchat.yml | 5 + config/initializers/hipchat_client_patch.rb | 15 + .../20190107151029_remove_hipchat_services.rb | 16 - doc/api/services.md | 39 ++ doc/integration/README.md | 4 +- doc/project_services/hipchat.md | 1 + doc/university/glossary/README.md | 2 +- doc/user/index.md | 4 +- doc/user/project/integrations/hipchat.md | 53 +++ doc/user/project/integrations/project_services.md | 1 + lib/api/services.rb | 40 ++ spec/factories/services.rb | 6 + .../projects/services/disable_triggers_spec.rb | 5 +- .../services/user_activates_hipchat_spec.rb | 40 ++ .../projects/services/user_views_services_spec.rb | 3 +- spec/lib/gitlab/import_export/all_models.yml | 1 + spec/lib/gitlab/import_export/project.json | 22 ++ .../project_services/hipchat_service_spec.rb | 410 +++++++++++++++++++++ spec/models/project_spec.rb | 1 + vendor/licenses.csv | 1 + 26 files changed, 963 insertions(+), 27 deletions(-) create mode 100644 app/models/project_services/hipchat_service.rb create mode 100644 changelogs/unreleased/restore-hipchat.yml create mode 100644 config/initializers/hipchat_client_patch.rb delete mode 100644 db/migrate/20190107151029_remove_hipchat_services.rb create mode 100644 doc/project_services/hipchat.md create mode 100644 doc/user/project/integrations/hipchat.md create mode 100644 spec/features/projects/services/user_activates_hipchat_spec.rb create mode 100644 spec/models/project_services/hipchat_service_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 97e39ce99cb..77ad4753c84 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -268,6 +268,7 @@ Rails/Presence: - 'app/models/clusters/platforms/kubernetes.rb' - 'app/models/concerns/mentionable.rb' - 'app/models/concerns/token_authenticatable.rb' + - 'app/models/project_services/hipchat_service.rb' - 'app/models/project_services/irker_service.rb' - 'app/models/project_services/jira_service.rb' - 'app/models/project_services/kubernetes_service.rb' diff --git a/Gemfile b/Gemfile index f36e2e38d6b..da005d40499 100644 --- a/Gemfile +++ b/Gemfile @@ -204,6 +204,9 @@ gem 'connection_pool', '~> 2.0' # Discord integration gem 'discordrb-webhooks-blackst0ne', '~> 3.3', require: false +# HipChat integration +gem 'hipchat', '~> 1.5.0' + # JIRA integration gem 'jira-ruby', '~> 1.4' diff --git a/Gemfile.lock b/Gemfile.lock index 1be6f228954..5de32fba5d9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -364,6 +364,9 @@ GEM hashie (>= 3.0) health_check (2.6.0) rails (>= 4.0) + hipchat (1.5.2) + httparty + mimemagic html-pipeline (2.8.4) activesupport (>= 2) nokogiri (>= 1.4) @@ -1041,6 +1044,7 @@ DEPENDENCIES hangouts-chat (~> 0.0.5) hashie-forbidden_attributes health_check (~> 2.6.0) + hipchat (~> 1.5.0) html-pipeline (~> 2.8) html2text httparty (~> 0.13.3) diff --git a/app/models/project.rb b/app/models/project.rb index 4039af7a330..56bc77a9686 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -147,6 +147,7 @@ class Project < ActiveRecord::Base has_one :pipelines_email_service has_one :irker_service has_one :pivotaltracker_service + has_one :hipchat_service has_one :flowdock_service has_one :assembla_service has_one :asana_service diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb new file mode 100644 index 00000000000..a69b7b4c4b6 --- /dev/null +++ b/app/models/project_services/hipchat_service.rb @@ -0,0 +1,311 @@ +# frozen_string_literal: true + +class HipchatService < Service + include ActionView::Helpers::SanitizeHelper + + MAX_COMMITS = 3 + HIPCHAT_ALLOWED_TAGS = %w[ + a b i strong em br img pre code + table th tr td caption colgroup col thead tbody tfoot + ul ol li dl dt dd + ].freeze + + prop_accessor :token, :room, :server, :color, :api_version + boolean_accessor :notify_only_broken_pipelines, :notify + validates :token, presence: true, if: :activated? + + def initialize_properties + if properties.nil? + self.properties = {} + self.notify_only_broken_pipelines = true + end + end + + def title + 'HipChat' + end + + def description + 'Private group chat and IM' + end + + def self.to_param + 'hipchat' + end + + def fields + [ + { type: 'text', name: 'token', placeholder: 'Room token', required: true }, + { type: 'text', name: 'room', placeholder: 'Room name or ID' }, + { type: 'checkbox', name: 'notify' }, + { type: 'select', name: 'color', choices: %w(yellow red green purple gray random) }, + { type: 'text', name: 'api_version', + placeholder: 'Leave blank for default (v2)' }, + { type: 'text', name: 'server', + placeholder: 'Leave blank for default. https://hipchat.example.com' }, + { type: 'checkbox', name: 'notify_only_broken_pipelines' } + ] + end + + def self.supported_events + %w(push issue confidential_issue merge_request note confidential_note tag_push pipeline) + end + + def execute(data) + return unless supported_events.include?(data[:object_kind]) + + message = create_message(data) + return unless message.present? + + gate[room].send('GitLab', message, message_options(data)) # rubocop:disable GitlabSecurity/PublicSend + end + + def test(data) + begin + result = execute(data) + rescue StandardError => error + return { success: false, result: error } + end + + { success: true, result: result } + end + + private + + def gate + options = { api_version: api_version.present? ? api_version : 'v2' } + options[:server_url] = server unless server.blank? + @gate ||= HipChat::Client.new(token, options) + end + + def message_options(data = nil) + { notify: notify.present? && Gitlab::Utils.to_boolean(notify), color: message_color(data) } + end + + def create_message(data) + object_kind = data[:object_kind] + + case object_kind + when "push", "tag_push" + create_push_message(data) + when "issue" + create_issue_message(data) unless update?(data) + when "merge_request" + create_merge_request_message(data) unless update?(data) + when "note" + create_note_message(data) + when "pipeline" + create_pipeline_message(data) if should_pipeline_be_notified?(data) + end + end + + def render_line(text) + markdown(text.lines.first.chomp, pipeline: :single_line) if text + end + + def create_push_message(push) + ref_type = Gitlab::Git.tag_ref?(push[:ref]) ? 'tag' : 'branch' + ref = Gitlab::Git.ref_name(push[:ref]) + + before = push[:before] + after = push[:after] + + message = [] + message << "#{push[:user_name]} " + + if Gitlab::Git.blank_ref?(before) + message << "pushed new #{ref_type} #{ref}"\ + " to #{project_link}\n" + elsif Gitlab::Git.blank_ref?(after) + message << "removed #{ref_type} #{ref} from #{project_name} \n" + else + message << "pushed to #{ref_type} #{ref} " + message << "of #{project.full_name.gsub!(/\s/, '')} " + message << "(Compare changes)" + + push[:commits].take(MAX_COMMITS).each do |commit| + message << "
- #{render_line(commit[:message])} (#{commit[:id][0..5]})" + end + + if push[:commits].count > MAX_COMMITS + message << "
... #{push[:commits].count - MAX_COMMITS} more commits" + end + end + + message.join + end + + def markdown(text, options = {}) + return "" unless text + + context = { + project: project, + pipeline: :email + } + + Banzai.render(text, context) + + context.merge!(options) + + html = Banzai.render_and_post_process(text, context) + sanitized_html = sanitize(html, tags: HIPCHAT_ALLOWED_TAGS, attributes: %w[href title alt]) + + sanitized_html.truncate(200, separator: ' ', omission: '...') + end + + def create_issue_message(data) + user_name = data[:user][:name] + + obj_attr = data[:object_attributes] + obj_attr = HashWithIndifferentAccess.new(obj_attr) + title = render_line(obj_attr[:title]) + state = obj_attr[:state] + issue_iid = obj_attr[:iid] + issue_url = obj_attr[:url] + description = obj_attr[:description] + + issue_link = "issue ##{issue_iid}" + + message = ["#{user_name} #{state} #{issue_link} in #{project_link}: #{title}"] + message << "
#{markdown(description)}
" + + message.join + end + + def create_merge_request_message(data) + user_name = data[:user][:name] + + obj_attr = data[:object_attributes] + obj_attr = HashWithIndifferentAccess.new(obj_attr) + merge_request_id = obj_attr[:iid] + state = obj_attr[:state] + description = obj_attr[:description] + title = render_line(obj_attr[:title]) + + merge_request_url = "#{project_url}/merge_requests/#{merge_request_id}" + merge_request_link = "merge request !#{merge_request_id}" + message = ["#{user_name} #{state} #{merge_request_link} in " \ + "#{project_link}: #{title}"] + + message << "
#{markdown(description)}
" + message.join + end + + def format_title(title) + "#{render_line(title)}" + end + + def create_note_message(data) + data = HashWithIndifferentAccess.new(data) + user_name = data[:user][:name] + + obj_attr = HashWithIndifferentAccess.new(data[:object_attributes]) + note = obj_attr[:note] + note_url = obj_attr[:url] + noteable_type = obj_attr[:noteable_type] + commit_id = nil + + case noteable_type + when "Commit" + commit_attr = HashWithIndifferentAccess.new(data[:commit]) + commit_id = commit_attr[:id] + subject_desc = commit_id + subject_desc = Commit.truncate_sha(subject_desc) + subject_type = "commit" + title = format_title(commit_attr[:message]) + when "Issue" + subj_attr = HashWithIndifferentAccess.new(data[:issue]) + subject_id = subj_attr[:iid] + subject_desc = "##{subject_id}" + subject_type = "issue" + title = format_title(subj_attr[:title]) + when "MergeRequest" + subj_attr = HashWithIndifferentAccess.new(data[:merge_request]) + subject_id = subj_attr[:iid] + subject_desc = "!#{subject_id}" + subject_type = "merge request" + title = format_title(subj_attr[:title]) + when "Snippet" + subj_attr = HashWithIndifferentAccess.new(data[:snippet]) + subject_id = subj_attr[:id] + subject_desc = "##{subject_id}" + subject_type = "snippet" + title = format_title(subj_attr[:title]) + end + + subject_html = "#{subject_type} #{subject_desc}" + message = ["#{user_name} commented on #{subject_html} in #{project_link}: "] + message << title + + message << "
#{markdown(note, ref: commit_id)}
" + message.join + end + + def create_pipeline_message(data) + pipeline_attributes = data[:object_attributes] + pipeline_id = pipeline_attributes[:id] + ref_type = pipeline_attributes[:tag] ? 'tag' : 'branch' + ref = pipeline_attributes[:ref] + user_name = (data[:user] && data[:user][:name]) || 'API' + status = pipeline_attributes[:status] + duration = pipeline_attributes[:duration] + + branch_link = "#{ref}" + pipeline_url = "##{pipeline_id}" + + "#{project_link}: Pipeline #{pipeline_url} of #{branch_link} #{ref_type} by #{user_name} #{humanized_status(status)} in #{duration} second(s)" + end + + def message_color(data) + pipeline_status_color(data) || color || 'yellow' + end + + def pipeline_status_color(data) + return unless data && data[:object_kind] == 'pipeline' + + case data[:object_attributes][:status] + when 'success' + 'green' + else + 'red' + end + end + + def project_name + project.full_name.gsub(/\s/, '') + end + + def project_url + project.web_url + end + + def project_link + "#{project_name}" + end + + def update?(data) + data[:object_attributes][:action] == 'update' + end + + def humanized_status(status) + case status + when 'success' + 'passed' + else + status + end + end + + def should_pipeline_be_notified?(data) + case data[:object_attributes][:status] + when 'success' + !notify_only_broken_pipelines? + when 'failed' + true + else + false + end + end +end diff --git a/app/models/service.rb b/app/models/service.rb index da523bfa426..cdae99d7727 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -255,6 +255,7 @@ class Service < ActiveRecord::Base external_wiki flowdock hangouts_chat + hipchat irker jira kubernetes diff --git a/changelogs/unreleased/restore-hipchat.yml b/changelogs/unreleased/restore-hipchat.yml new file mode 100644 index 00000000000..a4605a313cc --- /dev/null +++ b/changelogs/unreleased/restore-hipchat.yml @@ -0,0 +1,5 @@ +--- +title: Restore HipChat project service +merge_request: 27172 +author: +type: change diff --git a/config/initializers/hipchat_client_patch.rb b/config/initializers/hipchat_client_patch.rb new file mode 100644 index 00000000000..1879ecb15fb --- /dev/null +++ b/config/initializers/hipchat_client_patch.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true +# This monkey patches the HTTParty used in https://github.com/hipchat/hipchat-rb. +module HipChat + class Client + connection_adapter ::Gitlab::ProxyHTTPConnectionAdapter + end + + class Room + connection_adapter ::Gitlab::ProxyHTTPConnectionAdapter + end + + class User + connection_adapter ::Gitlab::ProxyHTTPConnectionAdapter + end +end diff --git a/db/migrate/20190107151029_remove_hipchat_services.rb b/db/migrate/20190107151029_remove_hipchat_services.rb deleted file mode 100644 index 4741ec88907..00000000000 --- a/db/migrate/20190107151029_remove_hipchat_services.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class RemoveHipchatServices < ActiveRecord::Migration[5.0] - DOWNTIME = false - - def up - execute "DELETE FROM services WHERE type = 'HipchatService'" - end - - def down - # no-op - end -end diff --git a/doc/api/services.md b/doc/api/services.md index dcc168af8d0..f4e7e72e05d 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -449,6 +449,45 @@ Get Hangouts Chat service settings for a project. GET /projects/:id/services/hangouts-chat ``` +## HipChat + +Private group chat and IM + +### Create/Edit HipChat service + +Set HipChat service for a project. + +``` +PUT /projects/:id/services/hipchat +``` + +Parameters: + +| Parameter | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `token` | string | true | Room token | +| `color` | string | false | The room color | +| `notify` | boolean | false | Enable notifications | +| `room` | string | false |Room name or ID | +| `api_version` | string | false | Leave blank for default (v2) | +| `server` | string | false | Leave blank for default. For example, `https://hipchat.example.com`. | + +### Delete HipChat service + +Delete HipChat service for a project. + +``` +DELETE /projects/:id/services/hipchat +``` + +### Get HipChat service settings + +Get HipChat service settings for a project. + +``` +GET /projects/:id/services/hipchat +``` + ## Irker (IRC gateway) Send IRC messages, on update, to a list of recipients through an Irker gateway. diff --git a/doc/integration/README.md b/doc/integration/README.md index f5bc0693b84..a539933f223 100644 --- a/doc/integration/README.md +++ b/doc/integration/README.md @@ -29,8 +29,8 @@ See the documentation below for details on how to configure these services. ## Project services -Integration with services such as Campfire, Flowdock, Pivotal Tracker, and Slack -are available in the form of a [Project Service][]. +Integration with services such as Campfire, Flowdock, HipChat, +Pivotal Tracker, and Slack are available in the form of a [Project Service][]. [Project Service]: ../user/project/integrations/project_services.md diff --git a/doc/project_services/hipchat.md b/doc/project_services/hipchat.md new file mode 100644 index 00000000000..4ae9f6c6b2e --- /dev/null +++ b/doc/project_services/hipchat.md @@ -0,0 +1 @@ +This document was moved to [user/project/integrations/hipchat.md](../user/project/integrations/hipchat.md). diff --git a/doc/university/glossary/README.md b/doc/university/glossary/README.md index 254e234a22c..0af2f8d2f54 100644 --- a/doc/university/glossary/README.md +++ b/doc/university/glossary/README.md @@ -41,7 +41,7 @@ Objects (usually binary and large) created by a build process. These can include ### Atlassian -A [company](https://www.atlassian.com) that develops software products for developers and project managers including Bitbucket, Jira, Confluence, Bamboo. +A [company](https://www.atlassian.com) that develops software products for developers and project managers including Bitbucket, Jira, Hipchat, Confluence, Bamboo. ### Audit Log diff --git a/doc/user/index.md b/doc/user/index.md index b84879601ff..aec314c7784 100644 --- a/doc/user/index.md +++ b/doc/user/index.md @@ -65,9 +65,7 @@ With GitLab Enterprise Edition, you can also: - View the current health and status of each CI environment running on Kubernetes with [Deploy Boards](https://docs.gitlab.com/ee/user/project/deploy_boards.html) - Leverage continuous delivery method with [Canary Deployments](https://docs.gitlab.com/ee/user/project/canary_deployments.html) -You can also [integrate](project/integrations/project_services.md) GitLab with -numerous third-party applications, such as Mattermost, Microsoft Teams, Trello, -Slack, Bamboo CI, JIRA, and a lot more. +You can also [integrate](project/integrations/project_services.md) GitLab with numerous third-party applications, such as Mattermost, Microsoft Teams, HipChat, Trello, Slack, Bamboo CI, JIRA, and a lot more. ## Projects diff --git a/doc/user/project/integrations/hipchat.md b/doc/user/project/integrations/hipchat.md new file mode 100644 index 00000000000..0fd847d415f --- /dev/null +++ b/doc/user/project/integrations/hipchat.md @@ -0,0 +1,53 @@ +# Atlassian HipChat + +GitLab provides a way to send HipChat notifications upon a number of events, +such as when a user pushes code, creates a branch or tag, adds a comment, and +creates a merge request. + +## Setup + +GitLab requires the use of a HipChat v2 API token to work. v1 tokens are +not supported at this time. Note the differences between v1 and v2 tokens: + +HipChat v1 API (legacy) supports "API Auth Tokens" in the Group API menu. A v1 +token is allowed to send messages to *any* room. + +HipChat v2 API has tokens that are can be created using the Integrations tab +in the Group or Room admin page. By design, these are lightweight tokens that +allow GitLab to send messages only to *one* room. + +### Complete these steps in HipChat + +1. Go to: +1. Click on "Group Admin" -> "Integrations". +1. Find "Build Your Own!" and click "Create". +1. Select the desired room, name the integration "GitLab", and click "Create". +1. In the "Send messages to this room by posting this URL" column, you should +see a URL in the format: + +``` +https://api.hipchat.com/v2/room//notification?auth_token= +``` + +HipChat is now ready to accept messages from GitLab. Next, set up the HipChat +service in GitLab. + +### Complete these steps in GitLab + +1. Navigate to the project you want to configure for notifications. +1. Navigate to the [Integrations page](project_services.md#accessing-the-project-services) +1. Click "HipChat". +1. Select the "Active" checkbox. +1. Insert the `token` field from the URL into the `Token` field on the Web page. +1. Insert the `room` field from the URL into the `Room` field on the Web page. +1. Save or optionally click "Test Settings". + +## Troubleshooting + +If you do not see notifications, make sure you are using a HipChat v2 API +token, not a v1 token. + +Note that the v2 token is tied to a specific room. If you want to be able to +specify arbitrary rooms, you can create an API token for a specific user in +HipChat under "Account settings" and "API access". Use the `XXX` value under +`auth_token=XXX`. diff --git a/doc/user/project/integrations/project_services.md b/doc/user/project/integrations/project_services.md index e2f23827360..42c7824a125 100644 --- a/doc/user/project/integrations/project_services.md +++ b/doc/user/project/integrations/project_services.md @@ -36,6 +36,7 @@ Click on the service links to see further configuration instructions and details | External Wiki | Replaces the link to the internal wiki with a link to an external wiki | | Flowdock | Flowdock is a collaboration web app for technical teams | | [Hangouts Chat](hangouts_chat.md) | Receive events notifications in Google Hangouts Chat | +| [HipChat](hipchat.md) | Private group chat and IM | | [Irker (IRC gateway)](irker.md) | Send IRC messages, on update, to a list of recipients through an Irker gateway | | [JIRA](jira.md) | JIRA issue tracker | | JetBrains TeamCity CI | A continuous integration and build server | diff --git a/lib/api/services.rb b/lib/api/services.rb index bda6be51553..f52bd8c2622 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -1,3 +1,4 @@ +# coding: utf-8 # frozen_string_literal: true module API class Services < Grape::API @@ -371,6 +372,44 @@ module API }, CHAT_NOTIFICATION_EVENTS ].flatten, + 'hipchat' => [ + { + required: true, + name: :token, + type: String, + desc: 'The room token' + }, + { + required: false, + name: :room, + type: String, + desc: 'The room name or ID' + }, + { + required: false, + name: :color, + type: String, + desc: 'The room color' + }, + { + required: false, + name: :notify, + type: Boolean, + desc: 'Enable notifications' + }, + { + required: false, + name: :api_version, + type: String, + desc: 'Leave blank for default (v2)' + }, + { + required: false, + name: :server, + type: String, + desc: 'Leave blank for default. https://hipchat.example.com' + } + ], 'irker' => [ { required: true, @@ -674,6 +713,7 @@ module API ExternalWikiService, FlowdockService, HangoutsChatService, + HipchatService, IrkerService, JiraService, KubernetesService, diff --git a/spec/factories/services.rb b/spec/factories/services.rb index 70c34f8640b..0d8c26a2ee9 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -62,4 +62,10 @@ FactoryBot.define do project_key: 'jira-key' ) end + + factory :hipchat_service do + project + type 'HipchatService' + token 'test_token' + end end diff --git a/spec/features/projects/services/disable_triggers_spec.rb b/spec/features/projects/services/disable_triggers_spec.rb index 65b597da269..1a13fe03a67 100644 --- a/spec/features/projects/services/disable_triggers_spec.rb +++ b/spec/features/projects/services/disable_triggers_spec.rb @@ -14,11 +14,10 @@ describe 'Disable individual triggers' do end context 'service has multiple supported events' do - let(:service_name) { 'JIRA' } + let(:service_name) { 'HipChat' } it 'shows trigger checkboxes' do - event_count = JiraService.supported_events.count - expect(event_count).to be > 1 + event_count = HipchatService.supported_events.count expect(page).to have_content "Trigger" expect(page).to have_css(checkbox_selector, count: event_count) diff --git a/spec/features/projects/services/user_activates_hipchat_spec.rb b/spec/features/projects/services/user_activates_hipchat_spec.rb new file mode 100644 index 00000000000..d6b69a5bd68 --- /dev/null +++ b/spec/features/projects/services/user_activates_hipchat_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'User activates HipChat' do + let(:project) { create(:project) } + let(:user) { create(:user) } + + before do + project.add_maintainer(user) + sign_in(user) + + visit(project_settings_integrations_path(project)) + + click_link('HipChat') + end + + context 'with standart settings' do + it 'activates service' do + check('Active') + fill_in('Room', with: 'gitlab') + fill_in('Token', with: 'verySecret') + click_button('Save') + + expect(page).to have_content('HipChat activated.') + end + end + + context 'with custom settings' do + it 'activates service' do + check('Active') + fill_in('Room', with: 'gitlab_custom') + fill_in('Token', with: 'secretCustom') + fill_in('Server', with: 'https://chat.example.com') + click_button('Save') + + expect(page).to have_content('HipChat activated.') + end + end +end diff --git a/spec/features/projects/services/user_views_services_spec.rb b/spec/features/projects/services/user_views_services_spec.rb index b0a838a7d2b..e9c8cf0fe34 100644 --- a/spec/features/projects/services/user_views_services_spec.rb +++ b/spec/features/projects/services/user_views_services_spec.rb @@ -14,6 +14,7 @@ describe 'User views services' do it 'shows the list of available services' do expect(page).to have_content('Project services') expect(page).to have_content('Campfire') + expect(page).to have_content('HipChat') expect(page).to have_content('Assembla') expect(page).to have_content('Pushover') expect(page).to have_content('Atlassian Bamboo') @@ -21,7 +22,5 @@ describe 'User views services' do expect(page).to have_content('Asana') expect(page).to have_content('Irker (IRC gateway)') expect(page).to have_content('Packagist') - expect(page).to have_content('Mattermost') - expect(page).to have_content('Slack') end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 01da3ea7081..017624d7b55 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -220,6 +220,7 @@ project: - packagist_service - pivotaltracker_service - prometheus_service +- hipchat_service - flowdock_service - assembla_service - asana_service diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 773651dd226..4a7accc4c52 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -6794,6 +6794,28 @@ "default": false, "wiki_page_events": true }, + { + "id": 93, + "title": "HipChat", + "project_id": 5, + "created_at": "2016-06-14T15:01:51.219Z", + "updated_at": "2016-06-14T15:01:51.219Z", + "active": false, + "properties": { + "notify_only_broken_pipelines": true + }, + "template": false, + "push_events": true, + "issues_events": true, + "merge_requests_events": true, + "tag_push_events": true, + "note_events": true, + "pipeline_events": true, + "type": "HipchatService", + "category": "common", + "default": false, + "wiki_page_events": true + }, { "id": 91, "title": "Flowdock", diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb new file mode 100644 index 00000000000..b9c6213ed06 --- /dev/null +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -0,0 +1,410 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe HipchatService do + describe "Associations" do + it { is_expected.to belong_to :project } + it { is_expected.to have_one :service_hook } + end + + describe 'Validations' do + context 'when service is active' do + before do + subject.active = true + end + + it { is_expected.to validate_presence_of(:token) } + end + + context 'when service is inactive' do + before do + subject.active = false + end + + it { is_expected.not_to validate_presence_of(:token) } + end + end + + describe "Execute" do + let(:hipchat) { described_class.new } + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + let(:api_url) { 'https://hipchat.example.com/v2/room/123456/notification?auth_token=verySecret' } + let(:project_name) { project.full_name.gsub(/\s/, '') } + let(:token) { 'verySecret' } + let(:server_url) { 'https://hipchat.example.com'} + let(:push_sample_data) do + Gitlab::DataBuilder::Push.build_sample(project, user) + end + + before do + allow(hipchat).to receive_messages( + project_id: project.id, + project: project, + room: 123456, + server: server_url, + token: token + ) + WebMock.stub_request(:post, api_url) + end + + it 'tests and return errors' do + allow(hipchat).to receive(:execute).and_raise(StandardError, 'no such room') + result = hipchat.test(push_sample_data) + + expect(result[:success]).to be_falsey + expect(result[:result].to_s).to eq('no such room') + end + + it 'uses v1 if version is provided' do + allow(hipchat).to receive(:api_version).and_return('v1') + expect(HipChat::Client).to receive(:new).with( + token, + api_version: 'v1', + server_url: server_url + ).and_return(double(:hipchat_service).as_null_object) + hipchat.execute(push_sample_data) + end + + it 'uses v2 as the version when nothing is provided' do + allow(hipchat).to receive(:api_version).and_return('') + expect(HipChat::Client).to receive(:new).with( + token, + api_version: 'v2', + server_url: server_url + ).and_return(double(:hipchat_service).as_null_object) + hipchat.execute(push_sample_data) + end + + context 'push events' do + it "calls Hipchat API for push events" do + hipchat.execute(push_sample_data) + + expect(WebMock).to have_requested(:post, api_url).once + end + + it "creates a push message" do + message = hipchat.send(:create_push_message, push_sample_data) + + push_sample_data[:object_attributes] + branch = push_sample_data[:ref].gsub('refs/heads/', '') + expect(message).to include("#{user.name} pushed to branch " \ + "#{branch} of " \ + "#{project_name}") + end + end + + context 'tag_push events' do + let(:push_sample_data) do + Gitlab::DataBuilder::Push.build( + project, + user, + Gitlab::Git::BLANK_SHA, + '1' * 40, + 'refs/tags/test', + []) + end + + it "calls Hipchat API for tag push events" do + hipchat.execute(push_sample_data) + + expect(WebMock).to have_requested(:post, api_url).once + end + + it "creates a tag push message" do + message = hipchat.send(:create_push_message, push_sample_data) + + push_sample_data[:object_attributes] + expect(message).to eq("#{user.name} pushed new tag " \ + "test to " \ + "#{project_name}\n") + end + end + + context 'issue events' do + let(:issue) { create(:issue, title: 'Awesome issue', description: '**please** fix') } + let(:issue_service) { Issues::CreateService.new(project, user) } + let(:issues_sample_data) { issue_service.hook_data(issue, 'open') } + + it "calls Hipchat API for issue events" do + hipchat.execute(issues_sample_data) + + expect(WebMock).to have_requested(:post, api_url).once + end + + it "creates an issue message" do + message = hipchat.send(:create_issue_message, issues_sample_data) + + obj_attr = issues_sample_data[:object_attributes] + expect(message).to eq("#{user.name} opened " \ + "issue ##{obj_attr["iid"]} in " \ + "#{project_name}: " \ + "Awesome issue" \ + "
please fix
") + end + end + + context 'merge request events' do + let(:merge_request) { create(:merge_request, description: '**please** fix', title: 'Awesome merge request', target_project: project, source_project: project) } + let(:merge_service) { MergeRequests::CreateService.new(project, user) } + let(:merge_sample_data) { merge_service.hook_data(merge_request, 'open') } + + it "calls Hipchat API for merge requests events" do + hipchat.execute(merge_sample_data) + + expect(WebMock).to have_requested(:post, api_url).once + end + + it "creates a merge request message" do + message = hipchat.send(:create_merge_request_message, + merge_sample_data) + + obj_attr = merge_sample_data[:object_attributes] + expect(message).to eq("#{user.name} opened " \ + "merge request !#{obj_attr["iid"]} in " \ + "#{project_name}: " \ + "Awesome merge request" \ + "
please fix
") + end + end + + context "Note events" do + let(:user) { create(:user) } + let(:project) { create(:project, :repository, creator: user) } + + context 'when commit comment event triggered' do + let(:commit_note) do + create(:note_on_commit, author: user, project: project, + commit_id: project.repository.commit.id, + note: 'a comment on a commit') + end + + it "calls Hipchat API for commit comment events" do + data = Gitlab::DataBuilder::Note.build(commit_note, user) + hipchat.execute(data) + + expect(WebMock).to have_requested(:post, api_url).once + + message = hipchat.send(:create_message, data) + + obj_attr = data[:object_attributes] + commit_id = Commit.truncate_sha(data[:commit][:id]) + title = hipchat.send(:format_title, data[:commit][:message]) + + expect(message).to eq("#{user.name} commented on " \ + "commit #{commit_id} in " \ + "#{project_name}: " \ + "#{title}" \ + "
a comment on a commit
") + end + end + + context 'when merge request comment event triggered' do + let(:merge_request) do + create(:merge_request, source_project: project, + target_project: project) + end + + let(:merge_request_note) do + create(:note_on_merge_request, noteable: merge_request, + project: project, + note: "merge request **note**") + end + + it "calls Hipchat API for merge request comment events" do + data = Gitlab::DataBuilder::Note.build(merge_request_note, user) + hipchat.execute(data) + + expect(WebMock).to have_requested(:post, api_url).once + + message = hipchat.send(:create_message, data) + + obj_attr = data[:object_attributes] + merge_id = data[:merge_request]['iid'] + title = data[:merge_request]['title'] + + expect(message).to eq("#{user.name} commented on " \ + "merge request !#{merge_id} in " \ + "#{project_name}: " \ + "#{title}" \ + "
merge request note
") + end + end + + context 'when issue comment event triggered' do + let(:issue) { create(:issue, project: project) } + let(:issue_note) do + create(:note_on_issue, noteable: issue, project: project, + note: "issue **note**") + end + + it "calls Hipchat API for issue comment events" do + data = Gitlab::DataBuilder::Note.build(issue_note, user) + hipchat.execute(data) + + message = hipchat.send(:create_message, data) + + obj_attr = data[:object_attributes] + issue_id = data[:issue]['iid'] + title = data[:issue]['title'] + + expect(message).to eq("#{user.name} commented on " \ + "issue ##{issue_id} in " \ + "#{project_name}: " \ + "#{title}" \ + "
issue note
") + end + + context 'with confidential issue' do + before do + issue.update!(confidential: true) + end + + it 'calls Hipchat API with issue comment' do + data = Gitlab::DataBuilder::Note.build(issue_note, user) + hipchat.execute(data) + + message = hipchat.send(:create_message, data) + + expect(message).to include("
issue note
") + end + end + end + + context 'when snippet comment event triggered' do + let(:snippet) { create(:project_snippet, project: project) } + let(:snippet_note) do + create(:note_on_project_snippet, noteable: snippet, + project: project, + note: "snippet note") + end + + it "calls Hipchat API for snippet comment events" do + data = Gitlab::DataBuilder::Note.build(snippet_note, user) + hipchat.execute(data) + + expect(WebMock).to have_requested(:post, api_url).once + + message = hipchat.send(:create_message, data) + + obj_attr = data[:object_attributes] + snippet_id = data[:snippet]['id'] + title = data[:snippet]['title'] + + expect(message).to eq("#{user.name} commented on " \ + "snippet ##{snippet_id} in " \ + "#{project_name}: " \ + "#{title}" \ + "
snippet note
") + end + end + end + + context 'pipeline events' do + let(:pipeline) { create(:ci_empty_pipeline, user: create(:user)) } + let(:data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } + + context 'for failed' do + before do + pipeline.drop + end + + it "calls Hipchat API" do + hipchat.execute(data) + + expect(WebMock).to have_requested(:post, api_url).once + end + + it "creates a build message" do + message = hipchat.__send__(:create_pipeline_message, data) + + project_url = project.web_url + project_name = project.full_name.gsub(/\s/, '') + pipeline_attributes = data[:object_attributes] + ref = pipeline_attributes[:ref] + ref_type = pipeline_attributes[:tag] ? 'tag' : 'branch' + duration = pipeline_attributes[:duration] + user_name = data[:user][:name] + + expect(message).to eq("#{project_name}: " \ + "Pipeline ##{pipeline.id} " \ + "of #{ref} #{ref_type} " \ + "by #{user_name} failed in #{duration} second(s)") + end + end + + context 'for succeeded' do + before do + pipeline.succeed + end + + it "calls Hipchat API" do + hipchat.notify_only_broken_pipelines = false + hipchat.execute(data) + expect(WebMock).to have_requested(:post, api_url).once + end + + it "notifies only broken" do + hipchat.notify_only_broken_pipelines = true + hipchat.execute(data) + expect(WebMock).not_to have_requested(:post, api_url).once + end + end + end + + context "#message_options" do + it "is set to the defaults" do + expect(hipchat.__send__(:message_options)).to eq({ notify: false, color: 'yellow' }) + end + + it "sets notify to true" do + allow(hipchat).to receive(:notify).and_return('1') + + expect(hipchat.__send__(:message_options)).to eq({ notify: true, color: 'yellow' }) + end + + it "sets the color" do + allow(hipchat).to receive(:color).and_return('red') + + expect(hipchat.__send__(:message_options)).to eq({ notify: false, color: 'red' }) + end + + context 'with a successful build' do + it 'uses the green color' do + data = { object_kind: 'pipeline', + object_attributes: { status: 'success' } } + + expect(hipchat.__send__(:message_options, data)).to eq({ notify: false, color: 'green' }) + end + end + + context 'with a failed build' do + it 'uses the red color' do + data = { object_kind: 'pipeline', + object_attributes: { status: 'failed' } } + + expect(hipchat.__send__(:message_options, data)).to eq({ notify: false, color: 'red' }) + end + end + end + end + + context 'with UrlBlocker' do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + let(:hipchat) { described_class.new(project: project) } + let(:push_sample_data) { Gitlab::DataBuilder::Push.build_sample(project, user) } + + describe '#execute' do + before do + hipchat.server = 'http://localhost:9123' + end + + it 'raises UrlBlocker for localhost' do + expect(Gitlab::UrlBlocker).to receive(:validate!).and_call_original + expect { hipchat.execute(push_sample_data) }.to raise_error(Gitlab::HTTP::BlockedUrlError) + end + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 3beddaeddbd..b09ea108e81 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -41,6 +41,7 @@ describe Project do it { is_expected.to have_one(:pipelines_email_service) } it { is_expected.to have_one(:irker_service) } it { is_expected.to have_one(:pivotaltracker_service) } + it { is_expected.to have_one(:hipchat_service) } it { is_expected.to have_one(:flowdock_service) } it { is_expected.to have_one(:assembla_service) } it { is_expected.to have_one(:slack_slash_commands_service) } diff --git a/vendor/licenses.csv b/vendor/licenses.csv index ed79ec5bac3..d706d76358a 100644 --- a/vendor/licenses.csv +++ b/vendor/licenses.csv @@ -520,6 +520,7 @@ hashie-forbidden_attributes,0.1.1,MIT he,1.1.1,MIT health_check,2.6.0,MIT highlight.js,9.13.1,New BSD +hipchat,1.5.2,MIT hmac-drbg,1.0.1,MIT hoopy,0.1.4,MIT html-pipeline,2.8.4,MIT -- cgit v1.2.1