From e7dc35bfd6a07a4c23f83b8173df2e4d1963402a Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 8 Jan 2024 03:13:30 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/models/organizations/organization.rb | 4 + app/models/organizations/organization_user.rb | 12 +++ app/services/organizations/create_service.rb | 12 ++- .../16-8-license-scanning-sbt-1-0-x.yml | 11 +++ ..._add_access_level_to_organization_users.rb | 9 ++ ..._start_date_column_from_vulnerabilities.rb | 15 ++++ ...ate_foreign_key_ci_build_trace_metadata.rb | 5 +- ...idate_foreign_key_ci_job_artifact_state.rb | 5 +- db/schema_migrations/20240101133628 | 1 + db/schema_migrations/20240102184844 | 1 + db/structure.sql | 8 +- doc/update/deprecations.md | 16 ++++ .../dependency_scanning/index.md | 5 +- gems/gitlab-housekeeper/README.md | 86 ++++++++++++++++++- .../organizations/organization_users.rb | 4 + .../models/organizations/organization_spec.rb | 26 ++++++ .../organizations/organization_user_spec.rb | 23 +++++ .../organizations/create_service_spec.rb | 2 + 18 files changed, 228 insertions(+), 17 deletions(-) create mode 100644 data/deprecations/16-8-license-scanning-sbt-1-0-x.yml create mode 100644 db/migrate/20240102184844_add_access_level_to_organization_users.rb create mode 100644 db/post_migrate/20240101133628_remove_start_date_column_from_vulnerabilities.rb create mode 100644 db/schema_migrations/20240101133628 create mode 100644 db/schema_migrations/20240102184844 diff --git a/app/models/organizations/organization.rb b/app/models/organizations/organization.rb index 7f86bf81375..df6f0109d57 100644 --- a/app/models/organizations/organization.rb +++ b/app/models/organizations/organization.rb @@ -54,6 +54,10 @@ module Organizations organization_users.exists?(user: user) end + def owner?(user) + organization_users.owners.exists?(user: user) + end + def web_url(only_path: nil) Gitlab::UrlBuilder.build(self, only_path: only_path) end diff --git a/app/models/organizations/organization_user.rb b/app/models/organizations/organization_user.rb index 5aa1133b017..9e06870dcc6 100644 --- a/app/models/organizations/organization_user.rb +++ b/app/models/organizations/organization_user.rb @@ -4,5 +4,17 @@ module Organizations class OrganizationUser < ApplicationRecord belongs_to :organization, inverse_of: :organization_users, optional: false belongs_to :user, inverse_of: :organization_users, optional: false + + validates :user, uniqueness: { scope: :organization_id } + validates :access_level, presence: true + + enum access_level: { + # Until we develop more access_levels, we really don't know if the default access_level will be what we think of + # as a guest. For now, we'll set to same value as guest, but call it default to denote the current ambivalence. + default: Gitlab::Access::GUEST, + owner: Gitlab::Access::OWNER + } + + scope :owners, -> { where(access_level: Gitlab::Access::OWNER) } end end diff --git a/app/services/organizations/create_service.rb b/app/services/organizations/create_service.rb index ab70799a095..f29065b8ffd 100644 --- a/app/services/organizations/create_service.rb +++ b/app/services/organizations/create_service.rb @@ -7,13 +7,21 @@ module Organizations organization = Organization.create(params) - return error_creating(organization) unless organization.persisted? + if organization.persisted? + add_organization_owner(organization) - ServiceResponse.success(payload: { organization: organization }) + ServiceResponse.success(payload: { organization: organization }) + else + error_creating(organization) + end end private + def add_organization_owner(organization) + organization.organization_users.create(user: current_user, access_level: :owner) + end + def error_no_permissions ServiceResponse.error(message: [_('You have insufficient permissions to create organizations')]) end diff --git a/data/deprecations/16-8-license-scanning-sbt-1-0-x.yml b/data/deprecations/16-8-license-scanning-sbt-1-0-x.yml new file mode 100644 index 00000000000..232fe87de0a --- /dev/null +++ b/data/deprecations/16-8-license-scanning-sbt-1-0-x.yml @@ -0,0 +1,11 @@ +- title: "License Scanning support for sbt 1.0.X" + removal_milestone: "17.0" + announcement_milestone: "16.8" + breaking_change: true + reporter: thiagocsf + stage: Secure + issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/437591 + body: | # (required) Don't change this line. + GitLab 17.0 removes License Scanning support for sbt 1.0.x. + + Users are advised to upgrade from sbt 1.0.x. diff --git a/db/migrate/20240102184844_add_access_level_to_organization_users.rb b/db/migrate/20240102184844_add_access_level_to_organization_users.rb new file mode 100644 index 00000000000..8e37d056e56 --- /dev/null +++ b/db/migrate/20240102184844_add_access_level_to_organization_users.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddAccessLevelToOrganizationUsers < Gitlab::Database::Migration[2.2] + milestone '16.8' + + def change + add_column :organization_users, :access_level, :integer, default: 10, limit: 2, null: false + end +end diff --git a/db/post_migrate/20240101133628_remove_start_date_column_from_vulnerabilities.rb b/db/post_migrate/20240101133628_remove_start_date_column_from_vulnerabilities.rb new file mode 100644 index 00000000000..747ff35377b --- /dev/null +++ b/db/post_migrate/20240101133628_remove_start_date_column_from_vulnerabilities.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class RemoveStartDateColumnFromVulnerabilities < Gitlab::Database::Migration[2.2] + milestone '16.8' + + enable_lock_retries! + + def up + remove_column :vulnerabilities, :start_date + end + + def down + add_column :vulnerabilities, :start_date, :date + end +end diff --git a/db/post_migrate/20240104091627_validate_foreign_key_ci_build_trace_metadata.rb b/db/post_migrate/20240104091627_validate_foreign_key_ci_build_trace_metadata.rb index fcf303acc41..16e97bd5e6a 100644 --- a/db/post_migrate/20240104091627_validate_foreign_key_ci_build_trace_metadata.rb +++ b/db/post_migrate/20240104091627_validate_foreign_key_ci_build_trace_metadata.rb @@ -3,10 +3,9 @@ class ValidateForeignKeyCiBuildTraceMetadata < Gitlab::Database::Migration[2.2] milestone '16.8' - FK_NAME = :fk_21d25cac1a_p - + # We first need to introduce this FK for self-managed def up - validate_foreign_key(:ci_build_trace_metadata, [:partition_id, :trace_artifact_id], name: FK_NAME) + # no-op end def down diff --git a/db/post_migrate/20240104091858_validate_foreign_key_ci_job_artifact_state.rb b/db/post_migrate/20240104091858_validate_foreign_key_ci_job_artifact_state.rb index bc67558fbfb..bd705fd07a5 100644 --- a/db/post_migrate/20240104091858_validate_foreign_key_ci_job_artifact_state.rb +++ b/db/post_migrate/20240104091858_validate_foreign_key_ci_job_artifact_state.rb @@ -3,10 +3,9 @@ class ValidateForeignKeyCiJobArtifactState < Gitlab::Database::Migration[2.2] milestone '16.8' - FK_NAME = :fk_rails_80a9cba3b2_p - + # We first need to introduce this FK for self-managed def up - validate_foreign_key(:ci_job_artifact_states, [:partition_id, :job_artifact_id], name: FK_NAME) + # no-op end def down diff --git a/db/schema_migrations/20240101133628 b/db/schema_migrations/20240101133628 new file mode 100644 index 00000000000..129206c5564 --- /dev/null +++ b/db/schema_migrations/20240101133628 @@ -0,0 +1 @@ +61b8bed56b1aa5fbce448cc8b90ec863801fbffb0a81f857ce512052f47ba1cb \ No newline at end of file diff --git a/db/schema_migrations/20240102184844 b/db/schema_migrations/20240102184844 new file mode 100644 index 00000000000..8b8a099bc2d --- /dev/null +++ b/db/schema_migrations/20240102184844 @@ -0,0 +1 @@ +6afdff39b79900760124493eaa5a7ab8c15f81e77f024338352eeff731479e34 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 950792e9d52..97a95c00c4a 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -20102,7 +20102,8 @@ CREATE TABLE organization_users ( organization_id bigint NOT NULL, user_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL + updated_at timestamp with time zone NOT NULL, + access_level smallint DEFAULT 10 NOT NULL ); CREATE SEQUENCE organization_users_id_seq @@ -24904,7 +24905,6 @@ CREATE TABLE vulnerabilities ( epic_id bigint, project_id bigint NOT NULL, author_id bigint NOT NULL, - start_date date, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, title character varying(255) NOT NULL, @@ -37468,7 +37468,7 @@ ALTER TABLE ONLY ci_build_trace_metadata ADD CONSTRAINT fk_21d25cac1a FOREIGN KEY (trace_artifact_id) REFERENCES ci_job_artifacts(id) ON DELETE CASCADE; ALTER TABLE ONLY ci_build_trace_metadata - ADD CONSTRAINT fk_21d25cac1a_p FOREIGN KEY (partition_id, trace_artifact_id) REFERENCES ci_job_artifacts(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE; + ADD CONSTRAINT fk_21d25cac1a_p FOREIGN KEY (partition_id, trace_artifact_id) REFERENCES ci_job_artifacts(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID; ALTER TABLE ONLY users_star_projects ADD CONSTRAINT fk_22cd27ddfc FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; @@ -39406,7 +39406,7 @@ ALTER TABLE ONLY ci_job_artifact_states ADD CONSTRAINT fk_rails_80a9cba3b2 FOREIGN KEY (job_artifact_id) REFERENCES ci_job_artifacts(id) ON DELETE CASCADE; ALTER TABLE ONLY ci_job_artifact_states - ADD CONSTRAINT fk_rails_80a9cba3b2_p FOREIGN KEY (partition_id, job_artifact_id) REFERENCES ci_job_artifacts(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE; + ADD CONSTRAINT fk_rails_80a9cba3b2_p FOREIGN KEY (partition_id, job_artifact_id) REFERENCES ci_job_artifacts(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID; ALTER TABLE ONLY approval_merge_request_rules_users ADD CONSTRAINT fk_rails_80e6801803 FOREIGN KEY (approval_merge_request_rule_id) REFERENCES approval_merge_request_rules(id) ON DELETE CASCADE; diff --git a/doc/update/deprecations.md b/doc/update/deprecations.md index aff68c7ea1b..80cebf268e2 100644 --- a/doc/update/deprecations.md +++ b/doc/update/deprecations.md @@ -800,6 +800,22 @@ The table below lists the deprecated metrics and their respective replacements.
+### License Scanning support for sbt 1.0.X + +
+- Announced in GitLab 16.8 +- Removal in GitLab 17.0 ([breaking change](https://docs.gitlab.com/ee/update/terminology.html#breaking-change)) +- To discuss this change or learn more, see the [deprecation issue](https://gitlab.com/gitlab-org/gitlab/-/issues/437591). +
+ +GitLab 17.0 removes License Scanning support for sbt 1.0.x. + +Users are advised to upgrade from sbt 1.0.x. + +
+ +
+ ### List repository directories Rake task
diff --git a/doc/user/application_security/dependency_scanning/index.md b/doc/user/application_security/dependency_scanning/index.md index 02d23d2b020..61a0544d6ba 100644 --- a/doc/user/application_security/dependency_scanning/index.md +++ b/doc/user/application_security/dependency_scanning/index.md @@ -277,7 +277,10 @@ The following languages and dependency managers are supported:

  • - Support for sbt 1.3 and above was added in GitLab 13.9. +

    • diff --git a/gems/gitlab-housekeeper/README.md b/gems/gitlab-housekeeper/README.md index f707b99c6f0..e468c12dade 100644 --- a/gems/gitlab-housekeeper/README.md +++ b/gems/gitlab-housekeeper/README.md @@ -1,6 +1,84 @@ # Gitlab::Housekeeper -Housekeeping following https://gitlab.com/gitlab-org/gitlab/-/merge_requests/134487 +This is a gem which can be run locally or in CI to do static and dynamic +analysis of the GitLab codebase and, using a list of predefined "keeps", it will +automatically create merge requests for things that developers would have +otherwise needed to remember to do themselves. + +It is analogous to a mix of `rubocop -a` and GitLab Dependency Bot. + +The word "keep" is used to describe a specific rule to apply to the code to +match a required change and actually edit the code. The word "keep" was chosen +as it sounds like "cop" and is very similar to implementing a rubocop rule as +well as code to autocorrect the rule. + +You can see the existing keeps in +https://gitlab.com/gitlab-org/gitlab/-/tree/master/keeps . + +## How the code is organized + +The code is organized in a very similar way to RuboCop in that we have an +overall gem called `gitlab-housekeeper` that contains the generic logic of +looping over all `keeps` (analogous to Cops) which are rules for how to detect +changes that can be made to the code and then actually how to correct them. + +Then users of this gem are expected to add a `keeps` directory in their project +with all the keeps specific to their project. This gem may at some point +include keeps that are generic enough to be used by other projects. + +## How to implement a keep + +The only thing you need to implement is an `each_change` method. The method +should yield changes in the form of a `::Gitlab::Housekeeper::Change` object, +where each change object represents a merge request that will be created. +The object describes the files that should be commited and other metadata +should be added to the merge request. Before yielding the `Change` the keep +should also edit the files locally. + +Here is an example of a very simple keep that creates 3 new files called +`new_file1.txt`, `new_file2.txt` and `new_file3.txt`: + +```ruby +# keeps/pretty_useless_keep.rb + +module Keeps + class PrettyUselessKeep < ::Gitlab::Housekeeper::Keep + def each_change + (1..3).each do |i| + file_name = "new_file#{i}.txt" + + `touch #{file_name}` + + identifiers = [self.class.name.demodulize, "new_file#{i}"] + + title = "Make new file #{file_name}" + + description = <<~MARKDOWN + ## New files + + This MR makes a new file #{file_name} + MARKDOWN + + changed_files = [file_name] + + yield(::Gitlab::Housekeeper::Change.new( + identifiers, title, description, changed_files + )) + end + end + end +end +``` + +You can dry-run this locally with the following command: + +``` +bundle exec gitlab-housekeeper -r keeps/pretty_useless_keep.rb -k Keeps::PrettyUselessKeep -d -m 3 +``` + +The `-d` just prints the contents of the merge request. Removing this will +actually create merge requests and requires setting a few environment +variables described below. ## Running @@ -14,7 +92,7 @@ a mistake. The alternative of using your own API token with it's permissions to 1. Create a project access token for that project 1. Set `housekeeper` remote to the fork you created ``` - git remote add housekeeper git@gitlab.com:DylanGriffith/gitlab.git + git remote add housekeeper ``` 1. Open a Postgres.ai tunnel on localhost port 6305 1. Set the Postgres AI env vars matching the tunnel details for your tunnel @@ -24,8 +102,8 @@ a mistake. The alternative of using your own API token with it's permissions to ``` 1. Set the GitLab client details. Will be used to create MR from housekeeper remote: ``` - export HOUSEKEEPER_FORK_PROJECT_ID=52263761 # Same project as housekeeper remote - export HOUSEKEEPER_TARGET_PROJECT_ID=52263761 # Can be 278964 (gitlab-org/gitlab) when ready to create real MRs + export HOUSEKEEPER_FORK_PROJECT_ID= # Same project as housekeeper remote + export HOUSEKEEPER_TARGET_PROJECT_ID= # Can be 278964 (gitlab-org/gitlab) when ready to create real MRs export HOUSEKEEPER_GITLAB_API_TOKEN=the-api-token ``` 1. Run it: diff --git a/spec/factories/organizations/organization_users.rb b/spec/factories/organizations/organization_users.rb index 761f260ccb3..d73d2386356 100644 --- a/spec/factories/organizations/organization_users.rb +++ b/spec/factories/organizations/organization_users.rb @@ -4,5 +4,9 @@ FactoryBot.define do factory :organization_user, class: 'Organizations::OrganizationUser' do user organization + + trait :owner do + access_level { Gitlab::Access::OWNER } + end end end diff --git a/spec/models/organizations/organization_spec.rb b/spec/models/organizations/organization_spec.rb index aba2b03d4d1..7a3c743eddd 100644 --- a/spec/models/organizations/organization_spec.rb +++ b/spec/models/organizations/organization_spec.rb @@ -204,6 +204,32 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel end end + describe '#owner?' do + let_it_be(:user) { create(:user) } + + subject { organization.owner?(user) } + + context 'when user is an owner' do + before do + create(:organization_user, :owner, organization: organization, user: user) + end + + it { is_expected.to eq true } + end + + context 'when user is not an owner' do + before do + create(:organization_user, organization: organization, user: user) + end + + it { is_expected.to eq false } + end + + context 'when user is not an organization user' do + it { is_expected.to eq false } + end + end + describe '#web_url' do it 'returns web url from `Gitlab::UrlBuilder`' do web_url = 'http://127.0.0.1:3000/-/organizations/default' diff --git a/spec/models/organizations/organization_user_spec.rb b/spec/models/organizations/organization_user_spec.rb index 9a2f11e0654..c3416c93ec9 100644 --- a/spec/models/organizations/organization_user_spec.rb +++ b/spec/models/organizations/organization_user_spec.rb @@ -8,6 +8,18 @@ RSpec.describe Organizations::OrganizationUser, type: :model, feature_category: it { is_expected.to belong_to(:user).inverse_of(:organization_users).required } end + describe 'validations' do + subject { build(:organization_user) } + + it { is_expected.to define_enum_for(:access_level).with_values(described_class.access_levels) } + it { is_expected.to validate_presence_of(:access_level) } + it { is_expected.to validate_uniqueness_of(:user).scoped_to(:organization_id) } + + it 'does not allow invalid enum value' do + expect { build(:organization_user, access_level: '_invalid_') }.to raise_error(ArgumentError) + end + end + context 'with loose foreign key on organization_users.organization_id' do it_behaves_like 'cleanup by a loose foreign key' do let_it_be(:parent) { create(:organization) } @@ -21,4 +33,15 @@ RSpec.describe Organizations::OrganizationUser, type: :model, feature_category: let_it_be(:model) { create(:organization_user, user: parent) } end end + + describe '.owners' do + it 'returns the owners of the organization' do + organization_user = create(:organization_user, :owner) + create(:organization_user) + + expect(described_class.owners).to match([organization_user]) + end + end + + it_behaves_like 'having unique enum values' end diff --git a/spec/services/organizations/create_service_spec.rb b/spec/services/organizations/create_service_spec.rb index aae89517c15..bbc0f3d7515 100644 --- a/spec/services/organizations/create_service_spec.rb +++ b/spec/services/organizations/create_service_spec.rb @@ -29,11 +29,13 @@ RSpec.describe Organizations::CreateService, feature_category: :cell do shared_examples 'creating an organization' do it 'creates the organization' do expect { response }.to change { Organizations::Organization.count } + .and change { Organizations::OrganizationUser.count }.by(1) expect(response).to be_success expect(created_organization.name).to eq(params[:name]) expect(created_organization.path).to eq(params[:path]) expect(created_organization.description).to eq(params[:description]) expect(created_organization.avatar.filename).to eq(avatar_filename) + expect(created_organization.owner?(current_user)).to be(true) end end