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.

Sign up for GitHub

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

Jump to bottom

Check-yaml exception for Ansible Vault #273

Closed
blackillzone opened this issue Mar 19, 2018 · 11 comments
Closed

Check-yaml exception for Ansible Vault #273

blackillzone opened this issue Mar 19, 2018 · 11 comments
Labels
enhancement

Comments

@blackillzone
Copy link

Hi,

I've recently used the embed encrypt variables in my Ansible repos, and the check-yaml hook is failing because of the specific !vault usage.

Check Yaml...............................................................Failed
hookid: check-yaml

could not determine a constructor for the tag '!vault'
  in "inventories/group_vars/all.yml", line 62, column 13

Do you think it's possible to add this as an exception inside the hook (and make it pass for everyone who's using encypted variables in their playbooks) ?

Or should I add explicit exception inside my .pre-commit-config.yaml, for each files where I use those encrypted variables ?
@asottile
Copy link
Member

Might be with adding a "check-yaml-unsafe" which just attempts to validate syntax (and thus allowing extensions and unsafe constructs) instead of loading. Wouldn't be able to do some of the more advanced ideas I have for check-yaml such as duplicate key checking but it would enable this use case.

Thoughts?

@asottile asottile mentioned this issue Mar 19, 2018
Merged
@asottile
Copy link
Member

Made a patch for this, please try it out! #274 (via args: [--unsafe])

@blackillzone
Copy link
Author

blackillzone commented Mar 19, 2018 •
edited
Loading

Working like a charm for me and this use case ! Thank you for this quick patch :).

Looking at the PR, there is a small typo error on line 55 in the README.md, and on line 45 in pre_commit_hooks/check_yaml.py :

--unsafe` - Instaed of loading the files, simply parse them for syntax.

@asottile
Copy link
Member

woops, that's what I get for not copy pasting /me fixes

@asottile
Copy link
Member

This has been (finally!) released as part of v1.3.0! 🎉

@ssbarnea
Copy link
Contributor

ssbarnea commented Dec 1, 2018

I am not sure that is the best approach but thanks for implementig that workaround.

@asottile
Copy link
Member

asottile commented Dec 1, 2018

@ssbarnea do you have any suggestions?

@ViViDboarder
Copy link

This is old, but the ability to ignore a single line would be one way to handle this and is a fairly common feature for linters.

@asottile
Copy link
Member

@ViViDboarder did you try the solution that's been released above in 2018?

@ViViDboarder
Copy link

Yes, that works but it apparently prevents the other handy features such as checking for duplicate keys, no?

@asottile
Copy link
Member

yeah, yaml loading is all-or-nothing

StevenMaude added a commit to opensafely/documentation that referenced this issue Aug 10, 2022
@StevenMaude
Use --unsafe for check-yaml
f4f1927 
See pre-commit/pre-commit-hooks#273

The check currently fails because we use the `!!` syntax in the current
`mkdocs.yml` configuration.
StevenMaude added a commit to opensafely/documentation that referenced this issue Aug 10, 2022
@StevenMaude
Use --unsafe YAML check for mkdocs.yml
9cf8cdb 
See pre-commit/pre-commit-hooks#273

The check currently fails because we use the `!!` syntax in the current
`mkdocs.yml` configuration.

`--unsafe` refers to the YAML compliance: it doesn't load the file, merely
checks the syntax:

> Instead of loading the files, simply parse them for syntax.
> A syntax-only check enables extensions and unsafe constructs which would otherwise be forbidden.
> Using this option removes all guarantees of portability to other yaml implementations.

https://github.com/pre-commit/pre-commit-hooks/blob/98c88b23b7ea9a42f920adc1d7c8790f39772021/README.md

This is still better than failing completely because:

* a complete failure requires a committer to use `--no-verify`
* there is still a rudimentary check of this YAML

In practice, it gets loaded by MkDocs, which tells us if it's broken
anyway.
StevenMaude added a commit to opensafely/documentation that referenced this issue Aug 10, 2022
@StevenMaude
Use --unsafe YAML check for mkdocs.yml
2cab209 
See pre-commit/pre-commit-hooks#273

The check currently fails because we use the `!!` syntax in the current
`mkdocs.yml` configuration.

`--unsafe` refers to the YAML compliance: it doesn't load the file, merely
checks the syntax:

> Instead of loading the files, simply parse them for syntax.
> A syntax-only check enables extensions and unsafe constructs which would otherwise be forbidden.
> Using this option removes all guarantees of portability to other yaml implementations.

https://github.com/pre-commit/pre-commit-hooks/blob/98c88b23b7ea9a42f920adc1d7c8790f39772021/README.md#check-yaml

This is still better than failing completely because:

* a complete failure requires a committer to use `--no-verify`
* there is still a rudimentary check of this YAML

In practice, it gets loaded by MkDocs, which tells us if it's broken
anyway.
StevenMaude added a commit to opensafely/documentation that referenced this issue Aug 10, 2022
@StevenMaude
Use --unsafe YAML check for mkdocs.yml
0bfd032 
See pre-commit/pre-commit-hooks#273

The check currently fails because we use the `!!` syntax in the current
`mkdocs.yml` configuration.

`--unsafe` refers to the YAML compliance: it doesn't load the file, merely
checks the syntax:

> Instead of loading the files, simply parse them for syntax.
> A syntax-only check enables extensions and unsafe constructs which would otherwise be forbidden.
> Using this option removes all guarantees of portability to other yaml implementations.

https://github.com/pre-commit/pre-commit-hooks/blob/98c88b23b7ea9a42f920adc1d7c8790f39772021/README.md#check-yaml

This is still better than the current situation because:

* a complete failure requires a committer to use `--no-verify`
  or spend time trying to work out what's wrong with the pre-commit
  failure
* there is still a rudimentary check of this YAML, which doesn't happen
  in the current broken state

In practice, this YAML file gets loaded by MkDocs, which tells us if it's broken
anyway.

Other YAML files in this repository are still checked fully.
@StevenMaude StevenMaude mentioned this issue Aug 10, 2022
Merged
adrianlzt added a commit to adrianlzt/pre-commit-hooks that referenced this issue Dec 2, 2022
@adrianlzt
yaml: add new option to ignore "!ansible" tags
ce8b0e9 
This new option for the yaml checker modify ruamel to remove the "!"
from the "!vault" tag if it is found.
Removing that part allows the file to be parsed correctly, so other
errors could be found.

fixes: pre-commit#273
@adrianlzt adrianlzt mentioned this issue Dec 2, 2022
Closed
adrianlzt added a commit to adrianlzt/pre-commit-hooks that referenced this issue Dec 2, 2022
@adrianlzt
yaml: add new option to ignore "!ansible" tags
f605969 
This new option for the yaml checker modify ruamel to remove the "!"
from the "!vault" tag if it is found.
Removing that part allows the file to be parsed correctly, so other
errors could be found.

fixes: pre-commit#273
ldming pushed a commit to ldming/runbooks that referenced this issue Jul 17, 2023
Remove ops-central from the list of clusters to push prometheus rules to
a594a12 
Also this adds `--unsafe` to the `check-yaml` pre-commit hook. This is
because we are using `!reference` in `.gitlab-ci.yml` and without
`--unsafe`, it bombs out:

```
could not determine a constructor for the tag '!reference'
```

See: pre-commit/pre-commit-hooks#273
@Matt-STN Matt-STN mentioned this issue Nov 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

yaml: add new option to ignore "!ansible" tags adrianlzt/pre-commit-hooks
4 participants
@ssbarnea @ViViDboarder @asottile @blackillzone