Since we now have a recent Puppet in ports, I started looking at how packages 
are handled with Puppet.
My current trouble is that it wasn't really possible to install banches 
properly: i.e. can't properly install gimp, 
or auto* based on branch. Or esp, when want to install multiple of them, it was 
just not possible.
For ports where branches conflict, i.e. postfix, this was working, but had to 
specify exact version, and on every
upgrade bump the version .... _very_ annoying.

Currently Puppet allows to install packages of a given version (ensure => 
"X.Y.Z"), or to follow updates (ensure => "latest").
I think for both option, they don't make much sense on OpenBSD. On stable or 
snapshots, forcing a given version is usually
useless, as there is only one. Following latest, is half-baked, as not every 
package is maintained via Puppet, so for example,
it would miss, if just a dependecncy of an installed package gets an update. 
After an update, it's probably more sensical 
to run a general pkg_add -u and being done with it.....

However, the missing support for branches caused me lots of trouble. With 
specifying branches, Puppet can take care to 
either "follow" a given branch, i.e. postfix stable, or stable35. Also with the 
branch support, it now allows to install multiple
packages of same name, i.e. two versions of gimp, or all the auto* tools, if 
you will.

In contrast to a new parameter "branch", the branch to choose has to be 
suffixed to the package name, i.e.:

package { "gimp%stable": ensure => "present"}
package { "postfix%stable": flavor => "ldap", ensure => "present"}
package { "puppetserver%8": ensure => "present"}

Now, that we have to be explicit when installing, we also have to be explicit 
when uninstalling, i.e. in the past, for example, when a given flavor of a 
package was installed, it was enough to:
package { "postfix": ensure => "absent"}

Now, you have to be explicit: 
package { "postfix%stable": flavor => "ldap", ensure => "absent"}

actually, the old behaviour, I think was a bit sloppy, and just worked "by 
accident".


I don't know about any, but, with the proper support of branches, and keeping 
also the support of flavors, would anyone have a use case for providing 
versions on packages ensure, or following updates with ensure => "latest"? If 
anyone really has a use-case for it, it could be re-added if necessary, but 
that would make the package handling in the provider way more complicated, than 
it should be ;)

Any feedback welcome, otherwise, I'd really like to go ahead with it in a 
couple of days, and try to get it upstreamed.

This is just for Puppet 8. Anyone still on Puppet 7? It should be easily ported 
to Puppet 7 as well. 

cheers,
Sebastian


Index: Makefile
===================================================================
RCS file: /cvs/ports/sysutils/ruby-puppet/8/Makefile,v
diff -u -r1.2 Makefile
--- Makefile    20 Mar 2024 21:21:14 -0000      1.2
+++ Makefile    9 Apr 2024 19:51:40 -0000
@@ -1,6 +1,7 @@
 PORTROACH=             limit:^7
 
 VERSION=               8.5.1
+REVISION=              0
 
 RUN_DEPENDS+=          converters/ruby-multi_json,${MODRUBY_FLAVOR}>=1.13,<2 \
                        devel/ruby-concurrent-ruby,${MODRUBY_FLAVOR}>=1,<2 \
Index: patches/patch-lib_puppet_provider_package_openbsd_rb
===================================================================
RCS file: 
/cvs/ports/sysutils/ruby-puppet/8/patches/patch-lib_puppet_provider_package_openbsd_rb,v
diff -u -r1.2 patch-lib_puppet_provider_package_openbsd_rb
--- patches/patch-lib_puppet_provider_package_openbsd_rb        20 Mar 2024 
21:21:14 -0000      1.2
+++ patches/patch-lib_puppet_provider_package_openbsd_rb        9 Apr 2024 
19:51:40 -0000
@@ -1,42 +1,89 @@
-- Handle errors from pkg_add
-- Handle uninstall_options being 'nil' by default
-- If no flavor speficied, force the empty flavor with '--'
-  but skipping the % un-ambiguity pkg names
-- Bail out on shortform PKG_PATH (i.e. 'ftp.openbsd.org')
-- pkg.conf is gone
-- properly handle packages with multiple versions and flavors,
-  i.e. postfix-XXX-flavor
-
+- get rid of versionable (no ensure => "version X.X.X")
+- get rid of upgradeable (ensure => latest)
+- properly support branches
 
 Index: lib/puppet/provider/package/openbsd.rb
 --- lib/puppet/provider/package/openbsd.rb.orig
 +++ lib/puppet/provider/package/openbsd.rb
-@@ -24,6 +24,8 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
-   has_feature :upgradeable
+@@ -6,10 +6,14 @@ require_relative '../../../puppet/provider/package'
+ Puppet::Type.type(:package).provide :openbsd, :parent => 
Puppet::Provider::Package do
+   desc "OpenBSD's form of `pkg_add` support.
+ 
++    OpenBSD has the concept of package branches, providing multiple versions 
of the
++    same package, i.e. `stable` vs. `snapshot`. To select a specific branch,
++    suffix the package name with % sign follwed by the branch name, i.e. 
`gimp%stable`.
++
+     This provider supports the `install_options` and `uninstall_options`
+     attributes, which allow command-line flags to be passed to pkg_add and 
pkg_delete.
+     These options should be specified as an array where each element is 
either a
+-     string or a hash."
++    string or a hash."
+ 
+   commands :pkginfo => "pkg_info",
+            :pkgadd => "pkg_add",
+@@ -18,220 +22,94 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
+   defaultfor 'os.name' => :openbsd
+   confine 'os.name' => :openbsd
+ 
+-  has_feature :versionable
+   has_feature :install_options
+   has_feature :uninstall_options
+-  has_feature :upgradeable
    has_feature :supports_flavors
  
-+  mk_resource_methods
-+
    def self.instances
-     packages = []
- 
-@@ -46,12 +48,6 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
- 
-             packages << new(hash)
-             hash = {}
+-    packages = []
+-
++    final = []
+     begin
+-      execpipe(listcmd) do |process|
+-        # our regex for matching pkg_info output
+-        regex = /^(.*)-(\d[^-]*)[-]?([\w-]*)(.*)$/
+-        fields = [:name, :ensure, :flavor]
+-        hash = {}
++      packages = listcmd
++      packages.each { |package, value|
++        if !package.empty?()
++          value[:provider] = self.name
++          final << new(value)
++        end
++      }
++      return final
+ 
+-        # now turn each returned line into a package object
+-        process.each_line { |line|
+-          match = regex.match(line.split[0])
+-          if match
+-            fields.zip(match.captures) { |field, value|
+-              hash[field] = value
+-            }
+-
+-            hash[:provider] = self.name
+-
+-            packages << new(hash)
+-            hash = {}
 -          else
 -            unless line =~ /Updating the pkgdb/
 -              # Print a warning on lines we can't match, but move
 -              # on, since it should be non-fatal
 -              warning(_("Failed to match line %{line}") % { line: line })
 -            end
-           end
-         }
-       end
-@@ -67,26 +63,17 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
+-          end
+-        }
+-      end
+-
+-      return packages
+     rescue Puppet::ExecutionFailure
+-      return nil
++      nil
+     end
    end
  
-   def latest
+   def self.listcmd
+-    [command(:pkginfo), "-a"]
+-  end
+-
+-  def latest
 -    parse_pkgconf
 -
 -    if @resource[:source][-1, 1] == ::File::SEPARATOR
@@ -45,55 +92,51 @@
 -      e_vars = {}
 -    end
 -
-     if @resource[:flavor]
-       query = "#{@resource[:name]}--#{@resource[:flavor]}"
-     else
+-    if @resource[:flavor]
+-      query = "#{@resource[:name]}--#{@resource[:flavor]}"
+-    else
 -      query = @resource[:name]
