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

Gitea's path system quirk and bug #24222

Closed
wxiaoguang opened this issue Apr 20, 2023 · 28 comments · Fixed by #25330
Closed

Gitea's path system quirk and bug #24222

wxiaoguang opened this issue Apr 20, 2023 · 28 comments · Fixed by #25330
Labels

Comments

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 20, 2023

Description

In history, Gitea's binary was designed to use the binary directory as working directory, it tries to create/use its "data"/"custom" directories in the binary path. For example: running /usr/local/bin/gitea makes Gitea try to use /usr/local/bin/custom directory, which is improper/wrong, and it causes a lot of problems.

POSIX users please use https://github.com/go-gitea/gitea/tree/main/contrib/fhs-compliant-script , do not put Gitea binary into /usr/loca/bin directory -- unless you are 100% sure about the details.

"custom" bug

A feedback from discord


Update gitea from 1.15.6 to 1.15.7 (and then 1.19.1):

Failed to include custom: open /usr/local/bin/custom/conf/app.ini: permission denied
2023/04/20 06:06:09 cmd/dump.go:150:fatal() [F] Failed to include custom: open /usr/local/bin/custom/conf/app.ini: permission denied

I'm using the script in contrib/upgrade.sh and as far as I'm concerned I have never used this "custom" folder, why is it needed? (I am using sqlite3 as well)

I don't get why it's trying to read/write in that particular folder and not from /var/lib/gitea/custom

@aru
Copy link

aru commented Apr 20, 2023

I was able to solve this issue by just adding the proper backupopts:

 giteauser=gitea backupopts="-c /etc/gitea/app.ini -C /var/lib/gitea/custom" ./gitea_upgrade.sh -v 1.15.7

@zeripath
Copy link
Contributor

zeripath commented May 4, 2023

It's difficult to come up with a reasonable way to improve what's going on without understanding why things are the way they are.

I've said this before, but for the sake of completeness I'll say it again.


Gitea was initially intended to be run from the directory it was built in or a directory similar to it and NOT from /usr/bin/gitea.

It would use the directory of its binary as the basis for where it would look for its assets and where it would create its data.

Over time, options and configuration were added to allow this directories to be changed from these defaults but ... fundamentally the paths are all built up in a hierarchical fashion from the work-path - that is, the absolute path of the Gitea binary.

Now... changing that is difficult without causing lots of breakage.


  1. I believe that it is important that Gitea should just run from the directory that you compile it in.
  2. Similarly Gitea should continue to be able to be run from any directory that it is run in.
  3. However, the current situation is not ideal because people want to be able to dump Gitea in /usr/local/bin and /usr/bin and to have it just work.

We have been bodging 3 for a while by providing the script to wrap Gitea in a filesystem hierarchy compliant way and we can compile Gitea with paths in-built to avoid having dependencies on the work-path.

Our Dockers finally wrap Gitea in the FHS script which means that these problems should no longer occur in the docker.

Our documentation used to advise people to wrap Gitea in the FHS script instead of placing it in /usr/bin but clearly someone thought that was too complicated and has removed it. Unsurprisingly this will have caused problems to resurface.


Now... We have several options (for this particular problem):

  1. Go back to advising people to wrap Gitea in the script or to compile it properly with the paths built-in. HOWEVER this is yet another ill-considered documentation simplification PR away from being broken.
  2. Make Gitea search for its configuration in some standard places depending on where it is started.

The search path in 2 could depend on the directory that Gitea finds itself in or it could just be common depending on how gitea was started.


