Merge ~pfsmorigo/review-tools:pfsmorigo/core24 into review-tools:master

[Paulo Flabiano Smorigo]

Hello team, can you review this pull request? Some doubts I had while doing it:

r/overrides.py:1038:
func_base_state_files_snaps_overrides = {"core22": "20220421"}
Do I need to do a state files overrride for core24 too?

r/store.py:46:
I use "undefined" for snap_to_release. Is that ok?

r/t/test_store.py:2121:
There is this test and to add one for core24 I need the release name right?
test_check_get_ubuntu_release_from_manifest_os_release_neon_bionic_core22

[Alex Murray (alexmurray)]

> r/overrides.py:1038:
> func_base_state_files_snaps_overrides = {"core22": "20220421"}
> Do I need to do a state files overrride for core24 too?
>

This state files override exists so that as soon as that date passes, we consider the contents of the snap to be stable and from that point onwards will run the stateful checks - since we don't know the exact release date of 24.04 this is a bit tricky, but perhaps set this as 20240430 for now since it should definitely be released not too long before that date.

> r/store.py:46:
> I use "undefined" for snap_to_release. Is that ok?
>

We know this will be NN - so how about nanimal for now (as this kind of nomenclature has been used before)?

> r/t/test_store.py:2121:
> There is this test and to add one for core24 I need the release name right?
> test_check_get_ubuntu_release_from_manifest_os_release_neon_bionic_core22

You could use nanimal again here if you want.

review: Needs Fixing

[Alex Murray (alexmurray)]

LGTM - but could you please add code comments around the overrides timestamp and nanimal name bits so we don't forget to update these in the future - something like:

// TODO: update when 24.04 is announced / released

review: Needs Fixing

[Paulo Flabiano Smorigo (pfsmorigo)]

Add the comments and ran the review over the snap locally: `core24.snap: pass`.

[Alex Murray (alexmurray)]

Thanks @pfsmorigo - LGTM!

review: Approve

diff --git a/reviewtools/overrides.py b/reviewtools/overrides.py
index 2b1bcba..708f662 100644
--- a/reviewtools/overrides.py
+++ b/reviewtools/overrides.py
@@ -132,6 +132,7 @@ redflagged_snap_types_overrides = {
         "core20",
         "core22",
         "core22-desktop",
+        "core24",
         "core24-desktop",
         "test-snapd-base",
         "test-snapd-core18",
@@ -367,6 +368,26 @@ sec_mode_overrides = {
         "./usr/sbin/unix_chkpwd": "rwxr-sr-x",
         "./var/mail": "rwxrwsr-x",
     },
+    "core24": {
+        "./usr/bin/chage": "rwxr-sr-x",
+        "./usr/bin/chfn": "rwsr-xr-x",
+        "./usr/bin/chsh": "rwsr-xr-x",
+        "./usr/bin/expiry": "rwxr-sr-x",
+        "./usr/bin/gpasswd": "rwsr-xr-x",
+        "./usr/bin/mount": "rwsr-xr-x",
+        "./usr/bin/newgrp": "rwsr-xr-x",
+        "./usr/bin/passwd": "rwsr-xr-x",
+        "./usr/bin/ssh-agent": "rwxr-sr-x",
+        "./usr/bin/su": "rwsr-xr-x",
+        "./usr/bin/sudo": "rwsr-xr-x",
+        "./usr/bin/umount": "rwsr-xr-x",
+        "./usr/bin/wall": "rwxr-sr-x",
+        "./usr/lib/dbus-1.0/dbus-daemon-launch-helper": "rwsr-xr--",
+        "./usr/lib/openssh/ssh-keysign": "rwsr-xr-x",
+        "./usr/sbin/pam_extrausers_chkpwd": "rwxr-sr-x",
+        "./usr/sbin/unix_chkpwd": "rwxr-sr-x",
+        "./var/mail": "rwxrwsr-x",
+    },
     "core24-desktop": {
         "./usr/bin/chage": "rwxr-sr-x",
         "./usr/bin/chfn": "rwsr-xr-x",
@@ -672,6 +693,12 @@ sec_mode_dev_overrides = {
         "./dev/urandom": ("crw-rw-rw-", "0/0"),
         "./dev/zero": ("crw-rw-rw-", "0/0"),
     },
+    "core24": {
+        "./dev/null": ("crw-rw-rw-", "0/0"),
+        "./dev/random": ("crw-rw-rw-", "0/0"),
+        "./dev/urandom": ("crw-rw-rw-", "0/0"),
+        "./dev/zero": ("crw-rw-rw-", "0/0"),
+    },
     "core24-desktop": {
         "./dev/null": ("crw-rw-rw-", "0/0"),
         "./dev/random": ("crw-rw-rw-", "0/0"),
@@ -1008,7 +1035,7 @@ func_base_state_files_overrides = {
 }
 
 # Allow skipping checks for some snaps
-func_base_state_files_snaps_overrides = {"core22": "20220421"}
+func_base_state_files_snaps_overrides = {"core24": "20240430"}
 
 # By default we don't regulate which snaps specify which base snaps, but some
 # base snaps are highly specialized, so we limit what can use them. Base snaps
diff --git a/reviewtools/sr_lint.py b/reviewtools/sr_lint.py
index dd1d3b7..a950329 100644
--- a/reviewtools/sr_lint.py
+++ b/reviewtools/sr_lint.py
@@ -66,8 +66,8 @@ class SnapReviewLint(SnapReview):
             "core18",
             "core20",
             "core22",
-            "core22-desktop",
-            "godot-bare",
+            "core24",
+            "test-snapd-refresh-control-base",
             "keepstock-base-scot",
             "nix-base",
             "test-snapd-base-bare",
diff --git a/reviewtools/store.py b/reviewtools/store.py
index 43e6b8c..e84cd00 100644
--- a/reviewtools/store.py
+++ b/reviewtools/store.py
@@ -44,6 +44,7 @@ snap_to_release = {
     "core18": "bionic",
     "core20": "focal",
     "core22": "jammy",
+    "core24": "nanimal",  # TODO: update when 24.04 is announced / released
 }
 
 
diff --git a/reviewtools/tests/test_store.py b/reviewtools/tests/test_store.py
index 23d1c1b..cf7a129 100644
--- a/reviewtools/tests/test_store.py
+++ b/reviewtools/tests/test_store.py
@@ -2127,6 +2127,16 @@ class TestStore(TestCase):
         res = store.get_ubuntu_release_from_manifest(m)
         self.assertEqual(res, "jammy")
 
+    # TODO: update when 24.04 is announced / released
+    def test_check_get_ubuntu_release_from_manifest_os_release_neon_bionic_core24(self):
+        """Test get_ubuntu_release_from_manifest() - neon/18.04 with base: core24"""
+        m = copy.deepcopy(self.manifest_basic)
+        m["base"] = "core24"
+        m["snapcraft-os-release-id"] = "neon"
+        m["snapcraft-os-release-version-id"] = "18.04"
+        res = store.get_ubuntu_release_from_manifest(m)
+        self.assertEqual(res, "nanimal")
+
     def test_check_get_ubuntu_release_from_manifest_os_release_nonexist_os_and_ver(
         self,
     ):
diff --git a/tests/manual-lzo-part2.sh b/tests/manual-lzo-part2.sh
index 6fa085a..826a4e0 100755
--- a/tests/manual-lzo-part2.sh
+++ b/tests/manual-lzo-part2.sh
@@ -35,11 +35,7 @@ TARBALL=$(readlink -f "$1")
 
 # TODO: share common snippets of this script with manual-lzo.sh ?
 function is_core_snap {
-    if [ "$1" = "core" ] || [ "$1" = "core18" ] || [ "$1" = "core20" ]  || [ "$1" = "core22" ]; then
-        return 0
-    else
-        return 1
-    fi
+    [[ $1 =~ ^core[0-9]*$ ]] && return 0 || return 1
 }
 
 # step 0. ensure pre-reqs are available
diff --git a/tests/manual-lzo.sh b/tests/manual-lzo.sh
index 1e7e620..60b2885 100755
--- a/tests/manual-lzo.sh
+++ b/tests/manual-lzo.sh
@@ -96,11 +96,7 @@ function is_core_or_snapd_snap {
 }
 
 function is_core_snap {
-    if [ "$1" = "core" ] || [ "$1" = "core18" ] || [ "$1" = "core20" ] || [ "$1" = "core22" ]; then
-        return 0
-    else
-        return 1
-    fi
+    [[ $1 =~ ^core[0-9]*$ ]] && return 0 || return 1
 }