[RFC PATCH] fstests: Request inferred user

[RFC PATCH] fstests: Request inferred user

[Frederick Lawler]

Currently when running make fstests for bare-metal setups, the data_user
'vagrant' is set for the /data partition. This is currently set as the default
data_user. We have the option to edit this in extra_vars.yml.

make linux checks for an inferred user prior to setting the uid/gid for
the /data partition.

In the situation where we don't add the 'vagrant' user to the
bare-metal, we get a "user does not exist" on make fstests.

In both cases we do not want to switch users between the two targets,
nor do we want to necessarily ensure that the 'vagrant' user exists on
the metal. Therefore, request the inferred user on make fstests.

Signed-off-by: Frederick Lawler <fred@cloudflare.com>
---
Since this is essentially in two places, I figure this should be more
generic and then included in other make targets like cxl. But this is to
get the discussion going if this or some other generic approach is
preferred.
---
 playbooks/roles/fstests/tasks/main.yml | 37 ++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/playbooks/roles/fstests/tasks/main.yml b/playbooks/roles/fstests/tasks/main.yml
index e2d7ce5295b7..2898833b0732 100644
--- a/playbooks/roles/fstests/tasks/main.yml
+++ b/playbooks/roles/fstests/tasks/main.yml
@@ -10,6 +10,43 @@
       skip: true
   tags: vars
 
+- name: Get username we are using
+  command:
+    cmd: whoami
+  register: username_on_target
+  when:
+    - infer_uid_and_group|bool
+
+- name: Set target user as a fact
+  set_fact:
+    target_user: "{{ username_on_target.stdout }}"
+  when:
+    - infer_uid_and_group|bool
+
+- name: Run getent against the inferred target user
+  getent:
+    database: passwd
+    key: "{{ target_user }}"
+  register: getent_running_user
+  when:
+    - infer_uid_and_group|bool
+
+- name: Run getent against the inferred target group
+  getent:
+    database: group
+    key: "{{ target_user }}"
+  register: getent_on_group
+  when:
+    - infer_uid_and_group|bool
+
+- name: Override user and group with inferred settings if feature is enabled
+  set_fact:
+    user: "hplip"
+    data_user: "{{ target_user }}"
+    data_group: "{{ ((getent_on_group.values() | first).values() | first).keys() | first }}"
+  when:
+    - infer_uid_and_group|bool
+
 - include_role:
     name: create_data_partition
   tags: [ 'oscheck', 'data_partition' ]
-- 
2.34.1

Re: [RFC PATCH] fstests: Request inferred user

[Jeff Layton]

On Wed, 2023-08-30 at 18:53 -0500, Frederick Lawler wrote:
> Currently when running make fstests for bare-metal setups, the data_user
> 'vagrant' is set for the /data partition. This is currently set as the default
> data_user. We have the option to edit this in extra_vars.yml.
> 
> make linux checks for an inferred user prior to setting the uid/gid for
> the /data partition.
> 
> In the situation where we don't add the 'vagrant' user to the
> bare-metal, we get a "user does not exist" on make fstests.
> 
> In both cases we do not want to switch users between the two targets,
> nor do we want to necessarily ensure that the 'vagrant' user exists on
> the metal. Therefore, request the inferred user on make fstests.
> 
> Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> ---
> Since this is essentially in two places, I figure this should be more
> generic and then included in other make targets like cxl. But this is to
> get the discussion going if this or some other generic approach is
> preferred.
> ---
>  playbooks/roles/fstests/tasks/main.yml | 37 ++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/playbooks/roles/fstests/tasks/main.yml b/playbooks/roles/fstests/tasks/main.yml
> index e2d7ce5295b7..2898833b0732 100644
> --- a/playbooks/roles/fstests/tasks/main.yml
> +++ b/playbooks/roles/fstests/tasks/main.yml
> @@ -10,6 +10,43 @@
>        skip: true
>    tags: vars
>  
> +- name: Get username we are using
> +  command:
> +    cmd: whoami
> +  register: username_on_target
> +  when:
> +    - infer_uid_and_group|bool
> +
> +- name: Set target user as a fact
> +  set_fact:
> +    target_user: "{{ username_on_target.stdout }}"
> +  when:
> +    - infer_uid_and_group|bool
> +
> +- name: Run getent against the inferred target user
> +  getent:
> +    database: passwd
> +    key: "{{ target_user }}"
> +  register: getent_running_user
> +  when:
> +    - infer_uid_and_group|bool
> +
> +- name: Run getent against the inferred target group
> +  getent:
> +    database: group
> +    key: "{{ target_user }}"
> +  register: getent_on_group
> +  when:
> +    - infer_uid_and_group|bool
> +
> +- name: Override user and group with inferred settings if feature is enabled
> +  set_fact:
> +    user: "hplip"

