Move mirror configuration into it's own dialog.

src/clients/inst_casp_admin_role.rb

@@ -1,2 +1,5 @@
 require "y2caasp/clients/admin_role_dialog"
+require "y2caasp/clients/admin_role_mirror_dialog"
+
 Y2Caasp::AdminRoleDialog.new.run
+Y2Caasp::AdminRoleMirrorDialog.new.run

src/lib/y2caasp/clients/admin_role_dialog.rb

@@ -21,7 +21,6 @@
 
 require "yast"
 require "cwm/dialog"
-require "y2caasp/clients/admin_role_mirror_subdialog"
 require "y2caasp/widgets/ntp_server"
 require "y2caasp/dhcp_ntp_servers"
 
@@ -32,14 +31,12 @@ module Y2Caasp
   #   - the configuration of a mirror for registry.suse.com
   class AdminRoleDialog < CWM::Dialog
     include DhcpNtpServers
-    attr_reader :subdialog
 
     def initialize
       textdomain "caasp"
 
       Yast.import "Product"
       Yast.import "ProductFeatures"
-      @subdialog = AdminRoleMirrorSubDialog.new
       super
     end
 
@@ -62,8 +59,7 @@ def contents
           # preselect the servers from the DHCP response
           VBox(
             Y2Caasp::Widgets::NtpServer.new(ntp_servers),
-            VSpacing(2),
-            @subdialog.contents
+            VSpacing(2)
           )
         )
       )

src/lib/y2caasp/clients/admin_role_mirror_subdialog.rb → src/lib/y2caasp/clients/admin_role_mirror_dialog.rb

@@ -4,6 +4,7 @@
 
 require "yast"
 require "cwm/widget"
+require "y2caasp/ssl_certificate"
 require "y2caasp/widgets/container_registry_fingerprint"
 require "y2caasp/widgets/container_registry_mirror"
 require "y2caasp/widgets/container_registry_setup_mirror"
@@ -15,7 +16,7 @@ module Y2Caasp
   # Aggregate dialog for mirror configuration. It allows the user to specify the
   # mirror URL, as well as verify the certificate, should the mirror be served
   # over https
-  class AdminRoleMirrorSubDialog
+  class AdminRoleMirrorDialog < CWM::Dialog
     include Yast::UIShortcuts
     include Yast::I18n
     attr_reader :checkbox, :mirror, :fingerprint, :fingerprint_verify, :setup_mirror
@@ -46,6 +47,15 @@ def contents
       )
     end
 
+    def run
+      ret = super()
+      if ret == :next
+        ensure_url_prefix
+        download_certificate
+      end
+      ret
+    end
+
     def handle_mirror_setup(sender)
       if sender.checked?
         enable
@@ -69,9 +79,8 @@ def handle_insecure_checkbox(sender)
     def handle_certificate_verification(_sender)
       secure = @checkbox.unchecked?
       return unless role && secure
