Skip to content

secrets: enhance logging for shell driver and store operations #2537

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.

Sign up for GitHub

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

Closed
ZuhairM7 wants to merge 1 commit into from
Closed

secrets: enhance logging for shell driver and store operations #2537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
51 changes: 27 additions & 24 deletions pkg/secrets/secrets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,55 +10,56 @@ import (
"strings"
"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"
"github.com/containers/storage/pkg/lockfile"
"github.com/containers/storage/pkg/stringid"
"github.com/sirupsen/logrus"
"go.podman.io/common/pkg/secrets/define"
"go.podman.io/common/pkg/secrets/filedriver"
"go.podman.io/common/pkg/secrets/passdriver"
"go.podman.io/common/pkg/secrets/shelldriver"
"go.podman.io/storage/pkg/lockfile"
"go.podman.io/storage/pkg/stringid"
)

// maxSecretSize is the max size for secret data - 512kB
// maxSecretSize is the max size for secret data - 512kB.
const maxSecretSize = 512000

// secretIDLength is the character length of a secret ID - 25
// secretIDLength is the character length of a secret ID - 25.
const secretIDLength = 25

// errInvalidPath indicates that the secrets path is invalid
// errInvalidPath indicates that the secrets path is invalid.
var errInvalidPath = errors.New("invalid secrets path")

// ErrNoSuchSecret indicates that the secret does not exist
// ErrNoSuchSecret indicates that the secret does not exist.
var ErrNoSuchSecret = define.ErrNoSuchSecret

// errSecretNameInUse indicates that the secret name is already in use
// errSecretNameInUse indicates that the secret name is already in use.
var errSecretNameInUse = errors.New("secret name in use")

// errInvalidSecretName indicates that the secret name is invalid
// errInvalidSecretName indicates that the secret name is invalid.
var errInvalidSecretName = errors.New("invalid secret name")

// errInvalidDriver indicates that the driver type is invalid
// errInvalidDriver indicates that the driver type is invalid.
var errInvalidDriver = errors.New("invalid driver")

// errInvalidDriverOpt indicates that a driver option is invalid
// errInvalidDriverOpt indicates that a driver option is invalid.
var errInvalidDriverOpt = errors.New("invalid driver option")

// errAmbiguous indicates that a secret is ambiguous
// errAmbiguous indicates that a secret is ambiguous.
var errAmbiguous = errors.New("secret is ambiguous")

// errDataSize indicates that the secret data is too large or too small
// errDataSize indicates that the secret data is too large or too small.
var errDataSize = errors.New("secret data must be larger than 0 and less than 512000 bytes")

// errIgnoreIfExistsAndReplace indicates that ignoreIfExists and replace cannot be used together.
var errIgnoreIfExistsAndReplace = errors.New("ignoreIfExists and replace cannot be used together")

// secretsFile is the name of the file that the secrets database will be stored in
// secretsFile is the name of the file that the secrets database will be stored in.
var secretsFile = "secrets.json"

