PDK validate should have the ability to exclude directories

[jacobmw]

I am using pdk on my control repo and would like the ability to ignore everything under my modules directory and only test my code.
something like:
'pdk validate --exclude modules/'

[DavidS]

It looks like the validators could use a bit more strict patterns. See e.g.

pdk/lib/pdk/validators/puppet/puppet_syntax.rb

Line 17 in 9de9532

'**/**.pp'

.

Until this is fixed, you can pdk validate manifests/ lib/ spec/ to only check the files you're interested in.

[cdenneen]

With the following Rakefile puppet-lint and puppet syntax checks should be excluding certain paths but this isn't working:

PuppetLint.configuration.log_format = '%{path}:%{line}:%{check}:%{KIND}:%{message}'
PuppetLint.configuration.fail_on_warnings = true
PuppetLint.configuration.send('relative')
PuppetLint.configuration.send('disable_140chars')
PuppetLint.configuration.send('disable_class_inherits_from_params_class')
PuppetLint.configuration.send('disable_documentation')
PuppetLint.configuration.send('disable_single_quote_string_with_variables')
PuppetLint.configuration.send('disable_trailing_comma')
PuppetLint.configuration.send('disable_arrow_on_right_operand_line')

exclude_paths = %w(
  pkg/**/*
  vendor/**/*
  .vendor/**/*
  bundle/**/*
  .bundle/**/*
  spec/**/*
)

Rake::Task[:lint].clear

PuppetLint::RakeTask.new :lint do |config|
  # Pattern of files to ignore
  config.ignore_paths = exclude_paths

  # Should puppet-lint prefix it's output with the file being checked,
  # defaults to true
  config.with_filename = false

  # Should the task fail if there were any warnings, defaults to false
  config.fail_on_warnings = true

  # Format string for puppet-lint's output (see the puppet-lint help output
  # for details
  config.log_format = '%{path}:%{line}:%{check}:%{KIND}:%{message}'

  # Print out the context for the problem, defaults to false
  config.with_context = true

  # Enable automatic fixing of problems, defaults to false
  config.fix = false

  # Show ignored problems in the output, defaults to false
  config.show_ignored = true

  # Compare module layout relative to the module root
  config.relative = true
end

PuppetSyntax.exclude_paths = exclude_paths

[jeremy-asher]

Until this is fixed, you can pdk validate manifests/ lib/ spec/ to only check the files you're interested in.

@DavidS
This type of invocation actually causes problems when the validators each try to find appropriate files in each of the listed paths:

[root@3947da1779e7 test]# pdk validate manifests/ lib/ spec/
pdk (INFO): Running all available validators...
[✔] Checking Puppet manifest syntax (**/**.pp).
[✔] Checking Puppet manifest style (**/*.pp).
[✔] Checking Ruby code style (**/**.rb).
info: metadata-syntax: manifests/: Target does not contain any files to validate (["metadata.json", "tasks/*.json"]).
info: metadata-syntax: spec/: Target does not contain any files to validate (["metadata.json", "tasks/*.json"]).
error: metadata-syntax: lib/: File does not exist.
info: metadata-json-lint: manifests/: Target does not contain any files to validate (metadata.json).
info: metadata-json-lint: spec/: Target does not contain any files to validate (metadata.json).
error: metadata-json-lint: lib/: File does not exist.
info: task-metadata-lint: manifests/: Target does not contain any files to validate (tasks/*.json).
info: task-metadata-lint: spec/: Target does not contain any files to validate (tasks/*.json).
error: task-metadata-lint: lib/: File does not exist.
info: puppet-syntax: spec/: Target does not contain any files to validate (**/**.pp).
error: puppet-syntax: lib/: File does not exist.
info: puppet-lint: spec/: Target does not contain any files to validate (**/*.pp).
error: puppet-lint: lib/: File does not exist.
info: rubocop: manifests/: Target does not contain any files to validate (**/**.rb).
error: rubocop: lib/: File does not exist.

I could run each validator individually, but this means I could no longer use --parallel and it would lead to a pretty brittle CI configuration, needing to track whenever a folder is added or removed. CLI options would be a great start, though I think it would be most convenient if includes/excludes could be controlled by a configuration file of some sort, similar to the way this can be done for Ruby with.rubocop.yml, but for PDK as a whole.

