AkkomaGang/akkoma
AkkomaGang
/
akkoma
12
63
Fork 83
Code Issues 167 Pull Requests 16 Packages Projects 2 Releases 13 Wiki Activity

Explicitly allow configDB keys #628

Closed
floatingghost wants to merge 9 commits from config-db-keys into develop
pull from: config-db-keys
merge into: AkkomaGang:develop
Conversation 1 Commits 9 Files Changed 10 +297 -71
floatingghost commented 2023-08-15 11:42:23 +00:00
Owner
Copy Link
No description provided.
floatingghost added 3 commits 2023-08-15 11:42:23 +00:00
ci/woodpecker/push/lint Pipeline was successful Details
ci/woodpecker/push/test Pipeline was successful Details
ci/woodpecker/push/build-amd64 Pipeline was successful Details
ci/woodpecker/push/build-arm64 Pipeline was successful Details
ci/woodpecker/push/docs Pipeline was successful Details
ci/woodpecker/pr/lint Pipeline was successful Details
ci/woodpecker/pr/test Pipeline failed Details
ci/woodpecker/pr/build-arm64 unknown status Details
ci/woodpecker/pr/docs unknown status Details
ci/woodpecker/pr/build-amd64 unknown status Details
d63bb3f6b1 Remove now-unused database_config_whitelist
floatingghost added 1 commit 2023-08-15 12:59:38 +00:00
ci/woodpecker/push/build-amd64 Pipeline is pending Details
ci/woodpecker/push/build-arm64 Pipeline is pending Details
ci/woodpecker/push/docs Pipeline is pending Details
ci/woodpecker/push/lint Pipeline is pending Details
ci/woodpecker/push/test Pipeline is pending Details
ci/woodpecker/pr/build-amd64 Pipeline is pending Details
ci/woodpecker/pr/build-arm64 Pipeline is pending Details
ci/woodpecker/pr/docs Pipeline is pending Details
ci/woodpecker/pr/lint Pipeline is pending Details
ci/woodpecker/pr/test Pipeline is pending Details
6f55ca14f2 Add exclusion reasons, mix task to check enabled keys
floatingghost added 1 commit 2023-08-15 13:00:45 +00:00
ci/woodpecker/push/build-amd64 Pipeline is pending Details
ci/woodpecker/push/build-arm64 Pipeline is pending Details
ci/woodpecker/push/docs Pipeline is pending Details
ci/woodpecker/push/lint Pipeline is pending Details
ci/woodpecker/push/test Pipeline is pending Details
ci/woodpecker/pr/lint Pipeline was successful Details
ci/woodpecker/pr/test Pipeline was successful Details
ci/woodpecker/pr/build-amd64 Pipeline was successful Details
ci/woodpecker/pr/build-arm64 Pipeline was successful Details
ci/woodpecker/pr/docs Pipeline was successful Details
7e1732c654 mix format
floatingghost added 3 commits 2023-08-17 10:42:28 +00:00
ci/woodpecker/push/build-amd64 Pipeline is pending Details
ci/woodpecker/push/build-arm64 Pipeline is pending Details
ci/woodpecker/push/docs Pipeline is pending Details
ci/woodpecker/push/lint Pipeline is pending Details
ci/woodpecker/push/test Pipeline is pending Details
ci/woodpecker/pr/build-amd64 Pipeline is pending Details
ci/woodpecker/pr/build-arm64 Pipeline is pending Details
ci/woodpecker/pr/docs Pipeline is pending Details
ci/woodpecker/pr/lint Pipeline is pending Details
ci/woodpecker/pr/test Pipeline is pending Details
16512acc3b Add warning for config that probably shouldn't be in the DB
floatingghost added 1 commit 2023-08-17 11:19:41 +00:00
ci/woodpecker/push/build-amd64 Pipeline is pending Details
ci/woodpecker/push/build-arm64 Pipeline is pending Details
ci/woodpecker/push/docs Pipeline is pending Details
ci/woodpecker/push/lint Pipeline is pending Details
ci/woodpecker/push/test Pipeline is pending Details
ci/woodpecker/pr/build-amd64 Pipeline is pending Details
ci/woodpecker/pr/build-arm64 Pipeline is pending Details
ci/woodpecker/pr/docs Pipeline is pending Details
ci/woodpecker/pr/lint Pipeline is pending Details
ci/woodpecker/pr/test Pipeline is pending Details
15aa62f178 Add a final few DB exclusion reasons
ilja commented 2023-08-17 14:02:51 +00:00
Contributor
Copy Link

