Fix some logic errors on IsDependency (#1627)

* Fix some logic errors on IsDependency Fix isdep backedge searching and Package searching without namespace. Update examples that are not updated to latest schemaa Signed-off-by: Jeff Mendoza <jlm@jlm.name> * Mark deps.dev tests as integration, as they use an external service. Increase timeout. Signed-off-by: Jeff Mendoza <jlm@jlm.name> --------- Signed-off-by: Jeff Mendoza <jlm@jlm.name>
guacsec · Jan 8, 2024 · 0b8fc18 · 0b8fc18
1 parent 5692dc6
commit 0b8fc18
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 47 deletions.
diff --git a/pkg/assembler/backends/keyvalue/isDependency.go b/pkg/assembler/backends/keyvalue/isDependency.go
@@ -177,27 +177,7 @@ func (c *demoClient) IsDependency(ctx context.Context, filter *model.IsDependenc
 			search = append(search, pkg.IsDependencyLinks...)
 		}
 	}
-	if !foundOne && filter != nil && filter.DependencyPackage != nil {
-		if filter.DependencyPackage.Version == nil { // FIXME this logic isn't exactly correct
-			exactPackage, err := c.exactPackageName(ctx, filter.DependencyPackage)
-			if err != nil {
-				return nil, gqlerror.Errorf("%v :: %v", funcName, err)
-			}
-			if exactPackage != nil {
-				search = append(search, exactPackage.IsDependencyLinks...)
-				foundOne = true
-			}
-		} else {
-			pkgs, err := c.findPackageVersion(ctx, filter.DependencyPackage)
-			if err != nil {
-				return nil, gqlerror.Errorf("%v :: %v", funcName, err)
-			}
-			foundOne = len(pkgs) > 0
-			for _, pkg := range pkgs {
-				search = append(search, pkg.IsDependencyLinks...)
-			}
-		}
-	}
+	// Dont search on DependencyPackage as it can be either package-name or package-version
 
 	var out []*model.IsDependency
 	if foundOne {
@@ -243,27 +223,19 @@ func (c *demoClient) buildIsDependency(ctx context.Context, link *isDependencyLi
 	var err error
 	if filter != nil {
 		p, err = c.buildPackageResponse(ctx, link.PackageID, filter.Package)
-		if err != nil {
-			return nil, err
-		}
 	} else {
 		p, err = c.buildPackageResponse(ctx, link.PackageID, nil)
-		if err != nil {
-			return nil, err
-		}
 	}
-	if filter != nil && filter.DependencyPackage != nil {
-		depPkgFilter := &model.PkgSpec{Type: filter.DependencyPackage.Type, Namespace: filter.DependencyPackage.Namespace,
-			Name: filter.DependencyPackage.Name}
-		dep, err = c.buildPackageResponse(ctx, link.DepPackageID, depPkgFilter)
-		if err != nil {
-			return nil, err
-		}
+	if err != nil {
+		return nil, err
+	}
+	if filter != nil {
+		dep, err = c.buildPackageResponse(ctx, link.DepPackageID, filter.DependencyPackage)
 	} else {
 		dep, err = c.buildPackageResponse(ctx, link.DepPackageID, nil)
-		if err != nil {
-			return nil, err
-		}
+	}
+	if err != nil {
+		return nil, err
 	}
 
 	// if package not found during ingestion or if ID is provided in filter, send error. On query do not send error to continue search

diff --git a/pkg/assembler/backends/keyvalue/pkg.go b/pkg/assembler/backends/keyvalue/pkg.go
@@ -943,7 +943,7 @@ func (c *demoClient) exactPackageName(ctx context.Context, filter *model.PkgSpec
 			return nil, nil
 		}
 	}
-	if filter.Type == nil || filter.Namespace != nil || filter.Name == nil {
+	if filter.Type == nil || filter.Namespace == nil || filter.Name == nil {
 		return nil, nil
 	}
 	inType := &pkgType{

diff --git a/pkg/assembler/graphql/examples/is_dependency.gql b/pkg/assembler/graphql/examples/is_dependency.gql
@@ -82,7 +82,7 @@ query IsDependencyQ5 {
 }
 
 query IsDependencyQ6 {
-  IsDependency(isDependencySpec: { dependentPackage: { name: "openssl" } }) {
+  IsDependency(isDependencySpec: { dependencyPackage: { name: "openssl" } }) {
     ...allIsDependencyTree
   }
 }
diff --git a/pkg/assembler/graphql/examples/packages.gql b/pkg/assembler/graphql/examples/packages.gql
@@ -119,8 +119,17 @@ query PkgQD {
   }
 }
 
+fragment allPkgIDs on PackageIDs {
+  packageTypeID
+  packageNamespaceID
+  packageNameID
+  packageVersionID
+}
+
 mutation PkgM1 {
-  ingestPackage(pkg: { type: "pypi", name: "tensorflow" })
+  ingestPackage(pkg: { type: "pypi", name: "tensorflow" }) {
+    ...allPkgIDs
+  }
 }
 
 mutation PkgM2 {
@@ -133,11 +142,15 @@ mutation PkgM2 {
         { key: "distro", value: "stretch" }
       ]
     }
-  )
+  ) {
+    ...allPkgIDs
+  }
 }
 
 mutation PkgM3 {
-  ingestPackage(pkg: { type: "pypi", name: "tensorflow", version: "2.12.0" })
+  ingestPackage(pkg: { type: "pypi", name: "tensorflow", version: "2.12.0" }) {
+    ...allPkgIDs
+  }
 }
 
 mutation PkgM4 {
@@ -151,7 +164,9 @@ mutation PkgM4 {
         { key: "distro", value: "stretch" }
       ]
     }
-  )
+  ) {
+    ...allPkgIDs
+  }
 }
 
 mutation PkgM5 {
@@ -165,7 +180,9 @@ mutation PkgM5 {
         { key: "arch", value: "amd64" }
       ]
     }
-  )
+  ) {
+    ...allPkgIDs
+  }
 }
 
 mutation PkgM6 {
@@ -180,7 +197,9 @@ mutation PkgM6 {
         { key: "arch", value: "amd64" }
       ]
     }
-  )
+  ) {
+    ...allPkgIDs
+  }
 }
 
 mutation PkgM7 {
@@ -195,5 +214,7 @@ mutation PkgM7 {
         { key: "arch", value: "amd64" }
       ]
     }
-  )
+  ) {
+    ...allPkgIDs
+  }
 }
diff --git a/pkg/handler/collector/deps_dev/deps_dev_test.go b/pkg/handler/collector/deps_dev/deps_dev_test.go
@@ -13,6 +13,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+//go:build integration
+
 package deps_dev
 
 import (
@@ -328,7 +330,7 @@ func TestPerformanceDepsCollector(t *testing.T) {
 			"pkg:golang/github.com/caarlos0/env/v6@v6.10.0",
 		},
 		poll:                 true,
-		interval:             time.Second * 3,
+		interval:             time.Second * 5,
 		wantErr:              false,
 		ignoreResultsForPerf: true,
 	}