[PATCH conductor] BZ 865833 - Prevent editting vsphere provider name

Jason Guiditta jguiditt at redhat.com
Mon Oct 22 15:50:24 UTC 2012


On 22/10/12 11:21 -0400, Jason Guiditta wrote:
>On 18/10/12 17:43 +0200, jzigmund at redhat.com wrote:
>>From: Jozef Zigmund <jzigmund at redhat.com>
>>
>>Used the same way as for rhevm provider name
>>
>>https://bugzilla.redhat.com/show_bug.cgi?id=865833
>>---
>>src/app/models/provider.rb | 8 +++++---
>>1 file changed, 5 insertions(+), 3 deletions(-)
>>
>>diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb
>>index fb2ac84..24828e1 100644
>>--- a/src/app/models/provider.rb
>>+++ b/src/app/models/provider.rb
>>@@ -74,7 +74,9 @@ class Provider < ActiveRecord::Base
>>          false
>>        end
>>      when "RHEV-M"
>>-        load_rhevm_json(name)
>>+        load_json(name, "rhevm")
>>+      when "VMware vSphere"
>>+        load_json(name, "vsphere")
>>      when "Amazon EC2"
>>        if name.starts_with?("ec2-")
>>          true
>>@@ -300,8 +302,8 @@ class Provider < ActiveRecord::Base
>>    true
>>  end
>>
>>-  def load_rhevm_json(provider_name)
>>-    path_to_json = "/etc/imagefactory/rhevm.json"
>>+  def load_json(provider_name, provider_type)
>>+    path_to_json = "/etc/imagefactory/#{provider_type}.json"
>>    if File.exists?(path_to_json)
>>      json = File.read(path_to_json)
>>      json_hash = ActiveSupport::JSON.decode(json)
>>-- 
>>1.7.11.7
>>
>Please don't put hard-coded paths into our models like this. I won't
>get into the larger issue of conditionally doing things based on
>provider (or any, really) string, as this patch does not introduce
>that particular issue, but we should get this path in some other way.
>For while these things (system-setting factory config files) exist,
>perhaps something like a setting in the initializer (I would guess
>this is settings.yml, but whatever one applies for factory) would be a
>simple compromise for the time being.  Then we can have a default that
>is easy to override, either manually or via configure.
>
>Marginally related, it seems like there might be a security hole for
>conductor can read files in a director owned by factory, but perhaps I
>am concerned about nothing there..?
>
>-j

To clarify, I realize this path only moves the terrible hard coding,
so it is reasonable to fix that separately, I just couldn't let it
pass without comment, that kind of stuff existing in our code irks me,
especially since we are supposed to be cross platform, which obviously
makes this more of a problem.

-j



More information about the aeolus-devel mailing list