Add a new type mounttab that does only ensure present/absent in fstab. This is a much more simpler type than the original mounttype.
A few other things were changed, too: * property options is an array now this means that options cannot be specified as a comma separated list and it allows us to treat options in sync if just the order of options is different. * defaultvalue for the pass-property is now '-' on Solaris. It is still 0 on other OS * defaultvalue for blockdevice is now '-' if fstype is NFS (#4689) * property atboot now doesnt only support yes/no but also true/false (and yes/no will still be put in vfstab) * no property supports whitespace so we wont write currupt fstab entries to disk (#6409) Signed-off-by: Stefan Schulte <[email protected]> --- Local-branch: feature/2.7.x/mounttab lib/puppet/provider/mounttab/parsed.rb | 36 ++++ lib/puppet/type/mounttab.rb | 139 +++++++++++++ spec/unit/provider/mounttab/parsed_spec.rb | 105 ++++++++++ spec/unit/type/mounttab_spec.rb | 295 ++++++++++++++++++++++++++++ 4 files changed, 575 insertions(+), 0 deletions(-) create mode 100755 lib/puppet/provider/mounttab/parsed.rb create mode 100755 lib/puppet/type/mounttab.rb create mode 100755 spec/unit/provider/mounttab/parsed_spec.rb create mode 100755 spec/unit/type/mounttab_spec.rb diff --git a/lib/puppet/provider/mounttab/parsed.rb b/lib/puppet/provider/mounttab/parsed.rb new file mode 100755 index 0000000..740c413 --- /dev/null +++ b/lib/puppet/provider/mounttab/parsed.rb @@ -0,0 +1,36 @@ +require 'puppet/provider/parsedfile' + +fstab = case Facter.value(:operatingsystem) +when "Solaris" + "/etc/vfstab" +else + "/etc/fstab" +end + +Puppet::Type.type(:mounttab).provide(:parsed, :parent => Puppet::Provider::ParsedFile, :default_target => fstab, :filetype => :flat) do + + case Facter.value(:operatingsystem) + when "Solaris" + @fields = [:device, :blockdevice, :name, :fstype, :pass, :atboot, :options] + else + @fields = [:device, :name, :fstype, :options, :dump, :pass] + @fielddefaults = [ nil ] * 4 + [ "0", "2" ] + end + + text_line :comment, :match => /^\s*#/ + text_line :blank, :match => /^\s*$/ + + optional_fields = @fields - [:device, :name, :blockdevice] + mandatory_fields = @fields - optional_fields + + # fstab will ignore lines that have fewer than the mandatory number of columns, + # so we should, too. + field_pattern = '(\s*(?>\S+))' + text_line :incomplete, :match => /^(?!#{field_pattern}{#{mandatory_fields.length}})/ + + record_line :parsed, + :fields => @fields, + :separator => /\s+/, + :joiner => "\t", + :optional => optional_fields +end diff --git a/lib/puppet/type/mounttab.rb b/lib/puppet/type/mounttab.rb new file mode 100755 index 0000000..940d879 --- /dev/null +++ b/lib/puppet/type/mounttab.rb @@ -0,0 +1,139 @@ +require 'puppet/property/list' +require 'puppet/provider/parsedfile' + +module Puppet + newtype(:mounttab) do + @doc = "Manages entries in the filesystem table." + + ensurable + + newproperty(:device) do + desc "The device providing the mount. This can be whatever + device is supporting by the mount, including network + devices or devices specified by UUID rather than device + path, depending on the operating system." + + validate do |value| + raise Puppet::Error, "device must not contain whitespace: #{value}" if value =~ /\s/ + end + end + + newproperty(:blockdevice) do + desc "The device to fsck. This is property is only valid + on Solaris, and in most cases will default to the correct + value." + + defaultto do + if Facter.value(:operatingsystem) == "Solaris" + if device = @resource[:device] and device =~ %r{/dsk/} + device.sub(%r{/dsk/}, "/rdsk/") + elsif fstype = resource[:fstype] and fstype.downcase == 'nfs' + '-' + else + nil + end + else + nil + end + end + + validate do |value| + raise Puppet::Error, "blockdevice must not contain whitespace: #{value}" if value =~ /\s/ + end + + end + + newproperty(:fstype) do + desc "The mount type. Valid values depend on the + operating system. This is a required option." + + validate do |value| + raise Puppet::Error, "fstype must not contain whitespace: #{value}" if value =~ /\s/ + end + end + + newproperty(:options, :parent => Puppet::Property::List) do + desc "Mount options for the mount. More than one option should + be specified as an array" + + def delimiter + "," + end + + def inclusive? + true + end + + validate do |value| + raise Puppet::Error, "multiple options have to be specified as an array not a comma separated list" if value =~ /,/ + raise Puppet::Error, "option must not contain whitespace: #{value}" if value =~ /\s/ + end + + end + + newproperty(:pass) do + desc "The pass in which the mount is checked." + + defaultto do + if resource.managed? + if Facter.value(:operatingsystem) == 'Solaris' + '-' + else + 0 + end + end + end + end + + newproperty(:atboot) do + desc "Whether to mount the mount at boot. Not all platforms + support this." + + newvalues :yes, :no + aliasvalue :true, :yes + aliasvalue :false, :no + end + + newproperty(:dump) do + desc "Whether to dump the mount. Not all platform support this. + Valid values are `1` or `0`. or `2` on FreeBSD, Default is `0`." + + if Facter.value(:operatingsystem) == "FreeBSD" + newvalue(%r{(0|1|2)}) + else + newvalue(%r{(0|1)}) + end + + defaultto do + 0 if resource.managed? + end + + end + + newproperty(:target) do + desc "The file in which to store the mount table. 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 + + newparam(:name) do + desc "The mount path for the mount." + + isnamevar + + validate do |value| + raise Puppet::Error, "mount name must not contain whitespace: #{value}" if value =~ /\s/ + # Except of root / a trailing slash can cause problems + raise Puppet::Error, "mount should be specified without a trailing slash: #{value}" if value =~ /.+\/$/ + end + end + + end +end diff --git a/spec/unit/provider/mounttab/parsed_spec.rb b/spec/unit/provider/mounttab/parsed_spec.rb new file mode 100755 index 0000000..7b25e47 --- /dev/null +++ b/spec/unit/provider/mounttab/parsed_spec.rb @@ -0,0 +1,105 @@ +#!/usr/bin/env ruby + +require 'spec_helper' + +describe Puppet::Type.type(:mounttab).provider(:parsed) do + + before :each do + @mounttab_class = Puppet::Type.type(:mounttab) + @provider = @mounttab_class.provider(:parsed) + @provider.stubs(:suitable?).returns true + end + + # LAK:FIXME I can't mock Facter because this test happens at parse-time. + it "should default to /etc/vfstab on Solaris" do + pending "This test only works on Solaris" unless Facter.value(:operatingsystem) == 'Solaris' + @provider.default_target.should == '/etc/vfstab' + end + + it "should default to /etc/fstab on anything else" do + pending "This test does not work on Solaris" if Facter.value(:operatingsystem) == 'Solaris' + @provider.default_target.should == '/etc/fstab' + end + + describe "when parsing a line" do + + it "should not crash on incomplete lines in fstab" do + parse = @provider.parse <<-FSTAB +/dev/incomplete +/dev/device name +FSTAB + lambda{ @provider.to_line(parse[0]) }.should_not raise_error + end + + + describe "on Solaris", :if => Facter.value(:operatingsystem) == 'Solaris' do + + before :each do + @example_line = "/dev/dsk/c0d0s0 /dev/rdsk/c0d0s0 \t\t / \t ufs 1 no\t-" + end + + it "should extract device from the first field" do + @provider.parse_line(@example_line)[:device].should == '/dev/dsk/c0d0s0' + end + + it "should extract blockdevice from second field" do + @provider.parse_line(@example_line)[:blockdevice].should == "/dev/rdsk/c0d0s0" + end + + it "should extract name from third field" do + @provider.parse_line(@example_line)[:name].should == "/" + end + + it "should extract fstype from fourth field" do + @provider.parse_line(@example_line)[:fstype].should == "ufs" + end + + it "should extract pass from fifth field" do + @provider.parse_line(@example_line)[:pass].should == "1" + end + + it "should extract atboot from sixth field" do + @provider.parse_line(@example_line)[:atboot].should == "no" + end + + it "should extract options from seventh field" do + @provider.parse_line(@example_line)[:options].should == "-" + end + + end + + describe "on other platforms than Solaris", :if => Facter.value(:operatingsystem) != 'Solaris' do + + before :each do + @example_line = "/dev/vg00/lv01\t/spare \t \t ext3 defaults\t1 2" + end + + it "should extract device from the first field" do + @provider.parse_line(@example_line)[:device].should == '/dev/vg00/lv01' + end + + it "should extract name from second field" do + @provider.parse_line(@example_line)[:name].should == "/spare" + end + + it "should extract fstype from third field" do + @provider.parse_line(@example_line)[:fstype].should == "ext3" + end + + it "should extract options from fourth field" do + @provider.parse_line(@example_line)[:options].should == "defaults" + end + + it "should extract dump from fifth field" do + @provider.parse_line(@example_line)[:dump].should == "1" + end + + it "should extract options from sixth field" do + @provider.parse_line(@example_line)[:pass].should == "2" + end + + end + + end + +end diff --git a/spec/unit/type/mounttab_spec.rb b/spec/unit/type/mounttab_spec.rb new file mode 100755 index 0000000..ca4bb6a --- /dev/null +++ b/spec/unit/type/mounttab_spec.rb @@ -0,0 +1,295 @@ +#!/usr/bin/env ruby + +require 'spec_helper' + +describe Puppet::Type.type(:mounttab) do + + before do + @class = described_class + @provider_class = @class.provide(:fake) { mk_resource_methods } + @provider = @provider_class.new + @resource = stub 'resource', :resource => nil, :provider => @provider + + @class.stubs(:defaultprovider).returns @provider_class + @class.any_instance.stubs(:provider).returns @provider + end + + it "should have :name as its keyattribute" do + @class.key_attributes.should == [:name] + end + + describe "when validating attributes" do + + [:name, :provider].each do |param| + it "should have a #{param} parameter" do + @class.attrtype(param).should == :param + end + end + + [:ensure, :device, :blockdevice, :fstype, :options, :pass, :dump, :atboot, :target].each do |param| + it "should have a #{param} property" do + @class.attrtype(param).should == :property + end + end + + end + + describe "when validating values" do + + describe "for name" do + + it "should support absolute paths" do + proc { @class.new(:name => "/foo", :ensure => :present) }.should_not raise_error + end + + it "should not support whitespace" do + proc { @class.new(:name => "/foo bar", :ensure => :present) }.should raise_error(Puppet::Error, /name.*whitespace/) + end + + it "should not allow trailing slashes" do + proc { @class.new(:name => "/foo/", :ensure => :present) }.should raise_error(Puppet::Error, /mount should be specified without a trailing slash/) + proc { @class.new(:name => "/foo//", :ensure => :present) }.should raise_error(Puppet::Error, /mount should be specified without a trailing slash/) + end + + end + + describe "for ensure" do + it "should support present as a value for ensure" do + proc { @class.new(:name => "/foo", :ensure => :present) }.should_not raise_error + end + + it "should support absent as a value for ensure" do + proc { @class.new(:name => "/foo", :ensure => :absent) }.should_not raise_error + end + end + + describe "for device" do + + it "should support normal /dev paths for device" do + proc { @class.new(:name => "/foo", :ensure => :present, :device => '/dev/hda1') }.should_not raise_error + proc { @class.new(:name => "/foo", :ensure => :present, :device => '/dev/dsk/c0d0s0') }.should_not raise_error + end + + it "should support labels for device" do + proc { @class.new(:name => "/foo", :ensure => :present, :device => 'LABEL=/boot') }.should_not raise_error + proc { @class.new(:name => "/foo", :ensure => :present, :device => 'LABEL=SWAP-hda6') }.should_not raise_error + end + + it "should support pseudo devices for device" do + proc { @class.new(:name => "/foo", :ensure => :present, :device => 'ctfs') }.should_not raise_error + proc { @class.new(:name => "/foo", :ensure => :present, :device => 'swap') }.should_not raise_error + proc { @class.new(:name => "/foo", :ensure => :present, :device => 'sysfs') }.should_not raise_error + proc { @class.new(:name => "/foo", :ensure => :present, :device => 'proc') }.should_not raise_error + end + + it "should not support whitespace in device" do + proc { @class.new(:name => "/foo", :ensure => :present, :device => '/dev/my dev/foo') }.should raise_error Puppet::Error, /device.*whitespace/ + proc { @class.new(:name => "/foo", :ensure => :present, :device => "/dev/my\tdev/foo") }.should raise_error Puppet::Error, /device.*whitespace/ + end + + end + + describe "for blockdevice" do + + before :each do + Facter.stubs(:value).with(:operatingsystem).returns 'Solaris' + end + + it "should support normal /dev/rdsk paths for blockdevice" do + proc { @class.new(:name => "/foo", :ensure => :present, :blockdevice => '/dev/rdsk/c0d0s0') }.should_not raise_error + end + + it "should support a dash for blockdevice" do + proc { @class.new(:name => "/foo", :ensure => :present, :blockdevice => '-') }.should_not raise_error + end + + it "should not support whitespace in blockdevice" do + proc { @class.new(:name => "/foo", :ensure => :present, :blockdevice => '/dev/my dev/foo') }.should raise_error Puppet::Error, /blockdevice.*whitespace/ + proc { @class.new(:name => "/foo", :ensure => :present, :blockdevice => "/dev/my\tdev/foo") }.should raise_error Puppet::Error, /blockdevice.*whitespace/ + end + + it "should default to /dev/rdsk/DEVICE if device is /dev/dsk/DEVICE" do + obj = @class.new(:name => "/foo", :device => '/dev/dsk/c0d0s0') + obj[:blockdevice].should == '/dev/rdsk/c0d0s0' + end + + it "should default to - if it is an nfs-share" do + obj = @class.new(:name => "/foo", :device => "server://share", :fstype => 'nfs') + obj[:blockdevice].should == '-' + end + + it "should have no default otherwise" do + @class.new(:name => "/foo")[:blockdevice].should == nil + @class.new(:name => "/foo", :device => "/foo")[:blockdevice].should == nil + end + + it "should overwrite any default if blockdevice is explicitly set" do + @class.new(:name => "/foo", :device => '/dev/dsk/c0d0s0', :blockdevice => '/foo')[:blockdevice].should == '/foo' + @class.new(:name => "/foo", :device => "server://share", :fstype => 'nfs', :blockdevice => '/foo')[:blockdevice].should == '/foo' + end + + end + + describe "for fstype" do + + it "should support valid fstypes" do + proc { @class.new(:name => "/foo", :ensure => :present, :fstype => 'ext3') }.should_not raise_error + proc { @class.new(:name => "/foo", :ensure => :present, :fstype => 'proc') }.should_not raise_error + proc { @class.new(:name => "/foo", :ensure => :present, :fstype => 'sysfs') }.should_not raise_error + end + + it "should support auto as a special fstype" do + proc { @class.new(:name => "/foo", :ensure => :present, :fstype => 'auto') }.should_not raise_error + end + + it "should not support whitespace in fstype" do + proc { @class.new(:name => "/foo", :ensure => :present, :fstype => 'ext 3') }.should raise_error Puppet::Error, /fstype.*whitespace/ + end + + end + + describe "for options" do + + it "should support a single option" do + proc { @class.new(:name => "/foo", :ensure => :present, :options => 'ro') }.should_not raise_error + end + + it "should support muliple options as an array" do + proc { @class.new(:name => "/foo", :ensure => :present, :options => ['ro','rsize=4096']) }.should_not raise_error + end + + it "should support an empty array as options" do + proc { @class.new(:name => "/foo", :ensure => :present, :options => []) }.should_not raise_error + end + + it "should not support a comma separated option" do + proc { @class.new(:name => "/foo", :ensure => :present, :options => ['ro','foo,bar','intr']) }.should raise_error Puppet::Error, /option.*have to be specified as an array/ + end + + it "should not support blanks in options" do + proc { @class.new(:name => "/foo", :ensure => :present, :options => ['ro','foo bar','intr']) }.should raise_error Puppet::Error, /option.*whitespace/ + end + + end + + describe "for pass" do + + it "should support numeric values" do + proc { @class.new(:name => "/foo", :ensure => :present, :pass => '0') }.should_not raise_error + proc { @class.new(:name => "/foo", :ensure => :present, :pass => '1') }.should_not raise_error + proc { @class.new(:name => "/foo", :ensure => :present, :pass => '2') }.should_not raise_error + end + + it "should support - on Solaris" do + Facter.stubs(:value).with(:operatingsystem).returns 'Solaris' + proc { @class.new(:name => "/foo", :ensure => :present, :pass => '-') }.should_not raise_error + end + + it "should default to 0 on non Solaris" do + Facter.stubs(:value).with(:operatingsystem).returns 'HP-UX' + @class.new(:name => "/foo", :ensure => :present)[:pass].should == 0 + end + + it "should default to - on Solaris" do + Facter.stubs(:value).with(:operatingsystem).returns 'Solaris' + @class.new(:name => "/foo", :ensure => :present)[:pass].should == '-' + end + + end + + describe "for dump" do + + it "should support 0 as a value for dump" do + proc { @class.new(:name => "/foo", :ensure => :present, :dump => '0') }.should_not raise_error + end + + it "should support 1 as a value for dump" do + proc { @class.new(:name => "/foo", :ensure => :present, :dump => '1') }.should_not raise_error + end + + # stefan: Looks like I'm unable to stub facter here + it "should support 2 as a value for dump on FreeBSD", :if => Facter.value(:operatingsystem) == 'FreeBSD' do + proc { @class.new(:name => "/foo", :ensure => :present, :dump => '2') }.should_not raise_error + end + + # stefan: Looks like I'm unable to stub facter here + it "should not support 2 as a value for dump when not on FreeBSD", :if => Facter.value(:operatingsystem) != 'FreeBSD' do + proc { @class.new(:name => "/foo", :ensure => :present, :dump => '2') }.should raise_error Puppet::Error, /Invalid value/ + end + + it "should default to 0" do + @class.new(:name => "/foo", :ensure => :present)[:dump].should == 0 + end + + end + + describe "for atboot" do + + it "should support true as a value for atboot" do + proc { @class.new(:name => "/foo", :ensure => :present, :atboot => :true) }.should_not raise_error + end + + it "should support false as a value for atboot" do + proc { @class.new(:name => "/foo", :ensure => :present, :atboot => :false) }.should_not raise_error + end + + it "should support yes as a value for atboot" do + proc { @class.new(:name => "/foo", :ensure => :present, :atboot => :yes) }.should_not raise_error + end + + it "should support no as a value for atboot" do + proc { @class.new(:name => "/foo", :ensure => :present, :atboot => :no) }.should_not raise_error + end + + it "should alias true to yes" do + @class.new(:name => "/foo", :ensure => :present, :atboot => :true)[:atboot].should == :yes + end + + it "should alias false to no" do + @class.new(:name => "/foo", :ensure => :present, :atboot => :false)[:atboot].should == :no + end + + it "should not support other values for atboot" do + proc { @class.new(:name => "/foo", :ensure => :present, :atboot => :please_dont) }.should raise_error Puppet::Error, /Invalid value/ + end + + end + + end + + describe "when syncing options" do + + before :each do + @options = @class.attrclass(:options).new(:resource => @resource, :should => %w{rw rsize=2048 wsize=2048}) + end + + it "should pass the sorted joined array to the provider" do + @provider.expects(:options=).with('rsize=2048,rw,wsize=2048') + @options.sync + end + + it "should report out of sync if one option is missing" do + @options.insync?(%w{rw rsize=2048}).should == false + end + + it "should report out of sync if there is an unwanted option" do + @options.insync?(%w{rw rsize=2048 wsize=2048 intr}).should == false + end + + it "should report out of sync if at least one option is incorrect" do + @options.insync?(%w{rw rsize=1024 wsize=2048}).should == false + end + + it "should not care about the order of options" do + @options.insync?(%w{rw rsize=2048 wsize=2048}).should == true + @options.insync?(%w{rw wsize=2048 rsize=2048}).should == true + @options.insync?(%w{rsize=2048 rw wsize=2048}).should == true + @options.insync?(%w{rsize=2048 wsize=2048 rw}).should == true + @options.insync?(%w{wsize=2048 rw rsize=2048}).should == true + @options.insync?(%w{wsize=2048 rsize=2048 rw}).should == true + @options.insync?(%w{rw rsize=2048 wsize=2048}).should == true + end + + end + +end -- 1.7.5.rc3 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