Why "hplip" here?

> +    data_user: "{{ target_user }}"
> +    data_group: "{{ ((getent_on_group.values() | first).values() | first).keys() | first }}"
> +  when:
> +    - infer_uid_and_group|bool
> +
>  - include_role:
>      name: create_data_partition
>    tags: [ 'oscheck', 'data_partition' ]

-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH] fstests: Request inferred user

[Frederick Lawler]

On Thu, Aug 31, 2023 at 06:13:23AM -0400, Jeff Layton wrote:
> On Wed, 2023-08-30 at 18:53 -0500, Frederick Lawler wrote:
> > Currently when running make fstests for bare-metal setups, the data_user
> > 'vagrant' is set for the /data partition. This is currently set as the default
> > data_user. We have the option to edit this in extra_vars.yml.
> > 
> > make linux checks for an inferred user prior to setting the uid/gid for
> > the /data partition.
> > 
> > In the situation where we don't add the 'vagrant' user to the
> > bare-metal, we get a "user does not exist" on make fstests.
> > 
> > In both cases we do not want to switch users between the two targets,
> > nor do we want to necessarily ensure that the 'vagrant' user exists on
> > the metal. Therefore, request the inferred user on make fstests.
> > 
> > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > ---
> > Since this is essentially in two places, I figure this should be more
> > generic and then included in other make targets like cxl. But this is to
> > get the discussion going if this or some other generic approach is
> > preferred.
> > ---
> >  playbooks/roles/fstests/tasks/main.yml | 37 ++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> > 
> > diff --git a/playbooks/roles/fstests/tasks/main.yml b/playbooks/roles/fstests/tasks/main.yml
> > index e2d7ce5295b7..2898833b0732 100644
> > --- a/playbooks/roles/fstests/tasks/main.yml
> > +++ b/playbooks/roles/fstests/tasks/main.yml
> > @@ -10,6 +10,43 @@
> >        skip: true
> >    tags: vars
> >  
> > +- name: Get username we are using
> > +  command:
> > +    cmd: whoami
> > +  register: username_on_target
> > +  when:
> > +    - infer_uid_and_group|bool
> > +
> > +- name: Set target user as a fact
> > +  set_fact:
> > +    target_user: "{{ username_on_target.stdout }}"
> > +  when:
> > +    - infer_uid_and_group|bool
> > +
> > +- name: Run getent against the inferred target user
> > +  getent:
> > +    database: passwd
> > +    key: "{{ target_user }}"
> > +  register: getent_running_user
> > +  when:
> > +    - infer_uid_and_group|bool
> > +
> > +- name: Run getent against the inferred target group
> > +  getent:
> > +    database: group
> > +    key: "{{ target_user }}"
> > +  register: getent_on_group
> > +  when:
> > +    - infer_uid_and_group|bool
> > +
> > +- name: Override user and group with inferred settings if feature is enabled
> > +  set_fact:
> > +    user: "hplip"
> 
> Why "hplip" here?

This was mostly copy/pasted from
./playbooks/roles/bootlinux/tasks/main.yml. I see it was introduced in commit
f10add06446b ("kdevops: remove ansible galaxy dependency") for that file,
but I'm unclear the use there. I'd like to know as well.

> 
> > +    data_user: "{{ target_user }}"
> > +    data_group: "{{ ((getent_on_group.values() | first).values() | first).keys() | first }}"
> > +  when:
> > +    - infer_uid_and_group|bool
> > +
> >  - include_role:
> >      name: create_data_partition
> >    tags: [ 'oscheck', 'data_partition' ]
> 
> -- 
> Jeff Layton <jlayton@kernel.org>

Fred

