Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented a Delete functionality for KeyValue #2033

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions internal/testing/backend/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ var skipMatrix = map[string]map[string]bool{
"TestVEXBulkIngest": {arango: true, redis: true},
"TestFindSoftware": {redis: true, arango: true},
// remove these once its implemented for the other backends
"TestDeleteCertifyVuln": {arango: true, memmap: true, redis: true, tikv: true},
"TestDeleteHasSBOM": {arango: true, memmap: true, redis: true, tikv: true},
"TestDeleteHasSLSAs": {arango: true, memmap: true, redis: true, tikv: true},
"TestDeleteCertifyVuln": {arango: true},
"TestDeleteHasSBOM": {arango: true},
"TestDeleteHasSLSAs": {arango: true},
"TestQueryPackagesListForScan": {arango: true, redis: true, tikv: true},
}

Expand Down
4 changes: 4 additions & 0 deletions internal/testing/stablememmap/stablememmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ func (s *store) Set(ctx context.Context, c, k string, v any) error {
return s.mm.Set(ctx, c, k, v)
}

func (s *store) Remove(ctx context.Context, c, k string) error {
nathannaveen marked this conversation as resolved.
Show resolved Hide resolved
return s.mm.Remove(ctx, c, k)
}

func (s *store) Keys(c string) kv.Scanner {
return &scanner{mms: s.mm.Keys(c)}
}
Expand Down
49 changes: 49 additions & 0 deletions pkg/assembler/backends/keyvalue/certifyVuln.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,55 @@ func (n *certifyVulnerabilityLink) Key() string {
}, ":"))
}

// Helper function to remove vulnerability links. This works by setting all the links expect the specified linkID.
func (c *demoClient) removeLinks(ctx context.Context, linkID string, links []string, col string, id string) error {
var newLinks []string
for _, id := range links {
if id != linkID {
newLinks = append(newLinks, id)
}
}

// Update the entity in the KeyValue store
return c.kv.Set(ctx, col, id, newLinks)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't correct. It is replacing the vuln and package structs/nodes with a string array. You need to get / modify / and set those structs. Should probably use the setkv() helper (https://github.com/guacsec/guac/blob/main/pkg/assembler/backends/keyvalue/backend.go#L248)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this review! This review made me think deeply, and I realized that I was setting the key value wrong, so I have fixed that, as well as modifying the structs and not replacing them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still not right and doing the same thing.

}

// DeleteCertifyVuln deletes a specified certifyVuln node along with all associated relationships.
func (c *demoClient) DeleteCertifyVuln(ctx context.Context, id string) (bool, error) {
// Retrieve the certifyVulnerabilityLink by ID
link, err := byIDkv[*certifyVulnerabilityLink](ctx, id, c)
if err != nil {
if errors.Is(err, kv.NotFoundError) {
return false, nil // Not found, nothing to delete
}
return false, gqlerror.Errorf("failed to retrieve certifyVulnerabilityLink by ID: %v", err)
}

// Remove backlinks from associated package and vulnerability
foundPackage, err := c.returnFoundPkgVersion(ctx, &model.IDorPkgInput{PackageVersionID: &link.PackageID})
if err != nil {
return false, gqlerror.Errorf("failed to retrieve package version: %v", err)
}
if err := c.removeLinks(ctx, link.ThisID, foundPackage.CertifyVulnLinks, "packages", foundPackage.ID()); err != nil {
return false, gqlerror.Errorf("failed to remove package backlinks: %v", err)
}

foundVulnNode, err := c.returnFoundVulnerability(ctx, &model.IDorVulnerabilityInput{VulnerabilityNodeID: &link.VulnerabilityID})
if err != nil {
return false, gqlerror.Errorf("failed to retrieve vulnerability node: %v", err)
}
if err := c.removeLinks(ctx, link.ThisID, foundVulnNode.CertifyVulnLinks, "vulnerabilities", foundVulnNode.ID()); err != nil {
return false, gqlerror.Errorf("failed to remove vulnerability backlinks: %v", err)
}

// Delete the link from the KeyValue store
if err := c.kv.Remove(ctx, cVulnCol, link.Key()); err != nil {
return false, gqlerror.Errorf("failed to remove certifyVuln link from KeyValue store: %v", err)
}

return true, nil
}

func (n *certifyVulnerabilityLink) Neighbors(allowedEdges edgeMap) []string {
out := make([]string, 0, 2)
if allowedEdges[model.EdgeCertifyVulnPackage] {
Expand Down
53 changes: 53 additions & 0 deletions pkg/assembler/backends/keyvalue/hasSBOM.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,59 @@ func (n *hasSBOMStruct) Key() string {
}, ":"))
}

