Feature #16064

Have some sanity checks on puppet code

Added by groente 2018-10-17 13:47:53 . Updated 2020-03-28 13:59:34 .

Status:
Resolved
Priority:
Normal
Assignee:
intrigeri
Category:
Target version:
Start date:
2018-10-17
Due date:
% Done:

40%

Feature Branch:
feature/16064-sanity-checks-for-puppet
Type of work:
Sysadmin
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

A pre-receive hook can do some basic checks on any puppet code being pushed

Subtasks

History

#1 Updated by groente 2018-10-17 13:58:46

  • Assignee changed from groente to intrigeri
  • % Done changed from 0 to 40
  • QA Check set to Ready for QA
  • Feature Branch set to feature/16064-sanity-checks-for-puppet

Since we briefly discussed sanity checks, here’s something that should ensure you won’t have to wade through my typo’s again ;-) Let me know what you think!

#2 Updated by groente 2018-10-18 14:13:06

  • Assignee changed from intrigeri to bertagaz

Hey bertagaz, actually, I’d quite like to know your thoughts on this aswell!

#3 Updated by intrigeri 2018-12-09 14:02:20

FWIW, I don’t mind taking this over if it helps move it forward.

#4 Updated by intrigeri 2019-06-02 14:42:56

  • Status changed from Confirmed to Needs Validation

#5 Updated by intrigeri 2019-08-22 14:05:35

  • Assignee changed from bertagaz to Sysadmins

#6 Updated by intrigeri 2019-09-10 05:58:38

The work anarcat is doing at Tor on this front could be relevant here: https://trac.torproject.org/projects/tor/ticket/31226

#7 Updated by intrigeri 2019-09-19 08:09:26

intrigeri wrote:
> The work anarcat is doing at Tor on this front could be relevant here: https://trac.torproject.org/projects/tor/ticket/31226

While it would be nice, on the long term, to use the same validator as anarcat (it does much more than ours and we don’t have to maintain it), for now I opted for improving the initial code proposed by groente, as a shortest path towards having some checks.

I’ve improved the code quite a bit (bug fixes, 1 new feature, performance improvements, robustness, code style) and it Works On My Machine™. Please review and deploy if happy :)

#8 Updated by zen 2020-03-28 10:55:01

  • Assignee changed from Sysadmins to zen

#9 Updated by zen 2020-03-28 12:48:56

  • Assignee changed from zen to intrigeri

I’ve reviewed and merged your changes, and I’ve left 2 more improvements in the tip of the force-pushed feature branch.

Please review and merge. Then I think we can close this as it’s enough for now.

#10 Updated by intrigeri 2020-03-28 13:59:34

  • Status changed from Needs Validation to Resolved

Hi @zen,

> I’ve reviewed and merged your changes,

Thank you!

I see that this failed to deploy to production due to an error in tails::gitolite::hooks::puppet.
I’ve fixed that in 002b4be873e9f474c1f4353ac822420f606f911a, then deployed to puppet-git.lizard.
Then I’ve verified that the puppet-lint check works as expected.

> and I’ve left 2 more improvements in the tip of the force-pushed feature branch.
> Please review and merge.

Merged, then deployed, and verified that it works as expected.

> Then I think we can close this as it’s enough for now.

Agreed!

Finally, I did a little bit of linting all over the place, to establish a slightly better baseline and avoid alert fatigue triggering too early.

In passing, FWIW, I’ve seen “Warning: tag is a metaparam; this value will inherit to all contained resources in the tails::pip_package_from_repo definition”.
I did not investigate.