Merge branch '1790-remove-repository-storage-from-settings-api' into 'master'

Remove repository_storage from V4 "/application/settings" settings API

Closes #1790

See merge request !1307

changelogs/unreleased-ee/1790-remove-repository-storage-from-settings-api.yml

+---
+title: Remove repository_storage from V4 "/application/settings" settings API
+merge_request:
+author:

doc/api/settings.md

@@ -41,7 +41,6 @@ Example response:
    "gravatar_enabled" : true,
    "sign_in_text" : null,
    "container_registry_token_expire_delay": 5,
-   "repository_storage": "default",
    "repository_storages": ["default"],
    "koding_enabled": false,
    "koding_url": null,
@@ -78,7 +77,6 @@ PUT /application/settings
 | `after_sign_out_path` | string | no | Where to redirect users after logout |
 | `container_registry_token_expire_delay` | integer | no | Container Registry token duration in minutes |
 | `repository_storages` | array of strings | no | A list of names of enabled storage paths, taken from `gitlab.yml`. New projects will be created in one of these stores, chosen at random. |
-| `repository_storage` | string | no | The first entry in `repository_storages`. Deprecated, but retained for compatibility reasons |
 | `enabled_git_access_protocol` | string | no | Enabled protocols for Git access. Allowed values are: `ssh`, `http`, and `nil` to allow both protocols. |
 | `koding_enabled` | boolean | no | Enable Koding integration. Default is `false`. |
 | `koding_url` | string | yes (if `koding_enabled` is `true`) |  The Koding instance URL for integration. |
@@ -123,7 +121,7 @@ Example response:
   "user_oauth_applications": true,
   "after_sign_out_path": "",
   "container_registry_token_expire_delay": 5,
-  "repository_storage": "default",
+  "repository_storages": ["default"],
   "koding_enabled": false,
   "koding_url": null,
   "plantuml_enabled": false,

doc/api/v3_to_v4.md

@@ -40,6 +40,10 @@ changes are in V4:
   - POST/PUT/DELETE `:id/repository/files`
 - Renamed `branch_name` to `branch` on DELETE `id/repository/branches/:branch` response [!8936](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8936)
 - Remove `public` param from create and edit actions of projects [!8736](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8736)
-- Remove the ProjectGitHook API. Use the ProjectPushRule API instead [!1301](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1301)
 - Notes do not return deprecated field `upvote` and `downvote` [!9384](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9384)
 - Return HTTP status code `400` for all validation errors when creating or updating a member instead of sometimes `422` error. [!9523](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9523)
+
+#### EE-specific
+
+- Remove the ProjectGitHook API. Use the ProjectPushRule API instead [!1301](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1301)
+- Removed `repository_storage` from `PUT /application/settings` and `GET /application/settings` (use `repository_storages` instead) [!1307](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1307)
\ No newline at end of file

lib/api/api.rb

@@ -21,6 +21,7 @@ module API
       mount ::API::V3::Projects
       mount ::API::V3::ProjectSnippets
       mount ::API::V3::Repositories
+      mount ::API::V3::Settings
       mount ::API::V3::Subscriptions
       mount ::API::V3::SystemHooks
       mount ::API::V3::Tags

lib/api/entities.rb

@@ -616,7 +616,6 @@ module API
       expose :user_oauth_applications
       expose :after_sign_out_path
       expose :container_registry_token_expire_delay
-      expose :repository_storage
       expose :repository_storages
       expose :koding_enabled
       expose :koding_url

lib/api/settings.rb

@@ -118,8 +118,6 @@ module API
         requires :elasticsearch_port, type: Integer, desc: 'The TCP/IP port that Elasticsearch listens to. The default value is 9200'
       end
       optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'
-      # TODO: Remove repository_storage in 9.0
-      optional :repository_storage, type: String, desc: 'The first entry in `repository_storages`. Deprecated, but retained for compatibility reasons'
       optional :repository_storages, type: Array[String], desc: 'A list of names of enabled storage paths, taken from `gitlab.yml`. New projects will be created in one of these stores, chosen at random.'
       optional :repository_size_limit, type: Integer, desc: 'Size limit per repository (MB)'
       at_least_one_of :default_branch_protection, :default_project_visibility, :default_snippet_visibility,
