Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manage apps with hooks #466

Open
wrenix opened this issue Nov 11, 2023 · 10 comments
Open

Manage apps with hooks #466

wrenix opened this issue Nov 11, 2023 · 10 comments
Labels
enhancement New feature or request

Comments

@wrenix
Copy link
Collaborator

wrenix commented Nov 11, 2023

i like to add a easy mechanis to setup and configure appplication.

so i see the following new options in values.yaml:

nextcloud:
  appManagement:
    # -- if app not listed under apps it will be disabled
    autoDisable: true
    # -- if app not listed under apps it will be uninstalled
    autoRemove: true
    apps:
       deck:
         enabled: true
       drawio:
          enabled: true
          config:
            DrawioUrl: "https://embed.diagrams.net"
            DrawioOffline: "yes"

Therefore

  • i will use the occ command with an initContainer for enable and check there installation
  • nextcloud.appManagement.apps is an dict for easy use multiple value.yaml files and merge them
  • config of applications are stored in an kubernetes-secret because they could containe sensitiv informations

Does you like to merge such code? Do you like to change something in the outside behaviour ?


ping @jessebot and #272

@jessebot
Copy link
Collaborator

This does sound pretty neat and streamlined 👀 I think it's a good idea. How about you friends, @provokateurin @tvories ? :)

@provokateurin
Copy link
Member

I like the idea as well, I wonder how much work it is to concert the yaml into the php array syntax with nesting etc. (and how easy it could break).

@jessebot
Copy link
Collaborator

I like the idea as well, I wonder how much work it is to concert the yaml into the php array syntax with nesting etc. (and how easy it could break).

yeah, that makes sense. I think my main concern is the config section with each app. It may make more sense to start with support for just installing, enabling, and disabling, as config can get hairy, as you mentioned with regards to how nested a config for a given app might be, especially considering some apps will have different methods for config than others 🤔

@wrenix
Copy link
Collaborator Author

wrenix commented Nov 11, 2023

That is an implementation detail, that i like to discuss in my PR later.
but if you like to know - i wanted to write shell script and with using occ config:set (or config:import) without writing any php code ( and handling arrays in php).

@provokateurin
Copy link
Member

Sounds good, but how would you delete keys that got removed from the config?

@jessebot
Copy link
Collaborator

When this PR, nextcloud/docker#2115, get merged, we could use the docker level hooks by mounting configMaps into the post-installation or before-starting folders as per the docs. It was discussed a bit in nextcloud/docker#763 (comment) as to why we can't do that for now, but knowing these folders exist, reduces the need for more init containers or external post install jobs, not just for managing apps, for running other occ commands as well.

@provokateurin
Copy link
Member

Agreed, I feel like this is not the job of the chart. If there is a way to do this with the docker image we can and should support that.

@wrenix wrenix changed the title Manage apps with initContainer Manage apps with hooks Dec 25, 2023
@wrenix
Copy link
Collaborator Author

wrenix commented Dec 25, 2023

A Preview of my draft:

here i start to develop it: https://github.com/wrenix/nextcloud-helm
(current commit: wrenix@efddf1f <- maybe i amend it later)

just the logic of the app-management.sh is over to develop it here:
https://github.com/wrenix/nextcloud-helm/blob/main/charts/nextcloud/files/app-management.sh.gotmpl

the rest is logic to put that app-management.sh into a secret and mount it in the deployment under /docker-entrypoint-hooks.d/before-starting/app-management.sh
(that is high in development, so some command could change any time like an switch to occ config:import).


PS: thanks for the hint of the hooks @jessebot. i believe that is my code is just an helm-values to shell mapping, to put that into the image is to trivial, maybe it should be part of the nextcloud-server or just keep it here.

If you are not willing to merge such code, please say it now.
So i could switch for my setup to the values of extraVolumenMount and so on.

@wrenix
Copy link
Collaborator Author

wrenix commented Feb 8, 2024

@provokateurin
Copy link
Member

I think that approach is better, as it is more secure and less likely to fail, especially with upgrades.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants