Broken links in the View Trends and the View Histogram menu

[Tim Lourey]

[Impact]
The links for the following are broken in the Service Information or the Host Information menus for the following links:

View Trends For This Service
View Alert Histogram For This Service
View Trends For This Host
View Alert Histogram For This Host

[Test Plan]
See deployment steps in comment #15.
Just access those links in: Current status / Hosts and Current status / Services and choosing a host or a service
Test all other links following the menu/pages for no broken links / regressions.

[Where problems could occur]
The URLs for the affected pages have been changed from relative to absolute. Nothing else has changed so this patch should not affect any other page. The worst case scenario would be that the patch has inadvertently modified the link for other pages. In any case the absolute links should also work.

[Other Info]
Upstream code has been patched to fix a bug that required that the html files (on /nagios4) should be one folder level below cgi-bin. This was because sometimes Nagios tried to reach html files as ../file.html while the base url was pointing to the cgi-bin folder. This relative URLs where substituted for absolute URLs. The deb package sets the cgi url to /cgi-bin/nagios4 so was affected by this issue.

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Stefan Hattrell (stefan-hattrell)]

Is there any chance this will get addressed?

[Jorge Merlino (jorge-merlino)]

Patch for Noble, Mantic and Jammy

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "lp1953572_jammy_noble.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

[Jorge Merlino (jorge-merlino)]

Patch for focal

[Jorge Merlino (jorge-merlino)]

Update patch for Noble, Mantic and Jammy

[Heitor Alves de Siqueira (halves)]

Thanks for the debdiff, Jorge! A few comments:

Even though the fix is the same for J/M/N, we'll need separate debdiffs and versions for each. We'll need to keep the upgrade path working between each release, so I'd suggest something like below (according to [0]):
- 4.4.6-4ubuntu0.22.04.1 for Jammy
- 4.4.6-4ubuntu0.23.10.1 for Mantic
- 4.4.6-4ubuntu0.24.04.1 or 4.4.6-4ubuntu1 for Noble (depending on whether O will need fixing as well)

And it's great that you ran the update-maintainer script as well. Would you be able to add a mention of this in the changelog entries, too?

Finally, I'd really appreciate it if we had any mention of the fix on the SRU template. Maybe a short section explaining why we're changing the --with-cgiurl flag, and where that fix came from?

[0] https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation#Update_the_packaging

[Jorge Merlino (jorge-merlino)]

New patch for focal

[Jorge Merlino (jorge-merlino)]

New patch for jammy

[Jorge Merlino (jorge-merlino)]

New patch for mantic

[Jorge Merlino (jorge-merlino)]

New patch for noble

[Jorge Merlino (jorge-merlino)]

patch for Oracular

[Mauricio Faria de Oliveira (mfo)]

Hi Jorge,

Thanks for your work on the analysis and SRU bug template!

I checked this per your request, and we talked about it today.
So, just some notes for documentation purposes and next steps.

The proposed changed to `--with-cgiurl=` just works around the
issue, that is the links are broken due to hard-coded relative
paths, when accessed from different paths, as you mentioned in
'Other Info'.

Other than that, this might be problematic in stable releases,
as users may use the existing URLs in bookmarks and automation
(even if unlikely), that would break if the cgi-bin URL change.

The actual solution to the problem (and the minimal chage from
an SRU perspective) is identifying how to write the links that
always land in the HTML root.

Fortunately, this seems to be simple: change to absolute paths.

I have tested that on our Oracular package and Upstream, which
seems to work, and submitted a PR upstream. If that is approved,
we can go with that, instead:

https://github.com/NagiosEnterprises/nagioscore/pull/969

Let's see how it goes. Marking bug tasks as 'Incomplete' for now.

[Mauricio Faria de Oliveira (mfo)]

Test Steps:
---

lxc launch ubuntu-daily:oracular nagios4-oracular
lxc exec nagios4-oracular -- su - ubuntu

sudo apt update
sudo apt install nagios4 apache2 --yes --no-install-recommends

sudo a2dismod mpm_event
sudo a2dismod mpm_worker
sudo a2enmod mpm_prefork
sudo a2enmod php8.3

sudo a2enmod cgi
sudo a2enmod cgid
sudo a2enconf serve-cgi-bin

sudo a2enmod auth_digest # for AuthDigestDomain in nagios4-cgi.conf
sudo a2enmod authz_groupfile # for AuthGroupFile in nagios4-cgi.conf

