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

(PDK-1618)(PDK-1613)(PDK-1616) Add Control Repo support to Validators #858

Merged
merged 5 commits into from
Feb 24, 2020

Conversation

glennsarti
Copy link
Contributor

@glennsarti glennsarti commented Feb 22, 2020

Builds on #857 Merged


Now that Validators have a context to know whether they are operating in a
Module or Control Repo, the valdiator concrete classes need to be updated to
change their behaviour depending on the context.

  • When in a Control Repo, previously module level patterns now need to be
    changed to each modulepath in the Control Repo. The helper method
    contextual_pattern on the InvokableValidator class provides an easy way to
    do this
  • Update the concrete validators to handle patterns which are Arrays. Previously
    many just assumed a String
  • Update bundler helper to check for a Gemfile before processing binstubs.
    Previously this threw an obtuse error. Now that Control Repos are supported
    it is common that they do not have a Gemfile.
  • Updated pdk validate to support the controlrepo feature flag
  • Updated pdk validate to support NOT having a Gemfile, which is common in
    control repos.
  • Updated the ExternalCommandValidator to support being used in a non-Gemfile
    environment. This meant searching for the external command not only as a
    binstub, but also in packaged or default bin directories

(PDK-1618) Add Feature Flags

(PDK-1618) Add controlrepo feature flag

(PDK-1613) Pass context into the Validators

(PDK-1616) Extend validators for a control-repo

(maint) Validators should use unique target list


TODO

  • Working on a real control-repo, with no Gemfile
  • Behind a featureflag
  • Acceptance tests green?
  • Adhoc package testing
  • Fix duplicate target issue.

@glennsarti glennsarti requested a review from a team as a code owner February 22, 2020 13:47
@coveralls
Copy link

coveralls commented Feb 22, 2020

Coverage Status

Coverage decreased (-0.1%) to 91.228% when pulling 2dce4c6 on glennsarti:implement-a-pdk-context into 6b2e642 on puppetlabs:master.

@glennsarti glennsarti force-pushed the implement-a-pdk-context branch 2 times, most recently from eb213fb to f44d91d Compare February 23, 2020 13:04
@glennsarti glennsarti changed the title {WIP} Implement a PDK context into the validators (PDK-1618)(PDK-1613)(PDK-1616) Add Control Repo support to Validators Feb 23, 2020
Previously all features in the PDK were visible however in order to bring new
features faster, the PDK really needs feature flags. This commit

* Adds the feature flag mechanism using the PDK_FEATURE_FLAGS environment
  variable. An environment variable is used instead of configuration file
  because the the PDK::Config classes can be affected by feature flags, causing
  a circular reference.  Later commits may work around this limitation.
* Adds a section to the `user.` settings to display both the available and
  currently set feature flags

Later commits will add/remove feature flags as required.
The new Control Repo support is being put behind a feature flag so that users
can opt in, while the PDK team complete the feature.

This commit changes the context to detection to only detect Control Repos if the
flag is set.  The default behaviour of assuming everything is a module is still
maintained, unless the flag is set.
Now that the PDK was the concept of a Context, this needs to be passed into all
of the validators and used instead of methods like PDK::Util.module_root.  This
commit:

* Adds a `context` method to the base Validator abstract class, which is set
  during object initialization
* Changes all calls to create Validators to include the mandatory context
* Change references of PDK::Util.module_root to context.root_path for all
  validator abstract classes
* Updates tests for the new mandatory context
Now that Validators have a context to know whether they are operating in a
Module or Control Repo, the valdiator concrete classes need to be updated to
change their behaviour depending on the context.

* When in a Control Repo, previously module level patterns now need to be
  changed to each modulepath in the Control Repo.  The helper method
  `contextual_pattern` on the InvokableValidator class provides an easy way to
  do this
* Update the concrete validators to handle patterns which are Arrays. Previously
  many just assumed a String
* Update bundler helper to check for a Gemfile before processing binstubs.
  Previously this threw an obtuse error.  Now that Control Repos are supported
  it is common that they do not have a Gemfile.
* Updated `pdk validate` to support the controlrepo feature flag
* Updated `pdk validate` to support NOT having a Gemfile, which is common in
  control repos.
* Updated the ExternalCommandValidator to support being used in a non-Gemfile
  environment.  This meant searching for the external command not only as a
  binstub, but also in packaged or default bin directories
Previously the parse_targets method could return targets more than once which
causes the Puppet Parser Validator to fail due to duplicate definitions.  This
commit updates the parse_targets method to ensure that the targets are unique.

Ideally this should be done by only adding to the target list if it didn't
already exist, however due to the globbing, it's much easier to just use the
.uniq method on the array.
@glennsarti
Copy link
Contributor Author

IT'S HAPPENING!!

@glennsarti glennsarti merged commit 918945b into puppetlabs:master Feb 24, 2020
@rodjek rodjek added this to the February 2020 milestone Feb 26, 2020
@rodjek rodjek added the feature label Feb 26, 2020
@glennsarti glennsarti deleted the implement-a-pdk-context branch March 5, 2020 04:56
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.

3 participants