@@ -137,7 +135,7 @@ module API
                       :version_check_enabled, :email_author_in_body, :html_emails_enabled,
                       # GitLab-EE specific settings
                       :help_text, :elasticsearch_indexing, :usage_ping_enabled,
-                      :repository_storage, :repository_storages, :repository_size_limit
+                      :repository_storages, :repository_size_limit
     end
     put "application/settings" do
       if current_settings.update_attributes(declared_params(include_missing: false))

lib/api/v3/entities.rb

@@ -45,6 +45,39 @@ module API
         expose :created_at, :updated_at
         expose :awardable_id, :awardable_type
       end
+
+      class ApplicationSetting < Grape::Entity
+        expose :id
+        expose :default_projects_limit
+        expose :signup_enabled
+        expose :signin_enabled
+        expose :gravatar_enabled
+        expose :sign_in_text
+        expose :after_sign_up_text
+        expose :created_at
+        expose :updated_at
+        expose :home_page_url
+        expose :default_branch_protection
+        expose :restricted_visibility_levels
+        expose :max_attachment_size
+        expose :session_expire_delay
+        expose :default_project_visibility
+        expose :default_snippet_visibility
+        expose :default_group_visibility
+        expose :domain_whitelist
+        expose :domain_blacklist_enabled
+        expose :domain_blacklist
+        expose :user_oauth_applications
+        expose :after_sign_out_path
+        expose :container_registry_token_expire_delay
+        expose :repository_storage
+        expose :repository_storages
+        expose :koding_enabled
+        expose :koding_url
+        expose :plantuml_enabled
+        expose :plantuml_url
+        expose :terminal_max_session_time
+      end
     end
   end
 end

lib/api/v3/settings.rb

