Address review: key token cache by repo, run the tests in CI

get_repo_installation() resolves per repo, so an owner-keyed cache is wrong
for a 'Selected repositories' install: one repo's token would be handed to a
sibling, or one repo's 'not installed' result would park the whole owner on
the PAT path for 15 minutes. Key by owner/repo instead.

The test file was never collected -- tests.yml only runs aws/lambda/tests,
and the deploy job is gated to push-on-main. Add a test job to the lambda's
own workflow, matching opensearch-gha-jobs-lambda.yml. pytest goes on the
install line rather than requirements.txt, which the Makefile ships in the
deployment zip.

Author: huydhn

.github/workflows/github-status-test-lambda.yml

@@ -1,4 +1,4 @@
-name: Deploy github-status-test
+name: Test and deploy github-status-test
 
 on:
   push:
@@ -17,7 +17,19 @@ defaults:
     working-directory: aws/lambda/github-status-test/
 
 jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+      - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
+        with:
+          python-version: '3.11'
+          cache: pip
+      - run: pip3 install -r requirements.txt pytest
+      - run: pytest -v test_lambda_function.py
+
   deploy:
+    needs: test
     runs-on: ubuntu-latest
     if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}
     permissions:

aws/lambda/github-status-test/lambda_function.py

@@ -28,7 +28,7 @@
 # Installation tokens last an hour. Refresh early so a warm invocation never
 # signs a request with a token that expires mid-flight.
 TOKEN_EXPIRY_MARGIN = 300
-# How long to remember that an owner has no app installation, so repos outside
+# How long to remember that a repo has no app installation, so repos outside
 # the installation don't trigger a lookup for every job.
 NO_INSTALLATION_TTL = 900
 # Used when a credential is rejected without telling us when it recovers.
@@ -37,7 +37,11 @@
 # (403 or 429) or rejected outright (401).
 FALLBACK_STATUSES = (401, 403, 429)
 
-# owner -> (installation token or None, epoch seconds the entry goes stale)
+# Keyed by "owner/repo", not owner: get_repo_installation() resolves per repo,
+# so a "Selected repositories" install can cover one repo of an owner and not
+# its sibling. Sharing an owner's entry would hand a repo a token minted for a
+# different one, or let one repo's "not installed" result mask another's.
+# full_name -> (installation token or None, epoch seconds the entry goes stale)
 _token_cache = {}
 
 
@@ -63,7 +67,7 @@ def cool_off_until(response):
 def fetch_installation_token(full_name):
     """Mint an installation token for the app installation covering full_name.
 
-    Returns (None, expiry) when the app isn't installed on that owner, so the
+    Returns (None, expiry) when the app isn't installed on that repo, so the
     caller falls back to a PAT instead of retrying the lookup for every job.
     """
     owner, repo = full_name.split("/", 1)
@@ -81,12 +85,11 @@ def fetch_installation_token(full_name):
 
 
 def installation_token(full_name):
-    """Cached installation token for full_name's owner, or None if unavailable."""
+    """Cached installation token for full_name, or None if unavailable."""
     if not GITHUB_APP_ID or not GITHUB_APP_PRIVATE_KEY:
         return None
 
-    owner = full_name.split("/")[0]
-    cached = _token_cache.get(owner)
+    cached = _token_cache.get(full_name)
     if cached and time.time() < cached[1]:
         return cached[0]
 
@@ -97,10 +100,10 @@ def installation_token(full_name):
         # GitHub blip must degrade to the PAT pool, never fail the webhook and
         # lose the payload archiving that happens after the log download.
         # Not cached either, so the next invocation retries.
-        print(f"ERROR minting installation token for {owner}: {err}")
+        print(f"ERROR minting installation token for {full_name}: {err}")
         return None
 
-    _token_cache[owner] = (token, expires_at)
+    _token_cache[full_name] = (token, expires_at)
     return token
 
 
