From a5b80d76e2916c55c5b3629c87bff20b5b2552a4 Mon Sep 17 00:00:00 2001 From: ZuhairM7 Date: Tue, 9 Dec 2025 22:19:53 -0600 Subject: [PATCH] secrets: enhance logging for shell driver and store operations This patch adds DEBUG logging to the shell driver to print the exact command and environment variables being executed. It also adds TRACE logging to the SecretsManager.Store method and improves error wrapping to distinguish between driver errors and metadata storage errors. Fixes containers/podman#27635 Signed-off-by: ZuhairM7 --- pkg/secrets/secrets.go | 51 ++++++++++++++------------ pkg/secrets/shelldriver/shelldriver.go | 20 ++++++---- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/pkg/secrets/secrets.go b/pkg/secrets/secrets.go index 958b3971d..0a34c6daf 100644 --- a/pkg/secrets/secrets.go +++ b/pkg/secrets/secrets.go @@ -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 @@ -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"` @@ -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) @@ -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 @@ -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) @@ -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 diff --git a/pkg/secrets/shelldriver/shelldriver.go b/pkg/secrets/shelldriver/shelldriver.go index 262ee5cfc..6b64447f5 100644 --- a/pkg/secrets/shelldriver/shelldriver.go +++ b/pkg/secrets/shelldriver/shelldriver.go @@ -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 { @@ -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 } @@ -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{} @@ -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) @@ -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 @@ -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{} @@ -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 @@ -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 @@ -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