Tails

#1 Updated by intrigeri 2018-01-08 17:37:23

• blocks Feature #13284: Core work: Sysadmin (Adapt our infrastructure) added

#2 Updated by intrigeri 2018-01-08 17:38:29

• Parent task set to Feature #15154

#5 Updated by bertagaz 2018-01-30 10:26:00

• Assignee changed from bertagaz to intrigeri
• QA Check changed from Info Needed to Dev Needed

intrigeri wrote:
> Hi bertagaz! As said elsewhere I want to complete this by the end of 2018Q1. Three weeks have passed since I’ve asked the question above and only 2 months are left for me to do the work. So I would greatly appreciate a prompt reply so I can do my work in a non-rushed, less stressful manner. Thanks in advance!

Right, sorry forgot this.

> Could you please take a look and tell me what upstream I should upgrade to?

I’v used https://github.com/Icinga/puppet-icinga2.git, from the develop branch.

But as said once when we discussed about this upstream module, its development is quite chaotic, so it may be difficult to update it. It also depends on full featured exported resources (to support hash parameters), so we’re only using a tiny part of it. SO depending on the energy you want to put in there, we may also just stick to the version we have now.

#6 Updated by intrigeri 2018-01-30 11:08:52

• Assignee changed from intrigeri to bertagaz
• QA Check changed from Dev Needed to Info Needed

>> Could you please take a look and tell me what upstream I should upgrade to?

> I’v used https://github.com/Icinga/puppet-icinga2.git, from the develop branch.

That’s what I thought but it’s good to have an authoritative confirmation, thanks!

Any idea what version we’re running? As said initially on this ticket:

> I could not find an upstream that has the commit we’re currently running, and git tag --contains=4838b4e3e2b5f778d593695b94d755ffb6c9f362 returns nothing.

I’m asking because to plan the upgrade I need to inspect the changes made upstream since we cloned their repo, and I cannot do that with the info I have in hand currently. Or is there no useful versioning whatsoever on that develop branch?

As we can see on this ticket, it seems that their develop branch makes upgrades a PITA and is thus not a good fit for a production deployment such as ours. So I have to ask: is there any reason why we should stay on their develop branch, i.e. why we cannot switch to regular stable releases with versioning and no “interesting” Git history rewriting? (I’m sure there was a good reason in March 2016, but hopefully things have changed in the meantime.)

> SO depending on the energy you want to put in there, we may also just stick to the version we have now.

OK, thanks for clarifying. While I could certainly skip that module and look elsewhere, I’m concerned we’re running 2 years old code with no clear plan to upgrade it in the future. For example, I’ve no idea if/how we would notice in case a serious security issue was found in that module. To me, that does not sound like something we should be doing in production. So time permitting (within the budget I have for Feature #15154) I’d like to do what’s needed to make this situation manageable :)

#7 Updated by bertagaz 2018-01-31 11:56:33

• Assignee changed from bertagaz to intrigeri
• QA Check changed from Info Needed to Dev Needed

intrigeri wrote:
> Any idea what version we’re running? As said initially on this ticket:
>
> > I could not find an upstream that has the commit we’re currently running, and git tag --contains=4838b4e3e2b5f778d593695b94d755ffb6c9f362 returns nothing.
>
> I’m asking because to plan the upgrade I need to inspect the changes made upstream since we cloned their repo, and I cannot do that with the info I have in hand currently. Or is there no useful versioning whatsoever on that develop branch?

Doesn’t seem so. The tags seem to be based on their master’s branch commits. I’ve picked the tip of their develop branch at that time, because the master branch was way older and was not compatible with recent Icinga2 versions (the one in Debian and we were using). IIRC that’s because at some point in Icinga2 development they decided to completely rewrite the code they had inherited from Icinga, and it worked/was configured very differently.

> As we can see on this ticket, it seems that their develop branch makes upgrades a PITA and is thus not a good fit for a production deployment such as ours. So I have to ask: is there any reason why we should stay on their develop branch, i.e. why we cannot switch to regular stable releases with versioning and no “interesting” Git history rewriting? (I’m sure there was a good reason in March 2016, but hopefully things have changed in the meantime.)

Maybe nowadays their master branch do support recent Icinga2 and is in a better shape.

> > SO depending on the energy you want to put in there, we may also just stick to the version we have now.
>
> OK, thanks for clarifying. While I could certainly skip that module and look elsewhere, I’m concerned we’re running 2 years old code with no clear plan to upgrade it in the future. For example, I’ve no idea if/how we would notice in case a serious security issue was found in that module. To me, that does not sound like something we should be doing in production. So time permitting (within the budget I have for Feature #15154) I’d like to do what’s needed to make this situation manageable :)

Ack. Note that we’re using a very tiny part of this upstream module code, so if that’s too complecated to upgrade, another path could be to move that part of their code in our own and then drop this uptream module maybe.

#8 Updated by intrigeri 2018-02-19 06:25:47

• Status changed from Confirmed to Rejected
• % Done changed from 0 to 100
• QA Check deleted (Dev Needed)

It looks like we’re using exactly one resource from that module i.e. ::icinga2::object::service. In the version we’re running, that defined resource is self-contained except it uses the icinga2/object/service.conf.erb template, that’s itself self-contained as long as one does not pass a non-empty hash to the vars parameter (we don’t). But in the current upstream version (v1.3.5) that defined resource has include ::icinga2::params (used for $::icinga2::params::conf_dir) and delegates to icinga2::object the actual config file deployment (which is now done with concat). The interface does not change much so in theory it should be doable to upgrade: I think the only required change is in the — now obsolete — target_* parameters we pass to ::icinga2::object::service.

OTOH, seriously: it does not make sense to me to rely on a complex external module merely to use one single defined resource that deploy a file from a template. I mean, the only piece of code we actually use in there is:

    file { "${target_dir}/${target_file_name}":
      ensure  => $target_file_ensure,
      owner   => $target_file_owner,
      group   => $target_file_group,
      mode    => $target_file_mode,
      content => template('icinga2/object/service.conf.erb'),
    }

… which we could do ourselves just as well. The icinga2/object/service.conf.erb template is entirely trivial.

Reusing that piece of code from the upstream icinga2 module made some sense back when we had a plan (Bug #13285) to start using that module more extensively to replace our own code (that reimplements much of its functionality). That plan was added to our 2017Q2→2018Q1 roadmap in 2016, no progress was made and the we removed it from our 2018Q2→2019Q1 roadmap due to unclear cost/benefit.

So I won’t bother upgrading this module: it’s pretty unlikely that there’s ever a security issue in the trivial piece of code we use from it, and I see no point in upgrading that trivial piece of code to a more complex one with exactly zero benefits.

I’m even tempted to drop that module entirely and to copy the bits we use into puppet-tails, so that nobody has to ever do this analysis again. If you agree, let me know and I’ll create a dedicated ticket about it.