janiussyafiq commented on issue #12170:
URL: https://github.com/apache/apisix/issues/12170#issuecomment-4072688034
Hi @Baoyuantop, here's my proposed fix for this issue:
Approach:
1. Remove `generate_yaml()` entirely — this eliminates any possibility of
config.yaml being rewritten destructively
2. Keep the auto-generated key behaviour (agree with @fekitibi that the key
generation security could be improved, but that can be a separate discussion)
- When admin_key is empty, a warning log is emitted notifying the user that
a key has been auto-generated, along with the key value
- Instead of rewriting the entire file, only the key field is written back
via targeted string substitution — all comments, formatting, and structure in
config.yaml are fully preserved
Impact on existing tests:
Some test cases rely on reading the generated key from config.yaml after
startup:
https://github.com/apache/apisix/blob/4990927937280037602e81bb1b9554a784afa076/t/cli/test_admin.sh#L78
Since the targeted substitution only works when `key: ''` is explicitly
present in the raw file, test cases that write a minimal config without the
admin_key block (relying on merged defaults to supply it) will need to be
updated to explicitly include the admin_key block so the substitution can find
and replace it. E.g.
https://github.com/apache/apisix/blob/4990927937280037602e81bb1b9554a784afa076/t/cli/test_admin.sh#L26-L35
If this approach looks good, I will proceed with the full implementation and
test updates in this PR #13091 .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
