[POC] Automatic inspection of user-created provisioning templates

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
<https://github.com/theforeman/foreman/commit/7b966530c9ba48b2a37416465a3c9619f7143387>
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:

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.

··· On Mon, Aug 14, 2017 at 04:29:01AM -0700, sshtein@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.

After a great talk on community demo
<https://www.youtube.com/watch?v=aOqA8-wpPKQ>, here is a follow up with the
points that were raised during the discussion:

Use cases:

  1. Run all cops as part of community templates CI against the whole
    repository
  2. Run all cops against a single template invoked by the user from
    template editing screen (foreman core)
  3. Upgrade scenario: Preferably run cops for the next foreman version
    before the actual upgrade to make sure the templates will remain valid.

Features:

  1. List of rues should be pluggable [Shim]: It looks like it is a
    must-have for the engine.
  2. Deployment options
  3. Engine as a separate gem, cops in a relevant repository - core cops
    in core, plugin cops in plugins.
    2. Engine with all cops in a single gem, versioned per foreman
    version.
    3. Engine as part of templates plugin, cops as part of relevant
    plugins.
    4. Separate gems for everything: foreman-cops-engine,
    foreman-cops-core, foreman-cops-plugin1, foreman-cops-plugin2 e.t.c. Engine
    is versioned per foreman release version (for the sake of rubocop version),
    cops are versioned per plugin version.

General comments:

  1. Cops writing should be enforced on PR's that are changing the way to
    write templates [mhulan]
  2. Cops are dependent on core/plugin version [gwmngilfen]
··· On Monday, August 14, 2017 at 2:29:02 PM UTC+3, ssh...@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. > >

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 ewoud@kohlvanwijngaarden.nl wrote:

On Mon, Aug 14, 2017 at 04:29:01AM -0700, sshtein@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
https://github.com/theforeman/foreman/commit/7b966530c9ba48b2a37416465a3c9619f7143387
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+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

First attempt to create a design. It's an open discussion, everyone who
wants to chime in, please do.

The engine: will be deployed as a separate gem. My name suggestion
the-detective <https://en.wikipedia.org/wiki/The_Detective_(1968_film)> (Sinatra
plays a cop).

It will wrap the invocation of rubocop with defaults and parameters needed
to support our use case:

  1. Support for erb
  2. Support for completely customized set of cops.
  3. Parametrized list of folders containing cops to be added to the list.

In addition it will add tooling to expose a rack endpoint for rubocop
invocation:

  1. List of all available cops (kind of metadata)
  2. A POST method that receives a source file, list of cops, and output
    format that will return the result of rubocop's analysis.
  3. Will be mountable to any Rails application
  4. Will have an option to run as a standalone process (probably using
    passenger with sort-lived process retention settings, since its one process
    per request nature)

Usage for foreman needs:

Use case 1 (community templates CI):

  1. Reference the detective gem from templates plugin.
  2. Deploy foreman-core with templates plugin enabled.
  3. Add rake task that will invoke rubocop on specified folder using
    detective's invocation wrapper.

