From aef0afd34008eb5971e5e8b155230e2617509a7d Mon Sep 17 00:00:00 2001 From: Maciej Mensfeld Date: Mon, 4 Sep 2023 14:59:08 +0200 Subject: [PATCH] Config validation + session management (#102) --- CHANGELOG.md | 17 ++ lib/karafka/web.rb | 10 + lib/karafka/web/config.rb | 10 + lib/karafka/web/contracts/config.rb | 62 +++++++ lib/karafka/web/management/clean_boot_file.rb | 1 + .../web/management/extend_boot_file.rb | 13 +- lib/karafka/web/ui/base.rb | 5 + spec/lib/karafka/web/contracts/config_spec.rb | 172 ++++++++++++++++++ .../web/management/extend_boot_file_spec.rb | 11 +- 9 files changed, 298 insertions(+), 3 deletions(-) create mode 100644 lib/karafka/web/contracts/config.rb create mode 100644 spec/lib/karafka/web/contracts/config_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index bf0975a6..e9a9bc92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ - [Improvement] Prevent locking in sampler for time of OS data aggregation. - [Improvement] Collect and report number of messages in particular jobs. - [Improvement] Limit segment size for Web topics to ensure, that Web-UI does not drain resources. +- [Improvement] Introduce cookie based sessions management for future usage. +- [Improvement] Introduce config validation. - [Fix] Return 402 status instead of 500 on Pro features that are not available in OSS. - [Fix] Fix a case where errors would not be visible without Rails due to the `String#first` usage. - [Fix] Fix a case where live-poll would be disabled but would still update data. @@ -66,6 +68,21 @@ This is a **major** release that brings many things to the table. +#### Configuration + +Karafka Web UI now relies on Roda session management. Please configure the `ui.sessions.secret` key with a secret value string of at least 64 characters: + +```ruby +# Configure it BEFORE enabling +Karafka::Web.setup do |config| + # REPLACE THIS with your own value. You can use `SecureRandom.hex(64)` to generate it + # You may want to set it per ENV + config.ui.sessions.secret = 'REPLACE ME! b94b2215cc66371f2c34b7d0c0df1a010f83ca45 REPLACE ME!' +end + +Karafka::Web.enable! +``` + #### Deployment Because of the reporting schema update and new web-ui topics introduction, it is recommended to: diff --git a/lib/karafka/web.rb b/lib/karafka/web.rb index 9e4c8c51..0f48a9f3 100644 --- a/lib/karafka/web.rb +++ b/lib/karafka/web.rb @@ -6,6 +6,7 @@ etc open3 zlib + securerandom ].each { |lib| require lib } module Karafka @@ -31,7 +32,16 @@ def config # Activates all the needed routing and sets up listener, etc # This needs to run **after** the optional configuration of the web component def enable! + # Make sure config is as expected + # It should be configured before enabling the Web UI + Contracts::Config.new.validate!(config.to_h) + Installer.new.enable! + + # Inject correct settings for the Web-UI sessions plugin based on the user configuration + # We cannot configure this automatically like other Roda plugins because it requires safe + # custom values provided by our user + Ui::Base.plugin(:sessions, **config.ui.sessions.to_h) end end end diff --git a/lib/karafka/web/config.rb b/lib/karafka/web/config.rb index 241a172e..dd7514be 100644 --- a/lib/karafka/web/config.rb +++ b/lib/karafka/web/config.rb @@ -81,6 +81,16 @@ class Config end setting :ui do + # UI session settings + # Should be set per ENV. + setting :sessions do + # Cookie key name + setting :key, default: '_karafka_session' + + # Secret for the session cookie + setting :secret, default: SecureRandom.hex(32) + end + # UI cache to improve performance of views that reuse states that are not often changed setting :cache, default: Ui::Lib::TtlCache.new(60_000 * 5) diff --git a/lib/karafka/web/contracts/config.rb b/lib/karafka/web/contracts/config.rb new file mode 100644 index 00000000..cbd537e1 --- /dev/null +++ b/lib/karafka/web/contracts/config.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module Karafka + module Web + module Contracts + # Contract to validate Web-UI configuration + class Config < Web::Contracts::Base + configure + + # Use the same regexp as Karafka for topics validation + TOPIC_REGEXP = ::Karafka::Contracts::TOPIC_REGEXP + + required(:ttl) { |val| val.is_a?(Numeric) && val.positive? } + + nested(:topics) do + required(:errors) { |val| val.is_a?(String) && TOPIC_REGEXP.match?(val) } + + nested(:consumers) do + required(:reports) { |val| val.is_a?(String) && TOPIC_REGEXP.match?(val) } + required(:states) { |val| val.is_a?(String) && TOPIC_REGEXP.match?(val) } + required(:metrics) { |val| val.is_a?(String) && TOPIC_REGEXP.match?(val) } + end + end + + nested(:tracking) do + # Do not report more often then every second, this could overload the system + required(:interval) { |val| val.is_a?(Integer) && val >= 1_000 } + + nested(:consumers) do + required(:reporter) { |val| !val.nil? } + required(:sampler) { |val| !val.nil? } + required(:listeners) { |val| val.is_a?(Array) } + end + + nested(:producers) do + required(:reporter) { |val| !val.nil? } + required(:sampler) { |val| !val.nil? } + required(:listeners) { |val| val.is_a?(Array) } + end + end + + nested(:processing) do + required(:active) { |val| [true, false].include?(val) } + required(:consumer_group) { |val| val.is_a?(String) && TOPIC_REGEXP.match?(val) } + # Do not update data more often not to overload and not to generate too much data + required(:interval) { |val| val.is_a?(Integer) && val >= 1_000 } + end + + nested(:ui) do + nested(:sessions) do + required(:key) { |val| val.is_a?(String) && !val.empty? } + required(:secret) { |val| val.is_a?(String) && val.length >= 64 } + end + + required(:cache) { |val| !val.nil? } + required(:per_page) { |val| val.is_a?(Integer) && val >= 1 && val <= 100 } + required(:visibility_filter) { |val| !val.nil? } + end + end + end + end +end diff --git a/lib/karafka/web/management/clean_boot_file.rb b/lib/karafka/web/management/clean_boot_file.rb index c3069d41..ece3cfd0 100644 --- a/lib/karafka/web/management/clean_boot_file.rb +++ b/lib/karafka/web/management/clean_boot_file.rb @@ -20,6 +20,7 @@ def call File.write(Karafka.boot_file, karafka_rb.join) puts "Karafka boot file #{successfully} updated." + puts 'Make sure to remove configuration and other customizations as well.' else puts 'Karafka Web UI components not found in the boot file.' end diff --git a/lib/karafka/web/management/extend_boot_file.rb b/lib/karafka/web/management/extend_boot_file.rb index 8046ff54..de36b5de 100644 --- a/lib/karafka/web/management/extend_boot_file.rb +++ b/lib/karafka/web/management/extend_boot_file.rb @@ -8,6 +8,17 @@ class ExtendBootFile < Base # Code that is needed in the `karafka.rb` to connect Web UI to Karafka ENABLER_CODE = 'Karafka::Web.enable!' + # Template with initial Web UI configuration + # Session secret needs to be set per user and per env + SETUP_TEMPLATE = <<~CONFIG.freeze + Karafka::Web.setup do |config| + # You may want to set it per ENV. This value was randomly generated. + config.ui.sessions.secret = '#{SecureRandom.hex(32)}' + end + + #{ENABLER_CODE} + CONFIG + # Adds needed code def call if File.read(Karafka.boot_file).include?(ENABLER_CODE) @@ -15,7 +26,7 @@ def call else puts 'Updating the Karafka boot file...' File.open(Karafka.boot_file, 'a') do |f| - f << "\n#{ENABLER_CODE}\n" + f << "\n#{SETUP_TEMPLATE}\n" end puts "Karafka boot file #{successfully} updated." end diff --git a/lib/karafka/web/ui/base.rb b/lib/karafka/web/ui/base.rb index d00d50db..02a1b083 100644 --- a/lib/karafka/web/ui/base.rb +++ b/lib/karafka/web/ui/base.rb @@ -27,6 +27,11 @@ class Base < Roda plugin :error_handler plugin :not_found plugin :path + # The secret here will be reconfigured after Web UI configuration setup + # This is why we assign here a random value as it will have to be changed by the end + # user to make the Web UI work. + plugin :sessions, key: '_karafka_session', secret: SecureRandom.hex(64) + plugin :route_csrf # Based on # https://github.com/sidekiq/sidekiq/blob/ae6ca119/lib/sidekiq/web/application.rb#L8 diff --git a/spec/lib/karafka/web/contracts/config_spec.rb b/spec/lib/karafka/web/contracts/config_spec.rb new file mode 100644 index 00000000..e8ce8bfa --- /dev/null +++ b/spec/lib/karafka/web/contracts/config_spec.rb @@ -0,0 +1,172 @@ +# frozen_string_literal: true + +RSpec.describe_current do + subject(:contract) { described_class.new } + + let(:params) do + { + ttl: 5000, + topics: { + errors: 'errors-topic', + consumers: { + reports: 'reports-topic', + states: 'states-topic', + metrics: 'metrics-topic' + } + }, + tracking: { + interval: 2_000, + consumers: { + reporter: Object.new, + sampler: Object.new, + listeners: [] + }, + producers: { + reporter: Object.new, + sampler: Object.new, + listeners: [] + } + }, + processing: { + active: true, + consumer_group: 'consumer-group-topic', + interval: 3_000 + }, + ui: { + sessions: { + key: 'some_key', + secret: 'a' * 64 + }, + cache: Object.new, + per_page: 50, + visibility_filter: Object.new + } + } + end + + context 'when all values are valid' do + it 'is valid' do + expect(contract.call(params)).to be_success + end + end + + context 'when ttl is not numeric' do + before { params[:ttl] = 'string_value' } + + it { expect(contract.call(params)).not_to be_success } + end + + context 'when validating topics topics' do + context 'when errors topic does not match the regexp' do + before { params[:topics][:errors] = 'invalid topic!' } + + it { expect(contract.call(params)).not_to be_success } + end + + context 'when validating consumer scoped fields' do + %i[ + reports + states + metrics + ].each do |field| + context "when #{field} does not match the regexp" do + before { params[:topics][:consumers][field] = 'invalid topic!' } + + it { expect(contract.call(params)).not_to be_success } + end + end + end + end + + context 'when validating tracking related settings' do + context 'when interval is less than 1000' do + before { params[:tracking][:interval] = 999 } + + it { expect(contract.call(params)).not_to be_success } + end + + context 'when interval is not an integer' do + before { params[:tracking][:interval] = 1000.5 } + + it { expect(contract.call(params)).not_to be_success } + end + + %i[consumers producers].each do |entity| + context "when checking #{entity} scoped data" do + %i[reporter sampler].each do |field| + context "when #{field} is nil" do + before { params[:tracking][entity][field] = nil } + + it { expect(contract.call(params)).not_to be_success } + end + end + + context 'when listeners is not an array' do + before { params[:tracking][entity][:listeners] = 'not_an_array' } + + it { expect(contract.call(params)).not_to be_success } + end + end + end + end + + context 'when validating processing related settings' do + context 'when active is not a boolean' do + before { params[:processing][:active] = 'maybe' } + + it { expect(contract.call(params)).not_to be_success } + end + + context 'when consumer_group does not match the regexp' do + before { params[:processing][:consumer_group] = 'invalid topic!' } + + it { expect(contract.call(params)).not_to be_success } + end + + context 'when interval is less than 1000' do + before { params[:processing][:interval] = 999 } + + it { expect(contract.call(params)).not_to be_success } + end + end + + context 'when validating ui related settings' do + context 'when validating sessions related settings' do + context 'when key is empty' do + before { params[:ui][:sessions][:key] = '' } + + it { expect(contract.call(params)).not_to be_success } + end + + context 'when secret is less than 64 characters long' do + before { params[:ui][:sessions][:secret] = 'short' } + + it { expect(contract.call(params)).not_to be_success } + end + end + + context 'when cache is nil' do + before { params[:ui][:cache] = nil } + + it { expect(contract.call(params)).not_to be_success } + end + + context 'when per_page is more than 100' do + before { params[:ui][:per_page] = 101 } + + it { expect(contract.call(params)).not_to be_success } + end + + context 'when per_page is less than 1' do + before { params[:ui][:per_page] = 0 } + + it { expect(contract.call(params)).not_to be_success } + end + + context 'when visibility_filter is nil' do + before { params[:ui][:visibility_filter] = nil } + + it { expect(contract.call(params)).not_to be_success } + end + end +end diff --git a/spec/lib/karafka/web/management/extend_boot_file_spec.rb b/spec/lib/karafka/web/management/extend_boot_file_spec.rb index 8a9ef8c4..d513368b 100644 --- a/spec/lib/karafka/web/management/extend_boot_file_spec.rb +++ b/spec/lib/karafka/web/management/extend_boot_file_spec.rb @@ -34,9 +34,16 @@ before { File.write(boot_file, content) } - it 'expect to add it at the end' do + it 'expect to add the enabled' do extend_boot_file - expect(File.read(boot_file)).to eq("#{content}\nKarafka::Web.enable!\n") + expect(File.read(boot_file)).to include("\nKarafka::Web.enable!\n") + end + + it 'expect to add the configurator' do + extend_boot_file + updated = File.read(boot_file) + expect(updated).to include('config.ui.sessions.secret') + expect(updated).to include('Karafka::Web.setup do |config|') end end end