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

Configure with env variables #350

Closed
tboerger opened this issue Dec 5, 2016 · 61 comments
Closed

Configure with env variables #350

tboerger opened this issue Dec 5, 2016 · 61 comments
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality

Comments

@tboerger
Copy link
Member

tboerger commented Dec 5, 2016

To make the configuration easier especially in the world of systemd and docker we should make it possible to entirely configure gitea with environment variables. To work around the required variables used within the ssh shell we can generate an environment file for it automatically on application start. That way we can get rid of the requirement of the custom app.ini.

@tboerger tboerger added the type/enhancement An improvement of existing functionality label Dec 5, 2016
@tboerger tboerger added this to the 1.0.0 milestone Dec 5, 2016
@lunny
Copy link
Member

lunny commented Dec 5, 2016

First is that we can indicate one configuration file via command line?

@tboerger
Copy link
Member Author

tboerger commented Dec 5, 2016

The configuration file can already be defined at least via an environment variable. But I would like to provide flags for the available options and also bind them to environment variables so that we get the full flexibility out of that.

@bkcsoft
Copy link
Member

bkcsoft commented Dec 5, 2016

This will be a bitch, we'd have to build a new module on top of the current ini-loader 😒

@tboerger tboerger modified the milestones: 1.1.0, 1.0.0 Dec 5, 2016
@tboerger
Copy link
Member Author

tboerger commented Dec 5, 2016

At least I will give it a try and then we will see how complex that gets.

@stevenroose
Copy link

There are Go packages (I thought envflags is an example) that automatically load configs, in order of increasing priority, a config file, env flags and command line flags. Config variables only have to be defined once and they can be set in all three ways.

@tboerger
Copy link
Member Author

We already use codegangsta cli which provides most scenarios

@tboerger
Copy link
Member Author

And additionally to that I can think of dotenv.

@lunny
Copy link
Member

lunny commented Jan 27, 2017

any update?

@lunny lunny modified the milestones: 1.2.0, 1.1.0 Feb 7, 2017
@tboerger
Copy link
Member Author

I think setting it to 1.2.0 is fine so far unless somebody provides a PR in time.

@lunny lunny modified the milestones: 1.x.x, 1.2.0 Apr 19, 2017
@TheAssassin
Copy link

Referring to @bkcsoft's comment:

This will be a bitch, we'd have to build a new module on top of the current ini-loader 😒

The common approach to avoid code changes (as this is just a matter of deployment) is to use tools that generate the config file from the environment variables and then load the actual application.

Within the scope of Docker, an application called dockerize has become quite popular for these tasks. Its workflow first loads a configuration file template (which could even be generated from the default .ini file easily with some shell or other scripting magic). Then, it looks for environment variables, and inserts them into the template. If there's a default value given in the template, and an environment variable is not present, this value is used.

This way, there's no code changes required, just add dockerize and the template to the image, and change the entrypoint to dockerize (it should be okay if the config file is rendered even if the CMD is modified on startup).

I saw Gitea is using the alpine base image. This makes installation as easy as:

apk add --update-cache --repository http://dl-3.alpinelinux.org/alpine/edge/testing/ dockerize

