App manifest.json overwrites my app config

[kulmann]

Describe the bug

With #8523 we've introduced a simple mechanism to load apps into ocis. My personal deployment using that feature is looking good so far.
@jstehle built the external-sites app in owncloud/web-extensions#13
I'll use this app as an example to demonstrate the issue I'm having.

Steps to reproduce

1. Download the zip file from https://github.com/owncloud/web-extensions/releases/tag/external-sites-v0.1.0
2. Extract it in your ocis apps folder
3. Adjust the original manifest.json from

{
  "entrypoint": "external-sites.js",
  "config": {
    "sites": []
  }
}

to

{
  "entrypoint": "external-sites.js",
  "config": {
    "sites": [
      {
        "name": "Wikipedia",
        "url": "https://wikipedia.com",
        "target": "embedded"
      }
    ]
  }
}

4. When the next release is available, download it and extract it to your ocis apps folder

Expected behavior

Your previously built config remains untouched during an app update.

Actual behavior

Your previously built config gets overwritten to the default during an app update. 😢

Proposal

It makes sense that the app can ship a default config. It might be that it needs certain config to be able to run. E.g. the draw-io app uses the manifest.json to ship a default service url, see https://github.com/owncloud/web-extensions/blob/848cfce3384324fd7001dbd5c289aafd7d9aba0c/packages/web-app-draw-io/public/manifest.json#L4

I see two options (happy to hear more!):

1. Ship the manifest.json with a different file name so that the admin needs to rename it from e.g. manifest.dist.json to manifest.json.
2. Allow to place a config.json file into the app folder which is then used by ocis to overwrite the config object from the manifest.json.

[kulmann]

@fschade since you've built the feature in ocis, what are your thoughts on this? :-)

[micbar]

IIRC we designed that a bit differently. You should not edit the manifest.json because that holds the default and will get overridden by an app update.

The config can be changed in a yaml file. Need to check where it needs to be put.

[fschade]

@kulmann as Michael wrote, the manifest should not be touched. Please use the yaml file, like here: https://github.com/owncloud/ocis/pull/9601/files#diff-436f194d94cba3a96cb32372d4995167583996c0bc800a76679093a28f053c19

[kulmann]

@kulmann as Michael wrote, the manifest should not be touched. Please use the yaml file, like here: https://github.com/owncloud/ocis/pull/9601/files#diff-436f194d94cba3a96cb32372d4995167583996c0bc800a76679093a28f053c19

I need a solution where the config can be placed inside the app folder. Otherwise it loses its simplicity.
Also, why is yaml a strict requirement? I feel much more comfortable writing json than yaml.

[fschade]

apps.json should work too, but i have to verify it.
if the apps.yaml thing is a problem, the APP/config.json would work for me too.

I have to add one thing, the idea behind the app folder was that it stays untouched because of the updates.

@micbar fine for you?

[kulmann]

apps.json should work too, but i have to verify it. if the apps.yaml thing is a problem, the APP/config.json would work for me too.

apps.json is already too complicated. Configuring the individual apps exactly where they are placed by the admin is very compelling!

I have to add one thing, the idea behind the app folder was that it stays untouched because of the updates.

I never had this idea. 😅 I wanted to put the individual configs for the respective apps into the respective app folders from the very beginning of the feature... it makes things so easy!

[fschade]

start working on it, nothing big.

[fschade]

@kulmann please test if it fits you needs, you just have to add a config.json inside the individual app directory:

{
  "disabled": false,
  "config": {
    "from_local": 1,
    "bar": 3
  }
}

one new thing beside the config overloading is that apps can now be disabled from the config.json and or apps.yaml.

the overloading priority is as follows: local.config > global.config > manifest.config, e.g.

manifest.json

{
  "entrypoint": "index.js",
  "config": {
    "from_manifest": 1,
    "bar": 1
  }
}

apps.yaml

bar:
  config:
    from_global: 1
    bar: 2

config.json

{
  "config": {
    "from_local": 1,
    "bar": 3
  }
}

result https://localhost:9200/config.json

{
  "external_apps": [
    {
      "id": "bar",
      "path": "/assets/apps/bar/index.js",
      "config": {
        "bar": 3,
        "from_global": 1,
        "from_local": 1,
        "from_manifest": 1
      }
    }
  ]
}

https://github.com/fschade/ocis/tree/app-config

please provide ur feedback, then ill create a pr for it.

[fschade]

PR: #9691

[fschade]

@kulmann does it work for you the way it is now?