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 validate --auto-correct" removes the first two letters of the class module name when "pdk" encounters "include ::class_name" #1214

Open
bschonec opened this issue Feb 21, 2023 · 5 comments
Labels
bug community needs-triage Newly created issue that has not been reviewed by a PDK contributor

Comments

@bschonec
Copy link

bschonec commented Feb 21, 2023

Describe the bug

pdk-2.6.1.0-1.el7.x86_64

"pdk validate" incorrectly removes both the "::" and the first two letters of the class module to be included in the following instance:

class profile::base {
  lookup('classes', Array[String], 'unique', []).include
  include ::puppet
  include ::class1
  include ::class2
}

The resulting file after correction:

class profile::base {
  lookup('classes', Array[String], 'unique', []).include
  include ::ppet
  include ::class1
  include ::class2
}

Notice how "include ::puppet" has been changed to "include ::ppet". The bug happens after every occurrence of ".include":

BEFORE:

class profile::base {
  lookup('classes', Array[String], 'unique', []).include
  include :: first_class
  lookup('classes', Array[String], 'unique', []).include
  include ::second_class
  lookup('classes', Array[String], 'unique', []).include
  include ::third_class
}

AFTER:


class profile::base {
  lookup('classes', Array[String], 'unique', []).include
  include rst_class
  lookup('classes', Array[String], 'unique', []).include
  include cond_class
  lookup('classes', Array[String], 'unique', []).include
  include ird_class
}

To Reproduce

The first "include ::class_module" following a line that contains ".include is incorrect. If you remove the ".include" after the lookup function, then no error occurs.

Expected behavior

Expectation is that the class module name is not altered (incorrectly) when removing the "::'.

Additional context

@bschonec bschonec added bug needs-triage Newly created issue that has not been reviewed by a PDK contributor labels Feb 21, 2023
@david22swan
Copy link
Member

@bschonec Hey, having some issues with replicating this, could you confirm it is still an issue and if so, could you tell me what rule is enforcing this change.
PDK valid should output it.

@bschonec
Copy link
Author

bschonec commented Sep 11, 2023

I neglected to include the '-a' argument for 'pdk validate'.

I noticed that if I have only a "../manifests/a.pp" file with the above format that 'pdk validate -a' will remove the "pu" letters from the file.

If I have another .pp file in the ../manifests directory, then the problem doesn't occur.

Try your test again with only a single .pp file, please.

@david22swan
Copy link
Member

david22swan commented Sep 12, 2023

@bschonec Could you tell me exactly what rule is being autocorrected to cause this?
I am still having difficulties replicating and it may be that my default rule setup doesn't include it.

Given the file being corrected it seems likely that this is an issue with puppet-lint specifically.

@bschonec
Copy link
Author

bschonec commented Sep 25, 2023

It seems to happen when:

lookup('classes', Array[String], 'unique', []).include

is included in the class module. I don't know if this helps:

pdk (INFO): Using Ruby 3.2.2
pdk (INFO): Using Puppet 8.1.0
pdk (INFO): Running all available validators...
pdk (INFO): Validator 'puppet-plan-syntax' skipped for '/home/nfiiseed/pdk_validate'. No files matching '["plans//*.pp"]' found to validate.
pdk (INFO): Validator 'puppet-epp' skipped for '/home/nfiiseed/pdk_validate'. No files matching '["
/.epp"]' found to validate.
pdk (INFO): Validator 'task-metadata-lint' skipped for '/home/nfiiseed/pdk_validate'. 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 (CORRECTED): puppet-lint: class included by absolute name (::$class) (manifests/init.pp:9:11)
pdk (CORRECTED): puppet-lint: class included by absolute name (::$class) (manifests/init.pp:9:11)
pdk (CORRECTED): puppet-lint: class included by absolute name (::$class) (manifests/init.pp:10:11)
pdk (CORRECTED): puppet-lint: class included by absolute name (::$class) (manifests/init.pp:11:11)

@bschonec
Copy link
Author

The linter removes the first two characters after the double-colons only when this line precedes:

lookup('classes', Array[String], 'unique', []).include

Once the double-colons are removed by the linter, then this behavior stops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug community needs-triage Newly created issue that has not been reviewed by a PDK contributor
Projects
None yet
Development

No branches or pull requests

3 participants