canonical / charmcraft

Collaborate, build and publish charmed operators for Kubernetes, Linux and Windows.
Apache License 2.0
65 stars 70 forks source link

pack bundle with overlay (support more than a single document in the stream) #549

Open sed-i opened 2 years ago

sed-i commented 2 years ago

I'm trying to pack this bundle, which has multiple documents (bundle + overlay):

> $ charmcraft pack
Failed to read/parse config file '/home/tux/charms/lma-light-bundle/bundle.yaml': ComposerError('expected a single document in the stream', <yaml.error.Mark object at 0x7fe74d238be0>, 'but found another document', <yaml.error.Mark object at 0x7fe74d483310>)
Missing or invalid main bundle file: '/home/tux/charms/lma-light-bundle/bundle.yaml'. (full execution logs in '/home/tux/snap/charmcraft/common/charmcraft-log-2zyl3g7y')

> $ cat /home/tux/snap/charmcraft/common/charmcraft-log-2zyl3g7y
2021-09-21 16:11:05,982  charmcraft.guard                         DEBUG    Starting charmcraft version 1.3.0
2021-09-21 16:11:05,983  charmcraft.main                          DEBUG    Raw pre-parsed sysargs: args={'help': False, 'verbose': False, 'quiet': False, 'project_dir': None} filtered=['pack']
2021-09-21 16:11:05,984  charmcraft.main                          DEBUG    General parsed sysargs: command='pack' args=[]
2021-09-21 16:11:05,985  charmcraft.main                          DEBUG    Command parsed sysargs: Namespace(bases_index=None, debug=False, destructive_mode=False, entrypoint=None, force=False, requirement=None, shell=False, shell_after=False)
2021-09-21 16:11:05,987  charmcraft.commands                      ERROR    Failed to read/parse config file '/home/tux/charms/lma-light-bundle/bundle.yaml': ComposerError('expected a single document in the stream', <yaml.error.Mark object at 0x7fe74d238be0>, 'but found another document', <yaml.error.Mark object at 0x7fe74d483310>)
2021-09-21 16:11:05,987  charmcraft                               ERROR    Missing or invalid main bundle file: '/home/tux/charms/lma-light-bundle/bundle.yaml'. (full execution logs in '/home/tux/snap/charmcraft/common/charmcraft-log-2zyl3g7y')

the failure is while reding the YAML. It's not Charmcraft per se, it's the YAML library failing to parse that YAML. It seem we could support it, if we open the YAML file with load_all -- @facundobatista

Ref: charm-bundles/1058

mmanciop commented 2 years ago

This functionality is super important for LMA 2. We intend to ship LMA 2 as fundamentally "an appliance" in a self-contained Juju model, which means that being able to define the offers upfront in the bundle is a far, far better UX that not doing so.

mmanciop commented 2 years ago

Turns out that, for now, this is limitation was meant to be there, as overlays are mostly specific to a deployment (see https://github.com/juju/charmstore/pull/884 ). Nevertheless, I feel very strongly that default offers should be a valid exception to the rule.

facundobatista commented 2 years ago

At least charmcraft should not crash trying to read the file. Probably we should load_all the file and use only the first part.

sed-i commented 2 years ago

if you could force trust like that, and given that trust gives you access to cloud credentials I would be very weary deploying any bundle from the store without having to carefully audit its contents first... (which you would also have to do every time there is a revision bump) [...] you would need to add logic to juju to distinguish between local and remote bundles. also patch pylibjuju. also make sure it can work in non-interactive mode for automated deployments -- @achilleasa

I agree that load vs load_all in charmcraft should not be our safety measure against unsafe bundles.

achilleasa commented 2 years ago

if you could force trust like that, and given that trust gives you access to cloud credentials I would be very weary deploying any bundle from the store without having to carefully audit its contents first... (which you would also have to do every time there is a revision bump) [...] you would need to add logic to juju to distinguish between local and remote bundles. also patch pylibjuju. also make sure it can work in non-interactive mode for automated deployments -- @achilleasa

As it turns out my understanding of how the trust attribute works in the context of bundles was wrong. As @jameinel correctly pointed out, even if the bundle annotates some charms as trusted, the user must explicitly invoke the juju deploy command with the --trust flag to confirm that those charms should be trusted. Apologies for any confusion.

facundobatista commented 2 years ago

We will fix charmcraft, to not crash in this situation, and add a linter that will detect that the bundle has overlays, and refuse to pack if so.

But, as linters can be ignored, we will wait for Charmhub to start rejecting this kind of bundles (tracked here).