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

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

Closed
cianyleow opened this issue Jun 14, 2024 · 6 comments · Fixed by #763
Closed

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

cianyleow opened this issue Jun 14, 2024 · 6 comments · Fixed by #763

Comments

@cianyleow
Copy link
Contributor

Versions:

  • mgmt version (eg: mgmt --version):
    mgmt built with this commit

  • operating system/distribution (eg: uname -a):
    Bookworm (via Docker Desktop on an ARM64 Mac)

  • golang version (eg: go version):
    go version go1.22.4 linux/arm64

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@purpleidea
Copy link
Owner

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
Copy link
Owner

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

@cianyleow
Copy link
Contributor Author

Related PR for auto edges: #764

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants