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

The --static-dir flag is not respected #340

Closed
alerque opened this issue May 5, 2021 · 12 comments
Closed

The --static-dir flag is not respected #340

alerque opened this issue May 5, 2021 · 12 comments
Labels
bug Something isn't working

Comments

@alerque
Copy link
Contributor

alerque commented May 5, 2021

The --static-dir CLI flag seems to be completely ignored. I can pass it a path to the static resources and it throws an error about not being able to find them. If instead I cd into the static resources dir I previously mentioned as an arg and skip the arg it finds the resource files.

$ cd /tmp

$ sudo -u listmonk listmonk --config /etc/listmonk/config.toml --static-dir /usr/share/listmonk
2021/05/05 16:10:55 init.go:123: using local filesystem for static assets
2021/05/05 16:10:55 init.go:143: failed to initialize local file for assets: stat config.toml.sample: permission denied

$ cd /usr/share/listmonk

$ sudo -u listmonk listmonk --config /etc/listmonk/config.toml
2021/05/05 16:13:05 init.go:123: using local filesystem for static assets

The latter invocation works, but the former should have.

@knadh
Copy link
Owner

knadh commented May 6, 2021

Definitely something to do with permissions on your system. This feature works. Here's it running from the binary downloaded from the releases page. Custom config path and custom static dir path.

$ ./listmonk --config=../config.toml --static-dir=/tmp/listmonk/static
2021/05/06 12:42:03 main.go:84: v0.9.0-beta (e90fb1d 2021-02-13T12:41:40Z)
2021/05/06 12:42:03 init.go:98: reading config: ../config.toml
2021/05/06 12:42:03 init.go:175: connecting to db: localhost:9432/listmonk
2021/05/06 12:42:03 init.go:149: loading static files from: /tmp/listmonk/static
2021/05/06 12:42:03 init.go:420: media upload provider: filesystem
2021/05/06 12:42:03 init.go:344: loaded email (SMTP) messenger: [email protected]
⇨ http server started on [::]:9000

@alerque
Copy link
Contributor Author

alerque commented May 6, 2021

@knadh I'm using a pretty bone-stock Linux setup, no special ACLs etc.

By the way this whole procedure is for setting up packaging on Arch Linux. I'm testing on several systems including stripped down minimal systems just making sure this package works.

Can you show a few more details from your working arrangement? Maybe we can spot the difference. First of all I'm suspicious of you running ./listmonk. That suggests you are perhaps running inside the source directory where you compiled it, and almost certainly running as the same user that owns the binary. Your /tmp dir for statics is probably owned as the same as your UID & the binary owner too. Those should not be prerequisites.

Lets see the output of these:

$ stat ./listmonk /tmp/listmonk/static $(pwd)
$ id

For context here are relevant lines from my system(s). The difference I expect to see that is not reflected in your testing is that the owner of the binary and static files is the system, but running as a non-privileged user (with rx permission on the binary and r on the statics (+x for dir).

$ stat $(which listmonk) /usr/share/listmonk
  File: /usr/bin/listmonk
  Size: 15922472        Blocks: 31104      IO Block: 4096   regular file
Device: 1ah/26d Inode: 440712      Links: 1
Access: (0755/-rwxr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
  File: /usr/share/listmonk
  Size: 114             Blocks: 0          IO Block: 4096   directory
Device: 1ah/26d Inode: 440715      Links: 1
Access: (0755/drwxr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
$ id
uid=966(listmonk) gid=966(listmonk) groups=966(listmonk)

Can you confirm that it still works for you if the binary and statics dir are owned by root but run by your user?

@alerque
Copy link
Contributor Author

alerque commented May 6, 2021

Another clue, although I don't really understand how to interpret it, is that I'm seeing this error even when running from inside the statics folder (so a fully working system that I ran a test campaign on). Specifying the statics folder should probably keep the binary from even trying to access embedded files but it evidently does not.

2021/05/06 10:17:54 init.go:123: unable to initialize embedded filesystem: no ID found in the file

@alerque
Copy link
Contributor Author

alerque commented May 6, 2021

New idea: this bug is probably related to not using stuffbin to embed assets. These builds are expected to use a static directory to serve assets. The binary is being used after being built by go without the post-processing that the upstream make dist or make pack-bin steps take.

Other than this snafu it seems to work fine that way. The service runs, the admin site is served, mail campaigns run, etc.

Apparently when it fails to find the embed resources it also fails to process the CLI flag for where to find them and only looks at the current working directory.

@knadh
Copy link
Owner

knadh commented May 6, 2021

First of all I'm suspicious of you running ./listmonk. That suggests you are perhaps running inside the source directory where you compiled it

That would be a futile test! As I said, it's a fresh binary downloaded from the releases page (and as I already said, this release has been running in production in many places.)

Binary in a directory.

kailash@nova (/tmp/listmonk_0.9.0-beta_linux_amd64)
$ ls -lh
total 12M
-rwxr-xr-x. 1 kailash kailash 12M Feb 13 18:11 listmonk

Config and static dir in another.

kailash@nova (/tmp/listmonk)
$ ls -lh
total 4.0K
-rw-r--r--. 1 kailash kailash 288 May  6 15:45 config.toml
drwxr-xr-x. 4 kailash kailash  80 May  6 15:43 static

Putting it all together.

kailash@nova (/home/kailash)
$ /tmp/listmonk_0.9.0-beta_linux_amd64/listmonk --config=/tmp/listmonk/config.toml --static-dir=/home/kailash/code/go/my/knadh/listmonk/static

2021/05/06 15:45:45 main.go:84: v0.9.0-beta (e90fb1d 2021-02-13T12:41:40Z)
2021/05/06 15:45:45 init.go:98: reading config: /tmp/listmonk/config.toml
2021/05/06 15:45:45 init.go:175: connecting to db: localhost:9432/listmonk
2021/05/06 15:45:45 init.go:149: loading static files from: /home/kailash/code/go/my/knadh/listmonk/static
2021/05/06 15:45:45 init.go:420: media upload provider: filesystem
2021/05/06 15:45:45 init.go:344: loaded email (SMTP) messenger: [email protected]
⇨ http server started on [::]:9000

Your /tmp dir for statics is probably owned as the same as your UID & the binary owner too.

Yes indeed. As the above example shows, the binary indeed has static files embedded in it and works as expected. Now if uid/permissions cause the binary to not be able to read itself (as I've been suggesting), that's a different problem altogether, unrelated to the release and the binary.

Specifying the statics folder should probably keep the binary from even trying to access embedded files but it evidently does not.

This is by design and allows you to override individual static files without having to copy and maintain the entire tree. Files picked up from the external static dir ar "merged" into whatever is in memory.

@knadh
Copy link
Owner

knadh commented May 6, 2021

New idea: this bug is probably related to not using stuffbin to embed assets. These builds are expected to use a static directory to serve assets. The binary is being used after being built by go without the post-processing that the upstream make dist or make pack-bin steps take.

Ah! All this while, I assumed you were referring to the pre-built binary on the releases page! If you're building locally, you've to indeed use stuffbin to embed the static files post go build. make dist does this.

@alerque
Copy link
Contributor Author

alerque commented May 6, 2021

For the record, this is a different issue from #338. I should have clarified, but:

  • the issue with files in the static bin compiled upstream not being accessible when run under a different user is still an issue (see Static files not embeded #338)
  • this issue about the CLI flag being ignored when the embedded file set is not included and everything should be coming from a specified static directory is a different issue.

Respectfully even if my use case in distro packaging is contrary to your expectations, the fact that it 95% works and only fails in an unexpected way at run time by silently using CWD to read files is a bug.

Here is how I think it should be fixed:

  • The main binary should not throw an error if there are no embedded files, maybe a warning maybe not.
  • The CLI flag handling for --static-dir should work whether or not embedded files are found at all.
  • If no embeded files are found, the --static-dir argument should actually be mandatory and a check that the directory exists and is readable should replace the check for internal files.

As a bonus measure:

  • The config file should recognize a static_dir option and respect it in place of the CLI flag. This would make systems with default Systemd services easier to customize.

@knadh knadh added the question Further information is requested label May 6, 2021
@knadh
Copy link
Owner

knadh commented May 6, 2021

the issue with files in the static bin compiled upstream not being accessible when run under a different user is still an issue (see #338)

But this has nothing to do with listmonk or the way it embeds static files though. It's merely doing a os.Open(), a simple file open, on itself. That in a certain environment under certain kind of permissions, os.Open() fails to open a path is really an external problem.

this issue about the CLI flag being ignored when the embedded file set is not included and everything should be coming from a specified static directory is a different issue.

This is also not an issue and again, seems to do with permissions. Below is a binary with no embedded files running, picking up local static files as a fallback, then respecting --static-dir and loading it from /tmp/listmonk/static

kailash@nova (/listmonk) ● master                                                                                            
$ ./listmonk --static-dir=/tmp/listmonk/static/                                                                                                            
2021/05/06 17:33:06 main.go:84: v0.9.0-beta (#68b80d0 2021-05-06T12:03:04+0000)
2021/05/06 17:33:06 init.go:99: reading config: config.toml
2021/05/06 17:33:06 init.go:189: connecting to db: localhost:9432/listmonk
2021/05/06 17:33:06 init.go:123: unable to initialize embedded filesystem: no ID found in the file
2021/05/06 17:33:06 init.go:124: using local filesystem for static assets
2021/05/06 17:33:06 init.go:150: loading static files from: /tmp/listmonk/static/
2021/05/06 17:33:06 init.go:438: media upload provider: filesystem
2021/05/06 17:33:06 init.go:362: loaded email (SMTP) messenger: [email protected]
⇨ http server started on [::]:9000

The main binary should not throw an error if there are no embedded files, maybe a warning maybe not.

It doesn't. It merely warns. See above.

The CLI flag handling for --static-dir should work whether or not embedded files are found at all.

It does. See above.

If no embeded files are found, the --static-dir argument should actually be mandatory and a check that the directory exists and is readable should replace the check for internal files.

It fails right correctly, but yes, I agree, the error message can specifically mandate --static-dir.

The config file should recognize a static_dir option and respect it in place of the CLI flag. This would make systems with default Systemd services easier to customize.

It does (yep, this needs to be documented). Putting static-dir in the config.toml outside of the root [app] works.

@knadh
Copy link
Owner

knadh commented May 6, 2021

The CLI flag handling for --static-dir should work whether or not embedded files are found at all.

Correction: I see what you mean. It works even if no embedded files are found, but not if the static dir is not in the path. You are right, this needs to be fixed.

@alerque
Copy link
Contributor Author

alerque commented May 6, 2021

The config file should recognize a static_dir option and respect it in place of the CLI flag. This would make systems with default Systemd services easier to customize.

It does (yep, this needs to be documented). Putting static-dir in the config.toml outside of the root [app] works.

Okay I had tried inside [app], it didn't occur to me to try it outside. Documentation would fix that bit.

@knadh
Copy link
Owner

knadh commented May 6, 2021

On a different note, Go 1.16 has built in support for embedding files, negating the need for an external tool like stuffbin. I haven't had a chance to look at this, but once this is implemented, static file embedded will become an inherent part of go build and solve this issue for good. I'll explore this.

@knadh knadh closed this as completed in d695bb3 May 16, 2021
@knadh knadh added bug Something isn't working and removed question Further information is requested labels May 16, 2021
@alerque
Copy link
Contributor Author

alerque commented May 17, 2021

Edit: Moved to new issue #361, it's a continuation of the same problem but the issue is beyond just the "static" resources, it is with all assets and the fact that the config doesn't apply to all of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants