minio / kes

Key Managament Server for Object Storage and more
https://min.io/docs/kes/concepts/
GNU Affero General Public License v3.0
456 stars 95 forks source link

Parse address from config if not provided with '--addr' #419

Closed rluetzner closed 10 months ago

rluetzner commented 10 months ago

This fixes a bug where kes would try to split and evaluate the listen address, before the config file was read. This caused kes to fail if no --addr value was provided.

With this change, the config file is parsed before we try to determine the listen address. If the --addr parameter is provided, we override the value from the config as expected.

This fixes #418 .

rluetzner commented 10 months ago

If I remember correctly, kes previously used 7373 as it's default port. Has the default changed intentionally? If not, we should restore the default in case addr is neither defined as a CLI parameter or in a config file.

aead commented 10 months ago

Dup of #416 However, LGTM 👍

rluetzner commented 10 months ago

If I remember correctly, kes previously used 7373 as it's default port. Has the default changed intentionally? If not, we should restore the default in case addr is neither defined as a CLI parameter or in a config file.

@aead , have you read this comment as well? Is it expected that a listen address is defined "somewhere", either as a CLI parameter or in a config file? If no, we should add something like

if (addrFlag=="") {
    addrFlag == ":7373"
}

after we tried the config.

aead commented 10 months ago

I thought kesconf does set a default but apparently it does not. Also, it seems cleaner to read the config file as it is and not set any defaults at this layer. So, agree that the server should set the addr to 0.0.0.0:7373 if neither a CLI argument nor a config file value is present. 👍

rluetzner commented 10 months ago

Perfect. I see you've added #420 already. 🙂