Don't error out on secret create --replace on missing secret

pkg/secrets/define/secrets.go

@@ -0,0 +1,16 @@
+package define
+
+import (
+	"errors"
+)
+
+var (
+	// ErrNoSuchSecret indicates that the secret does not exist
+	ErrNoSuchSecret = errors.New("no such secret")
+
+	// ErrSecretIDExists indicates that there is secret data already associated with an id
+	ErrSecretIDExists = errors.New("secret data with ID already exists")
+
+	// ErrInvalidKey indicates that something about your key is wrong
+	ErrInvalidKey = errors.New("invalid key")
+)

pkg/secrets/filedriver/filedriver.go

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"sort"
 
+	"github.com/containers/common/pkg/secrets/define"
 	"github.com/containers/storage/pkg/fileutils"
 	"github.com/containers/storage/pkg/lockfile"
 	"golang.org/x/exp/maps"
@@ -17,12 +18,6 @@ import (
 // secretsDataFile is the file where secrets data/payload will be stored
 var secretsDataFile = "secretsdata.json"
 
-// errNoSecretData indicates that there is not data associated with an id
-var errNoSecretData = errors.New("no secret data with ID")
-
-// errNoSecretData indicates that there is secret data already associated with an id
-var errSecretIDExists = errors.New("secret data with ID already exists")
-
 // Driver is the filedriver object
 type Driver struct {
 	// secretsDataFilePath is the path to the secretsfile
@@ -75,7 +70,7 @@ func (d *Driver) Lookup(id string) ([]byte, error) {
 	if data, ok := secretData[id]; ok {
 		return data, nil
 	}
-	return nil, fmt.Errorf("%s: %w", id, errNoSecretData)
+	return nil, fmt.Errorf("%s: %w", id, define.ErrNoSuchSecret)
 }
 
 // Store stores the bytes associated with an ID. An error is returned if the ID already exists
@@ -88,7 +83,7 @@ func (d *Driver) Store(id string, data []byte) error {
 		return err
 	}
 	if _, ok := secretData[id]; ok {
-		return fmt.Errorf("%s: %w", id, errSecretIDExists)
+		return fmt.Errorf("%s: %w", id, define.ErrSecretIDExists)
 	}
 	secretData[id] = data
 	marshalled, err := json.MarshalIndent(secretData, "", "  ")
@@ -113,7 +108,7 @@ func (d *Driver) Delete(id string) error {
 	if _, ok := secretData[id]; ok {
 		delete(secretData, id)
 	} else {
-		return fmt.Errorf("%s: %w", id, errNoSecretData)
+		return fmt.Errorf("%s: %w", id, define.ErrNoSuchSecret)
 	}
 	marshalled, err := json.MarshalIndent(secretData, "", "  ")
 	if err != nil {

pkg/secrets/passdriver/passdriver.go

@@ -12,20 +12,10 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/containers/common/pkg/secrets/define"
 	"github.com/containers/storage/pkg/fileutils"
 )
 
-var (
-	// errNoSecretData indicates that there is not data associated with an id
-	errNoSecretData = errors.New("no secret data with ID")
-
-	// errNoSecretData indicates that there is secret data already associated with an id
-	errSecretIDExists = errors.New("secret data with ID already exists")
-
-	// errInvalidKey indicates that something about your key is wrong
-	errInvalidKey = errors.New("invalid key")
-)
-
 type driverConfig struct {
 	// Root contains the root directory where the secrets are stored
 	Root string
@@ -128,18 +118,18 @@ func (d *Driver) Lookup(id string) ([]byte, error) {
 		return nil, err
 	}
 	if err := d.gpg(context.TODO(), nil, out, "--decrypt", key); err != nil {
-		return nil, fmt.Errorf("%s: %w", id, errNoSecretData)
+		return nil, fmt.Errorf("%s: %w", id, define.ErrNoSuchSecret)
 	}
 	if out.Len() == 0 {
-		return nil, fmt.Errorf("%s: %w", id, errNoSecretData)
+		return nil, fmt.Errorf("%s: %w", id, define.ErrNoSuchSecret)
 	}
 	return out.Bytes(), nil
 }
 
 // Store saves the bytes associated with an ID. An error is returned if the ID already exists
 func (d *Driver) Store(id string, data []byte) error {
 	if _, err := d.Lookup(id); err == nil {
-		return fmt.Errorf("%s: %w", id, errSecretIDExists)
+		return fmt.Errorf("%s: %w", id, define.ErrSecretIDExists)
 	}
 	in := bytes.NewReader(data)
 	key, err := d.getPath(id)
@@ -156,7 +146,10 @@ func (d *Driver) Delete(id string) error {
 		return err
 	}
 	if err := os.Remove(key); err != nil {
-		return fmt.Errorf("%s: %w", id, errNoSecretData)
+		if errors.Is(err, os.ErrNotExist) {
+			return fmt.Errorf("%s: %w", id, define.ErrNoSuchSecret)
+		}
+		return fmt.Errorf("%s: %w", id, err)
 	}
 	return nil
 }
@@ -176,10 +169,10 @@ func (d *Driver) gpg(ctx context.Context, in io.Reader, out io.Writer, args ...s
 func (d *Driver) getPath(id string) (string, error) {
 	path, err := filepath.Abs(filepath.Join(d.Root, id))
 	if err != nil {
-		return "", errInvalidKey
+		return "", define.ErrInvalidKey
 	}
 	if !strings.HasPrefix(path, d.Root) {
-		return "", errInvalidKey
+		return "", define.ErrInvalidKey
 	}
 	return path + ".gpg", nil
 }

pkg/secrets/passdriver/passdriver_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/containers/common/pkg/secrets/define"
 	"github.com/stretchr/testify/require"
 )
 
@@ -54,7 +55,7 @@ func TestStoreAndLookup(t *testing.T) {
 			name:        "storing into a sneaky key fails",
 			key:         "../../../sneaky",
 			value:       []byte("abc"),
-			expStoreErr: errInvalidKey,
+			expStoreErr: define.ErrInvalidKey,
 		},
 	}
 
@@ -102,12 +103,12 @@ func TestLookup(t *testing.T) {
 		{
 			name:   "lookup of a non-existing key fails",
 			key:    "invalid",
-			expErr: fmt.Errorf("invalid: %w", errNoSecretData),
+			expErr: fmt.Errorf("invalid: %w", define.ErrNoSuchSecret),
 		},
 		{
 			name:   "lookup of a sneaky key fails",
 			key:    "../../../etc/shadow",
-			expErr: errInvalidKey,
+			expErr: define.ErrInvalidKey,
 		},
 	}
 
@@ -151,12 +152,12 @@ func TestDelete(t *testing.T) {
 		{
 			name:   "deleting an non-existing item fails",
 			key:    "wrong",
-			expErr: fmt.Errorf("wrong: %w", errNoSecretData),
+			expErr: fmt.Errorf("wrong: %w", define.ErrNoSuchSecret),
 		},
 		{
 			name:   "using a sneaky path fails",
 			key:    "../../../etc/shadow",
-			expErr: errInvalidKey,
+			expErr: define.ErrInvalidKey,
 		},
 	}
 

pkg/secrets/secrets.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/containers/common/pkg/secrets/define"
 	"github.com/containers/common/pkg/secrets/filedriver"
 	"github.com/containers/common/pkg/secrets/passdriver"
 	"github.com/containers/common/pkg/secrets/shelldriver"
@@ -26,7 +27,7 @@ const secretIDLength = 25
 var errInvalidPath = errors.New("invalid secrets path")
 
 // ErrNoSuchSecret indicates that the secret does not exist
-var ErrNoSuchSecret = errors.New("no such secret")
+var ErrNoSuchSecret = define.ErrNoSuchSecret
 
 // errSecretNameInUse indicates that the secret name is already in use
 var errSecretNameInUse = errors.New("secret name in use")
@@ -151,7 +152,7 @@ func (s *SecretsManager) newID() (string, error) {
 		newID = newID[0:secretIDLength]
 		_, err := s.lookupSecret(newID)
 		if err != nil {
-			if errors.Is(err, ErrNoSuchSecret) {
+			if errors.Is(err, define.ErrNoSuchSecret) {
 				return newID, nil
 			}
 			return "", err
@@ -217,12 +218,14 @@ func (s *SecretsManager) Store(name string, data []byte, driverType string, opti
 	}
 
 	if options.Replace {
-		if err := driver.Delete(secr.ID); err != nil {
+		if err := driver.Delete(secr.ID); err != nil && !errors.Is(err, define.ErrNoSuchSecret) {
 			return "", fmt.Errorf("deleting secret %s: %w", secr.ID, err)
 		}
 
-		if err := s.delete(secr.ID); err != nil {
-			return "", fmt.Errorf("deleting secret %s: %w", secr.ID, err)
+		if err == nil {
+			if err := s.delete(secr.ID); err != nil && !errors.Is(err, define.ErrNoSuchSecret) {
+				return "", fmt.Errorf("deleting secret %s: %w", secr.ID, err)
+			}
 		}
 	}
 

pkg/secrets/secrets_test.go

@@ -147,6 +147,25 @@ func TestAddSecretDupName(t *testing.T) {
 
 	_, err = manager.Store("mysecret", []byte("mydata"), drivertype, storeOpts)
 	require.Error(t, err)
+
+	storeOpts.Replace = true
+	_, err = manager.Store("mysecret", []byte("mydata"), drivertype, storeOpts)
+	require.NoError(t, err)
+}
+
+func TestAddReplaceSecretName(t *testing.T) {
+	manager, opts := setup(t)
+
+	storeOpts := StoreOptions{
+		DriverOpts: opts,
+		Replace:    true,
+	}
+
+	_, err := manager.Store("mysecret", []byte("mydata"), drivertype, storeOpts)
+	require.NoError(t, err)
+
+	_, err = manager.Store("mysecret", []byte("mydata.diff"), drivertype, storeOpts)
+	require.NoError(t, err)
 }
 
 func TestAddSecretPrefix(t *testing.T) {

pkg/secrets/secretsdb.go

@@ -8,6 +8,8 @@ import (
 	"os"
 	"strings"
 	"time"
+
+	"github.com/containers/common/pkg/secrets/define"
 )
 
 type db struct {
@@ -70,14 +72,14 @@ func (s *SecretsManager) getNameAndID(nameOrID string) (name, id string, err err
 	name, id, err = s.getExactNameAndID(nameOrID)
 	if err == nil {
 		return name, id, nil
-	} else if !errors.Is(err, ErrNoSuchSecret) {
+	} else if !errors.Is(err, define.ErrNoSuchSecret) {
 		return "", "", err
 	}
 
 	// ID prefix may have been given, iterate through all IDs.
 	// ID and partial ID has a max length of 25, so we return if its greater than that.
 	if len(nameOrID) > secretIDLength {
-		return "", "", fmt.Errorf("no secret with name or id %q: %w", nameOrID, ErrNoSuchSecret)
+		return "", "", fmt.Errorf("no secret with name or id %q: %w", nameOrID, define.ErrNoSuchSecret)
 	}
 	exists := false
 	var foundID, foundName string
@@ -95,7 +97,7 @@ func (s *SecretsManager) getNameAndID(nameOrID string) (name, id string, err err
 	if exists {
 		return foundName, foundID, nil
 	}
-	return "", "", fmt.Errorf("no secret with name or id %q: %w", nameOrID, ErrNoSuchSecret)
+	return "", "", fmt.Errorf("no secret with name or id %q: %w", nameOrID, define.ErrNoSuchSecret)
 }
 
 // getExactNameAndID takes a secret's name or ID and returns both its name and full ID.
@@ -114,15 +116,15 @@ func (s *SecretsManager) getExactNameAndID(nameOrID string) (name, id string, er
 		return name, id, nil
 	}
 
-	return "", "", fmt.Errorf("no secret with name or id %q: %w", nameOrID, ErrNoSuchSecret)
+	return "", "", fmt.Errorf("no secret with name or id %q: %w", nameOrID, define.ErrNoSuchSecret)
 }
 
 // exactSecretExists checks if the secret exists, given a name or ID
 // Does not match partial name or IDs
 func (s *SecretsManager) exactSecretExists(nameOrID string) (bool, error) {
 	_, _, err := s.getExactNameAndID(nameOrID)
 	if err != nil {
-		if errors.Is(err, ErrNoSuchSecret) {
+		if errors.Is(err, define.ErrNoSuchSecret) {
 			return false, nil
 		}
 		return false, err
@@ -157,7 +159,7 @@ func (s *SecretsManager) lookupSecret(nameOrID string) (*Secret, error) {
 		return &secret, nil
 	}
 
-	return nil, fmt.Errorf("no secret with name or id %q: %w", nameOrID, ErrNoSuchSecret)
+	return nil, fmt.Errorf("no secret with name or id %q: %w", nameOrID, define.ErrNoSuchSecret)
 }
 
 // Store creates a new secret in the secrets database.

pkg/secrets/shelldriver/shelldriver.go

@@ -9,20 +9,13 @@ import (
 	"os/exec"
 	"sort"
 	"strings"
-)
-
-var (
-
-	// errMissingConfig indicates that one or more of the external actions are not configured
-	errMissingConfig = errors.New("missing config value")
 
-	// errNoSecretData indicates that there is not data associated with an id
-	errNoSecretData = errors.New("no secret data with ID")
-
-	// errInvalidKey indicates that something about your key is wrong
-	errInvalidKey = errors.New("invalid key")
+	"github.com/containers/common/pkg/secrets/define"
 )
 
+// errMissingConfig indicates that one or more of the external actions are not configured
+var errMissingConfig = errors.New("missing config value")
+
 type driverConfig struct {
 	// DeleteCommand contains a shell command that deletes a secret.
 	// The secret id is provided as environment variable SECRET_ID
@@ -111,7 +104,7 @@ func (d *Driver) List() (secrets []string, err error) {
 // Lookup returns the bytes associated with a secret ID
 func (d *Driver) Lookup(id string) ([]byte, error) {
 	if strings.Contains(id, "..") {
-		return nil, errInvalidKey
+		return nil, define.ErrInvalidKey
 	}
 
 	cmd := exec.CommandContext(context.TODO(), "/bin/sh", "-c", d.LookupCommand)
@@ -124,15 +117,15 @@ func (d *Driver) Lookup(id string) ([]byte, error) {
 
 	err := cmd.Run()
 	if err != nil {
-		return nil, fmt.Errorf("%s: %w", id, errNoSecretData)
+		return nil, fmt.Errorf("%s: %w", id, define.ErrNoSuchSecret)
 	}
 	return buf.Bytes(), nil
 }
 
 // Store saves the bytes associated with an ID. An error is returned if the ID already exists
 func (d *Driver) Store(id string, data []byte) error {
 	if strings.Contains(id, "..") {
-		return errInvalidKey
+		return define.ErrInvalidKey
 	}
 
 	cmd := exec.CommandContext(context.TODO(), "/bin/sh", "-c", d.StoreCommand)
@@ -149,7 +142,7 @@ func (d *Driver) Store(id string, data []byte) error {
 // Delete removes the secret associated with the specified ID.  An error is returned if no matching secret is found.
 func (d *Driver) Delete(id string) error {
 	if strings.Contains(id, "..") {
-		return errInvalidKey
+		return define.ErrInvalidKey
 	}
 
 	cmd := exec.CommandContext(context.TODO(), "/bin/sh", "-c", d.DeleteCommand)
@@ -161,7 +154,7 @@ func (d *Driver) Delete(id string) error {
 
 	err := cmd.Run()
 	if err != nil {
-		return fmt.Errorf("%s: %w", id, errNoSecretData)
+		return fmt.Errorf("%s: %w", id, define.ErrNoSuchSecret)
 	}
 
 	return nil