Skip to content

Suggest reorderings and comment improvements #278

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
msugakov merged 3 commits into from
Dec 4, 2025
Merged

Suggest reorderings and comment improvements #278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aad3ea0
Reorder functions from top-to-bottom reading
msugakov Dec 4, 2025
4546e10
Reorder types.go and update comments
msugakov Dec 4, 2025
95ded28
Unexport `ChannelLineage`
msugakov Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
214 changes: 107 additions & 107 deletions cmd/generate-catalog/generate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,88 @@ func readInputFile(filename string) (Configuration, error) {
}, nil
}

// getAllVersions extracts all operator versions from the input images.
func getAllVersions(images []BundleImage) []*semver.Version {
versions := make([]*semver.Version, 0, len(images))
for _, img := range images {
versions = append(versions, img.Version)
}
return versions
}

// validateVersionsAreSorted checks that the operator versions are sorted in ascending order and that there are no duplicates.
// The sorted order is important for the correct functioning of the rest of the program.
func validateVersionsAreSorted(versions []*semver.Version) error {
for i := 0; i < len(versions)-1; i++ {
currentVersion := versions[i]
nextVersion := versions[i+1]
if currentVersion.GreaterThanEqual(nextVersion) {
return fmt.Errorf("versions are not sorted in ascending order: %s is not less than %s", currentVersion, nextVersion)
}
}
return nil
}

func hasGapInVersions(versions []*semver.Version) error {
for i := 0; i < len(versions)-1; i++ {
var expectedNextVersion *semver.Version
currentVersion := versions[i]
nextVersion := versions[i+1]

if currentVersion.Major() != nextVersion.Major() {
expectedNextVersion = semver.New(currentVersion.Major()+1, 0, 0, "", "")
}
if currentVersion.Major() == nextVersion.Major() && currentVersion.Minor() != nextVersion.Minor() {
expectedNextVersion = semver.New(currentVersion.Major(), currentVersion.Minor()+1, 0, "", "")
}
if currentVersion.Major() == nextVersion.Major() && currentVersion.Minor() == nextVersion.Minor() {
expectedNextVersion = semver.New(currentVersion.Major(), currentVersion.Minor(), currentVersion.Patch()+1, "", "")
}

if expectedNextVersion.Major() != nextVersion.Major() || expectedNextVersion.Minor() != nextVersion.Minor() || expectedNextVersion.Patch() != nextVersion.Patch() {
return fmt.Errorf("unexpected version sequence [%s, %s]: %s should be followed by %s", currentVersion, nextVersion, currentVersion, expectedNextVersion)
}
}

return nil
}

// validateImageReferences checks that all images in the input bundle have valid container image references with a digest.
func validateImageReferences(images []BundleImage) error {
for _, img := range images {
if err := validateImageReference(img.Image); err != nil {
return err
}
}
return nil
}

// validateImageReference checks that the given image reference string is a valid container image reference and includes a registry, repository and digest.
// Also check that tag is not present. See tag related issue: https://redhat-internal.slack.com/archives/C031USXS2FJ/p1755792504667849?thread_ts=1755622785.895239&cid=C031USXS2FJ
func validateImageReference(imageRef string) error {
ref, err := reference.Parse(imageRef)
if err != nil {
return fmt.Errorf("cannot parse string as container image reference %s: %w", imageRef, err)
}

canonical, ok := ref.(reference.Canonical)
if !ok || canonical.Digest() == "" {
return fmt.Errorf("image reference %s does not include a digest", imageRef)
}
if canonical.Digest().Algorithm() != digest.SHA256 {
return fmt.Errorf("image reference %s digest algorithm is not sha256", imageRef)
}

if reference.Domain(canonical) == "" {
return fmt.Errorf("image reference %s needs the registry to be explicitly defined", imageRef)
}
if tagged, ok := ref.(reference.Tagged); ok && tagged.Tag() != "" {
return fmt.Errorf("image reference %s should not contain a tag", imageRef)
}

return nil
}

