guacsec / guac

GUAC aggregates software security metadata into a high fidelity graph database.
https://guac.sh
Apache License 2.0
1.27k stars 173 forks source link

[feature] Adding optional IDs as part of mutation API #1261

Closed lumjjb closed 7 months ago

lumjjb commented 1 year ago

From the discussion in slack around #594 , we discussed what the interface should be in creating links between multiple evidence tree nodes.

This brought up the question of how node inputs are done today, for example in IsDependency, PkgInputSpec is used to determine which pkg and depPkg to link.

This is usually implemented in the backend through making a look up to the node, and getting its ID to form the edge. Lookups during ingestion generally slow down the process (e.g. thus introducing the guacKey).

This proposal is to add an optional ID to each InputSpec so that, if provided the backend can use it to enable quick retrieval of the nodes that it needs to form edges around.

"Adds a dependency between two packages. The returned ID can be empty string."
  ingestDependency(
    pkg: PkgInputSpec!
    depPkg: PkgInputSpec!
    depPkgMatchType: MatchFlags!
    dependency: IsDependencyInputSpec!
  ): ID!

Would use the IDs provided in pkg and depPkg in:

"""
PkgInputSpec specifies a package for mutations.

This is different than PkgSpec because we want to encode mandatory fields:
type and name. All optional fields are given empty default values.
"""
input PkgInputSpec {
  ID: String               //Added optional field
  type: String!
  namespace: String = ""
  name: String!
  version: String = ""
  qualifiers: [PackageQualifierInputSpec!] = []
  subpath: String = ""
}
pxp928 commented 12 months ago

Based on the discussion in slack, the pkgInputSpec (and the other software tries: source and vulnerability) would need to contain all the IDs (for pkgType, pkgNamespace...etc) and not just the pkgVersion ID that the current pkgInputSpec returns.

So it would have to be:

input PkgInputSpec {
  typeID: ID               //Added optional field
  type: String!
  nameSpaceID: ID               //Added optional field
  namespace: String = ""
  nameID: ID               //Added optional field
  name: String!
  versionID: ID               //Added optional field
  version: String = ""
  qualifiers: [PackageQualifierInputSpec!] = []
  subpath: String = ""
}

similar for the other tries (source and vulnerability)

pxp928 commented 12 months ago

I will work on getting this completed to move forward with a more stable API for future releases

pxp928 commented 12 months ago

Based on discussions with @jeffmendoza and thinking about this more, we would need to have some sort of union between the inputSpec and the ID. As it should be either and not require both. For example, for this usecase: https://github.com/guacsec/guac/pull/1367#discussion_r1370722663 we would want it to be either or and not both.

pxp928 commented 11 months ago

Based on discussion and experimentation, a union type cannot be created but a new inputType can be created similar to:

input IDorPkgInputSpec {
  packageID: ID
  pkg: PkgInputSpec
}

where the mutations would change to:

extend type Mutation {
  "Ingests a new package and returns a corresponding package hierarchy containing only the IDs. The returned ID can be empty string."
  ingestPackage(pkg: IDorPkgInputSpec!): PackageIDs!
  "Bulk ingests packages and returns the list of corresponding package hierarchies containing only the IDs. The returned array of IDs can be empty strings."
  ingestPackages(pkgs: [IDorPkgInputSpec!]!): [PackageIDs!]!
}

in this case, either the ID or the pkgInputSpec would have to be specified. In the case of pkgID, the ID can either be the pkgName ID or the pkgVersion ID and it is up to the backend to determine which it is.

Similar considerations would have to be done for the other nouns.

pxp928 commented 7 months ago

closed via #1708