From 8aee6a1ac1e6482f110179d3c70d9197772b377a Mon Sep 17 00:00:00 2001 From: Paul Gibert Date: Fri, 13 Dec 2024 10:40:52 -0500 Subject: [PATCH 1/6] Updated melange to fail if a duplicate package config name is encountered --- pkg/melange/melange.go | 13 ++++++++++--- pkg/melange/melange_test.go | 7 +++++++ pkg/melange/testdata/duplicates_dir/foo1.yaml | 3 +++ pkg/melange/testdata/duplicates_dir/foo2.yaml | 3 +++ 4 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 pkg/melange/testdata/duplicates_dir/foo1.yaml create mode 100644 pkg/melange/testdata/duplicates_dir/foo2.yaml diff --git a/pkg/melange/melange.go b/pkg/melange/melange.go index f149d1af..eb2eff1b 100644 --- a/pkg/melange/melange.go +++ b/pkg/melange/melange.go @@ -112,7 +112,7 @@ func ReadAllPackagesFromRepo(ctx context.Context, dir string) (map[string]*Packa // guarantee a consistent sort order for test comparisons sort.Strings(fileList) - + for _, fi := range fileList { data, err := os.ReadFile(fi) if err != nil { @@ -143,8 +143,15 @@ func ReadAllPackagesFromRepo(ctx context.Context, dir string) (map[string]*Packa if err != nil { return p, fmt.Errorf("failed to read package config %s: %w", fi, err) } - - p[packageConfig.Package.Name] = &Packages{ + + name := packageConfig.Package.Name + + // check that package config name is unique + _, exists := p[name] + if exists { + return p, fmt.Errorf("Package config names must be unique. Found duplicate '%s'", name) + } + p[name] = &Packages{ Config: *packageConfig, Filename: relativeFilename, Dir: dir, diff --git a/pkg/melange/melange_test.go b/pkg/melange/melange_test.go index 978e91e0..f9cef6f2 100644 --- a/pkg/melange/melange_test.go +++ b/pkg/melange/melange_test.go @@ -30,6 +30,13 @@ func TestMelange_readAllPackages(t *testing.T) { assert.Equal(t, 4, len(packages)) } +func TestMelange_readAllPackagesErrorOnDuplicate(t *testing.T) { + + ctx := context.Background() + _, err := ReadAllPackagesFromRepo(ctx, filepath.Join("testdata", "duplicates_dir")) + assert.Error(t, err, "Package config names must be unique. Found duplicate 'foo'") +} + func TestMelange_readPackageConfigForBar(t *testing.T) { ctx := context.Background() packages, err := ReadPackageConfigs(ctx, []string{"bar"}, filepath.Join("testdata", "melange_dir")) diff --git a/pkg/melange/testdata/duplicates_dir/foo1.yaml b/pkg/melange/testdata/duplicates_dir/foo1.yaml new file mode 100644 index 00000000..6d6fb81a --- /dev/null +++ b/pkg/melange/testdata/duplicates_dir/foo1.yaml @@ -0,0 +1,3 @@ +package: + name: foo + version: 1.2.3 diff --git a/pkg/melange/testdata/duplicates_dir/foo2.yaml b/pkg/melange/testdata/duplicates_dir/foo2.yaml new file mode 100644 index 00000000..6d6fb81a --- /dev/null +++ b/pkg/melange/testdata/duplicates_dir/foo2.yaml @@ -0,0 +1,3 @@ +package: + name: foo + version: 1.2.3 From e9a053c404b9a305032b8a0b99648e33063878be Mon Sep 17 00:00:00 2001 From: Paul Gibert Date: Fri, 13 Dec 2024 11:41:36 -0500 Subject: [PATCH 2/6] Updated duplicate test files with different versions --- pkg/melange/testdata/duplicates_dir/foo2.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/melange/testdata/duplicates_dir/foo2.yaml b/pkg/melange/testdata/duplicates_dir/foo2.yaml index 6d6fb81a..5b3a8e5e 100644 --- a/pkg/melange/testdata/duplicates_dir/foo2.yaml +++ b/pkg/melange/testdata/duplicates_dir/foo2.yaml @@ -1,3 +1,3 @@ package: name: foo - version: 1.2.3 + version: 1.2.4 From 6f6cab901e36c24ba1b2e79bc8ef1ca12056214f Mon Sep 17 00:00:00 2001 From: Paul Gibert Date: Fri, 13 Dec 2024 11:44:43 -0500 Subject: [PATCH 3/6] Update comment typo --- pkg/melange/melange.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/melange/melange.go b/pkg/melange/melange.go index eb2eff1b..5aeebe85 100644 --- a/pkg/melange/melange.go +++ b/pkg/melange/melange.go @@ -146,7 +146,7 @@ func ReadAllPackagesFromRepo(ctx context.Context, dir string) (map[string]*Packa name := packageConfig.Package.Name - // check that package config name is unique + // check that the package config name is unique _, exists := p[name] if exists { return p, fmt.Errorf("Package config names must be unique. Found duplicate '%s'", name) From c797060d69d1ffc4080784a5db76ac6e05ae6d0e Mon Sep 17 00:00:00 2001 From: Paul Gibert Date: Tue, 7 Jan 2025 16:28:27 -0500 Subject: [PATCH 4/6] Added file names to error messages and added check to ensure the package name matches the file name. --- pkg/melange/melange.go | 9 +++++++-- pkg/melange/melange_test.go | 5 ++--- .../testdata/duplicates_dir/{foo1.yaml => foo.yaml} | 0 3 files changed, 9 insertions(+), 5 deletions(-) rename pkg/melange/testdata/duplicates_dir/{foo1.yaml => foo.yaml} (100%) diff --git a/pkg/melange/melange.go b/pkg/melange/melange.go index 5aeebe85..c70c83c2 100644 --- a/pkg/melange/melange.go +++ b/pkg/melange/melange.go @@ -143,13 +143,18 @@ func ReadAllPackagesFromRepo(ctx context.Context, dir string) (map[string]*Packa if err != nil { return p, fmt.Errorf("failed to read package config %s: %w", fi, err) } - + + // check that the package name matches the file name name := packageConfig.Package.Name + fiBase := strings.TrimSuffix(filepath.Base(fi), filepath.Ext(fi)) + if name != fiBase { + return p, fmt.Errorf("package name does not match file name in '%s': '%s' != '%s'", fi, name, fiBase) + } // check that the package config name is unique _, exists := p[name] if exists { - return p, fmt.Errorf("Package config names must be unique. Found duplicate '%s'", name) + return p, fmt.Errorf("package config names must be unique. Found a package called '%s' in '%s' and '%s'", name, fi, p[name].Filename) } p[name] = &Packages{ Config: *packageConfig, diff --git a/pkg/melange/melange_test.go b/pkg/melange/melange_test.go index f9cef6f2..30bf956e 100644 --- a/pkg/melange/melange_test.go +++ b/pkg/melange/melange_test.go @@ -30,11 +30,10 @@ func TestMelange_readAllPackages(t *testing.T) { assert.Equal(t, 4, len(packages)) } -func TestMelange_readAllPackagesErrorOnDuplicate(t *testing.T) { - +func TestMelange_readAllPackagesErrorOnDuplicate(t *testing.T) { ctx := context.Background() _, err := ReadAllPackagesFromRepo(ctx, filepath.Join("testdata", "duplicates_dir")) - assert.Error(t, err, "Package config names must be unique. Found duplicate 'foo'") + assert.Error(t, err, "package name does not match file name in 'foo1.yaml': 'foo' != 'foo1") } func TestMelange_readPackageConfigForBar(t *testing.T) { diff --git a/pkg/melange/testdata/duplicates_dir/foo1.yaml b/pkg/melange/testdata/duplicates_dir/foo.yaml similarity index 100% rename from pkg/melange/testdata/duplicates_dir/foo1.yaml rename to pkg/melange/testdata/duplicates_dir/foo.yaml From cb7fa8417fc4e935f357827079f6fec28b7fbb51 Mon Sep 17 00:00:00 2001 From: Paul Gibert Date: Wed, 8 Jan 2025 12:38:03 -0500 Subject: [PATCH 5/6] Linted melange files --- pkg/melange/melange.go | 6 +++--- pkg/melange/melange_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/melange/melange.go b/pkg/melange/melange.go index c70c83c2..a3265f60 100644 --- a/pkg/melange/melange.go +++ b/pkg/melange/melange.go @@ -112,7 +112,7 @@ func ReadAllPackagesFromRepo(ctx context.Context, dir string) (map[string]*Packa // guarantee a consistent sort order for test comparisons sort.Strings(fileList) - + for _, fi := range fileList { data, err := os.ReadFile(fi) if err != nil { @@ -143,14 +143,14 @@ func ReadAllPackagesFromRepo(ctx context.Context, dir string) (map[string]*Packa if err != nil { return p, fmt.Errorf("failed to read package config %s: %w", fi, err) } - + // check that the package name matches the file name name := packageConfig.Package.Name fiBase := strings.TrimSuffix(filepath.Base(fi), filepath.Ext(fi)) if name != fiBase { return p, fmt.Errorf("package name does not match file name in '%s': '%s' != '%s'", fi, name, fiBase) } - + // check that the package config name is unique _, exists := p[name] if exists { diff --git a/pkg/melange/melange_test.go b/pkg/melange/melange_test.go index 30bf956e..0ba7cff6 100644 --- a/pkg/melange/melange_test.go +++ b/pkg/melange/melange_test.go @@ -30,7 +30,7 @@ func TestMelange_readAllPackages(t *testing.T) { assert.Equal(t, 4, len(packages)) } -func TestMelange_readAllPackagesErrorOnDuplicate(t *testing.T) { +func TestMelange_readAllPackagesErrorOnDuplicate(t *testing.T) { ctx := context.Background() _, err := ReadAllPackagesFromRepo(ctx, filepath.Join("testdata", "duplicates_dir")) assert.Error(t, err, "package name does not match file name in 'foo1.yaml': 'foo' != 'foo1") From 8a5519540b0f0f3c2e987e037422a88bd2b2e755 Mon Sep 17 00:00:00 2001 From: Paul Gibert Date: Wed, 8 Jan 2025 13:59:57 -0500 Subject: [PATCH 6/6] Fixed check-subpipeline-version-matches test --- pkg/lint/rules_test.go | 2 +- pkg/lint/testdata/files/check-subpipeline-version-matches.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/lint/rules_test.go b/pkg/lint/rules_test.go index d2cef71f..be9d4be1 100644 --- a/pkg/lint/rules_test.go +++ b/pkg/lint/rules_test.go @@ -328,7 +328,7 @@ func TestLinter_Rules(t *testing.T) { file: "check-subpipeline-version-matches.yaml", minSeverity: SeverityWarning, want: EvalResult{ - File: "check-version-matches", + File: "check-subpipeline-version-matches", Errors: EvalRuleErrors{ { Rule: Rule{ diff --git a/pkg/lint/testdata/files/check-subpipeline-version-matches.yaml b/pkg/lint/testdata/files/check-subpipeline-version-matches.yaml index da963c5b..2b8c3626 100644 --- a/pkg/lint/testdata/files/check-subpipeline-version-matches.yaml +++ b/pkg/lint/testdata/files/check-subpipeline-version-matches.yaml @@ -1,5 +1,5 @@ package: - name: check-version-matches + name: check-subpipeline-version-matches version: 0.9.0 epoch: 0 description: "a package with an out of date comment"