+module API
+  module V3
+    class Settings < Grape::API
+      before { authenticated_as_admin! }
+
+      helpers do
+        def current_settings
+          @current_setting ||=
+            (ApplicationSetting.current || ApplicationSetting.create_from_defaults)
+        end
+      end
+
+      desc 'Get the current application settings' do
+        success Entities::ApplicationSetting
+      end
+      get "application/settings" do
+        present current_settings, with: Entities::ApplicationSetting
+      end
+
+      desc 'Modify application settings' do
+        success Entities::ApplicationSetting
+      end
+      params do
+        optional :default_branch_protection, type: Integer, values: [0, 1, 2], desc: 'Determine if developers can push to master'
+        optional :default_project_visibility, type: Integer, values: Gitlab::VisibilityLevel.values, desc: 'The default project visibility'
+        optional :default_snippet_visibility, type: Integer, values: Gitlab::VisibilityLevel.values, desc: 'The default snippet visibility'
+        optional :default_group_visibility, type: Integer, values: Gitlab::VisibilityLevel.values, desc: 'The default group visibility'
+        optional :restricted_visibility_levels, type: Array[String], desc: 'Selected levels cannot be used by non-admin users for projects or snippets. If the public level is restricted, user profiles are only visible to logged in users.'
+        optional :import_sources, type: Array[String], values: %w[github bitbucket gitlab google_code fogbugz git gitlab_project],
+                                  desc: 'Enabled sources for code import during project creation. OmniAuth must be configured for GitHub, Bitbucket, and GitLab.com'
+        optional :disabled_oauth_sign_in_sources, type: Array[String], desc: 'Disable certain OAuth sign-in sources'
+        optional :enabled_git_access_protocol, type: String, values: %w[ssh http nil], desc: 'Allow only the selected protocols to be used for Git access.'
+        optional :gravatar_enabled, type: Boolean, desc: 'Flag indicating if the Gravatar service is enabled'
+        optional :default_projects_limit, type: Integer, desc: 'The maximum number of personal projects'
+        optional :max_attachment_size, type: Integer, desc: 'Maximum attachment size in MB'
+        optional :session_expire_delay, type: Integer, desc: 'Session duration in minutes. GitLab restart is required to apply changes.'
+        optional :user_oauth_applications, type: Boolean, desc: 'Allow users to register any application to use GitLab as an OAuth provider'
+        optional :user_default_external, type: Boolean, desc: 'Newly registered users will by default be external'
+        optional :signup_enabled, type: Boolean, desc: 'Flag indicating if sign up is enabled'
+        optional :send_user_confirmation_email, type: Boolean, desc: 'Send confirmation email on sign-up'
+        optional :domain_whitelist, type: String, desc: 'ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com'
+        optional :domain_blacklist_enabled, type: Boolean, desc: 'Enable domain blacklist for sign ups'
+        given domain_blacklist_enabled: ->(val) { val } do
+          requires :domain_blacklist, type: String, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com'
+        end
+        optional :after_sign_up_text, type: String, desc: 'Text shown after sign up'
+        optional :signin_enabled, type: Boolean, desc: 'Flag indicating if sign in is enabled'
+        optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication'
+        given require_two_factor_authentication: ->(val) { val } do
+          requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication'
+        end
+        optional :home_page_url, type: String, desc: 'We will redirect non-logged in users to this page'
+        optional :after_sign_out_path, type: String, desc: 'We will redirect users to this page after they sign out'
+        optional :sign_in_text, type: String, desc: 'The sign in text of the GitLab application'
+        optional :help_page_text, type: String, desc: 'Custom text displayed on the help page'
+        optional :shared_runners_enabled, type: Boolean, desc: 'Enable shared runners for new projects'
+        given shared_runners_enabled: ->(val) { val } do
+          requires :shared_runners_text, type: String, desc: 'Shared runners text '
+        end
+        optional :max_artifacts_size, type: Integer, desc: "Set the maximum file size each build's artifacts can have"
+        optional :max_pages_size, type: Integer, desc: 'Maximum size of pages in MB'
+        optional :container_registry_token_expire_delay, type: Integer, desc: 'Authorization token duration (minutes)'
+        optional :metrics_enabled, type: Boolean, desc: 'Enable the InfluxDB metrics'
+        given metrics_enabled: ->(val) { val } do
+          requires :metrics_host, type: String, desc: 'The InfluxDB host'
+          requires :metrics_port, type: Integer, desc: 'The UDP port to use for connecting to InfluxDB'
+          requires :metrics_pool_size, type: Integer, desc: 'The amount of InfluxDB connections to open'
+          requires :metrics_timeout, type: Integer, desc: 'The amount of seconds after which an InfluxDB connection will time out'
+          requires :metrics_method_call_threshold, type: Integer, desc: 'A method call is only tracked when it takes longer to complete than the given amount of milliseconds.'
+          requires :metrics_sample_interval, type: Integer, desc: 'The sampling interval in seconds'
+          requires :metrics_packet_size, type: Integer, desc: 'The amount of points to store in a single UDP packet'
+        end
+        optional :sidekiq_throttling_enabled, type: Boolean, desc: 'Enable Sidekiq Job Throttling'
+        given sidekiq_throttling_enabled: ->(val) { val } do
+          requires :sidekiq_throttling_queus, type: Array[String], desc: 'Choose which queues you wish to throttle'
+          requires :sidekiq_throttling_factor, type: Float, desc: 'The factor by which the queues should be throttled. A value between 0.0 and 1.0, exclusive.'
+        end
+        optional :recaptcha_enabled, type: Boolean, desc: 'Helps prevent bots from creating accounts'
+        given recaptcha_enabled: ->(val) { val } do
+          requires :recaptcha_site_key, type: String, desc: 'Generate site key at http://www.google.com/recaptcha'
+          requires :recaptcha_private_key, type: String, desc: 'Generate private key at http://www.google.com/recaptcha'
+        end
+        optional :akismet_enabled, type: Boolean, desc: 'Helps prevent bots from creating issues'
+        given akismet_enabled: ->(val) { val } do
+          requires :akismet_api_key, type: String, desc: 'Generate API key at http://www.akismet.com'
+        end
+        optional :admin_notification_email, type: String, desc: 'Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.'
+        optional :sentry_enabled, type: Boolean, desc: 'Sentry is an error reporting and logging tool which is currently not shipped with GitLab, get it here: https://getsentry.com'
+        given sentry_enabled: ->(val) { val } do
+          requires :sentry_dsn, type: String, desc: 'Sentry Data Source Name'
+        end
+        optional :repository_storage, type: String, desc: 'Storage paths for new projects'
+        optional :repository_checks_enabled, type: Boolean, desc: "GitLab will periodically run 'git fsck' in all project and wiki repositories to look for silent disk corruption issues."
+        optional :koding_enabled, type: Boolean, desc: 'Enable Koding'
+        given koding_enabled: ->(val) { val } do
+          requires :koding_url, type: String, desc: 'The Koding team URL'
+        end
+        optional :plantuml_enabled, type: Boolean, desc: 'Enable PlantUML'
+        given plantuml_enabled: ->(val) { val } do
+          requires :plantuml_url, type: String, desc: 'The PlantUML server URL'
+        end
+        optional :version_check_enabled, type: Boolean, desc: 'Let GitLab inform you when an update is available.'
+        optional :email_author_in_body, type: Boolean, desc: 'Some email servers do not support overriding the email sender name. Enable this option to include the name of the author of the issue, merge request or comment in the email body instead.'
+        optional :html_emails_enabled, type: Boolean, desc: 'By default GitLab sends emails in HTML and plain text formats so mail clients can choose what format to use. Disable this option if you only want to send emails in plain text format.'
+        optional :housekeeping_enabled, type: Boolean, desc: 'Enable automatic repository housekeeping (git repack, git gc)'
+        given housekeeping_enabled: ->(val) { val } do
+          requires :housekeeping_bitmaps_enabled, type: Boolean, desc: "Creating pack file bitmaps makes housekeeping take a little longer but bitmaps should accelerate 'git clone' performance."
+          requires :housekeeping_incremental_repack_period, type: Integer, desc: "Number of Git pushes after which an incremental 'git repack' is run."
+          requires :housekeeping_full_repack_period, type: Integer, desc: "Number of Git pushes after which a full 'git repack' is run."
+          requires :housekeeping_gc_period, type: Integer, desc: "Number of Git pushes after which 'git gc' is run."
+        end
+        optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.'
+        # GitLab-EE specific settings
+        optional :help_text, type: String, desc: 'GitLab server administrator information'
+        optional :elasticsearch_indexing, type: Boolean, desc: 'Enable Elasticsearch indexing'
+        given elasticsearch_indexing: ->(val) { val } do
+          optional :elasticsearch_search, type: Boolean, desc: 'Enable Elasticsearch search'
+          requires :elasticsearch_host, type: String, desc: 'The TCP/IP host to use for connecting to Elasticsearch. Use a comma-separated list to support clustering (e.g., "host1, host2")'
+          requires :elasticsearch_port, type: Integer, desc: 'The TCP/IP port that Elasticsearch listens to. The default value is 9200'
+        end
+        optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'
+        optional :repository_storage, type: String, desc: 'The first entry in `repository_storages`. Deprecated, but retained for compatibility reasons'
+        optional :repository_storages, type: Array[String], desc: 'A list of names of enabled storage paths, taken from `gitlab.yml`. New projects will be created in one of these stores, chosen at random.'
+        optional :repository_size_limit, type: Integer, desc: 'Size limit per repository (MB)'
+        at_least_one_of :default_branch_protection, :default_project_visibility, :default_snippet_visibility,
+                        :default_group_visibility, :restricted_visibility_levels, :import_sources,
+                        :enabled_git_access_protocol, :gravatar_enabled, :default_projects_limit,
+                        :max_attachment_size, :session_expire_delay, :disabled_oauth_sign_in_sources,
+                        :user_oauth_applications, :user_default_external, :signup_enabled,
+                        :send_user_confirmation_email, :domain_whitelist, :domain_blacklist_enabled,
+                        :after_sign_up_text, :signin_enabled, :require_two_factor_authentication,
+                        :home_page_url, :after_sign_out_path, :sign_in_text, :help_page_text,
+                        :shared_runners_enabled, :max_artifacts_size, :max_pages_size, :container_registry_token_expire_delay,
+                        :metrics_enabled, :sidekiq_throttling_enabled, :recaptcha_enabled,
+                        :akismet_enabled, :admin_notification_email, :sentry_enabled,
+                        :repository_checks_enabled, :koding_enabled, :housekeeping_enabled, :terminal_max_session_time, :plantuml_enabled,
+                        :version_check_enabled, :email_author_in_body, :html_emails_enabled,
+                        # GitLab-EE specific settings
+                        :help_text, :elasticsearch_indexing, :usage_ping_enabled,
+                        :repository_storage, :repository_storages, :repository_size_limit
+      end
+      put "application/settings" do
+        if current_settings.update_attributes(declared_params(include_missing: false))
+          present current_settings, with: Entities::ApplicationSetting
+        else
+          render_validation_error!(current_settings)
+        end
+      end
+    end
+  end
+end

