Merge ~mitchdz/ubuntu/+source/apache2:noble-add-ubuntu-logo into ubuntu/+source/apache2:ubuntu/devel

[Mitchell Dzurick]

PPA - https://launchpad.net/~mitchdz/+archive/ubuntu/apache2-lp1947459-missing-ubuntu-logo

ppa-dev-tools.ppa tests --show-url ppa:mitchdz/apache2-lp1947459-missing-ubuntu-logo --release noble
  - apache2/2.4.57-2ubuntu3~noble1
    + ✅ apache2 on noble for amd64 @ 14.11.23 20:26:33
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-mitchdz-apache2-lp1947459-missing-ubuntu-logo/noble/amd64/a/apache2/20231114_202633_7095a@/log.gz
    + ✅ apache2 on noble for arm64 @ 14.11.23 21:04:37
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-mitchdz-apache2-lp1947459-missing-ubuntu-logo/noble/arm64/a/apache2/20231114_210437_8ddac@/log.gz
    + ✅ apache2 on noble for armhf @ 14.11.23 21:02:15
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-mitchdz-apache2-lp1947459-missing-ubuntu-logo/noble/armhf/a/apache2/20231114_210215_f91aa@/log.gz
      • Status: FAIL
      • run-test-suite FAIL 🟥
      • duplicate-module-load PASS 🟩
      • default-mods PASS 🟩
      • htcacheclean PASS 🟩
      • ssl-passphrase PASS 🟩
      • check-http2 PASS 🟩
      • check-ubuntu-branding PASS 🟩
      • chroot FAIL 🟥
      • chroot FAIL 🟥
    + ✅ apache2 on noble for ppc64el @ 14.11.23 20:34:34
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-mitchdz-apache2-lp1947459-missing-ubuntu-logo/noble/ppc64el/a/apache2/20231114_203434_c3053@/log.gz
    + ✅ apache2 on noble for s390x @ 14.11.23 20:25:59
      • Log: https://autopkgtest.ubuntu.com/results/autopkgtest-noble-mitchdz-apache2-lp1947459-missing-ubuntu-logo/noble/s390x/a/apache2/20231114_202559_93af5@/log.gz

NOTE: armhf is passing all tests, possible bug in ppa-dev-tools filed here - https://bugs.launchpad.net/ppa-dev-tools/+bug/2043505

[Mitchell Dzurick (mitchdz)]

Removed breaks-testbed as restriction as shouldn't be needed. Re-running ppa autopkgtest and will set back to Needs-Review once done.

[Sergio Durigan Junior (sergiodj)]

Thanks, Mitchell.

I'm leaving a few comments to improve the dep8 test below. Let me know when you address those and I'll finish the review & sponsorship.

review: Needs Fixing

[Mitchell Dzurick (mitchdz)]

Thanks Sergio. I applied most suggestions except `set -o lastpipe` since I am using sh instead of bash in this script.

local autopkgtest result on amd64:

$ autopkgtest -U -s -o "dep8-apache2" --test-name=check-ubuntu-branding ../apache2_2.4.57-2ubuntu3\~noble3.dsc -- qemu "/var/lib/adt-images/autopkgtest-noble-amd64.img"
....
autopkgtest [23:36:22]: test check-ubuntu-branding: [-----------------------
+ ubuntu_logo_path=icons/ubuntu-logo.png
+ grep -qF icons/ubuntu-logo.png
+ curl -s -k http://localhost
+ tr -d [:space:]
+ cut -d -f 2-
+ grep Content-Type
+ curl -s -I http://localhost/icons/ubuntu-logo.png
+ content_type=image/png
+ expected=image/png
+ [ image/png != image/png ]
autopkgtest [23:36:23]: test check-ubuntu-branding: -----------------------]
check-ubuntu-branding PASS
autopkgtest [23:36:23]: test check-ubuntu-branding: - - - - - - - - - - results - - - - - - - - - -
autopkgtest [23:36:24]: @@@@@@@@@@@@@@@@@@@@ summary
check-ubuntu-branding PASS

[Sergio Durigan Junior (sergiodj)]

Thanks, Mitchell.

It's not a problem to require bash to run the script, since it will be installed inside the autopkgtest testbed due to the package's priority.

I really think we should use pipefail here because of two things: first, you're not explicitly checking the return status of each command (due to "set -e", which is problematic by itself, but let's not go there), and second because of how you're calculating the value of ${content_type}:

