diff options
16 files changed, 312 insertions, 12 deletions
diff --git a/app/assets/javascripts/users_select/index.js b/app/assets/javascripts/users_select/index.js index e1e5cc565c6..0904aae0347 100644 --- a/app/assets/javascripts/users_select/index.js +++ b/app/assets/javascripts/users_select/index.js @@ -249,7 +249,7 @@ function UsersSelect(currentUser, els, options = {}) { )} <% } %>`, ); assigneeTemplate = template( - `<% if (username) { %> <a class="author-link bold" href="/<%- username %>"> <% if( avatar ) { %> <img width="32" class="avatar avatar-inline s32" alt="" src="<%- avatar %>"> <% } %> <span class="author"><%- name %></span> <span class="username"> @<%- username %> </span> </a> <% } else { %> <span class="no-value assign-yourself"> + `<% if (username) { %> <a class="author-link gl-font-weight-bold" href="/<%- username %>"> <% if( avatar ) { %> <img width="32" class="avatar avatar-inline s32" alt="" src="<%- avatar %>"> <% } %> <span class="author"><%- name %></span> <span class="username"> @<%- username %> </span> </a> <% } else { %> <span class="no-value assign-yourself"> ${sprintf(s__('UsersSelect|No assignee - %{openingTag} assign yourself %{closingTag}'), { openingTag: '<a href="#" class="js-assign-yourself">', closingTag: '</a>', @@ -585,7 +585,7 @@ function UsersSelect(currentUser, els, options = {}) { )}</a></li>`; } else { // 0 margin, because it's now handled by a wrapper - img = `<img src='${avatar}' class='avatar avatar-inline m-0' width='32' />`; + img = `<img src='${avatar}' class='avatar avatar-inline gl-m-0!' width='32' />`; } return userSelect.renderRow( @@ -806,9 +806,9 @@ UsersSelect.prototype.renderRow = function ( : user.name; return ` <li data-user-id=${user.id}> - <a href="#" class="dropdown-menu-user-link d-flex align-items-center ${linkClasses}" ${tooltipAttributes}> + <a href="#" class="dropdown-menu-user-link gl-display-flex! gl-align-items-center ${linkClasses}" ${tooltipAttributes}> ${this.renderRowAvatar(issuableType, user, img)} - <span class="d-flex flex-column overflow-hidden"> + <span class="gl-display-flex gl-flex-direction-column gl-overflow-hidden"> <strong class="dropdown-menu-user-full-name gl-font-weight-bold"> ${escape(name)} </strong> @@ -836,7 +836,7 @@ UsersSelect.prototype.renderRowAvatar = function (issuableType, user, img) { ? spriteIcon('warning-solid', 's12 merge-icon') : ''; - return `<span class="position-relative mr-2"> + return `<span class="gl-relative gl-mr-3"> ${img} ${mergeIcon} </span>`; @@ -851,7 +851,7 @@ UsersSelect.prototype.renderApprovalRules = function (elsClassName, approvalRule const [rule] = approvalRules; const countText = sprintf(__('(+%{count} rules)'), { count }); - const renderApprovalRulesCount = count > 1 ? `<span class="ml-1">${countText}</span>` : ''; + const renderApprovalRulesCount = count > 1 ? `<span class="gl-ml-2">${countText}</span>` : ''; const ruleName = rule.rule_type === 'code_owner' ? __('Code Owner') : escape(rule.name); return `<div class="gl-display-flex gl-font-sm"> diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index f478af32788..0a30e125c83 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -69,6 +69,11 @@ class ProjectFeature < ApplicationRecord default_value_for :metrics_dashboard_access_level, value: PRIVATE, allows_nil: false default_value_for :operations_access_level, value: ENABLED, allows_nil: false default_value_for :security_and_compliance_access_level, value: PRIVATE, allows_nil: false + default_value_for :monitor_access_level, value: ENABLED, allows_nil: false + default_value_for :infrastructure_access_level, value: ENABLED, allows_nil: false + default_value_for :feature_flags_access_level, value: ENABLED, allows_nil: false + default_value_for :environments_access_level, value: ENABLED, allows_nil: false + default_value_for :releases_access_level, value: ENABLED, allows_nil: false default_value_for(:pages_access_level, allows_nil: false) do |feature| if ::Gitlab::Pages.access_control_is_forced? diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index fb810af3e6b..5708421014a 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -10,6 +10,7 @@ module Projects def execute build_topics remove_unallowed_params + mirror_operations_access_level_changes validate! ensure_wiki_exists if enabling_wiki? @@ -82,6 +83,21 @@ module Projects params.delete(:emails_disabled) unless can?(current_user, :set_emails_disabled, project) end + # Temporary code to sync permissions changes as operations access setting + # is being split into monitor_access_level, deployments_access_level, infrastructure_access_level. + # To be removed as part of https://gitlab.com/gitlab-org/gitlab/-/issues/364240 + def mirror_operations_access_level_changes + return if Feature.enabled?(:split_operations_visibility_permissions, project) + + operations_access_level = params.dig(:project_feature_attributes, :operations_access_level) + + return if operations_access_level.nil? + + [:monitor_access_level, :infrastructure_access_level, :feature_flags_access_level, :environments_access_level].each do |key| + params[:project_feature_attributes][key] = operations_access_level + end + end + def after_update todos_features_changes = %w( issues_access_level diff --git a/config/feature_flags/development/split_operations_visibility_permissions.yml b/config/feature_flags/development/split_operations_visibility_permissions.yml new file mode 100644 index 00000000000..612876a2dcd --- /dev/null +++ b/config/feature_flags/development/split_operations_visibility_permissions.yml @@ -0,0 +1,8 @@ +--- +name: split_operations_visibility_permissions +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89089 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/364240 +milestone: '15.1' +type: development +group: group::respond +default_enabled: false diff --git a/db/migrate/20220531024905_add_operations_access_levels_to_project_feature.rb b/db/migrate/20220531024905_add_operations_access_levels_to_project_feature.rb new file mode 100644 index 00000000000..68921cd1468 --- /dev/null +++ b/db/migrate/20220531024905_add_operations_access_levels_to_project_feature.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class AddOperationsAccessLevelsToProjectFeature < Gitlab::Database::Migration[2.0] + OPERATIONS_DEFAULT_VALUE = 20 + + enable_lock_retries! + + # rubocop:disable Layout/LineLength + def up + add_column :project_features, :monitor_access_level, :integer, null: false, default: OPERATIONS_DEFAULT_VALUE + add_column :project_features, :infrastructure_access_level, :integer, null: false, default: OPERATIONS_DEFAULT_VALUE + add_column :project_features, :feature_flags_access_level, :integer, null: false, default: OPERATIONS_DEFAULT_VALUE + add_column :project_features, :environments_access_level, :integer, null: false, default: OPERATIONS_DEFAULT_VALUE + add_column :project_features, :releases_access_level, :integer, null: false, default: OPERATIONS_DEFAULT_VALUE + end + + def down + remove_column :project_features, :monitor_access_level + remove_column :project_features, :infrastructure_access_level + remove_column :project_features, :feature_flags_access_level + remove_column :project_features, :environments_access_level + remove_column :project_features, :releases_access_level + end +end diff --git a/db/post_migrate/20220531035113_populate_operation_visibility_permissions.rb b/db/post_migrate/20220531035113_populate_operation_visibility_permissions.rb new file mode 100644 index 00000000000..1d385b13f75 --- /dev/null +++ b/db/post_migrate/20220531035113_populate_operation_visibility_permissions.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class PopulateOperationVisibilityPermissions < Gitlab::Database::Migration[2.0] + BATCH_SIZE = 50_000 + MAX_BATCH_SIZE = 50_000 + SUB_BATCH_SIZE = 1_000 + INTERVAL = 2.minutes + MIGRATION = 'PopulateOperationVisibilityPermissionsFromOperations' + + disable_ddl_transaction! + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + def up + queue_batched_background_migration( + MIGRATION, + :project_features, + :id, + job_interval: INTERVAL, + batch_size: BATCH_SIZE, + max_batch_size: MAX_BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, :project_features, :id, []) + end +end diff --git a/db/schema_migrations/20220531024905 b/db/schema_migrations/20220531024905 new file mode 100644 index 00000000000..3892c437701 --- /dev/null +++ b/db/schema_migrations/20220531024905 @@ -0,0 +1 @@ +3470fa801f5d6c343c95d78a710aa1907a581575465718c8d971f4b8f305a39b
\ No newline at end of file diff --git a/db/schema_migrations/20220531035113 b/db/schema_migrations/20220531035113 new file mode 100644 index 00000000000..133741d8a36 --- /dev/null +++ b/db/schema_migrations/20220531035113 @@ -0,0 +1 @@ +4e4e158655d40797c4f9152ad3e4f8b9b4894ce1ce92bf89c6219f9c69847c45
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 69f9a4f93f7..7a966564e06 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -19350,7 +19350,12 @@ CREATE TABLE project_features ( analytics_access_level integer DEFAULT 20 NOT NULL, security_and_compliance_access_level integer DEFAULT 10 NOT NULL, container_registry_access_level integer DEFAULT 0 NOT NULL, - package_registry_access_level integer DEFAULT 0 NOT NULL + package_registry_access_level integer DEFAULT 0 NOT NULL, + monitor_access_level integer DEFAULT 20 NOT NULL, + infrastructure_access_level integer DEFAULT 20 NOT NULL, + feature_flags_access_level integer DEFAULT 20 NOT NULL, + environments_access_level integer DEFAULT 20 NOT NULL, + releases_access_level integer DEFAULT 20 NOT NULL ); CREATE SEQUENCE project_features_id_seq diff --git a/doc/administration/audit_event_streaming.md b/doc/administration/audit_event_streaming.md index 0401812331f..bd67d62f384 100644 --- a/doc/administration/audit_event_streaming.md +++ b/doc/administration/audit_event_streaming.md @@ -156,7 +156,8 @@ Each streaming destination can have up to 20 custom HTTP headers included with e ### Add with the API -Group owners can add a HTTP header using the GraphQL `auditEventsStreamingHeadersCreate` mutation. +Group owners can add a HTTP header using the GraphQL `auditEventsStreamingHeadersCreate` mutation. You can retrieve the destination ID +by [listing the external audit destinations](#list-streaming-destinations) on the group. ```graphql mutation { @@ -166,19 +167,48 @@ mutation { } ``` +The header is created if the returned `errors` object is empty. + ### Delete with the API -Group owners can remove a HTTP header using the GraphQL `auditEventsStreamingHeadersDestroy` mutation. +Group owners can remove a HTTP header using the GraphQL `auditEventsStreamingHeadersDestroy` mutation. You can retrieve the header ID +by [listing all the custom headers](#list-all-custom-headers-with-the-api) on the group. ```graphql mutation { - auditEventsStreamingHeadersDestroy(input: { headerId: "gid://gitlab/AuditEvents::ExternalAuditEventDestination/24601" }) { + auditEventsStreamingHeadersDestroy(input: { headerId: "gid://gitlab/AuditEvents::Streaming::Header/1" }) { errors } } ``` -The header is created if the returned `errors` object is empty. +The header is deleted if the returned `errors` object is empty. + +### List all custom headers with the API + +You can list all custom headers for a top-level group as well as their value and ID using the GraphQL `externalAuditEventDestinations` query. The ID +value returned by this query is what you need to pass to the `deletion` mutation. + +```graphql +query { + group(fullPath: "your-group") { + id + externalAuditEventDestinations { + nodes { + destinationUrl + id + headers { + nodes { + key + value + id + } + } + } + } + } +} +``` ## Verify event authenticity diff --git a/doc/api/users.md b/doc/api/users.md index a0aba8681a3..4c6f1f4ab3a 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -700,7 +700,7 @@ GET /user/status ``` ```shell -curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/user/status" +curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/user/status" ``` Example response: diff --git a/lib/gitlab/background_migration/populate_operation_visibility_permissions_from_operations.rb b/lib/gitlab/background_migration/populate_operation_visibility_permissions_from_operations.rb new file mode 100644 index 00000000000..3f04e04fc4d --- /dev/null +++ b/lib/gitlab/background_migration/populate_operation_visibility_permissions_from_operations.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Migrates the value operations_access_level to the new colums + # monitor_access_level, deployments_access_level, infrastructure_access_level. + # The operations_access_level setting is being split into three seperate toggles. + class PopulateOperationVisibilityPermissionsFromOperations < BatchedMigrationJob + def perform + each_sub_batch(operation_name: :populate_operations_visibility) do |batch| + batch.update_all('monitor_access_level=operations_access_level,' \ + 'infrastructure_access_level=operations_access_level,' \ + ' feature_flags_access_level=operations_access_level,'\ + ' environments_access_level=operations_access_level') + end + end + + private + + def mark_job_as_succeeded(*arguments) + Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded( + 'PopulateOperationVisibilityPermissionsFromOperations', + arguments + ) + end + end + end +end diff --git a/spec/lib/gitlab/background_migration/populate_operation_visibility_permissions_from_operations_spec.rb b/spec/lib/gitlab/background_migration/populate_operation_visibility_permissions_from_operations_spec.rb new file mode 100644 index 00000000000..1ebdca136a3 --- /dev/null +++ b/spec/lib/gitlab/background_migration/populate_operation_visibility_permissions_from_operations_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::PopulateOperationVisibilityPermissionsFromOperations do + let(:namespaces) { table(:namespaces) } + let(:project_features) { table(:project_features) } + let(:projects) { table(:projects) } + + let(:namespace) { namespaces.create!(name: 'user', path: 'user') } + + let(:proj_namespace1) { namespaces.create!(name: 'proj1', path: 'proj1', type: 'Project', parent_id: namespace.id) } + let(:proj_namespace2) { namespaces.create!(name: 'proj2', path: 'proj2', type: 'Project', parent_id: namespace.id) } + let(:proj_namespace3) { namespaces.create!(name: 'proj3', path: 'proj3', type: 'Project', parent_id: namespace.id) } + + let(:project1) { create_project('test1', proj_namespace1) } + let(:project2) { create_project('test2', proj_namespace2) } + let(:project3) { create_project('test3', proj_namespace3) } + + let!(:record1) { create_project_feature(project1) } + let!(:record2) { create_project_feature(project2, 20) } + let!(:record3) { create_project_feature(project3) } + + let(:sub_batch_size) { 2 } + let(:start_id) { record1.id } + let(:end_id) { record3.id } + let(:batch_table) { :project_features } + let(:batch_column) { :id } + let(:pause_ms) { 1 } + let(:connection) { ApplicationRecord.connection } + + let(:job) do + described_class.new( + start_id: start_id, + end_id: end_id, + batch_table: batch_table, + batch_column: batch_column, + sub_batch_size: sub_batch_size, + pause_ms: pause_ms, + connection: connection + ) + end + + subject(:perform) { job.perform } + + it 'updates all project settings records from their operations_access_level', :aggregate_failures do + perform + + expect_project_features_match_operations_access_level(record1) + expect_project_features_match_operations_access_level(record2) + expect_project_features_match_operations_access_level(record3) + end + + private + + def expect_project_features_match_operations_access_level(record) + record.reload + expect(record.monitor_access_level).to eq(record.operations_access_level) + expect(record.infrastructure_access_level).to eq(record.operations_access_level) + expect(record.feature_flags_access_level).to eq(record.operations_access_level) + expect(record.environments_access_level).to eq(record.operations_access_level) + end + + def create_project(proj_name, proj_namespace) + projects.create!( + namespace_id: namespace.id, + project_namespace_id: proj_namespace.id, + name: proj_name, + path: proj_name + ) + end + + def create_project_feature(project, operations_access_level = 10) + project_features.create!( + project_id: project.id, + pages_access_level: 10, + operations_access_level: operations_access_level + ) + end +end diff --git a/spec/migrations/populate_operation_visibility_permissions_spec.rb b/spec/migrations/populate_operation_visibility_permissions_spec.rb new file mode 100644 index 00000000000..6737a6f84c3 --- /dev/null +++ b/spec/migrations/populate_operation_visibility_permissions_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require_migration! + +RSpec.describe PopulateOperationVisibilityPermissions, :migration do + let(:migration) { described_class::MIGRATION } + + before do + stub_const("#{described_class.name}::SUB_BATCH_SIZE", 2) + end + + it 'schedules background migrations', :aggregate_failures do + migrate! + + expect(migration).to have_scheduled_batched_migration( + table_name: :project_features, + column_name: :id, + interval: described_class::INTERVAL + ) + end + + describe '#down' do + it 'deletes all batched migration records' do + migrate! + schema_migrate_down! + + expect(migration).not_to have_scheduled_batched_migration + end + end +end diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index fa193c05222..8d3622ca17d 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -124,6 +124,11 @@ project_feature: - created_at - metrics_dashboard_access_level - package_registry_access_level + - monitor_access_level + - infrastructure_access_level + - feature_flags_access_level + - environments_access_level + - releases_access_level - project_id - updated_at computed_attributes: diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 7b5bf1db030..a5e77104735 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -289,6 +289,42 @@ RSpec.describe Projects::UpdateService do end end + context 'when changing operations feature visibility' do + let(:feature_params) { { operations_access_level: ProjectFeature::DISABLED } } + + it 'does not sync the changes to the related fields' do + result = update_project(project, user, project_feature_attributes: feature_params) + + expect(result).to eq({ status: :success }) + feature = project.project_feature + + expect(feature.operations_access_level).to eq(ProjectFeature::DISABLED) + expect(feature.monitor_access_level).not_to eq(ProjectFeature::DISABLED) + expect(feature.infrastructure_access_level).not_to eq(ProjectFeature::DISABLED) + expect(feature.feature_flags_access_level).not_to eq(ProjectFeature::DISABLED) + expect(feature.environments_access_level).not_to eq(ProjectFeature::DISABLED) + end + + context 'when split_operations_visibility_permissions feature is disabled' do + before do + stub_feature_flags(split_operations_visibility_permissions: false) + end + + it 'syncs the changes to the related fields' do + result = update_project(project, user, project_feature_attributes: feature_params) + + expect(result).to eq({ status: :success }) + feature = project.project_feature + + expect(feature.operations_access_level).to eq(ProjectFeature::DISABLED) + expect(feature.monitor_access_level).to eq(ProjectFeature::DISABLED) + expect(feature.infrastructure_access_level).to eq(ProjectFeature::DISABLED) + expect(feature.feature_flags_access_level).to eq(ProjectFeature::DISABLED) + expect(feature.environments_access_level).to eq(ProjectFeature::DISABLED) + end + end + end + context 'when updating a project that contains container images' do before do stub_container_registry_config(enabled: true) |