Re: [foreman-dev] Revert removal of @host.params for host_param

[Marek Hulán]

Hello

The discussion has diverged into something different. People agree we should 
add an extra layer but implemented through proxy objects. It's also clear no 
one starts actively working on this right now. At the same time I want to add 
new macro, let's say rpm_distro? which returns true/false based on 
@host.operatingsystem, how do I do it?

So far the answer was to add a new macro. Now with the proxy object, should I 
define it in host so one would use @host.rpm_distro?. Honestly I don't want to 
add more stuff into Host object, not even through concerns. So should I wait 
until we have a proxy layer? That means I can't add any improvements. For me 
the best answer is add new macro called "rpm_distro?". This will result in two 
approaches in future, we'll have both @host.param and rpm_distro?

I think the best approach is just reverting the deprecation warning and add a 
value to host_param macro. It can check that user is asking for a parameter 
that does not exist and raise a exception of a specific class so rendering 
could display nicer error message such as "You tried to use parameter with 
name abc but it's not defined for host xyz" instead of "Undefined method 
upcase on nil". We could extend Host#param for this too but it's not right, 
this exception is specific for template rendering, other internal usage of 
this method might rely on fact it returns nil.

My probably last comment in this thread - the PR was opened Nov 1st 2016, 
deprecation was suggested Nov 2nd, merged Jan 3rd 2017 [1]. I know a lot of 
devs were offline during December but still, please try to raise you concerns 
in PRs rather than revert stuff.

[1] https://github.com/theforeman/foreman/pull/3983

--
Marek

On pondělí 16. ledna 2017 14:59:43 CET Tomer Brisker wrote:
> Hi,
> I think that while there are benefits to moving away from the host object,
> we have a de-facto API based on it.
> A way to change without breaking user's template would be to use a "proxy"
> object that maintains the same endpoints and allows adding functionality or
> handling changes in the underlying objects without breaking the way users
> use them.
> So the templates will still have a "@host" object with the same methods as
> before, except it will in fact be proxy object and not an active record.
> For example, we could have a nice error message if the actual host is nil.
> We could also leverage method_missing to easily handle passing anything not
> defined in the api to the host (possibly only if safe mode is off), so that
> any users relying on active record methods etc. won't have breakage.
> 
> Tomer
> 
> On Mon, Jan 16, 2017 at 2:49 PM, Stephen Benjamin 
> 
> wrote:
> > On Mon, Jan 16, 2017 at 3:24 AM, Ondrej Prazak  wrote:
> > > +1 for keeping the macros. IMHO, just because we did something a certain
> > 
> > way for a long time should not prevent us from changing it if there are
> > reasons for a change. This is also not the first change in core that
> > affected plugin(s) in a negative way and I doubt it will be the last.
> > Breaking plugin tests is unpleasant, but it is still a second best
> > scenario.
> > 
> > 
> > The plugin test failure is relatively minor, the main objection here
> > is the number of users who rely on this interface now need to change.
> > After raising this issue we got some PR's that help the migrations
> > which is nice, but there's
> > organizations who have less sophisticated technical users who learned
> > basic ERB who have to re-learn this, not to mention that git repos of
> > users using foreman_templates now have to update, and we now have lots
> > of incorrect info out there on the internet and internal intranets
> > that need to be updated.  We're making our users do a lot of work for
> > not a lot of good.
> > 
> > The change was not needed, and it was merged while a significant
> > number of developers were away for the holidays.  I think it should be
> > changed to restore the @host interface, or reverted. I know there are
> > some benefits to stop using the Host::Managed object, but we could've
> > implemented @host as an instance of some proxy class without breakage
> > for users.
> > 
> > FWIW, this change, if we actually followed semver, should require a
> > bump to 2.0 once the deprecated methods are removed.
> > 
> > - Stephen
> > 
> > --
> > You received this message because you are subscribed to the Google Groups
> > "foreman-dev" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to foreman-dev+unsubscr...@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.


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