Use case 2 (Validate single template from templates UI)

  1. Reference detective gem from templates plugin.
  2. Add cops declaration ability to plugins in foreman core
  3. Templates plugin is responsible for adding/maintaining detective's
    endpoint.
  4. Foreman core exposes an option to add actions to template editing screen.
  5. Templates plugin uses extension point from 4 to add its own action that
    will invoke detective's endpoint and modify template editor to show the
    result as linting (it's possible with ace and monaco).

Use case 3 (upgrade scenario):
As a first step, we can try and report broken templates after the upgrade.
It will be pretty similar to community templates CI use case, only the
templates code will be exported from user's database.

I want to start working on the engine gem as soon as possible, so I would
really appreciate any inputs on the process before I have started with this
implementation.

Shim.

··· On Wednesday, August 30, 2017 at 11:48:09 AM UTC+3, ssh...@redhat.com wrote: > > > After a great talk on community demo > , here is a follow up with > the points that were raised during the discussion: > > Use cases: > > 1. Run all cops as part of community templates CI against the whole > repository > 2. Run all cops against a single template invoked by the user from > template editing screen (foreman core) > 3. Upgrade scenario: Preferably run cops for the next foreman version > before the actual upgrade to make sure the templates will remain valid. > > > Features: > > 1. List of rues should be pluggable [Shim]: It looks like it is a > must-have for the engine. > 2. Deployment options > 1. Engine as a separate gem, cops in a relevant repository - core cops > in core, plugin cops in plugins. > 2. Engine with all cops in a single gem, versioned per foreman > version. > 3. Engine as part of templates plugin, cops as part of relevant > plugins. > 4. Separate gems for everything: foreman-cops-engine, > foreman-cops-core, foreman-cops-plugin1, foreman-cops-plugin2 e.t.c. Engine > is versioned per foreman release version (for the sake of rubocop version), > cops are versioned per plugin version. > > General comments: > > 1. Cops writing should be enforced on PR's that are changing the way > to write templates [mhulan] > 2. Cops are dependent on core/plugin version [gwmngilfen] > > > > > On Monday, August 14, 2017 at 2:29:02 PM UTC+3, ssh...@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. >> >>

How would be the authentication/authorization work with this approach?

    • Ivan
··· On Wed, Sep 13, 2017 at 9:05 PM, wrote: > > First attempt to create a design. It's an open discussion, everyone who > wants to chime in, please do. > > The engine: will be deployed as a separate gem. My name suggestion > the-detective (Sinatra plays a cop). > > It will wrap the invocation of rubocop with defaults and parameters needed > to support our use case: > 1. Support for erb > 2. Support for completely customized set of cops. > 3. Parametrized list of folders containing cops to be added to the list. > > In addition it will add tooling to expose a rack endpoint for rubocop > invocation: > 1. List of all available cops (kind of metadata) > 2. A POST method that receives a source file, list of cops, and output > format that will return the result of rubocop's analysis. > 3. Will be mountable to any Rails application > 4. Will have an option to run as a standalone process (probably using > passenger with sort-lived process retention settings, since its one process > per request nature) > > Usage for foreman needs: > > Use case 1 (community templates CI): > 1. Reference the detective gem from templates plugin. > 2. Deploy foreman-core with templates plugin enabled. > 3. Add rake task that will invoke rubocop on specified folder using > detective's invocation wrapper. > > Use case 2 (Validate single template from templates UI) > 1. Reference detective gem from templates plugin. > 2. Add cops declaration ability to plugins in foreman core > 3. Templates plugin is responsible for adding/maintaining detective's > endpoint. > 4. Foreman core exposes an option to add actions to template editing screen. > 5. Templates plugin uses extension point from 4 to add its own action that > will invoke detective's endpoint and modify template editor to show the > result as linting (it's possible with ace and monaco). > > Use case 3 (upgrade scenario): > As a first step, we can try and report broken templates after the upgrade. > It will be pretty similar to community templates CI use case, only the > templates code will be exported from user's database. > > > I want to start working on the engine gem as soon as possible, so I would > really appreciate any inputs on the process before I have started with this > implementation. > > Shim. > > > > On Wednesday, August 30, 2017 at 11:48:09 AM UTC+3, ssh...@redhat.com wrote: >> >> >> After a great talk on community demo, here is a follow up with the points >> that were raised during the discussion: >> >> Use cases: >> >> Run all cops as part of community templates CI against the whole >> repository >> Run all cops against a single template invoked by the user from template >> editing screen (foreman core) >> Upgrade scenario: Preferably run cops for the next foreman version before >> the actual upgrade to make sure the templates will remain valid. >> >> >> Features: >> >> List of rues should be pluggable [Shim]: It looks like it is a must-have >> for the engine. >> Deployment options >> >> Engine as a separate gem, cops in a relevant repository - core cops in >> core, plugin cops in plugins. >> Engine with all cops in a single gem, versioned per foreman version. >> Engine as part of templates plugin, cops as part of relevant plugins. >> Separate gems for everything: foreman-cops-engine, foreman-cops-core, >> foreman-cops-plugin1, foreman-cops-plugin2 e.t.c. Engine is versioned per >> foreman release version (for the sake of rubocop version), cops are >> versioned per plugin version. >> >> General comments: >> >> Cops writing should be enforced on PR's that are changing the way to write >> templates [mhulan] >> Cops are dependent on core/plugin version [gwmngilfen] >> >> >> >> >> On Monday, August 14, 2017 at 2:29:02 PM UTC+3, ssh...@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: >>> >>> 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. >>> Run rubocop with custom rules and erb monkey patch and produce a json >>> report >>> >>> 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. >>> foreman_erb_monkey_patch.rb file: Patches rubocop engine to treat *.erb >>> files as source files and not skip them. >>> >>> 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+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.