The linked issue (#23301) is related but it's also likely related to incomplete configuration - in this case there should be a conf.ini but there is still an important path (APP_DATA_PATH) that is incompletely defined and so is dependent on the APP_WORK_PATH. A breaking option for dealing with this is to assert that the conf.ini must contain the absolute path to the data directory, this would be very breaking indeed as far as I can see.


So to reiterate, Gitea is the way it is because it was never intended to be placed in /usr/bin or /usr/local/bin.

The documentation USED to advise the use of the wrapper script for such placement.

We should add this back in to the documentation. I can't understand why this was ever removed.

I think we should consider a search path approach to finding the configuration if -c is not provided. We can add a warning if there is any dependency on --work-path.

@jolheiser
Copy link
Member

I don't recall which docs referenced the FHS script, and a cursory search wouldn't tell me when it existed or was removed.

The install from binary docs mention setting the correct env vars, but I don't recall it ever mentioning the FHS script. (Although it may be worth adding)
The systemd page also points to the example service, which also sets those env vars.

Do you recall where it was at? I agree with adding it somewhere, it would potentially save us some bug reports. I'm just curious to the reasoning to remove it.

@DanielGibson
Copy link

DanielGibson commented May 9, 2023

Three things that might make all this less painful:

  1. Let gitea (esp. doctor --fix) ignore if the custom directory doesn't exist and/or can't be created (after all, using it is optional and only needed if you want to modify gitea's data like html templates or whatever) - unless the user explicitly specifies that they want to use a custom directory (or use the embedded command to extract the resources to a custom directory
  2. Allow specifying AppWorkPath and CustomPath (and maybe more?) in app.ini, so the user only needs to set the CustomConf path instead of all the others as well when calling gitea on the commandline
  3. Search for app.ini in some more directories (if it wasn't found in one try the next). For example, on Linux (or generally Unix-likes) it could try to use /etc/gitea/app.ini if $CustomPath/conf/app.ini doesn't exist, and maybe if that doesn't exist try $HOME/.config/gitea/app.ini or something like that? (or maybe first try the path in $HOME and then in /etc/)

I think all these changes could be implemented in a backwards-compatible way.

@DanielGibson
Copy link

If the AppWorkPath was stored in the app.ini, blobless clones would work in systemwide installations, see https://codeberg.org/forgejo/forgejo/issues/869
(I haven't tested this with Gitea, but would be surprised if it behaved any differently, unless you recently fixed this bug)

@DanielGibson
Copy link

I just tested forgejo 1.20rc0, and it looks like the working_path/data is now stored in the app.ini (under [server] APP_DATA_PATH) and that is used by forgejo serve --config /path/to/app.ini .... so the aforementioned issue seems fixed now - cool! :)

Did you also happen to introduce an environment variable to set the path to the app.ini?
Then users wouldn't have to specify that for every command, either :)

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Jun 18, 2023

I don't understand forgejo.

Gitea's related code hasn't been changed for long time, APP_DATA_PATH is always stored in app.ini, from very long time ago

And, if it is not absolute path, there should already be a log message to remind users (since 1.17)

@DanielGibson
Copy link

DanielGibson commented Jun 18, 2023

Gitea's related code hasn't been changed for long time, APP_DATA_PATH is always stored in app.ini, from very long time ago

Nope, I just tested gitea 1.19.3, same as in forgejo 1.19.3.
I downloaded the binary to /tmp/, then did:

$ cd /tmp/
$ chmod 755 gitea-1.19.3-linux-amd64
$ mkdir -p /tmp/gitea_work_dir/etc
$ ./gitea-1.19.3-linux-amd64 -w /tmp/gitea_work_dir -c /tmp/gitea_work_dir/etc/app.ini

Then I opened http://localhost:3000/ in a webbrowser and did the initial installation/configuration step.
Here's the app.ini that was generated - as you can see, it doesn't contain an APP_DATA_PATH line.

And here's the output I got in the console: gitea-output.txt
I don't see any warnings about APP_DATA_PATH there.

@wxiaoguang
Copy link
Contributor Author

I see, there is something wrong with #19815

@DanielGibson
Copy link

I see, there is something wrong with #19815

No, why? The path I passed to gitea is absolute, it's just not saved in the app.ini (at least as APP_DATA_PATH, it is set correctly as part of database.PATH, repository.ROOT, lfs.PATH, etc)

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Jun 18, 2023

#19815 introduced another patch for APP_DATA_PATH, but it forgot to write the correct APP_DATA_PATH when installing.

If there was no such patch but a complete refactoring, then I think there won't be such bug.


Update: Write absolute AppDataPath to app.ini when installing #25331 is still not ideal

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Jun 18, 2023

And to be clear, although the code was not good, it seems that only (mainly) the users who use forked release are affected by your problem (update: if a Gitea user runs the binary with changed work path but doesn't set it correctly, there is still a problem, but it's not documented)

Gitea document never mentions to run the server with $ ./gitea-1.19.3-linux-amd64 -w ... -c ... . I do not know how this command comes and maybe the forked release maintainers don't understand it (ps: before you asked the question, I didn't understand the problem either, indeed, it was not designed as so)

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Jun 18, 2023

I read Gitea's document again:

  • Install from Docker: no problem.
  • Install from binary
    • with builtin SSH server: no problem, the GITEA_WORK_DIR is inherited (but do not use "--work-path")
    • with OpenSSH server: the "work path" might be incorrect in this case if it's not default to the app binary path

That's really a longstanding problem, but I think it could be completly fixed by #25330 , but maybe the fix will only be in 1.21, I guess no backport (unless maintainers prefer to do so .....)

@DanielGibson
Copy link

Gitea document never mentions to run the server with $ ./gitea-1.19.3-linux-amd64 -w ... -c ... . I do not know how this command comes

Yes, I forgot the web part, but it still worked (i.e. it should've been ./gitea-1.19.3-linux-amd64 web -w /tmp/gitea_work_dir -c /tmp/gitea_work_dir/etc/app.ini).
This has nothing to do with forgejo, I didn't explicitly follow documentation when making up the minimal testcase above, I just improvised.
Apart from the missing web it's pretty much equivalent to what https://github.com/go-gitea/gitea/blob/main/contrib/systemd/gitea.service does, except I used -w to set the work dir instead of using the GITEA_WORK_DIR environment variable (and used -c instead of --config, which, like -w, is documented in gitea --help).
Note that even leaving out web is documented in gitea --help:

DESCRIPTION:
   By default, gitea will start serving using the webserver with no
arguments - which can alternatively be run by running the subcommand web.

By the way, it would be super useful if the --help command could, instead of printing the DEFAULT CONFIGURATION for CustomPath etc, print the current configuration, based on environment variables and the passed config, if any, like:

/tmp/gitea-1.19.3-linux-amd64 -c /tmp/gitea_work_dir/etc/app.ini -h
NAME: 
   ...
CURRENT CONFIGURATION:
     CustomPath:  /tmp/custom 
     CustomConf:  /tmp/gitea_work_dir/etc/app.ini
     AppPath:     /tmp/gitea-1.19.3-linux-amd64
     AppWorkPath: /tmp

(or, after your fixes, AppWorkPath: /tmp/gitea_work_dir)

@wxiaoguang
Copy link
Contributor Author

I see, the document is not clear either. I think zeripath has explained about this problem with backgrounds #24222 (comment)

If #25330 could be merged, I think I could also improve the documents about the "work path" topic, and at that time, we do not need to play with the "path tricks" any more.

Thank you very much for helping investigating the problem 🙏 , I guess we are near to the complete solution now. 😄

@DanielGibson
Copy link

That's really a longstanding problem, but I think it could be completly fixed by #25330 , but maybe the fix will only be in 1.21, I guess no backport (unless maintainers prefer to do so .....)

IMO this should really be backported, as this is quite a serious problem - I think most people (on Linux) will use OpenSSH instead of the builtin server, and I guess that systemwide installations (or any kind of installation with non-default work dir) aren't that unusual either. And this breaks blobless clones, probably signing (#24503) and who knows what else..

Thank you very much for helping investigating the problem 🙏 , I guess we are near to the complete solution now. 😄

I hope so =)

@DanielGibson
Copy link

FWIW, I just tried Gitea 1.20.0 rc0 with a fresh work dir, like

$ cd /tmp/
$ chmod 755 gitea-1.20-linux-amd64
$ mkdir -p /tmp/gitea_work_dir3/etc
$ GITEA_WORK_DIR=/tmp/gitea_work_dir3/ ./gitea-1.20-linux-amd64 -c /tmp/gitea_work_dir/etc/app.ini

(and to be double-sure I also tried the variant without setting the environment variable and using -w /tmp/gitea_work_dir3 instead)

And, unlike 1.19.3, it did write APP_DATA_PATH=/tmp/gitea_work_dir3/data to the app.ini, so it appears the this was already fixed even before #25331 ?

And while at it, I also tried setting a custom path (-C /tmp/gitea_work_dir3/custom/) - that was not saved in the app.ini.
Not sure if it's supposed to - it sure would make sense to have [server] CUSTOM_PATH = /tmp/gitea_work_dir3/custom or similar - https://docs.gitea.com/next/administration/config-cheat-sheet#server-server is a bit contradicting here: It does not explicitly list CUSTOM_PATH under Server, but the documentation for server.CERT_FILE and server.KEY_FILE says "Paths are relative to CUSTOM_PATH", which somehow implies the existence of that option.

@DanielGibson
Copy link

DanielGibson commented Jun 18, 2023

Oh, and one more thing, about running 1.20 on a former 1.19 data dir:
When first running 1.20 on a former 1.19.3 repo, the app.ini gets updated automatically - but for this it must be writable by gitea, of course. However, https://docs.gitea.com/next/installation/install-from-binary#create-required-directory-structure suggests making the app.ini read-only after the installation step, and https://docs.gitea.com/next/installation/upgrade-from-gitea doesn't mention the app.ini at all.

@wxiaoguang
Copy link
Contributor Author

so it appears the this was already fixed even before #25331 ?

Nope, that's a side-affect by "oauth" related code.

-C /tmp/gitea_work_dir3/custom/

Well, "--custom-path" is not recommended for this purpose, at least no document says doing so.

It does not explicitly list CUSTOM_PATH under Server, but the documentation for server.CERT_FILE and server.KEY_FILE says "Paths are relative to CUSTOM_PATH", which somehow implies the existence of that option.

Some document words are not accurate, indeed they are crowd-contributed, so not that strict. You can also see that sometimes "GITEA_WORK_DIR" sometimes "Work Path", some code was written without careful thinking and the naming is not strict.

suggests making the app.ini read-only after the installation step

I believe the "read-only" is a wrong word from non-English-native contributors.

They meant "0640" , which is readable-writable to the owner. You can see the demo commands below, they are 750 + 640 , not "0400" indeed.

@DanielGibson
Copy link

Well, "--custom-path" is not recommended for this purpose, at least no document says doing so.

For what purpose? To set the custom path?
My point is that for the custom path it would also make sense to store it in the app.ini, so (if you want to use the custom path) it doesn't have to be specified every time you call gitea

I believe the "read-only" is a wrong word from non-English-native contributors.

They meant "0640" , which is readable-writable to the owner. You can see the demo commands below, they are 750 + 640 , not "0400" indeed.

good to know

@wxiaoguang
Copy link
Contributor Author

For what purpose? To set the custom path?

I am pretty sure no particular purpose for now, it's just technical debt. Some options were introduced one by one, patch by patch in history, then, nowadays, they causes problems. If no time spent on them .... they would be kept there forever and cause more surprises for end users.

@DanielGibson
Copy link

As far as I know the custom path option is needed to modify the html templates and CSS files (incl. installing themes), so I wouldn't say they have no particular purpose.

@wxiaoguang
Copy link
Contributor Author

But there is already a "work path", just use "{WorkPath}/custom", that's better and clearer.

@DanielGibson
Copy link

I haven't tried it, but I assumed it would use /usr/local/bin/custom, when not explicitly specifying it with -C/--custom-path/$GITEA_CUSTOM

But either way, does that solve the problem?
app.ini doesn't store the work dir or custom dir, only the APP_DATA_DIR (well, and a few others) - or does STATIC_ROOT_PATH hold the work path (and thus will use a custom/ subdir of that path as CustomPath?

@wxiaoguang
Copy link
Contributor Author

#25330 stores the WORK_PATH

@DanielGibson
Copy link

That's good, because right now gitea indeed assumes that the work dir is the binary dir, if not set explicitly:

caedes@spiderdemon:/tmp$ ./gitea-1.20.0-rc0-linux-amd64 -c /tmp/gitea_work_dir3/etc/app.ini doctor 

[1] Check paths and basic configuration
 - [I] Configuration File Path:    "/tmp/gitea_work_dir3/etc/app.ini"
 - [I] Repository Root Path:       "/tmp/gitea_work_dir3/data/gitea-repositories"
 - [I] Data Root Path:             "/tmp/gitea_work_dir3/data"
 - [I] Custom File Root Path:      "/tmp/custom"
 - [I] Work directory:             "/tmp"
 - [I] Log Root Path:              "/tmp/gitea_work_dir3/log"
OK
(...)

So, from the app.ini, it sets the Data Root Path and the Repository Root Path and the Log Root Path correctly, but Work Directory and Custom File Root Path are incorrect.

And even if I explicitly set the work directory, the Custom File Root Path is still wrong:

caedes@spiderdemon:/tmp$ ./gitea-1.20.0-rc0-linux-amd64 -w /tmp/gitea_work_dir3/ -c /tmp/gitea_work_dir3/etc/app.ini doctor 

[1] Check paths and basic configuration
 - [I] Configuration File Path:    "/tmp/gitea_work_dir3/etc/app.ini"
 - [I] Repository Root Path:       "/tmp/gitea_work_dir3/data/gitea-repositories"
 - [I] Data Root Path:             "/tmp/gitea_work_dir3/data"
 - [I] Custom File Root Path:      "/tmp/custom"
 - [I] Work directory:             "/tmp/gitea_work_dir3/"
 - [I] Log Root Path:              "/tmp/gitea_work_dir3/log"
OK

This is at least consistent with https://docs.gitea.com/next/administration/config-cheat-sheet#default-configuration-non-appini-configuration:

CustomPath: This is the base directory for custom templates and other options. It is determined by using the first set thing in the following hierarchy:

  • The --custom-path flag passed to the binary
  • The environment variable $GITEA_CUSTOM
  • A built-in value set at build time (see building from source)
  • Otherwise it defaults to AppWorkPath/custom
  • If any of the above are relative paths then they are made absolute against the the directory of the AppWorkPath

So the default path (binary_dir/custom) has precedence over work_dir/custom.

Though I doubt that work_dir/custom/ is even used/tried at all, because if I copy gitea-1.20.0-rc0-linux-amd64 to /usr/local/bin/gitea, I get the following:

$ gitea -w /tmp/gitea_work_dir3/ -c /tmp/gitea_work_dir3/etc/app.ini doctor 

[1] Check paths and basic configuration
 - [I] Configuration File Path:    "/tmp/gitea_work_dir3/etc/app.ini"
 - [I] Repository Root Path:       "/tmp/gitea_work_dir3/data/gitea-repositories"
 - [I] Data Root Path:             "/tmp/gitea_work_dir3/data"
 - [I] Custom File Root Path:      "/usr/local/bin/custom"
 - [W]     NOTICE: is not accessible (Error: stat /usr/local/bin/custom: no such file or directory)
 - [I] Work directory:             "/tmp/gitea_work_dir3/"
 - [I] Log Root Path:              "/tmp/gitea_work_dir3/log"
OK

I guess if it would fall back to work_dir/custom/ (that directory even exists in my testcase), it wouldn't show that warning..

No offense, but my impression is that the path handling code in Gitea overall is pretty fragile and buggy..
I hope #25330 will make it more stable :)

@wxiaoguang
Copy link
Contributor Author

No offense, but my impression is that the path handling code in Gitea overall is pretty fragile and buggy..

That's true ..... 🤣 and it's quite complex that's why there are many issues which are difficult to fix before refactoring.

For me, I have done my best to optimize it, since it is really complex, I won't say my code is bug-free, while I think it is the right way and if there is any bug in my code, I will always fix them in the first time (as usual).

@DanielGibson
Copy link

I think it is the right way and if there is any bug in my code, I will always fix them in the first time (as usual)

I'm sure you will, your work (and the speed of your fixes) is incredible! :)

lunny pushed a commit that referenced this issue Jun 21, 2023
# The problem

There were many "path tricks":

* By default, Gitea uses its program directory as its work path
* Gitea tries to use the "work path" to guess its "custom path" and
"custom conf (app.ini)"
* Users might want to use other directories as work path
* The non-default work path should be passed to Gitea by GITEA_WORK_DIR
or "--work-path"
* But some Gitea processes are started without these values
    * The "serv" process started by OpenSSH server
    * The CLI sub-commands started by site admin
* The paths are guessed by SetCustomPathAndConf again and again
* The default values of "work path / custom path / custom conf" can be
changed when compiling

# The solution

* Use `InitWorkPathAndCommonConfig` to handle these path tricks, and use
test code to cover its behaviors.
* When Gitea's web server runs, write the WORK_PATH to "app.ini", this
value must be the most correct one, because if this value is not right,
users would find that the web UI doesn't work and then they should be
able to fix it.
* Then all other sub-commands can use the WORK_PATH in app.ini to
initialize their paths.
* By the way, when Gitea starts for git protocol, it shouldn't output
any log, otherwise the git protocol gets broken and client blocks
forever.

The "work path" priority is: WORK_PATH in app.ini > cmd arg --work-path
> env var GITEA_WORK_DIR > builtin default

The "app.ini" searching order is: cmd arg --config > cmd arg "work path
/ custom path" > env var "work path / custom path" > builtin default


## ⚠️ BREAKING

If your instance's "work path / custom path / custom conf" doesn't meet
the requirements (eg: work path must be absolute), Gitea will report a
fatal error and exit. You need to set these values according to the
error log.



----

Close #24818
Close #24222
Close #21606
Close #21498
Close #25107
Close #24981
Maybe close #24503

Replace #23301
Replace #22754

And maybe more
wxiaoguang added a commit to wxiaoguang/gitea that referenced this issue Jun 21, 2023
# The problem

There were many "path tricks":

* By default, Gitea uses its program directory as its work path
* Gitea tries to use the "work path" to guess its "custom path" and
"custom conf (app.ini)"
* Users might want to use other directories as work path
* The non-default work path should be passed to Gitea by GITEA_WORK_DIR
or "--work-path"
* But some Gitea processes are started without these values
    * The "serv" process started by OpenSSH server
    * The CLI sub-commands started by site admin
* The paths are guessed by SetCustomPathAndConf again and again
* The default values of "work path / custom path / custom conf" can be
changed when compiling

# The solution

* Use `InitWorkPathAndCommonConfig` to handle these path tricks, and use
test code to cover its behaviors.
* When Gitea's web server runs, write the WORK_PATH to "app.ini", this
value must be the most correct one, because if this value is not right,
users would find that the web UI doesn't work and then they should be
able to fix it.
* Then all other sub-commands can use the WORK_PATH in app.ini to
initialize their paths.
* By the way, when Gitea starts for git protocol, it shouldn't output
any log, otherwise the git protocol gets broken and client blocks
forever.

The "work path" priority is: WORK_PATH in app.ini > cmd arg --work-path
> env var GITEA_WORK_DIR > builtin default

The "app.ini" searching order is: cmd arg --config > cmd arg "work path
/ custom path" > env var "work path / custom path" > builtin default

## ⚠️ BREAKING

If your instance's "work path / custom path / custom conf" doesn't meet
the requirements (eg: work path must be absolute), Gitea will report a
fatal error and exit. You need to set these values according to the
error log.

----

Close go-gitea#24818
Close go-gitea#24222
Close go-gitea#21606
Close go-gitea#21498
Close go-gitea#25107
Close go-gitea#24981
Maybe close go-gitea#24503

Replace go-gitea#23301
Replace go-gitea#22754

And maybe more
# Conflicts:
#	cmd/web.go
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants