diff --git a/lib/fog/orchestration/openstack/requests/create_stack.rb b/lib/fog/orchestration/openstack/requests/create_stack.rb index bb99615b5..42abed9a3 100644 --- a/lib/fog/orchestration/openstack/requests/create_stack.rb +++ b/lib/fog/orchestration/openstack/requests/create_stack.rb @@ -34,15 +34,14 @@ def create_stack(arg1, arg2 = nil) # Templates should always: # - be strings # - contain URI references instead of relative paths. - # Passing :template_url may not work well with `get_files` and remote `type`: + # Passing :template_url may not work well with `get_file` and remote `type`: # the python client implementation in shade retrieves from :template_uri # and replaces it with :template. # see https://github.com/openstack-infra/shade/blob/master/shade/openstackcloud.py#L1201 # see https://developer.openstack.org/api-ref/orchestration/v1/index.html#create-stack file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(options[:template] || options[:template_url], options[:files]) - files = file_resolver.get_files options[:template] = file_resolver.template - options[:files] = files if files + options[:files] = file_resolver.files unless file_resolver.files.empty? request( :expects => 201, @@ -91,8 +90,7 @@ def create_stack(arg1, arg2 = nil) if options.key?(:template) || options.key?(:template_url) file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(options[:template] || options[:template_url], options[:files]) - files = file_resolver.get_files - response.body['files'] = files if files + response.body['files'] = file_resolver.files unless file_resolver.files.empty? end response diff --git a/lib/fog/orchestration/openstack/requests/update_stack.rb b/lib/fog/orchestration/openstack/requests/update_stack.rb index 3ded18bd5..9b3cb3dc7 100644 --- a/lib/fog/orchestration/openstack/requests/update_stack.rb +++ b/lib/fog/orchestration/openstack/requests/update_stack.rb @@ -34,15 +34,14 @@ def update_stack(arg1, arg2 = nil, arg3 = nil) # Templates should always: # - be strings # - contain URI references instead of relative paths. - # Passing :template_url may not work well with `get_files` and remote `type`: + # Passing :template_url may not work well with `get_file` and remote `type`: # the python client implementation in shade retrieves from :template_uri # and replaces it with :template. # see https://github.com/openstack-infra/shade/blob/master/shade/openstackcloud.py#L1201 # see https://developer.openstack.org/api-ref/orchestration/v1/index.html#create-stack file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(options[:template] || options[:template_url], options[:files]) - files = file_resolver.get_files options[:template] = file_resolver.template - options[:files] = files if files + options[:files] = file_resolver.files unless file_resolver.files.empty? request( :expects => 202, @@ -77,8 +76,7 @@ def update_stack(arg1, arg2 = nil, arg3 = nil) if options.key?(:template) || options.key?(:template_url) file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(options[:template] || options[:template_url], options[:files]) - files = file_resolver.get_files - response.body['files'] = files if files + response.body['files'] = file_resolver.files unless file_resolver.files.empty? end response = Excon::Response.new diff --git a/lib/fog/orchestration/util/recursive_hot_file_loader.rb b/lib/fog/orchestration/util/recursive_hot_file_loader.rb index 007b2da03..cf96674fe 100644 --- a/lib/fog/orchestration/util/recursive_hot_file_loader.rb +++ b/lib/fog/orchestration/util/recursive_hot_file_loader.rb @@ -23,22 +23,14 @@ class RecursiveHotFileLoader attr_reader :template def initialize(template, files = nil) - # Serialize the template hash to deep-copy it and - # avoid modifying the argument. Instead create a - # new one to be modified by get_file_contents. - # # According to https://github.com/fog/fog-openstack/blame/master/docs/orchestration.md#L122 # templates can be either String or Hash. - @template = deep_copy(template) - @files = files || {} + # If it's an Hash, we deep_copy it so the passed argument + # is not modified by get_file_contents. + template = deep_copy(template) @visited = Set.new - end - - def get_files - Fog::Logger.debug("Processing template #{@template}") - @template = get_template_contents(@template) - Fog::Logger.debug("Template processed. Populated #{@files}") - @files + @files = files || {} + @template = get_template_contents(template) end # Return string @@ -64,9 +56,9 @@ def url_join(prefix, suffix) # XXX: we could use named parameters # and better mimic heatclient implementation. def get_template_contents(template_file) - Fog::Logger.debug("get_template_contents #{template_file}") + Fog::Logger.debug("get_template_contents [#{template_file}]") - raise "template_file should be Hash or String" unless + raise "template_file should be Hash or String, not #{template_file.class.name}" unless template_file.kind_of?(String) || template_file.kind_of?(Hash) local_base_url = url_join("file:/", File.absolute_path(Dir.pwd)) @@ -232,4 +224,4 @@ def deep_copy(item) end end end -end \ No newline at end of file +end diff --git a/test/requests/orchestration/stack_files_util_helper_tests.rb b/test/requests/orchestration/stack_files_util_helper_tests.rb index 08854a4c7..62760c452 100644 --- a/test/requests/orchestration/stack_files_util_helper_tests.rb +++ b/test/requests/orchestration/stack_files_util_helper_tests.rb @@ -12,12 +12,12 @@ before do @oldcwd = Dir.pwd Dir.chdir("test/requests/orchestration") - @orchestration = Fog::Orchestration[:openstack] - @file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(@template_yaml) @base_url = "file://" + File.absolute_path(".") - @data =YAML.load_file("stack_files_util_tests.yaml") - @template_yaml =YAML.load_file("template.yaml") - @local_yaml =YAML.load_file("local.yaml") + @data = YAML.load_file("stack_files_util_tests.yaml") + @template_yaml = YAML.load_file("template.yaml") + @local_yaml = YAML.load_file("local.yaml") + @orchestration = Fog::Orchestration[:openstack] + @file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new({}) end after do Dir.chdir(@oldcwd) diff --git a/test/requests/orchestration/stack_files_util_tests.rb b/test/requests/orchestration/stack_files_util_tests.rb index 6a49629e6..afaaf2113 100644 --- a/test/requests/orchestration/stack_files_util_tests.rb +++ b/test/requests/orchestration/stack_files_util_tests.rb @@ -12,12 +12,12 @@ before do @oldcwd = Dir.pwd Dir.chdir("test/requests/orchestration") - @orchestration = Fog::Orchestration[:openstack] - @file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(@template_yaml) @base_url = "file://" + File.absolute_path(".") @data = YAML.load_file("stack_files_util_tests.yaml") @template_yaml = YAML.load_file("template.yaml") @local_yaml = YAML.load_file("local.yaml") + @orchestration = Fog::Orchestration[:openstack] + @file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new({}) end after do Dir.chdir(@oldcwd) @@ -33,7 +33,7 @@ [{"get_file" => "foo.sh", "b" => "values"}, {'foo.sh'=>'# Just a mock'}], ] test_cases.each do |data, expected| - file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(@template_yaml) + file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new({}) file_resolver.send(:get_file_contents, data) assert_equal(file_resolver.files, expected) end @@ -49,7 +49,7 @@ test_cases.each do |data, expected| Fog::Logger.warning("Testing with #{data} #{expected}") expected = prefix_with_url(expected, @base_url) - file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(@template_yaml) + file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new({}) file_resolver.send(:get_file_contents, data, @base_url) assert_equal(file_resolver.files.keys, expected) end @@ -60,10 +60,8 @@ [testcase['input'], testcase['expected']] end.compact test_cases.each do |data, _| - file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(data) - assert_raises ArgumentError, URI::InvalidURIError do - file_resolver.get_files + file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(data) end end end @@ -75,7 +73,6 @@ end.compact test_cases.each do |data, expected| file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(data) - file_resolver.get_files assert_equal_set(file_resolver.files.keys, expected) end end @@ -88,14 +85,12 @@ test_cases.each do |data, expected| expected = prefix_with_url(expected, @base_url) file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(data) - files = file_resolver.get_files - assert_equal_set(files.keys, expected) + assert_equal_set(file_resolver.files.keys, expected) end end it "#dont_modify_passed_template" do file_resolver = Fog::Orchestration::Util::RecursiveHotFileLoader.new(@local_yaml) - file_resolver.get_files template = file_resolver.template # The template argument should be modified.