Not possible to configure some settings in the log section. #691

New Issue

2024-07-26T20:16:32Z

CompPhy commented

2024-07-26 20:16:32 +00:00

(Migrated from gitea.com)

We have need to enable access logs in our helm deployment, but ran into issues when trying to configure the log section. There's actually two issues here, both appear to be related to the handling inside config_environment.sh and maybe could be fixed in the same PR???

The first issue is around this line of code: https://gitea.com/gitea/helm-chart/src/branch/main/templates/gitea/config.yaml#L96
This code escapes characters in the section heading, but not in the setting itself. This causes things like logger.access.MODE: ',' to fail in the init-app-ini container. We were able to work around this error by putting logger_0X2E_access_0X2E_MODE: ',' into our values.yaml. However, it sure would be nice if periods were escaped properly at configuration time.

/usr/sbin/config_environment.sh: line 44: export: `GITEA__LOG__LOGGER.ACCESS.MODE=,': not a valid identifier

The second issue is around how it's capitalizing the setting keys. The setting should be logger.access.MODE = , but after init container renders the configuration it is actually LOGGER.ACCESS.MODE = , in app.ini file. This appears to prevent Gitea from seeing the setting properly, as our container is not generating the access log. We do know that logging is working, as the other default logs are showing up.

We have need to enable access logs in our helm deployment, but ran into issues when trying to configure the log section. There's actually two issues here, both appear to be related to the handling inside config_environment.sh and maybe could be fixed in the same PR??? 1) The first issue is around this line of code: https://gitea.com/gitea/helm-chart/src/branch/main/templates/gitea/config.yaml#L96 This code escapes characters in the section heading, but not in the setting itself. This causes things like `logger.access.MODE: ','` to fail in the init-app-ini container. We were able to work around this error by putting `logger_0X2E_access_0X2E_MODE: ','` into our values.yaml. However, it sure would be nice if periods were escaped properly at configuration time. ``` /usr/sbin/config_environment.sh: line 44: export: `GITEA__LOG__LOGGER.ACCESS.MODE=,': not a valid identifier ``` 2) The second issue is around how it's capitalizing the setting keys. The setting should be `logger.access.MODE = ,` but after init container renders the configuration it is actually `LOGGER.ACCESS.MODE = ,` in app.ini file. This appears to prevent Gitea from seeing the setting properly, as our container is not generating the access log. We do know that logging is working, as the other default logs are showing up.

👍 1

justusbunsi commented

2024-07-26 20:32:12 +00:00

(Migrated from gitea.com)

Thanks for reporting. Please share your values.yaml or at least the part where you configure the problematic setting.

CompPhy commented

2024-07-29 11:27:56 +00:00

(Migrated from gitea.com)

It's pretty simple, if you do this you will get the error mentioned in my previous comment. This error does prevent Gitea from starting.

config:
  log:
    logger.acces.MODE: ','

This will at least allow Gitea to start, but it won't actually output any access log information. As you can see, I'm just manually escaping the period.

config:
  log:
    logger_0X2E_access_0X2E_MODE: ','

This is the rendered config lines from the later, as you can see it doesn't have the proper case as indicated in Gitea documentation. I'm making an assumption here that configuration strings are case sensitive and that's why it's not applying the configuration to the log output.

[log]
LOGGER.ACCESS.MODE = ,

It's pretty simple, if you do this you will get the error mentioned in my previous comment. This error does prevent Gitea from starting. ```yaml config: log: logger.acces.MODE: ',' ``` This will at least allow Gitea to start, but it won't actually output any access log information. As you can see, I'm just manually escaping the period. ```yaml config: log: logger_0X2E_access_0X2E_MODE: ',' ``` This is the rendered config lines from the later, as you can see it doesn't have the proper case as indicated in Gitea documentation. I'm making an assumption here that configuration strings are case sensitive and that's why it's not applying the configuration to the log output. ```ini [log] LOGGER.ACCESS.MODE = , ```

👍 1

CompPhy commented

2024-07-29 13:21:28 +00:00

(Migrated from gitea.com)

Something else interesting I just noticed about handling of the config file. Deleting options from the helm release settings didn't remove the option from the config file. I was just trying to remove this bad option so I can try some other debug options, and noticed it didn't remove the line. It did add the new options but I had to actually edit the app.ini file manually in the container to remove.

This lead me down the road of manually trying some things inside the app.ini file. I can confirm that changing the option manually to lower case logger.access.MODE = , works as expected. I am now getting access logs in my output.

Something else interesting I just noticed about handling of the config file. Deleting options from the helm release settings didn't remove the option from the config file. I was just trying to remove this bad option so I can try some other debug options, and noticed it didn't remove the line. It did add the new options but I had to actually edit the app.ini file manually in the container to remove. This lead me down the road of manually trying some things inside the app.ini file. I can confirm that changing the option manually to lower case `logger.access.MODE = ,` works as expected. I am now getting access logs in my output.

pat-s commented

2024-07-29 14:05:02 +00:00

(Migrated from gitea.com)

-> #356

justusbunsi commented

2024-07-29 16:36:25 +00:00

(Migrated from gitea.com)

@CompPhy

It's pretty simple, if you do this you will get the error mentioned in my previous comment. This error does prevent Gitea from starting.
config:
 log:
   logger.acces.MODE: ','
This will at least allow Gitea to start, but it won't actually output any access log information. As you can see, I'm just manually escaping the period.
config:
 log:
   logger_0X2E_access_0X2E_MODE: ','
This is the rendered config lines from the later, as you can see it doesn't have the proper case as indicated in Gitea documentation. I'm making an assumption here that configuration strings are case sensitive and that's why it's not applying the configuration to the log output.
[log]
LOGGER.ACCESS.MODE = ,