Re: [RFC PATCH] fstests: Request inferred user

[Luis Chamberlain]

On Thu, Aug 31, 2023 at 08:51:22AM -0500, Frederick Lawler wrote:
> On Thu, Aug 31, 2023 at 06:13:23AM -0400, Jeff Layton wrote:
> > On Wed, 2023-08-30 at 18:53 -0500, Frederick Lawler wrote:
> > > Currently when running make fstests for bare-metal setups, the data_user
> > > 'vagrant' is set for the /data partition. This is currently set as the default
> > > data_user. We have the option to edit this in extra_vars.yml.
> > > 
> > > make linux checks for an inferred user prior to setting the uid/gid for
> > > the /data partition.
> > > 
> > > In the situation where we don't add the 'vagrant' user to the
> > > bare-metal, we get a "user does not exist" on make fstests.
> > > 
> > > In both cases we do not want to switch users between the two targets,
> > > nor do we want to necessarily ensure that the 'vagrant' user exists on
> > > the metal. Therefore, request the inferred user on make fstests.
> > > 
> > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > ---
> > > Since this is essentially in two places, I figure this should be more
> > > generic and then included in other make targets like cxl. But this is to
> > > get the discussion going if this or some other generic approach is
> > > preferred.
> > > ---
> > >  playbooks/roles/fstests/tasks/main.yml | 37 ++++++++++++++++++++++++++
> > >  1 file changed, 37 insertions(+)
> > > 
> > > diff --git a/playbooks/roles/fstests/tasks/main.yml b/playbooks/roles/fstests/tasks/main.yml
> > > index e2d7ce5295b7..2898833b0732 100644
> > > --- a/playbooks/roles/fstests/tasks/main.yml
> > > +++ b/playbooks/roles/fstests/tasks/main.yml
> > > @@ -10,6 +10,43 @@
> > >        skip: true
> > >    tags: vars
> > >  
> > > +- name: Get username we are using
> > > +  command:
> > > +    cmd: whoami
> > > +  register: username_on_target
> > > +  when:
> > > +    - infer_uid_and_group|bool
> > > +
> > > +- name: Set target user as a fact
> > > +  set_fact:
> > > +    target_user: "{{ username_on_target.stdout }}"
> > > +  when:
> > > +    - infer_uid_and_group|bool
> > > +
> > > +- name: Run getent against the inferred target user
> > > +  getent:
> > > +    database: passwd
> > > +    key: "{{ target_user }}"
> > > +  register: getent_running_user
> > > +  when:
> > > +    - infer_uid_and_group|bool
> > > +
> > > +- name: Run getent against the inferred target group
> > > +  getent:
> > > +    database: group
> > > +    key: "{{ target_user }}"
> > > +  register: getent_on_group
> > > +  when:
> > > +    - infer_uid_and_group|bool
> > > +
> > > +- name: Override user and group with inferred settings if feature is enabled
> > > +  set_fact:
> > > +    user: "hplip"
> > 
> > Why "hplip" here?
> 
> This was mostly copy/pasted from
> ./playbooks/roles/bootlinux/tasks/main.yml. I see it was introduced in commit
> f10add06446b ("kdevops: remove ansible galaxy dependency") for that file,
> but I'm unclear the use there. I'd like to know as well.

I recall fstets having odd user requirements but that was just for some
odd tests. I can't trace hplip down to anything so I say we just remove
it for both.

  Luis

Re: [RFC PATCH] fstests: Request inferred user

[Jeff Layton]

