On Tue, Mar 22, 2011 at 3:35 PM, Stefan Schulte <
stefan.schu...@taunusstein.net> wrote:

> This new type "port" handles entries in /etc/services. It uses multiple
> key_attributes (name and protocol), so you are able to add e.g.
> multiple telnet lines for tcp and udp. Sample usage
>

Is "port" the best name for this?

That feels like we're grabbing an awfully generic name for a quite specific
task.






>
>    port { 'telnet':
>      number      => '23',
>      protocol    => 'tcp',
>      description => 'Telnet'
>    }
>
> Because the type makes use of the title_patterns function this can also
> be written as
>
>    port { 'telnet/tcp':
>      number      => '23',
>      description => 'Telnet'
>    }
>
> This type only supports tcp and udp and might not work on OS X
>
> Signed-off-by: Stefan Schulte <stefan.schu...@taunusstein.net>
> ---
> Local-branch: feature/next/5660N
>  lib/puppet/type/port.rb     |  258
> ++++++++++++++++++++++-------------------
>  spec/unit/type/port_spec.rb |  270
> +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 410 insertions(+), 118 deletions(-)
>  create mode 100644 spec/unit/type/port_spec.rb
>
> diff --git a/lib/puppet/type/port.rb b/lib/puppet/type/port.rb
> index e199885..f895785 100755
> --- a/lib/puppet/type/port.rb
> +++ b/lib/puppet/type/port.rb
> @@ -1,119 +1,141 @@
> -#module Puppet
> -#    newtype(:port) do
> -#        @doc = "Installs and manages port entries.  For most systems,
> these
> -#            entries will just be in /etc/services, but some systems
> (notably OS X)
> -#            will have different solutions."
> -#
> -#        ensurable
> -#
> -#        newproperty(:protocols) do
> -#            desc "The protocols the port uses.  Valid values are *udp*
> and *tcp*.
> -#                Most services have both protocols, but not all.  If you
> want
> -#                both protocols, you must specify that; Puppet replaces
> the
> -#                current values, it does not merge with them.  If you
> specify
> -#                multiple protocols they must be as an array."
> -#
> -#            def is=(value)
> -#                case value
> -#                when String
> -#                    @is = value.split(/\s+/)
> -#                else
> -#                    @is = value
> -#                end
> -#            end
> -#
> -#            def is
> -#                @is
> -#            end
> -#
> -#            # We actually want to return the whole array here, not just
> the first
> -#            # value.
> -#            def should
> -#                if defined?(@should)
> -#                    if @should[0] == :absent
> -#                        return :absent
> -#                    else
> -#                        return @should
> -#                    end
> -#                else
> -#                    return nil
> -#                end
> -#            end
> -#
> -#            validate do |value|
> -#                valids = ["udp", "tcp", "ddp", :absent]
> -#                unless valids.include? value
> -#                    raise Puppet::Error,
> -#                        "Protocols can be either 'udp' or 'tcp', not
> #{value}"
> -#                end
> -#            end
> -#        end
> -#
> -#        newproperty(:number) do
> -#            desc "The port number."
> -#        end
> -#
> -#        newproperty(:description) do
> -#            desc "The port description."
> -#        end
> -#
> -#        newproperty(:port_aliases) do
> -#            desc 'Any aliases the port might have.  Multiple values must
> be
> -#                specified as an array.  Note that this property is not
> the same as
> -#                the "alias" metaparam; use this property to add aliases
> to a port
> -#                in the services file, and "alias" to aliases for use in
> your Puppet
> -#                scripts.'
> -#
> -#            # We actually want to return the whole array here, not just
> the first
> -#            # value.
> -#            def should
> -#                if defined?(@should)
> -#                    if @should[0] == :absent
> -#                        return :absent
> -#                    else
> -#                        return @should
> -#                    end
> -#                else
> -#                    return nil
> -#                end
> -#            end
> -#
> -#            validate do |value|
> -#                if value.is_a? String and value =~ /\s/
> -#                    raise Puppet::Error,
> -#                        "Aliases cannot have whitespace in them: %s" %
> -#                        value.inspect
> -#                end
> -#            end
> -#
> -#            munge do |value|
> -#                unless value == "absent" or value == :absent
> -#                    # Add the :alias metaparam in addition to the
> property
> -#                    @resource.newmetaparam(
> -#                        @resource.class.metaparamclass(:alias), value
> -#                    )
> -#                end
> -#                value
> -#            end
> -#        end
> -#
> -#        newproperty(:target) do
> -#            desc "The file in which to store service information.  Only
> used by
> -#                those providers that write to disk."
> -#
> -#            defaultto { if
> @resource.class.defaultprovider.ancestors.include?(Puppet::Provider::ParsedFile)
> -#                    @resource.class.defaultprovider.default_target
> -#                else
> -#                    nil
> -#                end
> -#            }
> -#        end
> -#
> -#        newparam(:name) do
> -#            desc "The port name."
> -#
> -#            isnamevar
> -#        end
> -#    end
> -#end
> +require 'puppet/property/ordered_list'
>
> +module Puppet
> +  newtype(:port) do
> +    @doc = "Installs and manages port entries. For most systems, these
> +      entries will just be in `/etc/services`, but some systems (notably
> OS X)
> +      will have different solutions.
> +
> +      This type uses a composite key of (port) `name` and (port) `number`
> to
> +      identify a resource. You are able to set both keys with the resource
> +      title if you seperate them with a slash. So instead of specifying
> protocol
> +      explicitly:
> +
> +          port { \"telnet\":
> +            protocol => tcp,
> +            number   => 23,
> +          }
> +
> +      you can also specify both name and protocol implicitly through the
> title:
> +
> +          port { \"telnet/tcp\":
> +            number => 23,
> +          }
> +
> +      The second way is the prefered way if you want to specifiy a port
> that
> +      uses both tcp and udp as a protocol. You need to define two
> resources
> +      for such a port but the resource title still has to be uniq.
> +
> +      Example: To make sure you have the telnet port in your
> `/etc/services`
> +      file you will now write:
> +
> +          port { \"telnet/tcp\":
> +            number => 23,
> +          }
> +          port { \"telnet/udp\":
> +            number => 23,
> +          }
> +
> +      Currently only tcp and udp are supported and recognised when setting
> +      the protocol via the title."
> +
> +    def self.title_patterns
> +      [
> +        # we have two title_patterns "name" and "name:protocol". We won't
> use
> +        # one pattern (that will eventually set :protocol to nil) because
> we
> +        # want to use a default value for :protocol. And that does only
> work
> +        # if :protocol is not put in the parameter hash while initialising
> +        [
> +          /^(.*?)\/(tcp|udp)$/, # Set name and protocol
> +          [
> +            # We don't need a lot of post-parsing
> +            [ :name, lambda{|x| x} ],
> +            [ :protocol, lambda{ |x| x.intern unless x.nil? } ]
> +          ]
> +        ],
> +        [
> +          /^(.*)$/,
> +          [
> +            [ :name, lambda{|x| x} ]
> +          ]
> +        ]
> +      ]
> +    end
> +
> +    ensurable
> +
> +    newparam(:name) do
> +      desc "The port name."
> +
> +      validate do |value|
> +        raise Puppet::Error "Port name must not contain whitespace:
> #{value}" if value =~ /\s/
> +      end
> +
> +      isnamevar
> +    end
> +
> +    newparam(:protocol) do
> +      desc "The protocol the port uses. Valid values are *udp* and *tcp*.
> +        Most services have both protocols, but not all. If you want both
> +        protocols you have to define two resources. Remeber that you
> cannot
> +        specify two resources with the same title but you can use a title
> +        to set both, name and protocol if you use ':' as a seperator. So
> +        port { \"telnet/tcp\": ... } sets both name and protocol and you
> don't
> +        have to specify them explicitly."
> +
> +      newvalues :tcp, :udp
> +
> +      defaultto :tcp
> +
> +      isnamevar
> +    end
> +
> +
> +    newproperty(:number) do
> +      desc "The port number."
> +
> +      validate do |value|
> +        raise Puppet::Error, "number has to be numeric, not #{value}"
> unless value =~ /^[0-9]+$/
> +        raise Puppet::Error, "number #{value} out of range (0-65535)"
> unless (0...2**16).include?(Integer(value))
> +      end
> +
> +    end
> +
> +    newproperty(:description) do
> +      desc "The description for the port. The description will appear"
> +        "as a comment in the `/etc/services` file"
> +    end
> +
> +    newproperty(:port_aliases, :parent => Puppet::Property::OrderedList)
> do
> +      desc "Any aliases the port might have. Multiple values must be
> +        specified as an array."
> +
> +      def inclusive?
> +        true
> +      end
> +
> +      def delimiter
> +        " "
> +      end
> +
> +      validate do |value|
> +        raise Puppet::Error, "Aliases must not contain whitespace:
> #{value}" if value =~ /\s/
> +      end
> +    end
> +
> +
> +    newproperty(:target) do
> +      desc "The file in which to store service information. Only used by
> +        those providers that write to disk."
> +
> +      defaultto do
> +        if
> @resource.class.defaultprovider.ancestors.include?(Puppet::Provider::ParsedFile)
> +          @resource.class.defaultprovider.default_target
> +        else
> +          nil
> +        end
> +      end
> +    end
> +
> +  end
> +end
> diff --git a/spec/unit/type/port_spec.rb b/spec/unit/type/port_spec.rb
> new file mode 100644
> index 0000000..8386ae5
> --- /dev/null
> +++ b/spec/unit/type/port_spec.rb
> @@ -0,0 +1,270 @@
> +#!/usr/bin/env ruby
> +
> +require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
> +require 'puppet/property/ordered_list'
> +
> +port = Puppet::Type.type(:port)
> +
> +describe port do
> +  before do
> +    @class = port
> +    @provider_class = stub 'provider_class', :name => 'fake', :ancestors
> => [], :suitable? => true, :supports_parameter? => true
> +    @class.stubs(:defaultprovider).returns @provider_class
> +    @class.stubs(:provider).returns @provider_class
> +
> +    @provider = stub 'provider', :class => @provider_class, :clean => nil,
> :exists? => false
> +    @resource = stub 'resource', :resource => nil, :provider => @provider
> +
> +    @provider.stubs(:port_aliases).returns :absent
> +
> +    @provider_class.stubs(:new).returns(@provider)
> +    @catalog = Puppet::Resource::Catalog.new
> +  end
> +
> +  it "should have a title pattern that splits name and protocol" do
> +    regex = @class.title_patterns[0][0]
> +    regex.match("telnet/tcp").captures.should == ['telnet','tcp' ]
> +    regex.match("telnet/udp").captures.should == ['telnet','udp' ]
> +    regex.match("telnet/baz").should == nil
> +  end
> +
> +  it "should have a second title pattern that will set only name" do
> +    regex = @class.title_patterns[1][0]
> +    regex.match("telnet/tcp").captures.should == ['telnet/tcp' ]
> +    regex.match("telnet/udp").captures.should == ['telnet/udp' ]
> +    regex.match("telnet/baz").captures.should == ['telnet/baz' ]
> +  end
> +
> +  it "should have two key_attributes" do
> +    @class.key_attributes.size.should == 2
> +  end
> +
> +  it "should have :name as a key_attribute" do
> +    @class.key_attributes.should include :name
> +  end
> +
> +  it "should have :protocol as a key_attribute" do
> +    @class.key_attributes.should include :protocol
> +  end
> +
> +  describe "when validating attributes" do
> +
> +    [:name, :provider, :protocol].each do |param|
> +      it "should have a #{param} parameter" do
> +        @class.attrtype(param).should == :param
> +      end
> +    end
> +
> +    [:ensure, :port_aliases, :description, :number].each do |property|
> +      it "should have #{property} property" do
> +        @class.attrtype(property).should == :property
> +      end
> +    end
> +
> +    it "should have a list port_aliases" do
> +      @class.attrclass(:port_aliases).ancestors.should include
> Puppet::Property::OrderedList
> +    end
> +
> +  end
> +
> +  describe "when validating values" do
> +
> +    it "should support present as a value for ensure" do
> +      lambda { @class.new(:name => "whev", :protocol => :tcp, :ensure =>
> :present) }.should_not raise_error
> +    end
> +
> +    it "should support absent as a value for ensure" do
> +      proc { @class.new(:name => "whev", :protocol => :tcp, :ensure =>
> :absent) }.should_not raise_error
> +    end
> +
> +    it "should support :tcp  as a value for protocol" do
> +      proc { @class.new(:name => "whev", :protocol => :tcp) }.should_not
> raise_error
> +    end
> +
> +    it "should support :udp  as a value for protocol" do
> +      proc { @class.new(:name => "whev", :protocol => :udp) }.should_not
> raise_error
> +    end
> +
> +    it "should not support other protocols than tcp and udp" do
> +      proc { @class.new(:name => "whev", :protocol => :tcpp) }.should
> raise_error(Puppet::Error)
> +    end
> +
> +    it "should use tcp as default protocol" do
> +      port_test = @class.new(:name => "whev")
> +      port_test[:protocol].should == :tcp
> +    end
> +
> +    it "should support valid portnumbers" do
> +      proc { @class.new(:name => "whev", :protocol => :tcp, :number =>
> '0') }.should_not raise_error
> +      proc { @class.new(:name => "whev", :protocol => :tcp, :number =>
> '1') }.should_not raise_error
> +      proc { @class.new(:name => "whev", :protocol => :tcp, :number =>
> "#{2**16-1}") }.should_not raise_error
> +    end
> +
> +    it "should not support portnumbers that arent numeric" do
> +      proc { @class.new(:name => "whev", :protocol => :tcp, :number =>
> "aa") }.should raise_error(Puppet::Error)
> +      proc { @class.new(:name => "whev", :protocol => :tcp, :number =>
> "22a") }.should raise_error(Puppet::Error)
> +      proc { @class.new(:name => "whev", :protocol => :tcp, :number =>
> "a22") }.should raise_error(Puppet::Error)
> +    end
> +
> +    it "should not support portnumbers that are out of range" do
> +      proc { @class.new(:name => "whev", :protocol => :tcp, :number =>
> "-1") }.should raise_error(Puppet::Error)
> +      proc { @class.new(:name => "whev", :protocol => :tcp, :number =>
> "#{2**16}") }.should raise_error(Puppet::Error)
> +    end
> +
> +    it "should support single port_alias" do
> +      proc { @class.new(:name => "foo", :protocol => :tcp, :port_aliases
> => 'bar') }.should_not raise_error
> +    end
> +
> +    it "should support multiple port_aliases" do
> +      proc { @class.new(:name => "foo", :protocol => :tcp, :port_aliases
> => ['bar','bar2']) }.should_not raise_error
> +    end
> +
> +    it "should not support whitespaces in any port_alias" do
> +      proc { @class.new(:name => "whev", :protocol => :tcp, :port_aliases
> => ['bar','fo o']) }.should raise_error(Puppet::Error)
> +    end
> +
> +    it "should not support whitespaces in resourcename" do
> +      proc { @class.new(:name => "foo bar", :protocol => :tcp) }.should
> raise_error(Puppet::Error)
> +    end
> +
> +    it "should not allow a resource with no name" do
> +      proc { @class.new(:protocol => :tcp) }.should
> raise_error(Puppet::Error)
> +    end
> +
> +    it "should allow a resource with no protocol when the default is tcp"
> do
> +      proc { @class.new(:name => "foo") }.should_not
> raise_error(Puppet::Error)
> +    end
> +
> +    it "should not allow a resource with no protocol when we have no
> default" do
> +
>  
> @class.attrclass(:protocol).stubs(:method_defined?).with(:default).returns(false)
> +      proc { @class.new(:name => "foo") }.should
> raise_error(Puppet::Error)
> +    end
> +
> +    it "should extract name and protocol from title if not explicitly set"
> do
> +      res = @class.new(:title => 'telnet/tcp', :number => '23')
> +      res[:number].should == '23'
> +      res[:name].should == 'telnet'
> +      res[:protocol].should == :tcp
> +    end
> +
> +    it "should not extract name from title if explicitly set" do
> +      res = @class.new(:title => 'telnet/tcp', :name => 'ssh', :number =>
> '23')
> +      res[:number].should == '23'
> +      res[:name].should == 'ssh'
> +      res[:protocol].should == :tcp
> +    end
> +
> +    it "should not extract protocol from title if explicitly set" do
> +      res = @class.new(:title => 'telnet/tcp', :protocol => :udp, :number
> => '23')
> +      res[:number].should == '23'
> +      res[:name].should == 'telnet'
> +      res[:protocol].should == :udp
> +    end
> +
> +    it "should not extract name and protocol from title when they are
> explicitly set" do
> +      res = @class.new(:title => 'foo/udp', :name => 'bar', :protocol =>
> :tcp, :number => '23')
> +      res[:number].should == '23'
> +      res[:name].should == 'bar'
> +      res[:protocol].should == :tcp
> +    end
> +
> +  end
> +
> +  describe "when syncing" do
> +
> +    it "should send the first value to the provider for number property"
> do
> +      number = @class.attrclass(:number).new(:resource => @resource,
> :should => %w{100 200})
> +      @provider.expects(:number=).with '100'
> +      number.sync
> +    end
> +
> +    it "should send the joined array to the provider for port_aliases
> property" do
> +      port_aliases = @class.attrclass(:port_aliases).new(:resource =>
> @resource, :should => %w{foo bar})
> +      @provider.expects(:port_aliases=).with 'foo bar'
> +      port_aliases.sync
> +    end
> +
> +    it "should care about the order of port_aliases" do
> +      port_aliases = @class.attrclass(:port_aliases).new(:resource =>
> @resource, :should => %w{a z b})
> +      port_aliases.insync?(%w{a z b}).should == true
> +      port_aliases.insync?(%w{a b z}).should == false
> +      port_aliases.insync?(%w{b a z}).should == false
> +      port_aliases.insync?(%w{z a b}).should == false
> +      port_aliases.insync?(%w{z b a}).should == false
> +      port_aliases.insync?(%w{b z a}).should == false
> +    end
> +
> +  end
> +
> +  describe "when comparing uniqueness_key of two ports" do
> +
> +    it "should be equal if name and protocol are the same" do
> +      foo_tcp1 =  @class.new(:name => "foo", :protocol => :tcp, :number =>
> '23')
> +      foo_tcp2 =  @class.new(:name => "foo", :protocol => :tcp, :number =>
> '23')
> +      foo_tcp1.uniqueness_key.should == ['foo', :tcp ]
> +      foo_tcp2.uniqueness_key.should == ['foo', :tcp ]
> +      foo_tcp1.uniqueness_key.should == foo_tcp2.uniqueness_key
> +    end
> +
> +    it "should not be equal if protocol differs" do
> +      foo_tcp =  @class.new(:name => "foo", :protocol => :tcp, :number =>
> '23')
> +      foo_udp =  @class.new(:name => "foo", :protocol => :udp, :number =>
> '23')
> +      foo_tcp.uniqueness_key.should == [ 'foo', :tcp ]
> +      foo_udp.uniqueness_key.should == [ 'foo', :udp ]
> +      foo_tcp.uniqueness_key.should_not == foo_udp.uniqueness_key
> +    end
> +
> +    it "should not be equal if name differs" do
> +      foo_tcp =  @class.new(:name => "foo", :protocol => :tcp, :number =>
> '23')
> +      bar_tcp =  @class.new(:name => "bar", :protocol => :tcp, :number =>
> '23')
> +      foo_tcp.uniqueness_key.should == [ 'foo', :tcp ]
> +      bar_tcp.uniqueness_key.should == [ 'bar', :tcp ]
> +      foo_tcp.uniqueness_key.should_not == bar_tcp.uniqueness_key
> +    end
> +
> +    it "should not be equal if both name and protocol differ" do
> +      foo_tcp =  @class.new(:name => "foo", :protocol => :tcp, :number =>
> '23')
> +      bar_udp =  @class.new(:name => "bar", :protocol => :udp, :number =>
> '23')
> +      foo_tcp.uniqueness_key.should == [ 'foo', :tcp ]
> +      bar_udp.uniqueness_key.should == [ 'bar', :udp ]
> +      foo_tcp.uniqueness_key.should_not == bar_udp.uniqueness_key
> +    end
> +
> +  end
> +
> +  describe "when adding resource to a catalog" do
> +
> +    it "should not allow two resources with the same name and protocol" do
> +      res1 =  @class.new(:name => "telnet", :protocol => :tcp, :number =>
> '23')
> +      res2 =  @class.new(:name => "telnet", :protocol => :tcp, :number =>
> '23')
> +      proc { @catalog.add_resource(res1) }.should_not raise_error
> +      proc { @catalog.add_resource(res2) }.should
> raise_error(Puppet::Resource::Catalog::DuplicateResourceError)
> +    end
> +
> +    it "should allow two resources with different name and protocol" do
> +      res1 =  @class.new(:name => "telnet", :protocol => :tcp, :number =>
> '23')
> +      res2 =  @class.new(:name => "git", :protocol => :tcp, :number =>
> '9418')
> +      proc { @catalog.add_resource(res1) }.should_not raise_error
> +      proc { @catalog.add_resource(res2) }.should_not raise_error
> +    end
> +
> +    it "should allow two resources with same name and different protocol"
> do
> +      # I would like to have a gentitle method that would not
> automatically set
> +      # title to resource[:name] but to uniqueness_key.join('/') or
> +      # similar - stschulte
> +      res1 =  @class.new(:title => 'telnet/tcp', :name => 'telnet',
> :protocol => :tcp, :number => '23')
> +      res2 =  @class.new(:title => 'telnet/udp', :name => 'telnet',
> :protocol => :udp, :number => '23')
> +      proc { @catalog.add_resource(res1) }.should_not raise_error
> +      proc { @catalog.add_resource(res2) }.should_not raise_error
> +    end
> +
> +    it "should allow two resources with the same protocol but different
> names" do
> +      res1 =  @class.new(:title => 'telnet/tcp', :name => 'telnet',
> :protocol => :tcp, :number => '23')
> +      res2 =  @class.new(:title => 'ssh/tcp', :name => 'ssh', :protocol =>
> :tcp, :number => '23')
> +      proc { @catalog.add_resource(res1) }.should_not raise_error
> +      proc { @catalog.add_resource(res2) }.should_not raise_error
> +    end
> +
> +  end
> +
> +end
> --
> 1.7.4.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Developers" group.
> To post to this group, send email to puppet-dev@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-dev+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-dev?hl=en.
>
>


-- 
Nigel Kersten
Product, Puppet Labs
@nigelkersten

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com.
To unsubscribe from this group, send email to 
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to