(This needs to be updated as soon as the dockerize package is moved to the main repository, but that's the only issue).

@ivlis
Copy link

ivlis commented Feb 27, 2018

I can spend some time on integrating dockerize into gitea. @lunny @tboerger will you accept a PR on this?

@lunny
Copy link
Member

lunny commented Feb 28, 2018

@ivlis Please follow CONTRIBUTING and send PR.

@TheAssassin
Copy link

TheAssassin commented Feb 28, 2018

@ivlis
Copy link

ivlis commented Feb 28, 2018

@lunny @TheAssassin alright, thanks, will do.

@raucao
Copy link

raucao commented Nov 2, 2018

Any news on this? We can't seem to find how to enable Let's Encrypt when deploying via Docker to GKE, as there's no ENV var for it.

@techknowlogick
Copy link
Member

@skddc letsencrypt support hasn't been released in a stable version of gitea. It is in 1.6.0-rc1, but no earlier versions.

@raucao
Copy link

raucao commented Nov 2, 2018

We're fine with using a release candidate. But does that mean it's a missing feature? Is there an issue for making it configurable?

@tboerger
Copy link
Member Author

tboerger commented Nov 2, 2018

So far most config variables are only configurable via the configuration file.

@raucao
Copy link

raucao commented Nov 2, 2018

Hence my original question if there are any news on this particular issue/feature. So the fact that LE support is not in a stable version yet has nothing to do with some variables not being possible to configure in Docker via ENV vars right now, correct?

@tboerger
Copy link
Member Author

tboerger commented Nov 3, 2018

When you are deploying to Kubernetes you can't configure all by env variables, I think the best would be a configmap for the app.ini. Just disable the install lock. The first user that gets registered will be automatically an admin user.

@raucao
Copy link

raucao commented Nov 3, 2018

Cool, that was exactly my idea. Will do that then. Thank you!

@tboerger
Copy link
Member Author

tboerger commented Nov 3, 2018

You also got the option of an init container which starts gitea initially to get the database migrated, and you could execute the gitea cli once to create an admin user. Never tried that, but should work with some basic bash scripting.

@zeripath
Copy link
Contributor

I think we still need to write out these env variables to a config file at entry point.

My reason is the SSH hooks and serv need to be able to find the Gitea server to speak to. Therefore if we don't write these out we have to push the entire environment across to SSH processes and into the .SSH/authorized_keys. Further we risk accidental environment overrides in other hooks and possibly malicious overrides by commands. However, as I say it might be possible to massively reduce the config needs for serv and hook as they're now thin shims on webserver calls.

The other problem is that afaiu the docker environment you run in isn't passed to the docker environment you exec. So say you want to use a Gitea subcommand in exec you won't be able to use it without copying the entire environment.

@zeripath
Copy link
Contributor

Regarding making the docker app.ini file completely settable - I can understand why we haven't done it. It looks very much not fun to do and awkward and fragile to keep updated. I'd far rather do the generic gitea docker-environment-config thing mentioned above. That at least would mean that I never need to change it unless I wanted to add a special non-1-1 mapping.

@christhomas
Copy link
Contributor

christhomas commented Jun 23, 2019

The other problem is that afaiu the docker environment you run in isn't passed to the docker environment you exec. So say you want to use a Gitea subcommand in exec you won't be able to use it without copying the entire environment.

Just to address this. If I run a docker container with a set bunch of environment variables. If I exec into that container to run another command. The same environment variables I set when I ran the container are available to the command when it's executed inside the same container.

Or did I misunderstand you?

@christhomas
Copy link
Contributor

Regarding:

into the .SSH/authorized_keys.

I don't think environment variables are a good way to inject this information into the container. You can mount a docker volume containing the keys, or in kubernetes write a configmap and mount that as a file inside the container if you want to do this.

So in fact, pushing this through environment variables, in my opinion, is the wrong thing to do. It's a file, and I think it should be treated like one.

@zeripath
Copy link
Contributor

You're misunderstanding the problem in .SSH/authorized_keys.

We have to run gitea serv for each connection.

Therefore we need to be able to configure gitea for each connection.

If you do not save the configuration to a file you have to set that configuration somehow.

Then within each SSH session git will call Gitea hook. That has to be configured too.

@christhomas
Copy link
Contributor

No no, I'm saying that you should save it to a file. It's just that how it gets into that file shouldn't be through an environment variable, but a normal file mount, or a kubernetes mechanism of various type.

Or are we talking about different things here?

@zeripath
Copy link
Contributor

Yes, you're missing the problem. If you allow arbitrary override of configuration by environment variable without saving it to a config file - how do you configure gitea serv and gitea hook. Even with the changes in #6993 there is still a requirement to configure these calls - but we also don't want their configuration to necessarily be overridden by environment calls.

@TheAssassin
Copy link

@zeripath why can't those commands just read their configuration from the environment as well? Should be possible to have one central config component that all commands share and use to e.g., read config files or alternatively read config values from the environment.
The environment is available for every process in the container, after all.

If using environment variables is too awkward and you prefer to use config files, a config file generator that runs before the main processes is the way to go, this concept is also widely spread. See #350 (comment).

An alternative would be to do the heavy lifting in the main gitea process, and have the hook stuff use RPC instead of doing anything on its own. Then, some simple auto-discovery of the main process (e.g., using a Unix socket in a well-known location) is all you need in these hooks, no configuration needs to be forwarded.

@christhomas
Copy link
Contributor

They're not being overridden arbitrarily. I'm setting environment data when I run the container. So I want my configuration used, not what is written in any config file. Thats why you load config data in the precedence that you do. Command line overrides anything in the env, the env overrides anything in the file.

I want those commands using the config I set. It's literally the point of setting the data in the first place.

@christhomas
Copy link
Contributor

I think if you're executing files inside a container they all inherit the same runtime environment as their parents, no? Or at least they should.

If each command used the same config loader, which was configured to read from command line > env > config file. Then every command would automatically be configured with the correct configuration data without ever having to change any of those commands.

The problem is that gitea right now only loads from the config file. Why not replace the config file loader with a service like I mentioned in an above comment and then you'd have no more problems, you'd naturally obtain the right configuration every time, regardless of what subprocess you're using

@TheAssassin
Copy link

I think if you're executing files inside a container they all inherit the same runtime environment as their parents, no? Or at least they should.

Exactly. Unless a process doesn't forward/override those when spawning new subprocesses. I am not sure whether SSH servers will forward the environment they're called with (normally, they just spawn a clean login shell per session, right?), so there you might need a config file.

@christhomas
Copy link
Contributor

Well ok, but even in the case of the SSH server. That should use data mounted into the filesystem in terms of the files inside the .ssh directory. So that shouldn't even matter. Right? Given your last sentence. Seems we agree.

@TheAssassin
Copy link

@christhomas not really, the processes spawned by the SSH server are gitea tasks (git pre-/post-push hooks for instance, that's what @zeripath means with gitea serve etc.), these also need the configuration values. If the SSH server doesn't forward those information, then they can't do their job.

Workflow is:

  • git push/pull/...
  • connects to Gitea's SSH server
  • remote git runs hooks in the repository (gitea serve etc.)

Now, obviously, these gitea ... commands need to have the same information (read: configuration data) as the main process, or there will be inconsistencies.
An environment variable based system relies on an information flow through the SSH server process that OpenSSH spawns for that connection, i.e., OpenSSH must pass through system environment variables (the ones the main server process sees) to the shell in the per-connection process. That's unlikely to happen with OpenSSH, though, for security reasons. SSH is, after all, primarily designed for secure, remote shells, not for usage as a transport protocol for e.g., Git, ...

TL;DR: the Git hooks will most likely not see any of the env vars set for the Docker container. But they need the same config as the main server process.

I already illustrated a few options how to work around that limitation, e.g., using RPC and performing the tasks in the main processes (i.e., you only need some auto-discovery to find the main Gitea process, and need to set up permissions correctly so only SSH induced stuff can access these endpoints), or writing a config file from the environment variables and allow those processes to read that as well.

@christhomas
Copy link
Contributor

ahhh, thats what he meant, I misunderstood and didn't think this was an issue for the ssh daemon

@tboerger
Copy link
Member Author

The official image or my image?

As already commented on the roadmap issue, it would make much more sense to reduce the config file to a minimum and make the rest part of the database which can be changed via cli and optionally via Web ui.

@TheAssassin
Copy link

That's a good alternative, as long as deployment-critical data can be set via env vars (ports, public (as in: load balancer) URLs, etc.).

@tboerger
Copy link
Member Author

Infrastructure/deployment parts got to be part of the config, the rest should be part of the db.

@zeripath
Copy link
Contributor

These are infrastructure questions: e.g. what is the internal URL that serv/hook talks to.

There needs to be some way of configuring these non s6/entrypoint processes, and allowing environment overrides at that point is a security risk.

I think we're going round in circles. If docker requires that you replace an ini file with an exploded set of environment variables so be it. Overlaying this config on top of a provided config á la gitea docker-environment-config is the simplest solution I can think of. Yes, we should consider moving config to the db but unless you're willing to remove any dependency on modules/setting in serv and hook and replace it with arguments - and do so carefully for multiple docker environments and account for non docker users too - you're going to have to have some way of configuring this.

@tboerger
Copy link
Member Author

@zeripath you are missing the point, but whatever. I will just unsubscribe.

@TheAssassin
Copy link

@zeripath I have doubts you ever used some container-based systems for deployment. Have you ever tried a random, more well known Docker container? For instance, PostgreSQL is a good example: https://hub.docker.com/_/postgres. Check their "How to use this image". You're thinking way too complicated...

@christhomas
Copy link
Contributor

christhomas commented Jun 23, 2019

@zeripath perhaps you can explain what these security risks are when it comes to configuring ssh using environment variables? I've already stated that installing keys and files into the .ssh directory should be done through mounting files into the file system and not environment variables. But you continue to restate that it's a security risk and I'm not sure what risk you are talking about.

@tboerger
Copy link
Member Author

A config file is fine, just reduce it to a minimum which includes database config, cache config (like redis) and bindings like port, internal domain. That's it. All the other stuff could be stored on the database.

@TheAssassin
Copy link

TheAssassin commented Jun 23, 2019

I still don't see why a config file would have to contain all the things by default even.

When I develop web apps, there's a default config that contains standard values, and any user config file or env vars can override those keys. That should exist already anyway. If required settings are missing, the app refuses to start up.

You don't have to expose every setting as env var either. Just the stuff we really need to be able to set via env var, which are important for deployment.

@go-gitea go-gitea locked and limited conversation to collaborators Jun 23, 2019
@techknowlogick
Copy link
Member

techknowlogick commented Jun 23, 2019

Temporarily locking this issue due to amount of comments on it. More than 400 people get an email each comment. Let’s discuss this over on discord for now, and I’ll reopen in the near future.

Maintainers: you should still be able to comment. If not ping me in discord and I’ll post on your behalf.

@tboerger
Copy link
Member Author

Just some closing words, just env variables without a config at all is pretty difficult because the executable gets called from different contexts.

The web application itself should work by env variables, but you can't hand over this environment to the script called by ssh commands like cloning or pushing, which is calling gitea serv for every connection.

So you can see that there is some reasoning for a config file.

@lunny lunny removed this from the 1.x.x milestone Dec 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.