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

(GH-1118) Add ability to skip validating files #1114

Merged
merged 1 commit into from
Jul 20, 2021

Conversation

jpogran
Copy link
Contributor

@jpogran jpogran commented Jun 28, 2021

This introduces a project level setting for a list of ignored files or patterns for the pdk validate command. Discussion: #1066

# New directory with malformed yaml files
❯ ls foo | select name

Name
----
foo.yaml
wakka.yaml

# rando malformed yaml in working directory
❯ ls *.yaml  | select name

Name
----
hiera.yaml
outside.yaml

# add a wildcard for the foo directory to the ignore list
❯ pdk set config project.validate.ignore "'foo/*.yaml'"
pdk (INFO): Added new value 'foo/*.yaml' to 'project.pdk_validate.ignore'
project.pdk_validate.ignore=["foo/*.yaml"]

# run validate which ignores anything in foo, but validates outside.yaml
❯ pdk validate yaml
pdk (INFO): Using Ruby 2.7.3
pdk (INFO): Using Puppet 7.7.0
+ [X] Running yaml validators ...
|__ [X] Checking YAML syntax (**/*.yaml **/*.yml).
pdk (ERROR): yaml-syntax: Unsupported class: Tried to load unspecified class: Date (outside.yaml)

Closes #1118 and #1117

@jpogran jpogran requested a review from a team as a code owner June 28, 2021 19:52
@jpogran jpogran marked this pull request as draft June 28, 2021 19:52
@jpogran jpogran force-pushed the gh-1111-block-files branch 2 times, most recently from a6b9815 to fc94626 Compare June 30, 2021 18:20
@jpogran jpogran self-assigned this Jun 30, 2021
@TuningYourCode
Copy link

Hi,