-      # There is no focus lost event on which to download the certificate, so it
-      # might not be available here yet and must be downloaded explicitly
-      @mirror.download_certificate if role["registry_certificate"].nil?
+
+      download_certificate
 
       if role["registry_certificate"].verify_sha1_fingerprint(@fingerprint)
         Yast::Popup.Notify(
@@ -104,6 +113,24 @@ def disable
       @fingerprint_verify.disable
     end
 
+    # Ensure that the mirror the customer entered, starts with the correct prefix,
+    # based on `secure` or `non-secure` selection.
+    # This is required as the customer can remove the prefix from the textfield and
+    # enter the incorrect one.
+    def ensure_url_prefix
+      # If no registry is to be setup we don't need to fix the URL
+      return unless role && role["registry_setup"]
+      secure = role["registry_secure"]
+      prefix = secure ? "https://" : "http://"
+      url = role["registry_mirror"].gsub(/https?:\/\//, "")
+      role["registry_mirror"] = prefix + url
+    end
+
+    def download_certificate
+      return unless role && role["registry_secure"]
+      role["registry_certificate"] = SSLCertificate.download(value)
+    end
+
     # All other widgets have this
     def role
       ::Installation::SystemRole.current_role

src/lib/y2caasp/ssl_certificate.rb

@@ -30,7 +30,6 @@ def to_der
 
     class << self
       def download(url)
-        return if insecure_url(url)
         ctx = OpenSSL::SSL::SSLContext.new
         sock = TCPSocket.new(url.gsub(/^https?:\/\//, ""), 443)
         ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx)
@@ -44,12 +43,6 @@ def download(url)
           SSLCertificate.new(nil)
         end
       end
-
-    private
-
-      def insecure_url(value)
-        !(/^http:\/\// =~ value).nil?
-      end
     end
 
   private

src/lib/y2caasp/widgets/container_registry_mirror.rb

@@ -23,8 +23,6 @@
 require "cwm/widget"
 require "installation/system_role"
 
-require "y2caasp/ssl_certificate"
-
 module Y2Caasp
   module Widgets
     # This widget is responsible to validate and store the registry mirror.
@@ -49,7 +47,6 @@ def store
         raise("No role selected") unless role
 
         role["registry_mirror"] = value unless empty_url(value)
-        download_certificate
       end
 
       # The input field is initialized with previous stored value
@@ -76,11 +73,6 @@ def validate
         false
       end
 
-      def download_certificate
-        return if empty_url(value)
-        role["registry_certificate"] = SSLCertificate.download(value)
-      end
-
       def opt
         [:disabled]
       end

src/lib/y2caasp/widgets/container_registry_setup_mirror.rb

@@ -3,6 +3,7 @@
 
 module Y2Caasp
   module Widgets
+    # Widgets allows the user to select if to configure a mirror at all or not.
     class SetupMirrorCheckBox < CWM::CheckBox
       def initialize
         @observers = []
@@ -40,7 +41,7 @@ def opt
         [:notify]
       end
 
-      private
+    private
 
       def role
         ::Installation::SystemRole.current_role

src/lib/y2system_role_handlers/dashboard_role_finish.rb

@@ -73,7 +73,7 @@ def update_chrony_conf
     end
 
     def update_registry_conf
-      return unless role["registry_mirror"] && role["registry_setup"]
+      return unless role["registry_setup"]
       mirror_conf = ::Y2Caasp::CFA::MirrorConf.new
       mirror_conf.mirror_url = role["registry_mirror"]
       if role["registry_certificate"]

test/lib/widgets/container_registry_mirror_test.rb

@@ -118,6 +118,14 @@
       end
     end
 
+    context "when only a domain name is provided" do
+      let(:value) { "registry.suse.de" }
+
+      it "returns true" do
+        expect(widget.validate).to eq(true)
+      end
+    end
+
     context "when no value is provided" do
       let(:value) { "" }
 

test/lib/y2caasp/clients/admin_role_dialog_test.rb

@@ -3,7 +3,6 @@
 require_relative "../../../test_helper.rb"
 require_relative "role_dialog_examples"
 require "cwm/rspec"
-require "openssl"
 
 require "y2caasp/ssl_certificate"
 require "y2caasp/clients/admin_role_dialog.rb"
@@ -52,113 +51,5 @@
         subject.run
       end
     end
-
-    context "using an insecure registry" do
-      it "disables some inputs" do
-        # It seems that actual interaction with the widgets will not work in tests.
-        # Instead the expected events have to be mocked and tested if they are triggered.
-        expect(Yast::UI).to receive(:ChangeWidget).with(
-          Id(subject.subdialog.fingerprint.widget_id),
-          :Enabled,
-          false
-        )
-        expect(Yast::UI).to receive(:ChangeWidget).with(
-          Id(subject.subdialog.fingerprint_verify.widget_id),
-          :Enabled,
-          false
-        )
-        expect(Yast::UI).to receive(:ChangeWidget).with(
-          Id(subject.subdialog.mirror.widget_id),
-          :Value,
-          "http://"
-        )
-        allow(subject.subdialog.checkbox).to receive(:checked?).and_return(true)
-        allow(subject.subdialog.mirror).to receive(:value).and_return("https://")
-        subject.run
-        subject.subdialog.handle_insecure_checkbox(subject.subdialog.checkbox)
-      end
-    end
-
-    context "using an secure registry" do
-      it "enables some inputs" do
-        # It seems that actual interaction with the widgets will not work in tests.
-        # Instead the expected events have to be mocked and tested if they are triggered.
-        expect(Yast::UI).to receive(:ChangeWidget).with(
-          Id(subject.subdialog.fingerprint.widget_id),
-          :Enabled,
-          true
-        )
-        expect(Yast::UI).to receive(:ChangeWidget).with(
-          Id(subject.subdialog.fingerprint_verify.widget_id),
-          :Enabled,
-          true
-        )
-        expect(Yast::UI).to receive(:ChangeWidget).with(
-          Id(subject.subdialog.mirror.widget_id),
-          :Value,
-          "https://"
-        )
-        allow(subject.subdialog.checkbox).to receive(:checked?).and_return(false)
-        allow(subject.subdialog.mirror).to receive(:value).and_return("https://")
-        subject.run
-        subject.subdialog.handle_insecure_checkbox(subject.subdialog.checkbox)
-      end
-
-      it "can verify a valid registry certificate" do
-        expect(Yast::Popup).to receive(:Notify)
-        allow(Installation::SystemRole).to receive(:current_role).and_return(role)
-        allow(subject.subdialog.checkbox).to receive(:unchecked?).and_return(true)
-        allow(subject.subdialog.mirror).to receive(:value).and_return("https://test.de")
-        allow(subject.subdialog.fingerprint).to receive(:value).and_return(
-          "e75342ccce01f9e7ac3be3341a3a97618c373bb0"
-        )
-        allow(Y2Caasp::SSLCertificate).to receive(:download).and_return(
-          Y2Caasp::SSLCertificate.new(
-            OpenSSL::X509::Certificate.new(File.read(File.join(FIXTURES_PATH, "certificate.pem")))
-          )
-        )
-        subject.run
-        subject.subdialog.handle_certificate_verification(nil)
-      end
-
-      it "can verify an invalid registry certificate" do
-        expect(Yast::Popup).to receive(:Error)
-        allow(Installation::SystemRole).to receive(:current_role).and_return(role)
-        allow(subject.subdialog.checkbox).to receive(:unchecked?).and_return(true)
-        allow(subject.subdialog.mirror).to receive(:value).and_return("https://test.de")
-        allow(subject.subdialog.fingerprint).to receive(:value).and_return(
-          "wrong fingerprint"
-        )
-        allow(Y2Caasp::SSLCertificate).to receive(:download).and_return(
-          Y2Caasp::SSLCertificate.new(
-            OpenSSL::X509::Certificate.new(File.read(File.join(FIXTURES_PATH, "certificate.pem")))
-          )
-        )
-        subject.run
-        subject.subdialog.handle_certificate_verification(nil)
-      end
-
-      it "can disallow all input if no mirror is to be setup" do
-        expect(subject.subdialog.checkbox).to receive(:disable)
-        expect(subject.subdialog.mirror).to receive(:disable)
-        expect(subject.subdialog.fingerprint).to receive(:disable)
-        expect(subject.subdialog.fingerprint_verify).to receive(:disable)
-        allow(subject.subdialog.setup_mirror).to receive(:checked?).and_return(false)
-        allow(Installation::SystemRole).to receive(:current_role).and_return(role)
-        subject.run
-        subject.subdialog.handle_mirror_setup(subject.subdialog.setup_mirror)
-      end
-
-      it "can allow all input if a mirror is to be setup" do
-        expect(subject.subdialog.checkbox).to receive(:enable)
-        expect(subject.subdialog.mirror).to receive(:enable)
-        expect(subject.subdialog.fingerprint).to receive(:enable)
-        expect(subject.subdialog.fingerprint_verify).to receive(:enable)
-        allow(subject.subdialog.setup_mirror).to receive(:checked?).and_return(true)
-        allow(Installation::SystemRole).to receive(:current_role).and_return(role)
-        subject.run
-        subject.subdialog.handle_mirror_setup(subject.subdialog.setup_mirror)
-      end
-    end
   end
 end