[Ovirt-devel] [PATCH] Modified LDAPConnection to work with an SRV record, then fallback to the ldap.yml file if necessary.

[Hugh O. Brock]

On Thu, May 22, 2008 at 05:48:44PM -0400, Darryl L. Pierce wrote:
> ---
>  wui/src/app/controllers/hardware_controller.rb |    2 +-
>  wui/src/app/helpers/ldap_connection.rb         |   25 ++++++++++++++++++-----
>  wui/src/app/models/account.rb                  |    2 +-
>  3 files changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/wui/src/app/controllers/hardware_controller.rb b/wui/src/app/controllers/hardware_controller.rb
> index 296fec3..54d9e91 100644
> --- a/wui/src/app/controllers/hardware_controller.rb
> +++ b/wui/src/app/controllers/hardware_controller.rb
> @@ -144,7 +144,7 @@ class HardwareController < ApplicationController
>      dates = [ Date::ABBR_MONTHNAMES[today.month] + ' ' + today.day.to_s ]
>      1.upto(6){ |x|  # TODO get # of days from wui
>         dte = today - x
> -       dates.push ( Date::ABBR_MONTHNAMES[dte.month] + ' ' + dte.day.to_s )
> +       dates.push( Date::ABBR_MONTHNAMES[dte.month] + ' ' + dte.day.to_s )
>      }
>      dates.reverse! # want in ascending order

What's the reason for this change? Does the extra space between "push" and "(" matter?
  
> diff --git a/wui/src/app/helpers/ldap_connection.rb b/wui/src/app/helpers/ldap_connection.rb
> index af8b41b..b19e504 100644
> --- a/wui/src/app/helpers/ldap_connection.rb
> +++ b/wui/src/app/helpers/ldap_connection.rb
> @@ -21,15 +21,28 @@
>  # connections with an LDAP server.
>  #
>  class LDAPConnection
> -
> -  @@config = YAML::load(File.open("#{RAILS_ROOT}/config/ldap.yml"))
> +  @@config = YAML::load(File.open("#{RAILS_ROOT}/config/ldap.yml"))[ENV['RAILS_ENV']]
>  
>    # Connects the specified LDAP server.
>    def self.connect(base = nil, host = nil, port = nil)
> +
> +    # Use the SRV record for the LDAP server, otherwise
> +    # default to what is defined in the ldap.yml file.
> +    unless defined? host
> +      unless defined? @host
> +	Resolv::DNS.new
> +	
> +	dns.getresources("_ldap._tcp.#{`dnsdomainname`}",Resolv::DNS::Resource::IN::SRV).collect do |resource|
> +	  @host = resource.address
> +	end
> +      end
> +
> +      @host = @@config["host"] unless defined? @host
> +    end

OK, but there must be a way to get the local domain from a Ruby
syscall I would think... (i.e. instead of `dnsdomainname`)

>      
> -    base = @@config[ENV['RAILS_ENV']]["base"] if base == nil
> -    host = @@config[ENV['RAILS_ENV']]["host"] if host == nil
> -    port = @@config[ENV['RAILS_ENV']]["port"] if port == nil
> +    base = @@config["base"] if base == nil
> +    host = @@config["host"] if host == nil
> +    port = @@config["port"] if port == nil
>  
>      ActiveLdap::Base.establish_connection(:host => host,
>  					  :port => port,
> @@ -38,7 +51,7 @@ class LDAPConnection
>  
>    # Returns whether a connection already exists to the LDAP server.
>    def self.connected?
> -    return ActiveLdap::Base.connected?
> +    ActiveLdap::Base.connected?
>    end
>  
>    # Disconnects from the LDAP server.
> diff --git a/wui/src/app/models/account.rb b/wui/src/app/models/account.rb
> index a2ed1d2..384beda 100644
> --- a/wui/src/app/models/account.rb
> +++ b/wui/src/app/models/account.rb
> @@ -20,7 +20,7 @@
>  # +Account+ represents a single user's account from the LDAP server.
>  #
>  class Account < ActiveLdap::Base
> -  ldap_mapping :dn_attribute => 'cn', :scope => :one, :prefix => 'cn=users,cn=accounts'
> +  ldap_mapping :dn_attribute => 'cn', :scope => :one, :prefix => "cn=users,cn=accounts"
> 
>    @@users = nil

Again, do the single vs. double quotes matter here? Why the change?

Thanks,
--Hugh