From cc0246dfa5a8669f9476c5e4b40c1b30ee089c32 Mon Sep 17 00:00:00 2001 From: Luis Caparroz Date: Mon, 5 Feb 2024 16:49:02 +0100 Subject: [PATCH 1/9] Catch Net::ReadTimeout exceptions on SCC wizard This commit adds proper timeout exception treatment on the method `RMT::WizardSCCPage#scc_credentials_valid?`: * Before: an "Internal error" message would be shown if a request timed out when contacting SCC to validate the credentials. * Now: a popup dialog will be shown, giving the user the choice to retry the request or cancel the operation. Additionally, the request has been enriched with a proper "user agent" to allow better debugging with SCC. --- spec/rmt/wizard_scc_page_spec.rb | 71 ++++++++++++++++++++++++++------ src/lib/rmt/wizard_scc_page.rb | 23 +++++++++-- 2 files changed, 78 insertions(+), 16 deletions(-) diff --git a/spec/rmt/wizard_scc_page_spec.rb b/spec/rmt/wizard_scc_page_spec.rb index d9a06b4..d5289fd 100644 --- a/spec/rmt/wizard_scc_page_spec.rb +++ b/spec/rmt/wizard_scc_page_spec.rb @@ -127,26 +127,73 @@ expect(Yast::UI).to receive(:CloseDialog) expect_any_instance_of(Net::HTTP::Get).to receive(:basic_auth).with(config['scc']['username'], config['scc']['password']) - - expect(Net::HTTP).to receive(:start).and_return(response_double) - expect(response_double).to receive(:code).and_return(response_code) end - let(:response_double) { instance_double(Net::HTTPResponse) } + context 'when the request completes without errors' do + before do + expect(Net::HTTP).to receive(:start).and_return(response_double) + expect(response_double).to receive(:code).and_return(response_code) + end + + let(:response_double) { instance_double(Net::HTTPResponse) } - context 'when HTTP response code is 200' do - let(:response_code) { '200' } + context 'when HTTP response code is 200' do + let(:response_code) { '200' } - it 'returns true' do - expect(scc_page.scc_credentials_valid?).to be(true) + it 'returns true' do + expect(scc_page.scc_credentials_valid?).to be(true) + end + end + + context 'when HTTP response code is not 200' do + let(:response_code) { '401' } + + it 'returns false' do + expect(scc_page.scc_credentials_valid?).to be(false) + end end end - context 'when HTTP response code is not 200' do - let(:response_code) { '401' } + context 'when SCC times out and the user chooses not to try again' do + before do + expect(Yast::Popup).to receive(:ErrorAnyQuestion).and_return(false) + expect(Net::HTTP).to receive(:start).and_raise(Net::ReadTimeout) + end + + context 'and the user chooses to not try again' do + it 'returns false' do + expect(scc_page.scc_credentials_valid?).to be(false) + end + end + end + + context 'when SCC times out and the user chooses to try again' do + before do + expect(Yast::Popup).to receive(:ErrorAnyQuestion).and_return(true) + expect(Net::HTTP).to receive(:start).and_raise(Net::ReadTimeout) + expect(Net::HTTP).to receive(:start).and_return(response_double) + end + + let(:response_double) { instance_double(Net::HTTPResponse) } + + context 'when SCC responds quickly and the HTTP response code is 200' do + before do + expect(response_double).to receive(:code).and_return(200) + end + + it 'returns true' do + expect(scc_page.scc_credentials_valid?).to be(true) + end + end + + context 'when SCC responds quickly and the HTTP response code is not 200' do + before do + expect(response_double).to receive(:code).and_return(401) + end - it 'returns false' do - expect(scc_page.scc_credentials_valid?).to be(false) + it 'returns false' do + expect(scc_page.scc_credentials_valid?).to be(false) + end end end end diff --git a/src/lib/rmt/wizard_scc_page.rb b/src/lib/rmt/wizard_scc_page.rb index d9d29e5..510946b 100644 --- a/src/lib/rmt/wizard_scc_page.rb +++ b/src/lib/rmt/wizard_scc_page.rb @@ -26,6 +26,8 @@ module RMT; end class RMT::WizardSCCPage < Yast::Client include ::UI::EventDispatcher + YAST_RMT_USER_AGENT = 'yast2-rmt'.freeze + def initialize(config) textdomain 'rmt' @config = config @@ -126,14 +128,27 @@ def scc_credentials_valid? ) ) - uri = URI('https://scc.suse.com/connect/organizations/systems') + uri = URI('https://scc.suse.com/connect/organizations/orders') req = Net::HTTP::Get.new(uri) req.basic_auth(@config['scc']['username'], @config['scc']['password']) - - res = Net::HTTP.start(uri.host, uri.port, use_ssl: true) { |http| http.request(req) } + req['User-Agent'] = YAST_RMT_USER_AGENT + + valid_credentials = nil + while valid_credentials.nil? + begin + res = Net::HTTP.start(uri.host, uri.port, use_ssl: true) { |http| http.request(req) } + valid_credentials = (res.code.to_i == 200) + rescue Net::ReadTimeout + break valid_credentials = false unless Popup.ErrorAnyQuestion( + _('Request Timeout'), + _("The request to SCC timed out.\n\nWould you like to try again?"), + _('Retry'), _('Cancel'), :focus_yes + ) + end + end UI.CloseDialog - res.code.to_i == 200 + valid_credentials end end From b93c5b9dc5321b953b7d25e918fa1d0829703931 Mon Sep 17 00:00:00 2001 From: Luis Caparroz Date: Thu, 8 Feb 2024 16:58:33 +0100 Subject: [PATCH 2/9] Updates README with Docker dev setup instructions --- README.md | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index e792755..8ae4e4b 100644 --- a/README.md +++ b/README.md @@ -22,13 +22,35 @@ There different ways to run the module: * `DISPLAY= rake run` — forces to run in ncurses mode; * `Y2DIR=src/ /usr/sbin/yast2 --ncurses rmt` — same as above. +#### Docker Setup + +To run the module within a Docker container: + +1. Select a proper Docker container image for YaST from https://registry.opensuse.org, according to the branch, e.g.: + + * On branch `master`, use `yast/head/containers_tumbleweed/yast-ruby`. + * On branch `SLE-15-SP6`, use `yast/sle-15/sp6/containers/yast-ruby`. + +2. Run the Docker container with access to the localhost network with the chosen distribution and version: + + ```shell + docker run --network host -v "$(pwd):/usr/src/app" -w "/usr/src/app" -it registry.opensuse.org/yast/sle-15/sp6/containers/yast-ruby sh + ``` + +3. On the container, install the `rmt-server` package: + + ```shell + zypper --non-interactive install rmt-server + ``` + +4. Run the YaST RMT module with `rake run` or through the other ways previously described. + ### Running tests It is possible to run the specs in a Docker container: -``` -docker build -t yast-rmt-image . -docker run -it yast-rmt-image rspec +```shell +docker run -v "$(pwd):/usr/src/app" -w "/usr/src/app" -it registry.opensuse.org/yast/sle-15/sp6/containers/yast-ruby rake test:unit ``` ### Resources From 04dd0fb118fa3e0f382affbbbce29ab827de2b08 Mon Sep 17 00:00:00 2001 From: Luis Caparroz Date: Thu, 8 Feb 2024 17:15:46 +0100 Subject: [PATCH 3/9] Bump to version 1.3.5 --- package/yast2-rmt.changes | 9 +++++++++ package/yast2-rmt.spec | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/package/yast2-rmt.changes b/package/yast2-rmt.changes index 877987b..53ee620 100644 --- a/package/yast2-rmt.changes +++ b/package/yast2-rmt.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Thu Feb 8 16:10:29 UTC 2024 - Luis Caparroz + +- Adds UI dialog to allow the user to retry the SCC credential validation when + the request times out (bsc#1218084). +- Adds HTTP User-Agent to requests to the SCC API and changes the enpoint for + credential validation. +- Version 1.3.5 + ------------------------------------------------------------------- Thu Jun 9 10:47:13 UTC 2022 - Dominique Leuenberger diff --git a/package/yast2-rmt.spec b/package/yast2-rmt.spec index 4a12bf0..f5a574a 100644 --- a/package/yast2-rmt.spec +++ b/package/yast2-rmt.spec @@ -17,7 +17,7 @@ Name: yast2-rmt -Version: 1.3.4 +Version: 1.3.5 Release: 0 BuildRoot: %{_tmppath}/%{name}-%{version}-build From 89e919c6ba0cb98f7882ce158f603a3e495c23f0 Mon Sep 17 00:00:00 2001 From: Luis Caparroz Date: Wed, 14 Feb 2024 11:01:16 +0100 Subject: [PATCH 4/9] Add package version to user agent To enhance the 'User-Agent' used in the request headers, this commit adds the module constant `RMT::Version`, which must always match the package version specified in 'package/yast2-rmt.spec'. Additionally, a unit test was created to ensure the versions always match each other; if there is a version bump in packge spec file, the RSpec tests will break if 'src/lib/rmt.rb' is not updated accordingly. --- package/yast2-rmt.spec | 1 + spec/rmt_spec.rb | 44 ++++++++++++++++++++++++++++++++++ src/lib/rmt.rb | 21 ++++++++++++++++ src/lib/rmt/wizard_scc_page.rb | 3 ++- 4 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 spec/rmt_spec.rb create mode 100644 src/lib/rmt.rb diff --git a/package/yast2-rmt.spec b/package/yast2-rmt.spec index f5a574a..ef45b53 100644 --- a/package/yast2-rmt.spec +++ b/package/yast2-rmt.spec @@ -60,6 +60,7 @@ rake install DESTDIR="%{buildroot}" %defattr(-,root,root) %{yast_dir}/clients/*.rb %{yast_dir}/lib/rmt +%{yast_dir}/lib/rmt.rb %{yast_desktopdir}/rmt.desktop %{yast_dir}/data/rmt diff --git a/spec/rmt_spec.rb b/spec/rmt_spec.rb new file mode 100644 index 0000000..7f7f0f2 --- /dev/null +++ b/spec/rmt_spec.rb @@ -0,0 +1,44 @@ +# Copyright (c) 2024 SUSE LLC. +# All Rights Reserved. + +# This program is free software; you can redistribute it and/or +# modify it under the terms of version 2 or 3 of the GNU General +# Public License as published by the Free Software Foundation. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program; if not, contact SUSE LLC. + +# To contact SUSE about this file by physical or electronic mail, +# you may find current contact information at www.suse.com + +require 'rmt' + +describe RMT do + describe '.VERSION' do + subject(:version) { RMT::VERSION } + + let(:package_version) do + filename = './package/yast2-rmt.spec' + version = nil + + File.foreach(filename) do |line| + line.match(/^Version:\s+(?(\d+\.?){3})$/) do |match| + version ||= match.named_captures['version'] + end + end + + raise "'#{filename}' does not include any line matching the expected package version format." if version.nil? + + version + end + + it 'returns the same version as specified in the package spec file' do + expect(version).to eq package_version + end + end +end diff --git a/src/lib/rmt.rb b/src/lib/rmt.rb new file mode 100644 index 0000000..20ced44 --- /dev/null +++ b/src/lib/rmt.rb @@ -0,0 +1,21 @@ +# Copyright (c) 2024 SUSE LLC. +# All Rights Reserved. + +# This program is free software; you can redistribute it and/or +# modify it under the terms of version 2 or 3 of the GNU General +# Public License as published by the Free Software Foundation. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program; if not, contact SUSE LLC. + +# To contact SUSE about this file by physical or electronic mail, +# you may find current contact information at www.suse.com + +module RMT + VERSION = '1.3.5'.freeze +end diff --git a/src/lib/rmt/wizard_scc_page.rb b/src/lib/rmt/wizard_scc_page.rb index 510946b..248844f 100644 --- a/src/lib/rmt/wizard_scc_page.rb +++ b/src/lib/rmt/wizard_scc_page.rb @@ -18,6 +18,7 @@ require 'uri' require 'net/http' +require 'rmt' require 'rmt/utils' require 'ui/event_dispatcher' @@ -26,7 +27,7 @@ module RMT; end class RMT::WizardSCCPage < Yast::Client include ::UI::EventDispatcher - YAST_RMT_USER_AGENT = 'yast2-rmt'.freeze + YAST_RMT_USER_AGENT = "yast2-rmt/#{RMT::VERSION}".freeze def initialize(config) textdomain 'rmt' From 751d2e171baddf772f8032f4bc00872c70af0192 Mon Sep 17 00:00:00 2001 From: Luis Caparroz Date: Mon, 18 Mar 2024 15:26:52 +0100 Subject: [PATCH 5/9] Update bundler version to match Tumbleweed's This commit updates the gem version of 'bundler' to match the one provided in Tumbleweed. --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 68e3809..cb9f799 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -42,4 +42,4 @@ DEPENDENCIES yast-rake (~> 0.2.20) BUNDLED WITH - 1.16.1 + 2.5.3 From 3f506cea96925b662130b75fc380b3c33a0be054 Mon Sep 17 00:00:00 2001 From: Luis Caparroz Date: Mon, 18 Mar 2024 16:34:46 +0100 Subject: [PATCH 6/9] Remove 'scc-codestyle' from dependencies This commit removes the gem 'scc-codestyle' from the project, and adds 'rubocop-rspec' as a direct dependency to continue having the a similar set of linter rules. The 'scc-codestyle' version in use was too outdated, and updating it to a newer one would require a heavy restyling of the code. --- .github/workflows/ci.yml | 3 --- .rubocop.yml | 14 +++----------- Gemfile | 2 +- Gemfile.lock | 5 +---- 4 files changed, 5 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9d44200..63c592e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -53,9 +53,6 @@ jobs: - name: Git Checkout uses: actions/checkout@v2 - - name: Add SCC Codestyle - run: gem install --no-document scc-codestyle -v 0.1.4 - - name: Proper Rubocop Version run: rm /etc/alternatives/rubocop && ln -s /usr/lib64/ruby/gems/*/gems/rubocop-0.52.1/bin/rubocop diff --git a/.rubocop.yml b/.rubocop.yml index ff18c7f..0463e49 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,24 +1,16 @@ -# Rubocop version is locked in the dependencies of scc-codestyle gem. -# The version of scc-codestyle gem is specified in the Dockerfile. -inherit_gem: - scc-codestyle: - - default.yml +require: + - rubocop-rspec inherit_from: .rubocop_todo.yml AllCops: - Include: - - Gemfile - - config.ru + NewCops: disable Exclude: - .bundle/**/* - vendor/**/* - package/* - Rakefile -Rails: - Enabled: false - # Impossible to satisfy these rules in specs RSpec/ExpectInHook: Enabled: false diff --git a/Gemfile b/Gemfile index c9cf8f5..622a782 100644 --- a/Gemfile +++ b/Gemfile @@ -4,5 +4,5 @@ gem 'yast-rake', '~> 0.2.20' group :development do gem 'gettext' - gem 'scc-codestyle', '~> 0.1.4' + gem 'rubocop-rspec', '~> 1.19.0' end diff --git a/Gemfile.lock b/Gemfile.lock index cb9f799..359b19c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -24,9 +24,6 @@ GEM rubocop-rspec (1.19.0) rubocop (>= 0.51.0) ruby-progressbar (1.9.0) - scc-codestyle (0.1.4) - rubocop (= 0.52.1) - rubocop-rspec (= 1.19) text (1.3.1) unicode-display_width (1.3.0) yast-rake (0.2.20) @@ -38,7 +35,7 @@ PLATFORMS DEPENDENCIES gettext - scc-codestyle (~> 0.1.4) + rubocop-rspec (~> 1.19.0) yast-rake (~> 0.2.20) BUNDLED WITH From c463323fb03dae66e656271c3f44965441497dd3 Mon Sep 17 00:00:00 2001 From: Luis Caparroz Date: Wed, 20 Mar 2024 13:50:20 +0100 Subject: [PATCH 7/9] Configure rules for Rubocop After removing the gem 'scc-codestyle' (outdated version), Rubocop's default configuration caused a lot of warnings/errors when linting the project. This commit adds rules to 'rubocop.yml' so that the current code passes the linting process in the CI. Though I agree some of the rules are important to keep a good style for the project, the idea is just to adjust our CI process for Tumbleweed at the moment. --- .rubocop.yml | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 0463e49..39c140d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -11,6 +11,105 @@ AllCops: - package/* - Rakefile +Layout/EmptyLineAfterGuardClause: + Enabled: false + +Layout/EmptyLinesAroundMethodBody: + Enabled: false + +Layout/EmptyLines: + Enabled: false + +Layout/HashAlignment: + Enabled: false + +Layout/LineLength: + Enabled: true + Max: 160 + +Lint/ErbNewArguments: + Enabled: false + +Lint/MissingSuper: + Enabled: false + +Lint/MixedRegexpCaptureTypes: + Enabled: false + +Metrics/AbcSize: + Enabled: true + Max: 80 + +Metrics/BlockLength: + Enabled: false + +Metrics/ClassLength: + Enabled: true + Max: 160 + +Metrics/MethodLength: + Enabled: true + Max: 50 + +Metrics/PerceivedComplexity: + Enabled: true + Max: 12 + +Naming/MethodParameterName: + Enabled: false + +Naming/VariableNumber: + Enabled: false + +Style/ClassAndModuleChildren: + Enabled: false + +Style/ClassEqualityComparison: + Enabled: false + +Style/Documentation: + Enabled: false + +Style/ExpandPathArguments: + Enabled: false + +Style/FormatString: + Enabled: false + +Style/FrozenStringLiteralComment: + Enabled: false + +Style/HashTransformValues: + Enabled: false + +Style/NumericPredicate: + Enabled: false + +Style/ParenthesesAroundCondition: + Enabled: false + +Style/RedundantBegin: + Enabled: false + +Style/RedundantParentheses: + Enabled: false + +Style/StringConcatenation: + Enabled: false + +Style/TernaryParentheses: + Enabled: false + +Style/WordArray: + Enabled: false + +RSpec/AnyInstance: + Enabled: false + +RSpec/ExampleLength: + Enabled: true + Max: 16 + # Impossible to satisfy these rules in specs RSpec/ExpectInHook: Enabled: false From 84fc4b229a6efbcb917eea678993b6a4d4e6bbc8 Mon Sep 17 00:00:00 2001 From: Luis Caparroz Date: Wed, 20 Mar 2024 14:24:53 +0100 Subject: [PATCH 8/9] Bump Rubocop version This commit bumps the Rubocop gem version to the latest one that is supported for ruby 2.5 (system version for SLES 15 SP6), according to the information available at: https://docs.rubocop.org/rubocop/compatibility.html --- Gemfile | 1 + Gemfile.lock | 32 ++++++++++++++++++++------------ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/Gemfile b/Gemfile index 622a782..bddf7e9 100644 --- a/Gemfile +++ b/Gemfile @@ -4,5 +4,6 @@ gem 'yast-rake', '~> 0.2.20' group :development do gem 'gettext' + gem 'rubocop', '~> 1.28.0' gem 'rubocop-rspec', '~> 1.19.0' end diff --git a/Gemfile.lock b/Gemfile.lock index 359b19c..94ffd3b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,31 +1,38 @@ GEM remote: https://rubygems.org/ specs: - ast (2.4.0) + ast (2.4.2) gettext (3.2.9) locale (>= 2.0.5) text (>= 1.3.0) locale (2.1.2) packaging_rake_tasks (1.4.4) rake - parallel (1.12.1) - parser (2.5.0.0) - ast (~> 2.4.0) - powerpack (0.1.1) - rainbow (3.0.0) + parallel (1.24.0) + parser (3.3.0.5) + ast (~> 2.4.1) + racc + racc (1.7.3) + rainbow (3.1.1) rake (13.0.1) - rubocop (0.52.1) + regexp_parser (2.9.0) + rexml (3.2.6) + rubocop (1.28.2) parallel (~> 1.10) - parser (>= 2.4.0.2, < 3.0) - powerpack (~> 0.1) + parser (>= 3.1.0.0) rainbow (>= 2.2.2, < 4.0) + regexp_parser (>= 1.8, < 3.0) + rexml + rubocop-ast (>= 1.17.0, < 2.0) ruby-progressbar (~> 1.7) - unicode-display_width (~> 1.0, >= 1.0.1) + unicode-display_width (>= 1.4.0, < 3.0) + rubocop-ast (1.31.2) + parser (>= 3.3.0.4) rubocop-rspec (1.19.0) rubocop (>= 0.51.0) - ruby-progressbar (1.9.0) + ruby-progressbar (1.13.0) text (1.3.1) - unicode-display_width (1.3.0) + unicode-display_width (2.5.0) yast-rake (0.2.20) packaging_rake_tasks (>= 1.1.4) rake @@ -35,6 +42,7 @@ PLATFORMS DEPENDENCIES gettext + rubocop (~> 1.28.0) rubocop-rspec (~> 1.19.0) yast-rake (~> 0.2.20) From d7149ffedbb290d3c512607a4f4910be6478dc09 Mon Sep 17 00:00:00 2001 From: Luis Caparroz Date: Wed, 20 Mar 2024 14:37:12 +0100 Subject: [PATCH 9/9] Add CI step to install gem dependencies --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 63c592e..f428cd4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -53,6 +53,9 @@ jobs: - name: Git Checkout uses: actions/checkout@v2 + - name: Install project dependencies + run: bundle install + - name: Proper Rubocop Version run: rm /etc/alternatives/rubocop && ln -s /usr/lib64/ruby/gems/*/gems/rubocop-0.52.1/bin/rubocop