// SecretsManager holds information on handling secrets
//
// revive does not like the name because the package is already called secrets
//
//nolint:revive
// revive does not like the name because the package is already called secrets
type SecretsManager struct {
// secretsPath is the path to the db file where secrets are stored
secretsDBPath string
Expand All @@ -68,7 +69,7 @@ type SecretsManager struct {
db *db
}

// Secret defines a secret
// Secret defines a secret.
type Secret struct {
// Name is the name of the secret
Name string `json:"name"`
Expand All @@ -95,7 +96,7 @@ type Secret struct {
//
// revive does not like the name because the package is already called secrets
//
//nolint:revive
// revive does not like the name because the package is already called secrets
type SecretsDriver interface {
// List lists all secret ids in the secrets data store
List() ([]string, error)
Expand All @@ -107,7 +108,7 @@ type SecretsDriver interface {
Delete(id string) error
}

// StoreOptions are optional metadata fields that can be set when storing a new secret
// StoreOptions are optional metadata fields that can be set when storing a new secret.
type StoreOptions struct {
// DriverOptions are extra options used to run this driver
DriverOpts map[string]string
Expand All @@ -122,7 +123,7 @@ type StoreOptions struct {
}

// NewManager creates a new secrets manager
// rootPath is the directory where the secrets data file resides
// rootPath is the directory where the secrets data file resides.
func NewManager(rootPath string) (*SecretsManager, error) {
manager := new(SecretsManager)

Expand Down Expand Up @@ -245,14 +246,16 @@ func (s *SecretsManager) Store(name string, data []byte, driverType string, opti
return "", err
}

logrus.Tracef("Storing secret %s data using driver %s", name, driverType)
err = driver.Store(secr.ID, data)
if err != nil {
return "", fmt.Errorf("creating secret %s: %w", name, err)
return "", fmt.Errorf("driver failed to store secret %s data: %w", name, err)
}

logrus.Tracef("Storing secret %s metadata", name)
err = s.store(secr)
if err != nil {
return "", fmt.Errorf("creating secret %s: %w", name, err)
return "", fmt.Errorf("manager failed to store secret %s metadata: %w", name, err)
}

return secr.ID, nil
Expand Down
20 changes: 12 additions & 8 deletions pkg/secrets/shelldriver/shelldriver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import (
"sort"
"strings"

"github.com/containers/common/pkg/secrets/define"
"github.com/sirupsen/logrus"
"go.podman.io/common/pkg/secrets/define"
)

// errMissingConfig indicates that one or more of the external actions are not configured
// errMissingConfig indicates that one or more of the external actions are not configured.
var errMissingConfig = errors.New("missing config value")

type driverConfig struct {
Expand Down Expand Up @@ -56,7 +57,7 @@ func (cfg *driverConfig) ParseOpts(opts map[string]string) error {
return nil
}

// Driver is the passdriver object
// Driver is the passdriver object.
type Driver struct {
driverConfig
}
Expand All @@ -75,10 +76,11 @@ func NewDriver(opts map[string]string) (*Driver, error) {
return driver, nil
}

// List returns all secret IDs
// List returns all secret IDs.
func (d *Driver) List() (secrets []string, err error) {
cmd := exec.CommandContext(context.TODO(), "/bin/sh", "-c", d.ListCommand)
cmd.Env = os.Environ()
logrus.Debugf("Shell Driver: executing command %q with env %v", cmd.String(), cmd.Env)
cmd.Stderr = os.Stderr

buf := &bytes.Buffer{}
Expand All @@ -89,8 +91,7 @@ func (d *Driver) List() (secrets []string, err error) {
return nil, err
}

parts := bytes.Split(buf.Bytes(), []byte("\n"))
for _, part := range parts {
for part := range bytes.SplitSeq(buf.Bytes(), []byte("\n")) {
id := strings.Trim(string(part), " \r\n")
if len(id) > 0 {
secrets = append(secrets, id)
Expand All @@ -101,7 +102,7 @@ func (d *Driver) List() (secrets []string, err error) {
return secrets, nil
}

// Lookup returns the bytes associated with a secret ID
// Lookup returns the bytes associated with a secret ID.
func (d *Driver) Lookup(id string) ([]byte, error) {
if strings.Contains(id, "..") {
return nil, define.ErrInvalidKey
Expand All @@ -110,6 +111,7 @@ func (d *Driver) Lookup(id string) ([]byte, error) {
cmd := exec.CommandContext(context.TODO(), "/bin/sh", "-c", d.LookupCommand)
cmd.Env = os.Environ()
cmd.Env = append(cmd.Env, "SECRET_ID="+id)
logrus.Debugf("Shell Driver: executing command %q with env %v", cmd.String(), cmd.Env)
cmd.Stderr = os.Stderr

buf := &bytes.Buffer{}
Expand All @@ -122,7 +124,7 @@ func (d *Driver) Lookup(id string) ([]byte, error) {
return buf.Bytes(), nil
}

// Store saves the bytes associated with an ID. An error is returned if the ID already exists
// 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 define.ErrInvalidKey
Expand All @@ -131,6 +133,7 @@ func (d *Driver) Store(id string, data []byte) error {
cmd := exec.CommandContext(context.TODO(), "/bin/sh", "-c", d.StoreCommand)
cmd.Env = os.Environ()
cmd.Env = append(cmd.Env, "SECRET_ID="+id)
logrus.Debugf("Shell Driver: executing command %q with env %v", cmd.String(), cmd.Env)

cmd.Stderr = os.Stderr
cmd.Stdout = os.Stdout
Expand All @@ -148,6 +151,7 @@ func (d *Driver) Delete(id string) error {
cmd := exec.CommandContext(context.TODO(), "/bin/sh", "-c", d.DeleteCommand)
cmd.Env = os.Environ()
cmd.Env = append(cmd.Env, "SECRET_ID="+id)
logrus.Debugf("Shell Driver: executing command %q with env %v", cmd.String(), cmd.Env)

cmd.Stderr = os.Stderr
cmd.Stdout = os.Stdout
Expand Down