[PATCH conductor] BZ 847798 - prevent editting provider name

Francesco Vollero fvollero at redhat.com
Wed Sep 5 17:31:18 UTC 2012


I forgot to say that need to be rebased the patch, again.

On Wed, Sep 05, 2012 at 07:27:40PM +0200, Francesco Vollero wrote:
> Hi Jozef,
> 
> I wasn't able to apply the patch due to changes in the model: provider.rb
> 
> here the log: 
> 
> ---
> Applying: This is short-term fix. It prevents setting of provider name,
> error: patch failed: src/app/models/provider.rb:63
> error: src/app/models/provider.rb: patch does not apply
> Patch failed at 0001 This is short-term fix. It prevents setting of provider name,
> When you have resolved this problem run "git am --resolved".
> If you would prefer to skip this patch, instead run "git am --skip".
> To restore the original branch and stop patching run "git am --abort".
> 
> ---
> 
> 
> Cheers,
> Francesco
> 
> On Tue, Sep 04, 2012 at 07:57:15PM +0200, jzigmund at redhat.com wrote:
> > From: Jozef Zigmund <jzigmund at redhat.com>
> > 
> > This is short-term fix. It prevents setting of provider name,
> > that causes failing image push in imagefactory.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=847798
> > ---
> >  src/app/controllers/providers_controller.rb | 86 +++++++++++++++++++----------
> >  src/app/models/provider.rb                  | 40 ++++++++++++++
> >  src/config/locales/activerecord/en.yml      |  4 ++
> >  src/config/locales/en.yml                   |  1 +
> >  4 files changed, 101 insertions(+), 30 deletions(-)
> > 
> > diff --git a/src/app/controllers/providers_controller.rb b/src/app/controllers/providers_controller.rb
> > index f3a9302..80474de 100644
> > --- a/src/app/controllers/providers_controller.rb
> > +++ b/src/app/controllers/providers_controller.rb
> > @@ -121,24 +121,38 @@ class ProvidersController < ApplicationController
> >  
> >      @provider = Provider.new(params[:provider])
> >  
> > -    if @provider.save
> > +    begin
> > +      if @provider.save
> > +        @provider.assign_owner_roles(current_user)
> > +        respond_to do |format|
> > +          format.html do
> > +            flash[:notice] = t"providers.flash.notice.added"
> > +            redirect_to edit_provider_path(@provider)
> > +          end
> > +          format.xml { render :partial => 'detail', :locals => { :provider => @provider } }
> > +        end
> > +      else
> > +        respond_to do |format|
> > +          format.html do
> > +            flash[:warning] = t"providers.flash.error.not_added"
> > +            render :action => "new"
> > +          end
> > +          format.xml { render :template => 'api/validation_error', :locals => { :errors => @provider.errors }, :status => :bad_request }
> > +        end
> > +      end
> > +    rescue Errno::EACCES
> > +      Provider.skip_callback :save, :check_name
> > +      @provider.save
> >        @provider.assign_owner_roles(current_user)
> >        respond_to do |format|
> >          format.html do
> >            flash[:notice] = t"providers.flash.notice.added"
> > +          flash[:warning] = t"providers.flash.warning.check_config_file"
> >            redirect_to edit_provider_path(@provider)
> >          end
> > -        format.xml { render :partial => 'detail', :locals => { :provider => @provider } }
> > -      end
> > -    else
> > -      respond_to do |format|
> > -        format.html do
> > -          flash[:warning] = t"providers.flash.error.not_added"
> > -          render :action => "new"
> > -        end
> > -        format.xml { render :template => 'api/validation_error', :locals => { :errors => @provider.errors }, :status => :bad_request }
> >        end
> >      end
> > +
> >    end
> >  
> >    def update
> > @@ -152,31 +166,43 @@ class ProvidersController < ApplicationController
> >        return
> >      end
> >  
> > -    if @provider.save
> > -      @provider.update_availability
> > -      respond_to do |format|
> > -        format.html do
> > -          flash[:notice] = t"providers.flash.notice.updated"
> > -          redirect_to edit_provider_path(@provider)
> > +    begin
> > +      if @provider.save
> > +        @provider.update_availability
> > +        respond_to do |format|
> > +          format.html do
> > +            flash[:notice] = t"providers.flash.notice.updated"
> > +            redirect_to edit_provider_path(@provider)
> > +          end
> > +          format.xml { render :partial => 'detail', :locals => { :provider => @provider } }
> > +        end
> > +      else
> > +        # we reset 'enabled' attribute to real state
> > +        # if save failed
> > +        @provider.reset_enabled!
> > +        respond_to do |format|
> > +          format.html do
> > +            unless @provider.connect
> > +              flash.now[:warning] = t"providers.flash.warning.connect_failed"
> > +            else
> > +              flash[:error] = t"providers.flash.error.not_updated"
> > +            end
> > +            load_provider_tabs
> > +            @alerts = provider_alerts(@provider)
> > +            render :action => "edit"
> > +          end
> > +          format.xml { render :template => 'api/validation_error', :locals => { :errors => @provider.errors }, :status => :bad_request }
> >          end
> > -        format.xml { render :partial => 'detail', :locals => { :provider => @provider } }
> >        end
> > -    else
> > -      # we reset 'enabled' attribute to real state
> > -      # if save failed
> > -      @provider.reset_enabled!
> > +    rescue Errno::EACCES
> > +      Provider.skip_callback :save, :check_name
> > +      @provider.save
> >        respond_to do |format|
> >          format.html do
> > -          unless @provider.connect
> > -            flash.now[:warning] = t"providers.flash.warning.connect_failed"
> > -          else
> > -            flash[:error] = t"providers.flash.error.not_updated"
> > -          end
> > -          load_provider_tabs
> > -          @alerts = provider_alerts(@provider)
> > -          render :action => "edit"
> > +          flash[:notice] = t"providers.flash.notice.updated"
> > +          flash[:warning] = t"providers.flash.warning.check_config_file"
> > +          redirect_to edit_provider_path(@provider)
> >          end
> > -        format.xml { render :template => 'api/validation_error', :locals => { :errors => @provider.errors }, :status => :bad_request }
> >        end
> >      end
> >    end
> > diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb
> > index 64ddb33..80f57fc 100644
> > --- a/src/app/models/provider.rb
> > +++ b/src/app/models/provider.rb
> > @@ -63,6 +63,28 @@ class Provider < ActiveRecord::Base
> >    has_many :provider_priority_groups, :through => :provider_priority_group_elements
> >  
> >    before_destroy :destroyable?
> > +  before_save :check_name
> > +
> > +  def check_name
> > +    case provider_type.name
> > +      when "Mock"
> > +        if name.starts_with?("mock")
> > +          true
> > +        else
> > +          errors.add(:name, :start_with_mock)
> > +          false
> > +        end
> > +      when "RHEV-M"
> > +        load_rhevm_json(name)
> > +      when "Amazon EC2"
> > +        if name.starts_with?("ec2-")
> > +          true
> > +        else
> > +          errors.add(:name, :start_with_ec2)
> > +          false
> > +        end
> > +    end
> > +  end
> >  
> >    def derived_subtree(role = nil)
> >      subtree = super(role)
> > @@ -144,6 +166,7 @@ class Provider < ActiveRecord::Base
> >        end
> >      end
> >      if res[:failed_to_stop].blank? and res[:failed_to_terminate].blank?
> > +      Provider.skip_callback :save, :check_name
> >        update_attribute(:enabled, false)
> >      end
> >      res
> > @@ -277,4 +300,21 @@ class Provider < ActiveRecord::Base
> >      end
> >      true
> >    end
> > +
> > +  def load_rhevm_json(provider_name)
> > +    path_to_json = "/etc/imagefactory/rhevm.json"
> > +    if File.exists?(path_to_json)
> > +      json = File.read(path_to_json)
> > +      json_hash = ActiveSupport::JSON.decode(json)
> > +      if json_hash.key?(provider_name)
> > +        true
> > +      else
> > +        errors.add(:name, :not_found_in_config)
> > +        false
> > +      end
> > +    else
> > +      errors.add(:name, :config_not_exist)
> > +      false
> > +    end
> > +  end
> >  end
> > diff --git a/src/config/locales/activerecord/en.yml b/src/config/locales/activerecord/en.yml
> > index ae2514d..9d2242e 100644
> > --- a/src/config/locales/activerecord/en.yml
> > +++ b/src/config/locales/activerecord/en.yml
> > @@ -360,6 +360,10 @@ en:
> >            attributes:
> >              name:
> >                invalid: "must only contain: numbers, letters, spaces, '_' and '-'"
> > +              not_found_in_config: "was not found in configuration file"
> > +              config_not_exist: "cannot be verified, configuration file does not exist"
> > +              start_with_mock: "must start with 'mock'"
> > +              start_with_ec2: "must start with 'ec2-'"
> >              url:
> >                invalid_framework: "Must be a valid provider uri"
> >              deltacloud_provider:
> > diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
> > index c8fd262..2652dd5 100644
> > --- a/src/config/locales/en.yml
> > +++ b/src/config/locales/en.yml
> > @@ -1201,6 +1201,7 @@ en:
> >          connect_failed: "Failed to connect to Provider"
> >          not_stopped_instances: "Provider was not disabled. Failed to stop following instances:"
> >          not_terminated_instances: "Provider was not disabled. Failed to change status to 'stopped' for following instances:"
> > +        check_config_file: Cannot check if provider name is right. Please check config file
> >      alerts:
> >        type:
> >          critical: Critical
> > -- 
> > 1.7.11.4
> > 



More information about the aeolus-devel mailing list