Cloudreve icon indicating copy to clipboard operation
Cloudreve copied to clipboard

Func: set the initial content of config via environment variables & corrected `RedisConfig.DB` to int type

Open AH-dark opened this issue 2 years ago • 1 comments

In containerized deployments and Kubernetes cluster deployments, using conf.ini configuration is unnecessary, and in most cases we use environment variables instead of configuration files. Therefore, adaptation to environment variables config is necessary.

Env Configuration

In commit cc303e74ddec232c7743406f36eda1a2f914cdda, I created env.go in util package, which provides an better method for obtaining environment variables. This way, we can fully resolve the environment variables in the init section, while having no conflict on existing configuration.

// EnvStr returns the value of the environment variable named by the key.
func EnvStr(key, defaultValue string) string {
    if value, exist := os.LookupEnv(key); exist {
        return value
    }

    return defaultValue
}

DB property from String to Int

As we all know, convert string value to int is more danger than convert int value to string, so I changed the type of RedisConfig.DB and some configuration in commit 50c3100afb79284a2c596c60cae291cc8aa2f671.

Existing configurations will not be affected, and instead of handling errors for the strconv.Atoi() function, we use strconv.Itoa() without any error throwing.

// Old
db, err := strconv.Atoi(database)
if err != nil {
    return nil, err
}

c, err := redis.Dial(
    network,
    address,
    redis.DialDatabase(db),
    redis.DialPassword(password),
)

// New
c, err := redis.Dial(
    network,
    address,
    redis.DialDatabase(database),
    redis.DialPassword(password),
)

Skip configuration loading

In a container, we may not be able to get read and write permissions for the current directory. Therefore, we don't need to still generating and reading the conf.ini file in the mode of getting the configuration via environment variables.

So, I commited some code in commit cc303e74ddec232c7743406f36eda1a2f914cdda, which allowed using flag --skip-conf to skip configuration loading and generating.

AH-dark avatar Sep 17 '22 05:09 AH-dark

The test ci failed due to gomod conflicts, so should I make another commit for go mod tidy ?

AH-dark avatar Sep 17 '22 05:09 AH-dark

There are still problems, HashIDSalt and SessionSecret may be empty, which may bring security issues, but I don't have a better solution.

AH-dark avatar Sep 29 '22 15:09 AH-dark

Great job!

Perhaps we missed one thing - initializing the default config with generating HashIDSalt and SessionSecret at the first starting.

My opinion is to generate a conf.ini with HashIDSalt and SessionSecret even though skips conf.

But it creates conf.ini. 😿

I just realize this issue and made a comment at https://github.com/cloudreve/Cloudreve/pull/1475#issuecomment-1262415549 but I am still missing a solution.

AH-dark avatar Sep 29 '22 15:09 AH-dark

Great job!

Perhaps we missed one thing - initializing the default config with generating HashIDSalt and SessionSecret at the first starting.

My opinion is to generate a conf.ini with HashIDSalt and SessionSecret even though skips conf.

But it creates conf.ini. 😿

I tried to use random string in default value and it passed my simple test, but I don't know if there is an underlying problem.

AH-dark avatar Sep 29 '22 15:09 AH-dark

Clean up stale PR, feel free to reopen it if needed.

HFO4 avatar Apr 16 '23 01:04 HFO4