Merge pull request #1909 from rhatdan/secret

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