Jira (PUP-9578) Remove distinction between `puppet parser validate` and `puppet parser validate --tasks`

Reid Vandewiele (JIRA)

unread,

Mar 21, 2019, 5:13:02 PM3/21/19

to puppe...@googlegroups.com

Reid Vandewiele created an issue

Puppet /

PUP-9578

Remove distinction between `puppet parser validate` and `puppet parser validate --tasks`

Issue Type:	Improvement
Assignee:	Unassigned
Created:	2019/03/21 2:12 PM
Priority:	Normal
Reporter:	Reid Vandewiele

Right now users need to manually indicate to the syntax validation command whether or not they will be passing in manifest data that uses plan features.

puppet parser validate

puppet parser validate --tasks

Assertion: the only validation differences between these two commands can be automatically detected and set appropriately based on being in, or not being in, a plan block.

Given the above assertion, we should remove the user burden of needing to indicate to puppet parser validate when plan code is input. Rather, the parser/validator should Do The Right Thing™ automatically.

Eliminate the need to pass --tasks in any circumstance by auto-detecting plan blocks and validating inside them appropriately.

Add Comment

This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)

Henrik Lindberg (JIRA)

unread,

Mar 21, 2019, 7:16:03 PM3/21/19

to puppe...@googlegroups.com

Henrik Lindberg commented on

PUP-9578

Re: Remove distinction between `puppet parser validate` and `puppet parser validate --tasks`

Your assumptions are not correct - there are functions that may work in either or both modes.
Also, if it detects automatically if it should validate in one more or the other - what was it the user intended? What if you are validating something you think is a regular manifest and it has a plan in it? That is supposed to raise an error because it will not work if you intend to use that manifest for catalog compilation. (Obviously the vice versa applies).

Are you looking for something that validates a bunch of manifests and switches mode depending on where the manifest is stored perhaps?

Add Comment

Reid Vandewiele (JIRA)

unread,

Mar 21, 2019, 8:23:03 PM3/21/19

to puppe...@googlegroups.com

Reid Vandewiele commented on

PUP-9578

Re: Remove distinction between `puppet parser validate` and `puppet parser validate --tasks`

What does the user expect this command to do?

From the built-in help:

This action validates Puppet DSL syntax without compiling a catalog or
syncing any resources. [...]

As a user, I consider a plan to be Puppet DSL syntax just as much as I consider a desired-state manifest to be. While there is utility in making finer-grained validation available (such as are provided with the pdk validate command), I want a base syntax check to constrain itself to telling me if the content is valid Puppet DSL. Whether the content is valid for the purpose (based on e.g. if the name of the class or plan matches the modulename+filename, or if the plan or manifest is in a module-correct) is a higher level check.

The puppet parser validate command allows syntax checking on stdin, communicating that file name and location is not fundamentally important to its purpose.

The ask in this ticket is to make specification of purpose optional, or simply not part of this tool. This tool should consider a plan OR a manifest on stdin valid without the user needing to specify which is coming through. If the user chooses to specify, additional scrutiny can be applied. But if the user does not specify, a plan should not be reported as invalid syntax by default.

> Your assumptions are not correct - there are functions that may work in either or both modes.

I get that, but from a syntax and structure perspective (the value I want from this command), is it not possible to infer most anything we need to on that front based on what kind of block we're inside? E.g. in a plan block or in a class block, or not in a block at all. If not in a block, I would expect syntax validation to work for any function regardless of purpose. Syntax validation is not purpose validation.

> if it detects automatically if it should validate in one more or the other - what was it the user intended?

Per the above, I want an actual syntax validator that doesn't know about purpose. Purpose can certainly refine the check, but this ticket is asking for and asserting that it should not be necessary to know purpose to affirm that a file is valid DSL. It is only necessary to provide additional validation beyond that.

Add Comment

Henrik Lindberg (JIRA)

unread,

Mar 22, 2019, 9:26:03 AM3/22/19

to puppe...@googlegroups.com

Henrik Lindberg commented on

PUP-9578

Re: Remove distinction between `puppet parser validate` and `puppet parser validate --tasks`

Let's see... I can imagine that we validate and accept everything from both modes and then we check if there is a mix of constructs and if so fail
While it does not capture the error of "you cannot do this when..." it does at least catch constructs that will never work.

At runtime we must validate for one or the other - like now. This means that in any case, there needs to be a different validator for the parser validate case - it can probably be feature switched such that every encounter of a catalog related construct sets a flag and it remembers the first one it got - it does the same for task related constructs - when one trips the other it is an error and it can be reported as "cannot mix this... with that..." and get both locations.

So this would need to be a different mode - it could be the default for parser validate, and it could have options for taks and catalog compilation to test specifically for one or the other mode.

Add Comment

Reid Vandewiele (JIRA)

unread,

Mar 22, 2019, 1:15:03 PM3/22/19

to puppe...@googlegroups.com

Reid Vandewiele updated an issue

Puppet /

PUP-9578

Remove distinction between `puppet parser validate` and `puppet parser validate --tasks`

Change By:	Reid Vandewiele

Right now users need to manually indicate to the syntax validation command whether or not they will be passing in manifest data that uses plan features.

{code}puppet parser validate
{code}
{code}puppet parser validate --tasks
{code}
*Assertion:* what validation is appropriate can be detected automatically and set based on the first encounter of manifest-specific or plan-specific constructs.

Given the above assertion, we should remove the user burden of needing to indicate to {{puppet parser validate}} when plan code is input vs. when manifest code is input. Optionally, we can allow users to pass {{ \ - \ -tasks}} or a new flag such as {{ \ - \ -manifests}} to explicitly indicate purpose, but when run without flags the command should be able to validate either kind of Puppet DSL content.

Eliminate the _need_ to pass {{ \ - \ -tasks}} to validate plans by detecting the first plan- or manifest-specific constructs and selecting a validation mode accordingly.

Add Comment

Reid Vandewiele (JIRA)

unread,

Mar 22, 2019, 1:15:03 PM3/22/19

to puppe...@googlegroups.com

Reid Vandewiele updated an issue

Puppet /

PUP-9578

Remove distinction between `puppet parser validate` and `puppet parser validate --tasks`

Change By:	Reid Vandewiele

Right now users need to manually indicate to the syntax validation command whether or not they will be passing in manifest data that uses plan features.

{code}
puppet parser validate
{code}

{code}
puppet parser validate --tasks
{code}

*Assertion:* the only what validation differences between these two commands is appropriate can be automatically detected automatically and set appropriately based on being in, the first encounter of manifest-specific or not being in, a {{ plan }} block -specific constructs .

Given the above assertion, we should remove the user burden of needing to indicate to {{puppet parser validate}} when plan code is input vs . Rather when manifest code is input. Optionally , we can allow users to pass {{--tasks}} or a new flag such as {{--manifests}} to explicitly indicate purpose, but when run without flags the parser/validator command should Do The Right Thing™ automatically be able to validate either kind of Puppet DSL content .

Eliminate the need _need_ to pass {{--tasks}} in any circumstance to validate plans by auto- detecting the first plan blocks - or manifest-specific constructs and validating inside them appropriately selecting a validation mode accordingly .

Add Comment

Henrik Lindberg (JIRA)

unread,

Mar 24, 2019, 7:15:03 AM3/24/19

to puppe...@googlegroups.com

Henrik Lindberg commented on

PUP-9578

Re: Remove distinction between `puppet parser validate` and `puppet parser validate --tasks`

A bit of refactoring is needed:

The Checker4_0 validator is currently the base implementation. Changes in what it does is an API change so we should keep this or we have to do these changes on a major version boundary.
Refactor Checker4_0 into a common part BaseChecker and a "catalog compilation" part. We keep Checker4_0 as the name of the "catalog compilation" checker (probably deprecate it at the same time in favour of a better name - it is no longer checking just "4.0").
Refactor TaskChecker, the runtime part for --tasks and derive it from the BaseChecker instead of Checker4_0.
Implement a CommonChecker derived from BaseChecker that is agnostic, but does not allow expressions of mixed kind in the same file

By doing this, we get one advantage in that the CommonChecker would not be used at runtime and we could then add more elaborate static checks to it if we want to (note that we at runtime push many errors to be caught at runtime instead of being validated when they are hard to detect statically).

Add Comment

Jorie Tappa (JIRA)

unread,

Mar 25, 2019, 12:52:03 PM3/25/19

to puppe...@googlegroups.com

Jorie Tappa updated an issue

Puppet /

PUP-9578

Remove distinction between `puppet parser validate` and `puppet parser validate --tasks`

Change By:	Jorie Tappa
Team:	Server

Add Comment

Jorie Tappa (JIRA)

unread,

Mar 25, 2019, 12:52:04 PM3/25/19

to puppe...@googlegroups.com

Jorie Tappa updated an issue

Puppet /

PUP-9578

Remove distinction between `puppet parser validate` and `puppet parser validate --tasks`

Change By:	Jorie Tappa
Sub-team:	Language

Add Comment

Cas Donoghue (Jira)

unread,

Dec 2, 2020, 2:55:04 PM12/2/20

to puppe...@googlegroups.com

Cas Donoghue commented on

PUP-9578

Re: Remove distinction between `puppet parser validate` and `puppet parser validate --tasks`

Nick Walker This seems like it is useful for a bolt user to validate plans. Should we prioritize making a clear workflow in bolt for the equivalent of using the --tasks flag? That way you would just use the bolt feature for this and keep using the puppet parser version for manifests.