@@ -122,8 +125,7 @@ def download_log(full_name, conclusion, job_id):
         if response.status_code in FALLBACK_STATUSES:
             # Stop using the app until its window resets, otherwise every job
             # for the rest of the hour pays for a doomed request first.
-            owner = full_name.split("/")[0]
-            _token_cache[owner] = (None, cool_off_until(response))
+            _token_cache[full_name] = (None, cool_off_until(response))
             print(
                 f"App auth returned {response.status_code} for {full_name} "
                 f"job {job_id}, falling back to a PAT"

aws/lambda/github-status-test/test_lambda_function.py

@@ -27,21 +27,41 @@ def test_returns_none_without_app_credentials(self):
 
     @patch.object(lambda_function, "GITHUB_APP_ID", "4550824")
     @patch.object(lambda_function, "GITHUB_APP_PRIVATE_KEY", "key")
-    def test_token_is_cached_per_owner(self):
+    def test_token_is_cached_per_repo(self):
         with patch.object(
             lambda_function,
             "fetch_installation_token",
             return_value=("tok", 2**31),
         ) as fetch:
             self.assertEqual(installation_token("pytorch/pytorch"), "tok")
-            # Same owner, different repo: served from cache, no second mint
-            self.assertEqual(installation_token("pytorch/executorch"), "tok")
+            self.assertEqual(installation_token("pytorch/pytorch"), "tok")
+            # Same repo twice is one mint...
             fetch.assert_called_once()
+            # ...but a sibling repo resolves separately, because a "Selected
+            # repositories" install can cover one and not the other.
+            self.assertEqual(installation_token("pytorch/executorch"), "tok")
+            self.assertEqual(fetch.call_count, 2)
+
+    @patch.object(lambda_function, "GITHUB_APP_ID", "4550824")
+    @patch.object(lambda_function, "GITHUB_APP_PRIVATE_KEY", "key")
+    def test_uninstalled_sibling_does_not_mask_an_installed_repo(self):
+        # Under a "Selected repositories" install, querying the uninstalled
+        # sibling first must not park the whole owner on the PAT path.
+        def per_repo(full_name):
+            if full_name == "pytorch/not-installed":
+                return None, 2**31
+            return "tok", 2**31
+
+        with patch.object(
+            lambda_function, "fetch_installation_token", side_effect=per_repo
+        ):
+            self.assertIsNone(installation_token("pytorch/not-installed"))
+            self.assertEqual(installation_token("pytorch/pytorch"), "tok")
 
     @patch.object(lambda_function, "GITHUB_APP_ID", "4550824")
     @patch.object(lambda_function, "GITHUB_APP_PRIVATE_KEY", "key")
     def test_expired_cache_entry_is_refreshed(self):
-        lambda_function._token_cache["pytorch"] = ("stale", 0)
+        lambda_function._token_cache["pytorch/pytorch"] = ("stale", 0)
         with patch.object(
             lambda_function,
             "fetch_installation_token",
@@ -70,7 +90,7 @@ def test_mint_failure_is_not_cached(self):
             side_effect=lambda_function.requests.RequestException("boom"),
         ):
             self.assertIsNone(installation_token("pytorch/pytorch"))
-        self.assertNotIn("pytorch", lambda_function._token_cache)
+        self.assertNotIn("pytorch/pytorch", lambda_function._token_cache)
 
     @patch.object(lambda_function, "GITHUB_APP_ID", "4550824")
     @patch.object(lambda_function, "GITHUB_APP_PRIVATE_KEY", "bm90LWEta2V5")
@@ -136,7 +156,7 @@ def test_rate_limited_app_is_skipped_until_reset(self, urlopen, s3):
         # 2 calls for the first job (app then PAT), 1 for the second (PAT only)
         self.assertEqual(fetch_log.call_count, 3)
         self.assertEqual(
-            lambda_function._token_cache["pytorch"], (None, float(reset_at))
+            lambda_function._token_cache["pytorch/pytorch"], (None, float(reset_at))
         )
 
     def test_error_response_is_not_archived(self, urlopen, s3):