[foreman-dev] Foreman plugin accessing katello host data like asigned content view

[Daniel Kuffner]

Hi All,

I'm generally interested if it is possible to access from within a 
foreman_plugin data from Katello plugin (plugin accesses data from other 
plugin). 
For example I would like to extract the content-view/asigned repository 
information for a given host.

regards,
Daniel

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

[foreman-dev] Opinions from plugin maintainers wanted: permissions and roles

[oprazak]

Hi,
I recently started identifying problematic areas in Permissions and Roles, 
especially with regard to plugins. Foreman provides 'Viewer' and 'Manager' 
roles out of the box and users expect these roles to work for plugins as 
well. But plugins generally do not add their permissions to core's roles, 
some of them just have their own plugin-specific roles, some not. So if a 
plugin is installed, user has to go and find what role/permission is 
missing or ask someone who can grant permissions.

After a brief discussion in team C, it was decided that plugins should add 
their permissions to 'Manager' role and their 'view_' permissions to 
'Viewer' role. They should also keep their plugin-specific roles (or create 
them) for higher granularity and backward compatibility.
I started initial work which should allow plugin maintainers to do this, it 
can be found at [1]. I decided to create 2 methods that would be called 
from Engine, but we could take a different approach and add permissions 
from plugins automatically by modifying the 'permission' method that is 
called from 'security_block'. 

The way I see things:

* adding permissions explicitly, using DSL
  - extra work for plugin maintainers
  - permissions can be ommited/forgotten
 # can be covered with tests? something like the already existing 
AccessPermissionsTest 
  - extra code in Engine
  - better control, can handle special cases

* adding permissions automatically without plugin knowing about it by 
modifying 'permission' method
  - no need for plugin maintainers to do anything
  - cannot handle special cases
  - if user removes permission from these default roles, there is no way to 
detect it and permissions get added again on server restart
# users do not modify default roles, they can clone it and modify the 
cloned one
# if the default roles are not meant to be modified, they should be 
'frozen' and user should not be able to modify them, implementing this 
would add code and complexity

Anything that I have missed? Where do you stand as a plugin maintainers? 
Can you think of other ways to go?

Ondra

[1] https://github.com/theforeman/foreman/pull/4168  

-- 
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.

Re: Re: [foreman-dev] Revert removal of @host.params for host_param

[Tomer Brisker]

Hi,
I think that while there are benefits to moving away from the host object,
we have a de-facto API based on it.
A way to change without breaking user's template would be to use a "proxy"
object that maintains the same endpoints and allows adding functionality or
handling changes in the underlying objects without breaking the way users
use them.
So the templates will still have a "@host" object with the same methods as
before, except it will in fact be proxy object and not an active record.
For example, we could have a nice error message if the actual host is nil.
We could also leverage method_missing to easily handle passing anything not
defined in the api to the host (possibly only if safe mode is off), so that
any users relying on active record methods etc. won't have breakage.

Tomer

On Mon, Jan 16, 2017 at 2:49 PM, Stephen Benjamin 
wrote:

> On Mon, Jan 16, 2017 at 3:24 AM, Ondrej Prazak  wrote:
> >
> > +1 for keeping the macros. IMHO, just because we did something a certain
> way for a long time should not prevent us from changing it if there are
> reasons for a change. This is also not the first change in core that
> affected plugin(s) in a negative way and I doubt it will be the last.
> Breaking plugin tests is unpleasant, but it is still a second best scenario.
>
>
> The plugin test failure is relatively minor, the main objection here
> is the number of users who rely on this interface now need to change.
> After raising this issue we got some PR's that help the migrations
> which is nice, but there's
> organizations who have less sophisticated technical users who learned
> basic ERB who have to re-learn this, not to mention that git repos of
> users using foreman_templates now have to update, and we now have lots
> of incorrect info out there on the internet and internal intranets
> that need to be updated.  We're making our users do a lot of work for
> not a lot of good.
>
> The change was not needed, and it was merged while a significant
> number of developers were away for the holidays.  I think it should be
> changed to restore the @host interface, or reverted. I know there are
> some benefits to stop using the Host::Managed object, but we could've
> implemented @host as an instance of some proxy class without breakage
> for users.
>
> FWIW, this change, if we actually followed semver, should require a
> bump to 2.0 once the deprecated methods are removed.
>
> - Stephen
>
> --
> 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.
>