spec/requests/api/settings_spec.rb

@@ -9,11 +9,12 @@ describe API::Settings, 'Settings', api: true  do
   describe "GET /application/settings" do
     it "returns application settings" do
       get api("/application/settings", admin)
+
       expect(response).to have_http_status(200)
       expect(json_response).to be_an Hash
       expect(json_response['default_projects_limit']).to eq(42)
       expect(json_response['signin_enabled']).to be_truthy
-      expect(json_response['repository_storage']).to eq('default')
+      expect(json_response['repository_storages']).to eq(['default'])
       expect(json_response['koding_enabled']).to be_falsey
       expect(json_response['koding_url']).to be_nil
       expect(json_response['plantuml_enabled']).to be_falsey
@@ -30,12 +31,12 @@ describe API::Settings, 'Settings', api: true  do
 
       it "updates application settings" do
         put api("/application/settings", admin),
-          default_projects_limit: 3, signin_enabled: false, repository_storage: 'custom', koding_enabled: true, koding_url: 'http://koding.example.com',
+          default_projects_limit: 3, signin_enabled: false, repository_storages: ['custom'], koding_enabled: true, koding_url: 'http://koding.example.com',
           plantuml_enabled: true, plantuml_url: 'http://plantuml.example.com'
+
         expect(response).to have_http_status(200)
         expect(json_response['default_projects_limit']).to eq(3)
         expect(json_response['signin_enabled']).to be_falsey
-        expect(json_response['repository_storage']).to eq('custom')
         expect(json_response['repository_storages']).to eq(['custom'])
         expect(json_response['koding_enabled']).to be_truthy
         expect(json_response['koding_url']).to eq('http://koding.example.com')

spec/requests/api/v3/settings_spec.rb

+require 'spec_helper'
+
+describe API::V3::Settings, 'Settings', api: true  do
+  include ApiHelpers
+
+  let(:user) { create(:user) }
+  let(:admin) { create(:admin) }
+
+  describe "GET /application/settings" do
+    it "returns application settings" do
+      get v3_api("/application/settings", admin)
+
+      expect(response).to have_http_status(200)
+      expect(json_response).to be_an Hash
+      expect(json_response['default_projects_limit']).to eq(42)
+      expect(json_response['signin_enabled']).to be_truthy
+      expect(json_response['repository_storage']).to eq('default')
+      expect(json_response['koding_enabled']).to be_falsey
+      expect(json_response['koding_url']).to be_nil
+      expect(json_response['plantuml_enabled']).to be_falsey
+      expect(json_response['plantuml_url']).to be_nil
+    end
+  end
+
+  describe "PUT /application/settings" do
+    context "custom repository storage type set in the config" do
+      before do
+        storages = { 'custom' => 'tmp/tests/custom_repositories' }
+        allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
+      end
+
+      it "updates application settings" do
+        put v3_api("/application/settings", admin),
+          default_projects_limit: 3, signin_enabled: false, repository_storage: 'custom', koding_enabled: true, koding_url: 'http://koding.example.com',
+          plantuml_enabled: true, plantuml_url: 'http://plantuml.example.com'
+
+        expect(response).to have_http_status(200)
+        expect(json_response['default_projects_limit']).to eq(3)
+        expect(json_response['signin_enabled']).to be_falsey
+        expect(json_response['repository_storage']).to eq('custom')
+        expect(json_response['repository_storages']).to eq(['custom'])
+        expect(json_response['koding_enabled']).to be_truthy
+        expect(json_response['koding_url']).to eq('http://koding.example.com')
+        expect(json_response['plantuml_enabled']).to be_truthy
+        expect(json_response['plantuml_url']).to eq('http://plantuml.example.com')
+      end
+    end
+
+    context "missing koding_url value when koding_enabled is true" do
+      it "returns a blank parameter error message" do
+        put v3_api("/application/settings", admin), koding_enabled: true
+
+        expect(response).to have_http_status(400)
+        expect(json_response['error']).to eq('koding_url is missing')
+      end
+    end
+
+    context "missing plantuml_url value when plantuml_enabled is true" do
+      it "returns a blank parameter error message" do
+        put v3_api("/application/settings", admin), plantuml_enabled: true
+
+        expect(response).to have_http_status(400)
+        expect(json_response['error']).to eq('plantuml_url is missing')
+      end
+    end
+  end
+end