// generatePackageWithIcon creates a new "olm.package" object with an operator icon.
func generatePackageWithIcon() (Package, error) {
data, err := os.ReadFile(iconFile)
Expand All @@ -146,7 +228,7 @@ func generatePackageWithIcon() (Package, error) {
func generateChannels(versions []*semver.Version) []Channel {
latestLineage := newChannelLineage(latestChannelName, latestChannelFromVersion, latestChannelUntilVersion)
stableLineage := newChannelLineage(stableChannelName, stableChannelFromVersion, stableChannelUntilVersion)
lineages := []ChannelLineage{latestLineage, stableLineage}
lineages := []channelLineage{latestLineage, stableLineage}

emptyChannels := generateEmptyChannels(versions)
assignChannels(lineages, emptyChannels)
Expand Down Expand Up @@ -179,6 +261,17 @@ func generateEmptyChannels(versions []*semver.Version) []Channel {
return channels
}

// assignChannels assigns channels to the appropriate channel lineages based on their Y-Stream versions.
func assignChannels(lineages []channelLineage, channels []Channel) {
for _, ch := range channels {
for i := range lineages {
if versionBelongsToChannelLineage(ch.yStreamVersion, lineages[i]) {
lineages[i].YStreamChannels = append(lineages[i].YStreamChannels, ch)
}
}
}
}

// generateChannelEntries creates channel entries for each version, setting the appropriate `replaces` and `skipRange` fields.
func generateChannelEntries(versions []*semver.Version) []ChannelEntry {
channelEntries := make([]ChannelEntry, 0)
Expand All @@ -202,19 +295,8 @@ func generateChannelEntries(versions []*semver.Version) []ChannelEntry {
return channelEntries
}

// assignChannels assigns channels to the appropriate channel lineages based on their Y-Stream versions.
func assignChannels(lineages []ChannelLineage, channels []Channel) {
for _, ch := range channels {
for i := range lineages {
if versionBelongsToChannelLineage(ch.yStreamVersion, lineages[i]) {
lineages[i].YStreamChannels = append(lineages[i].YStreamChannels, ch)
}
}
}
}

// assignChannelEntries assigns channel entries to the appropriate channels within each lineage.
func assignChannelEntries(lineages []ChannelLineage, entries []ChannelEntry) {
func assignChannelEntries(lineages []channelLineage, entries []ChannelEntry) {
for _, entry := range entries {
for i := range lineages {
if versionBelongsToChannelLineage(entry.version, lineages[i]) {
Expand All @@ -229,8 +311,19 @@ func assignChannelEntries(lineages []ChannelLineage, entries []ChannelEntry) {
}
}

func versionBelongsToChannelLineage(version *semver.Version, lineage channelLineage) bool {
return lineage.FromVersion.LessThanEqual(version) && lineage.UntilVersion.GreaterThan(version)
}

func channelShouldHaveEntry(channel Channel, entry ChannelEntry) bool {
lesserX := entry.version.Major() < channel.yStreamVersion.Major()
sameXVersion := entry.version.Major() == channel.yStreamVersion.Major()
belongsToYStream := entry.version.Minor() <= channel.yStreamVersion.Minor()
return lesserX || (sameXVersion && belongsToYStream)
}

// flattenChannels flattens channels from multiple ChannelLineages into a single slice.
func flattenChannels(lineages []ChannelLineage) []Channel {
func flattenChannels(lineages []channelLineage) []Channel {
var channels []Channel
for _, lineage := range lineages {
channels = append(channels, lineage.YStreamChannels...)
Expand All @@ -248,13 +341,6 @@ func clearReplacesForStartingEntries(channels []Channel) {
}
}

func channelShouldHaveEntry(channel Channel, entry ChannelEntry) bool {
lesserX := entry.version.Major() < channel.yStreamVersion.Major()
sameXVersion := entry.version.Major() == channel.yStreamVersion.Major()
belongsToYStream := entry.version.Minor() <= channel.yStreamVersion.Minor()
return lesserX || (sameXVersion && belongsToYStream)
}

// generateDeprecations creates an object with a list of deprecations based on the provided versions.
func generateDeprecations(versions []*semver.Version, channels []Channel, oldestSupportedVersion *semver.Version) Deprecations {
var deprecations []DeprecationEntry
Expand Down Expand Up @@ -309,89 +395,3 @@ func writeToFile(filename string, ct CatalogTemplate) error {

return nil
}

// getAllVersions extracts all operator versions from the input images.
func getAllVersions(images []BundleImage) []*semver.Version {
versions := make([]*semver.Version, 0, len(images))
for _, img := range images {
versions = append(versions, img.Version)
}
return versions
}

// validateVersionsAreSorted checks that the operator versions are sorted in ascending order and that there are no duplicates.
// The sorted order is important for the correct functioning of the rest of the program.
func validateVersionsAreSorted(versions []*semver.Version) error {
for i := 0; i < len(versions)-1; i++ {
currentVersion := versions[i]
nextVersion := versions[i+1]
if currentVersion.GreaterThanEqual(nextVersion) {
return fmt.Errorf("versions are not sorted in ascending order: %s is not less than %s", currentVersion, nextVersion)
}
}
return nil
}

func hasGapInVersions(versions []*semver.Version) error {
for i := 0; i < len(versions)-1; i++ {
var expectedNextVersion *semver.Version
currentVersion := versions[i]
nextVersion := versions[i+1]

if currentVersion.Major() != nextVersion.Major() {
expectedNextVersion = semver.New(currentVersion.Major()+1, 0, 0, "", "")
}
if currentVersion.Major() == nextVersion.Major() && currentVersion.Minor() != nextVersion.Minor() {
expectedNextVersion = semver.New(currentVersion.Major(), currentVersion.Minor()+1, 0, "", "")
}
if currentVersion.Major() == nextVersion.Major() && currentVersion.Minor() == nextVersion.Minor() {
expectedNextVersion = semver.New(currentVersion.Major(), currentVersion.Minor(), currentVersion.Patch()+1, "", "")
}

if expectedNextVersion.Major() != nextVersion.Major() || expectedNextVersion.Minor() != nextVersion.Minor() || expectedNextVersion.Patch() != nextVersion.Patch() {
return fmt.Errorf("unexpected version sequence [%s, %s]: %s should be followed by %s", currentVersion, nextVersion, currentVersion, expectedNextVersion)
}
}

return nil
}

// validateImageReferences checks that all images in the input bundle have valid container image references with a digest.
func validateImageReferences(images []BundleImage) error {
for _, img := range images {
if err := validateImageReference(img.Image); err != nil {
return err
}
}
return nil
}

// validateImageReference checks that the given image reference string is a valid container image reference and includes a registry, repository and digest.
// Also check that tag is not present. See tag related issue: https://redhat-internal.slack.com/archives/C031USXS2FJ/p1755792504667849?thread_ts=1755622785.895239&cid=C031USXS2FJ
func validateImageReference(imageRef string) error {
ref, err := reference.Parse(imageRef)
if err != nil {
return fmt.Errorf("cannot parse string as container image reference %s: %w", imageRef, err)
}

canonical, ok := ref.(reference.Canonical)
if !ok || canonical.Digest() == "" {
return fmt.Errorf("image reference %s does not include a digest", imageRef)
}
if canonical.Digest().Algorithm() != digest.SHA256 {
return fmt.Errorf("image reference %s digest algorithm is not sha256", imageRef)
}

if reference.Domain(canonical) == "" {
return fmt.Errorf("image reference %s needs the registry to be explicitly defined", imageRef)
}
if tagged, ok := ref.(reference.Tagged); ok && tagged.Tag() != "" {
return fmt.Errorf("image reference %s should not contain a tag", imageRef)
}

return nil
}

func versionBelongsToChannelLineage(version *semver.Version, lineage ChannelLineage) bool {
return lineage.FromVersion.LessThanEqual(version) && lineage.UntilVersion.GreaterThan(version)
}
6 changes: 3 additions & 3 deletions cmd/generate-catalog/generate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ func TestGenerateBundles(t *testing.T) {
func TestAssignChannels(t *testing.T) {
latestLineage := newChannelLineage("latest", semver.MustParse("3.62.0"), semver.MustParse("4.0.0"))
stableLineage := newChannelLineage("stable", semver.MustParse("4.0.0"), semver.MustParse("9999.0.0"))
lineages := []ChannelLineage{latestLineage, stableLineage}
lineages := []channelLineage{latestLineage, stableLineage}

channels := []Channel{
{Name: "rhacs-3.62", yStreamVersion: semver.MustParse("3.62.0")},
Expand Down Expand Up @@ -487,7 +487,7 @@ func TestAssignChannelEntries(t *testing.T) {
{Name: "rhacs-operator.v5.0.1", Replaces: "rhacs-operator.v5.0.0", version: semver.MustParse("5.0.1")},
}

lineages := []ChannelLineage{latestLineage, stableLineage}
lineages := []channelLineage{latestLineage, stableLineage}
assignChannelEntries(lineages, entries)

latestLineage = lineages[0]
Expand Down Expand Up @@ -564,7 +564,7 @@ func TestFlattenChannels(t *testing.T) {
{Name: "rhacs-4.1"},
}

lineages := []ChannelLineage{latestLineage, stableLineage}
lineages := []channelLineage{latestLineage, stableLineage}
channels := flattenChannels(lineages)

// Should have 5 channels total: rhacs-3.62, latest, rhacs-4.0, rhacs-4.1, stable
Expand Down
Loading