// DeleteHasSBOM deletes a specified hasSBOM node along with all associated relationships.
func (c *demoClient) DeleteHasSBOM(ctx context.Context, id string) (bool, error) {

// Retrieve the hasSBOM link by ID
link, err := byIDkv[*hasSBOMStruct](ctx, id, c)
if err != nil {
if errors.Is(err, kv.NotFoundError) {
return false, nil // Not found, nothing to delete
}
return false, gqlerror.Errorf("failed to retrieve hasSBOM link by ID: %v", err)
}

// Delete associated isDependency nodes
for _, depID := range link.IncludedDependencies {
if err := c.kv.Remove(ctx, "dependencies", depID); !errors.Is(err, kv.NotFoundError) {
return false, gqlerror.Errorf("failed to remove dependency node with ID %s: %v", depID, err)
}
}

// Delete associated isOccurrence nodes
for _, occurID := range link.IncludedOccurrences {
if err := c.kv.Remove(ctx, "occurrences", occurID); !errors.Is(err, kv.NotFoundError) {
return false, gqlerror.Errorf("failed to remove occurrence node with ID %s: %v", occurID, err)
}
}

// Remove backlinks from associated package or artifact
if link.Pkg != "" {
foundPkg, err := c.returnFoundPkgVersion(ctx, &model.IDorPkgInput{PackageVersionID: &link.Pkg})
if err != nil {
return false, gqlerror.Errorf("failed to retrieve package version: %v", err)
}
if err := c.removeLinks(ctx, link.ThisID, foundPkg.HasSBOMs, "packages", foundPkg.ID()); err != nil {
return false, gqlerror.Errorf("failed to remove package backlinks: %v", err)
}
} else if link.Artifact != "" {
foundArtifact, err := c.returnFoundArtifact(ctx, &model.IDorArtifactInput{ArtifactID: &link.Artifact})
if err != nil {
return false, gqlerror.Errorf("failed to retrieve artifact: %v", err)
}
if err := c.removeLinks(ctx, link.ThisID, foundArtifact.HasSBOMs, "artifacts", foundArtifact.ID()); err != nil {
return false, gqlerror.Errorf("failed to remove artifact backlinks: %v", err)
}
}

// Delete the hasSBOM link from the KeyValue store
if err := c.kv.Remove(ctx, hasSBOMCol, link.Key()); err != nil {
return false, gqlerror.Errorf("failed to remove hasSBOM link from KeyValue store: %v", err)
}

return true, nil
}

func (n *hasSBOMStruct) Neighbors(allowedEdges edgeMap) []string {
var out []string
if n.Pkg != "" && allowedEdges[model.EdgeHasSbomPackage] {
Expand Down
49 changes: 49 additions & 0 deletions pkg/assembler/backends/keyvalue/hasSLSA.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,55 @@ func (n *hasSLSAStruct) Key() string {
}, ":"))
}

// DeleteHasSLSA deletes a specified SLSA node along with all associated relationships.
func (c *demoClient) DeleteHasSLSA(ctx context.Context, id string) (bool, error) {

// Retrieve the SLSA link by ID
link, err := byIDkv[*hasSLSAStruct](ctx, id, c)
if err != nil {
if errors.Is(err, kv.NotFoundError) {
return false, nil // Not found, nothing to delete
}
return false, gqlerror.Errorf("Error retrieving SLSA link by ID: %v", err)
}

// Remove backlinks from associated subject
foundSubject, err := c.returnFoundArtifact(ctx, &model.IDorArtifactInput{ArtifactID: &link.Subject})
if err != nil {
return false, gqlerror.Errorf("Error retrieving associated subject: %v", err)
}
if err := c.removeLinks(ctx, link.ThisID, foundSubject.HasSLSAs, "artifacts", foundSubject.ID()); err != nil {
return false, gqlerror.Errorf("Error removing backlinks from associated subject: %v", err)
}

// Remove backlinks from associated builtBy
foundBuiltBy, err := c.returnFoundBuilder(ctx, &model.IDorBuilderInput{BuilderID: &link.BuiltBy})
if err != nil {
return false, gqlerror.Errorf("Error retrieving associated builtBy: %v", err)
}
if err := c.removeLinks(ctx, link.ThisID, foundBuiltBy.HasSLSAs, "builders", foundBuiltBy.ID()); err != nil {
return false, gqlerror.Errorf("Error removing backlinks from associated builtBy: %v", err)
}

// Remove backlinks from associated builtFrom
for _, builtFromID := range link.BuiltFrom {
foundBuiltFrom, err := c.returnFoundArtifact(ctx, &model.IDorArtifactInput{ArtifactID: &builtFromID})
if err != nil {
return false, gqlerror.Errorf("Error retrieving associated builtFrom: %v", err)
}
if err := c.removeLinks(ctx, link.ThisID, foundBuiltFrom.HasSLSAs, "artifacts", foundBuiltFrom.ID()); err != nil {
return false, gqlerror.Errorf("Error removing backlinks from associated builtFrom: %v", err)
}
}

// Delete the link from the KeyValue store
if err := c.kv.Remove(ctx, slsaCol, link.Key()); err != nil {
return false, gqlerror.Errorf("failed to remove hasSLSA link from KeyValue store: %v", err)
}

return true, nil
}