>
>First attempt to create a design. It's an open discussion, everyone who
>wants to chime in, please do.
>
>The engine: will be deployed as a separate gem. My name suggestion
>the-detective <https://en.wikipedia.org/wiki/The_Detective_(1968_film)> (Sinatra
>plays a cop).
>
>It will wrap the invocation of rubocop with defaults and parameters needed
>to support our use case:
>1. Support for erb
>2. Support for completely customized set of cops.
>3. Parametrized list of folders containing cops to be added to the list.
>
>In addition it will add tooling to expose a rack endpoint for rubocop
>invocation:
>1. List of all available cops (kind of metadata)
>2. A POST method that receives a source file, list of cops, and output
>format that will return the result of rubocop's analysis.
>3. Will be mountable to any Rails application
>4. Will have an option to run as a standalone process (probably using
>passenger with sort-lived process retention settings, since its one process
>per request nature)

Why should it be a rack endpoint? My thinking was much more of a normal
ruby API with a command line tool around it. There should be no
passenger dependency to keep its dependencies small. foreman_templates
can consume the ruby API and expose it to the user as it wants.

>Usage for foreman needs:
>
>Use case 1 (community templates CI):
>1. Reference the detective gem from templates plugin.
>2. Deploy foreman-core with templates plugin enabled.
>3. Add rake task that will invoke rubocop on specified folder using
>detective's invocation wrapper.

Ideally we'd have a light command line tool that does:

detective
–cops /path/to/foreman/checkout/cops
–cops /path/to/katello/checkout/cops
–cops /path/to/other/plugin/with/cops
/path/to/some/template/dir
/path/to/another/template/dir

That way we can do a simple git clone foreman in community-templates,
bundle install and run it within Travis. This can indeed be wrapped in a
rake task but given the paths can change on a developers workstation it
is good to have an easy manual option.

··· On Wed, Sep 13, 2017 at 12:05:49PM -0700, sshtein@redhat.com wrote:

Use case 2 (Validate single template from templates UI)

  1. Reference detective gem from templates plugin.
  2. Add cops declaration ability to plugins in foreman core
  3. Templates plugin is responsible for adding/maintaining detective’s
    endpoint.
  4. Foreman core exposes an option to add actions to template editing screen.
  5. Templates plugin uses extension point from 4 to add its own action that
    will invoke detective’s endpoint and modify template editor to show the
    result as linting (it’s possible with ace and monaco).

Use case 3 (upgrade scenario):
As a first step, we can try and report broken templates after the upgrade.
It will be pretty similar to community templates CI use case, only the
templates code will be exported from user’s database.

I want to start working on the engine gem as soon as possible, so I would
really appreciate any inputs on the process before I have started with this
implementation.

Shim.

On Wednesday, August 30, 2017 at 11:48:09 AM UTC+3, ssh...@redhat.com wrote:

After a great talk on community demo
https://www.youtube.com/watch?v=aOqA8-wpPKQ, here is a follow up with
the points that were raised during the discussion:

Use cases:

  1. Run all cops as part of community templates CI against the whole
    repository
  2. Run all cops against a single template invoked by the user from
    template editing screen (foreman core)
  3. Upgrade scenario: Preferably run cops for the next foreman version
    before the actual upgrade to make sure the templates will remain valid.

Features:

  1. List of rues should be pluggable [Shim]: It looks like it is a
    must-have for the engine.
  2. Deployment options
  3. Engine as a separate gem, cops in a relevant repository - core cops
    in core, plugin cops in plugins.
    2. Engine with all cops in a single gem, versioned per foreman
    version.
    3. Engine as part of templates plugin, cops as part of relevant
    plugins.
    4. Separate gems for everything: foreman-cops-engine,
    foreman-cops-core, foreman-cops-plugin1, foreman-cops-plugin2 e.t.c. Engine
    is versioned per foreman release version (for the sake of rubocop version),
    cops are versioned per plugin version.

General comments:

  1. Cops writing should be enforced on PR’s that are changing the way
    to write templates [mhulan]
  2. Cops are dependent on core/plugin version [gwmngilfen]

On Monday, August 14, 2017 at 2:29:02 PM UTC+3, ssh...@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
https://github.com/theforeman/foreman/commit/7b966530c9ba48b2a37416465a3c9619f7143387
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+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

First, I don't think that this service has any secret information that
should be kept under authorization/authentication.

Even if we assume that some level of auth is indeed needed, the easiest way
to implement it would be through Rack middleware.
If the service is added to an application with existing auth middleware,
then you can add it to the relevant endpoints when mounting it.
In case we are trying to use the standalone version, we can either create
an extension point to add middleware, or mount it manually, just like in
the previous case.

