diff options
Diffstat (limited to 'doc/development')
35 files changed, 353 insertions, 60 deletions
diff --git a/doc/development/README.md b/doc/development/README.md index d5829e31343..13646cbfe48 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -49,6 +49,7 @@ description: 'Learn how to contribute to GitLab.' - [Working with the GitHub importer](github_importer.md) - [Import/Export development documentation](import_export.md) - [Working with Merge Request diffs](diffs.md) +- [Kubernetes integration guidelines](kubernetes.md) - [Permissions](permissions.md) - [Prometheus metrics](prometheus_metrics.md) - [Guidelines for reusing abstractions](reusing_abstractions.md) diff --git a/doc/development/architecture.md b/doc/development/architecture.md index e65c5f05505..63574b28edc 100644 --- a/doc/development/architecture.md +++ b/doc/development/architecture.md @@ -35,7 +35,7 @@ run: redis: (pid 30560) 14274s; run: log: (pid 13807) 2432047s run: redis-exporter: (pid 30946) 14205s; run: log: (pid 13869) 2432045s run: sidekiq: (pid 30953) 14205s; run: log: (pid 13810) 2432047s run: unicorn: (pid 30960) 14204s; run: log: (pid 13809) 2432047s -``` +``` ### Layers @@ -51,11 +51,11 @@ GitLab can be considered to have two layers from a process perspective: - Omnibus configuration options - Layer: Monitoring -[Alert manager](https://prometheus.io/docs/alerting/alertmanager/) is a tool provided by prometheus that _"handles alerts sent by client applications such as the Prometheus server. It takes care of deduplicating, grouping, and routing them to the correct receiver integration such as email, PagerDuty, or OpsGenie. It also takes care of silencing and inhibition of alerts."_ You can read more in [issue gitlab-ce#45740](https://gitlab.com/gitlab-org/gitlab-ce/issues/45740) about what we will be alerting on. +[Alert manager](https://prometheus.io/docs/alerting/alertmanager/) is a tool provided by prometheus that _"handles alerts sent by client applications such as the Prometheus server. It takes care of deduplicating, grouping, and routing them to the correct receiver integration such as email, PagerDuty, or OpsGenie. It also takes care of silencing and inhibition of alerts."_ You can read more in [issue gitlab-ce#45740](https://gitlab.com/gitlab-org/gitlab-ce/issues/45740) about what we will be alerting on. ### gitaly -- [Omnibus configuration options](https://gitlab.com/gitlab-org/gitaly/tree/master/doc/configuration) +- [Omnibus configuration options](https://gitlab.com/gitlab-org/gitaly/tree/master/doc/configuration) - Layer: Core Service (Data) Gitaly is a service designed by GitLab to remove our need for NFS for Git storage in distributed deployments of GitLab. (Think GitLab.com or High Availablity Deployments) As of 11.3.0, This service handles all Git level access in GitLab. You can read more about the project [in the project's readme](https://gitlab.com/gitlab-org/gitaly). @@ -63,7 +63,7 @@ Gitaly is a service designed by GitLab to remove our need for NFS for Git storag ### gitlab-monitor - Omnibus configuration options -- Layer: Monitoring +- Layer: Monitoring GitLab Monitor is a process disigned in house that allows us to export metrics about GitLab application internals to prometheus. You can read more [in the project's readme](https://gitlab.com/gitlab-org/gitlab-monitor) @@ -100,14 +100,14 @@ Nginx as an ingress port for all HTTP requests and routes them to the approriate - [Omnibus configuration options](https://docs.gitlab.com/ee/administration/monitoring/prometheus/postgres_exporter.html) - Layer: Monitoring -[Postgres-exporter](https://github.com/wrouesnel/postgres_exporter) is the community provided Prometheus exporter that will deliver data about Postgres to prometheus for use in Grafana Dashboards. +[Postgres-exporter](https://github.com/wrouesnel/postgres_exporter) is the community provided Prometheus exporter that will deliver data about Postgres to prometheus for use in Grafana Dashboards. ### postgresql - [Omnibus configuration options](https://docs.gitlab.com/omnibus/settings/database.html) - Layer: Core Service (Data) -GitLab packages the popular Database to provide storage for Application meta data and user information. +GitLab packages the popular Database to provide storage for Application meta data and user information. ### prometheus @@ -121,11 +121,11 @@ Prometheus is a time-series tool that helps GitLab administrators expose metrics - [Omnibus configuration options](https://docs.gitlab.com/omnibus/settings/redis.html) - Layer: Core Service (Data) -Redis is packaged to provide a place to store: +Redis is packaged to provide a place to store: - session data - temporary cache information -- background job queues. +- background job queues. ### redis-exporter @@ -146,7 +146,7 @@ Sidekiq is a Ruby background job processor that pulls jobs from the redis queue - [Omnibus configuration options](https://docs.gitlab.com/omnibus/settings/unicorn.html) - Layer: Core Service (Processor) -[Unicorn](https://bogomips.org/unicorn/) is a Ruby application server that is used to run the core Rails Application that provides the user facing features in GitLab. Often process output you will see this as `bundle` or `config.ru` depending on the GitLab version. +[Unicorn](https://bogomips.org/unicorn/) is a Ruby application server that is used to run the core Rails Application that provides the user facing features in GitLab. Often process output you will see this as `bundle` or `config.ru` depending on the GitLab version. ### Additional Processes @@ -176,7 +176,6 @@ When making a request to an HTTP Endpoint (Think `/users/sign_in`) the request w - unicorn - Since this is a web request, and it needs to access the application it will go to Unicorn. - Postgres/Gitaly/Redis - Depending on the type of request, it may hit these services to store or retreive data. - ### GitLab Git Request Cycle Below we describe the different pathing that HTTP vs. SSH Git requests will take. There is some overlap with the Web Request Cycle but also some differences. diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 25ea2211b64..1b591c7c322 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -23,6 +23,11 @@ one of the [Merge request coaches][team]. If you need assistance with security scans or comments, feel free to include the Security Team (`@gitlab-com/gl-security`) in the review. +The `danger-review` CI job will randomly pick a reviewer and a maintainer for +each area of the codebase that your merge request seems to touch. It only makes +recommendations - feel free to override it if you think someone else is a better +fit! + Depending on the areas your merge request touches, it must be **approved** by one or more [maintainers](https://about.gitlab.com/handbook/engineering/workflow/code-review/#maintainer): @@ -132,6 +137,14 @@ If a developer who happens to also be a maintainer was involved in a merge reque as a domain expert and/or reviewer, it is recommended that they are not also picked as the maintainer to ultimately approve and merge it. +Try to review in a timely manner; doing so allows everyone involved in the merge +request to iterate faster as the context is fresh in memory. Further, this +improves contributors' experiences significantly. Reviewers should aim to review +within two working days from the date they were assigned the merge request. If +you don't think you'll be able to review a merge request within that time, let +the author know as soon as possible. When the author of the merge request has not +heard anything after two days, a new reviewer should be assigned. + Maintainers should check before merging if the merge request is approved by the required approvers. diff --git a/doc/development/contributing/merge_request_workflow.md b/doc/development/contributing/merge_request_workflow.md index 19b6181c9a2..4e766f37871 100644 --- a/doc/development/contributing/merge_request_workflow.md +++ b/doc/development/contributing/merge_request_workflow.md @@ -89,6 +89,11 @@ request is as follows: 1. If your merge request introduces changes that require additional steps when installing GitLab from source, add them to `doc/install/installation.md` in the same merge request. +1. If your merge request introduces changes that require additional steps when + upgrading GitLab from source, add them to + `doc/update/upgrading_from_source.md` in the same merge request. If these + instructions are specific to a version, add them to the "Version specific + upgrading instructions" section. Please keep the change in a single MR **as small as possible**. If you want to contribute a large feature think very hard what the minimum viable change is. diff --git a/doc/development/database_debugging.md b/doc/development/database_debugging.md index b2c804b2ff0..f00c5ccb9e9 100644 --- a/doc/development/database_debugging.md +++ b/doc/development/database_debugging.md @@ -13,7 +13,6 @@ Available `RAILS_ENV` - `development` (this is your main GDK db) - `test` (used for tests like rspec) - ## Nuke everything and start over If you just want to delete everything and start over with an empty DB (~1 minute): @@ -36,7 +35,6 @@ If your test DB is giving you problems, it is safe to nuke it because it doesn't - `bundle exec rake db:migrate:up VERSION=20170926203418 RAILS_ENV=development`: Set up a migration - `bundle exec rake db:migrate:redo VERSION=20170926203418 RAILS_ENV=development`: Re-run a specific migration - ## Manually access the database Access the database via one of these commands (they all get you to the same place) @@ -54,7 +52,6 @@ bundle exec rails db RAILS_ENV=development - `SELECT * FROM schema_migrations WHERE version = '20170926203418';`: Check if a migration was run - `DELETE FROM schema_migrations WHERE version = '20170926203418';`: Manually remove a migration - ## FAQ ### `ActiveRecord::PendingMigrationError` with Spring diff --git a/doc/development/diffs.md b/doc/development/diffs.md index 43fc125c21d..56e869c21f8 100644 --- a/doc/development/diffs.md +++ b/doc/development/diffs.md @@ -59,28 +59,24 @@ Gitlab::Git::DiffCollection.collection_limits[:safe_max_files] = Gitlab::Git::Di File diffs will be collapsed (but be expandable) if 100 files have already been rendered. - ```ruby Gitlab::Git::DiffCollection.collection_limits[:safe_max_lines] = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000 ``` File diffs will be collapsed (but be expandable) if 5000 lines have already been rendered. - ```ruby Gitlab::Git::DiffCollection.collection_limits[:safe_max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:safe_max_files] * 5.kilobytes = 500.kilobytes ``` File diffs will be collapsed (but be expandable) if 500 kilobytes have already been rendered. - ```ruby Gitlab::Git::DiffCollection.collection_limits[:max_files] = Commit::DIFF_HARD_LIMIT_FILES = 1000 ``` No more files will be rendered at all if 1000 files have already been rendered. - ```ruby Gitlab::Git::DiffCollection.collection_limits[:max_lines] = Commit::DIFF_HARD_LIMIT_LINES = 50000 ``` @@ -129,4 +125,3 @@ Diff Viewers, which can be found on `models/diff_viewer/*` are classes used to m whether it's a binary, which partial should be used to render it or which File extensions this class accounts for. `DiffViewer::Base` validates _blobs_ (old and new versions) content, extension and file type in order to check if it can be rendered. - diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index aac98c6ee7f..287a472d0d8 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -12,7 +12,7 @@ In addition to this page, the following resources to help craft and contribute d - [Structure and template](structure.md) - Learn the typical parts of a doc page and how to write each one. - [Workflow](workflow.md) - A landing page for our key workflows: - [Feature-change documentation workflow](feature-change-workflow.md) - Adding required documentation when developing a GitLab feature. - - [Documentation improvement worflow](improvement-workflow.md) - New content not associated with a new feature. + - [Documentation improvement workflow](improvement-workflow.md) - New content not associated with a new feature. - [Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/) - A reference for the markdown implementation used by GitLab's documentation site and about.gitlab.com. - [Site architecture](site_architecture/index.md) - How docs.gitlab.com is built. diff --git a/doc/development/documentation/site_architecture/global_nav.md b/doc/development/documentation/site_architecture/global_nav.md index 62ca7d6c805..0aa3c41a225 100644 --- a/doc/development/documentation/site_architecture/global_nav.md +++ b/doc/development/documentation/site_architecture/global_nav.md @@ -234,7 +234,7 @@ Examples: ```yaml - category_title: Issues category_url: 'user/project/issues/' - # note that the above URL does not start with a slash and + # note that the above URL does not start with a slash and # does not include index.html at the end docs: @@ -295,7 +295,6 @@ On the other hand, if the user is looking at `/ce/` docs, all the links in the CE nav should link internally to `/ce/` files, except for [`ee-only` docs](#ee-only-docs). - ```html <% if dir != 'ce' %> <a href="/ee/<%= sec[:section_url] %>">...</a> diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index cda66447c2c..7a3a8f25c2d 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -112,7 +112,7 @@ table_display_block: true ## Emphasis - Use double asterisks (`**`) to mark a word or text in bold (`**bold**`). -- Use undescore (`_`) for text in italics (`_italic_`). +- Use underscore (`_`) for text in italics (`_italic_`). - Use greater than (`>`) for blockquotes. ## Punctuation diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index e0985922443..3e85c0e1995 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -831,6 +831,29 @@ should remain working as-is when EE is running without a license. Instead place EE specs in the `ee/spec` folder. +### Code in `spec/factories` + +Use `FactoryBot.modify` to extend factories already defined in CE. + +Note that you cannot define new factories (even nested ones) inside the `FactoryBot.modify` block. You can do so in a +separate `FactoryBot.define` block as shown in the example below: + +```ruby +# ee/spec/factories/notes.rb +FactoryBot.modify do + factory :note do + trait :on_epic do + noteable { create(:epic) } + project nil + end + end +end + +FactoryBot.define do + factory :note_on_epic, parent: :note, traits: [:on_epic] +end +``` + ## JavaScript code in `assets/javascripts/` To separate EE-specific JS-files we should also move the files into an `ee` folder. diff --git a/doc/development/fe_guide/accessibility.md b/doc/development/fe_guide/accessibility.md index 366b220cbb2..df32242a522 100644 --- a/doc/development/fe_guide/accessibility.md +++ b/doc/development/fe_guide/accessibility.md @@ -8,6 +8,5 @@ are useful for testing for potential accessibility problems in GitLab. Accessibility best-practices and more in-depth information is available on [the Audit Rules page][audit-rules] for the Chrome Accessibility Developer Tools. - [chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools [audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules diff --git a/doc/development/fe_guide/design_patterns.md b/doc/development/fe_guide/design_patterns.md index e05887a19af..0342d16a87c 100644 --- a/doc/development/fe_guide/design_patterns.md +++ b/doc/development/fe_guide/design_patterns.md @@ -74,5 +74,4 @@ new Foo({ container: '.my-element' }); ``` You can find an example of the above in this [class][container-class-example]; - [container-class-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/mini_pipeline_graph_dropdown.js diff --git a/doc/development/fe_guide/droplab/droplab.md b/doc/development/fe_guide/droplab/droplab.md index e6aa0be671f..2f8c79abde1 100644 --- a/doc/development/fe_guide/droplab/droplab.md +++ b/doc/development/fe_guide/droplab/droplab.md @@ -90,7 +90,6 @@ const list = document.getElementById('list'); droplab.addHook(trigger, list); ``` - ### Dynamic data Adding `data-dynamic` to your dropdown element will enable dynamic list rendering. diff --git a/doc/development/fe_guide/droplab/plugins/input_setter.md b/doc/development/fe_guide/droplab/plugins/input_setter.md index 8e28a41f32e..e229103e462 100644 --- a/doc/development/fe_guide/droplab/plugins/input_setter.md +++ b/doc/development/fe_guide/droplab/plugins/input_setter.md @@ -13,7 +13,6 @@ to update the `input` element with. You can also set the `InputSetter` config to an array of objects, which will allow you to update multiple elements. - ```html <input id="input" value=""> <div id="div" data-selected-id=""></div> diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 3a3cb77f592..86b8972a69e 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -1,7 +1,7 @@ # Frontend Development Guidelines > **Notice:** -We are currently in the process of re-writing our development guide to make it easier to find information. The new guide is still WIP but viewable in [development/new_fe_guide](../new_fe_guide/index.md) +> We are currently in the process of re-writing our development guide to make it easier to find information. The new guide is still WIP but viewable in [development/new_fe_guide](../new_fe_guide/index.md) This document describes various guidelines to ensure consistency and quality across GitLab's frontend team. @@ -32,32 +32,41 @@ For our currently-supported browsers, see our [requirements][requirements]. --- ## [Development Process](development_process.md) + How we plan and execute the work on the frontend. ## [Architecture](architecture.md) + How we go about making fundamental design decisions in GitLab's frontend team or make changes to our frontend development guidelines. ## [Testing](../testing_guide/frontend_testing.md) + How we write frontend tests, run the GitLab test suite, and debug test related issues. ## [Design Patterns](design_patterns.md) + Common JavaScript design patterns in GitLab's codebase. ## [Vue.js Best Practices](vue.md) + Vue specific design patterns and practices. ## [Vuex](vuex.md) + Vuex specific design patterns and practices. ## [Axios](axios.md) + Axios specific practices and gotchas. ## [GraphQL](graphql.md) + How to use GraphQL ## [Icons and Illustrations](icons.md) + How we use SVG for our Icons and Illustrations. ## [Components](components.md) @@ -70,7 +79,7 @@ How we use UI components. ### [JavaScript Style Guide](style_guide_js.md) -We use eslint to enforce our JavaScript style guides. Our guide is based on +We use eslint to enforce our JavaScript style guides. Our guide is based on the excellent [Airbnb][airbnb-js-style-guide] style guide with a few small changes. @@ -81,23 +90,26 @@ Our SCSS conventions which are enforced through [scss-lint][scss-lint]. --- ## [Performance](performance.md) + Best practices for monitoring and maximizing frontend performance. --- ## [Security](security.md) + Frontend security practices. --- ## [Accessibility](accessibility.md) + Our accessibility standards and resources. ## [Internationalization (i18n) and Translations](../i18n/externalization.md) + Frontend internationalization support is described in [this document](../i18n/). The [externalization part of the guide](../i18n/externalization.md) explains the helpers/methods available. - [rails]: http://rubyonrails.org/ [haml]: http://haml.info/ [hamlit]: https://github.com/k0kubun/hamlit @@ -116,6 +128,7 @@ The [externalization part of the guide](../i18n/externalization.md) explains the --- ## [DropLab](droplab/droplab.md) + Our internal `DropLab` dropdown library. - [DropLab](droplab/droplab.md) diff --git a/doc/development/fe_guide/performance.md b/doc/development/fe_guide/performance.md index e5a383c25f5..0aba38aca8c 100644 --- a/doc/development/fe_guide/performance.md +++ b/doc/development/fe_guide/performance.md @@ -169,7 +169,6 @@ General tips: - [Profiling with Chrome DevTools][google-devtools-profiling] - [Browser Diet][browser-diet] is a community-built guide that catalogues practical tips for improving web page performance. - [web-page-test]: http://www.webpagetest.org/ [pagespeed-insights]: https://developers.google.com/speed/pagespeed/insights/ [google-devtools-profiling]: https://developers.google.com/web/tools/chrome-devtools/profile/?hl=en diff --git a/doc/development/fe_guide/security.md b/doc/development/fe_guide/security.md index 19e72c1d368..83bb449e54d 100644 --- a/doc/development/fe_guide/security.md +++ b/doc/development/fe_guide/security.md @@ -77,7 +77,6 @@ Inline styles should be avoided in almost all cases, they should only be used when no alternatives can be found. This allows reusability of styles as well as readability. - [observatory-cli]: https://github.com/mozilla/http-observatory-cli [qualys-ssl]: https://www.ssllabs.com/ssltest/analyze.html [secure_headers]: https://github.com/twitter/secureheaders diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index 9c614e3468a..60f322c6e75 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -229,7 +229,6 @@ One should apply to be a Vue.js expert by opening an MR when the Merge Request's - Vuex code follows the [documented pattern](./vuex.md#actions-pattern-request-and-receive-namespaces) - Knowledge about the existing Vue and Vuex applications and existing reusable components - [vue-docs]: http://vuejs.org/guide/index.html [issue-boards]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/boards [environments-table]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/environments diff --git a/doc/development/feature_flags.md b/doc/development/feature_flags.md index b6161cd6163..67356a12eba 100644 --- a/doc/development/feature_flags.md +++ b/doc/development/feature_flags.md @@ -124,4 +124,3 @@ Feature.enable(:feature_flag_name) ## Enabling a feature flag (in production) Check how to [roll out changes using feature flags](rolling_out_changes_using_feature_flags.md). - diff --git a/doc/development/file_storage.md b/doc/development/file_storage.md index 597812c8c49..18e4dc2ca0c 100644 --- a/doc/development/file_storage.md +++ b/doc/development/file_storage.md @@ -20,7 +20,6 @@ There are many places where file uploading is used, according to contexts: - LFS Objects - Merge request diffs - ## Disk storage GitLab started saving everything on local disk. While directory location changed from previous versions, diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md index ac910e80a89..6054873cb46 100644 --- a/doc/development/i18n/proofreader.md +++ b/doc/development/i18n/proofreader.md @@ -85,7 +85,7 @@ are very appreciative of the work done by translators and proofreaders! - Spanish - Pedro Garcia - [GitLab](https://gitlab.com/pedgarrod), [Crowdin](https://crowdin.com/profile/breaking_pitt) - Turkish - - Proofreaders needed. + - Ali Demirtaş - [GitLab](https://gitlab.com/alidemirtas), [Crowdin](https://crowdin.com/profile/alidemirtas) - Ukrainian - Volodymyr Sobotovych - [GitLab](https://gitlab.com/wheleph), [Crowdin](https://crowdin.com/profile/wheleph) - Andrew Vityuk - [GitLab](https://gitlab.com/3_1_3_u), [Crowdin](https://crowdin.com/profile/andruwa13) diff --git a/doc/development/import_export.md b/doc/development/import_export.md index 71db1abb201..e2108605d54 100644 --- a/doc/development/import_export.md +++ b/doc/development/import_export.md @@ -60,12 +60,12 @@ class StuckImportJobsWorker Marked stuck import jobs as failed. JIDs: xyz ``` -``` +``` +-----------+ +-----------------------------------+ |Export Job |--->| Calls ActiveRecord `as_json` and | +-----------+ | `to_json` on all project models | +-----------------------------------+ - + +-----------+ +-----------------------------------+ |Import Job |--->| Loads all JSON in memory, then | +-----------+ | inserts into the DB in batches | @@ -109,13 +109,13 @@ The `AttributeCleaner` removes any prohibited keys: # Removes all `_ids` and other prohibited keys class AttributeCleaner ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + ['group_id'] - + def clean @relation_hash.reject do |key, _value| prohibited_key?(key) || !@relation_class.attribute_method?(key) || excluded_key?(key) end.except('id') end - + ... ``` @@ -133,7 +133,7 @@ The `AttributeConfigurationSpec` checks and confirms the addition of new columns SAFE_MODEL_ATTRIBUTES: #{File.expand_path(safe_attributes_file)} IMPORT_EXPORT_CONFIG: #{Gitlab::ImportExport.config_file} -MSG +MSG ``` The `ModelConfigurationSpec` checks and confirms the addition of new models: @@ -157,7 +157,7 @@ The `ExportFileSpec` detects encrypted or sensitive columns: ```ruby # ExportFileSpec <<-MSG - Found a new sensitive word <#{key_found}>, which is part of the hash #{parent.inspect} + Found a new sensitive word <#{key_found}>, which is part of the hash #{parent.inspect} If you think this information shouldn't get exported, please exclude the model or attribute in IMPORT_EXPORT_CONFIG. @@ -214,7 +214,6 @@ We do not need to bump the version up in any of the following cases: - Remove a column or model (unless there is a DB constraint) - Export new things (such as a new type of upload) - Every time we bump the version, the integration specs will fail and can be fixed with: ```bash @@ -231,7 +230,7 @@ meaning that we want to bump the version up in the next version (or patch releas For example: 1. Add rename to `RelationRenameService` in X.Y -2. Remove it from `RelationRenameService` in X.Y + 1 +2. Remove it from `RelationRenameService` in X.Y + 1 3. Bump Import/Export version in X.Y + 1 ```ruby @@ -270,8 +269,8 @@ included_attributes: user: - :id - :email - ... - + ... + ``` Do not include the following attributes for the models specified: @@ -319,7 +318,7 @@ module Gitlab ensure remove_import_file end - + def restorers [repo_restorer, wiki_restorer, project_tree, avatar_restorer, uploads_restorer, lfs_restorer, statistics_restorer] @@ -346,7 +345,7 @@ module Projects end def save_services - [version_saver, avatar_saver, project_tree_saver, uploads_saver, repo_saver, + [version_saver, avatar_saver, project_tree_saver, uploads_saver, repo_saver, wiki_repo_saver, lfs_saver].all?(&:save) end ``` diff --git a/doc/development/kubernetes.md b/doc/development/kubernetes.md new file mode 100644 index 00000000000..4b2d48903ac --- /dev/null +++ b/doc/development/kubernetes.md @@ -0,0 +1,126 @@ +# Kubernetes integration - development guidelines + +This document provides various guidelines when developing for GitLab's +[Kubernetes integration](../user/project/clusters/index.md). + +## Development + +### Architecture + +Some Kubernetes operations, such as creating restricted project +namespaces are performed on the GitLab Rails application. These +operations are performed using a [client library](#client-library). +These operations will carry an element of risk as the operations will be +run as the same user running the GitLab Rails application, see the +[security](#security) section below. + +Some Kubernetes operations, such as installing cluster applications are +performed on one-off pods on the Kubernetes cluster itself. These +installation pods are currently named `install-<application_name>` and +are created within the `gitlab-managed-apps` namespace. + +In terms of code organization, we generally add objects that represent +Kubernetes resources in +[`lib/gitlab/kubernetes`](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/lib/gitlab/kubernetes). + +### Client library + +We use the [`kubeclient`](https://rubygems.org/gems/kubeclient) gem to +perform Kubernetes API calls. As the `kubeclient` gem does not support +different API Groups (e.g. `apis/rbac.authorization.k8s.io`) from a +single client, we have created a wrapper class, +[`Gitlab::Kubernetes::KubeClient`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/kubernetes/kube_client.rb) +that will enable you to achieve this. + +Selected Kubernetes API groups are currently supported. Do add support +for new API groups or methods to +[`Gitlab::Kubernetes::KubeClient`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/kubernetes/kube_client.rb) +if you need to use them. New API groups or API group versions can be +added to `SUPPORTED_API_GROUPS` - internally, this will create an +internal client for that group. New methods can be added as a delegation +to the relevant internal client. + +### Performance considerations + +All calls to the Kubernetes API must be in a background process. Do not +perform Kubernetes API calls within a web request as this will block +unicorn and can easily lead to a Denial Of Service (DoS) attack in GitLab as +the Kubernetes cluster response times are outside of our control. + +The easiest way to ensure your calls happen a background process is to +delegate any such work to happen in a [sidekiq +worker](sidekiq_style_guide.md). + +There are instances where you would like to make calls to Kubernetes and +return the response and as such a background worker does not seem to be +a good fit. For such cases you should make use of [reactive +caching](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/models/concerns/reactive_caching.rb). +For example: + +```ruby + def calculate_reactive_cache! + { pods: cluster.platform_kubernetes.kubeclient.get_pods } + end + + def pods + with_reactive_cache do |data| + data[:pods] + end + end +``` + +### Testing + +We have some Webmock stubs in +[`KubernetesHelpers`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/support/helpers/kubernetes_helpers.rb) +which can help with mocking out calls to Kubernetes API in your tests. + +## Security + +### SSRF + +As URLs for Kubernetes clusters are user controlled it is easily +susceptible to Server Side Request Forgery (SSRF) attacks. You should +understand the mitigation strategies if you are adding more API calls to +a cluster. + +Mitigation strategies include: + +1. Not allowing redirects to attacker controller resources: + [`Kubeclient::KubeClient`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/kubernetes/kube_client.rb#) + can be configured to disallow any redirects by passing in + `http_max_redirects: 0` as an option. +1. Not exposing error messages: by doing so, we + prevent attackers from triggering errors to expose results from + attacker controlled requests. For example, we do not expose (or store) + raw error messages: + + ```ruby + rescue Kubernetes::HttpError => e + # bad + # app.make_errored!("Kubernetes error: #{e.message}") + + # good + app.make_errored!("Kubernetes error: #{e.error_code}") + ``` + +## Debugging + +Logs related to the Kubernetes integration can be found in +[kubernetes.log](../administration/logs.md#kuberneteslog). On a local +GDK install, this will be present in `log/kubernetes.log`. + +Some services such as +[`Clusters::Applications::InstallService`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/services/clusters/applications/install_service.rb#L18) +rescues `StandardError` which can make it harder to debug issues in an +development environment. The current workaround is to temporarily +comment out the `rescue` in your local development source. + +You can also follow the installation pod logs to debug issues related to +installation. Once the installation/upgrade is underway, wait for the +pod to be created. Then run the following to obtain the pods logs as +they are written: + +```bash +kubectl logs <pod_name> --follow -n gitlab-managed-apps +``` diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 98b54684d39..bb40c0d32b4 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -48,7 +48,6 @@ various database operations, such as and whether they require downtime and how to work around that whenever possible. - ## Downtime Tagging Every migration must specify if it requires downtime or not, and if it should diff --git a/doc/development/new_fe_guide/development/testing.md b/doc/development/new_fe_guide/development/testing.md index d74f141f08f..8441089418e 100644 --- a/doc/development/new_fe_guide/development/testing.md +++ b/doc/development/new_fe_guide/development/testing.md @@ -339,7 +339,6 @@ afterEach(() => { Some regressions only affect a specific browser version. We can install and test in particular browsers with either Firefox or Browserstack using the following steps: - ### Browserstack [Browserstack](https://www.browserstack.com/) allows you to test more than 1200 mobile devices and browsers. diff --git a/doc/development/new_fe_guide/event_tracking.md b/doc/development/new_fe_guide/event_tracking.md new file mode 100644 index 00000000000..1958f1ce528 --- /dev/null +++ b/doc/development/new_fe_guide/event_tracking.md @@ -0,0 +1,74 @@ +# Event Tracking + +We use [Snowplow](https://github.com/snowplow/snowplow) for tracking custom events. + +## Generic tracking function + +In addition to Snowplow's built-in method for tracking page views, we use a generic tracking function which enables us to selectively apply listeners to events. + +The generic tracking function can be imported in EE-specific JS files as follows: + +```javascript +import { trackEvent } from `ee/stats`; +``` + +This gives the user access to the `trackEvent` method, which takes the following parameters: + +| parameter | type | description | required | +| ---------------- | ------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | +| `category` | string | Describes the page that you're capturing click events on. Unless infeasible, please use the Rails page attribute `document.body.dataset.page` by default. | true | +| `eventName` | string | Describes the action the user is taking. The first word should always describe the action. For example, clicks should be `click` and activations should be `activate`. Use underscores to describe what was acted on. For example, activating a form field would be `activate_form_input`. Clicking on a dropdown is `click_dropdown`. | true | +| `additionalData` | object | Additional data such as `label`, `property`, and `value` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy). | false | + +Read more about instrumentation and the taxonomy in the [Product Handbook](https://about.gitlab.com/handbook/product/feature-instrumentation). + +### Tracking in `.js` and `.vue` files + +The most simple use case is to add tracking programmatically to an event of interest in Javascript. + +The following example demonstrates how to track a click on a button in Javascript by calling the `trackEvent` method explicitly: + +```javascript +import { trackEvent } from `ee/stats`; + +trackEvent('dashboard:projects:index', 'click_button', { + label: 'create_from_template', + property: 'template_preview', + value: 'rails', +}); +``` + +### Tracking in HAML templates + +Sometimes we want to track clicks for multiple elements on a page. Creating event handlers for all elements could soon turn into a tedious task. + +There's a more convenient solution to this problem. When working with HAML templates, we can add `data-track-*` attributes to elements of interest. This way, all elements that have both `data-track-label` and `data-track-event` attributes assigned get marked for event tracking. All we have to do is call the `bindTrackableContainer` method on a container which allows for better scoping. + +Below is an example of `data-track-*` attributes assigned to a button in HAML: + +```ruby +%button.btn{ data: { track_label: "create_from_template", track_property: "template_preview", track_event: "click_button", track_value: "my-template" } } +``` + +By calling `bindTrackableContainer('.my-container')`, click handlers get bound to all elements located in `.my-container` provided that they have the necessary `data-track-*` attributes assigned to them. + +```javascript +import Stats from 'ee/stats'; + +document.addEventListener('DOMContentLoaded', () => { + Stats.bindTrackableContainer('.my-container', 'category'); +}); +``` + +The second parameter in `bindTrackableContainer` is optional. If omitted, the value of `document.body.dataset.page` will be used as category instead. + +Below is a list of supported `data-track-*` attributes: + +| attribute | description | required | +| --------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | +| `data-track-label` | The `label` in `trackEvent` | true | +| `data-track-event` | The `eventName` in `trackEvent` | true | +| `data-track-property` | The `property` in `trackEvent`. If omitted, an empty string will be used as a default value. | false | +| `data-track-value` | The `value` in `trackEvent`. If omitted, this will be `target.value` or empty string. For checkboxes, the default value being tracked will be the element's checked attribute if `data-track-value` is omitted. | false | + +Since Snowplow is an Enterprise Edition feature, it's necessary to create a CE backport when adding `data-track-*` attributes to HAML templates in most cases. diff --git a/doc/development/new_fe_guide/index.md b/doc/development/new_fe_guide/index.md index bfcca9cec7b..5fd5af252ef 100644 --- a/doc/development/new_fe_guide/index.md +++ b/doc/development/new_fe_guide/index.md @@ -27,6 +27,10 @@ Learn about all the internal JavaScript modules that make up our frontend. Style guides to keep our code consistent. +## [Event Tracking with Snowplow](event_tracking.md) + +How we use Snowplow to track custom events. + ## [Tips](tips.md) Tips from our frontend team to develop more efficiently and effectively. diff --git a/doc/development/performance.md b/doc/development/performance.md index 972c93be817..32970152911 100644 --- a/doc/development/performance.md +++ b/doc/development/performance.md @@ -336,7 +336,6 @@ the same method won't end up retrieving data from Redis upon every call. When memoizing cached data in an instance variable, make sure to also reset the instance variable when flushing the cache. An example: - ```ruby def first_branch @first_branch ||= cache.fetch(:first_branch) { branches.first } diff --git a/doc/development/policies.md b/doc/development/policies.md index 97424d90fb5..6a3b90f3604 100644 --- a/doc/development/policies.md +++ b/doc/development/policies.md @@ -8,7 +8,6 @@ The policy used is based on the subject's class name - so `Ability.allowed?(user Permissions are broken into two parts: `conditions` and `rules`. Conditions are boolean expressions that can access the database and the environment, while rules are statically configured combinations of expressions and other rules that enable or prevent certain abilities. For an ability to be allowed, it must be enabled by at least one rule, and not prevented by any. - ### Conditions Conditions are defined by the `condition` method, and are given a name and a block. The block will be executed in the context of the policy object - so it can access `@user` and `@subject`, as well as call any methods defined on the policy. Note that `@user` may be nil (in the anonymous case), but `@subject` is guaranteed to be a real instance of the subject class. @@ -66,7 +65,51 @@ Within the rule DSL, you can use: To see how the rules get evaluated into a judgment, it is useful in a console to use `policy.debug(:some_ability)`. This will print the rules in the order they are evaluated. -When a policy is asked whether a particular ability is allowed (`policy.allowed?(:some_ability)`), it does not necessarily have to compute all the conditions on the policy. First, only the rules relevant to that particular ability are selected. Then, the execution model takes advantage of short-circuiting, and attempts to sort rules based on a heuristic of how expensive they will be to calculate. The sorting is dynamic and cache-aware, so that previously calculated conditions will be considered first, before computing other conditions. +For example, let's say you wanted to debug `IssuePolicy`. You might run +the debugger in this way: + +```ruby +user = User.find_by(username: 'john') +issue = Issue.first +policy = IssuePolicy.new(user, issue) +policy.debug(:read_issue) +``` + +An example debug output would look as follows: + +```ruby +- [0] prevent when all?(confidential, ~can_read_confidential) ((@john : Issue/1)) +- [0] prevent when archived ((@john : Project/4)) +- [0] prevent when issues_disabled ((@john : Project/4)) +- [0] prevent when all?(anonymous, ~public_project) ((@john : Project/4)) ++ [32] enable when can?(:reporter_access) ((@john : Project/4)) +``` + +Each line represents a rule that was evaluated. There are a few things to note: + +1. The `-` or `+` symbol indicates whether the rule block was evaluated to be +`false` or `true`, respectively. +2. The number inside the brackets indicates the score. +3. The last part of the line (e.g. `@john : Issue/1`) shows the username +and subject for that rule. + +Here you can see that the first four rules were evaluated `false` for +which user and subject. For example, you can see in the last line that +the rule was activated because the user `root` had at reporter access to +the `Project/4`. + +When a policy is asked whether a particular ability is allowed +(`policy.allowed?(:some_ability)`), it does not necessarily have to +compute all the conditions on the policy. First, only the rules relevant +to that particular ability are selected. Then, the execution model takes +advantage of short-circuiting, and attempts to sort rules based on a +heuristic of how expensive they will be to calculate. The sorting is +dynamic and cache-aware, so that previously calculated conditions will +be considered first, before computing other conditions. + +Note that the score is chosen by a developer via the `score:` parameter +in a `condition` to denote how expensive evaluating this rule would be +relative to other rules. ## Scope diff --git a/doc/development/profiling.md b/doc/development/profiling.md index 0b0c6dfc8cf..f41d635de43 100644 --- a/doc/development/profiling.md +++ b/doc/development/profiling.md @@ -72,6 +72,15 @@ Gitlab::Profiler.print_by_total_time(result, max_percent: 60, min_percent: 2) # 0.02 0.865 0.000 0.000 0.864 638 *Enumerable#inject ``` +To print the profile in HTML format, use the following example: + +```ruby +result = Gitlab::Profiler.profile('/my-user') + +printer = RubyProf::CallStackPrinter.new(result) +printer.print(File.open('/tmp/profile.html', 'w')) +``` + [GitLab-Profiler](https://gitlab.com/gitlab-com/gitlab-profiler) is a project that builds on this to add some additional niceties, such as allowing configuration with a single Yaml file for multiple URLs, and uploading of the @@ -82,7 +91,6 @@ Redash to Looker](https://gitlab.com/gitlab-com/Product/issues/5#note_121194467) We are [currently investigating how to make this data public](https://gitlab.com/meltano/looker/issues/294). - ## Sherlock Sherlock is a custom profiling tool built into GitLab. Sherlock is _only_ diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md index ae9bf863419..4cc500ed1b6 100644 --- a/doc/development/rake_tasks.md +++ b/doc/development/rake_tasks.md @@ -151,7 +151,6 @@ following: bundle exec rake gemojione:digests ``` - This will update the file `fixtures/emojis/digests.json` based on the currently available Emoji. diff --git a/doc/development/shell_commands.md b/doc/development/shell_commands.md index 73893f9dd46..7bdf676be58 100644 --- a/doc/development/shell_commands.md +++ b/doc/development/shell_commands.md @@ -190,7 +190,7 @@ A check like this could have avoided CVE-2013-4583. ## Properly anchor regular expressions to the start and end of strings -When using regular expressions to validate user input that is passed as an argument to a shell command, make sure to use the `\A` and `\z` anchors that designate the start and end of the string, rather than `^` and `$`, or no anchors at all. +When using regular expressions to validate user input that is passed as an argument to a shell command, make sure to use the `\A` and `\z` anchors that designate the start and end of the string, rather than `^` and `$`, or no anchors at all. If you don't, an attacker could use this to execute commands with potentially harmful effect. @@ -198,7 +198,7 @@ For example, when a project's `import_url` is validated like below, the user cou ```ruby validates :import_url, format: { with: URI.regexp(%w(ssh git http https)) } -# URI.regexp(%w(ssh git http https)) roughly evaluates to /(ssh|git|http|https):(something_that_looks_like_a_url)/ +# URI.regexp(%w(ssh git http https)) roughly evaluates to /(ssh|git|http|https):(something_that_looks_like_a_url)/ ``` Suppose the user submits the following as their import URL: @@ -211,7 +211,6 @@ Since there are no anchors in the used regular expression, the `git:/tmp/lol` in When importing, GitLab would execute the following command, passing the `import_url` as an argument: - ```sh git clone file://git:/tmp/lol ``` diff --git a/doc/development/testing_guide/ci.md b/doc/development/testing_guide/ci.md index d685cacf9ea..5aa668290b4 100644 --- a/doc/development/testing_guide/ci.md +++ b/doc/development/testing_guide/ci.md @@ -24,7 +24,7 @@ Our current CI parallelization setup is as follows: uploaded to S3. After that, the next pipeline will use the up-to-date -`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file. +`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file. ### Monitoring diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md index 3af97717775..85b47f88cc4 100644 --- a/doc/development/testing_guide/review_apps.md +++ b/doc/development/testing_guide/review_apps.md @@ -143,17 +143,21 @@ thousands of unused Docker images.** **How big are the Kubernetes clusters (`review-apps-ce` and `review-apps-ee`)?** > The clusters are currently set up with a single pool of preemptible nodes, - with a minimum of 1 node and a maximum of 100 nodes. + with a minimum of 1 node and a maximum of 50 nodes. **What are the machine running on the cluster?** - > We're currently using `n1-standard-4` (4 vCPUs, 15 GB memory) machines. + > We're currently using `n1-standard-16` (16 vCPUs, 60 GB memory) machines. **How do we secure this from abuse? Apps are open to the world so we need to find a way to limit it to only us.** > This isn't enabled for forks. +## Other resources + +* [Review Apps integration for CE/EE (presentation)](https://docs.google.com/presentation/d/1QPLr6FO4LduROU8pQIPkX1yfGvD13GEJIBOenqoKxR8/edit?usp=sharing) + [charts-1068]: https://gitlab.com/charts/gitlab/issues/1068 [gitlab-pipeline]: https://gitlab.com/gitlab-org/gitlab-ce/pipelines/44362587 [gitlab:assets:compile]: https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/149511610 diff --git a/doc/development/testing_guide/testing_levels.md b/doc/development/testing_guide/testing_levels.md index 070b6477a7a..a7a3459719b 100644 --- a/doc/development/testing_guide/testing_levels.md +++ b/doc/development/testing_guide/testing_levels.md @@ -159,6 +159,10 @@ Every new feature should come with a [test plan]. > See [end-to-end tests](end_to_end_tests.md) for more information. +Note that `qa/spec` contains unit tests of the QA framework itself, not to be +confused with the application's [unit tests](#unit-tests) or +[end-to-end tests](#black-box-tests-at-the-system-level-aka-end-to-end-tests). + [multiple pieces]: ../architecture.md#components [GitLab Shell]: https://gitlab.com/gitlab-org/gitlab-shell [GitLab Workhorse]: https://gitlab.com/gitlab-org/gitlab-workhorse |