purpleidea / mgmt

Next generation distributed, event-driven, parallel config management!
https://purpleidea.com/tags/mgmtconfig/
GNU General Public License v3.0
3.6k stars 312 forks source link

File Resource Checks Host for User/Group in Validation #762

Closed cianyleow closed 3 months ago

cianyleow commented 3 months ago

Versions:

Description:

When attempting to create a file that is owned by a user/group that are being managed within the resource graph, mgmt fails early with an error during the Validate stage (before any resources have been managed) because the user/group do not exist on the host (checks are here).

A sample mcl that demonstrates this is below:

group "mygroup" {
  state => "exists",
  gid => 1234,
}

user "myuser" {
  state => "exists",
  uid => 1234,
  group => "mygroup",
}

file "/tmp/myfile" {
  state => "exists",
  content => "mycontents",
  owner => "myuser",
  group => "mygroup",
}

When run, mgmt logs an error and then is not able to manage the resources:

user@host: mgmt run lang example.mcl
...start up
main: graph validate failed: file[/tmp/myfile] did not Validate: user lookup error (myuser): user: unknown user myuser
main: waiting...
...and then nothing happens
purpleidea commented 3 months ago

This is a bug. Those checks in Validate should be removed. (I think they already exist in CheckApply.)

We also need to add automatic edges if they aren't already there. (Or in the meantime you should add edges.)

cianyleow commented 3 months ago

Ah ok, I wasn't sure if the intention here was to check the graph - is there another step where we check the integrity of the graph, or is it in CheckApply?

cianyleow commented 3 months ago

I'll raise a PR with AutoEdges for owner/group, I'm just working on some tests for it

purpleidea commented 3 months ago

I wasn't sure if the intention here was to check the graph - is there another step where we check the integrity of the graph

What do you mean?

Validate() is a step just to catch anything that is definitely wrong with the graph before we run it.

CheckApply() is the run-time check state and potentially change it (apply) method. It can error if we've asked it to do something impossible such as setting an owner that doesn't exist.

I'll raise a PR with AutoEdges for owner/group, I'm just working on some tests for it

Awesome!

purpleidea commented 3 months ago

/cc @frebib since landing your autoedges rework is related here.

cianyleow commented 3 months ago

Related PR for auto edges: https://github.com/purpleidea/mgmt/pull/764