Anyway I don't see a reason to limit access to those endpoints, except only
for cases of DDOS (which should be handled even before the Rack stack
anyway).

··· On Thu, Sep 14, 2017 at 10:44 AM, Ivan Necas wrote:

How would be the authentication/authorization work with this approach?

    • Ivan

On Wed, Sep 13, 2017 at 9:05 PM, sshtein@redhat.com wrote:

First attempt to create a design. It’s an open discussion, everyone who
wants to chime in, please do.

The engine: will be deployed as a separate gem. My name suggestion
the-detective (Sinatra plays a cop).

It will wrap the invocation of rubocop with defaults and parameters
needed
to support our use case:

  1. Support for erb
  2. Support for completely customized set of cops.
  3. Parametrized list of folders containing cops to be added to the list.

In addition it will add tooling to expose a rack endpoint for rubocop
invocation:

  1. List of all available cops (kind of metadata)
  2. A POST method that receives a source file, list of cops, and output
    format that will return the result of rubocop’s analysis.
  3. Will be mountable to any Rails application
  4. Will have an option to run as a standalone process (probably using
    passenger with sort-lived process retention settings, since its one
    process
    per request nature)

Usage for foreman needs:

Use case 1 (community templates CI):

  1. Reference the detective gem from templates plugin.
  2. Deploy foreman-core with templates plugin enabled.
  3. Add rake task that will invoke rubocop on specified folder using
    detective’s invocation wrapper.

Use case 2 (Validate single template from templates UI)

  1. Reference detective gem from templates plugin.
  2. Add cops declaration ability to plugins in foreman core
  3. Templates plugin is responsible for adding/maintaining detective’s
    endpoint.
  4. Foreman core exposes an option to add actions to template editing
    screen.
  5. Templates plugin uses extension point from 4 to add its own action
    that
    will invoke detective’s endpoint and modify template editor to show the
    result as linting (it’s possible with ace and monaco).

Use case 3 (upgrade scenario):
As a first step, we can try and report broken templates after the
upgrade.
It will be pretty similar to community templates CI use case, only the
templates code will be exported from user’s database.

I want to start working on the engine gem as soon as possible, so I would
really appreciate any inputs on the process before I have started with
this
implementation.

Shim.

On Wednesday, August 30, 2017 at 11:48:09 AM UTC+3, ssh...@redhat.com > wrote:

After a great talk on community demo, here is a follow up with the
points

that were raised during the discussion:

Use cases:

Run all cops as part of community templates CI against the whole
repository
Run all cops against a single template invoked by the user from template
editing screen (foreman core)
Upgrade scenario: Preferably run cops for the next foreman version
before

the actual upgrade to make sure the templates will remain valid.

Features:

List of rues should be pluggable [Shim]: It looks like it is a must-have
for the engine.
Deployment options

Engine as a separate gem, cops in a relevant repository - core cops in
core, plugin cops in plugins.
Engine with all cops in a single gem, versioned per foreman version.
Engine as part of templates plugin, cops as part of relevant plugins.
Separate gems for everything: foreman-cops-engine, foreman-cops-core,
foreman-cops-plugin1, foreman-cops-plugin2 e.t.c. Engine is versioned
per

foreman release version (for the sake of rubocop version), cops are
versioned per plugin version.

General comments:

Cops writing should be enforced on PR’s that are changing the way to
write

templates [mhulan]
Cops are dependent on core/plugin version [gwmngilfen]

On Monday, August 14, 2017 at 2:29:02 PM UTC+3, ssh...@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:

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.
Run rubocop with custom rules and erb monkey patch and produce a json
report

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.
foreman_erb_monkey_patch.rb file: Patches rubocop engine to treat *.erb
files as source files and not skip them.

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+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


You received this message because you are subscribed to a topic in the
Google Groups “foreman-dev” group.
To unsubscribe from this topic, visit https://groups.google.com/d/
topic/foreman-dev/KtVSqPKzXjA/unsubscribe.
To unsubscribe from this group and all its topics, send an email to
foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Ewoud:

About rack endpoint: It should be created for use case 2, where the check
is running in "online"-ish mode.
I prefer creating it as a microservice for scalability reasons, so I
wouldn't want to tie it too tightly with foreman-templates. Besides that,
tying it into foreman-templates will mean the same life cycle that I would
prefer to avoid.
As I already wrote for case 1 and 3, we could use the underlying ruby
wrapper directly, hence avoid the usage of the API.
As for additional dependencies, I probably would add passenger as an
optional (development) dependency, so you will have the full feature set on
development machines. In production it will check if if passenger is
available only if it runs in standalone mode. It will not ask for passenger
if someone has decided to use something else, like webrick (personally I
would recommend against webrick, it's not parallel at all which is
important here).