On Thu, 2023-08-31 at 12:30 -0700, Luis Chamberlain wrote:
> On Thu, Aug 31, 2023 at 08:51:22AM -0500, Frederick Lawler wrote:
> > On Thu, Aug 31, 2023 at 06:13:23AM -0400, Jeff Layton wrote:
> > > On Wed, 2023-08-30 at 18:53 -0500, Frederick Lawler wrote:
> > > > Currently when running make fstests for bare-metal setups, the data_user
> > > > 'vagrant' is set for the /data partition. This is currently set as the default
> > > > data_user. We have the option to edit this in extra_vars.yml.
> > > > 
> > > > make linux checks for an inferred user prior to setting the uid/gid for
> > > > the /data partition.
> > > > 
> > > > In the situation where we don't add the 'vagrant' user to the
> > > > bare-metal, we get a "user does not exist" on make fstests.
> > > > 
> > > > In both cases we do not want to switch users between the two targets,
> > > > nor do we want to necessarily ensure that the 'vagrant' user exists on
> > > > the metal. Therefore, request the inferred user on make fstests.
> > > > 
> > > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > > ---
> > > > Since this is essentially in two places, I figure this should be more
> > > > generic and then included in other make targets like cxl. But this is to
> > > > get the discussion going if this or some other generic approach is
> > > > preferred.
> > > > ---
> > > >  playbooks/roles/fstests/tasks/main.yml | 37 ++++++++++++++++++++++++++
> > > >  1 file changed, 37 insertions(+)
> > > > 
> > > > diff --git a/playbooks/roles/fstests/tasks/main.yml b/playbooks/roles/fstests/tasks/main.yml
> > > > index e2d7ce5295b7..2898833b0732 100644
> > > > --- a/playbooks/roles/fstests/tasks/main.yml
> > > > +++ b/playbooks/roles/fstests/tasks/main.yml
> > > > @@ -10,6 +10,43 @@
> > > >        skip: true
> > > >    tags: vars
> > > >  
> > > > +- name: Get username we are using
> > > > +  command:
> > > > +    cmd: whoami
> > > > +  register: username_on_target
> > > > +  when:
> > > > +    - infer_uid_and_group|bool
> > > > +
> > > > +- name: Set target user as a fact
> > > > +  set_fact:
> > > > +    target_user: "{{ username_on_target.stdout }}"
> > > > +  when:
> > > > +    - infer_uid_and_group|bool
> > > > +
> > > > +- name: Run getent against the inferred target user
> > > > +  getent:
> > > > +    database: passwd
> > > > +    key: "{{ target_user }}"
> > > > +  register: getent_running_user
> > > > +  when:
> > > > +    - infer_uid_and_group|bool
> > > > +
> > > > +- name: Run getent against the inferred target group
> > > > +  getent:
> > > > +    database: group
> > > > +    key: "{{ target_user }}"
> > > > +  register: getent_on_group
> > > > +  when:
> > > > +    - infer_uid_and_group|bool
> > > > +
> > > > +- name: Override user and group with inferred settings if feature is enabled
> > > > +  set_fact:
> > > > +    user: "hplip"
> > > 
> > > Why "hplip" here?
> > 
> > This was mostly copy/pasted from
> > ./playbooks/roles/bootlinux/tasks/main.yml. I see it was introduced in commit
> > f10add06446b ("kdevops: remove ansible galaxy dependency") for that file,
> > but I'm unclear the use there. I'd like to know as well.
> 
> I recall fstets having odd user requirements but that was just for some
> odd tests. I can't trace hplip down to anything so I say we just remove
> it for both.

If you have an HP printer/scanner/copier then there is an "hplip"
package for Fedora with some of their drivers and such.

That's why it seemed so weird here.

-- 
Jeff Layton <jlayton@kernel.org>

Re: [RFC PATCH] fstests: Request inferred user

[Luis Chamberlain]

