Skip to content

Commit

Permalink
Fix missing files from kickstart indexes [RHELDST-22387]
Browse files Browse the repository at this point in the history
RHELDST-22148 introduced the concept of a partial autoindex, which is
triggered as soon as entry points are added onto a publish without
waiting until commit.

This introduced a problem for kickstart repos. The problem is that a
kickstart repo is simultaneously a valid yum repo, containing both yum
and kickstart entrypoints. There is no guarantee in which order the
client will add these onto a publish.

If the client added yum repodata first, partial autoindex could be
triggered and successfully parse the repo as a yum repo (only). If the
client then later added kickstart repodata, the autoindex would not be
regenerated to include kickstart files, because the index is treated
as immutable and assumed valid if it already exists on the publish.

This commit fixes the problem by effectively disabling partial autoindex
for kickstart repos, allowing them to instead generate indexes at commit
time as it worked originally.

This can be considered more of a workaround than a proper fix. A better
fix might be done later to make autoindex_partial work correctly for
kickstart repos rather than disabling it.
  • Loading branch information
rohanpm committed Jan 9, 2024
1 parent f1f3276 commit da71c5e
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 1 deletion.
15 changes: 14 additions & 1 deletion exodus_gw/routers/publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,10 +339,23 @@ def update_publish_items(
if item.object_key == "absent":
# deleted items don't get indexed
continue

basename = os.path.basename(item.web_uri)
for entrypoint_basename in settings.entry_point_files:
if basename == entrypoint_basename:
entrypoint_paths.add(item.web_uri)
if any(
[
exclude in item.web_uri
for exclude in settings.autoindex_partial_excludes
]
):
# Not eligible for partial autoindex, e.g. /kickstart/ repos
# because the kickstart and yum repodata might arrive separately.
LOG.info(
"%s: excluded from partial autoindex", item.web_uri
)
else:
entrypoint_paths.add(item.web_uri)

if entrypoint_paths:
msg = worker.autoindex_partial.send(
Expand Down
5 changes: 5 additions & 0 deletions exodus_gw/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ class Settings(BaseSettings):
Can be set to an empty string to disable generation of indexes.
"""

autoindex_partial_excludes: List[str] = ["/kickstart/"]
"""Background processing of autoindexes will be disabled for paths matching
any of these values.
"""

config_cache_ttl: int = 2
"""Time (in minutes) config is expected to live in components that consume it.
Expand Down
49 changes: 49 additions & 0 deletions tests/routers/test_publish.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import logging
from datetime import datetime, timedelta

import mock
Expand Down Expand Up @@ -290,6 +291,54 @@ def test_update_publish_items_autoindex(db, auth_header):
]


def test_update_publish_items_autoindex_excluded(
db, auth_header, caplog: pytest.LogCaptureFixture
):
"""PUTting items including entry points under an excluded path will NOT trigger
partial autoindex.
"""

publish_id = "11224567-e89b-12d3-a456-426614174000"

publish = Publish(id=publish_id, env="test", state="PENDING")

with TestClient(app) as client:
# Ensure a publish object exists
db.add(publish)
db.commit()

# Try to add some items to it
r = client.put(
"/test/publish/%s" % publish_id,
json=[
{
"web_uri": "/some/kickstart/repo1/repodata/repomd.xml",
"object_key": "1" * 64,
},
{
"web_uri": "/some/kickstart/repo1/other",
"object_key": "2" * 64,
},
],
headers=auth_header(roles=["test-publisher"]),
)

# It should have succeeded
assert r.status_code == 200

# Check the enqueued messages...
messages: list[DramatiqMessage] = db.query(DramatiqMessage).all()

# It should have enqueued no messages
assert len(messages) == 0

# And it should have logged about the exclusion
assert (
"/some/kickstart/repo1/repodata/repomd.xml: excluded from partial autoindex"
in caplog.text
)


def test_update_publish_items_path_normalization(db, auth_header):
"""URI and link target paths are normalized in PUT items."""

Expand Down

0 comments on commit da71c5e

Please sign in to comment.