sudo a2enconf nagios4-cgi

sudo systemctl restart apache2

<LXD>$ ip -4 a l type veth | grep inet
    inet 10.191.59.205/24 metric 100 brd 10.191.59.255 scope global dynamic eth0

<HOST>$ $BROWSER http://10.191.59.205/nagios4/

1) Menu > Current Status > Hosts
2) Click 'localhost' under 'Host'
3) Click 'View Trends For This Host' or 'View Alert Histogram For This Host'
4) Check 'Not Found / The requested URL was not found on this server.' page
   (or is it found?)

1) Menu > Current Status > Services
2) Click 'Current Load' under 'Service' for 'localhost'
3) Click 'View Trends For This Service' and 'View Alert Histogram For This Service'
4) Check 'Not Found / The requested URL was not found on this server.' page
   (or is it found?)

[Brian Murray (brian-murray)]

Ubuntu 23.10 (Mantic Minotaur) has reached end of life, so this bug will not be fixed for that specific release.

[Jorge Merlino (jorge-merlino)]

Patch for Focal

[Jorge Merlino (jorge-merlino)]

Patch for Jammy

[Jorge Merlino (jorge-merlino)]

Patch for Noble

[Jorge Merlino (jorge-merlino)]

Patch for Oracular

[Mauricio Faria de Oliveira (mfo)]

Hi Jorge,

Thanks for the debdiffs! And sorry about the review delay.

In the future, once you address the pending requests in bugs,
please switch the status back from Incomplete, so it reflects
that you addressed those.

For the debdiffs, please adjust:

1) version strings (can't be identical) for noble/jammy (see [1], ubuntu0.YY.MM.1)
2) version string for focal (see [1], ubuntu0.1)
3) series capitalization for Jammy/Focal
4) add 'd/p/<file.patch>:' prefix for clarity

And lastly, please confirm the changes have been tested.

Thanks again,
Mauricio

[1] https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation#Update_the_packaging

[Mauricio Faria de Oliveira (mfo)]

Jorge, one other thing.
The Other Info section needs to be updated about the different approach.
Thanks!

[Jorge Merlino (jorge-merlino)]

Patch for Oracular v2

Note: for the patch to compile in Oracular the environment variables DEB_VERSION, DEB_SOURCE and DEB_BUILD_OS_RELEASE_ID must be defined. I'm not sure what the correct values are but I just set them to one in order to build the deb files:

export DEB_VERSION=1
export DEB_SOURCE=1
export DEB_BUILD_OS_RELEASE_ID=1

[Jorge Merlino (jorge-merlino)]

Patch for Noble v2

[Jorge Merlino (jorge-merlino)]

Patch for Jammy v2

[Jorge Merlino (jorge-merlino)]

Patch for Focal v2

[Jorge Merlino (jorge-merlino)]

Patches updated according to comment #21. All patches were tested according to the test plan and verified that they fix the bug.

[Mauricio Faria de Oliveira (mfo)]

Thanks, Jorge!

