include all pulpcore plugins conditionally

[evgeni]

No description provided.

[evgeni]

wasn't exactly sure about this one

[evgeni]

and should certguard only be loaded if deb or rpm are enabled?

[ekohl]

and should certguard only be loaded if deb or rpm are enabled?

If deb / rpm require certguard, the actual classes could require them, right? So https://github.com/theforeman/puppet-pulpcore/blob/master/manifests/plugin/rpm.pp. I'm not sure if it should be done unconditionally though. Not too familiar with the actual internals.

[evgeni]

Well, they don't require it. Without you still can do unprotected repos, but when you want sub-man "protection", you need certguard.

Maybe @jlsherrill has an opinion?

[ekohl]

Oh yes, I recall that conversation. With Puppet you can repeat include statements, so it's fine to have:

if $rpm {
  include pulpcore::plugin::rpm
  include pulpcore::plugin::certguard
}

if $deb {
  include pulpcore::plugin::deb
  include pulpcore::plugin::certguard
}

[evgeni]

But that only works as long as you do include and not pass params to class, so IMHO the most future proof way is

if ($rpm || $deb) {
  include …
}

[jlsherrill]

Generally makes sense to me. I think for now we should likely always deploy certguard. For things like smart proxy syncing, we always just try to create one if it doesn't exist. we could enhance this to only create it if there's at least one yum or deb repo to be synced, but today thats not the case

[ehelms]

Based on @jlsherrill comment, I think this can go in as is.

[ekohl]

@jlsherrill is there a reason to have certguard if you don't have RPMs nor debs?

[jlsherrill]

@ekohl the reason is that katello today expects rhsm certguard to be present when 'syncing' a smart proxy. It tries to create one via the api. We can enhance katello to be smarter about this (feel free to file an issue), but its not there today.

Aside from today's behavior, no, there is no reason

[ekohl]

That is useful information to document somewhere. Either in this module's README, in init.pp as a description of what the module does or inline in the code. Maybe there's even a better place that I haven't thought of.

[ehelms]

To move this along and save some time, I opened #312 so at least when looking at the code its clear. I think if that works, then we can pull these in and I can update my big Pulp 3 PR.