we struggled on the yaml linting in our own modules too.
Use-Case we deploy own filebeat-"modules" (which are basiclly just yaml files with some optional features not covered by yaml specs - e.g. https://github.com/elastic/beats/blob/master/filebeat/module/nginx/access/config/nginx-access.yml#L3-L5). We put these files into the files/ directory of our module but pdk validate would complain about them.

Our current "solution" is we put them into own puppet modules and don't validate them with pdk.

The only thing i don't understand on your implementation is where it is stored. Is it possible to set the ignore in the .sync.yml (or another one?) or is it done on the development system?
As we would like to have an option to set it in/for the module that we do not have to share any configs outside of our puppet-code repository which ships with all configs for linting etc. already.

Basiclly a developer should be possible to git clone $module-repo && pdk validate without having to set anything on his maschine to match the code standard of our modules.

Best regards,
Stephan

@cdenneen
Copy link

cdenneen commented Jul 1, 2021

The only thing i don't understand on your implementation is where it is stored. Is it possible to set the ignore in the .sync.yml (or another one?) or is it done on the development system?
As we would like to have an option to set it in/for the module that we do not have to share any configs outside of our puppet-code repository which ships with all configs for linting etc. already.

Basiclly a developer should be possible to git clone $module-repo && pdk validate without having to set anything on his maschine to match the code standard of our modules.

Best regards,
Stephan

Yes this would add a pdk.yaml file to your module which would be present in the module after git clone so pdk validate would perform the same on any developers machine.

@jpogran jpogran changed the title (GH-1111) Add ability to skip validating files (GH-???) Add ability to skip validating files Jul 1, 2021
@jpogran jpogran force-pushed the gh-1111-block-files branch 5 times, most recently from dcf3695 to 614936d Compare July 8, 2021 18:58
@codecov
Copy link

codecov bot commented Jul 8, 2021

Codecov Report

Merging #1114 (d8f02bb) into main (11416c3) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head d8f02bb differs from pull request most recent head d7ee603. Consider uploading reports for the commit d7ee603 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1114      +/-   ##
==========================================
- Coverage   91.25%   91.25%   -0.01%     
==========================================
  Files         137      137              
  Lines        5570     5578       +8     
==========================================
+ Hits         5083     5090       +7     
- Misses        487      488       +1     
Impacted Files Coverage Δ
lib/pdk/config.rb 98.56% <100.00%> (+0.03%) ⬆️
lib/pdk/validate/invokable_validator.rb 93.25% <100.00%> (+0.31%) ⬆️
lib/pdk/cli/release.rb 49.43% <0.00%> (-0.57%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dca0eb7...d7ee603. Read the comment docs.

@jpogran jpogran changed the title (GH-???) Add ability to skip validating files (GH-1118) Add ability to skip validating files Jul 8, 2021
@jpogran jpogran marked this pull request as ready for review July 19, 2021 15:56
Copy link
Contributor

@michaeltlombardi michaeltlombardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🎉

@michaeltlombardi
Copy link
Contributor

@jpogran I'm happy to merge as-is unless we want to do a quick commit fixup for commit history?

Add a new config setting that represents a list of files to ignore when running any of the validators in `pdk validate`.

Add ignore files to invokable_validator so all validators use the ignore list

Co-authored-by: Dave Armstrong <david.armstrong@puppet.com>
@michaeltlombardi
Copy link
Contributor

Code coverage error seems to be a non-issue, merging.

@michaeltlombardi michaeltlombardi merged commit d07b66f into puppetlabs:main Jul 20, 2021
@cdenneen
Copy link

@da-ar @michaeltlombardi @jpogran can you tell me if this is in the nightlies yet?

export PDK_DEB_URL="https://nightlies.puppetlabs.com/apt/pool/bionic/puppet6-nightly/p/pdk/pdk_2.1.1.0.pre.1.g92b6829-1bionic_amd64.deb"
export PDK_VERSION="2.1.1.0.pre.1.g92b6829"
export PDK_RELEASE_TYPE="nightly"

❯ more pdk.yaml
---
ignore:
- cloudformation/*.yaml


pdk (WARN): This module is compatible with a newer version of PDK. Upgrade your version of PDK to ensure compatibility.
pdk (INFO): Using Ruby 2.7.3
pdk (INFO): Using Puppet 7.9.0
pdk (INFO): Running all available validators...
pdk (INFO): Validator 'puppet-epp' skipped for '/code'. No files matching '["**/*.epp"]' found to validate.
pdk (INFO): Validator 'task-metadata-lint' skipped for '/code'. No files matching '["tasks/*.json"]' found to validate.
┌ [✔] Running metadata validators ...
├── [✔] Checking metadata syntax (metadata.json tasks/*.json).
└── [✔] Checking module metadata style (metadata.json).
┌ [✔] Running puppet validators ...
├── [✔] Checking Puppet manifest syntax (**/*.pp).
└── [✔] Checking Puppet manifest style (**/*.pp).
┌ [✔] Running ruby validators ...
└── [✔] Checking Ruby code style (**/**.rb).
┌ [✔] Running tasks validators ...
├── [✔] Checking task names (tasks/**/*).
└── [✔] Checking task metadata style (tasks/*.json).
┌ [✖] Running yaml validators ...
└── [✖] Checking YAML syntax (**/*.yaml **/*.yml).
pdk (ERROR): yaml-syntax: Unsupported class: Tried to load unspecified class: Date (cloudformation/agent_iam.yaml)
pdk (ERROR): yaml-syntax: Unsupported class: Tried to load unspecified class: Date (cloudformation/agent_stack.yaml)
pdk (ERROR): yaml-syntax: Unsupported class: Tried to load unspecified class: Date (cloudformation/agent_static_iam.yaml)

@da-ar
Copy link

da-ar commented Jul 28, 2021

@cdenneen The nightlies aren't running at the moment as we've been working on the pipelines. This will be part of PDK 2.2.0.

@da-ar @michaeltlombardi @jpogran can you tell me if this is in the nightlies yet?

@cdenneen
Copy link

@da-ar thanks... are there instructions for how to build the pdk.deb package myself? I need to get this functionality pushed.... I tried to run a bundle binstubs but it doesn't seem to be getting me everything I need to replace the binary

Any help would be greatly appreciated

@cdenneen
Copy link

Just for context I built this inside a ruby:2.7 container:

❯ docker run --rm -ti -v $(pwd):/app -w /app ruby:2.7 bash -c "bundle install; bundle binstubs pdk"

But when I ran the resulting binary locally it didn't use any embedded ruby as expected it tried to use my system one as you can see here with Ruby 2.5.7:

❯ ~/src/gitlab/forks/pdk/bin/pdk validate
pdk (WARN): This module is compatible with an older version of PDK. Run `pdk update` to update it to your version of PDK.
pdk (INFO): Using Ruby 2.5.7
pdk (INFO): Using Puppet 7.9.0
pdk (INFO): Running all available validators...
[✖] Resolving default Gemfile dependencies.
pdk (FATAL):
/Users/cdenneen/.dotfiles/rbenv/versions/2.5.7/lib/ruby/2.5.0/rubygems.rb:284:in `find_spec_for_exe': can't find gem bundler (>= 0.a) with executable bundle (Gem::GemNotFoundException)
	from /Users/cdenneen/.dotfiles/rbenv/versions/2.5.7/lib/ruby/2.5.0/rubygems.rb:303:in `activate_bin_path'
	from /Users/cdenneen/.dotfiles/rbenv/versions/2.5.7/bin/bundle:23:in `<main>'

pdk (FATAL): Unable to resolve default Gemfile dependencies.
❯ 

@da-ar
Copy link

da-ar commented Jul 28, 2021

Rbenv plays havoc. I believe if you unset the environment variables found here: https://github.com/puppetlabs/pdk-vanagon/blob/0aa54c9129b137c2deabb0a417d59215df36fd91/resources/files/posix/pdk_env_wrapper you will have more success. Thats the best i can do for now.

@cdenneen
Copy link

@da-ar thanks but the issue I don't think is rbenv but more that in example you pasted your pointing at the embedded ruby inside of pdk but when I'm building pdk from source I don't have an /opt/puppetlabs like this:

/opt/puppetlabs/pdk/private/ruby/@@@RUBY_VERSION@@@/bin/pdk "$@"

pdk was built inside a ruby:2.7 docker image and the binary was placed into cloned git repo pdk/bin/pdk just don't have any pdk/private/ruby which is what need to add to pdk build instructions.

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

Successfully merging this pull request may close these issues.

Add ability to ignore files in pdk validate
5 participants