feat: aio app install command (complement to aio app pack)

[shazron]

Complement to the aio app pack command in #650
This will validate the app package created by aio app pack, and uncompresses the app package to your local disk.

• depends on feat: aio app pack command #650 to generate the app package file to test the command

How Has This Been Tested?

• npm test
• manual test of the command

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.

[codecov]

Codecov Report

Merging #662 (b6dd585) into master (2134f1e) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #662   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           55        57    +2     
  Lines         2955      3022   +67     
  Branches       554       561    +7     
=========================================
+ Hits          2955      3022   +67     

Impacted Files	Coverage Δ
src/lib/defaults.js	100.00% <ø> (ø)
src/commands/app/install.js	100.00% <100.00%> (ø)
src/lib/import-helper.js	100.00% <100.00%> (ø)
src/lib/install-helper.js	100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[shazron]

Pending completion of the schemas for app.config.yaml and deploy.yaml. Once those are done, and the tests updated for it (there are simple tests in there right now), this can be ready for review.

[shazron]

deploy.yaml schema is complete (there might be edits -- having a discussion with the api mesh team what properties are absolutely required).
app.config.yaml schema is complete as well, with minimal runtimeManifest checking for our purposes.

Some of the esoteric keys for the app.config.yaml application key (that users will not really use or encounter, since we never document these) are defined in https://github.com/adobe/aio-cli-lib-app-config/blob/ca93bf4740342601bab1df3d1657a0b47855cadc/src/index.js#L444 (search for singleUserConfig.)

There needs clarification for key name validation etc in the app.config.yaml schema (package name, annotation name, input name etc). For now, there is no validation for the annotations that can be set -- any annotation name can be set.

[moritzraho]

one comment and 2 nitpicks, lgtm otherwise

[shazron]

This is awesome. One thing - I see that we have the full schema defined for app.config.yaml and we are validating those, but it seems like if the package is an extension, then we won't end up doing validation on ext.config.yaml (Or however the developer decides to name their extension config). Should we extend validation to inspect extension configs too?

@MichaelGoberling I'm not sure if we can do this validation via the schema only since $include is our own definition. It will have to be an external validation where we parse app.config.yaml and search for any external configs to validate.
@raho one question: this $include only extends one level deep, or is it unbounded?

[shazron]

This is awesome. One thing - I see that we have the full schema defined for app.config.yaml and we are validating those, but it seems like if the package is an extension, then we won't end up doing validation on ext.config.yaml (Or however the developer decides to name their extension config). Should we extend validation to inspect extension configs too?

fixed in 724be85

[moritzraho]

one question: this $include only extends one level deep, or is it unbounded?

It is unbounded...
I actually missed that, but ext.config.yaml is a file included in our templates, but any part of the config could be included, and ext.config.yaml is not a standard config file and hence shouldn't have a schema...

I think the clean way would be to coalesce all the included files into a single app.config.yaml during packaging... And ideally, we should have a function in aio-cli-lib-app-config that outputs a coalesced configuration as most of the parsing logic is there already.

[shazron]

I actually missed that, but ext.config.yaml is a file included in our templates, but any part of the config could be included, and ext.config.yaml is not a standard config file and hence shouldn't have a schema...

Actually it looks like anything under application key in a stand-alone app (to me anyways). See the app.config.yaml schema.

I think the clean way would be to coalesce all the included files into a single app.config.yaml during packaging... And ideally, we should have a function in aio-cli-lib-app-config that outputs a coalesced configuration as most of the parsing logic is there already.

I think we want to avoid making the packaged app any different from the existing app, just to reduce complexity -- I'm not sure how it will affect any existing config file handling that we have.

[moritzraho]

Ok about not modifying the existing app, but at least to be able to validate the config fully we need to coalesce into one (in memory) yaml.
Edit: The problem is that anything could be included, even just a one liner config, so it doesn't make sense to validate ext.config.yaml because it is not a standard.

[shazron]

Edit: The problem is that anything could be included, even just a one liner config, so it doesn't make sense to validate ext.config.yaml because it is not a standard.

What about runtimeManifest though? Actually hooks is standard at least: https://developer.adobe.com/app-builder/docs/guides/app-hooks/

[shazron]

Spoke with @raho. It's much clear now, an $include is just like a C #include, where the contents are replaced in, thus the included config can be anything and not have a schema. So for validation, the best way is what Moritz suggested, coalesce the config, then do the validation. I will rework the implementation for this.

[shazron]

@raho @MichaelGoberling I don't think I should do the validation of app.config.yaml with $includes here in the app plugin:

1. firstly, I can't get access to the "pure" config since I can only load everything via this one export: https://github.com/adobe/aio-cli-lib-app-config/blob/ca93bf4740342601bab1df3d1657a0b47855cadc/src/index.js#L607
2. As discussed in a thread above, this validation should be in the @adobe/aio-cli-lib-app-config anyway, when loading the configs, and tracked in Move app.config.yaml schema to @adobe/aio-cli-lib-app-config #667 and Use a JSON schema to describe app.config.yaml and output of the lib aio-cli-lib-app-config#11

This could be a fast follow issue to implement, and #667 will include using loadConfig() (with an option to specify where the config is, or I can just change the current working directory) -- either that or have multiple exports (and publishing a major version update).

If there are no other issues (besides punting this off to #667) I believe all issues are resolved.

[moritzraho]

@shazron sounds good!
However, if possible, I would remove the ext.config.yaml schema

[shazron]

Oh yeah I missed this one , will do

…

________________________________ From: Moritz Raho ***@***.***> Sent: Tuesday, May 9, 2023 7:21:56 PM To: adobe/aio-cli-plugin-app ***@***.***> Cc: Shazron Abdullah ***@***.***>; Mention ***@***.***> Subject: Re: [adobe/aio-cli-plugin-app] feat: `aio app install` command (complement to `aio app pack`) (PR #662) EXTERNAL: Use caution when clicking on links or opening attachments. @shazron<https://github.com/shazron> sounds good! However, if possible, I would remove the ext.config.yaml schema — Reply to this email directly, view it on GitHub<#662 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAAI2C5REQBYNZCF5M6JXC3XFISFJANCNFSM6AAAAAAXGKVYZM>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[shazron]

@shazron sounds good! However, if possible, I would remove the ext.config.yaml schema

removed in 1b0ab58

[shazron]

I'll merge this first thing tomorrow and do more testing, which will include #650