Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addition of stdlib::getpwnam to return /etc/passwd entry. #1419

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 123 additions & 0 deletions REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ last Period).
* [`stdlib::fqdn_rand_string`](#stdlib--fqdn_rand_string): Generates a random alphanumeric string. Combining the `$fqdn` fact and an
optional seed for repeatable randomness.
* [`stdlib::fqdn_rotate`](#stdlib--fqdn_rotate): Rotates an array or string a random number of times, combining the `fqdn` fact and an optional seed for repeatable randomness.
* [`stdlib::getpwnam`](#stdlib--getpwnam): Given a username returns the user's entry from the `/etc/passwd` file.
* [`stdlib::has_function`](#stdlib--has_function): Returns whether the Puppet runtime has access to a given function.
* [`stdlib::has_interface_with`](#stdlib--has_interface_with): Returns boolean based on network interfaces present and their attribute values.
* [`stdlib::ip_in_range`](#stdlib--ip_in_range): Returns true if the ipaddress is within the given CIDRs
Expand All @@ -139,6 +140,7 @@ Puppet structure
* [`stdlib::seeded_rand_string`](#stdlib--seeded_rand_string): Generates a consistent random string of specific length based on provided seed.
* [`stdlib::sha256`](#stdlib--sha256): Run a SHA256 calculation against a given value.
* [`stdlib::shell_escape`](#stdlib--shell_escape): Escapes a string so that it can be safely used in a Bourne shell command line.
* [`stdlib::sort_by`](#stdlib--sort_by): Sort an Array, Hash or String by mapping values through a given block.
* [`stdlib::start_with`](#stdlib--start_with): Returns true if str starts with one of the prefixes given. Each of the prefixes should be a String.
* [`stdlib::str2resource`](#stdlib--str2resource): This converts a string to a puppet resource.
* [`stdlib::time`](#stdlib--time): This function is deprecated. It implements the functionality of the original non-namespaced stdlib `time` function.
Expand Down Expand Up @@ -3455,6 +3457,58 @@ Data type: `Optional[Variant[Integer,String]]`

One of more values to use as a custom seed. These will be combined with the host's FQDN

### <a name="stdlib--getpwnam"></a>`stdlib::getpwnam`

Type: Ruby 4.x API

>* Note:* The `stdlib::getpwnam` function will work only on platforms that support
`getpwnam`. Typically that is on POSIX like OSes and not Windows.

#### Examples

##### Get a password entry for user steve as a hash.

```puppet
$passwd_entry = stdlib::getpwnam('steve')
```

##### Get the UID of user steve

```puppet
$uid = stdlib::getpwnam('steve')["uid"]
```

#### `stdlib::getpwnam(String $user)`

>* Note:* The `stdlib::getpwnam` function will work only on platforms that support
`getpwnam`. Typically that is on POSIX like OSes and not Windows.

Returns: `Hash` [Hash] For example `{"name"=>"root", "passwd"=>"x", "uid"=>0, "gid"=>0, "gecos"=>"root", "dir"=>"/root", "shell"=>"/bin/bash"}`

Raises:

* `ArgumentError` if no password entry can be found for the specified user.

##### Examples

###### Get a password entry for user steve as a hash.

```puppet
$passwd_entry = stdlib::getpwnam('steve')
```

###### Get the UID of user steve

```puppet
$uid = stdlib::getpwnam('steve')["uid"]
```

##### `user`

Data type: `String`

The username to fetch password record for.

### <a name="stdlib--has_function"></a>`stdlib::has_function`

Type: Ruby 4.x API
Expand Down Expand Up @@ -4003,6 +4057,75 @@ Data type: `Any`

The string to escape

### <a name="stdlib--sort_by"></a>`stdlib::sort_by`

Type: Ruby 4.x API

Sort an Array, Hash or String by mapping values through a given block.

#### Examples

##### Sort local devices according to their used space.

```puppet
$facts['mountpoints'].stdlib::sort_by |$m| { $m.dig(1, 'used_bytes') }
```

#### `stdlib::sort_by(Array $ary, Callable[1,1] &$block)`

The stdlib::sort_by function.

Returns: `Array` Returns an ordered copy of ary.

##### `ary`

Data type: `Array`

The Array to sort.

##### `&block`

Data type: `Callable[1,1]`

The block for transforming elements of ary.

#### `stdlib::sort_by(String $str, Callable[1,1] &$block)`

The stdlib::sort_by function.

Returns: `String` Returns an ordered copy of str.

##### `str`

Data type: `String`

The String to sort.

##### `&block`

Data type: `Callable[1,1]`

The block for transforming elements of str.

#### `stdlib::sort_by(Hash $hsh, Variant[Callable[1,1], Callable[2,2]] &$block)`

The stdlib::sort_by function.

Returns: `Hash` Returns an ordered copy of hsh.

##### `hsh`

Data type: `Hash`

The Hash to sort.

##### `&block`

Data type: `Variant[Callable[1,1], Callable[2,2]]`

The block for transforming elements of hsh.
The block may have arity of one or two.

### <a name="stdlib--start_with"></a>`stdlib::start_with`

Type: Ruby 4.x API
Expand Down
31 changes: 31 additions & 0 deletions lib/puppet/functions/stdlib/getpwnam.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# frozen_string_literal: true

# @summary
# Given a username returns the user's entry from the `/etc/passwd` file.
#
# >* Note:* The `stdlib::getpwnam` function will work only on platforms that support
# `getpwnam`. Typically that is on POSIX like OSes and not Windows.
#
Puppet::Functions.create_function(:'stdlib::getpwnam') do
# @param user
# The username to fetch password record for.
#
# @example Get a password entry for user steve as a hash.
# $passwd_entry = stdlib::getpwnam('steve')
#
# @example Get the UID of user steve
# $uid = stdlib::getpwnam('steve')["uid"]
#
# @raise ArgumentError if no password entry can be found for the specified user.
#
# @return
# [Hash] For example `{"name"=>"root", "passwd"=>"x", "uid"=>0, "gid"=>0, "gecos"=>"root", "dir"=>"/root", "shell"=>"/bin/bash"}`
dispatch :getpwnam do
param 'String', :user
return_type 'Hash'
end

def getpwnam(user)
Etc.getpwnam(user).to_h.first(7).map { |k, v| { k.to_s => v } }.reduce(:merge)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the user isn't found this will probably end up being nil.to_h. Also, why do you even need to do the map? Isn't this sufficient?

Suggested change
Etc.getpwnam(user).to_h.first(7).map { |k, v| { k.to_s => v } }.reduce(:merge)
Etc.getpwnam(user)&.to_h

I'd expect Puppet to translate the symbols to strings.

Copy link
Contributor Author

@traylenator traylenator Mar 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With just Etc.getpwnam(user).to_h you get a Tuple rather than a Hash back - changing the return to Tuple
shows this:

       expected stdlib::getpwnam("steve") to have returned {"name"=>"steve", "passwd"=>"x", "uid"=>1000, "gid"=>1001, "gecos"=>"Steve", "dir"=>"/home/steve", "shell"=>"/bin/fish"} instead of [[:name, "steve"], [:passwd, "x"], [:uid, 1000], [:gid, 1001], [:gecos, "Steve"], [:dir, "/home/steve"], [:shell, "/bin/fish"]]

which does not seem much good.

For the no such user it fails in Etc.getpwnam itself.

ArgumentError(can't find user for notexisting)

which seemed reasonable to me.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With just Etc.getpwnam(user).to_h you get a Tuple rather than a Hash back - changing the return to Tuple

This is odd, perhaps Ruby version dependent?

$ ruby --version
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux]
$ ruby -retc -e "puts Etc.getpwnam('ekohl').to_h"
{:name=>"ekohl", :passwd=>"x", :uid=>1000, :gid=>1000, :gecos=>"ekohl", :dir=>"/home/ekohl", :shell=>"/bin/bash"}

which seemed reasonable to me.

Sounds fair to me. Is there a @raises tag in puppet-strings to document this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the @raises

With Etc.getpwnam(user).to_h :

  • ruby 3.2.2 and puppet 8.5.0 (should switch to 3.3.X I think)
  • ruby 2.7.7 and puppet 7.29.0

I get

[[:name, "steve"], [:passwd, "x"], [:uid, 1000], [:gid, 1001], [:gecos, "Steve"], [:dir, "/home/steve"], [:shell, "/bin/fish"]]

for both.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, I checked on a bunch of versions of ruby and Etc.getpwnam('romain').to_h return a Hash for all of them (2.5, 2.7, 3.1 and 3.2). What you get in your last message seems to be the output of to_a applied on a Hash 🤔.

Also why only keep the 7 first entries? (also #first on a Hash return an Array 😉) All OS do not return the same information, so I think it make sense to keep it unchanged and allow the end-user to consume what they want from the hash?

The suggested change by @ekohl seems to really be what I would expect, can you please double-check this and if you reproduce the issue show us the failing code so that we can better understand what is going on?

end
end
109 changes: 109 additions & 0 deletions spec/acceptance/getpwnam_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
# frozen_string_literal: true

require 'spec_helper_acceptance'

describe 'function stdlib::getpwnam' do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better make the whole test conditional?

Suggested change
describe 'function stdlib::getpwnam' do
describe 'function stdlib::getpwnam', unless os[:family] == 'windows' do

context 'finding the UID of the root user' do
let(:pp) do
<<-MANIFEST
$_password_entry = stdlib::getpwnam('root')
file{ '/tmp/roots_uid.txt':
ensure => file,
content => "${_password_entry['uid']}\n",
}

# Test the use case for systemd module
user{ 'testu':
ensure => present,
uid => 250,
}
MANIFEST
end

it 'works idempotently with no errors' do
apply_manifest(pp, catch_failures: true) unless os[:family] == 'windows'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of repeating the condition (unless os[:family] == 'windows') on each line…

apply_manifest(pp, catch_changes: true) unless os[:family] == 'windows'
end

describe file('/tmp/roots_uid.txt') do
it { is_expected.to contain '0' } unless os[:family] == 'windows'
end
end

context 'put the uid into environment of an exec ' do
let(:pp) do
<<-MANIFEST
exec{ 'env_to_file':
user => 'testu',
environment => Deferred(
'inline_epp',
[
'MY_XDG_RUNTIME_DIR=/run/user/<%= $pwval["uid"] %>',
{ 'pwval' => Deferred('stdlib::getpwnam',['testu'])}
]
),
command => 'echo $MY_XDG_RUNTIME_DIR > /tmp/test-xdg-dir',
path => $facts['path'],
creates => '/tmp/test-xdg-dir',
}
MANIFEST
end

it 'works idempotently with no errors' do
apply_manifest(pp, catch_failures: true) unless os[:family] == 'windows'
apply_manifest(pp, catch_changes: true) unless os[:family] == 'windows'
end

describe file('/tmp/test-xdg-dir') do
it { is_expected.to contain '/run/user/250' } unless os[:family] == 'windows'
it { is_expected.to be_owned_by 'testu' } unless os[:family] == 'windows'
end
end

# This puppet fails on Puppet7
# It fails with " Error: can't find user for testx"
# This is
# https://puppet.atlassian.net/browse/PUP-11526
# Code below should be fine but the fact function is not defined
# which I don't understand so I can't mark as pending on Puppet7.
# context 'put the uid into environment of an exec' do
# let(:pp) do
# <<-MANIFEST
# user{ 'testx':
# ensure => present,
# uid => 251,
# }
# exec{ 'env_to_file':
# user => 'testx',
# environment => Deferred(
# 'inline_epp',
# [
# 'MY_XDG_RUNTIME_DIR=/run/user/<%= $pwval["uid"] %>',
# { 'pwval' => Deferred('stdlib::getpwnam',['testx'])}
# ]
# ),
# command => 'echo $MY_XDG_RUNTIME_DIR > /tmp/testx-xdg-dir',
# path => $facts['path'],
# creates => '/tmp/testx-xdg-dir',
# require => User['testx'],
# }
# MANIFEST
# end
#
# it 'works idempotently with no errors' do
# pending('puppet7 having https://puppet.atlassian.net/browse/PUP-11526') if fact('puppetversion').to_f < 8 && (os[:family] != 'windows')
# apply_manifest(pp, catch_failures: true) unless os[:family] == 'windows'
# apply_manifest(pp, catch_changes: true) unless os[:family] == 'windows'
# end
#
# describe file('/tmp/testx-xdg-dir') do
# it {
# pending('puppet7 having https://puppet.atlassian.net/browse/PUP-11526') if fact('puppetversion').to_f < 8 && (os[:family] != 'windows')
# is_expected.to contain '/run/user/251' unless os[:family] == 'windows'
# }
# it {
# pending('puppet7 having https://puppet.atlassian.net/browse/PUP-11526') if fact('puppetversion').to_f < 8 && (os[:family] != 'windows')
# is_expected.to be_owned_by 'testx' unless os[:family] == 'windows'
# }
# end
end
35 changes: 35 additions & 0 deletions spec/functions/getpwnam_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# frozen_string_literal: true

require 'spec_helper'

describe 'stdlib::getpwnam' do
it { is_expected.not_to be_nil }
it { is_expected.to run.with_params.and_raise_error(ArgumentError, %r{expects 1 argument, got none}i) }

it {
passwd_entry = Etc::Passwd.new(
'steve',
'x',
1000,
1001,
'Steve',
'/home/steve',
'/bin/fish',
)
allow(Etc).to receive(:getpwnam).with(anything, anything).and_call_original
allow(Etc).to receive(:getpwnam).with('steve').and_return(passwd_entry)
is_expected.to run.with_params('steve').and_return(
{
'name' => 'steve',
'passwd' => 'x',
'uid' => 1000,
'gid' => 1001,
'gecos' => 'Steve',
'dir' => '/home/steve',
'shell' => '/bin/fish'
},
)
Comment on lines +10 to +31
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if mocking is the best strategy. I would only have a unit test (and skip it on windows) that get info for a user we are sure will exist (either we get the name of the current user or we look for root) and check that the returned hash include {name: 'root'}

}

it { is_expected.to run.with_params('notexisting').and_raise_error(ArgumentError, %r{can't find user for notexisting}i) }
end
Loading