About the command line tool, I have nothing against it, it should be
available in the gem.

··· On Thu, Sep 14, 2017 at 2:06 PM, Ewoud Kohl van Wijngaarden < ewoud@kohlvanwijngaarden.nl> wrote:

On Wed, Sep 13, 2017 at 12:05:49PM -0700, sshtein@redhat.com wrote:

First attempt to create a design. It’s an open discussion, everyone who
wants to chime in, please do.

The engine: will be deployed as a separate gem. My name suggestion
the-detective https://en.wikipedia.org/wiki/The_Detective_(1968_film)
(Sinatra
plays a cop).

It will wrap the invocation of rubocop with defaults and parameters needed
to support our use case:

  1. Support for erb
  2. Support for completely customized set of cops.
  3. Parametrized list of folders containing cops to be added to the list.

In addition it will add tooling to expose a rack endpoint for rubocop
invocation:

  1. List of all available cops (kind of metadata)
  2. A POST method that receives a source file, list of cops, and output
    format that will return the result of rubocop’s analysis.
  3. Will be mountable to any Rails application
  4. Will have an option to run as a standalone process (probably using
    passenger with sort-lived process retention settings, since its one
    process
    per request nature)

Why should it be a rack endpoint? My thinking was much more of a normal
ruby API with a command line tool around it. There should be no passenger
dependency to keep its dependencies small. foreman_templates can consume
the ruby API and expose it to the user as it wants.

Usage for foreman needs:

Use case 1 (community templates CI):

  1. Reference the detective gem from templates plugin.
  2. Deploy foreman-core with templates plugin enabled.
  3. Add rake task that will invoke rubocop on specified folder using
    detective’s invocation wrapper.

Ideally we’d have a light command line tool that does:

detective
–cops /path/to/foreman/checkout/cops
–cops /path/to/katello/checkout/cops
–cops /path/to/other/plugin/with/cops
/path/to/some/template/dir
/path/to/another/template/dir

That way we can do a simple git clone foreman in community-templates,
bundle install and run it within Travis. This can indeed be wrapped in a
rake task but given the paths can change on a developers workstation it is
good to have an easy manual option.

Use case 2 (Validate single template from templates UI)

  1. Reference detective gem from templates plugin.
  2. Add cops declaration ability to plugins in foreman core
  3. Templates plugin is responsible for adding/maintaining detective’s
    endpoint.
  4. Foreman core exposes an option to add actions to template editing
    screen.
  5. Templates plugin uses extension point from 4 to add its own action that
    will invoke detective’s endpoint and modify template editor to show the
    result as linting (it’s possible with ace and monaco).

Use case 3 (upgrade scenario):
As a first step, we can try and report broken templates after the upgrade.
It will be pretty similar to community templates CI use case, only the
templates code will be exported from user’s database.

I want to start working on the engine gem as soon as possible, so I would
really appreciate any inputs on the process before I have started with
this
implementation.

Shim.

On Wednesday, August 30, 2017 at 11:48:09 AM UTC+3, ssh...@redhat.com >> wrote:

After a great talk on community demo
https://www.youtube.com/watch?v=aOqA8-wpPKQ, here is a follow up with
the points that were raised during the discussion:

Use cases:

  1. Run all cops as part of community templates CI against the whole
    repository
  2. Run all cops against a single template invoked by the user from
    template editing screen (foreman core)
  3. Upgrade scenario: Preferably run cops for the next foreman version
    before the actual upgrade to make sure the templates will remain
    valid.

Features:

  1. List of rues should be pluggable [Shim]: It looks like it is a
    must-have for the engine.
  2. Deployment options
  3. Engine as a separate gem, cops in a relevant repository - core cops
    in core, plugin cops in plugins.
    2. Engine with all cops in a single gem, versioned per foreman
    version.
    3. Engine as part of templates plugin, cops as part of relevant
    plugins.
    4. Separate gems for everything: foreman-cops-engine,
    foreman-cops-core, foreman-cops-plugin1, foreman-cops-plugin2
    e.t.c. Engine
    is versioned per foreman release version (for the sake of rubocop
    version),
    cops are versioned per plugin version.