[brwyatt]

There is also a .rubocop.yml that has various excludes that is getting ignored during the Ruby validation, too. This is especially annoying as it is currently including the contents of spec/fixtures, which ends up spitting out a lot of junk if you happen to depend on puppetlabs/apt. The validators shouldn't be passed a list of files (which seems to be the case), but should use their config files (if present). PDK generates a pretty good .rubocop.yml file, it really should use it.

[jeremy-asher]

@brwyatt good point. I discovered there is a poorly documented rubocop option that could be passed --force-exclusion, though it can't be set in.rubocop.yml:
https://github.com/bbatsov/rubocop/blob/c0ab99934420431a1b2c25c7eac00943d25364cb/lib/rubocop/options.rb#L304

As a workaround for rubocop issues, you should be able to set specific cop exclusions for the affected files in .rubocop.yml. For example:

Metrics/LineLength:
  Exclude: not_my_bad_code/**/*

Normally, you shouldn't have anything in the fixtures folder when validate runs, but an aborted unit test run will skip the cleanup step and leave fixtures sitting around that can get picked up by validate. If you run unit tests again to completion that should be resolved.

[brwyatt]

Odd, the unit tests are running to completion... so maybe there's an additional bug here, as they don't seem to be getting cleaned up after the unit test run (though it does claim to have cleaned up).

Either way, the exclusions should really be working, especially since pdk new module creates a perfectly valid .rubocop.yml with the proper/expected exclusions.

Until then, is there a way to make rubocop run with --force-exclusion from pdk validate or pdk validate ruby?

[jeremy-asher]

Odd, the unit tests are running to completion... so maybe there's an additional bug here, as they don't seem to be getting cleaned up after the unit test run (though it does claim to have cleaned up).

Not sure what's going on there then. You might want to open a separate issue on that.

Either way, the exclusions should really be working, especially since pdk new module creates a perfectly valid .rubocop.yml with the proper/expected exclusions.

Agreed. My guess is that PDK will ultimately handle all file inclusion / exclusions at some point instead of deferring that responsibility to the individual tools and their separate config files. This will probably be a better story for new users, but it is causing some trouble at the moment. @DavidS do you have anything you can share on the intent here?

Until then, is there a way to make rubocop run with --force-exclusion from pdk validate or pdk validate ruby?

No, that was just a crazy idea I had. PDK doesn't support arbitrary options, but, if you ask me, it sounds like a good flag to tack on if better support for exclusions is going to be hard to add.

[cdenneen]

Adding exclude_paths array should be possible to all rake tasks I would think. So I don’t think passing this to all jobs should be hard.

[DavidS]

@cdenneen Opened https://tickets.puppetlabs.com/browse/PDK-663 to track adding support for some of the existing configuration patterns.

@jeremy-asher @brwyatt @jacobmw the PDK currently has no provisions for working in a control-repo. Some folks, like https://github.com/example42/psick had some success retrofitting their control-repo to look like a module to the PDK, but that is not what we ultimately want as a workflow.

[jeremy-asher]

@DavidS
I'm not sure what you mean by control-repo. I am experiencing this issue with a normal Puppet module and would like to use PDK for all of my internal modules individually.

I discovered this issue when I ran pdk validate against a module with a .pp file in its files folder that wasn't a Puppet manifest. PDK enumerated it when running puppet-lint which triggered a validation failure. I still can't come up with a workaround that I like for my purposes.

Do you have any suggestions how the exclusions can be addressed in the PDK codebase? I'd be happy to take a stab at a PR.

[DavidS]

@jeremy-asher This issue starts with "I am using pdk on my control repo and would like the ability to ignore everything under my modules directory", if you have a different use-case/failure/issue, please open a new one describing your experience.

[jeremy-asher]

@DavidS ah, sorry. I must be blind. Didn't mean to hijack this issue, but it was related. I can open another more general exclusion issue.

[cdenneen]

@DavidS yeah, while this issue is "control-repo" related I did send the same similar "module" issue to the mailinglist about it happening with non "control-repo" as well. Not sure where that sits but that would be better related to @jeremy-asher issue.