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

Tzu-Mainn Chen tzumainn at redhat.com
Mon Oct 22 15:33:58 UTC 2012


That's a good point, but I've ACK'd and pushed anyway, since it's a blocker and the issue mentioned isn't new.

Mainn

----- Original Message -----
> From: "Jason Guiditta" <jguiditt at redhat.com>
> To: jzigmund at redhat.com
> Cc: aeolus-devel at lists.fedorahosted.org
> Sent: Monday, October 22, 2012 11:21:33 AM
> Subject: Re: [PATCH conductor] BZ 865833 - Prevent editting vsphere provider	name
> 
> 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
> 



More information about the aeolus-devel mailing list