Lint error when dependency with same is defined multiple times (#3068)

* Add lint for dependencies with same name Having multiple dependencies with the same name results in undefined behaviour. This change introduces a lint to ensure that no dependencies share the same name. Signed-off-by: Kim Christensen <[email protected]> --------- Signed-off-by: Kim Christensen <[email protected]>
getporter · Apr 8, 2024 · e7fdd77 · e7fdd77
1 parent 5beb467
commit e7fdd77
Show file tree

Hide file tree

Showing 6 changed files with 145 additions and 4 deletions.
diff --git a/docs/content/docs/references/linter.md b/docs/content/docs/references/linter.md
@@ -30,3 +30,14 @@ Using a reserved prefix can be problematic as it can overwrite a predefined para
 To fix the problem indicated by the porter-100 error, you should replace the prefix of any newly defined parameters to not start with "porter".
 
 You can find more information about parameters in following URL: https://porter.sh/quickstart/parameters/.
+
+## porter-102
+
+The porter-102 error is a message generated by the porter lint command when it detects that multiple dependencies are defined with the same
+name.
+
+Multiple dependencies with the same name results in undefined behaviour.
+
+To fix the problem, you should name ensure all dependencies have different names.
+
+You can find more information about dependencies in [Dependencies](/docs/development/authoring-a-bundle/working-with-dependencies/).
diff --git a/go.sum b/go.sum
@@ -43,8 +43,6 @@ cloud.google.com/go/storage v1.8.0/go.mod h1:Wv1Oy7z6Yz3DshWRJFhqM/UCfaWIRTdp0RX
 cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9ullr3+Kg0=
 cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3fOKtUw0Xmo=
 dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
-get.porter.sh/magefiles v0.6.2 h1:kHuurvykssL6iZsE5S2XLtTsps79h9D4rrxDSCiHOU8=
-get.porter.sh/magefiles v0.6.2/go.mod h1:YsSlQWtGoXCGC4pdD7NxPpvh/FryM1bM0wzMWlJC+Bg=
 get.porter.sh/magefiles v0.6.3 h1:OE9YqCArn9fvM+VdanGlXNyIjy2F8W4yJGy5kcC/xD0=
 get.porter.sh/magefiles v0.6.3/go.mod h1:w37oTKICvvaEKR5KVB9UfN2EX30uYO9Qk0oRoz80DOU=
 github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24 h1:bvDV9vkmnHYOMsOr4WLk+Vo07yKIzd94sVoIqshQ4bU=
@@ -565,8 +563,6 @@ github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+o
 github.com/klauspost/compress v1.4.1/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A=
 github.com/klauspost/compress v1.11.4/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
 github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk=
-github.com/klauspost/compress v1.17.6 h1:60eq2E/jlfwQXtvZEeBUYADs+BwKBWURIY+Gj2eRGjI=
-github.com/klauspost/compress v1.17.6/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM=
 github.com/klauspost/compress v1.17.7 h1:ehO88t2UGzQK66LMdE8tibEd1ErmzZjNEqWkjLAKQQg=
 github.com/klauspost/compress v1.17.7/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw=
 github.com/klauspost/cpuid v1.2.0/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek=

diff --git a/pkg/linter/linter.go b/pkg/linter/linter.go
@@ -186,6 +186,28 @@ func (l *Linter) Lint(ctx context.Context, m *manifest.Manifest) (Results, error
 	ctx, span := tracing.StartSpan(ctx)
 	defer span.EndSpan()
 
+	deps := make(map[string]interface{}, len(m.Dependencies.Requires))
+	for _, dep := range m.Dependencies.Requires {
+		if _, exists := deps[dep.Name]; exists {
+			res := Result{
+				Level: LevelError,
+				Location: Location{
+					Action:          "",
+					Mixin:           "",
+					StepNumber:      0,
+					StepDescription: "",
+				},
+				Code:    "porter-102",
+				Title:   "Dependency error",
+				Message: fmt.Sprintf("The dependency %s is defined multiple times", dep.Name),
+				URL:     "https://porter.sh/reference/linter/#porter-102",
+			}
+			results = append(results, res)
+		} else {
+			deps[dep.Name] = nil
+		}
+	}
+
 	span.Debug("Running linters for each mixin used in the manifest...")
 	q := query.New(l.Context, l.Mixins)
 	responses, err := q.Execute(ctx, "lint", query.NewManifestGenerator(m))

diff --git a/pkg/linter/linter_test.go b/pkg/linter/linter_test.go
@@ -173,3 +173,63 @@ func TestLinter_Lint(t *testing.T) {
 		require.NotContains(t, results[0].String(), ": 0th step in the mixin ()")
 	})
 }
