diff --git a/app/assets/stylesheets/framework/layout.scss b/app/assets/stylesheets/framework/layout.scss index bb5213977dd..02b76b89482 100644 --- a/app/assets/stylesheets/framework/layout.scss +++ b/app/assets/stylesheets/framework/layout.scss @@ -136,6 +136,13 @@ body { } } +.gl--flex-full { + @include gl-display-flex; + @include gl-align-items-stretch; + @include gl-overflow-hidden; +} + + .with-performance-bar .layout-page { margin-top: calc(#{$header-height} + #{$performance-bar-height}); } diff --git a/app/controllers/groups/observability_controller.rb b/app/controllers/groups/observability_controller.rb new file mode 100644 index 00000000000..5b6503494c4 --- /dev/null +++ b/app/controllers/groups/observability_controller.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true +module Groups + class ObservabilityController < Groups::ApplicationController + feature_category :tracing + + content_security_policy do |p| + next if p.directives.blank? + + default_frame_src = p.directives['frame-src'] || p.directives['default-src'] + + # When ObservabilityUI is not authenticated, it needs to be able to redirect to the GL sign-in page, hence 'self' + frame_src_values = Array.wrap(default_frame_src) | [ObservabilityController.observability_url, "'self'"] + + p.frame_src(*frame_src_values) + end + + before_action :check_observability_allowed, only: :index + + def index + # Format: https://observe.gitlab.com/-/GROUP_ID + @observability_iframe_src = "#{ObservabilityController.observability_url}/-/#{@group.id}" + + # Uncomment below for testing with local GDK + # @observability_iframe_src = "#{ObservabilityController.observability_url}/9970?groupId=14485840" + + render layout: 'group', locals: { base_layout: 'layouts/fullscreen' } + end + + private + + def self.observability_url + return ENV['OVERRIDE_OBSERVABILITY_URL'] if ENV['OVERRIDE_OBSERVABILITY_URL'] + # TODO Make observability URL configurable https://gitlab.com/gitlab-org/opstrace/opstrace-ui/-/issues/80 + return "https://staging.observe.gitlab.com" if Gitlab.staging? + + "https://observe.gitlab.com" + end + + def check_observability_allowed + return render_404 unless self.class.observability_url.present? + + render_404 unless can?(current_user, :read_observability, @group) + end + end +end diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index bee32d789ac..96da0518dc0 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -59,6 +59,10 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy access_level(for_any_session: true) >= GroupMember::GUEST || valid_dependency_proxy_deploy_token end + condition(:observability_enabled) do + Feature.enabled?(:observability_group_tab, @subject) + end + desc "Deploy token with read_package_registry scope" condition(:read_package_registry_deploy_token) do @user.is_a?(DeployToken) && @user.groups.include?(@subject) && @user.read_package_registry @@ -293,6 +297,10 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy enable :destroy_resource_access_tokens end + rule { can?(:developer_access) & observability_enabled }.policy do + enable :read_observability + end + def access_level(for_any_session: false) return GroupMember::NO_ACCESS if @user.nil? return GroupMember::NO_ACCESS unless user_is_user? diff --git a/app/views/groups/observability/index.html.haml b/app/views/groups/observability/index.html.haml new file mode 100644 index 00000000000..582651c329b --- /dev/null +++ b/app/views/groups/observability/index.html.haml @@ -0,0 +1,2 @@ +- page_title _("Observability") +%iframe{ id: 'observability-ui-iframe', src: @observability_iframe_src, frameborder: 0, width: "100%", height: "100%" } diff --git a/app/views/layouts/fullscreen.html.haml b/app/views/layouts/fullscreen.html.haml index 2a865aeda40..61a57240ed5 100644 --- a/app/views/layouts/fullscreen.html.haml +++ b/app/views/layouts/fullscreen.html.haml @@ -6,12 +6,16 @@ = header_message = render partial: "layouts/header/default", locals: { project: @project, group: @group } .mobile-overlay - .alert-wrapper.hide-when-top-nav-responsive-open - = render 'shared/outdated_browser' - = render "layouts/broadcast" - = yield :flash_message - = render "layouts/flash" - .content-wrapper.hide-when-top-nav-responsive-open{ id: "content-body", class: "d-flex flex-column align-items-stretch" } - = yield + .hide-when-top-nav-responsive-open.gl--flex-full.gl-h-full{ class: nav ? ["layout-page", page_with_sidebar_class, "gl-mt-0!"]: '' } + - if defined?(nav) && nav + = render "layouts/nav/sidebar/#{nav}" + .gl--flex-full.gl-flex-direction-column.gl-w-full + .alert-wrapper + = render 'shared/outdated_browser' + = render "layouts/broadcast" + = yield :flash_message + = render "layouts/flash" + .content-wrapper{ id: "content-body", class: "d-flex flex-column align-items-stretch" } + = yield = render "layouts/nav/top_nav_responsive", class: "gl-flex-grow-1 gl-overflow-y-auto" = footer_message diff --git a/app/views/layouts/group.html.haml b/app/views/layouts/group.html.haml index 5a043704b76..97c2b8bb7e3 100644 --- a/app/views/layouts/group.html.haml +++ b/app/views/layouts/group.html.haml @@ -4,6 +4,7 @@ - nav "group" - display_subscription_banner! - @left_sidebar = true +- base_layout = local_assigns[:base_layout] - content_for :flash_message do = dispensable_render_if_exists "groups/storage_enforcement_alert", context: @group @@ -15,4 +16,4 @@ :plain window.uploads_path = "#{group_uploads_path(@group)}"; -= render template: "layouts/application" += render template: base_layout || "layouts/application" diff --git a/config/feature_flags/development/observability_group_tab.yml b/config/feature_flags/development/observability_group_tab.yml new file mode 100644 index 00000000000..b588a74e7d0 --- /dev/null +++ b/config/feature_flags/development/observability_group_tab.yml @@ -0,0 +1,8 @@ +--- +name: observability_group_tab +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/96374 +rollout_issue_url: +milestone: '15.3' +type: development +group: group::observability +default_enabled: false diff --git a/config/routes/group.rb b/config/routes/group.rb index 591a1983435..4a47b349665 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -121,6 +121,8 @@ constraints(::Constraints::GroupUrlConstrainer.new) do resource :dependency_proxy, only: [:show, :update] resources :email_campaigns, only: :index + resources :observability, only: :index + namespace :harbor do resources :repositories, only: [:index, :show], constraints: { id: %r{[a-zA-Z./:0-9_\-]+} } do resources :artifacts, only: [:index] do diff --git a/lib/sidebars/groups/menus/observability_menu.rb b/lib/sidebars/groups/menus/observability_menu.rb new file mode 100644 index 00000000000..b479ff3c492 --- /dev/null +++ b/lib/sidebars/groups/menus/observability_menu.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Sidebars + module Groups + module Menus + class ObservabilityMenu < ::Sidebars::Menu + override :link + def link + group_observability_index_path(context.group) + end + + override :title + def title + _('Observability') + end + + override :sprite_icon + def sprite_icon + 'monitor' + end + + override :render? + def render? + can?(context.current_user, :read_observability, context.group) + end + + override :active_routes + def active_routes + { controller: :observability, path: 'groups#observability' } + end + end + end + end +end diff --git a/lib/sidebars/groups/panel.rb b/lib/sidebars/groups/panel.rb index 463c2571b14..e8b815bdce7 100644 --- a/lib/sidebars/groups/panel.rb +++ b/lib/sidebars/groups/panel.rb @@ -12,6 +12,7 @@ module Sidebars add_menu(Sidebars::Groups::Menus::MergeRequestsMenu.new(context)) add_menu(Sidebars::Groups::Menus::CiCdMenu.new(context)) add_menu(Sidebars::Groups::Menus::KubernetesMenu.new(context)) + add_menu(Sidebars::Groups::Menus::ObservabilityMenu.new(context)) add_menu(Sidebars::Groups::Menus::PackagesRegistriesMenu.new(context)) add_menu(Sidebars::Groups::Menus::CustomerRelationsMenu.new(context)) add_menu(Sidebars::Groups::Menus::SettingsMenu.new(context)) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 07eb2451694..3d138d02cc0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -27126,6 +27126,9 @@ msgstr "" msgid "Object does not exist on the server or you don't have permissions to access it" msgstr "" +msgid "Observability" +msgstr "" + msgid "Oct" msgstr "" diff --git a/spec/features/groups/navbar_spec.rb b/spec/features/groups/navbar_spec.rb index b140e680012..b3fb563a202 100644 --- a/spec/features/groups/navbar_spec.rb +++ b/spec/features/groups/navbar_spec.rb @@ -19,6 +19,7 @@ RSpec.describe 'Group navbar' do stub_config(dependency_proxy: { enabled: false }) stub_config(registry: { enabled: false }) stub_feature_flags(harbor_registry_integration: false) + stub_feature_flags(observability_group_tab: false) stub_group_wikis(false) group.add_maintainer(user) sign_in(user) @@ -95,4 +96,16 @@ RSpec.describe 'Group navbar' do it_behaves_like 'verified navigation bar' end + + context 'when observability tab is enabled' do + before do + stub_feature_flags(observability_group_tab: true) + + insert_observability_nav + + visit group_path(group) + end + + it_behaves_like 'verified navigation bar' + end end diff --git a/spec/lib/sidebars/groups/menus/observability_menu_spec.rb b/spec/lib/sidebars/groups/menus/observability_menu_spec.rb new file mode 100644 index 00000000000..3a91b1aea2f --- /dev/null +++ b/spec/lib/sidebars/groups/menus/observability_menu_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::Groups::Menus::ObservabilityMenu do + let_it_be(:owner) { create(:user) } + let_it_be(:root_group) do + build(:group, :private).tap do |g| + g.add_owner(owner) + end + end + + let(:group) { root_group } + let(:user) { owner } + let(:context) { Sidebars::Groups::Context.new(current_user: user, container: group) } + let(:menu) { described_class.new(context) } + + describe '#render?' do + before do + allow(menu).to receive(:can?).and_call_original + end + + context 'when user can :read_observability' do + before do + allow(menu).to receive(:can?).with(user, :read_observability, group).and_return(true) + end + + it 'returns true' do + expect(menu.render?).to eq true + end + end + + context 'when user cannot :read_observability' do + before do + allow(menu).to receive(:can?).with(user, :read_observability, group).and_return(false) + end + + it 'returns false' do + expect(menu.render?).to eq false + end + end + end +end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 8fb02dc0f68..da0270c15b9 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -916,6 +916,74 @@ RSpec.describe GroupPolicy do end end + describe 'observability' do + using RSpec::Parameterized::TableSyntax + + let(:allowed) { be_allowed(:read_observability) } + let(:disallowed) { be_disallowed(:read_observability) } + + # rubocop:disable Layout/LineLength + where(:feature_enabled, :admin_matcher, :owner_matcher, :maintainer_matcher, :developer_matcher, :reporter_matcher, :guest_matcher, :non_member_matcher, :anonymous_matcher) do + false | ref(:disallowed) | ref(:disallowed) | ref(:disallowed) | ref(:disallowed) | ref(:disallowed) | ref(:disallowed) | ref(:disallowed) | ref(:disallowed) + true | ref(:allowed) | ref(:allowed) | ref(:allowed) | ref(:allowed) | ref(:disallowed) | ref(:disallowed) | ref(:disallowed) | ref(:disallowed) + end + # rubocop:enable Layout/LineLength + + with_them do + before do + stub_feature_flags(observability_group_tab: feature_enabled) + end + + context 'admin', :enable_admin_mode do + let(:current_user) { admin } + + it { is_expected.to admin_matcher } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to owner_matcher } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to maintainer_matcher } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to developer_matcher } + end + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to reporter_matcher } + end + + context 'with guest' do + let(:current_user) { guest } + + it { is_expected.to guest_matcher } + end + + context 'with non member' do + let(:current_user) { create(:user) } + + it { is_expected.to non_member_matcher } + end + + context 'with anonymous' do + let(:current_user) { nil } + + it { is_expected.to anonymous_matcher } + end + end + end + describe 'dependency proxy' do context 'feature disabled' do let(:current_user) { owner } diff --git a/spec/requests/groups/observability_controller_spec.rb b/spec/requests/groups/observability_controller_spec.rb new file mode 100644 index 00000000000..9be013d4385 --- /dev/null +++ b/spec/requests/groups/observability_controller_spec.rb @@ -0,0 +1,190 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::ObservabilityController do + include ContentSecurityPolicyHelpers + + let_it_be(:group) { create(:group) } + let_it_be(:user) { create(:user) } + + subject do + get group_observability_index_path(group) + response + end + + describe 'GET #index' do + context 'when user is not authenticated' do + it 'returns 404' do + expect(subject).to have_gitlab_http_status(:not_found) + end + end + + context 'when observability url is missing' do + before do + allow(described_class).to receive(:observability_url).and_return("") + end + + it 'returns 404' do + expect(subject).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is not a developer' do + before do + sign_in(user) + end + + it 'returns 404' do + expect(subject).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is authenticated and a developer' do + before do + sign_in(user) + group.add_developer(user) + end + + it 'returns 200' do + expect(subject).to have_gitlab_http_status(:ok) + end + + it 'renders the proper layout' do + expect(subject).to render_template("layouts/group") + expect(subject).to render_template("layouts/fullscreen") + expect(subject).not_to render_template('layouts/nav/breadcrumbs') + expect(subject).to render_template("nav/sidebar/_group") + end + + describe 'iframe' do + subject do + get group_observability_index_path(group) + Nokogiri::HTML.parse(response.body).at_css('iframe#observability-ui-iframe') + end + + it 'sets the iframe src to the proper URL' do + expect(subject.attributes['src'].value).to eq("https://observe.gitlab.com/-/#{group.id}") + end + + it 'when the env is staging, sets the iframe src to the proper URL' do + stub_config_setting(url: Gitlab::Saas.staging_com_url) + expect(subject.attributes['src'].value).to eq("https://staging.observe.gitlab.com/-/#{group.id}") + end + + it 'overrides the iframe src url if specified by OVERRIDE_OBSERVABILITY_URL env' do + stub_env('OVERRIDE_OBSERVABILITY_URL', 'http://foo.test') + + expect(subject.attributes['src'].value).to eq("http://foo.test/-/#{group.id}") + end + end + + describe 'CSP' do + before do + setup_existing_csp_for_controller(described_class, csp) + end + + subject do + get group_observability_index_path(group) + response.headers['Content-Security-Policy'] + end + + context 'when there is no CSP config' do + let(:csp) { ActionDispatch::ContentSecurityPolicy.new } + + it 'does not add any csp header' do + expect(subject).to be_blank + end + end + + context 'when frame-src exists in the CSP config' do + let(:csp) do + ActionDispatch::ContentSecurityPolicy.new do |p| + p.frame_src 'https://something.test' + end + end + + it 'appends the proper url to frame-src CSP directives' do + expect(subject).to include( + "frame-src https://something.test https://observe.gitlab.com 'self'") + end + + it 'appends the proper url to frame-src CSP directives when Gilab.staging?' do + stub_config_setting(url: Gitlab::Saas.staging_com_url) + + expect(subject).to include( + "frame-src https://something.test https://staging.observe.gitlab.com 'self'") + end + + it 'appends the proper url to frame-src CSP directives when OVERRIDE_OBSERVABILITY_URL is specified' do + stub_env('OVERRIDE_OBSERVABILITY_URL', 'http://foo.test') + + expect(subject).to include( + "frame-src https://something.test http://foo.test 'self'") + end + end + + context 'when self is already present in the policy' do + let(:csp) do + ActionDispatch::ContentSecurityPolicy.new do |p| + p.frame_src "'self'" + end + end + + it 'does not append self again' do + expect(subject).to include( + "frame-src 'self' https://observe.gitlab.com;") + end + end + + context 'when default-src exists in the CSP config' do + let(:csp) do + ActionDispatch::ContentSecurityPolicy.new do |p| + p.default_src 'https://something.test' + end + end + + it 'does not change default-src' do + expect(subject).to include( + "default-src https://something.test;") + end + + it 'appends the proper url to frame-src CSP directives' do + expect(subject).to include( + "frame-src https://something.test https://observe.gitlab.com 'self'") + end + + it 'appends the proper url to frame-src CSP directives when Gilab.staging?' do + stub_config_setting(url: Gitlab::Saas.staging_com_url) + + expect(subject).to include( + "frame-src https://something.test https://staging.observe.gitlab.com 'self'") + end + + it 'appends the proper url to frame-src CSP directives when OVERRIDE_OBSERVABILITY_URL is specified' do + stub_env('OVERRIDE_OBSERVABILITY_URL', 'http://foo.test') + + expect(subject).to include( + "frame-src https://something.test http://foo.test 'self'") + end + end + + context 'when frame-src and default-src exist in the CSP config' do + let(:csp) do + ActionDispatch::ContentSecurityPolicy.new do |p| + p.default_src 'https://something_default.test' + p.frame_src 'https://something.test' + end + end + + it 'appends to frame-src CSP directives' do + expect(subject).to include( + "frame-src https://something.test https://observe.gitlab.com 'self'") + expect(subject).to include( + "default-src https://something_default.test") + end + end + end + end + end +end diff --git a/spec/routing/group_routing_spec.rb b/spec/routing/group_routing_spec.rb index 9f5f821cc61..ae69b222280 100644 --- a/spec/routing/group_routing_spec.rb +++ b/spec/routing/group_routing_spec.rb @@ -71,6 +71,10 @@ RSpec.shared_examples 'groups routing' do it 'routes to the harbor tags controller' do expect(get("groups/#{group_path}/-/harbor/repositories/test/artifacts/test/tags")).to route_to('groups/harbor/tags#index', group_id: group_path, repository_id: 'test', artifact_id: 'test') end + + it 'routes to the observability controller' do + expect(get("groups/#{group_path}/-/observability")).to route_to('groups/observability#index', group_id: group_path) + end end RSpec.describe "Groups", "routing" do diff --git a/spec/support/helpers/navbar_structure_helper.rb b/spec/support/helpers/navbar_structure_helper.rb index 3d51c022b39..b44552d6479 100644 --- a/spec/support/helpers/navbar_structure_helper.rb +++ b/spec/support/helpers/navbar_structure_helper.rb @@ -85,6 +85,16 @@ module NavbarStructureHelper ) end + def insert_observability_nav + insert_after_nav_item( + _('Kubernetes'), + new_nav_item: { + nav_item: _('Observability'), + nav_sub_items: [] + } + ) + end + def insert_infrastructure_google_cloud_nav insert_after_sub_nav_item( _('Terraform'), diff --git a/spec/views/groups/observability.html.haml_spec.rb b/spec/views/groups/observability.html.haml_spec.rb new file mode 100644 index 00000000000..db280d5a2ba --- /dev/null +++ b/spec/views/groups/observability.html.haml_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'groups/observability/index' do + let_it_be(:iframe_url) { "foo.test" } + + before do + assign(:observability_iframe_src, iframe_url) + end + + it 'renders as expected' do + render + page = Capybara.string(rendered) + iframe = page.find('iframe#observability-ui-iframe') + expect(iframe['src']).to eq(iframe_url) + end +end diff --git a/spec/views/layouts/fullscreen.html.haml_spec.rb b/spec/views/layouts/fullscreen.html.haml_spec.rb index 0ae2c76ebcb..14b382bc238 100644 --- a/spec/views/layouts/fullscreen.html.haml_spec.rb +++ b/spec/views/layouts/fullscreen.html.haml_spec.rb @@ -9,5 +9,46 @@ RSpec.describe 'layouts/fullscreen' do allow(view).to receive(:current_user_mode).and_return(Gitlab::Auth::CurrentUserMode.new(user)) end + it 'renders a flex container' do + render + + expect(rendered).to have_selector(".gl--flex-full.gl-h-full") + expect(rendered).to have_selector(".gl--flex-full.gl-w-full") + end + it_behaves_like 'a layout which reflects the application theme setting' + + describe 'sidebar' do + context 'when nav is set' do + before do + allow(view).to receive(:nav).and_return("admin") + render + end + + it 'renders the sidebar' do + expect(rendered).to render_template("layouts/nav/sidebar/_admin") + expect(rendered).to have_selector("aside.nav-sidebar") + end + + it 'adds the proper classes' do + expect(rendered).to have_selector(".layout-page.gl-mt-0\\!") + end + end + + describe 'when nav is not set' do + before do + allow(view).to receive(:nav).and_return(nil) + render + end + + it 'does not render the sidebar' do + expect(rendered).not_to render_template("layouts/nav/sidebar/_admin") + expect(rendered).not_to have_selector("aside.nav-sidebar") + end + + it 'not add classes' do + expect(rendered).not_to have_selector(".layout-page.gl-mt-0\\!") + end + end + end end