diff options
42 files changed, 908 insertions, 74 deletions
diff --git a/.gitlab/ci/docs.gitlab-ci.yml b/.gitlab/ci/docs.gitlab-ci.yml index d90bf4337e8..2e4d13448ae 100644 --- a/.gitlab/ci/docs.gitlab-ci.yml +++ b/.gitlab/ci/docs.gitlab-ci.yml @@ -16,6 +16,7 @@ name: review-docs/$DOCS_GITLAB_REPO_SUFFIX-$CI_MERGE_REQUEST_IID # DOCS_REVIEW_APPS_DOMAIN and DOCS_GITLAB_REPO_SUFFIX are CI variables # Discussion: https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/14236/diffs#note_40140693 + auto_stop_in: 2 weeks url: http://docs-preview-$DOCS_GITLAB_REPO_SUFFIX-$CI_MERGE_REQUEST_IID.$DOCS_REVIEW_APPS_DOMAIN/$DOCS_GITLAB_REPO_SUFFIX on_stop: review-docs-cleanup before_script: diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js index 010eda9b6c5..2a5571543fb 100644 --- a/app/assets/javascripts/boards/stores/boards_store.js +++ b/app/assets/javascripts/boards/stores/boards_store.js @@ -337,7 +337,8 @@ const boardsStore = { return ( (listTo.type !== 'label' && listFrom.type === 'assignee') || (listTo.type !== 'assignee' && listFrom.type === 'label') || - listFrom.type === 'backlog' + listFrom.type === 'backlog' || + listFrom.type === 'closed' ); }, moveIssueInList(list, issue, oldIndex, newIndex, idArray) { diff --git a/app/assets/javascripts/groups/constants.js b/app/assets/javascripts/groups/constants.js index 9c246cf3ba6..e27265b7b4a 100644 --- a/app/assets/javascripts/groups/constants.js +++ b/app/assets/javascripts/groups/constants.js @@ -39,7 +39,9 @@ export const GROUP_VISIBILITY_TYPE = { export const PROJECT_VISIBILITY_TYPE = { public: __('Public - The project can be accessed without any authentication.'), internal: __('Internal - The project can be accessed by any logged in user.'), - private: __('Private - Project access must be granted explicitly to each user.'), + private: __( + 'Private - Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group.', + ), }; export const VISIBILITY_TYPE_ICON = { diff --git a/app/helpers/system_note_helper.rb b/app/helpers/system_note_helper.rb index 05d698a6d99..c982b48a94e 100644 --- a/app/helpers/system_note_helper.rb +++ b/app/helpers/system_note_helper.rb @@ -2,7 +2,7 @@ module SystemNoteHelper ICON_NAMES_BY_ACTION = { - 'cherry_pick' => 'link', + 'cherry_pick' => 'cherry-pick-commit', 'commit' => 'commit', 'description' => 'pencil-square', 'merge' => 'git-merge', diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index a36de5dc548..d62839cf037 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -31,7 +31,7 @@ module VisibilityLevelHelper def project_visibility_level_description(level) case level when Gitlab::VisibilityLevel::PRIVATE - _("Project access must be granted explicitly to each user.") + _("Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group.") when Gitlab::VisibilityLevel::INTERNAL _("The project can be accessed by any logged in user.") when Gitlab::VisibilityLevel::PUBLIC diff --git a/app/models/clusters/applications/knative.rb b/app/models/clusters/applications/knative.rb index eebcbcba2d3..1f90318f845 100644 --- a/app/models/clusters/applications/knative.rb +++ b/app/models/clusters/applications/knative.rb @@ -33,6 +33,12 @@ module Clusters FETCH_IP_ADDRESS_DELAY, application.name, application.id) end end + + after_transition any => [:installed, :updated] do |application| + application.run_after_commit do + ClusterConfigureIstioWorker.perform_async(application.cluster_id) + end + end end default_value_for :version, VERSION @@ -41,6 +47,8 @@ module Clusters scope :for_cluster, -> (cluster) { where(cluster: cluster) } + has_one :pages_domain, through: :serverless_domain_cluster + def chart 'knative/knative' end @@ -49,6 +57,14 @@ module Clusters { "domain" => hostname }.to_yaml end + def available_domains + PagesDomain.instance_serverless + end + + def find_available_domain(pages_domain_id) + available_domains.find_by(id: pages_domain_id) + end + def allowed_to_uninstall? !pre_installed? end diff --git a/app/serializers/cluster_application_entity.rb b/app/serializers/cluster_application_entity.rb index 632718df780..ac59a9df9e5 100644 --- a/app/serializers/cluster_application_entity.rb +++ b/app/serializers/cluster_application_entity.rb @@ -13,4 +13,6 @@ class ClusterApplicationEntity < Grape::Entity expose :modsecurity_enabled, if: -> (e, _) { e.respond_to?(:modsecurity_enabled) } expose :update_available?, as: :update_available, if: -> (e, _) { e.respond_to?(:update_available?) } expose :can_uninstall?, as: :can_uninstall + expose :available_domains, using: Serverless::DomainEntity, if: -> (e, _) { e.respond_to?(:available_domains) } + expose :pages_domain, using: Serverless::DomainEntity, if: -> (e, _) { e.respond_to?(:pages_domain) } end diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index c48e60064ed..6df26de529d 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -94,7 +94,8 @@ class MergeRequestWidgetEntity < Grape::Entity merge_request.source_project&.uses_default_ci_config? && merge_request.all_pipelines.none? && merge_request.commits_count.positive? && - can?(current_user, :push_code, merge_request.source_project) + can?(current_user, :read_build, merge_request.source_project) && + can?(current_user, :create_pipeline, merge_request.source_project) end end diff --git a/app/serializers/serverless/domain_entity.rb b/app/serializers/serverless/domain_entity.rb new file mode 100644 index 00000000000..556e3c99eee --- /dev/null +++ b/app/serializers/serverless/domain_entity.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module Serverless + class DomainEntity < Grape::Entity + expose :id + expose :domain + end +end diff --git a/app/services/clusters/applications/base_service.rb b/app/services/clusters/applications/base_service.rb index 2585d815e07..a3b39f0994d 100644 --- a/app/services/clusters/applications/base_service.rb +++ b/app/services/clusters/applications/base_service.rb @@ -35,6 +35,12 @@ module Clusters application.oauth_application = create_oauth_application(application, request) end + if application.instance_of?(Knative) + Serverless::AssociateDomainService + .new(application, pages_domain_id: params[:pages_domain_id], creator: current_user) + .execute + end + worker = worker_class(application) application.make_scheduled! diff --git a/app/services/clusters/kubernetes/configure_istio_ingress_service.rb b/app/services/clusters/kubernetes/configure_istio_ingress_service.rb index fe577beaa8a..a81014d99ff 100644 --- a/app/services/clusters/kubernetes/configure_istio_ingress_service.rb +++ b/app/services/clusters/kubernetes/configure_istio_ingress_service.rb @@ -27,6 +27,10 @@ module Clusters return configure_certificates if serverless_domain_cluster configure_passthrough + rescue Kubeclient::HttpError => e + knative.make_errored!(_('Kubernetes error: %{error_code}') % { error_code: e.error_code }) + rescue StandardError + knative.make_errored!(_('Failed to update.')) end private diff --git a/app/services/serverless/associate_domain_service.rb b/app/services/serverless/associate_domain_service.rb new file mode 100644 index 00000000000..673f1f83260 --- /dev/null +++ b/app/services/serverless/associate_domain_service.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Serverless + class AssociateDomainService + PLACEHOLDER_HOSTNAME = 'example.com'.freeze + + def initialize(knative, pages_domain_id:, creator:) + @knative = knative + @pages_domain_id = pages_domain_id + @creator = creator + end + + def execute + return if unchanged? + + knative.hostname ||= PLACEHOLDER_HOSTNAME + + knative.pages_domain = knative.find_available_domain(pages_domain_id) + knative.serverless_domain_cluster.update(creator: creator) if knative.pages_domain + end + + private + + attr_reader :knative, :pages_domain_id, :creator + + def unchanged? + knative.pages_domain&.id == pages_domain_id + end + end +end diff --git a/changelogs/unreleased/25550-there-is-a-drag-and-drop-bug-in-boards.yml b/changelogs/unreleased/25550-there-is-a-drag-and-drop-bug-in-boards.yml new file mode 100644 index 00000000000..e4777065f07 --- /dev/null +++ b/changelogs/unreleased/25550-there-is-a-drag-and-drop-bug-in-boards.yml @@ -0,0 +1,5 @@ +--- +title: Resolves the disappearance of a ticket when it was moved from the closed list. +merge_request: +author: Gwen_ +type: fixed diff --git a/changelogs/unreleased/27300-add-filepath-to-release-links-api.yml b/changelogs/unreleased/27300-add-filepath-to-release-links-api.yml new file mode 100644 index 00000000000..2616aa2cc5b --- /dev/null +++ b/changelogs/unreleased/27300-add-filepath-to-release-links-api.yml @@ -0,0 +1,5 @@ +--- +title: Add filepath to release links API +merge_request: 25533 +author: +type: added diff --git a/changelogs/unreleased/fj-fix-internal-api-return-code.yml b/changelogs/unreleased/fj-fix-internal-api-return-code.yml new file mode 100644 index 00000000000..6809d48013a --- /dev/null +++ b/changelogs/unreleased/fj-fix-internal-api-return-code.yml @@ -0,0 +1,5 @@ +--- +title: Change back internal api return code +merge_request: 26063 +author: +type: fixed diff --git a/changelogs/unreleased/update-private-project-wording.yml b/changelogs/unreleased/update-private-project-wording.yml new file mode 100644 index 00000000000..db2434589f3 --- /dev/null +++ b/changelogs/unreleased/update-private-project-wording.yml @@ -0,0 +1,5 @@ +--- +title: Clarify private visibility for projects. +merge_request: 25852 +author: +type: other diff --git a/lib/api/entities/releases/link.rb b/lib/api/entities/releases/link.rb index 6cc01e0e981..f4edb83bd58 100644 --- a/lib/api/entities/releases/link.rb +++ b/lib/api/entities/releases/link.rb @@ -7,7 +7,17 @@ module API expose :id expose :name expose :url + expose :direct_asset_url expose :external?, as: :external + + def direct_asset_url + return object.url unless object.filepath + + release = object.release + project = release.project + + Gitlab::Routing.url_helpers.project_release_url(project, release) << object.filepath + end end end end diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index 577a6e890d7..ba66404e406 100644 --- a/lib/api/internal/base.rb +++ b/lib/api/internal/base.rb @@ -50,7 +50,11 @@ module API @project ||= access_checker.project result rescue Gitlab::GitAccess::ForbiddenError => e - return response_with_status(code: 403, success: false, message: e.message) + # The return code needs to be 401. If we return 403 + # the custom message we return won't be shown to the user + # and, instead, the default message 'GitLab: API is not accessible' + # will be displayed + return response_with_status(code: 401, success: false, message: e.message) rescue Gitlab::GitAccess::TimeoutError => e return response_with_status(code: 503, success: false, message: e.message) rescue Gitlab::GitAccess::NotFoundError => e diff --git a/lib/api/release/links.rb b/lib/api/release/links.rb index def36dc8529..f72230c084c 100644 --- a/lib/api/release/links.rb +++ b/lib/api/release/links.rb @@ -39,6 +39,7 @@ module API params do requires :name, type: String, desc: 'The name of the link' requires :url, type: String, desc: 'The URL of the link' + optional :filepath, type: String, desc: 'The filepath of the link' end post 'links' do authorize! :create_release, release @@ -73,6 +74,7 @@ module API params do optional :name, type: String, desc: 'The name of the link' optional :url, type: String, desc: 'The URL of the link' + optional :filepath, type: String, desc: 'The filepath of the link' at_least_one_of :name, :url end put do diff --git a/lib/gitlab/checks/snippet_check.rb b/lib/gitlab/checks/snippet_check.rb new file mode 100644 index 00000000000..26dd772764a --- /dev/null +++ b/lib/gitlab/checks/snippet_check.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Gitlab + module Checks + class SnippetCheck < BaseChecker + ERROR_MESSAGES = { + create_delete_branch: 'You can not create or delete branches.' + }.freeze + + ATTRIBUTES = %i[oldrev newrev ref branch_name tag_name logger].freeze + attr_reader(*ATTRIBUTES) + + def initialize(change, logger:) + @oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref) + @branch_name = Gitlab::Git.branch_name(@ref) + @tag_name = Gitlab::Git.tag_name(@ref) + + @logger = logger + @logger.append_message("Running checks for ref: #{@branch_name || @tag_name}") + end + + def exec + if creation? || deletion? + raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:create_delete_branch] + end + + # TODO: https://gitlab.com/gitlab-org/gitlab/issues/205628 + # Check operation will not result in more than one file in the repository + + true + end + end + end +end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index d6c87b858a8..45db423187c 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -60,7 +60,6 @@ module Gitlab @logger = Checks::TimedLogger.new(timeout: INTERNAL_TIMEOUT, header: LOG_HEADER) @changes = changes - check_namespace! check_protocol! check_valid_actor! check_active_user! @@ -72,11 +71,7 @@ module Gitlab return custom_action if custom_action check_db_accessibility!(cmd) - - ensure_project_on_push!(cmd, changes) - - check_project_accessibility! - add_project_moved_message! + check_project!(changes, cmd) check_repository_existence! case cmd @@ -113,6 +108,13 @@ module Gitlab private + def check_project!(changes, cmd) + check_namespace! + ensure_project_on_push!(cmd, changes) + check_project_accessibility! + add_project_moved_message! + end + def check_custom_action(cmd) nil end diff --git a/lib/gitlab/git_access_snippet.rb b/lib/gitlab/git_access_snippet.rb index d99b9c3fe89..ff1af9bede4 100644 --- a/lib/gitlab/git_access_snippet.rb +++ b/lib/gitlab/git_access_snippet.rb @@ -2,7 +2,13 @@ module Gitlab class GitAccessSnippet < GitAccess + extend ::Gitlab::Utils::Override + ERROR_MESSAGES = { + authentication_mechanism: 'The authentication mechanism is not supported.', + read_snippet: 'You are not allowed to read this snippet.', + update_snippet: 'You are not allowed to update this snippet.', + project_not_found: 'The project you were looking for could not be found.', snippet_not_found: 'The snippet you were looking for could not be found.', repository_not_found: 'The snippet repository you were looking for could not be found.' }.freeze @@ -12,25 +18,47 @@ module Gitlab def initialize(actor, snippet, protocol, **kwargs) @snippet = snippet - super(actor, project, protocol, **kwargs) + super(actor, snippet&.project, protocol, **kwargs) + + @auth_result_type = nil + @authentication_abilities &= [:download_code, :push_code] end - def check(cmd, _changes) + def check(cmd, changes) + # TODO: Investigate if expanding actor/authentication types are needed. + # https://gitlab.com/gitlab-org/gitlab/issues/202190 + if actor && !actor.is_a?(User) && !actor.instance_of?(Key) + raise UnauthorizedError, ERROR_MESSAGES[:authentication_mechanism] + end + unless Feature.enabled?(:version_snippets, user) - raise NotFoundError, ERROR_MESSAGES[:snippet_not_found] + raise NotFoundError, ERROR_MESSAGES[:project_not_found] end check_snippet_accessibility! - success_result(cmd) + super end - def project - snippet&.project + private + + override :check_project! + def check_project!(cmd, changes) + if snippet.is_a?(ProjectSnippet) + check_namespace! + check_project_accessibility! + # TODO add add_project_moved_message! to handle non-project repo https://gitlab.com/gitlab-org/gitlab/issues/205646 + end end - private + override :check_push_access! + def check_push_access! + raise UnauthorizedError, ERROR_MESSAGES[:update_snippet] unless user + + check_change_access! + end + override :repository def repository snippet&.repository end @@ -39,10 +67,64 @@ module Gitlab if snippet.blank? raise NotFoundError, ERROR_MESSAGES[:snippet_not_found] end + end + + override :check_download_access! + def check_download_access! + passed = guest_can_download_code? || user_can_download_code? + + unless passed + raise UnauthorizedError, ERROR_MESSAGES[:read_snippet] + end + end + + override :guest_can_download_code? + def guest_can_download_code? + Guest.can?(:read_snippet, snippet) + end + + override :user_can_download_code? + def user_can_download_code? + authentication_abilities.include?(:download_code) && user_access.can_do_action?(:read_snippet) + end + + override :check_change_access! + def check_change_access! + unless user_access.can_do_action?(:update_snippet) + raise UnauthorizedError, ERROR_MESSAGES[:update_snippet] + end + + changes_list.each do |change| + # If user does not have access to make at least one change, cancel all + # push by allowing the exception to bubble up + check_single_change_access(change) + end + end + + def check_single_change_access(change) + change_access = Checks::SnippetCheck.new(change, logger: logger) + + change_access.exec + rescue Checks::TimedLogger::TimeoutError + raise TimeoutError, logger.full_message + end - unless repository&.exists? + override :check_repository_existence! + def check_repository_existence! + unless repository.exists? raise NotFoundError, ERROR_MESSAGES[:repository_not_found] end end + + override :user_access + def user_access + @user_access ||= UserAccessSnippet.new(user, snippet: snippet) + end + + # TODO: Implement EE/Geo https://gitlab.com/gitlab-org/gitlab/issues/205629 + override :check_custom_action + def check_custom_action(cmd) + nil + end end end diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index 097b502316e..ae4c77255c5 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -98,7 +98,7 @@ module Gitlab @permission_cache ||= {} end - def can_access_git? + request_cache def can_access_git? user && user.can?(:access_git) end diff --git a/lib/gitlab/user_access_snippet.rb b/lib/gitlab/user_access_snippet.rb new file mode 100644 index 00000000000..bfed86c4df4 --- /dev/null +++ b/lib/gitlab/user_access_snippet.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module Gitlab + class UserAccessSnippet < UserAccess + extend ::Gitlab::Cache::RequestCache + # TODO: apply override check https://gitlab.com/gitlab-org/gitlab/issues/205677 + + request_cache_key do + [user&.id, snippet&.id] + end + + attr_reader :snippet + + def initialize(user, snippet: nil) + @user = user + @snippet = snippet + @project = snippet&.project + end + + def can_do_action?(action) + return false unless can_access_git? + + permission_cache[action] = + permission_cache.fetch(action) do + Ability.allowed?(user, action, snippet) + end + end + + def can_create_tag?(ref) + false + end + + def can_delete_branch?(ref) + false + end + + def can_push_to_branch?(ref) + super + return false unless snippet + return false unless can_do_action?(:update_snippet) + + true + end + + def can_merge_to_branch?(ref) + false + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 009e916112f..a425217fbe9 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -14396,7 +14396,7 @@ msgstr "" msgid "Private" msgstr "" -msgid "Private - Project access must be granted explicitly to each user." +msgid "Private - Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group." msgstr "" msgid "Private - The group and its projects can only be viewed by members." @@ -14834,7 +14834,7 @@ msgstr "" msgid "Project URL" msgstr "" -msgid "Project access must be granted explicitly to each user." +msgid "Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group." msgstr "" msgid "Project already deleted" diff --git a/spec/fixtures/api/schemas/cluster_status.json b/spec/fixtures/api/schemas/cluster_status.json index 29c56b5c820..6017ca9e2d5 100644 --- a/spec/fixtures/api/schemas/cluster_status.json +++ b/spec/fixtures/api/schemas/cluster_status.json @@ -39,9 +39,15 @@ "stack": { "type": ["string", "null"] }, "modsecurity_enabled": { "type": ["boolean", "null"] }, "update_available": { "type": ["boolean", "null"] }, - "can_uninstall": { "type": "boolean" } + "can_uninstall": { "type": "boolean" }, + "available_domains": { + "type": "array", + "items": { "$ref": "#/definitions/domain" } + }, + "pages_domain": { "type": [ { "$ref": "#/definitions/domain" }, "null"] } }, "required" : [ "name", "status" ] - } + }, + "domain": { "id": "integer", "domain": "string" } } } diff --git a/spec/fixtures/api/schemas/release/link.json b/spec/fixtures/api/schemas/release/link.json index 97347cb91cc..bf175be2bc0 100644 --- a/spec/fixtures/api/schemas/release/link.json +++ b/spec/fixtures/api/schemas/release/link.json @@ -4,7 +4,9 @@ "properties": { "id": { "type": "integer" }, "name": { "type": "string" }, + "filepath": { "type": "string" }, "url": { "type": "string" }, + "direct_asset_url": { "type": "string" }, "external": { "type": "boolean" } }, "additionalProperties": false diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index df338fac228..debe4401308 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -59,7 +59,7 @@ describe VisibilityLevelHelper do describe "#project_visibility_level_description" do it "describes private projects" do expect(project_visibility_level_description(Gitlab::VisibilityLevel::PRIVATE)) - .to eq _('Project access must be granted explicitly to each user.') + .to eq _('Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group.') end it "describes public projects" do diff --git a/spec/javascripts/groups/mock_data.js b/spec/javascripts/groups/mock_data.js index 2fdc844f3d9..380dda9f7b1 100644 --- a/spec/javascripts/groups/mock_data.js +++ b/spec/javascripts/groups/mock_data.js @@ -14,7 +14,8 @@ export const GROUP_VISIBILITY_TYPE = { export const PROJECT_VISIBILITY_TYPE = { public: 'Public - The project can be accessed without any authentication.', internal: 'Internal - The project can be accessed by any logged in user.', - private: 'Private - Project access must be granted explicitly to each user.', + private: + 'Private - Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group.', }; export const VISIBILITY_TYPE_ICON = { diff --git a/spec/lib/gitlab/checks/snippet_check_spec.rb b/spec/lib/gitlab/checks/snippet_check_spec.rb new file mode 100644 index 00000000000..7cb29debd1e --- /dev/null +++ b/spec/lib/gitlab/checks/snippet_check_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Checks::SnippetCheck do + include_context 'change access checks context' + + let(:snippet) { create(:personal_snippet, :repository) } + let(:user_access) { Gitlab::UserAccessSnippet.new(user, snippet: snippet) } + + subject { Gitlab::Checks::SnippetCheck.new(changes, logger: logger) } + + describe '#exec' do + it 'does not raise any error' do + expect { subject.exec }.not_to raise_error + end + + context 'trying to delete the branch' do + let(:newrev) { '0000000000000000000000000000000000000000' } + + it 'raises an error' do + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can not create or delete branches.') + end + end + + context 'trying to create the branch' do + let(:oldrev) { '0000000000000000000000000000000000000000' } + + it 'raises an error' do + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can not create or delete branches.') + end + end + end +end diff --git a/spec/lib/gitlab/git_access_snippet_spec.rb b/spec/lib/gitlab/git_access_snippet_spec.rb index ffb3d86408a..de19db38176 100644 --- a/spec/lib/gitlab/git_access_snippet_spec.rb +++ b/spec/lib/gitlab/git_access_snippet_spec.rb @@ -3,24 +3,30 @@ require 'spec_helper' describe Gitlab::GitAccessSnippet do - include GitHelpers + include ProjectHelpers + include TermsHelper + include_context 'ProjectPolicyTable context' + using RSpec::Parameterized::TableSyntax let_it_be(:user) { create(:user) } - let_it_be(:personal_snippet) { create(:personal_snippet, :private, :repository) } + let_it_be(:project) { create(:project, :public) } + let_it_be(:snippet) { create(:project_snippet, :public, :repository, project: project) } + let(:actor) { user } let(:protocol) { 'ssh' } let(:changes) { Gitlab::GitAccess::ANY } + let(:authentication_abilities) { [:download_code, :push_code] } + let(:push_access_check) { access.check('git-receive-pack', changes) } let(:pull_access_check) { access.check('git-upload-pack', changes) } - let(:snippet) { personal_snippet } - let(:actor) { personal_snippet.author } - describe 'when feature flag :version_snippets is enabled' do - it 'allows push and pull access' do - aggregate_failures do - expect { pull_access_check }.not_to raise_error - expect { push_access_check }.not_to raise_error - end + subject(:access) { Gitlab::GitAccessSnippet.new(actor, snippet, protocol, authentication_abilities: authentication_abilities) } + + describe 'when actor is a DeployKey' do + let(:actor) { build(:deploy_key) } + + it 'does not allow push and pull access' do + expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:authentication_mechanism]) end end @@ -30,56 +36,186 @@ describe Gitlab::GitAccessSnippet do end it 'does not allow push and pull access' do - aggregate_failures do - expect { push_access_check }.to raise_snippet_not_found - expect { pull_access_check }.to raise_snippet_not_found - end + expect { pull_access_check }.to raise_project_not_found end end describe '#check_snippet_accessibility!' do context 'when the snippet exists' do - it 'allows push and pull access' do - aggregate_failures do - expect { pull_access_check }.not_to raise_error - expect { push_access_check }.not_to raise_error - end + it 'allows access' do + project.add_developer(actor) + + expect { pull_access_check }.not_to raise_error end end context 'when the snippet is nil' do let(:snippet) { nil } - it 'blocks push and pull with "not found"' do - aggregate_failures do - expect { pull_access_check }.to raise_snippet_not_found - expect { push_access_check }.to raise_snippet_not_found - end + it 'blocks access with "not found"' do + expect { pull_access_check }.to raise_snippet_not_found end end context 'when the snippet does not have a repository' do let(:snippet) { build_stubbed(:personal_snippet) } - it 'blocks push and pull with "not found"' do - aggregate_failures do - expect { pull_access_check }.to raise_snippet_not_found - expect { push_access_check }.to raise_snippet_not_found + it 'blocks access with "not found"' do + expect { pull_access_check }.to raise_snippet_not_found + end + end + end + + context 'terms are enforced', :aggregate_failures do + before do + enforce_terms + end + + let(:user) { snippet.author } + + it 'blocks access when the user did not accept terms' do + message = /must accept the Terms of Service in order to perform this action/ + + expect { push_access_check }.to raise_unauthorized(message) + expect { pull_access_check }.to raise_unauthorized(message) + end + + it 'allows access when the user accepted the terms' do + accept_terms(user) + + expect { push_access_check }.not_to raise_error + expect { pull_access_check }.not_to raise_error + end + end + + context 'project snippet accessibility', :aggregate_failures do + let(:snippet) { create(:project_snippet, :private, :repository, project: project) } + let(:user) { membership == :author ? snippet.author : create_user_from_membership(project, membership) } + + shared_examples_for 'checks accessibility' do + [:anonymous, :non_member, :guest, :reporter, :maintainer, :admin, :author].each do |membership| + context membership.to_s do + let(:membership) { membership } + + it 'respects accessibility' do + if Ability.allowed?(user, :update_snippet, snippet) + expect { push_access_check }.not_to raise_error + else + expect { push_access_check }.to raise_error(described_class::UnauthorizedError) + end + + if Ability.allowed?(user, :read_snippet, snippet) + expect { pull_access_check }.not_to raise_error + else + expect { pull_access_check }.to raise_error(described_class::UnauthorizedError) + end + end + end + end + end + + context 'when project is public' do + it_behaves_like 'checks accessibility' + end + + context 'when project is public but snippet feature is private' do + let(:project) { create(:project, :public) } + + before do + update_feature_access_level(project, :private) + end + + it_behaves_like 'checks accessibility' + end + + context 'when project is not accessible' do + let(:project) { create(:project, :private) } + + [:anonymous, :non_member].each do |membership| + context membership.to_s do + let(:membership) { membership } + + it 'respects accessibility' do + expect { push_access_check }.to raise_error(described_class::NotFoundError) + expect { pull_access_check }.to raise_error(described_class::NotFoundError) + end end end end end - private + context 'personal snippet accessibility', :aggregate_failures do + let(:snippet) { create(:personal_snippet, snippet_level, :repository) } + let(:user) { membership == :author ? snippet.author : create_user_from_membership(nil, membership) } + + where(:snippet_level, :membership, :_expected_count) do + permission_table_for_personal_snippet_access + end - def access - described_class.new(actor, snippet, protocol, - authentication_abilities: [], - namespace_path: nil, project_path: nil, - redirected_path: nil, auth_result_type: nil) + with_them do + it "respects accessibility" do + error_class = described_class::UnauthorizedError + + if Ability.allowed?(user, :update_snippet, snippet) + expect { push_access_check }.not_to raise_error + else + expect { push_access_check }.to raise_error(error_class) + end + + if Ability.allowed?(user, :read_snippet, snippet) + expect { pull_access_check }.not_to raise_error + else + expect { pull_access_check }.to raise_error(error_class) + end + end + end end + context 'when geo is enabled', if: Gitlab.ee? do + let(:user) { snippet.author } + let!(:primary_node) { FactoryBot.create(:geo_node, :primary) } + + # Without override, push access would return Gitlab::GitAccessResult::CustomAction + it 'skips geo for snippet' do + allow(::Gitlab::Database).to receive(:read_only?).and_return(true) + allow(::Gitlab::Geo).to receive(:secondary_with_primary?).and_return(true) + + expect { push_access_check }.to raise_unauthorized(/You can't push code to a read-only GitLab instance/) + end + end + + context 'when changes are specific' do + let(:changes) { 'oldrev newrev ref' } + let(:user) { snippet.author } + + it 'does not raise error if SnippetCheck does not raise error' do + expect_next_instance_of(Gitlab::Checks::SnippetCheck) do |check| + expect(check).to receive(:exec).and_call_original + end + + expect { push_access_check }.not_to raise_error + end + + it 'raises error if SnippetCheck raises error' do + expect_next_instance_of(Gitlab::Checks::SnippetCheck) do |check| + allow(check).to receive(:exec).and_raise(Gitlab::GitAccess::UnauthorizedError, 'foo') + end + + expect { push_access_check }.to raise_unauthorized('foo') + end + end + + private + def raise_snippet_not_found raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:snippet_not_found]) end + + def raise_project_not_found + raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found]) + end + + def raise_unauthorized(message) + raise_error(Gitlab::GitAccess::UnauthorizedError, message) + end end diff --git a/spec/lib/gitlab/user_access_snippet_spec.rb b/spec/lib/gitlab/user_access_snippet_spec.rb new file mode 100644 index 00000000000..57e52e2e93d --- /dev/null +++ b/spec/lib/gitlab/user_access_snippet_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::UserAccessSnippet do + subject(:access) { described_class.new(user, snippet: snippet) } + + let_it_be(:project) { create(:project, :private) } + let_it_be(:snippet) { create(:project_snippet, :private, project: project) } + let(:user) { create(:user) } + + describe '#can_do_action?' do + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :ability, snippet).and_return(:foo) + end + + context 'when can access_git' do + it 'calls Ability#allowed? and returns its result' do + expect(access.can_do_action?(:ability)).to eq(:foo) + end + end + + context 'when can not access_git' do + it 'disallows access' do + expect(Ability).to receive(:allowed?).with(user, :access_git, :global).and_return(false) + + expect(access.can_do_action?(:ability)).to eq(false) + end + end + + context 'when user is nil' do + let(:user) { nil } + + it 'disallows access' do + expect(access.can_do_action?(:ability)).to eq(false) + end + end + end + + describe '#can_push_to_branch?' do + include ProjectHelpers + + [:anonymous, :non_member, :guest, :reporter, :maintainer, :admin, :author].each do |membership| + context membership.to_s do + let(:user) do + membership == :author ? snippet.author : create_user_from_membership(project, membership) + end + + context 'when can access_git' do + it 'respects accessibility' do + expected_result = Ability.allowed?(user, :update_snippet, snippet) + + expect(access.can_push_to_branch?('random_branch')).to eq(expected_result) + end + end + + context 'when can not access_git' do + it 'disallows access' do + expect(Ability).to receive(:allowed?).with(user, :access_git, :global).and_return(false) if user + + expect(access.can_push_to_branch?('random_branch')).to eq(false) + end + end + end + end + + context 'when snippet is nil' do + let(:user) { create_user_from_membership(project, :admin) } + let(:snippet) { nil } + + it 'disallows access' do + expect(access.can_push_to_branch?('random_branch')).to eq(false) + end + end + end + + describe '#can_create_tag?' do + it 'returns false' do + expect(access.can_create_tag?('random_tag')).to be_falsey + end + end + + describe '#can_delete_branch?' do + it 'returns false' do + expect(access.can_delete_branch?('random_branch')).to be_falsey + end + end + + describe '#can_merge_to_branch?' do + it 'returns false' do + expect(access.can_merge_to_branch?('random_branch')).to be_falsey + end + end +end diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index 993cc7d0203..7ff7644e703 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -14,6 +14,7 @@ describe Clusters::Applications::Knative do before do allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_in) allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) + allow(ClusterConfigureIstioWorker).to receive(:perform_async) end describe 'associations' do @@ -47,6 +48,32 @@ describe Clusters::Applications::Knative do end end + describe 'configuring istio ingress gateway' do + context 'after installed' do + let(:application) { create(:clusters_applications_knative, :installing) } + + before do + application.make_installed! + end + + it 'schedules a ClusterConfigureIstioWorker' do + expect(ClusterConfigureIstioWorker).to have_received(:perform_async).with(application.cluster_id) + end + end + + context 'after updated' do + let(:application) { create(:clusters_applications_knative, :updating) } + + before do + application.make_installed! + end + + it 'schedules a ClusterConfigureIstioWorker' do + expect(ClusterConfigureIstioWorker).to have_received(:perform_async).with(application.cluster_id) + end + end + end + describe '#can_uninstall?' do subject { knative.can_uninstall? } @@ -196,4 +223,34 @@ describe Clusters::Applications::Knative do describe 'validations' do it { is_expected.to validate_presence_of(:hostname) } end + + describe '#available_domains' do + let!(:domain) { create(:pages_domain, :instance_serverless) } + + it 'returns all instance serverless domains' do + expect(PagesDomain).to receive(:instance_serverless).and_call_original + + domains = subject.available_domains + + expect(domains.length).to eq(1) + expect(domains).to include(domain) + end + end + + describe '#find_available_domain' do + let!(:domain) { create(:pages_domain, :instance_serverless) } + + it 'returns the domain scoped to available domains' do + expect(subject).to receive(:available_domains).and_call_original + expect(subject.find_available_domain(domain.id)).to eq(domain) + end + end + + describe '#pages_domain' do + let!(:sdc) { create(:serverless_domain_cluster, knative: knative) } + + it 'returns the the associated pages domain' do + expect(knative.reload.pages_domain).to eq(sdc.pages_domain) + end + end end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 4249ce105c9..fe0a3ffebd3 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -409,7 +409,7 @@ describe API::Internal::Base do it do pull(key, project) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response["status"]).to be_falsey expect(user.reload.last_activity_on).to be_nil end @@ -419,7 +419,7 @@ describe API::Internal::Base do it do push(key, project) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response["status"]).to be_falsey expect(user.reload.last_activity_on).to be_nil end @@ -518,7 +518,7 @@ describe API::Internal::Base do it do pull(key, personal_project) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response["status"]).to be_falsey expect(user.reload.last_activity_on).to be_nil end @@ -528,7 +528,7 @@ describe API::Internal::Base do it do push(key, personal_project) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response["status"]).to be_falsey expect(user.reload.last_activity_on).to be_nil end @@ -572,7 +572,7 @@ describe API::Internal::Base do it do push(key, project) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response["status"]).to be_falsey end end @@ -654,7 +654,7 @@ describe API::Internal::Base do it 'rejects the SSH push' do push(key, project) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response['status']).to be_falsey expect(json_response['message']).to eq 'Git access over SSH is not allowed' end @@ -662,7 +662,7 @@ describe API::Internal::Base do it 'rejects the SSH pull' do pull(key, project) - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response['status']).to be_falsey expect(json_response['message']).to eq 'Git access over SSH is not allowed' end @@ -676,7 +676,7 @@ describe API::Internal::Base do it 'rejects the HTTP push' do push(key, project, 'http') - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response['status']).to be_falsey expect(json_response['message']).to eq 'Git access over HTTP is not allowed' end @@ -684,7 +684,7 @@ describe API::Internal::Base do it 'rejects the HTTP pull' do pull(key, project, 'http') - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response['status']).to be_falsey expect(json_response['message']).to eq 'Git access over HTTP is not allowed' end diff --git a/spec/requests/api/release/links_spec.rb b/spec/requests/api/release/links_spec.rb index 3a59052bb29..cf2043ecc74 100644 --- a/spec/requests/api/release/links_spec.rb +++ b/spec/requests/api/release/links_spec.rb @@ -135,16 +135,44 @@ describe API::Release::Links do end end end + + describe '#direct_asset_url' do + let!(:link) { create(:release_link, release: release, url: url, filepath: filepath) } + let(:url) { 'https://google.com/-/jobs/140463678/artifacts/download' } + + context 'when filepath is provided' do + let(:filepath) { '/bin/bigfile.exe' } + + specify do + get api("/projects/#{project.id}/releases/v0.1/assets/links/#{link.id}", maintainer) + + expect(json_response['direct_asset_url']).to eq("http://localhost/#{project.namespace.path}/#{project.name}/-/releases/#{release.tag}/bin/bigfile.exe") + end + end + + context 'when filepath is not provided' do + let(:filepath) { nil } + + specify do + get api("/projects/#{project.id}/releases/v0.1/assets/links/#{link.id}", maintainer) + + expect(json_response['direct_asset_url']).to eq(url) + end + end + end end describe 'POST /projects/:id/releases/:tag_name/assets/links' do let(:params) do { name: 'awesome-app.dmg', + filepath: '/binaries/awesome-app.dmg', url: 'https://example.com/download/awesome-app.dmg' } end + let(:last_release_link) { release.links.last } + it 'accepts the request' do post api("/projects/#{project.id}/releases/v0.1/assets/links", maintainer), params: params @@ -157,8 +185,9 @@ describe API::Release::Links do end.to change { Releases::Link.count }.by(1) release.reload - expect(release.links.last.name).to eq('awesome-app.dmg') - expect(release.links.last.url).to eq('https://example.com/download/awesome-app.dmg') + expect(last_release_link.name).to eq('awesome-app.dmg') + expect(last_release_link.filepath).to eq('/binaries/awesome-app.dmg') + expect(last_release_link.url).to eq('https://example.com/download/awesome-app.dmg') end it 'matches response schema' do diff --git a/spec/serializers/cluster_application_entity_spec.rb b/spec/serializers/cluster_application_entity_spec.rb index c700c150461..873fbf812cc 100644 --- a/spec/serializers/cluster_application_entity_spec.rb +++ b/spec/serializers/cluster_application_entity_spec.rb @@ -59,5 +59,23 @@ describe ClusterApplicationEntity do expect(subject[:external_ip]).to eq('111.222.111.222') end end + + context 'for knative application' do + let(:pages_domain) { create(:pages_domain, :instance_serverless) } + let(:application) { build(:clusters_applications_knative, :installed) } + + before do + create(:serverless_domain_cluster, knative: application, pages_domain: pages_domain) + end + + it 'includes available domains' do + expect(subject[:available_domains].length).to eq(1) + expect(subject[:available_domains].first).to eq(id: pages_domain.id, domain: pages_domain.domain) + end + + it 'includes pages_domain' do + expect(subject[:pages_domain]).to eq(id: pages_domain.id, domain: pages_domain.domain) + end + end end end diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb index 597dae81cfb..beb6a0de0f6 100644 --- a/spec/serializers/merge_request_widget_entity_spec.rb +++ b/spec/serializers/merge_request_widget_entity_spec.rb @@ -123,6 +123,26 @@ describe MergeRequestWidgetEntity do expect(subject[:merge_request_add_ci_config_path]).not_to be_nil end end + + context 'when build feature is disabled' do + before do + project.project_feature.update(builds_access_level: ProjectFeature::DISABLED) + end + + it 'has no path' do + expect(subject[:merge_request_add_ci_config_path]).to be_nil + end + end + + context 'when creating the pipeline is not allowed' do + before do + user.state = 'blocked' + end + + it 'has no path' do + expect(subject[:merge_request_add_ci_config_path]).to be_nil + end + end end context 'when user does not have permissions' do diff --git a/spec/serializers/serverless/domain_entity_spec.rb b/spec/serializers/serverless/domain_entity_spec.rb new file mode 100644 index 00000000000..bdf0ccb176c --- /dev/null +++ b/spec/serializers/serverless/domain_entity_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Serverless::DomainEntity do + describe '#as_json' do + let(:domain) { create(:pages_domain, :instance_serverless) } + + subject { described_class.new(domain).as_json } + + it 'has an id' do + expect(subject[:id]).to eq(domain.id) + end + + it 'has a domain' do + expect(subject[:domain]).to eq(domain.domain) + end + end +end diff --git a/spec/services/clusters/applications/create_service_spec.rb b/spec/services/clusters/applications/create_service_spec.rb index f62af86f1bf..0b48af408e1 100644 --- a/spec/services/clusters/applications/create_service_spec.rb +++ b/spec/services/clusters/applications/create_service_spec.rb @@ -137,10 +137,14 @@ describe Clusters::Applications::CreateService do let(:params) do { application: 'knative', - hostname: 'example.com' + hostname: 'example.com', + pages_domain_id: domain.id } end + let(:domain) { create(:pages_domain, :instance_serverless) } + let(:associate_domain_service) { double('AssociateDomainService') } + before do expect_any_instance_of(Clusters::Applications::Knative) .to receive(:make_scheduled!) @@ -158,6 +162,20 @@ describe Clusters::Applications::CreateService do it 'sets the hostname' do expect(subject.hostname).to eq('example.com') end + + it 'executes AssociateDomainService' do + expect(Serverless::AssociateDomainService).to receive(:new) do |knative, args| + expect(knative).to be_a(Clusters::Applications::Knative) + expect(args[:pages_domain_id]).to eq(params[:pages_domain_id]) + expect(args[:creator]).to eq(user) + + associate_domain_service + end + + expect(associate_domain_service).to receive(:execute) + + subject + end end context 'elastic stack application' do diff --git a/spec/services/clusters/applications/update_service_spec.rb b/spec/services/clusters/applications/update_service_spec.rb index 2d299882af0..4676951faff 100644 --- a/spec/services/clusters/applications/update_service_spec.rb +++ b/spec/services/clusters/applications/update_service_spec.rb @@ -7,8 +7,9 @@ describe Clusters::Applications::UpdateService do let(:cluster) { create(:cluster, :project, :provided_by_gcp) } let(:user) { create(:user) } - let(:params) { { application: 'knative', hostname: 'udpate.example.com' } } + let(:params) { { application: 'knative', hostname: 'update.example.com', pages_domain_id: domain.id } } let(:service) { described_class.new(cluster, user, params) } + let(:domain) { create(:pages_domain, :instance_serverless) } subject { service.execute(test_request) } @@ -51,6 +52,24 @@ describe Clusters::Applications::UpdateService do subject end + + context 'knative application' do + let(:associate_domain_service) { double('AssociateDomainService') } + + it 'executes AssociateDomainService' do + expect(Serverless::AssociateDomainService).to receive(:new) do |knative, args| + expect(knative.id).to eq(application.id) + expect(args[:pages_domain_id]).to eq(params[:pages_domain_id]) + expect(args[:creator]).to eq(user) + + associate_domain_service + end + + expect(associate_domain_service).to receive(:execute) + + subject + end + end end context 'application is not schedulable' do diff --git a/spec/services/clusters/kubernetes/configure_istio_ingress_service_spec.rb b/spec/services/clusters/kubernetes/configure_istio_ingress_service_spec.rb index 572e2b91187..9238f7debd0 100644 --- a/spec/services/clusters/kubernetes/configure_istio_ingress_service_spec.rb +++ b/spec/services/clusters/kubernetes/configure_istio_ingress_service_spec.rb @@ -194,4 +194,36 @@ describe Clusters::Kubernetes::ConfigureIstioIngressService, '#execute' do ) end end + + context 'when there is an error' do + before do + cluster.application_knative = create(:clusters_applications_knative) + + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:configure_passthrough).and_raise(error) + end + end + + context 'Kubeclient::HttpError' do + let(:error) { Kubeclient::HttpError.new(404, nil, nil) } + + it 'puts Knative into an errored state' do + subject + + expect(cluster.application_knative).to be_errored + expect(cluster.application_knative.status_reason).to eq('Kubernetes error: 404') + end + end + + context 'StandardError' do + let(:error) { RuntimeError.new('something went wrong') } + + it 'puts Knative into an errored state' do + subject + + expect(cluster.application_knative).to be_errored + expect(cluster.application_knative.status_reason).to eq('Failed to update.') + end + end + end end diff --git a/spec/services/serverless/associate_domain_service_spec.rb b/spec/services/serverless/associate_domain_service_spec.rb new file mode 100644 index 00000000000..3d1a878bcf5 --- /dev/null +++ b/spec/services/serverless/associate_domain_service_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Serverless::AssociateDomainService do + subject { described_class.new(knative, pages_domain_id: pages_domain_id, creator: creator) } + + let(:sdc) { create(:serverless_domain_cluster, pages_domain: create(:pages_domain, :instance_serverless)) } + let(:knative) { sdc.knative } + let(:creator) { sdc.creator } + let(:pages_domain_id) { sdc.pages_domain_id } + + context 'when the domain is unchanged' do + let(:creator) { create(:user) } + + it 'does not update creator' do + expect { subject.execute }.not_to change { sdc.reload.creator } + end + end + + context 'when domain is changed to nil' do + let(:pages_domain_id) { nil } + let(:creator) { create(:user) } + + it 'removes the association between knative and the domain' do + expect { subject.execute }.to change { knative.reload.pages_domain }.from(sdc.pages_domain).to(nil) + end + + it 'does not attempt to update creator' do + expect { subject.execute }.not_to raise_error + end + end + + context 'when a new domain is associated' do + let(:pages_domain_id) { create(:pages_domain, :instance_serverless).id } + let(:creator) { create(:user) } + + it 'creates an association with the domain' do + expect { subject.execute }.to change { knative.pages_domain.id }.from(sdc.pages_domain.id).to(pages_domain_id) + end + + it 'updates creator' do + expect { subject.execute }.to change { sdc.reload.creator }.from(sdc.creator).to(creator) + end + end + + context 'when knative is not authorized to use the pages domain' do + let(:pages_domain_id) { create(:pages_domain).id } + + before do + expect(knative).to receive(:available_domains).and_return(PagesDomain.none) + end + + it 'sets pages_domain_id to nil' do + expect { subject.execute }.to change { knative.reload.pages_domain }.from(sdc.pages_domain).to(nil) + end + end + + context 'when knative hostname is nil' do + let(:knative) { build(:clusters_applications_knative, hostname: nil) } + + it 'sets hostname to a placeholder value' do + expect { subject.execute }.to change { knative.hostname }.to('example.com') + end + end + + context 'when knative hostname exists' do + let(:knative) { build(:clusters_applications_knative, hostname: 'hostname.com') } + + it 'does not change hostname' do + expect { subject.execute }.not_to change { knative.hostname } + end + end +end |