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

[Shimon Shtein]

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

> On Wed, Sep 13, 2017 at 12:05:49PM -0700, ssht...@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 w

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

[Shimon Shtein]

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 ev

Re: [foreman-dev] [RFC] Proposal: make foreman STI to work even when an inherited class is not found in Ruby.

[Shimon Shtein]

OK, it wasn't exactly my intention, but I see the discussion moves towards
general plugin install/uninstall procedure.

I have discovered a nice feature that enables us to isolate plugin
migrations, and run them on per-plugin manner:
Apparently we can run:
rake db:migrate SCOPE=my_plugin
rake db:migrate SCOPE=my_plugin VARSION=0 # to migrate down
And it will run only migrations that are tagged for that scope. One can
achieve that by generating proper migration names:
_..rb
I have added a PR that generates migrations for plugins with the proper
naming: https://github.com/theforeman/foreman/pull/4509.

This should be the base for complete removal. Then, if we add a down-only
migration that removes STI's - it should be enough to remove all DB traces
for a plugin.



On Wed, May 3, 2017 at 2:11 PM, Lukas Zapletal  wrote:

> I agree with Tomas, it's more cleaner to remove all the data right
> away. Therefore I suggest that we check for these kind of objects
> during initialization and if such an class is (not) found, then we can
> throw an error like "Run foreman-rake plugin:clean" to delete orhpaned
> records.
>
> I am for (A) - remove all the data.
>
> LZ
>
> On Wed, May 3, 2017 at 11:16 AM, Tomas Strachota 
> wrote:
> > This issue is tightly coupled with plugin uninstallation and I think
> > we should solve the two problems together. At the moment there's no
> > standard plugin uninstallation procedure that would take care of
> > cleaning up the system. This is something each plugin should provide.
> > One thing (imho the less important in this context) is where we put
> > such code. Should it be a rake task, part of installer, part of
> > maintanance script, somewhere else?
> >
> > What I see as more important is to decide what data will the
> > uninstallation process remove (keep all vs. keep only settings vs.
> > wipe all) and how we want the plugin to behave if it's installed
> > again. This will affect how we approach missing STI classes.
> >
> > I can see 3 options:
> >
> > a) Remove all data
> > Plugin would remove all tables and records it created. In such case we
> > probably don't need to change the STI behavior. Plugin uninstallation
> > should take care of removing the data correctly. If it fails it's fine
> > to throw exception to indicate the system integrity is broken. This
> > approach is imho safer for us as developers and requires less
> > defensive coding.
> >
> > b) Keep all the data
> > In this case the original data should be present when the plugin is
> > installed again. STI records without classes should be completely
> > hidden in the default scope. If all records are listed we should
> > return either instances of base class or some special class for the
> > missing types. I'm afraid this approach is quite fragile and can lead
> > to many surprises when a plugin is uninstalled, the foreman lives
> > without it for some time and then you install it again. I'm also not
> > sure how common is such usecase.
> >
> > c) Combination of a) and b)
> > We can keep data where it's safe (like settings) and delete the rest.
> >
> > I'm in favor of a) or c)
> >
> > T.
> >
> >
> >
> >
> >
> > On Sun, Apr 30, 2017 at 10:05 AM,   wrote:
> >>
> >> Hello,
> >>
> >> lately I was switching plugins on and off, and I stumbled upon an
> annoying
> >> issue: Many plugins that add some STI classes (for example Katello that
> adds
> >> settings, or Discovery that adds DiscoveredHost).
> >> A problem starts when such a plugin is removed from the system, since
> >> default STI implementation will throw an error if it can't load a class
> that
> >> corresponds to the :type column in the DB.
> >>
> >> I propose a custom handling for such cases, since they are not exactly
> >> exceptions in our system design.
> >> I was thinking about one of the following options to handle this case,
> and
> >> would like to see a voting which one you prefer. Of course other
> options can
> >> be proposed too.
> >>
> >> 1. Return a base class for such records (Maybe with an error already
> set)
> >> 2. Return nil/some kind of special AR object for such records (will
> require
> >> defensive coding while handling heterogeneous lists)
> >> 3. Filter those records out with default scope (will require more
> >> declaration in plugins, or more DB queries on system startup)
> >>
> >> Any thoughts in this area will be much appreciated,
> >>
> >> Shim
> >>
> >> --
> >> 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 http