-- 
Have a nice day,
Tomer Brisker
Red Hat Engineering

-- 
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.

Re: [foreman-dev] .ISO deploys with Discovery

[Lukas Zapletal]

Thanks Greg, that's what I want to work on this week :-)

More on this topic, we already have integration of Foreman Bootdisk
with VMWare. You create new VM, that gets Bootdisk attached as an ISO
file automatically and boots into Anaconda directly without PXE or
DHCP. This might be interesting for you as well, I think it does the
job better than discovery if you want to go through Anaconda.

In any way, we'd need to extend our Compute Resources with possibility
to attach ISOs, so far only Discovery can do that AFAIK. That's the
biggest limitation we have at the moment.

LZ

On Fri, Jan 13, 2017 at 1:31 PM, Greg Sutcliffe
 wrote:
> +1 from me - image deployment for hardware makes sense IMO. It's definitely
> something others have been thinking about - I'll leave Lukas to go into more
> detail, but here's something you could go take a look at :)
>
> https://github.com/theforeman/foreman-discovery-image/pull/44
>
> Greg
>
>
> --
> 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.



-- 
Later,
  Lukas @lzap Zapletal

-- 
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.

Re: [foreman-dev] HoundCI - annoying?

[Lukas Zapletal]

I feel actually quite the opposite, we used to have Hound via some
external service and this feels much better integrated. Since github
introduced reviews, Hound integration could take advantage of it in
the future? They will implode once a review is confirmed, that could
help.

On the other hand, we should definitely not insist on all Hound checks
what "community think is a good practice". We have discussed that,
there were flamewars on this topic here already :-) And we did great
job in trying to find the balance. If you have suggestions, just make
a PR with the checkstyle changes.

LZ

On Wed, Jan 11, 2017 at 5:18 PM, Timo Goebel  wrote:
> Hi devs,
>
> I'm usually not very easily annoyed. What get's me started though eventually
> is when things don't work properly.
> HoundCI is one of those things.
>
> My main concern is, that I get an e-mail and/or Github notification for
> every single comment. These can easily be ten or more e-mails. They're not
> grouped as other reviews.
> In addition, the inline comments are very distracting when reviewing a PR
> imho. When the issues are fixed, I'd prefer for the comments to be removed.
> When reviewing a PR, I usually don't care about the style issues. I just
> want to see if there are some problems or if all is fine.
>
> Back in the days, code style was checked by Jenkins. I think, it did a far
> better job in displaying style issues. With the current Jenkins Github
> plugin it believe would be easily possible to show style issues as a
> separate line along with all the other CI checks.
>
> One argument in favor of HoundCI is, that it checks JavaScript style. But I
> think, that can easily be set up in Jenkins as well by running eslint.
>
> Any comments? How do others feel?
>
> Timo
>
> --
> 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.



-- 
Later,
  Lukas @lzap Zapletal

-- 
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.

Re: [foreman-dev] Revert removal of @host.params for host_param

[Lukas Zapletal]

I am also for keeping the change but giving more attention to the
upgrades and providing some tooling to convert existing templates.

LZ

On Wed, Jan 11, 2017 at 5:05 PM, Daniel Lobato Garcia
 wrote:
> Hi foreman devs,
>
> Just noticed today https://github.com/theforeman/foreman/pull/3983/files
> after some comments on IRC. What's the background behind this change?
>
> As far as I can see, this merely moves
>
> @host.param to host_param
> @host.param_true? to host_param_true?
> @host.param_false? to host_param_false?
> @host.info to host_enc
>
> without gaining anything from the change. This will force people to
> change their templates (including our community templates) when the
> deprecation is removed, and there's nothing to gain.
>
> Does someone know what's the rationale behind this change? As it stands
> right now, I propose reverting that commit entirely to avoid inflicting
> that pain on users - which include many devs who maintain templates.
>
> Best,
>
> --
> Daniel Lobato Garcia
>
> @dLobatog
> blog.daniellobato.me
> daniellobato.me
>
> GPG: http://keys.gnupg.net/pks/lookup?op=get=0x7A92D6DD38D6DE30
> Keybase: https://keybase.io/elobato
>
> --
> 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.



-- 
Later,
  Lukas @lzap Zapletal

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

[foreman-dev] Puppetdb Plugin Options Update .

[Karen Harutyunyan]

Hi All ,

PuppetDB plugin has new version which bring a lot of interesting parameters 
to get puppetdb.4.3 working properly . like ssl-ca and etc. Please can 
someone send me docs how to add (or create pull request) to this params. If 
you have time enough to do that it`s also perfect for me .

Sharing new features below which I`ll prefer to see in next release of 
foreman.

Usage: 

Go to Administer > Settings > PuppetDB and set puppetdb_address with your 
PuppetDB address, puppetdb_dashboard_address (optional) to proxy the 
dashboard, puppetdb_enabled to either true or false if you want to enable 
or disable PuppetDB integration. Obviously you will need a PuppetDB 
instance at the address you provide.

Alternatively you can put your settings in config/settings.yaml. Please 
keep in mind these will be overwritten if changed in the application, and 
if the application is rebooted, YAML rules will overwrite again your 
manually changed settings. Hence passing your settings in this format is 
discouraged, but allowed.

for PuppetDB 4

:puppetdb:
  :enabled: true
  :address: 'https://puppetdb:8081/pdb/cmd/v1'
  :dashboard_address: 'http://puppetdb:8080/pdb/dashboard'


*  :puppetdb_ssl_ca_file: '/etc/puppetlabs/puppet/ssl/certs/ca.pem'
  :puppetdb_ssl_certificate: '/etc/puppetlabs/puppet/ssl/certs/FQDN.pem'
  :puppetdb_ssl_private_key: '/etc/puppetlabs/puppet/ssl/private_keys/FQDN.pem'*

Regards
Karen

-- 
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.

Re: Re: [foreman-dev] Revert removal of @host.params for host_param

[Ondrej Prazak]

+1 for keeping the macros. IMHO, just because we did something a certain
way for a long time should not prevent us from changing it if there are
reasons for a change. This is also not the first change in core that
affected plugin(s) in a negative way and I doubt it will be the last.
Breaking plugin tests is unpleasant, but it is still a second best scenario.

-- 
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.

Re: [foreman-dev] Community template repository labels

[Marek Hulán]

Hello,

labels might help so +1 for that. I plan to send a PR with new directory 
structure as discussed at [1] soon. It might help the bot to just check the 
directories of touched files.

[1] https://groups.google.com/forum/#!topic/foreman-dev/bzfBWEtIMpg

--
Marek

On pátek 13. ledna 2017 10:19:02 CET Eric D Helms wrote:
> If this will help with getting reviews or directing reviews then sounds all
> good. The bot could handle this afaik as long as they are predictable which
> is which.
> 
> On Jan 12, 2017 6:26 AM, "Ewoud Kohl van Wijngaarden" <
> 
> ew...@kohlvanwijngaarden.nl> wrote:
> > Hello all,
> > 
> > Since there are multiple types of templates in community templates, I
> > wonder if it's time to (automatically) attach labels to PRs. I was
> > thinking:
> > 
> > * Remote exection
> > * Provisioning
> > 
> > Maybe we need to split up Provisioning into a label per distro.
> > 
> > Any thoughts on this? Can the foreman bot be easily modified to do this
> > for us?
> > 
> > --
> > 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.