feat: narrow the special handling of github to just latest version (#182)

Author: CodeReaper

internal/generator/factory.go

@@ -8,14 +8,10 @@ import (
 	"regexp"
 	"runtime"
 	"slices"
-	"sort"
-	"strconv"
-	"strings"
 
 	"github.com/CustomResourceDefinition/catalog/internal/configuration"
 	"github.com/CustomResourceDefinition/catalog/internal/crd"
 	"github.com/CustomResourceDefinition/catalog/internal/registry"
-	"github.com/CustomResourceDefinition/catalog/internal/semver"
 )
 
 type Builder struct {
@@ -25,34 +21,24 @@ type Builder struct {
 	logger               io.Writer
 	config               configuration.Configuration
 	generator            Generator
-	versionFilter        *regexp.Regexp
 	registry             *registry.SourceRegistry
 }
 
-func NewBuilder(config configuration.Configuration, reader crd.CrdReader, generatedRepository, schemaRepository, definitionRepository string, logger io.Writer, reg *registry.SourceRegistry) (*Builder, error) {
+func NewBuilder(
+	config configuration.Configuration,
+	reader crd.CrdReader,
+	generatedRepository, schemaRepository, definitionRepository string,
+	logger io.Writer,
+	reg *registry.SourceRegistry,
+) (*Builder, error) {
 	generator, err := resolveGenerator(config, reader, logger)
 	if err != nil {
 		return nil, err
 	}
 
-	if len(config.Namespace) == 0 {
-		config.Namespace = "namespace"
-	}
-
-	pattern := defaultVersionPattern(config.Kind)
-	if len(config.VersionPattern) > 0 {
-		pattern = config.VersionPattern
-	}
-
-	re, err := regexp.Compile(pattern)
-	if err != nil {
-		return nil, err
-	}
-
 	return &Builder{
 		config:               config,
 		generator:            generator,
-		versionFilter:        re,
 		logger:               logger,
 		schemaRepository:     schemaRepository,
 		generatedRepository:  generatedRepository,
@@ -78,13 +64,20 @@ func (builder Builder) Build() error {
 	fmt.Fprintf(logger, "Producing for %s@%s:\n", builder.config.Name, builder.config.Kind)
 	defer fmt.Fprintf(logger, "End.\n")
 
-	latestVersion, isUpdated := builder.registryStatus()
+	if _, ok := builder.generator.(*PreparedGitGenerator); ok {
+		fmt.Fprintf(logger, " - using prepared git generator\n")
+	}
+
+	latestVersion, isUpdated, err := builder.registryStatus()
+	if err != nil {
+		return err
+	}
 	if isUpdated {
 		fmt.Fprintf(logger, " - skipping %s@%s (version %s unchanged)\n", builder.config.Name, builder.config.Kind, latestVersion)
 		return nil
 	}
 
-	versions, err := builder.versions()
+	versions, err := builder.generator.Versions()
 	if err != nil {
 		return err
 	}
@@ -157,51 +150,23 @@ func (builder Builder) Build() error {
 	return nil
 }
 
-func (builder Builder) versions() ([]string, error) {
-	versions, err := builder.generator.Versions()
+// registryStatus reports on the state in registry based on the latest version
+// available and only uses the decided interface method for latest version information
+func (builder Builder) registryStatus() (string, bool, error) {
+	version, err := builder.generator.LatestVersion()
 	if err != nil {
-		return nil, err
+		return "", false, fmt.Errorf("unable to check registry: %w", err)
 	}
 
-	filtered := make([]string, 0)
-	for _, v := range versions {
-		if builder.versionFilter.MatchString(v) {
-			filtered = append(filtered, v)
-		}
-	}
-
-	sort.Slice(filtered, func(i, j int) bool {
-		keyA, errA := builder.generator.VersionSortKey(filtered[i])
-		keyB, errB := builder.generator.VersionSortKey(filtered[j])
-
-		if errA == nil && errB == nil && keyA != 0 && keyB != 0 && keyA != keyB {
-			return keyA > keyB
-		}
-
-		a := normalizeVersion(builder.versionFilter.FindAllStringSubmatch(filtered[i], -1))
-		b := normalizeVersion(builder.versionFilter.FindAllStringSubmatch(filtered[j], -1))
-		return semver.Compare(a, b) > 0
-	})
-	return filtered, nil
-}
-
-func (builder Builder) registryStatus() (string, bool) {
-	versions, err := builder.versions()
-	if err != nil || len(versions) == 0 {
-		return "", false
-	}
-
-	version := versions[0]
-
 	if builder.registry == nil {
-		return version, false
+		return version, false, nil
 	}
 
 	if entry, ok := builder.registry.Get(builder.config.Name); ok {
-		return version, entry.Kind == string(builder.config.Kind) && entry.Version == version
+		return version, entry.Kind == string(builder.config.Kind) && entry.Version == version, nil
 	}
 
-	return version, false
+	return version, false, nil
 }
 
 func (builder Builder) updateRegistry(version string) {
@@ -212,37 +177,31 @@ func (builder Builder) updateRegistry(version string) {
 	builder.registry.Set(builder.config.Name, string(builder.config.Kind), version)
 }
 
-func normalizeVersion(matches [][]string) string {
-	if len(matches) == 0 || len(matches[0]) < 2 {
-		return "v0.0.0"
+func resolveGenerator(config configuration.Configuration, reader crd.CrdReader, logger io.Writer) (Generator, error) {
+	if len(config.Namespace) == 0 {
+		config.Namespace = "namespace"
 	}
 
-	version := matches[0][1]
-	parts := strings.Split(version, ".")
-	if len(parts) < 3 {
-		return "v0.0.0"
+	pattern := defaultVersionPattern(config.Kind)
+	if len(config.VersionPattern) > 0 {
+		pattern = config.VersionPattern
 	}
 
-	ints := make([]int, 3)
-	for i := range 3 {
-		n, _ := strconv.Atoi(parts[i])
-		ints[i] = n
+	filter, err := regexp.Compile(pattern)
+	if err != nil {
+		return nil, err
 	}
 
-	return fmt.Sprintf("v%d.%d.%d", ints[0], ints[1], ints[2])
-}
-
-func resolveGenerator(config configuration.Configuration, reader crd.CrdReader, logger io.Writer) (Generator, error) {
 	switch config.Kind {
 	case configuration.Git:
-		return NewGitGeneratorFactory(config, reader, logger).Build()
+		return NewGitGeneratorFactory(config, reader, filter, logger).Build()
 	case configuration.Http:
-		return NewHttpGenerator(config, reader), nil
+		return NewHttpGenerator(config, reader, filter), nil
 	case configuration.Helm:
 		target := config.Entries[len(config.Entries)-1]
-		return NewHelmGenerator(target, config, reader), nil
+		return NewHelmGenerator(target, config, reader, filter), nil
 	case configuration.HelmOci:
-		return NewOciGenerator(config, reader), nil
+		return NewOciGenerator(config, reader, filter), nil
 	default:
 		return nil, fmt.Errorf("no generators matched for kind '%s'", config.Kind)
 	}

internal/generator/factory_test.go

@@ -1,7 +1,6 @@
 package generator
 
 import (
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -14,107 +13,37 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestBuilderVersionSorting(t *testing.T) {
-	seedVersions := []string{
-		"2.10.0", "1.0.0", "2.2.0", "2.1.0", "1.01.01",
-	}
-	bundles := make([]gitBundle, 0)
-	for _, v := range seedVersions {
-		bundles = append(bundles, gitBundle{tag: v, paths: []gitPath{}})
+func TestBuildWithVersionPatternFiltering(t *testing.T) {
+	config := configuration.Configuration{
+		Kind:           configuration.Http,
+		Name:           "test",
+		ApiGroups:      []string{"chart.uri"},
+		VersionPattern: `^v([0-9]+\.[0-9]+\.[0-9]+)$`,
+		Downloads: []configuration.ConfigurationDownload{
+			{
+				Version: "v1.0.0",
+			},
+			{
+				Version: "v2.0.0",
+			},
+			{
+				Version: "3.0.0",
+			},
+		},
 	}
-	expectedVersions := []string{seedVersions[0], seedVersions[2], seedVersions[3], seedVersions[4], seedVersions[1]}
 
-	p, err := setupGit(t, bundles)
+	reader, err := crd.NewCrdReader(setupLogger())
 	assert.Nil(t, err)
-	assert.NotNil(t, p)
 
-	config := configuration.Configuration{
-		Kind:           configuration.Git,
-		Repository:     fmt.Sprintf("file://%s", *p),
-		VersionPattern: `^([0-9]+\.[0-9]+\.[0-9]+)$`,
-	}
+	tmpDir := t.TempDir()
 
-	b, err := NewBuilder(config, nil, "-", "-", "-", nil, nil)
+	builder, err := NewBuilder(config, reader, tmpDir, tmpDir, tmpDir, setupLogger(), nil)
 	assert.Nil(t, err)
 
-	versions, err := b.versions()
+	version, result, err := builder.registryStatus()
 	assert.Nil(t, err)
-	assert.NotNil(t, versions)
-	assert.Equal(t, expectedVersions, versions)
-}
-
-type testScenario struct {
-	versions         []string
-	expectedVersions []string
-	pattern          string
-}
-
-func TestBuilderVersionFiltering(t *testing.T) {
-	tests := []testScenario{
-		{
-			versions:         []string{"2.0.0", "1.3.0", "1.0.0"},
-			expectedVersions: []string{"2.0.0", "1.3.0", "1.0.0"},
-		},
-		{
-			versions:         []string{"v2.0.0", "v1.3.0", "v1.0.0"},
-			expectedVersions: []string{},
-		},
-		{
-			versions:         []string{"v2.0.0", "v1.3.0", "v1.0.0"},
-			expectedVersions: []string{"v2.0.0", "v1.3.0", "v1.0.0"},
-			pattern:          `^v([0-9]+\.[0-9]+\.[0-9]+)$`,
-		},
-		{
-			versions:         []string{"2.0.0", "v1.3.0", "v1.0.0"},
-			expectedVersions: []string{"2.0.0", "v1.3.0", "v1.0.0"},
-			pattern:          `^v?([0-9]+\.[0-9]+\.[0-9]+)$`,
-		},
-		{
-			versions:         []string{"2.0.0", "v1.3.0v", "v1.0.0"},
-			expectedVersions: []string{"2.0.0", "v1.0.0"},
-			pattern:          `^v?([0-9]+\.[0-9]+\.[0-9]+)$`,
-		},
-		{
-			versions:         []string{"2.0.0-2", "1.3.0-1892", "1.0.0-01"},
-			expectedVersions: []string{"2.0.0-2", "1.3.0-1892", "1.0.0-01"},
-			pattern:          `^([0-9]+\.[0-9]+\.[0-9]+-\d+)$`,
-		},
-		{
-			versions:         []string{"v1.33.2+k0s.0"},
-			expectedVersions: []string{"v1.33.2+k0s.0"},
-			pattern:          `^v([0-9]+\.[0-9]+\.[0-9]+\+k0s\.0)$`,
-		},
-		{
-			versions:         []string{"main", "v1.0", "master"},
-			expectedVersions: []string{"main", "master"},
-			pattern:          `^(main|master)$`,
-		},
-		{
-			versions:         []string{"main", "v1.0.0", "2.0.0", "master"},
-			expectedVersions: []string{"2.0.0", "main", "master"},
-			pattern:          `^([0-9]+\.[0-9]+\.[0-9]+)|(main|master)$`,
-		},
-	}
-
-	for i, test := range tests {
-		downloads := make([]configuration.ConfigurationDownload, 0)
-		for _, v := range test.versions {
-			downloads = append(downloads, configuration.ConfigurationDownload{Version: v})
-		}
-		config := configuration.Configuration{
-			Kind:           configuration.Http,
-			Downloads:      downloads,
-			VersionPattern: test.pattern,
-		}
-
-		b, err := NewBuilder(config, nil, "-", "-", "-", nil, nil)
-		assert.Nil(t, err)
-
-		versions, err := b.versions()
-		assert.Nil(t, err, "index %d failed", i)
-		assert.NotNil(t, versions, "index %d failed", i)
-		assert.Equal(t, test.expectedVersions, versions, "index %d failed", i)
-	}
+	assert.False(t, result)
+	assert.Equal(t, "v2.0.0", version)
 }
 
 func TestResolveGenerator(t *testing.T) {
@@ -241,11 +170,12 @@ func TestRegistryStatusNoRegistry(t *testing.T) {
 		Downloads: []configuration.ConfigurationDownload{{Version: "1.0.0"}},
 	}
 
-	b, err := NewBuilder(config, nil, "-", "-", "-", nil, nil)
+	builder, err := NewBuilder(config, nil, "-", "-", "-", nil, nil)
 	assert.Nil(t, err)
-	assert.NotNil(t, b)
+	assert.NotNil(t, builder)
 
-	version, result := b.registryStatus()
+	version, result, err := builder.registryStatus()
+	assert.Nil(t, err)
 	assert.False(t, result)
 	assert.Equal(t, "1.0.0", version)
 }
@@ -283,7 +213,8 @@ func TestRegistryStatusSameVersion(t *testing.T) {
 	builder, err := NewBuilder(config, reader, tmpDir, tmpDir, tmpDir, setupLogger(), reg)
 	assert.Nil(t, err)
 
-	version, result := builder.registryStatus()
+	version, result, err := builder.registryStatus()
+	assert.Nil(t, err)
 	assert.True(t, result)
 	assert.Equal(t, "1.0.0", version)
 }
@@ -321,7 +252,8 @@ func TestRegistryStatusDifferentVersion(t *testing.T) {
 	builder, err := NewBuilder(config, reader, tmpDir, tmpDir, tmpDir, setupLogger(), reg)
 	assert.Nil(t, err)
 
-	version, result := builder.registryStatus()
+	version, result, err := builder.registryStatus()
+	assert.Nil(t, err)
 	assert.False(t, result)
 	assert.Equal(t, "2.0.0", version)
 }
@@ -359,7 +291,8 @@ func TestRegistryStatusDifferentKind(t *testing.T) {
 	builder, err := NewBuilder(config, reader, tmpDir, tmpDir, tmpDir, setupLogger(), reg)
 	assert.Nil(t, err)
 
-	version, result := builder.registryStatus()
+	version, result, err := builder.registryStatus()
+	assert.Nil(t, err)
 	assert.False(t, result)
 	assert.Equal(t, "1.0.0", version)
 }

internal/generator/generator.go

@@ -8,8 +8,8 @@ import (
 
 type Generator interface {
 	Versions() ([]string, error)
+	LatestVersion() (string, error)
 	MetaData(version string) ([]crd.CrdMetaSchema, error)
 	Crds(version string) ([]crd.Crd, error)
-	VersionSortKey(version string) (int64, error)
 	io.Closer
 }