content_type=$(curl -s -I http://localhost/$ubuntu_logo_path | grep Content-Type | cut -d ' ' -f 2- | tr -d '[:space:]')

Without pipefail, this will always succeed. Even though you're explicitly checking the content of the variable below, it's nevertheless a good practice to fail early here.

I hope this makes sense, but let me know if you still have reservations about using bash explicitly.

review: Needs Fixing

[Mitchell Dzurick (mitchdz)]

I agree pipefail should be used here. same for `set -e`, I'm not the biggest fan of it, but kept it in for consistency across test scripts.

Since 2 tests do use bash (default-mods && run-test-suite) I'll change the interpreter to use bash and use pipefail to alleviate those concerns.

[Mitchell Dzurick (mitchdz)]

Sergio, I went to enable pipefail and went down a nice rabbit hole because the script became nondeterministic. I narrowed it down to this fun quirk - https://mywiki.wooledge.org/BashPitfalls?highlight=%28pipefail%29#pipefail

I'm revising the script to inherently do better error handling.

[Mitchell Dzurick (mitchdz)]

reworked the script now. Ready for review again.

[Sergio Durigan Junior (sergiodj)]

Thanks, Mitchell.

LGTM.

Uploaded:

$ dput ssh-ubuntu apache2_2.4.57-2ubuntu3_source.changes
D: Setting host argument.
Checking signature on .changes
gpg: /home/sergio/work/apache2/apache2_2.4.57-2ubuntu3_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/apache2/apache2_2.4.57-2ubuntu3.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ssh-ubuntu (via sftp to upload.ubuntu.com):
  Uploading apache2_2.4.57-2ubuntu3.dsc: done.
  Uploading apache2_2.4.57-2ubuntu3.debian.tar.xz: done.
  Uploading apache2_2.4.57-2ubuntu3_source.buildinfo: done.
  Uploading apache2_2.4.57-2ubuntu3_source.changes: done.
Successfully uploaded packages.

review: Approve

diff --git a/debian/changelog b/debian/changelog
index 118f242..dc4a0c2 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+apache2 (2.4.57-2ubuntu3) noble; urgency=medium
+
+  * d/icons/ubuntu-logo.png: add Ubuntu image for welcome page (LP: #1947459).
+  * d/t/check-ubuntu-branding: add check for ubuntu branding.
+
+ -- Mitchell Dzurick <[e-mail]>  Mon, 13 Nov 2023 10:49:48 -0700
+
 apache2 (2.4.57-2ubuntu2) mantic; urgency=medium
 
   * d/control: Upgrade lua build dependency to 5.4
diff --git a/debian/icons/ubuntu-logo.png b/debian/icons/ubuntu-logo.png
new file mode 100644
index 0000000..eee686c
Binary files /dev/null and b/debian/icons/ubuntu-logo.png differ
diff --git a/debian/tests/check-ubuntu-branding b/debian/tests/check-ubuntu-branding
new file mode 100644
index 0000000..0bf90b6
--- /dev/null
+++ b/debian/tests/check-ubuntu-branding
@@ -0,0 +1,28 @@
+#!/bin/bash
+#
+# Check the ubuntu branding exists
+set -uxe -o pipefail
+
+ubuntu_logo_path="icons/ubuntu-logo.png"
+
+# Use curl to fetch the HTML content and check its exit status
+if html_content=$(curl -s http://localhost); then
+  # The curl command succeeded, so proceed with further processing
+  if ! [[ "$html_content" =~ "$ubuntu_logo_path" ]]; then
+    echo "ERROR: $ubuntu_logo_path string not found in html page"
+    exit 1
+  fi
+else
+  # The curl command encountered an error
+  echo "ERROR: Curl command failed to fetch web content"
+  exit 1
+fi
+
+# Check the type of $ubuntu_logo_path
+content_type=$(curl -s -I http://localhost/$ubuntu_logo_path \
+	| grep Content-Type | cut -d ' ' -f 2- | tr -d '[:space:]')
+expected="image/png"
+if [ "$content_type" != "$expected" ]; then
+  echo "Content-Type is not $expected it is $content_type"
+  exit 1
+fi
diff --git a/debian/tests/control b/debian/tests/control
index 2453137..8a93e5f 100644
--- a/debian/tests/control
+++ b/debian/tests/control
@@ -23,6 +23,10 @@ Tests: check-http2
 Restrictions: needs-root allow-stderr breaks-testbed
 Depends: apache2, curl, ssl-cert, nghttp2-client
 
+Tests: check-ubuntu-branding
+Restrictions: allow-stderr
+Depends: apache2, curl
+
 Tests: chroot
 Features: no-build-needed
 Restrictions: needs-root allow-stderr breaks-testbed