internal/v1: use latest snapshots for CDN repos by default #1712

rverdile · 2025-10-23T19:36:39Z

Automatically transform CDN repository URLs to use latest snapshots from content sources API when no explicit SnapshotDate is provided. Non-CDN repos pass through unchanged.

ticket: https://issues.redhat.com/browse/HMS-8918

lzap · 2025-10-24T17:50:38Z

internal/v1/handler_compose_image.go

+		}
+	}
+
+	for _, r := range arch.Repositories {


This could be a single loop and errors could be collected and then joined via errors.Join. But this is fine enough.

I don't mind changing this

lzap · 2025-10-24T17:56:15Z

Looks ok.

croissanne · 2025-10-28T12:19:56Z

I wonder if we can just drop repositories from distributions for select distributions in this case

rverdile · 2025-10-29T14:07:15Z

I wonder if we can just drop repositories from distributions for select distributions in this case

@croissanne Happy to implement this however you guys want. What would that look like? Do you mean remove the definition of the repository from the distributions folder? I thought it might be simpler to let image builder maintain its own source of truth in terms of what should be available, since there are many repos not provided by content sources

avitova · 2025-10-29T15:26:09Z

internal/v1/handler_compose_image.go

+		if len(r.ImageTypeTags) > 0 && !slices.Contains(r.ImageTypeTags, string(imageType)) {
+			continue
+		}
+
+		composerRepo := composer.Repository{
+			CheckGpg: r.CheckGpg,
+		}
+
+		if r.Rhsm {
+			// CDN repository - try to use latest snapshot
+			var matchedRepo *content_sources.ApiRepositoryResponse
+			for _, apiRepo := range repoMap {
+				if apiRepo.Url != nil && *apiRepo.Url == *r.Baseurl {
+					matchedRepo = &apiRepo
+					break
+				}
+			}
+
+			if matchedRepo != nil && matchedRepo.LatestSnapshotUrl != nil && *matchedRepo.LatestSnapshotUrl != "" {
+				repoURL, err := url.Parse(*matchedRepo.LatestSnapshotUrl)
+				if err != nil {
+					return nil, fmt.Errorf("failed to parse snapshot URL for repository %s: %w", *r.Baseurl, err)
+				}
+				composerRepo.Baseurl = common.ToPtr(h.server.csReposURL.JoinPath(repoURL.Path).String())
+				composerRepo.Rhsm = common.ToPtr(false)
+				if matchedRepo.GpgKey != nil && *matchedRepo.GpgKey != "" {
+					composerRepo.Gpgkey = matchedRepo.GpgKey
+				}
+				composerRepo.ModuleHotfixes = matchedRepo.ModuleHotfixes
+				composerRepo.CheckRepoGpg = matchedRepo.MetadataVerification
+			} else {
+				return nil, fmt.Errorf("no latest snapshot URL available for CDN repository %s", *r.Baseurl)
+			}
+		} else {
+			// Non-CDN repository - use as-is
+			composerRepo.Baseurl = r.Baseurl
+			composerRepo.Metalink = r.Metalink
+			composerRepo.Rhsm = common.ToPtr(r.Rhsm)
+			composerRepo.Gpgkey = r.GpgKey
+		}


I'd even be up for extracting this into a separate function. This as a whole creates composerRepo.
This can have an early return after !r.Rhsm.

if !r.Rhsm { return &composer.Repository{ CheckGpg: r.CheckGpg, Baseurl: r.Baseurl, Metalink: r.Metalink, Rhsm: common.ToPtr(r.Rhsm), Gpgkey: r.GpgKey, }, nil } ...and then the rest

sure, I can do something like that

rverdile · 2025-10-29T20:07:49Z

Something else I noticed is this needs to fall back to the CDN when it's a repo we don't snapshot (like a RHEL minor release). I'll work on that too

rverdile · 2025-11-03T15:21:23Z

I have pushed changes based on the feedback, thanks all

avitova

I hahve only one suggestion but not a blocker. Looks nice, thanks!

avitova · 2025-11-05T11:52:17Z

internal/v1/handler_compose_image.go

+		if len(r.ImageTypeTags) == 0 || slices.Contains(r.ImageTypeTags, string(imageType)) {
+			if r.Rhsm {


Suggested change

if len(r.ImageTypeTags) == 0 || slices.Contains(r.ImageTypeTags, string(imageType)) {

if r.Rhsm {

if (len(r.ImageTypeTags) == 0 || slices.Contains(r.ImageTypeTags, string(imageType))) && r.Rhsm {

croissanne

Thanks! Let's keep a set of repositories in ibcrc then, but I wonder if it's not possible to clean up the code a little / reduce duplication of generating composer repositories from snapshots. (and maybe unit test that separately as well).

internal/v1/handler_compose_image.go

rverdile · 2025-11-07T19:54:39Z

I have pushed more changes based on the feedback, thanks all

croissanne

Thank you! The only thing i'd maybe add is also a 'why' in the commit message. Like what is the benefit of switching to snapshots for everything.

Other than that lgtm :)

Automatically transform CDN repository URLs to use latest snapshots from content sources API when no explicit SnapshotDate is provided. Non-CDN repos pass through unchanged. If a snapshot is not available, falls back to CDN Switching to latest snapshot aligns the method of authentication with the one used for other snapshots and templates, reducing some complexity

rverdile · 2025-11-26T14:15:16Z

Could someone merge this for me? I don't have merge access :)

lzap reviewed Oct 24, 2025

View reviewed changes

rverdile marked this pull request as ready for review October 28, 2025 13:53

rverdile requested a review from a team as a code owner October 28, 2025 13:53

rverdile requested review from avitova, ksiekl and regexowl October 28, 2025 13:53

avitova reviewed Oct 29, 2025

View reviewed changes

rverdile force-pushed the snapshots-rh branch from 50a478c to b21e1df Compare November 3, 2025 15:06

rverdile force-pushed the snapshots-rh branch from b21e1df to 0d04a01 Compare November 4, 2025 15:55

avitova reviewed Nov 5, 2025

View reviewed changes

croissanne requested changes Nov 5, 2025

View reviewed changes

internal/v1/handler_compose_image.go Show resolved Hide resolved

internal/v1/handler_compose_image.go Outdated Show resolved Hide resolved

rverdile force-pushed the snapshots-rh branch from 0d04a01 to 677be75 Compare November 7, 2025 19:50

rverdile requested review from avitova and croissanne November 12, 2025 14:36

croissanne previously approved these changes Nov 13, 2025

View reviewed changes

rverdile force-pushed the snapshots-rh branch from 677be75 to 7d6f0fa Compare November 13, 2025 15:25

rverdile dismissed croissanne’s stale review via 8de1ffc November 13, 2025 18:35

rverdile force-pushed the snapshots-rh branch from 7d6f0fa to 8de1ffc Compare November 13, 2025 18:35

rverdile requested a review from croissanne November 17, 2025 14:41

xbhouse mentioned this pull request Nov 20, 2025

Wizard: list all red hat and custom repos (HMS-8853) osbuild/image-builder-frontend#3823

Draft

rverdile force-pushed the snapshots-rh branch from 8de1ffc to 28e49ba Compare November 24, 2025 14:19

croissanne approved these changes Nov 24, 2025

View reviewed changes

kingsleyzissou approved these changes Nov 25, 2025

View reviewed changes

avitova added this pull request to the merge queue Nov 26, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 26, 2025

		if len(r.ImageTypeTags) == 0 \|\| slices.Contains(r.ImageTypeTags, string(imageType)) {
		if r.Rhsm {

	if len(r.ImageTypeTags) == 0 \|\| slices.Contains(r.ImageTypeTags, string(imageType)) {
	if r.Rhsm {
	if (len(r.ImageTypeTags) == 0 \|\| slices.Contains(r.ImageTypeTags, string(imageType))) && r.Rhsm {

internal/v1: use latest snapshots for CDN repos by default #1712

Are you sure you want to change the base?

internal/v1: use latest snapshots for CDN repos by default #1712

Conversation

rverdile commented Oct 23, 2025

Uh oh!

lzap Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rverdile Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

lzap commented Oct 24, 2025

Uh oh!

croissanne commented Oct 28, 2025

Uh oh!

rverdile commented Oct 29, 2025

Uh oh!

avitova Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

rverdile Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

rverdile commented Oct 29, 2025

Uh oh!

rverdile commented Nov 3, 2025

Uh oh!

avitova left a comment

Choose a reason for hiding this comment

Uh oh!

avitova Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

croissanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rverdile commented Nov 7, 2025

Uh oh!

croissanne left a comment

Choose a reason for hiding this comment

Uh oh!

rverdile commented Nov 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lzap Oct 24, 2025 •

edited

Loading