func (n *hasSLSAStruct) Neighbors(allowedEdges edgeMap) []string {
out := make([]string, 0, 2+len(n.BuiltFrom))
if allowedEdges[model.EdgeHasSlsaSubject] {
Expand Down
40 changes: 38 additions & 2 deletions pkg/assembler/backends/keyvalue/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package keyvalue
import (
"context"
"fmt"
"log"
"strings"

"github.com/vektah/gqlparser/v2/gqlerror"
Expand Down Expand Up @@ -192,6 +193,41 @@ func (c *demoClient) Nodes(ctx context.Context, ids []string) ([]model.Node, err

// Delete node and all associated relationships. This functionality is only implemented for
// certifyVuln, HasSBOM and HasSLSA.
func (c *demoClient) Delete(ctx context.Context, node string) (bool, error) {
panic(fmt.Errorf("not implemented: Delete"))
func (c *demoClient) Delete(ctx context.Context, nodeID string) (bool, error) {
// Retrieve the node type based on the node ID
var k string
if err := c.kv.Get(ctx, indexCol, nodeID, &k); err != nil {
return false, fmt.Errorf("%w : id not found in index %q", err, nodeID)
}

sub := strings.SplitN(k, ":", 2)
if len(sub) != 2 {
return false, fmt.Errorf("Bad value was stored in index map: %v", k)
}

nodeType := sub[0]

switch nodeType {
case "certifyVulns":
deleted, err := c.DeleteCertifyVuln(ctx, nodeID)
if err != nil {
return false, fmt.Errorf("failed to delete CertifyVuln via ID: %s, with error: %w", nodeID, err)
}
return deleted, nil
case "hasSBOMs":
deleted, err := c.DeleteHasSBOM(ctx, nodeID)
if err != nil {
return false, fmt.Errorf("failed to delete HasSBOM via ID: %s, with error: %w", nodeID, err)
}
return deleted, nil
case "hasSLSAs":
deleted, err := c.DeleteHasSLSA(ctx, nodeID)
if err != nil {
return false, fmt.Errorf("failed to delete HasSLSA via ID: %s, with error: %w", nodeID, err)
}
return deleted, nil
default:
log.Printf("Unknown node type: %s", nodeType)
nathannaveen marked this conversation as resolved.
Show resolved Hide resolved
}
return false, nil
}
4 changes: 3 additions & 1 deletion pkg/assembler/kv/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@ import (

// Store is an interface to define to serve as a keyvalue store
type Store interface {

// Retrieve value from store. If not found, returns NotFoundError. Ptr must
// be a pointer to the type of value stored.
Get(ctx context.Context, collection, key string, ptr any) error

// Sets a value, creates collection if necessary
Set(ctx context.Context, collection, key string, value any) error

// Delete a value from the store. If not found, returns NotFoundError.
Remove(ctx context.Context, collection, key string) error

// Create a scanner that will be used to get all the keys in a collection.
Keys(collection string) Scanner
}
Expand Down
12 changes: 12 additions & 0 deletions pkg/assembler/kv/memmap/memmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,18 @@ func (s *store) Set(_ context.Context, c, k string, v any) error {
return nil
}

func (s *store) Remove(_ context.Context, c, k string) error {
col, ok := s.m[c]
if !ok {
return fmt.Errorf("%w : Collection %q", kv.NotFoundError, c)
}
if _, ok := col[k]; !ok {
return fmt.Errorf("%w : Key %q", kv.NotFoundError, k)
}
delete(col, k)
return nil
}

func (s *store) Keys(c string) kv.Scanner {
return &scanner{
collection: c,
Expand Down
11 changes: 11 additions & 0 deletions pkg/assembler/kv/redis/redis.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,17 @@ func (s *store) Set(ctx context.Context, c, k string, v any) error {
return s.c.HSet(ctx, c, k, string(b)).Err()
}

func (s *store) Remove(ctx context.Context, c, k string) error {
res, err := s.c.HDel(ctx, c, k).Result()
if err != nil {
return err
}
if res == 0 {
return kv.NotFoundError
}
return nil
}

func (s *store) Keys(c string) kv.Scanner {
return &scanner{
collection: c,
Expand Down
18 changes: 18 additions & 0 deletions pkg/assembler/kv/tikv/tikv.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,24 @@ func (s *store) Set(ctx context.Context, c, k string, v any) error {
return s.c.Put(ctx, []byte(ck), bts)
}

func (s *store) Remove(ctx context.Context, c, k string) error {
ck := strings.Join([]string{c, k}, ":")
// Check if the key exists
bts, err := s.c.Get(ctx, []byte(ck))
if err != nil {
return err
}
if len(bts) == 0 {
return kv.NotFoundError
}
// Proceed to delete the key
err = s.c.Delete(ctx, []byte(ck))
if err != nil {
return err
}
return nil
}

func (s *store) Keys(c string) kv.Scanner {
return &scanner{
c: s.c,
Expand Down
Loading