-+      query = @resource[:name] + "--"
-     end
- 
+-    end
+-
 -    output = Puppet::Util.withenv(e_vars) { pkginfo "-Q", query }
 -    version = properties[:ensure]
-+    output = Puppet::Util.withenv({}) {pkginfo "-Q", query}
- 
-     if output.nil? or output.size == 0 or output =~ /Error from /
-       debug "Failed to query for #{resource[:name]}"
+-
+-    if output.nil? or output.size == 0 or output =~ /Error from /
+-      debug "Failed to query for #{resource[:name]}"
 -      return version
-+      return properties[:ensure]
-     else
-       # Remove all fuzzy matches first.
-       output = output.split.select { |p| p =~ 
/^#{resource[:name]}-(\d[^-]*)[-]?(\w*)/ }.join
-@@ -95,21 +82,22 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
- 
-     if output =~ /^#{resource[:name]}-(\d[^-]*)[-]?(\w*) \(installed\)$/
-       debug "Package is already the latest available"
+-    else
+-      # Remove all fuzzy matches first.
+-      output = output.split.select { |p| p =~ 
/^#{resource[:name]}-(\d[^-]*)[-]?(\w*)/ }.join
+-      debug "pkg_info -Q for #{resource[:name]}: #{output}"
+-    end
+-
+-    if output =~ /^#{resource[:name]}-(\d[^-]*)[-]?(\w*) \(installed\)$/
+-      debug "Package is already the latest available"
 -      return version
-+      return properties[:ensure]
-     else
-       match = /^(.*)-(\d[^-]*)[-]?(\w*)$/.match(output)
-       debug "Latest available for #{resource[:name]}: #{match[2]}"
- 
+-    else
+-      match = /^(.*)-(\d[^-]*)[-]?(\w*)$/.match(output)
+-      debug "Latest available for #{resource[:name]}: #{match[2]}"
+-
 -      if version.to_sym == :absent || version.to_sym == :purged
-+      if properties[:ensure].to_sym == :absent
-         return match[2]
-       end
- 
-       vcmp = version.split('.').map { |s| s.to_i } <=> 
match[2].split('.').map { |s| s.to_i }
-+      vcmp = properties[:ensure].split('.').map{|s|s.to_i} <=> 
match[2].split('.').map{|s|s.to_i}
-       if vcmp > 0
-         # The locally installed package may actually be newer than what a 
mirror
-         # has. Log it at debug, but ignore it otherwise.
+-        return match[2]
+-      end
+-
+-      vcmp = version.split('.').map { |s| s.to_i } <=> 
match[2].split('.').map { |s| s.to_i }
+-      if vcmp > 0
+-        # The locally installed package may actually be newer than what a 
mirror
+-        # has. Log it at debug, but ignore it otherwise.
 -        debug "Package #{resource[:name]} #{version} newer then available 
#{match[2]}"
 -        return version
-+        debug "Package #{resource[:name]} #{properties[:ensure]} newer then 
available #{match[2]}"
-+        return properties[:ensure]
-       else
-         return match[2]
-       end
-@@ -120,57 +108,25 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
-     self.install(true)
-   end
- 
+-      else
+-        return match[2]
+-      end
+-    end
+-  end
+-
+-  def update
+-    self.install(true)
+-  end
+-
 -  def parse_pkgconf
 -    unless @resource[:source]
 -      if Puppet::FileSystem.exist?("/etc/pkg.conf")
@@ -111,7 +154,20 @@
 -              end
 -            end
 -          end
--        end
++    regex_fuzzy = /^(.*)--([\w-]+)?(%[^w]+)?$/
++    f = []
++    f = pkginfo("-a", "-z").split("\n")
++    packages = {}
++    f.each do |line|
++        match = regex_fuzzy.match(line.split[0])
++        name = match.captures[0]
++        flavor = match.captures[1]
++        branch = match.captures[2]
++        if branch.nil?
++                      pname = name
++        else
++                      pname = name + branch
+         end
 -
 -        unless @resource[:source]
 -          raise Puppet::Error,
@@ -121,14 +177,18 @@
 -        raise Puppet::Error,
 -              _("You must specify a package source or configure an 
installpath in /etc/pkg.conf")
 -      end
--    end
--  end
--
-   def install(latest = false)
++        packages[pname] = { :name => pname, :flavor => flavor, :branch => 
branch, :ensure => "present" }
+     end
++    packages
+   end
+ 
+-  def install(latest = false)
++  def install
      cmd = []
  
 -    parse_pkgconf
--
++    full_name = get_full_name(action="install")
+ 
 -    if @resource[:source][-1, 1] == ::File::SEPARATOR
 -      e_vars = { 'PKG_PATH' => @resource[:source] }
 -      full_name = get_full_name(latest)
@@ -139,27 +199,24 @@
 -
 +    cmd << '-r'
      cmd << install_options
--    cmd << full_name
-+    cmd << get_full_name(latest)
+     cmd << full_name
  
-     if latest
+-    if latest
 -      cmd.unshift('-rz')
-+      cmd.unshift('-z')
-     end
- 
--    Puppet::Util.withenv(e_vars) { pkgadd cmd.flatten.compact }
 +    # pkg_add(1) doesn't set the return value upon failure so we have to peek
 +    # at it's output to see if something went wrong.
 +    output = Puppet::Util.withenv({}) { pkgadd cmd.flatten.compact }
-+    require 'pp'
-+    pp output
 +    if output =~ /Can't find /
 +      self.fail "pkg_add returned: #{output.chomp}"
-+    end
+     end
+-
+-    Puppet::Util.withenv(e_vars) { pkgadd cmd.flatten.compact }
    end
  
-   def get_full_name(latest = false)
-@@ -179,11 +135,20 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
+-  def get_full_name(latest = false)
++  def get_full_name(action="install")
+     # In case of a real update (i.e., the package already exists) then
+     # pkg_add(8) can handle the flavors. However, if we're actually
      # installing with 'latest', we do need to handle the flavors. This is
      # done so we can feed pkg_add(8) the full package name to install to
      # prevent ambiguity.
@@ -168,53 +225,35 @@
 -    elsif latest
 -      # Don't depend on get_version for updates.
 -      @resource[:name]
-+    if resource[:flavor]
-+      # If :ensure contains a version, use that instead of looking it up.
-+      # This allows for installing packages with the same stem, but multiple
-+      # version such as postfix-VERSION-flavor.
-+      if @resource[:ensure].to_s =~ /(\d[^-]*)$/
-+        use_version = @resource[:ensure]
-+      else
-+        use_version = ''
-+      end      
-+      "#{resource[:name]}-#{use_version}-#{resource[:flavor]}"
-+    elsif resource[:name].to_s.match(/[a-z0-9]%[0-9a-z]/i)
-+        "#{resource[:name]}"
-+    elsif not latest
-+      "#{resource[:name]}--"
-     else
-       # If :ensure contains a version, use that instead of looking it up.
-       # This allows for installing packages with the same stem, but multiple
-@@ -194,33 +159,41 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
-         use_version = get_version
-       end
+-    else
+-      # If :ensure contains a version, use that instead of looking it up.
+-      # This allows for installing packages with the same stem, but multiple
+-      # version such as openldap-server.
+-      if @resource[:ensure].to_s =~ /(\d[^-]*)$/
+-        use_version = @resource[:ensure]
+-      else
+-        use_version = get_version
+-      end
  
 -      [@resource[:name], use_version, 
@resource[:flavor]].join('-').gsub(/-+$/, '')
-+      if resource[:flavor]
-+        [ @resource[:name], use_version, 
@resource[:flavor]].join('-').gsub(/-+$/, '')
-+      else
-+        [ @resource[:name], use_version ]
-+      end
++    name_branch_regex = /^(\S*)(%\w*)$/
++    match = name_branch_regex.match(@resource[:name])
++    if match
++      use_name = match.captures[0]
++      use_branch = match.captures[1]
++    else
++      use_name = @resource[:name]
++      use_branch = ''
      end
-   end
+-  end
  
-   def get_version
+-  def get_version
 -    execpipe([command(:pkginfo), "-I", @resource[:name]]) do |process|
 -      # our regex for matching pkg_info output
 -      regex = /^(.*)-(\d[^-]*)[-]?(\w*)(.*)$/
 -      master_version = 0
 -      version = -1
-+    pkg_search_name = @resource[:name]
-+    unless pkg_search_name.match(/[a-z0-9]%[0-9a-z]/i) and not 
@resource[:flavor]
-+      # we are only called when no flavor is specified
-+      # so append '--' to the :name to avoid patch versions on flavors
-+      pkg_search_name << "--"
-+    end
-+    # our regex for matching pkg_info output
-+    regex = /^(.*)-(\d[^-]*)[-]?(\w*)(.*)$/
-+    master_version = 0
-+    version = -1
- 
+-
 -      process.each_line do |line|
 -        match = regex.match(line.split[0])
 -        if match
@@ -224,29 +263,36 @@
 -
 -          master_version = version unless master_version > version
 -        end
-+    # pkg_info -I might return multiple lines, i.e. flavors
-+    matching_pkgs = pkginfo("-I", "pkg_search_name")
-+    matching_pkgs.each_line do |line|
-+      if match = regex.match(line.split[0])
-+        # now we return the first version, unless ensure is latest
-+        version = match.captures[1]
-+        return version unless @resource[:ensure] == "latest"
-+        master_version = version unless master_version > version
-       end
-+    end
- 
+-      end
+-
 -      return master_version unless master_version == 0
 -      return '' if version == -1
-+    return master_version unless master_version == 0
-+    return '' if version == -1
-+    raise Puppet::Error, _("%{version} is not available for this package") % 
{ version: version }
- 
+-
 -      raise Puppet::Error, _("%{version} is not available for this package") 
% { version: version }
--    end
-   rescue Puppet::ExecutionFailure
-     return nil
++    if @resource[:flavor]
++      return "#{use_name}--#{@resource[:flavor]}#{use_branch}"
++    else
++      return "#{use_name}--#{use_branch}"
+     end
+-  rescue Puppet::ExecutionFailure
+-    return nil
++
    end
-@@ -239,7 +212,7 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
+ 
+   def query
+-    # Search for the version info
+-    if pkginfo(@resource[:name]) =~ /Information for 
(inst:)?#{@resource[:name]}-(\S+)/
+-      return { :ensure => $2 }
+-    else
+-      return nil
++    pkg = self.class.instances.find do |package|
++      @resource[:name] == package.name
+     end
++    pkg ? pkg.properties : nil
+   end
+ 
+   def install_options
+@@ -239,15 +117,19 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
    end
  
    def uninstall_options
@@ -254,4 +300,27 @@
 +    [join_options(resource[:uninstall_options])]
    end
  
-   def uninstall
+-  def uninstall
+-    pkgdelete uninstall_options.flatten.compact, @resource[:name]
++  def uninstall(purge = false)
++    if purge
++      pkgdelete "-c", "-qq", uninstall_options.flatten.compact, 
get_full_name(action="uninstall")
++    else
++      pkgdelete uninstall_options.flatten.compact, 
get_full_name(action="uninstall")
++    end
+   end
+ 
+   def purge
+-    pkgdelete "-c", "-q", @resource[:name]
++    uninstall(purge=true)
+   end
+ 
+   def flavor
+@@ -256,7 +138,6 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
+ 
+   def flavor=(value)
+     if flavor != @resource.should(:flavor)
+-      uninstall
+       install
+     end
+   end

Attachment: puppet-package.diff
Description: Binary data

Reply via email to