Re: [foreman-dev] [POC] Automatic inspection of user-created provisioning templates

[Marek Hulan]

Thanks Shim, it looks very promising. Couple of thoughts for future. I 
think it'd be great if it was checking community-templates PRs. I wonder if 
we should integrate it with core or rather foreman_templates (or merge 
templates to core). Since it should probably be interactive and users 
should decide what changes they want to apply, some UI wizard will be 
needed, which is why I think it does not belong to foreman-maintain. and as 
always, while the tool is great, we need to write actual cops too.


--
Marek

Sent with AquaMail for Android
http://www.aqua-mail.com


On August 14, 2017 15:50:06 Ewoud Kohl van Wijngaarden 
 wrote:



On Mon, Aug 14, 2017 at 04:29:01AM -0700, ssht...@redhat.com wrote:

TL;DR: I have developed a way to scan any template and see if there are
suspicious/incorrect code patterns in them, so the templates will remain
valid even after foreman code changes.

Recently I have started to think about user created templates and foreman
upgrades.

When user upgrades foreman, hist default templates get upgraded by the
installer/migrations, but templates created by the user (both cloned and
from scratch) are not touched.
This could lead to invalid templates and broken provisioning functionality
for the user.
Good example for this would be the change

from calling to <%= foreman_url %> to <%= foreman_url('built') %>

I was looking for a way to inspect any template, in order to identify
problematic code as soon as the system is upgraded.

I came down to a solution based on rubocop - it's already analyzing source
files for patterns.
I have created a POC that analyzes a template written to a file, and
presents the resulting errors as regular rubocop (clang style).
All source codes are available as gist:
https://gist.github.com/ShimShtein/341b746f15826261053e97c2f435ff1a

Small explanation for the gist:

Entry point: inspect_template.rb
Usage:
Put everything from the gist to a single folder and execute:


inspect_template /path/to/template_source.erb


This script aggregates all the parts that are needed to create the
clang-like output.

The process:

  1. Strip all non-ruby text from the template. This is done by
  erb_strip.rb. It turns everything that is not a ruby code into spaces, so
  the ruby code remains in the same places as it was in the original file.
  2. Run rubocop with custom rules and erb monkey patch and produce a json
  report
 1. foreman_callback_cop.rb custom rule file. The most interesting
 line is "def_node_matcher :foreman_url_call?, '(send nil :foreman_url)
 '". Here you define which pattern to look for in the AST, in our case
 we are looking for calls (send) for foreman_url method without parameters.
 2. foreman_erb_monkey_patch.rb file: Patches rubocop engine to treat
 *.erb files as source files and not skip them.
  3. Process the resulting json to convert it to clang style highlighting.

Possible usages:

  - Scanning all template after foreman upgrade to see that they are still
  valid.
  - Linting a template while while editing.
  - Using rubocop's autocorrect feature to automatically fix offences
  found by this process.

Long shot: we can create custom rules to inspect code for our plugins too,
especially if we start creating custom rules in rubocop.

I am interested in comments, opinions and usages to see how to proceed from
here.


I think this is great and it should already include deprecations so
users get a heads up something changed. Otherwise users are not aware of
it and continue writing the deprecated styles. Auto-fix is fine but
having them review it can also be a teaching tool.

Another thing to consider is automatically referring to the correct
documentation for functions.

--
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an 
email to foreman-dev+unsubscr...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.



--
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[foreman-dev] [POC] Automatic inspection of user-created provisioning templates

[sshtein]

TL;DR: I have developed a way to scan any template and see if there are 
suspicious/incorrect code patterns in them, so the templates will remain 
valid even after foreman code changes.

Recently I have started to think about user created templates and foreman 
upgrades.

When user upgrades foreman, hist default templates get upgraded by the 
installer/migrations, but templates created by the user (both cloned and 
from scratch) are not touched.
This could lead to invalid templates and broken provisioning functionality 
for the user.
Good example for this would be the change 

 
from calling to <%= foreman_url %> to <%= foreman_url('built') %> 

I was looking for a way to inspect any template, in order to identify 
problematic code as soon as the system is upgraded.

I came down to a solution based on rubocop - it's already analyzing source 
files for patterns.
I have created a POC that analyzes a template written to a file, and 
presents the resulting errors as regular rubocop (clang style).
All source codes are available as gist: 
https://gist.github.com/ShimShtein/341b746f15826261053e97c2f435ff1a

Small explanation for the gist:

Entry point: inspect_template.rb
Usage:
Put everything from the gist to a single folder and execute:

> inspect_template /path/to/template_source.erb

This script aggregates all the parts that are needed to create the 
clang-like output.

The process:

   1. Strip all non-ruby text from the template. This is done by 
   erb_strip.rb. It turns everything that is not a ruby code into spaces, so 
   the ruby code remains in the same places as it was in the original file.
   2. Run rubocop with custom rules and erb monkey patch and produce a json 
   report
  1. foreman_callback_cop.rb custom rule file. The most interesting 
  line is "def_node_matcher :foreman_url_call?, '(send nil :foreman_url)
  '". Here you define which pattern to look for in the AST, in our case 
  we are looking for calls (send) for foreman_url method without parameters.
  2. foreman_erb_monkey_patch.rb file: Patches rubocop engine to treat 
  *.erb files as source files and not skip them.
   3. Process the resulting json to convert it to clang style highlighting.

Possible usages:

   - Scanning all template after foreman upgrade to see that they are still 
   valid.
   - Linting a template while while editing.
   - Using rubocop's autocorrect feature to automatically fix offences 
   found by this process.

Long shot: we can create custom rules to inspect code for our plugins too, 
especially if we start creating custom rules in rubocop.

I am interested in comments, opinions and usages to see how to proceed from 
here.

-- 
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.