I can confirm the debdiff builds correctly in Oracular
in a PPA (all archs) and locally (didn't need flags?):

$ DEB_BUILD_OPTIONS=parallel=4 dpkg-buildpackage -us -uc

And verified the new build uses absolute paths (fixed),
while the old build used relative paths (not fixed).

New:

$ dpkg-deb -x nagios4-cgi_4.4.6-4.1ubuntu1_amd64.deb deb-new
$ strings deb-new/usr/lib/cgi-bin/nagios4/*.cgi | grep -F -e trends.html -e histogram.html
/nagios4/trends.html
/nagios4/histogram.html
/nagios4/trends.html
/nagios4/histogram.html
/nagios4/trends.html
/nagios4/trends.html
/nagios4/trends.html
/nagios4/trends.html
/nagios4/histogram.html

Old:

$ dpkg-deb -x nagios4-cgi_4.4.6-4.1_amd64.deb deb-old
$ strings deb-old/usr/lib/cgi-bin/nagios4/*.cgi | grep -F -e trends.html -e histogram.html
../trends.html
../histogram.html
../trends.html
../histogram.html
../trends.html
../trends.html
../trends.html
../trends.html
../histogram.html

[Mauricio Faria de Oliveira (mfo)]

Looking for sponsorship for Oracular.

[Mauricio Faria de Oliveira (mfo)]

Hi Jorge,

Something minor, but important to understand, for rigor.

`Origin: upstream,` should be `Origin: backport,` instead, as the upstream patch needed changes to apply.

> The allowed keywords are
> "upstream" (in the case of a patch cherry-picked from the upstream VCS),
> "backport" (in the case of an upstream patch that had to be modified to apply on the current version), [...]
@ https://dep-team.pages.debian.net/deps/dep3/

I can change it when uploading to the stable releases.

Thanks,

Note:

$ wget https://github.com/NagiosEnterprises/nagioscore/commit/c974362.patch

$ git ubuntu clone nagios4
$ cd nagios4

$ git checkout pkg/ubuntu/noble-devel

$ patch -p1 -F0 --dry-run -i ../c974362.patch
checking file Changelog
Hunk #1 FAILED at 2.
...

$ git checkout pkg/ubuntu/jammy-devel

$ patch -p1 -F0 --dry-run -i ../c974362.patch
checking file Changelog
Hunk #1 FAILED at 2.
...

$ git checkout pkg/ubuntu/focal-devel

$ patch -p1 -F0 --dry-run -i ../c974362.patch
checking file Changelog
Hunk #1 FAILED at 2.
...

[Lena Voytek (lvoytek)]

Hi Jorge and Mauricio, I will sponsor for oracular. The change looks good to me overall, I just have a few notes:

Since the changes to the THANKS and Changelog files are not relevant to the build, and the patch is already a backport due to line number changes, I would recommend removing those portions for better patch compatibility with future updates.

Mauricio also already mentioned changing Origin: upstream to Origin: backport. I can change that too

Last minor note - in the changelog for the lp bug number I'd recommend surrounding with parenthesis for formatting - (LP: #1953572). I've seen some of our automation miss bug numbers in the past without them.

If you would like I can update the debdiff and sponsor, thanks!

[Mauricio Faria de Oliveira (mfo)]

Thanks, Lena!

I'm not biased towards including or not including the non-code changes (eg, Changelog and THANKS),
as I'd presume any potential problems in future updates due to these files should happen because
the future patches would be touching these files as well. This would mean the future patch didn't
remove them either, and if it didn't, maybe having these changes introduced now would help keeping
these files more up-to-date, so that future patches apply without changes.
(Not sure I'm missing something in maintenance context that you're familiar with and I'm not -- happy to learn more!)

Yeah, the LP bug number without parentheses caught my eyes too, but AFAIK, the LP automation only strictly requires the `LP: #<number>` part (seen other SRUs like that). The parentheses are easier on the eye / more as usual too.

I'm happy to change that in the stable releases too.

If you don't mind the Changelog/THANKS changes, please feel free to proceed with the upload!

Thanks again for your help :)

[Lena Voytek (lvoytek)]

Sounds good to me! Sponsored with small changes only -

dput ubuntu ../nagios4_4.4.6-4.1ubuntu1_source.changes
D: Setting host argument.
Checking signature on .changes
gpg: ../nagios4_4.4.6-4.1ubuntu1_source.changes: Valid signature from 34B8AD7D9529E793
Checking signature on .dsc
gpg: ../nagios4_4.4.6-4.1ubuntu1.dsc: Valid signature from 34B8AD7D9529E793
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading nagios4_4.4.6-4.1ubuntu1.dsc: done.
  Uploading nagios4_4.4.6-4.1ubuntu1.debian.tar.xz: done.
  Uploading nagios4_4.4.6-4.1ubuntu1_source.buildinfo: done.
  Uploading nagios4_4.4.6-4.1ubuntu1_source.changes: done.
Successfully uploaded packages.

[Mauricio Faria de Oliveira (mfo)]

This should come up soon in Fix Committed / oracular-proposed.
https://launchpad.net/ubuntu/+source/nagios4/4.4.6-4.1ubuntu1

[Mauricio Faria de Oliveira (mfo)]

Uploaded to Noble, Jammy, Focal.

[Launchpad Janitor (janitor)]

This bug was fixed in the package nagios4 - 4.4.6-4.1ubuntu1

---------------
nagios4 (4.4.6-4.1ubuntu1) oracular; urgency=medium

  * d/p/lp1953572-Fix-inconsistent-links-for-View-Trends-Alert.patch:
    Fix broken links in the View Trends and View Histogram menus.
    (LP: #1953572)

 -- Jorge Merlino <email address hidden> Mon, 29 Jul 2024 11:56:48 -0300