Skip to content

feat: move config-default.yaml to hardcoded lua file#11343

Merged
moonming merged 24 commits into
apache:masterfrom
bzp2010:chore-hardcoded-default-config
Jul 22, 2024
Merged

feat: move config-default.yaml to hardcoded lua file#11343
moonming merged 24 commits into
apache:masterfrom
bzp2010:chore-hardcoded-default-config

Conversation

@bzp2010

@bzp2010 bzp2010 commented Jun 8, 2024

Copy link
Copy Markdown
Contributor

Description

For the reasons mentioned in the email https://lists.apache.org/thread/nc7qr5bl0z2bmkfb5rqc01c32lkx0gv2, I'm submitting this PR. The front blocking has been resolved #11312

It moves the config-default.yaml in APISIX to a hardcoded Lua. While it is still a script file in the distribution of APISIX, you can think of it as a "binary" and the user gets the clear and unambiguous message "it should never be modified".

And since it's a script file, it'll be more expressive than a pure Lua, e.g. we can read some of the values from environment variables (although I have a better way of doing that).


Email copy:

Hi, community.

I am posting to this mailing list a proposal to remove the config-default.yaml default configuration file, and below I will describe the background and reasons for this and consider the benefits to us.

The proposal does not represent a community resolution, although I am a committer in the APISIX community. We still want to hear a variety of voices to make sure this is truly beneficial.

Background

Now APISIX has two configuration files, the user configuration file config.yaml and the default configuration file config-default.yaml.
When APISIX starts up, it reads these two configuration files and merges and overwrites the user configuration file with the default configuration to create the configuration that is actually used at runtime.

The recommendation from the developers is that users should only modify the user configuration file config.yaml, while ensuring that config-default.yaml remains intact.
This is documented here: https://apisix.apache.org/docs/apisix/next/installation-guide/

APISIX's default configuration can be found in conf/config-default.yaml file and it should not be modified.
It is bound to the source code and the configuration should only be changed by the methods mentioned above.

Users often ask how they should modify custom configurations, or encounter unintended problems after modifying the default configuration file.
When explaining to them why they should not modify the default configuration file but copy those keys that need to be modified to the user configuration file and then modify them, and explaining the logic of merging the user configuration file with the default configuration file, I do find this issue to be a troubling one.

Also, some PRs encountered errors when improving config-default.yaml. For example: #11284

Let's shift our attention to other major open source projects in the world, I have indeed seen very few implementations with such built-in annoyances. Often software can have its default configuration hard-coded into the program, bundled into a binary for distribution, and the user can use a new configuration file, which they will load at runtime and override the modified keys into the default configuration.
This way, the user only needs to remember that he should modify config.yaml and nothing else; and APISIX will start normally without providing any configuration. This really reduces the cost of understanding.

Therefore, I suggest the following.

Proposal

  1. Remove config-default.yaml and move its contents to a Lua file as a Lua table
  2. Fix test errors
  3. Use documentation to explicitly document all configuration items, which ideally should be generated from the schema.

This is APISIX enhancement work, which is not perceived by users unless they are using APISIX in a non-correct way.

Benefit

  1. Reduce the cost of explaining about the section, and the cost of understanding for the user.
  2. Reduce the APISIX code base
  3. Improve the documentation quality through the entire process.

The End

The discussion is asking for feedback, and you can reply to the email directly to the mailing list.
We can discuss constructive feedback. If there is no constructive feedback, I will start working on it.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@bzp2010

bzp2010 commented Jun 8, 2024

Copy link
Copy Markdown
Contributor Author

This PR has completed most of the work in the proposal, but the update of the documentation is not yet complete.

The schema included in APISIX for checking configuration files is outdated and not at all representative of the actual situation. So it cannot be used to generate usable documentation. It needs to be overhauled.
https://github.com/apache/apisix/blob/master/apisix/cli/schema.lua

Currently, many configurations are not properly documented, some incorrect fields are documented, and the JSON Schema library used by APISIX is highly non-standard and outdated. JSON Schema does not prohibit unlisted fields, so a user can certainly enter something that is not validated.

If the issue were to be addressed in this PR, this PR would become very large, which is not conducive to submission. So I would like to make changes in the next PR to include schema's and document generation.


Evidence:

Configurations not properly documented: https://github.com/apache/apisix/blob/master/apisix/cli/schema.lua#L323-L331

Non-existent fields: https://github.com/apache/apisix/blob/master/apisix/cli/schema.lua#L270-L277

@bzp2010
bzp2010 marked this pull request as ready for review June 8, 2024 18:55
Comment thread conf/config-default.yaml
Comment thread t/core/etcd-mtls.t
Comment thread apisix/cli/config.lua Outdated
nic-6443
nic-6443 previously approved these changes Jun 13, 2024
SkyeYoung
SkyeYoung previously approved these changes Jun 18, 2024
@bzp2010 bzp2010 changed the title feat: move config-default.yaml to hardcoded lua file [WIP] feat: move config-default.yaml to hardcoded lua file Jul 4, 2024
@bzp2010
bzp2010 dismissed stale reviews from SkyeYoung, nic-6443, and moonming via 608a4c6 July 13, 2024 04:29
@bzp2010 bzp2010 changed the title [WIP] feat: move config-default.yaml to hardcoded lua file feat: move config-default.yaml to hardcoded lua file Jul 13, 2024
@bzp2010
bzp2010 requested review from SkyeYoung, juzhiyuan, moonming, nic-6443 and shreemaan-abhishek and removed request for juzhiyuan July 13, 2024 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants