Puppet TAPP: NetApp ONTAP Module Review

netapp_ontap_module_review.md

NetApp ONTAP Module

Reference documentation:

• Puppet Visual Index
• Puppet Type Reference
• Puppet Style Guide
• Puppet Facter: Core Facts
• Puppet Custom Functions
• Puppet Custom Facts
• Puppet: Updating manifests for 3.x to 4.x

General Notes:

• Adherence to Puppet Style
  • Please review the Puppet Style Guide
  • For instance, alignment of hash rockets
• Puppet types have weak to no validation
  • Should bolster your types by providing constraints on user provided data and conformance standards
• NetAppTruthy property provided but no evidence of usage
  • Make use of truthy type or remove unnecessary code
• Many providers do not have destroy methods
  • Was not sure if this was intentional
    • lib/puppet/provider/netapp_vserver_cifs_options/cmode.rb
    • lib/puppet/provider/netapp_vserver_cifs_domain_password_schedule/cmode.rb
    • lib/puppet/provider/netapp_system_node_autosupport/cmode.rb
• Puppet manifest classes are not documented in known standards
  • puppet describe face depends on Puppet strings style of documentation
    • Reference: Puppet strings
    • Example: init.pp from puppetlabs/ntp
• Data types should be used for parameters in Puppet DSL classes
  • Data types
  • Abstract data types
• Puppet DSL should contain data validation
  • Should provide proper validation of data provided by module users
    • Use data types where possible as this will provide basic validation
    • Provide additonal validation via custom functions
      • Example: validate_ipaddress

README.md

• Examples are in a node definition format (i.e., site.pp manifest)
  • This is an anti-pattern. Customers/partners are educated on the role & profile design pattern
• Consistency of code examples
  • Some examples are language specific and some are not
  • Instead of using three ticks use three ticks immediately proceeded by a language name. Works for other languages as well on GitHub's branded Markdown. Here is the README and the fully supported lanuage list
  • i.e.,

  netapp_lif { 'vserver01_lif':
    ensure        => present,
    homeport      => 'e0c',
    homenode      => 'ontap01',
    address       => '10.0.207.5',
    vserver       => 'vserver01',
    netmask       => '255.255.255.0',
    dataprotocols => ['nfs'],
  }

lib/puppet/util/network_device/netapp/facts.rb

• https://github.com/NetApp/ONTAP-Puppet/blob/07da7d21a4d69e98e4857a48e149298b60047c7d/lib/puppet/util/network_device/netapp/facts.rb#L39L46
  • Custom facts should use underscores in naming of facts, not hyphens

lib/puppet/type/netapp_nfs.rb

• https://github.com/NetApp/ONTAP-Puppet/blob/07da7d21a4d69e98e4857a48e149298b60047c7d/lib/puppet/type/netapp_nfs.rb#L44L60
  • Commented code should be removed

init.pp

• No code in this class
• Consider removing or provide additional Puppet DSL documentation here

iscsi.pp

• Needs Puppet strings formatted documentation
• No validation
  • Should provide proper validation of data provided by module users
    • Use data types where possible
    • Provide additonal validation via custom functions
      • Example: validate_ipaddress
• Word style ordering metaparams prefered for linking types
• require, before, subscribe, notify over arrows when ordering types
  • Puppet type ordering
• https://github.com/NetApp/ONTAP-Puppet/blob/07da7d21a4d69e98e4857a48e149298b60047c7d/manifests/iscsi.pp#L84L88
  • Needs to adhere to Puppet style guide
• Data types should be used for parameters in Puppet DSL classes
  • Data types
  • Abstract data types

nfs.pp

• Needs Puppet strings formatted documentation
• No validation
  • Should provide proper validation of data provided by module users
    • Use data types where possible
    • Provide additonal validation via custom functions
      • Example: validate_ipaddress
• Word style ordering metaparams prefered for linking Puppet types
  • require, before, subscribe, notify over arrows when ordering types
    • Puppet type ordering
• https://github.com/NetApp/ONTAP-Puppet/blob/07da7d21a4d69e98e4857a48e149298b60047c7d/manifests/nfs.pp#L70L72
  • Needs to adhere to Puppet style Guide
    • Line spacing between types should be consistent
• https://github.com/NetApp/ONTAP-Puppet/blob/07da7d21a4d69e98e4857a48e149298b60047c7d/manifests/nfs.pp#L70
  • Interpolation of string not required
  • Remove quotes and pass variable alone
• Data types should be used for parameters in Puppet DSL classes
  • Data types
  • Abstract data types

svm.pp

• Needs Puppet strings formatted documentation
• No validation
  • Should provide proper validation of data provided by module users
    • Use data types where possible
    • Provide additonal validation via custom functions
      • Example: validate_ipaddress
• Word style ordering metaparams preferred for linking Puppet types
  • https://github.com/NetApp/ONTAP-Puppet/blob/07da7d21a4d69e98e4857a48e149298b60047c7d/manifests/svm.pp#L76
• https://github.com/NetApp/ONTAP-Puppet/blob/07da7d21a4d69e98e4857a48e149298b60047c7d/manifests/svm.pp#L70
  • Interpolation of string not required
  • Remove quotes and pass variable alone
• Data types should be used for parameters in Puppet DSL classes
  • Data types
  • Abstract data types

vqe.pp

• Needs Puppet strings formatted documentation
• No validation
  • Should provide proper validation of data provided by module users
    • Use data types where possible
    • Provide additonal validation via custom functions
      • Example: validate_ipaddress
• Data types should be used for parameters in Puppet DSL classes
  • Data types
  • Abstract data types