Re: [foreman-dev] Rubocop enhancements for foreman plugins - your opinion is needed

[Shimon Shtein]

> It would be on plugin maintainers decision.
What would be a maintainers decision? The fact that the task will run only
on some setups? IMHO It's a bit annoying.

About the automation, my solution basically removes both
foreman_remote_execution:rubocop and the enhancement (And deleting code is
even better than writing one ;) ).
Plus you get a straight forward way of running rubocop as a developer: You
don't have to run a rake task from a parent project, you just run `rubocop`
from your current folder.

IMHO, It's simpler solution for devs...



On Tue, Jan 24, 2017 at 6:37 PM, Ivan Necas  wrote:

> Ivan Necas  writes:
>
> > Shimon Shtein  writes:
> >
> >>> expecing there will be foreman on  `../foreman` and including
> >> `../foreman/.rubocop.yml`in the plugin
> >> I wanted to avoid hard assumptions about folders layout (since the task
> >> simply won't run at all if foreman folder is located elsewhere).
> >
> > It would be on plugin maintainers decision.
> >
> >>
> >>>  using rubocop via running rake from the foreman repo
> >> Doesn't help for build automation - You can't enable automatic builds
> this
> >> way.
> >
> > Why? We already do this in REX
> > https://gitlab.cee.redhat.com/egolov/asciidoctor-ej/blob/
> vodafone-sat6/Vodafone-Satellite6-Capsule.adoc
>
> Sry, the right link is
> https://github.com/theforeman/foreman_remote_execution/blob/
> master/lib/tasks/foreman_remote_execution_tasks.rake#L44
> :)
>
> -- Ivan
>
> >
> >>
> >>> It would not need to require the downloading from outside.
> >> You are still downloading/refreshing foreman's repo. It's the same as
> >> getting the file by URL.
> >
> > It's not: you have the rubocop versioned and you can switch between
> > versions quickly.
> >
> >>
> >>> not needing to do anything with the current approach in the core
> >> If you are that concerned about core changes, we can always duplicate
> the
> >> file - core stays as is, and plugins are getting the file from
> development
> >> gem. It will just require a synchronization with the core once in a
> >> while.
> >
> > Not convinced about benefits of additional layer.
> >
> > -- Ivan
> >
> >>
> >> On Tue, Jan 24, 2017 at 12:08 PM, Ivan Necas  wrote:
> >>
> >>> ssht...@redhat.com writes:
> >>>
> >>> > As we all know, foreman core uses rubocop for enforcing style rules.
> Many
> >>> > plugins, especially those that are in theforeman organization, use
> >>> rubocop
> >>> > too.
> >>> > The problem is, that the rules are not unified between foreman core
> and
> >>> > plugins. In addition to that when rubocop version changes in core,
> core's
> >>> > rules change accordingly, but plugins remain often with the old
> ruleset
> >>> for
> >>> > no reason.
> >>> >
> >>> > I am suggesting inheriting foreman's ruleset as a base ruleset for
> >>> plugins.
> >>> > I have found two ways to accomplish that:
> >>> >
> >>> >1. Rubocop has an option to inherit a remote URL [1]
> >>> ><https://github.com/bbatsov/rubocop/blob/master/manual/
> >>> configuration.md#inheriting-configuration-from-a-remote-url>,
> >>> >so we can point a plugin to github URL for foreman master ruleset
> [2]
> >>> ><https://raw.githubusercontent.com/theforeman/foreman/develop/.
> >>> rubocop.yml>.
> >>> >
> >>> >Advantages: it's simple.
> >>> >Disadvantages: You have to be connected in order to download the
> file
> >>> >(Although rubocop has caching mechanism for it)
> >>> >2. We can utilize another Rubocop config option: inheriting the
> >>> settings
> >>> >from a gem [3]
> >>> ><https://github.com/bbatsov/rubocop/blob/master/manual/
> >>> configuration.md#inheriting-configuration-from-a-dependency-gem>.
> >>> >This will require us to create a special development gem, that
> would
> >>> be
> >>> >used by foreman core and plugins. This gem will contain a proper
> >>> ruleset.
> >>> >I am a bit biased, since I have already started foreman_devel
> plugin
> >>> [4]
> >>> ><https://github.com/ShimShtein/foreman_deve

