Proposal: support single app.config.yaml and extension manifest

[moritzraho]

Proposal

tl;dr Unify all the user specified configuration files in a single app.config.yaml in a non-breaking way and add support for extension manifest config

With the switch to extension registry, instead of exposing yet another configuration file we have the opportunity to unify all the user configuration bits.
Developers should be able to configure their application/extension fully using a single file. Their configuration should not to be mixed up with auto-generated changing fields, e.g. like it is the case now with.aio console config that changes on aio app use. This will also simplify the config parsing and merging logic on our side.

The proposal is to unify all the user configuration in app.config, i.e.:

1. user config currently supported by .aio.app as top level key,values
2. runtimeManifest config overwrites the current manifest.yml
3. extensionManifest config for the extension registry
4. env to store non secret commitable environment variables
5. hooks overwrites hooks in package.json

Note that this change is non-breaking, users are still free to use manifest.yml, define hooks in package.json or configure .aio.

But when using app.config only:

• .aio would hold only auto-generated values (console configuration) meaning it can be gitignored
• .env would only be used for secret environment variables

Example app.config.yaml:

# 1. app config overwrites aio.app
tvmurl: 'fakeurl'
actions: 'otherpath'
dist: 'other/path'
web: 'yet/another/web-src'
hostname: 'customhost'
htmlcacheduration: 'custom'
jsCacheDuration: 'custom'
cssCacheDuration: 'custom'
imageCacheDuration: 'custom'

# 2. Runtime manifest overwrites manifest.yml
runtimeManifest:
  packages:
    custom-package:
      license: Apache-2.0
      actions:
        generic:
          function: actions/generic/index.js
          web: 'yes'
          runtime: 'nodejs:12'
          inputs:
            LOG_LEVEL: debug
            rtenv: $RT_ENV
          annotations:
            require-adobe-auth: true
            final: true

# 3. extension manifest for extension registry
extensionManifest:
  endpoints:
    firefly/excshell/1":
      view:
        # this is a placeholder, we need a way to not specify the namespace here as it is part of the autogen config
        href: "https://<namespace>.adobeio-static.net/"

# 4. non-secret env only ! use .env for secrets
env:
  RT_ENV: stage

# 5. script hooks, overwrites hooks defined in package.json
hooks:
  post-app-build: 'echo hello'

Open points

The PR is a minimal POC the following is missing for a complete solution:

• (edit) only include extensions in extensionManifest ? see note below.
• non-secret env vars and hooks config in app.config.yaml not supported yet.
• keep support for deploy manifest.yml via runtime plugin: we could generate a compatible manifest file in dist/
• update generators to support app.config.yaml, remove manifest.yml, add .aio in .gitignore
• check for missing extension manifest fields
• support for json and hjson for app.config, read via aio-lib-core-config ?

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• I have signed the Adobe Open Source CLA.
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[purplecabbage]

This is great, some thoughts that will be added to as we go ...

1. Yes, although I think we need to be careful about existing projects, so this might be more work than we want
2. the existing manifest.yml syntax is well known, and understood. I think we should keep this intact, but improve existing .env value replacement, and add replacing values from appConfig as well
3. Yes, I don't like us referring to this as a manifest though, call this more config, or publish-config
4. I would say, not only private, but things that only I care about, and that have nothing to do with anyone else who might be working on a project, or things that are around just for a personal debug/dev process. We should also do a better job with defaulting to the config file, and not the env for things like namespace, which is really not a secret ...
5. I don't think we should move our script/hooks away from package.json. The reason being that this is sort of standard fare for modern web applications, and moving it, or providing the option to move it brings up new awkward choices, like which takes precedence ... do we run both if they are both present? The syntax for writing scripts in config.json/hson/yml is also less well known, so there is a whole category of issues this could bring up, multiline, string encoding, node_modules/bin referencing ...

• we should also make considerations for jest.config.js, .babelrc, .eslintrc.json, ... ultimately probably too problematic to move them, but they are also technically config and then there is .github/workflow things and .vscode/launch.json things also. At a minimum we should document what each of these files is for, and why it exists.

[moritzraho]

1. Yes, although I think we need to be careful about existing projects, so this might be more work than we want

+1 yes the idea is to make this non breaking, current config files still work, and fields in app.config.yml take precedence. So existing projects still work.

++1

3

Ok maybe we can add the string 'runtime': runtimePublishConfig ?

Agree, .env should be for the user. If .aio is gitignored it can hold autogen secrets (like runtime credentials) so that we can 'free' the .env for the user.
Maybe we should do this in 2 steps, as current users commit their .aio so there might be a risk of secret leakage while moving to the new config (aio app use stores secrets to .aio, user commits and push).
An alternative could be to introduce a new config file that is parse-able by aio-lib-core-config, like .aio.secrets or so

5. [...] The syntax for writing scripts in config.json/hson/yml is also less well known.

We simply execute the script via execa (not npm run ... anymore https://github.com/adobe/aio-cli-plugin-app/blob/master/src/lib/app-helper.js#L59) so I don't think there is any difference with writing a script in a package.json or an app.config.yml. And yaml supports multiline strings.
Also IMO scripts that are stored in package.json are intended to be run with npm run <script> by the user, however those are run by our internal tooling which does not interact with npm. Furthermore, I don't think there is a precedence problem, app.config.yml would simply take precedence as it is the new config file we want to promote. Finally, package.json is specific to js projects maybe one day we will need the config to be language agnostic.
But as this is a small part of the config, we can keep it there for now and move it if users ask for it.

At a minimum we should document what each of these files is for, and why it exists

this is standard stuff, I don't think we should wrap that config but +1 for clear documentation and links

[meryllblanchet]

Just filed AdobeDocs/app-builder#99 for a proper Application Configuration Guide in our docs.

[moritzraho]

Note extensionManifest field should actually hold a list to support multiple extension impl NO that's what the endpoints field is for

(edit) Note2: as of the extension manifest spec, the only field that is modifiable by the dev is endpoints. Other fields should be locked/autogenerated as those have a 1-1 mapping to the console project/workspace console config. Meaning the user should not touch those, and we should move them maybe to .aio

[icaraps]

@moritzraho Did you consider a JS config file ? I find them very flexible ie you can do a lot more compared to the YAML or JSON format.

[moritzraho]

@icaraps +1

[shazron]

5. I don't think we should move our script/hooks away from package.json. The reason being that this is sort of standard fare for modern web applications, and moving it, or providing the option to move it brings up new awkward choices, like which takes precedence ... do we run both if they are both present? The syntax for writing scripts in config.json/hson/yml is also less well known, so there is a whole category of issues this could bring up, multiline, string encoding, node_modules/bin referencing ...

I think the opposite, it shouldn't be in package.json -- right now it is the prescribed way to add hooks and scripts, and that's why it's there. Are devs actually needing to run these via npm run as well? If they are not, and are only putting it in package.json because of our requirements, then there is no need for them to be in package.json and we can unify this type of config into the proposed file.

[icaraps]

+1 to move the hooks out of package.json. Here are my arguments :

1. They can collide with dev defined scripts and cause unexpected bugs. Not every dev knows the hooks by heart and given even more hooks were added recently, the chances are higher now.
2. It doesn't scale. The more hooks we add, the higher the chances to have a cluttered and unreadable package.json. It's one of the reasons why you have so many different config files (babel, eslint, jest). Imagine all these configs would live in the package.json that would be a mess.
3. package.json doesn't support multiline commenting and therefore devs can't comment on what the hooks are actually used for which negatively impacts the dev experience.

In general, I'd prefer to have the aio specific config separated and isolated from any other config.