Skip to content

Enable advanced search by default for global search scope

Dylan Griffith requested to merge bring-back-advanced-global-search-default into master

What does this MR do?

NOTE: When reviewing the specs for this change you may wish to disable "Show whitespace changes" as many existing tests needed to be nested under a feature flag context to keep the same behaviour.

This reverts !39063 (merged) adding 2 extra feature flags to make it safer. When reverting the MR the only change I needed was to fix a conflict in ee/spec/requests/api/search_spec.rb. It may be easiest to review this MR commit by commit as the first commit is just the revert and the other 2 are introducing the feature flags.

Feature flags:

  • advanced_global_search_for_limited_indexing: just puts this original MR behaviour behind a feature flag
  • block_anonymous_global_searches: will allow us to disable anonymous global searches which will help with managing the impact of defaulting global searches to using Elasticsearch. The assumption being that unauthenticated users make up considerable global search traffic and blocking that would be sufficient along with blocking any obviously abusive user authenticated global searches. Searching via the API is already blocked for unauthenticed users so it was only necessary to add this check to the SearchController.

The feature flag rollout will be managed in #244276 but it's not got all the detail in it yet.

I've also opted to remove the docs changes from this MR and only update the docs when we remove the feature flag since it's simply too confusing. The relevant docs changes are primarily focused on self-managed customers which won't see the change until after we remove the feature flag anyway so it seems odd to document this before doing that. We could document the behaviour with the feature flag both on and off but I think this isn't that helpful as we expect we'll just remove these feature flags quite quickly once the feature is seen to be stable on GitLab.com .

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Sean McGivern

Merge request reports