Whitelisting settings definitely makes sense to me. I agree with the idea that a sysadmin is not the same as an instance admin, and thus it makes sense to me to keep certain settings away from an instance admin (And I don't see much reason to have sysadmin settings in the db since they should generally not change much or at all).

But now we have configs in

  • config/config.exs
  • config/description.exs
  • lib/pleroma/config/configurable_from_database.ex

If I understand correctly, config/description.exs is used to tell admin-fe what configs can be set. Can't that be used/considered as a whitelist? (Unless config/description.exs is supposed to become deprecated while moving more settings to akkoma-fe, but I'm unsure if deprecating this is a long term plan.)

I do wonder why db_exclusion_reason is a thing 🤔 Why add settings there, just to say they are excluded 🤷‍♀️ Maybe just to make the reasoning clear, idk. Or maybe it does more than I assume it does...

Whitelisting settings definitely makes sense to me. I agree with the idea that a sysadmin is not the same as an instance admin, and thus it makes sense to me to keep certain settings away from an instance admin (And I don't see much reason to have sysadmin settings in the db since they should generally not change much or at all). But now we have configs in * config/config.exs * config/description.exs * lib/pleroma/config/configurable_from_database.ex If I understand correctly, `config/description.exs` is used to tell admin-fe what configs can be set. Can't that be used/considered as a whitelist? (Unless `config/description.exs` is supposed to become deprecated while moving more settings to akkoma-fe, but I'm unsure if deprecating this is a long term plan.) I do wonder why `db_exclusion_reason` is a thing 🤔 Why add settings there, just to say they are excluded 🤷‍♀️ Maybe just to make the reasoning clear, idk. Or maybe it does more than I assume it does...
ilja reviewed 2023-08-17 14:09:03 +00:00
lib/pleroma/config/configurable_from_database.ex
@ -0,0 +3,4 @@
# Basically it's silly to let this be configurable
# set a list of things that we can set in the database
# this is mostly our stuff, with some extra in there
ilja commented 2023-08-17 14:08:28 +00:00
Contributor
Copy Link

The first line of this comment makes sense in the scope of this PR, but not for someone looking at the code as-is. I would remove that line.

I'm not sure what's meant in the last line with "our stuff", so maybe that should also be removed, or clarified?

The first line of this comment makes sense in the scope of this PR, but not for someone looking at the code as-is. I would remove that line. I'm not sure what's meant in the last line with "our stuff", so maybe that should also be removed, or clarified?
floatingghost closed this pull request 2023-12-11 10:16:38 +00:00
Some checks are pending
ci/woodpecker/push/build-amd64 Pipeline is pending
Details
ci/woodpecker/push/build-arm64 Pipeline is pending
Details
ci/woodpecker/push/docs Pipeline is pending
Details
ci/woodpecker/push/lint Pipeline is pending
Details
ci/woodpecker/push/test Pipeline is pending
Details
ci/woodpecker/pr/build-amd64 Pipeline is pending
Details
ci/woodpecker/pr/build-arm64 Pipeline is pending
Details
ci/woodpecker/pr/docs Pipeline is pending
Details
ci/woodpecker/pr/lint Pipeline is pending
Details
ci/woodpecker/pr/test Pipeline is pending
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
approved, awaiting change

   
bug

Something is not working

  
configuration

This requires config changes

  
documentation

This is about human-readable docs

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
extremely low priority

   
feature request

Something new?

  
Fix it yourself

Support has been attempted

  
help wanted

Need some help

  
invalid

Something is wrong

  
mastodon_api

   
needs docs

   
needs tests

   
not a bug

   
planned

   
pleroma_api

   
privacy

   
question

More information is needed

  
static_fe

   
triage

   
wontfix

This won't be fixed

No Label
approved, awaiting change
bug
configuration
documentation
duplicate
enhancement
extremely low priority
feature request
Fix it yourself
help wanted
invalid
mastodon_api
needs docs
needs tests
not a bug
planned
pleroma_api
privacy
question
static_fe
triage
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: AkkomaGang/akkoma#628
No description provided.
Delete Branch "config-db-keys"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?