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

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

Open
sed-i opened this issue Sep 21, 2021 · 6 comments
Open
Labels

Comments

@sed-i
Copy link
Contributor

sed-i commented Sep 21, 2021

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
Copy link

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
Copy link

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

@facundobatista
Copy link
Contributor

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
Copy link
Contributor Author

sed-i commented Sep 24, 2021

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
Copy link

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
Copy link
Contributor

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).

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

No branches or pull requests

4 participants