General comments:

  1. Cops writing should be enforced on PR’s that are changing the way
    to write templates [mhulan]
  2. Cops are dependent on core/plugin version [gwmngilfen]

On Monday, August 14, 2017 at 2:29:02 PM UTC+3, ssh...@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
<https://github.com/theforeman/foreman/commit/7b966530c9ba48
b2a37416465a3c9619f7143387>
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+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


You received this message because you are subscribed to a topic in the
Google Groups “foreman-dev” group.
To unsubscribe from this topic, visit https://groups.google.com/d/to
pic/foreman-dev/KtVSqPKzXjA/unsubscribe.
To unsubscribe from this group and all its topics, send an email to
foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Yes, I'm have mainly the DOS (I guess it might not even needed
distributed part here) in mind.

– Ivan

··· On Thu, Sep 14, 2017 at 12:18 PM, Shimon Shtein wrote: > > First, I don't think that this service has any secret information that > should be kept under authorization/authentication. > > Even if we assume that some level of auth is indeed needed, the easiest way > to implement it would be through Rack middleware. > If the service is added to an application with existing auth middleware, > then you can add it to the relevant endpoints when mounting it. > In case we are trying to use the standalone version, we can either create an > extension point to add middleware, or mount it manually, just like in the > previous case. > > Anyway I don't see a reason to limit access to those endpoints, except only > for cases of DDOS (which should be handled even before the Rack stack > anyway). > > On Thu, Sep 14, 2017 at 10:44 AM, Ivan Necas wrote: >> >> How would be the authentication/authorization work with this approach? >> >> - - Ivan >> >> On Wed, Sep 13, 2017 at 9:05 PM, wrote: >> > >> > First attempt to create a design. It's an open discussion, everyone who >> > wants to chime in, please do. >> > >> > The engine: will be deployed as a separate gem. My name suggestion >> > the-detective (Sinatra plays a cop). >> > >> > It will wrap the invocation of rubocop with defaults and parameters >> > needed >> > to support our use case: >> > 1. Support for erb >> > 2. Support for completely customized set of cops. >> > 3. Parametrized list of folders containing cops to be added to the list. >> > >> > In addition it will add tooling to expose a rack endpoint for rubocop >> > invocation: >> > 1. List of all available cops (kind of metadata) >> > 2. A POST method that receives a source file, list of cops, and output >> > format that will return the result of rubocop's analysis. >> > 3. Will be mountable to any Rails application >> > 4. Will have an option to run as a standalone process (probably using >> > passenger with sort-lived process retention settings, since its one >> > process >> > per request nature) >> > >> > Usage for foreman needs: >> > >> > Use case 1 (community templates CI): >> > 1. Reference the detective gem from templates plugin. >> > 2. Deploy foreman-core with templates plugin enabled. >> > 3. Add rake task that will invoke rubocop on specified folder using >> > detective's invocation wrapper. >> > >> > Use case 2 (Validate single template from templates UI) >> > 1. Reference detective gem from templates plugin. >> > 2. Add cops declaration ability to plugins in foreman core >> > 3. Templates plugin is responsible for adding/maintaining detective's >> > endpoint. >> > 4. Foreman core exposes an option to add actions to template editing >> > screen. >> > 5. Templates plugin uses extension point from 4 to add its own action >> > that >> > will invoke detective's endpoint and modify template editor to show the >> > result as linting (it's possible with ace and monaco). >> > >> > Use case 3 (upgrade scenario): >> > As a first step, we can try and report broken templates after the >> > upgrade. >> > It will be pretty similar to community templates CI use case, only the >> > templates code will be exported from user's database. >> > >> > >> > I want to start working on the engine gem as soon as possible, so I >> > would >> > really appreciate any inputs on the process before I have started with >> > this >> > implementation. >> > >> > Shim. >> > >> > >> > >> > On Wednesday, August 30, 2017 at 11:48:09 AM UTC+3, ssh...@redhat.com >> > wrote: >> >> >> >> >> >> After a great talk on community demo, here is a follow up with the >> >> points >> >> that were raised during the discussion: >> >> >> >> Use cases: >> >> >> >> Run all cops as part of community templates CI against the whole >> >> repository >> >> Run all cops against a single template invoked by the user from >> >> template >> >> editing screen (foreman core) >> >> Upgrade scenario: Preferably run cops for the next foreman version >> >> before >> >> the actual upgrade to make sure the templates will remain valid. >> >> >> >> >> >> Features: >> >> >> >> List of rues should be pluggable [Shim]: It looks like it is a >> >> must-have >> >> for the engine. >> >> Deployment options >> >> >> >> Engine as a separate gem, cops in a relevant repository - core cops in >> >> core, plugin cops in plugins. >> >> Engine with all cops in a single gem, versioned per foreman version. >> >> Engine as part of templates plugin, cops as part of relevant plugins. >> >> Separate gems for everything: foreman-cops-engine, foreman-cops-core, >> >> foreman-cops-plugin1, foreman-cops-plugin2 e.t.c. Engine is versioned >> >> per >> >> foreman release version (for the sake of rubocop version), cops are >> >> versioned per plugin version. >> >> >> >> General comments: >> >> >> >> Cops writing should be enforced on PR's that are changing the way to >> >> write >> >> templates [mhulan] >> >> Cops are dependent on core/plugin version [gwmngilfen] >> >> >> >> >> >> >> >> >> >> On Monday, August 14, 2017 at 2:29:02 PM UTC+3, ssh...@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: >> >>> >> >>> 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. >> >>> Run rubocop with custom rules and erb monkey patch and produce a json >> >>> report >> >>> >> >>> 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. >> >>> foreman_erb_monkey_patch.rb file: Patches rubocop engine to treat >> >>> *.erb >> >>> files as source files and not skip them. >> >>> >> >>> 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+unsubscribe@googlegroups.com. >> > For more options, visit https://groups.google.com/d/optout. >> >> -- >> You received this message because you are subscribed to a topic in the >> Google Groups "foreman-dev" group. >> To unsubscribe from this topic, visit >> https://groups.google.com/d/topic/foreman-dev/KtVSqPKzXjA/unsubscribe. >> To unsubscribe from this group and all its topics, send an email to >> foreman-dev+unsubscribe@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+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.