+
+func TestLinter_DependencyMultipleTimes(t *testing.T) {
+	t.Run("dependency defined multiple times", func(t *testing.T) {
+		cxt := portercontext.NewTestContext(t)
+		mixins := mixin.NewTestMixinProvider()
+		l := New(cxt.Context, mixins)
+
+		m := &manifest.Manifest{
+			Dependencies: manifest.Dependencies{
+				Requires: []*manifest.Dependency{
+					{Name: "mysql"},
+					{Name: "mysql"},
+				},
+			},
+		}
+
+		expectedResult := Results{
+			{
+				Code:    "porter-102",
+				Title:   "Dependency error",
+				Message: "The dependency mysql is defined multiple times",
+				URL:     "https://porter.sh/reference/linter/#porter-102",
+			},
+		}
+
+		results, err := l.Lint(context.Background(), m)
+		require.NoError(t, err, "Lint failed")
+		require.Len(t, results, 1, "linter should have returned 1 result")
+		require.Equal(t, expectedResult, results, "unexpected lint results")
+	})
+	t.Run("no dependency defined multiple times", func(t *testing.T) {
+		cxt := portercontext.NewTestContext(t)
+		mixins := mixin.NewTestMixinProvider()
+		l := New(cxt.Context, mixins)
+
+		m := &manifest.Manifest{
+			Dependencies: manifest.Dependencies{
+				Requires: []*manifest.Dependency{
+					{Name: "mysql"},
+					{Name: "mongo"},
+				},
+			},
+		}
+
+		results, err := l.Lint(context.Background(), m)
+		require.NoError(t, err, "Lint failed")
+		require.Len(t, results, 0, "linter should have returned 0 result")
+	})
+	t.Run("no dependencies", func(t *testing.T) {
+		cxt := portercontext.NewTestContext(t)
+		mixins := mixin.NewTestMixinProvider()
+		l := New(cxt.Context, mixins)
+
+		m := &manifest.Manifest{}
+
+		results, err := l.Lint(context.Background(), m)
+		require.NoError(t, err, "Lint failed")
+		require.Len(t, results, 0, "linter should have returned 0 result")
+	})
+}
diff --git a/tests/integration/lint_test.go b/tests/integration/lint_test.go
@@ -36,3 +36,15 @@ func TestLint(t *testing.T) {
 	require.NotContains(t, output, "unknown command", "an unsupported mixin command should not be printed to the console in info")
 
 }
+
+func TestLint_DependenciesSameName(t *testing.T) {
+	test, err := tester.NewTest(t)
+	defer test.Close()
+	require.NoError(t, err, "test setup failed")
+
+	_, output, _ := test.RunPorterWith(func(cmd *shx.PreparedCommand) {
+		cmd.Args("lint")
+		cmd.In(filepath.Join(test.RepoRoot, "tests/integration/testdata/bundles/bundle-with-samenamedeps-lint-error"))
+	})
+	require.Contains(t, output, "error(porter-102) - Dependency error", "multiple dependencies with the same name should be an error")
+}
diff --git a/tests/integration/testdata/bundles/bundle-with-samenamedeps-lint-error/porter.yaml b/tests/integration/testdata/bundles/bundle-with-samenamedeps-lint-error/porter.yaml
@@ -0,0 +1,40 @@
+# This bundle is designed to cause the porter lint/build commands to fail
+schemaType: Bundle
+schemaVersion: 1.0.1
+name: exec-mixin-lint-error
+version: 0.1.0
+description: "This bundle is designed to cause the porter lint/build commands to fail, use --no-lint to use it anyway"
+registry: "localhost:5000"
+
+mixins:
+  - exec
+
+dependencies:
+  requires:
+    - bundle:
+        reference: ghcr.io/getporter/porter-hello:v0.2.0
+      name: samename
+    - bundle:
+        reference: ghcr.io/getporter/porter-hello:v0.2.0
+      name: samename
+
+install:
+  - exec:
+      description: trigger a lint error
+      command: echo
+      arguments:
+        - install
+
+upgrade:
+  - exec:
+      description: "World 2.0"
+      command: echo
+      arguments:
+        - upgrade
+
+uninstall:
+  - exec:
+      description: "Uninstall Hello World"
+      command: echo
+      arguments:
+        - uninstall