On Thu, Aug 31, 2023 at 03:35:47PM -0400, Jeff Layton wrote:
> On Thu, 2023-08-31 at 12:30 -0700, Luis Chamberlain wrote:
> > On Thu, Aug 31, 2023 at 08:51:22AM -0500, Frederick Lawler wrote:
> > > On Thu, Aug 31, 2023 at 06:13:23AM -0400, Jeff Layton wrote:
> > > > On Wed, 2023-08-30 at 18:53 -0500, Frederick Lawler wrote:
> > > > > Currently when running make fstests for bare-metal setups, the data_user
> > > > > 'vagrant' is set for the /data partition. This is currently set as the default
> > > > > data_user. We have the option to edit this in extra_vars.yml.
> > > > > 
> > > > > make linux checks for an inferred user prior to setting the uid/gid for
> > > > > the /data partition.
> > > > > 
> > > > > In the situation where we don't add the 'vagrant' user to the
> > > > > bare-metal, we get a "user does not exist" on make fstests.
> > > > > 
> > > > > In both cases we do not want to switch users between the two targets,
> > > > > nor do we want to necessarily ensure that the 'vagrant' user exists on
> > > > > the metal. Therefore, request the inferred user on make fstests.
> > > > > 
> > > > > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > > > > ---
> > > > > Since this is essentially in two places, I figure this should be more
> > > > > generic and then included in other make targets like cxl. But this is to
> > > > > get the discussion going if this or some other generic approach is
> > > > > preferred.
> > > > > ---
> > > > >  playbooks/roles/fstests/tasks/main.yml | 37 ++++++++++++++++++++++++++
> > > > >  1 file changed, 37 insertions(+)
> > > > > 
> > > > > diff --git a/playbooks/roles/fstests/tasks/main.yml b/playbooks/roles/fstests/tasks/main.yml
> > > > > index e2d7ce5295b7..2898833b0732 100644
> > > > > --- a/playbooks/roles/fstests/tasks/main.yml
> > > > > +++ b/playbooks/roles/fstests/tasks/main.yml
> > > > > @@ -10,6 +10,43 @@
> > > > >        skip: true
> > > > >    tags: vars
> > > > >  
> > > > > +- name: Get username we are using
> > > > > +  command:
> > > > > +    cmd: whoami
> > > > > +  register: username_on_target
> > > > > +  when:
> > > > > +    - infer_uid_and_group|bool
> > > > > +
> > > > > +- name: Set target user as a fact
> > > > > +  set_fact:
> > > > > +    target_user: "{{ username_on_target.stdout }}"
> > > > > +  when:
> > > > > +    - infer_uid_and_group|bool
> > > > > +
> > > > > +- name: Run getent against the inferred target user
> > > > > +  getent:
> > > > > +    database: passwd
> > > > > +    key: "{{ target_user }}"
> > > > > +  register: getent_running_user
> > > > > +  when:
> > > > > +    - infer_uid_and_group|bool
> > > > > +
> > > > > +- name: Run getent against the inferred target group
> > > > > +  getent:
> > > > > +    database: group
> > > > > +    key: "{{ target_user }}"
> > > > > +  register: getent_on_group
> > > > > +  when:
> > > > > +    - infer_uid_and_group|bool
> > > > > +
> > > > > +- name: Override user and group with inferred settings if feature is enabled
> > > > > +  set_fact:
> > > > > +    user: "hplip"
> > > > 
> > > > Why "hplip" here?
> > > 
> > > This was mostly copy/pasted from
> > > ./playbooks/roles/bootlinux/tasks/main.yml. I see it was introduced in commit
> > > f10add06446b ("kdevops: remove ansible galaxy dependency") for that file,
> > > but I'm unclear the use there. I'd like to know as well.
> > 
> > I recall fstets having odd user requirements but that was just for some
> > odd tests. I can't trace hplip down to anything so I say we just remove
> > it for both.
> 
> If you have an HP printer/scanner/copier then there is an "hplip"
> package for Fedora with some of their drivers and such.
> 
> That's why it seemed so weird here.

Indeed, super odd! Well good riddance, I nuked it.

  Luis

Re: [RFC PATCH] fstests: Request inferred user

[Luis Chamberlain]

On Wed, Aug 30, 2023 at 06:53:33PM -0500, Frederick Lawler wrote:
> Currently when running make fstests for bare-metal setups, the data_user
> 'vagrant' is set for the /data partition. This is currently set as the default
> data_user. We have the option to edit this in extra_vars.yml.
> 
> make linux checks for an inferred user prior to setting the uid/gid for
> the /data partition.
> 
> In the situation where we don't add the 'vagrant' user to the
> bare-metal, we get a "user does not exist" on make fstests.
> 
> In both cases we do not want to switch users between the two targets,
> nor do we want to necessarily ensure that the 'vagrant' user exists on
> the metal. Therefore, request the inferred user on make fstests.
> 
> Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> ---
> Since this is essentially in two places, I figure this should be more
> generic and then included in other make targets like cxl. But this is to
> get the discussion going if this or some other generic approach is
> preferred.

Looks good to me, sharing is certainly desriable so we have a bug in
just one place. You can do that later.

I've applied this patch and then also removed hlip user from both
places. We'll see what breaks, I can't think of anything that should
depend on it.

What's your github username or gitlab? You should just get access to
commit.

  Luis