Oh wow. I completely missed https://github.com/go-gitea/gitea/pull/24726 introducing dotted keys in sections. Thanks again for reporting. Dotted keys were never allowed prior mentioned PR. Thus, not functional.

@CompPhy >It's pretty simple, if you do this you will get the error mentioned in my previous comment. This error does prevent Gitea from starting. >```yaml >config: > log: > logger.acces.MODE: ',' >``` > >This will at least allow Gitea to start, but it won't actually output any access log information. As you can see, I'm just manually escaping the period. >```yaml >config: > log: > logger_0X2E_access_0X2E_MODE: ',' >``` > >This is the rendered config lines from the later, as you can see it doesn't have the proper case as indicated in Gitea documentation. I'm making an assumption here that configuration strings are case sensitive and that's why it's not applying the configuration to the log output. >```ini >[log] >LOGGER.ACCESS.MODE = , >``` Oh wow. I completely missed https://github.com/go-gitea/gitea/pull/24726 introducing dotted keys in sections. Thanks again for reporting. Dotted keys were never allowed prior mentioned PR. Thus, not functional.

CompPhy commented

2024-08-02 14:19:57 +00:00

(Migrated from gitea.com)

This goes beyond just periods, and affects a whole bunch of things. Here's another example that I just found as being completely broken.

https://docs.gitea.com/administration/config-cheat-sheet#git---config-options-gitconfig

It seems like case sensitivity is an issue here as well. When it comes to things like git.confg, there could be any number of options here for git extensions, many of which use periods and/or camel case notation. One example that we use pretty heavily is the multimailhook extension, which requires git.config variables to function properly.

I get that the helm is trying to set some things that are specific to Kubernetes setup, and combine this with static configuration values, but it seems like the current approach is broken in many ways here. Maybe we better option here would be to take the bulk of the config section at face value and then overwrite those things that helm needs to customize??? Right now it seems like it's trying to do the opposite and being "smart" about it is just causing extra problems.

Personally, I would rather have it fail because I entered something wrong in the values. Not have the correct setting in values but then get completely different information in the config file. Right now it's throwing errors because helm can't just use what I gave it, which is correct configuration. The only option is to leave these settings out of helm, and then go manually edit files inside the container; which completely defeats the purpose of the helm config, and also posses problems if someone wants to reproduce the same configuration later.

This goes beyond just periods, and affects a whole bunch of things. Here's another example that I just found as being completely broken. https://docs.gitea.com/administration/config-cheat-sheet#git---config-options-gitconfig It seems like case sensitivity is an issue here as well. When it comes to things like `git.confg`, there could be any number of options here for git extensions, many of which use periods and/or camel case notation. One example that we use pretty heavily is the `multimailhook` extension, which requires git.config variables to function properly. I get that the helm is trying to set some things that are specific to Kubernetes setup, and combine this with static configuration values, but it seems like the current approach is broken in many ways here. Maybe we better option here would be to take the bulk of the config section at face value and then overwrite those things that helm needs to customize??? Right now it seems like it's trying to do the opposite and being "smart" about it is just causing extra problems. Personally, I would rather have it fail because I entered something wrong in the values. Not have the correct setting in values but then get completely different information in the config file. Right now it's throwing errors because helm can't just use what I gave it, which is correct configuration. The only option is to leave these settings out of helm, and then go manually edit files inside the container; which completely defeats the purpose of the helm config, and also posses problems if someone wants to reproduce the same configuration later.

justusbunsi commented

2024-08-02 16:13:40 +00:00

(Migrated from gitea.com)

I agree, case sensitivity is an issue. All these setting with dots in it were added after I implemented that script. At that time, the keys in a section had to be full uppercase. To ensure this, the script does exactly that.

I understand and see that the newly added settings vary in their case sensitivity. This is definitely a bug in our init script.

Thank you for your investigations so far and bringing it to our attention.

Personally, I would rather have it fail because I entered something wrong in the values.

I guess that will be the fix here. Let deployments fail upon incorrect user input. Do you fancy helping by providing a PR to address this issue?

I agree, case sensitivity is an issue. All these setting with dots in it were added after I implemented that script. At that time, the keys in a section had to be full uppercase. To ensure this, the script does exactly that. I understand and see that the newly added settings vary in their case sensitivity. This is definitely a bug in our init script. Thank you for your investigations so far and bringing it to our attention. > Personally, I would rather have it fail because I entered something wrong in the values. I guess that will be the fix here. Let deployments fail upon incorrect user input. Do you fancy helping by providing a PR to address this issue?

👍 1

richard.corfield commented

2024-11-06 15:23:36 +00:00

(Migrated from gitea.com)

I have also been having trouble with this bug, but I've found if you fully encode the lower case configuration it gets passed through to the app.ini correctly. In my case I'm having to do this for logger.access.MODE: console:

        log:
          _0x6C__0x6F__0x67__0x67__0x65__0x72__0x2E__0x61__0x63__0x63__0x65__0x73__0x73__0x2E_MODE: console

Ugly, but it's a workaround.

I have also been having trouble with this bug, but I've found if you fully encode the lower case configuration it gets passed through to the `app.ini` correctly. In my case I'm having to do this for `logger.access.MODE: console`: ``` log: _0x6C__0x6F__0x67__0x67__0x65__0x72__0x2E__0x61__0x63__0x63__0x65__0x73__0x73__0x2E_MODE: console ``` Ugly, but it's a workaround.

👀 1

justusbunsi commented

2024-11-10 16:03:29 +00:00

(Migrated from gitea.com)

I guess we would have to fix it by not enforcing uppercase via script (EDIT: and escape the dots for the user) - which should be released as a breaking change. Users should review their configured settings to not get duplicates or some other issues after upgrading.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lunny/helm-chart#691