Re: [foreman-dev] Rubocop enhancements for foreman plugins - your opinion is needed

[Shimon Shtein]

> expecing there will be foreman on  `../foreman` and including
`../foreman/.rubocop.yml`in the plugin
I wanted to avoid hard assumptions about folders layout (since the task
simply won't run at all if foreman folder is located elsewhere).

>  using rubocop via running rake from the foreman repo
Doesn't help for build automation - You can't enable automatic builds this
way.

> It would not need to require the downloading from outside.
You are still downloading/refreshing foreman's repo. It's the same as
getting the file by URL.

> not needing to do anything with the current approach in the core
If you are that concerned about core changes, we can always duplicate the
file - core stays as is, and plugins are getting the file from development
gem. It will just require a synchronization with the core once in a while.

On Tue, Jan 24, 2017 at 12:08 PM, Ivan Necas  wrote:

> ssht...@redhat.com writes:
>
> > As we all know, foreman core uses rubocop for enforcing style rules. Many
> > plugins, especially those that are in theforeman organization, use
> rubocop
> > too.
> > The problem is, that the rules are not unified between foreman core and
> > plugins. In addition to that when rubocop version changes in core, core's
> > rules change accordingly, but plugins remain often with the old ruleset
> for
> > no reason.
> >
> > I am suggesting inheriting foreman's ruleset as a base ruleset for
> plugins.
> > I have found two ways to accomplish that:
> >
> >1. Rubocop has an option to inherit a remote URL [1]
> > configuration.md#inheriting-configuration-from-a-remote-url>,
> >so we can point a plugin to github URL for foreman master ruleset [2]
> > rubocop.yml>.
> >
> >Advantages: it's simple.
> >Disadvantages: You have to be connected in order to download the file
> >(Although rubocop has caching mechanism for it)
> >2. We can utilize another Rubocop config option: inheriting the
> settings
> >from a gem [3]
> > configuration.md#inheriting-configuration-from-a-dependency-gem>.
> >This will require us to create a special development gem, that would
> be
> >used by foreman core and plugins. This gem will contain a proper
> ruleset.
> >I am a bit biased, since I have already started foreman_devel plugin
> [4]
> > that should help plugin
> >developers in multiple tasks.
> >Advantages: It's a plugin that can do a lot more than just ruleset
> repo.
> >It's versioned properly. It's offline, once you have the gem
> installed.
> >Disadvantages: Extra gem. Installation is more complicated. Affects
> the
> >core too
> >
> > I would like to hear your opinions about the issue. Both whether you like
> > the basic idea of inheriting the same ruleset and if so, which is the
> > preferred way to go.
>
> Another approach could be, in the .rubocop.yml of the plugin, expecing
> there will be foreman on  `../foreman` and including
> `../foreman/.rubocop.yml`in the plugin
> and using rubocop via running rake from the foreman repo (we already
> have `rake foreman_remote_execution:rubocop` for example). I guess this
> assumption of the layout of plugin development is reasonable to make (it
> would work also with the CI).
>
> The benefits would be not needing to do anything with the current
> approach in the core (that I guess works for the core), while the
> plugins using it as base. It would not need to require the downloading
> from outside. Also it would take into account the versioning and we
> would not have issues with running rubocop against foreman/plugin stable
> branches, that I guess should stay with the configuration there was at
> the time of branching.
>
> -- Ivan
>
> >
> > Shim,
> >
> >
> >
> > [1]
> > https://github.com/bbatsov/rubocop/blob/master/manual/
> configuration.md#inheriting-configuration-from-a-remote-url
> > [2]
> > https://raw.githubusercontent.com/theforeman/foreman/
> develop/.rubocop.yml
> > [3]
> > https://github.com/bbatsov/rubocop/blob/master/manual/
> configuration.md#inheriting-configuration-from-a-dependency-gem
> > [4] https://github.com/ShimShtein/foreman_devel
> >
> > --
> > 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.