Finally I have finished first stage of engine development - now there is a
sinatra app that could be added to any rack app (although not tested) and
can run as a standalone executable (for now using webrick).

Git repo is located here: https://github.com/ShimShtein/detective

Key files:

  • lib/detective/web/endpoint.rb: Sinatra app that exposes the resources.
  • lib/detective/launcher.rb: Rubocop's CLI wrapper to make use of custom
    cops.
  • bin/detective: Standalone mode executable.

I would really appreciate if you evaluate this code and see if it is
something we want to work with.

One caveat though: I still didn't add the code responsible for erb
handling, it will be added later.

Shim.

··· On Thursday, September 14, 2017 at 4:27:15 PM UTC+3, Shimon Shtein wrote: > > Ewoud: > > About rack endpoint: It should be created for use case 2, where the check > is running in "online"-ish mode. > I prefer creating it as a microservice for scalability reasons, so I > wouldn't want to tie it too tightly with foreman-templates. Besides that, > tying it into foreman-templates will mean the same life cycle that I would > prefer to avoid. > As I already wrote for case 1 and 3, we could use the underlying ruby > wrapper directly, hence avoid the usage of the API. > As for additional dependencies, I probably would add passenger as an > optional (development) dependency, so you will have the full feature set on > development machines. In production it will check if if passenger is > available only if it runs in standalone mode. It will not ask for passenger > if someone has decided to use something else, like webrick (personally I > would recommend against webrick, it's not parallel at all which is > important here). > > About the command line tool, I have nothing against it, it should be > available in the gem. > > > > > > On Thu, Sep 14, 2017 at 2:06 PM, Ewoud Kohl van Wijngaarden < > ewoud@kohlvanwijngaarden.nl> wrote: > >> On Wed, Sep 13, 2017 at 12:05:49PM -0700, sshtein@redhat.com wrote: >> >>> >>> First attempt to create a design. It's an open discussion, everyone who >>> wants to chime in, please do. >>> >>> The engine: will be deployed as a separate gem. My name suggestion >>> the-detective >>> (Sinatra >>> plays a cop). >>> >>> It will wrap the invocation of rubocop with defaults and parameters >>> needed >>> to support our use case: >>> 1. Support for erb >>> 2. Support for completely customized set of cops. >>> 3. Parametrized list of folders containing cops to be added to the list. >>> >>> In addition it will add tooling to expose a rack endpoint for rubocop >>> invocation: >>> 1. List of all available cops (kind of metadata) >>> 2. A POST method that receives a source file, list of cops, and output >>> format that will return the result of rubocop's analysis. >>> 3. Will be mountable to any Rails application >>> 4. Will have an option to run as a standalone process (probably using >>> passenger with sort-lived process retention settings, since its one >>> process >>> per request nature) >>> >> >> Why should it be a rack endpoint? My thinking was much more of a normal >> ruby API with a command line tool around it. There should be no passenger >> dependency to keep its dependencies small. foreman_templates can consume >> the ruby API and expose it to the user as it wants. >> >> Usage for foreman needs: >>> >>> Use case 1 (community templates CI): >>> 1. Reference the detective gem from templates plugin. >>> 2. Deploy foreman-core with templates plugin enabled. >>> 3. Add rake task that will invoke rubocop on specified folder using >>> detective's invocation wrapper. >>> >> >> Ideally we'd have a light command line tool that does: >> >> detective \ >> --cops /path/to/foreman/checkout/cops \ >> --cops /path/to/katello/checkout/cops \ >> --cops /path/to/other/plugin/with/cops \ >> /path/to/some/template/dir \ >> /path/to/another/template/dir >> >> That way we can do a simple git clone foreman in community-templates, >> bundle install and run it within Travis. This can indeed be wrapped in a >> rake task but given the paths can change on a developers workstation it is >> good to have an easy manual option. >> >> Use case 2 (Validate single template from templates UI) >>> 1. Reference detective gem from templates plugin. >>> 2. Add cops declaration ability to plugins in foreman core >>> 3. Templates plugin is responsible for adding/maintaining detective's >>> endpoint. >>> 4. Foreman core exposes an option to add actions to template editing >>> screen. >>> 5. Templates plugin uses extension point from 4 to add its own action >>> that >>> will invoke detective's endpoint and modify template editor to show the >>> result as linting (it's possible with ace and monaco). >>> >>> Use case 3 (upgrade scenario): >>> As a first step, we can try and report broken templates after the >>> upgrade. >>> It will be pretty similar to community templates CI use case, only the >>> templates code will be exported from user's database. >>> >>> >>> I want to start working on the engine gem as soon as possible, so I would >>> really appreciate any inputs on the process before I have started with >>> this >>> implementation. >>> >>> Shim. >>> >>> >>> >>> On Wednesday, August 30, 2017 at 11:48:09 AM UTC+3, ssh...@redhat.com >>> wrote: >>> >>>> >>>> >>>> After a great talk on community demo >>>> , here is a follow up with >>>> the points that were raised during the discussion: >>>> >>>> Use cases: >>>> >>>> 1. Run all cops as part of community templates CI against the whole >>>> repository >>>> 2. Run all cops against a single template invoked by the user from >>>> template editing screen (foreman core) >>>> 3. Upgrade scenario: Preferably run cops for the next foreman version >>>> before the actual upgrade to make sure the templates will remain >>>> valid. >>>> >>>> >>>> Features: >>>> >>>> 1. List of rues should be pluggable [Shim]: It looks like it is a >>>> must-have for the engine. >>>> 2. Deployment options >>>> 1. Engine as a separate gem, cops in a relevant repository - core >>>> cops >>>> in core, plugin cops in plugins. >>>> 2. Engine with all cops in a single gem, versioned per foreman >>>> version. >>>> 3. Engine as part of templates plugin, cops as part of relevant >>>> plugins. >>>> 4. Separate gems for everything: foreman-cops-engine, >>>> foreman-cops-core, foreman-cops-plugin1, foreman-cops-plugin2 >>>> e.t.c. Engine >>>> is versioned per foreman release version (for the sake of rubocop >>>> version), >>>> cops are versioned per plugin version. >>>> >>>> General comments: >>>> >>>> 1. Cops writing should be enforced on PR's that are changing the way >>>> to write templates [mhulan] >>>> 2. Cops are dependent on core/plugin version [gwmngilfen] >>>> >>>> >>>> >>>> >>>> On Monday, August 14, 2017 at 2:29:02 PM UTC+3, ssh...@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 >>>>> < >>>>> https://github.com/theforeman/foreman/commit/7b966530c9ba48b2a37416465a3c9619f7143387 >>>>> > >>>>> 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+unsubscribe@googlegroups.com. >>> For more options, visit https://groups.google.com/d/optout. >>> >> >> -- >> You received this message because you are subscribed to a topic in the >> Google Groups "foreman-dev" group. >> To unsubscribe from this topic, visit >> https://groups.google.com/d/topic/foreman-dev/KtVSqPKzXjA/unsubscribe. >> To unsubscribe from this group and all its topics, send an email to >> foreman-dev+unsubscribe@googlegroups.com. >> For more options, visit https://groups.google.com/d/optout. >> > >