Merge pull request #521 from teclator/fix_dashed_url

Fix dashed url (bsc#1024965)

package/yast2-installation.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Mon Feb 13 13:00:42 UTC 2017 - kanderssen@suse.com
+
+- CaaSP: do not crash when used a dashed url for the controller
+  node location (bsc#1024965)
+- 3.1.217.21
+
 -------------------------------------------------------------------
 Wed Feb  8 16:42:29 UTC 2017 - kanderssen@suse.com
 

package/yast2-installation.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-installation
-Version:        3.1.217.20
+Version:        3.1.217.21
 Release:        0
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build

src/lib/installation/system_roles/handlers.rb

@@ -19,11 +19,14 @@
 # current contact information at www.suse.com.
 # ------------------------------------------------------------------------------
 
+require "yast"
 require "installation/cfa/salt"
 
 module Installation
   module SystemRoleHandlers
     class WorkerRoleFinish
+      include Yast::Logger
+
       def self.run
         role = SystemRole.find("worker_role")
         master_conf = CFA::MinionMasterConf.new
@@ -34,7 +37,10 @@ def self.run
           log.info("The minion master.conf file does not exist, it will be created")
         end
         log.info("The controller node for this worker role is: #{master}")
-        master_conf.master = master
+        # FIXME: the cobblersettings lense does not support dashes in the url
+        # without single quotes, we need to use a custom lense for salt conf.
+        # As Salt can use also 'url' just use in case of dashed.
+        master_conf.master = master.include?("-") ? "'#{master}'" : master
         master_conf.save
       end
     end

test/roles_finish_test.rb

@@ -6,23 +6,6 @@
 require "fileutils"
 
 describe ::Installation::Clients::RolesFinish do
-  MASTER_CONF          = FIXTURES_DIR.join("minion.d/master.conf").freeze
-  MASTER_CONF_EXISTENT = FIXTURES_DIR.join("minion.d/master.conf_existent").freeze
-  MASTER_CONF_EXPECTED = FIXTURES_DIR.join("minion.d/master.conf_expected").freeze
-
-  let(:master_conf) { ::Installation::CFA::MinionMasterConf.new }
-  let(:master_conf_path) { MASTER_CONF }
-
-  after do
-    FileUtils.remove_file(MASTER_CONF, true)
-  end
-
-  before do
-    allow(Yast::Stage).to receive(:initial).and_return(true)
-    allow(subject).to receive(:master).and_return("salt_controller")
-    stub_const("Installation::CFA::MinionMasterConf::PATH", master_conf_path)
-  end
-
   describe "#title" do
     it "returns string with title" do
       expect(subject.title).to be_a ::String

test/system_role_handlers_test.rb

@@ -0,0 +1,45 @@
+#! /usr/bin/env rspec
+
+require_relative "./test_helper"
+require "installation/system_role"
+require "installation/system_roles/handlers"
+
+describe Installation::SystemRoleHandlers::WorkerRoleFinish do
+  let(:role) { instance_double("::Installation::SystemRole") }
+  let(:conf) do
+    instance_double("::Installation::CFA::MinionMasterConf", load: true, save: true)
+  end
+
+  before do
+    allow(::Installation::SystemRole).to receive("find")
+      .with("worker_role").and_return(role)
+    allow(::Installation::CFA::MinionMasterConf).to receive(:new).and_return(conf)
+  end
+
+  describe ".run" do
+    context "if the worker role controller node location contains dashes" do
+      it "surrounds the url with single quotes before save" do
+        expect(role).to receive(:[]).with("controller_node").and_return("controller-url")
+        expect(conf).to receive(:master=).with("'controller-url'")
+        described_class.run
+      end
+
+    end
+
+    context "if the worker role controller node location does not contain dashes" do
+      it "saves the url as defined" do
+        expect(role).to receive(:[]).with("controller_node").and_return("controller")
+        expect(conf).to receive(:master=).with("controller")
+        described_class.run
+      end
+    end
+
+    it "saves the controller node location into the minion master.conf file" do
+      expect(role).to receive(:[]).with("controller_node").and_return("controller")
+      expect(conf).to receive(:master=).with("controller")
+      expect(conf).to receive(:save)
+
+      described_class.run
+    end
+  end
+end