From 4e0bf29fbf8c7920a2e35dd07bab6b3186729a52 Mon Sep 17 00:00:00 2001 From: Tamar-Dinavetsky Date: Wed, 26 Aug 2026 12:34:21 +0300 Subject: [PATCH 1/8] feat(audit): add always-on persistent JSON audit log file --- cmd/apply/apply.go | 7 +- cmd/apply/apply_test.go | 6 +- cmd/export/export.go | 13 +- cmd/export/export_test.go | 4 + cmd/transfer-pvc/indirect.go | 4 +- cmd/transfer-pvc/transfer-pvc.go | 8 +- cmd/transform/listplugins/listplugins.go | 5 +- cmd/transform/optionals/optionals.go | 3 +- cmd/transform/transform.go | 8 +- cmd/transform/transform_test.go | 6 + cmd/validate/validate.go | 8 +- cmd/validate/validate_test.go | 20 ++ internal/audit/audit_logger.go | 97 ++++++++++ internal/audit/audit_logger_test.go | 227 +++++++++++++++++++++++ internal/flags/global_flags.go | 39 +++- main.go | 1 + 16 files changed, 428 insertions(+), 28 deletions(-) create mode 100644 internal/audit/audit_logger.go create mode 100644 internal/audit/audit_logger_test.go diff --git a/cmd/apply/apply.go b/cmd/apply/apply.go index c99dbc78..ee8a8b3e 100644 --- a/cmd/apply/apply.go +++ b/cmd/apply/apply.go @@ -45,12 +45,14 @@ type Flags struct { func (o *Options) Complete(c *cobra.Command, args []string) error { // Store positional arguments as requested stages o.RequestedStages = args + o.globalFlags.SetCmdName("apply") o.log = o.globalFlags.GetLoggerOrDefault() + return nil } func (o *Options) Validate() error { - log := o.globalFlags.GetLoggerOrDefault() + log := o.log info, err := os.Stat(o.TransformDir) if err != nil { if os.IsNotExist(err) { @@ -74,6 +76,7 @@ func (o *Options) Run() error { func NewApplyCommand(f *flags.GlobalFlags) *cobra.Command { o := &Options{ cobraGlobalFlags: f, + log: logrus.StandardLogger(), } cmd := &cobra.Command{ Use: "apply [stage...]", @@ -135,7 +138,7 @@ func addFlagsForOptions(o *Flags, cmd *cobra.Command) { } func (o *Options) run() error { - log := o.globalFlags.GetLoggerOrDefault() + log := o.log log.Infof("Starting apply...") transformDir, err := filepath.Abs(o.TransformDir) diff --git a/cmd/apply/apply_test.go b/cmd/apply/apply_test.go index 752e441a..00eb3160 100644 --- a/cmd/apply/apply_test.go +++ b/cmd/apply/apply_test.go @@ -147,7 +147,8 @@ func TestComplete(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - o := &Options{} + o := &Options{ + log: logrus.StandardLogger(),} cmd := &cobra.Command{} err := o.Complete(cmd, tt.args) @@ -237,6 +238,7 @@ func TestRun_UnresolvedStagesError(t *testing.T) { o := &Options{ cobraGlobalFlags: globalFlags, globalFlags: globalFlags, + log: logrus.StandardLogger(), Flags: Flags{ TransformDir: transformDir, OutputDir: outputDir, @@ -302,6 +304,7 @@ func TestValidate_TransformDir(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { o := &Options{ + log: logrus.StandardLogger(), Flags: Flags{ TransformDir: tt.transformDir, }, @@ -331,6 +334,7 @@ func TestValidate_MissingTransformDir_DoesNotCreateOutputDir(t *testing.T) { outputDir := filepath.Join(tmpDir, "output") o := &Options{ + log: logrus.StandardLogger(), Flags: Flags{ TransformDir: missingTransformDir, OutputDir: outputDir, diff --git a/cmd/export/export.go b/cmd/export/export.go index d39cbb85..38636de2 100644 --- a/cmd/export/export.go +++ b/cmd/export/export.go @@ -22,7 +22,6 @@ import ( "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd/api" - ) // ExportOptions holds CLI flags and runtime state for a single export run. @@ -54,6 +53,7 @@ type ExportOptions struct { // Complete loads kubeconfig context, namespace, and parses --as-extras into o.extras. func (o *ExportOptions) Complete(c *cobra.Command, args []string) error { var err error + o.globalFlags.SetCmdName("export") o.log = o.globalFlags.GetLoggerOrDefault() log := o.log @@ -106,8 +106,7 @@ func (o *ExportOptions) Complete(c *cobra.Command, args []string) error { // Validate checks flag combinations (e.g. --as-extras requires impersonation). func (o *ExportOptions) Validate() error { - log := o.globalFlags.GetLoggerOrDefault() - + log := o.log if o.configFlags.Context != nil && *o.configFlags.Context != "" { for _, f := range []struct { flag string @@ -205,7 +204,7 @@ func mergeImpersonationExtras(dest, src map[string][]string) map[string][]string func (o *ExportOptions) Run() error { var err error - log := o.globalFlags.GetLoggerOrDefault() + log := o.log log.Infof("Starting export for namespace %q", o.userSpecifiedNamespace) restConfig, err := o.configFlags.ToRESTConfig() @@ -339,10 +338,10 @@ func (o *ExportOptions) Run() error { // NewExportCommand builds the cobra export command with flags and viper wiring. func NewExportCommand(streams genericclioptions.IOStreams, f *flags.GlobalFlags) *cobra.Command { o := &ExportOptions{ - configFlags: genericclioptions.NewConfigFlags(true), - - IOStreams: streams, + configFlags: genericclioptions.NewConfigFlags(true), + IOStreams: streams, cobraGlobalFlags: f, + log: logrus.StandardLogger(), } cmd := &cobra.Command{ Use: "export", diff --git a/cmd/export/export_test.go b/cmd/export/export_test.go index ce048b49..a623a526 100644 --- a/cmd/export/export_test.go +++ b/cmd/export/export_test.go @@ -85,6 +85,7 @@ func TestComplete_AsExtras(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { o := &ExportOptions{ + log: logrus.StandardLogger(), configFlags: genericclioptions.NewConfigFlags(true), asExtras: tt.asExtras, } @@ -180,6 +181,7 @@ func TestValidate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { o := &ExportOptions{ configFlags: genericclioptions.NewConfigFlags(true), + log: logrus.StandardLogger(), asExtras: tt.asExtras, labelSelector: tt.labelSelector, } @@ -261,6 +263,7 @@ func TestValidate_ContextConflicts(t *testing.T) { t.Run(tt.name, func(t *testing.T) { o := &ExportOptions{ configFlags: genericclioptions.NewConfigFlags(true), + log: logrus.StandardLogger(), } if tt.context != nil { o.configFlags.Context = tt.context @@ -335,6 +338,7 @@ func TestValidate_CRDGroupConflict(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { o := &ExportOptions{ + log: logrus.StandardLogger(), configFlags: genericclioptions.NewConfigFlags(true), crdSkipGroups: tt.crdSkipGroups, crdIncludeGroups: tt.crdIncludeGroups, diff --git a/cmd/transfer-pvc/indirect.go b/cmd/transfer-pvc/indirect.go index 03fa8f81..1a50a44e 100644 --- a/cmd/transfer-pvc/indirect.go +++ b/cmd/transfer-pvc/indirect.go @@ -26,7 +26,7 @@ import ( ) func (t *TransferPVCCommand) runIndirect() error { - log := t.globalFlags.GetLoggerOrDefault() + log := t.log log.Infof("Starting indirect PVC transfer: %s/%s -> %s/%s", t.PVC.Namespace.source, t.PVC.Name.source, t.PVC.Namespace.destination, t.PVC.Name.destination) fmt.Fprintf(os.Stderr, "\ncrane transfer-pvc (indirect via cloud storage)\n") @@ -355,7 +355,7 @@ func checkRclonePartialSuccess(output, podName, namespace string, log *logrus.Lo } func (t *TransferPVCCommand) createTempRcloneSecretFromData(c client.Client, namespace string, configData []byte, labelPVCName string) (string, error) { - log := t.globalFlags.GetLoggerOrDefault() + log := t.log secretName := fmt.Sprintf("crane-rclone-config-%s", getValidatedResourceName(t.PVC.Name.source)) secret := &corev1.Secret{ diff --git a/cmd/transfer-pvc/transfer-pvc.go b/cmd/transfer-pvc/transfer-pvc.go index d981cad0..320ab24f 100644 --- a/cmd/transfer-pvc/transfer-pvc.go +++ b/cmd/transfer-pvc/transfer-pvc.go @@ -207,6 +207,7 @@ func addFlagsToTransferPVCCommand(c *Flags, cmd *cobra.Command) { } func (t *TransferPVCCommand) Complete(c *cobra.Command, args []string) error { + t.globalFlags.SetCmdName("transfer-pvc") t.log = t.globalFlags.GetLoggerOrDefault() config := t.configFlags.ToRawKubeConfigLoader() rawConfig, err := config.RawConfig() @@ -240,7 +241,10 @@ func (t *TransferPVCCommand) Complete(c *cobra.Command, args []string) error { } func (t *TransferPVCCommand) Validate() error { - log := t.globalFlags.GetLoggerOrDefault() + log := t.log + if log == nil { + log = t.globalFlags.GetLoggerOrDefault() + } if t.Flags.KeepCloudData && t.Flags.CloudStorage == "" { log.Debugf("--keep-cloud-data requires --cloud-storage") @@ -337,7 +341,7 @@ func (t *TransferPVCCommand) getRestConfigFromContext(ctx string) (*rest.Config, } func (t *TransferPVCCommand) run() (retErr error) { - log := t.globalFlags.GetLoggerOrDefault() + log := t.log log.Infof("Starting PVC transfer: %s/%s -> %s/%s", t.PVC.Namespace.source, t.PVC.Name.source, t.PVC.Namespace.destination, t.PVC.Name.destination) logrusLog := logrus.New() logrusLog.SetFormatter(&logrus.JSONFormatter{}) diff --git a/cmd/transform/listplugins/listplugins.go b/cmd/transform/listplugins/listplugins.go index 057910c4..a664ad0c 100644 --- a/cmd/transform/listplugins/listplugins.go +++ b/cmd/transform/listplugins/listplugins.go @@ -4,9 +4,9 @@ import ( "fmt" "path/filepath" + cranelib "github.com/konveyor/crane-lib/transform" "github.com/konveyor/crane/internal/flags" "github.com/konveyor/crane/internal/plugin" - cranelib "github.com/konveyor/crane-lib/transform" "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -33,6 +33,7 @@ type Flags struct { func (o *Options) Complete(c *cobra.Command, args []string) error { // TODO: @sseago + o.globalFlags.SetCmdName("transform listplugins") o.log = o.globalFlags.GetLoggerOrDefault() return nil } @@ -106,7 +107,7 @@ func getFilteredPlugins(pluginDir string, skipPlugins []string, log *logrus.Logg } func (o *Options) run() error { - log := o.globalFlags.GetLoggerOrDefault() + log := o.log plugins, err := getFilteredPlugins(o.PluginDir, o.SkipPlugins, log) if err != nil { diff --git a/cmd/transform/optionals/optionals.go b/cmd/transform/optionals/optionals.go index b3d52184..4490393e 100644 --- a/cmd/transform/optionals/optionals.go +++ b/cmd/transform/optionals/optionals.go @@ -32,6 +32,7 @@ type Flags struct { func (o *Options) Complete(c *cobra.Command, args []string) error { // TODO: @sseago + o.globalFlags.SetCmdName("transform optionals") o.log = o.globalFlags.GetLoggerOrDefault() return nil } @@ -78,7 +79,7 @@ func NewOptionalsCommand(f *flags.GlobalFlags) *cobra.Command { } func (o *Options) run() error { - log := o.globalFlags.GetLoggerOrDefault() + log := o.log pluginDir, err := filepath.Abs(o.PluginDir) if err != nil { diff --git a/cmd/transform/transform.go b/cmd/transform/transform.go index 7be947b1..18204740 100644 --- a/cmd/transform/transform.go +++ b/cmd/transform/transform.go @@ -57,12 +57,13 @@ type Flags struct { func (o *Options) Complete(c *cobra.Command, args []string) error { // Store positional arguments as requested stages o.RequestedStages = args + o.globalFlags.SetCmdName("transform") o.log = o.globalFlags.GetLoggerOrDefault() return nil } func (o *Options) Validate() error { - log := o.globalFlags.GetLoggerOrDefault() + log := o.log exportDir, err := filepath.Abs(o.ExportDir) if err != nil { @@ -106,7 +107,7 @@ func getPluginCompletions(f *flags.GlobalFlags) func(cmd *cobra.Command, args [] } // Get plugin names using shared function - log := f.GetLogger() + log := f.GetLoggerOrDefault() pluginNames, err := listplugins.GetPluginNames(pluginDir, skipPlugins, log) if err != nil { return nil, cobra.ShellCompDirectiveError @@ -119,6 +120,7 @@ func getPluginCompletions(f *flags.GlobalFlags) func(cmd *cobra.Command, args [] func NewTransformCommand(f *flags.GlobalFlags) *cobra.Command { o := &Options{ cobraGlobalFlags: f, + log: logrus.StandardLogger(), } cmd := &cobra.Command{ Use: "transform [stage...]", @@ -181,7 +183,7 @@ func addFlagsForOptions(o *Flags, cmd *cobra.Command) { } func (o *Options) run() error { - log := o.globalFlags.GetLoggerOrDefault() + log := o.log log.Infof("Starting transform...") exportDir, err := filepath.Abs(o.ExportDir) diff --git a/cmd/transform/transform_test.go b/cmd/transform/transform_test.go index c225d858..5c101393 100644 --- a/cmd/transform/transform_test.go +++ b/cmd/transform/transform_test.go @@ -476,6 +476,7 @@ func TestReconcileInstructionStages_Overwrite(t *testing.T) { func TestRun_InstructionsFileAndPositionalArgsConflict(t *testing.T) { o := &Options{ globalFlags: &flags.GlobalFlags{}, + log: logrus.StandardLogger(), RequestedStages: []string{"10_KubernetesPlugin"}, Flags: Flags{ InstructionsFile: "sample-transform-instructor-file.yaml", @@ -680,6 +681,7 @@ func TestResolveAndValidateStages_MultipleCustomStages(t *testing.T) { log.SetOutput(os.Stderr) o := &Options{ + log: logrus.StandardLogger(), Flags: Flags{ SkipPlugins: []string{}, }, @@ -823,6 +825,7 @@ func TestResolveAndValidateStages_CustomStageWithPreviousStageOutput(t *testing. log.SetOutput(os.Stderr) o := &Options{ + log: logrus.StandardLogger(), Flags: Flags{ SkipPlugins: []string{}, }, @@ -1060,6 +1063,7 @@ func TestValidate_ExportDir(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { o := &Options{ + log: logrus.StandardLogger(), Flags: Flags{ ExportDir: tt.exportDir, PluginDir: filepath.Join(tmpDir, "plugins"), @@ -1233,6 +1237,7 @@ func TestParseStageOptionals_MultiFieldJSON(t *testing.T) { func TestRun_InstructionsFileAndStageOptionalsConflict(t *testing.T) { o := &Options{ globalFlags: &flags.GlobalFlags{}, + log: logrus.StandardLogger(), Flags: Flags{ InstructionsFile: "instructions.yaml", StageOptionals: []string{`KubernetesPlugin={"key": "val"}`}, @@ -1253,6 +1258,7 @@ func TestValidate_MissingExportDir_FailsBeforeRun(t *testing.T) { transformDir := filepath.Join(tmpDir, "transform") o := &Options{ + log: logrus.StandardLogger(), Flags: Flags{ ExportDir: filepath.Join(tmpDir, "missing-export"), TransformDir: transformDir, diff --git a/cmd/validate/validate.go b/cmd/validate/validate.go index e03b5dbb..d33eb5fd 100644 --- a/cmd/validate/validate.go +++ b/cmd/validate/validate.go @@ -37,6 +37,7 @@ type ValidateOptions struct { // detected later in Run() when discovery is queried. // Skipped in offline mode (--api-resources). func (o *ValidateOptions) Complete(c *cobra.Command, args []string) error { + o.globalFlags.SetCmdName("validate") o.log = o.globalFlags.GetLoggerOrDefault() kubeconfigFlag := c.Flags().Lookup("kubeconfig") @@ -87,7 +88,7 @@ func determineClusterContext(cf *genericclioptions.ConfigFlags) (string, error) // Validate checks that flags have valid values. func (o *ValidateOptions) Validate(cmd *cobra.Command) error { - log := o.globalFlags.GetLoggerOrDefault() + log := o.log info, err := os.Stat(o.inputDir) if err != nil { @@ -144,7 +145,7 @@ func (o *ValidateOptions) Validate(cmd *cobra.Command) error { // Run performs the scan, match, and report steps. func (o *ValidateOptions) Run() error { - log := o.globalFlags.GetLoggerOrDefault() + log := o.log log.Infof("Starting validate") log.Debugf("Input directory: %s", o.inputDir) @@ -263,8 +264,9 @@ func (o *ValidateOptions) Run() error { func NewValidateCommand(streams genericclioptions.IOStreams, f *flags.GlobalFlags) *cobra.Command { o := &ValidateOptions{ configFlags: genericclioptions.NewConfigFlags(true), - IOStreams: streams, + IOStreams: streams, cobraGlobalFlags: f, + log: logrus.StandardLogger(), } cmd := &cobra.Command{ Use: "validate", diff --git a/cmd/validate/validate_test.go b/cmd/validate/validate_test.go index a598fbd0..0f9ef9bd 100644 --- a/cmd/validate/validate_test.go +++ b/cmd/validate/validate_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/konveyor/crane/internal/flags" + "github.com/sirupsen/logrus" "github.com/spf13/cobra" "k8s.io/cli-runtime/pkg/genericclioptions" ) @@ -53,6 +54,7 @@ func TestValidate_Flags(t *testing.T) { setup: func(t *testing.T) *ValidateOptions { missingDir := filepath.Join(t.TempDir(), "missing") return &ValidateOptions{ + log: logrus.StandardLogger(), inputDir: missingDir, outputFormat: "yaml", } @@ -69,6 +71,7 @@ func TestValidate_Flags(t *testing.T) { t.Fatal(err) } return &ValidateOptions{ + log: logrus.StandardLogger(), inputDir: f, outputFormat: "yaml", } @@ -80,6 +83,7 @@ func TestValidate_Flags(t *testing.T) { name: "invalid output format", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ + log: logrus.StandardLogger(), inputDir: t.TempDir(), outputFormat: "xml", } @@ -91,6 +95,7 @@ func TestValidate_Flags(t *testing.T) { name: "valid yaml format", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ + log: logrus.StandardLogger(), inputDir: t.TempDir(), outputFormat: "yaml", } @@ -101,6 +106,7 @@ func TestValidate_Flags(t *testing.T) { name: "valid json format", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ + log: logrus.StandardLogger(), inputDir: t.TempDir(), outputFormat: "json", } @@ -111,6 +117,7 @@ func TestValidate_Flags(t *testing.T) { name: "uppercase JSON accepted", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ + log: logrus.StandardLogger(), inputDir: t.TempDir(), outputFormat: "JSON", } @@ -121,6 +128,7 @@ func TestValidate_Flags(t *testing.T) { name: "uppercase YAML accepted", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ + log: logrus.StandardLogger(), inputDir: t.TempDir(), outputFormat: "YAML", } @@ -131,6 +139,7 @@ func TestValidate_Flags(t *testing.T) { name: "mixed case Json accepted", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ + log: logrus.StandardLogger(), inputDir: t.TempDir(), outputFormat: "Json", } @@ -141,6 +150,7 @@ func TestValidate_Flags(t *testing.T) { name: "api-resources file not found", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ + log: logrus.StandardLogger(), configFlags: genericclioptions.NewConfigFlags(true), inputDir: t.TempDir(), outputFormat: "json", @@ -162,6 +172,7 @@ func TestValidate_Flags(t *testing.T) { cf := genericclioptions.NewConfigFlags(true) cf.Context = &ctx return &ValidateOptions{ + log: logrus.StandardLogger(), configFlags: cf, inputDir: dir, outputFormat: "json", @@ -184,6 +195,7 @@ func TestValidate_Flags(t *testing.T) { cf := genericclioptions.NewConfigFlags(true) cf.KubeConfig = &kc return &ValidateOptions{ + log: logrus.StandardLogger(), configFlags: cf, inputDir: dir, outputFormat: "json", @@ -206,6 +218,7 @@ func TestValidate_Flags(t *testing.T) { cf := genericclioptions.NewConfigFlags(true) cf.APIServer = &server return &ValidateOptions{ + log: logrus.StandardLogger(), configFlags: cf, inputDir: dir, outputFormat: "json", @@ -228,6 +241,7 @@ func TestValidate_Flags(t *testing.T) { cf := genericclioptions.NewConfigFlags(true) cf.BearerToken = &token return &ValidateOptions{ + log: logrus.StandardLogger(), configFlags: cf, inputDir: dir, outputFormat: "json", @@ -250,6 +264,7 @@ func TestValidate_Flags(t *testing.T) { cf := genericclioptions.NewConfigFlags(true) cf.ClusterName = &cluster return &ValidateOptions{ + log: logrus.StandardLogger(), configFlags: cf, inputDir: dir, outputFormat: "json", @@ -272,6 +287,7 @@ func TestValidate_Flags(t *testing.T) { cf := genericclioptions.NewConfigFlags(true) cf.AuthInfoName = &user return &ValidateOptions{ + log: logrus.StandardLogger(), configFlags: cf, inputDir: dir, outputFormat: "json", @@ -291,6 +307,7 @@ func TestValidate_Flags(t *testing.T) { t.Fatal(err) } return &ValidateOptions{ + log: logrus.StandardLogger(), configFlags: genericclioptions.NewConfigFlags(true), inputDir: dir, outputFormat: "json", @@ -378,6 +395,7 @@ func TestRun_EmptyInputDirReturnsError(t *testing.T) { } o := &ValidateOptions{ + log: logrus.StandardLogger(), configFlags: genericclioptions.NewConfigFlags(true), IOStreams: genericclioptions.NewTestIOStreamsDiscard(), inputDir: emptyDir, @@ -484,6 +502,7 @@ current-context: existing-context cf.KubeConfig = &kc o := &ValidateOptions{ + log: logrus.StandardLogger(), configFlags: cf, globalFlags: &flags.GlobalFlags{}, inputDir: t.TempDir(), @@ -506,6 +525,7 @@ current-context: existing-context func TestComplete_SkippedInOfflineMode(t *testing.T) { o := &ValidateOptions{ + log: logrus.StandardLogger(), configFlags: genericclioptions.NewConfigFlags(true), globalFlags: &flags.GlobalFlags{}, inputDir: t.TempDir(), diff --git a/internal/audit/audit_logger.go b/internal/audit/audit_logger.go new file mode 100644 index 00000000..08b6c7c4 --- /dev/null +++ b/internal/audit/audit_logger.go @@ -0,0 +1,97 @@ +package audit + +import ( + "io" + "os" + "path/filepath" + + "github.com/sirupsen/logrus" +) + +// FileHook writes every log entry as a JSON line to a file. +type FileHook struct { + file *os.File + cmd *string + formatter logrus.Formatter +} + +// NewFileHook opens (or creates) the file at path in append mode and returns a hook. +// cmd is a pointer to a string that is read on every Fire() call, so it can be set after the hook is created. +func NewFileHook(path string, cmd *string) (*FileHook, error) { + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return nil, err + } + f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + return nil, err + } + return &FileHook{ + file: f, + cmd: cmd, + formatter: &logrus.JSONFormatter{}, + }, nil +} + +// Levels returns all log levels so the hook captures everything, including Debug. +func (h *FileHook) Levels() []logrus.Level { + return logrus.AllLevels +} + +// Fire is called by logrus for every log entry. It formats the entry as JSON and writes it to the file. +func (h *FileHook) Fire(entry *logrus.Entry) error { + if h.cmd != nil && *h.cmd != "" { + entry.Data["cmd"] = *h.cmd + } + line, err := h.formatter.Format(entry) + if err != nil { + return err + } + _, err = h.file.Write(line) + return err +} + +// Close closes the underlying file. Call this when the program exits. +func (h *FileHook) Close() error { + return h.file.Close() +} + +// ConsoleHook writes log entries to stderr, filtered by the levels it is given. +// This replaces logrus's default output so the level list can be controlled independently of the file hook. +type ConsoleHook struct { + writer io.Writer + levels []logrus.Level + formatter logrus.Formatter +} + +// NewConsoleHook returns a hook that writes to stderr. +// If debug is true, all levels are shown; otherwise only Info and above. +func NewConsoleHook(debug bool) *ConsoleHook { + levels := []logrus.Level{ + logrus.PanicLevel, + logrus.FatalLevel, + logrus.ErrorLevel, + logrus.WarnLevel, + logrus.InfoLevel, + } + if debug { + levels = logrus.AllLevels + } + return &ConsoleHook{ + writer: os.Stderr, + levels: levels, + formatter: &logrus.TextFormatter{}, + } +} + +func (h *ConsoleHook) Levels() []logrus.Level { + return h.levels +} + +func (h *ConsoleHook) Fire(entry *logrus.Entry) error { + line, err := h.formatter.Format(entry) + if err != nil { + return err + } + _, err = h.writer.Write(line) + return err +} diff --git a/internal/audit/audit_logger_test.go b/internal/audit/audit_logger_test.go new file mode 100644 index 00000000..91f01f80 --- /dev/null +++ b/internal/audit/audit_logger_test.go @@ -0,0 +1,227 @@ +package audit + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + "github.com/sirupsen/logrus" +) + +// makeEntry builds a minimal logrus.Entry for use in Fire() calls. +func makeEntry(msg string, level logrus.Level) *logrus.Entry { + return &logrus.Entry{ + Logger: logrus.New(), + Message: msg, + Level: level, + Time: time.Now(), + Data: logrus.Fields{}, + } +} + +// --- FileHook tests --- + +func TestNewFileHook(t *testing.T) { + dir := t.TempDir() + + // Create a plain file so that MkdirAll fails when we try to use it as a directory. + notADir := filepath.Join(dir, "not-a-dir") + if err := os.WriteFile(notADir, []byte("x"), 0644); err != nil { + t.Fatalf("setup: %v", err) + } + + tests := []struct { + name string + path string + wantErr bool + }{ + { + name: "valid path creates file", + path: filepath.Join(dir, "audit", ".crane-audit.log"), + wantErr: false, + }, + { + name: "empty path returns error", + path: "", + wantErr: true, + }, + { + name: "bad path returns error when dir is a file", + path: filepath.Join(notADir, "audit.log"), + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hook, err := NewFileHook(tt.path, nil) + if tt.wantErr && err == nil { + t.Fatal("expected error, got nil") + } + if !tt.wantErr && err != nil { + t.Fatalf("unexpected error: %v", err) + } + if hook != nil { + hook.Close() + } + if !tt.wantErr { + if _, statErr := os.Stat(tt.path); statErr != nil { + t.Fatalf("file was not created at %s: %v", tt.path, statErr) + } + } + }) + } +} + +func TestFileHook_Fire_WritesJSON(t *testing.T) { + path := filepath.Join(t.TempDir(), "audit.log") + hook, err := NewFileHook(path, nil) + if err != nil { + t.Fatalf("NewFileHook: %v", err) + } + defer hook.Close() + + if err := hook.Fire(makeEntry("test message", logrus.InfoLevel)); err != nil { + t.Fatalf("Fire: %v", err) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if len(data) == 0 { + t.Fatal("expected file to contain data, got empty file") + } + + var parsed map[string]interface{} + if err := json.Unmarshal(data[:len(data)-1], &parsed); err != nil { + t.Fatalf("file content is not valid JSON: %v\ncontent: %s", err, data) + } + if parsed["msg"] != "test message" { + t.Errorf("msg = %q, want %q", parsed["msg"], "test message") + } + if parsed["level"] != "info" { + t.Errorf("level = %q, want %q", parsed["level"], "info") + } +} + +func TestFileHook_AppendMode(t *testing.T) { + path := filepath.Join(t.TempDir(), "audit.log") + + for i, msg := range []string{"first", "second"} { + hook, err := NewFileHook(path, nil) + if err != nil { + t.Fatalf("run %d NewFileHook: %v", i, err) + } + if err := hook.Fire(makeEntry(msg, logrus.InfoLevel)); err != nil { + t.Fatalf("run %d Fire: %v", i, err) + } + hook.Close() + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + lines := bytes.Split(bytes.TrimRight(data, "\n"), []byte("\n")) + if len(lines) != 2 { + t.Fatalf("expected 2 lines (append mode), got %d:\n%s", len(lines), data) + } +} + +func TestFileHook_Close_PreventsFurtherWrites(t *testing.T) { + path := filepath.Join(t.TempDir(), "audit.log") + hook, err := NewFileHook(path, nil) + if err != nil { + t.Fatalf("NewFileHook: %v", err) + } + + if err := hook.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + + if err := hook.Fire(makeEntry("after close", logrus.InfoLevel)); err == nil { + t.Fatal("expected error when firing after Close, got nil") + } +} + +func TestFileHook_Levels_ReturnsAll(t *testing.T) { + path := filepath.Join(t.TempDir(), "audit.log") + hook, err := NewFileHook(path, nil) + if err != nil { + t.Fatalf("NewFileHook: %v", err) + } + defer hook.Close() + + levels := hook.Levels() + if len(levels) != len(logrus.AllLevels) { + t.Errorf("Levels() returned %d levels, want %d", len(levels), len(logrus.AllLevels)) + } +} + +func TestFileHook_Fire_InjectsCmdField(t *testing.T) { + path := filepath.Join(t.TempDir(), "audit.log") + cmd := "export" + hook, err := NewFileHook(path, &cmd) + if err != nil { + t.Fatalf("NewFileHook: %v", err) + } + defer hook.Close() + + if err := hook.Fire(makeEntry("test", logrus.InfoLevel)); err != nil { + t.Fatalf("Fire: %v", err) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + + var parsed map[string]interface{} + if err := json.Unmarshal(data[:len(data)-1], &parsed); err != nil { + t.Fatalf("not valid JSON: %v", err) + } + if parsed["cmd"] != "export" { + t.Errorf("cmd = %q, want %q", parsed["cmd"], "export") + } +} + +// --- ConsoleHook tests --- + +func TestNewConsoleHook_LevelsWithoutDebug(t *testing.T) { + hook := NewConsoleHook(false) + for _, l := range hook.Levels() { + if l == logrus.DebugLevel || l == logrus.TraceLevel { + t.Errorf("ConsoleHook without debug should not include level %s", l) + } + } +} + +func TestNewConsoleHook_LevelsWithDebug(t *testing.T) { + hook := NewConsoleHook(true) + if len(hook.Levels()) != len(logrus.AllLevels) { + t.Errorf("ConsoleHook with debug: got %d levels, want %d", len(hook.Levels()), len(logrus.AllLevels)) + } +} + +func TestConsoleHook_Fire_WritesToWriter(t *testing.T) { + var buf bytes.Buffer + hook := &ConsoleHook{ + writer: &buf, + levels: logrus.AllLevels, + formatter: &logrus.TextFormatter{DisableColors: true}, + } + + if err := hook.Fire(makeEntry("hello console", logrus.InfoLevel)); err != nil { + t.Fatalf("Fire: %v", err) + } + if buf.Len() == 0 { + t.Fatal("expected output in writer, got nothing") + } + if !bytes.Contains(buf.Bytes(), []byte("hello console")) { + t.Errorf("output does not contain message: %s", buf.Bytes()) + } +} diff --git a/internal/flags/global_flags.go b/internal/flags/global_flags.go index 353a54e5..3ca9b22f 100644 --- a/internal/flags/global_flags.go +++ b/internal/flags/global_flags.go @@ -1,24 +1,38 @@ package flags import ( + "io" + + "github.com/konveyor/crane/internal/audit" "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/viper" ) type GlobalFlags struct { - ConfigFile string - Debug bool - logger *logrus.Logger + ConfigFile string + Debug bool + AuditLogPath string `mapstructure:"audit-log"` + CmdName string + logger *logrus.Logger + fileHook *audit.FileHook } func (g *GlobalFlags) ApplyFlags(cmd *cobra.Command) { cobra.OnInitialize(g.initConfig) cmd.PersistentFlags().BoolVar(&g.Debug, "debug", false, "Debug the command by printing more information") cmd.PersistentFlags().StringVarP(&g.ConfigFile, "flags-file", "f", "", "Path to input file which contains a yaml representation of cli flags. Explicit flags take precedence over input file values.") + cmd.PersistentFlags().StringVar(&g.AuditLogPath, "audit-log", "audit/.crane-audit.log", "Path to the audit log file") viper.BindPFlags(cmd.PersistentFlags()) } +// SetCmdName sets the command name used in audit log entries. Safe to call on a nil GlobalFlags. +func (g *GlobalFlags) SetCmdName(name string) { + if g != nil { + g.CmdName = name + } +} + // GetLoggerOrDefault returns the configured logger, or logrus.StandardLogger() if GlobalFlags is nil. func (g *GlobalFlags) GetLoggerOrDefault() *logrus.Logger { if g == nil { @@ -30,13 +44,28 @@ func (g *GlobalFlags) GetLoggerOrDefault() *logrus.Logger { func (g *GlobalFlags) GetLogger() *logrus.Logger { if g.logger == nil { g.logger = logrus.New() - if g.Debug { - g.logger.SetLevel(logrus.DebugLevel) + g.logger.SetLevel(logrus.DebugLevel) + g.logger.SetOutput(io.Discard) + consoleHook := audit.NewConsoleHook(g.Debug) + fileHook, err := audit.NewFileHook(g.AuditLogPath, &g.CmdName) + g.logger.AddHook(consoleHook) + if err == nil { + g.fileHook = fileHook + g.logger.AddHook(fileHook) + } else { + g.logger.Warnf("Failed to open audit log file %s: %v", g.AuditLogPath, err) } } return g.logger } +// Close releases the audit log file. Call this when the program exits. +func (g *GlobalFlags) Close() { + if g.fileHook != nil { + g.fileHook.Close() + } +} + func (g *GlobalFlags) initConfig() { if g.ConfigFile != "" { viper.SetConfigFile(g.ConfigFile) diff --git a/main.go b/main.go index c39e0a46..40e44c4d 100644 --- a/main.go +++ b/main.go @@ -21,6 +21,7 @@ import ( func main() { f := &flags.GlobalFlags{} + defer f.Close() root := cobra.Command{ Use: filepath.Base(os.Args[0]), } From f411179375f47466c38a103ab159eeab9fb68dee Mon Sep 17 00:00:00 2001 From: Tamar-Dinavetsky Date: Wed, 26 Aug 2026 18:27:03 +0300 Subject: [PATCH 2/8] update some files --- cmd/skopeo-sync-gen/skopeo-sync-gen.go | 2 +- cmd/transfer-pvc/transfer-pvc.go | 4 +++ cmd/transform/transform.go | 1 + internal/audit/audit_logger_test.go | 44 +++++++++++++++++++++++--- internal/file/file_helper.go | 6 +++- internal/file/file_helper_test.go | 24 ++++++++++++++ internal/flags/global_flags.go | 6 ++-- internal/transform/orchestrator.go | 2 +- main.go | 14 ++++++-- 9 files changed, 91 insertions(+), 12 deletions(-) diff --git a/cmd/skopeo-sync-gen/skopeo-sync-gen.go b/cmd/skopeo-sync-gen/skopeo-sync-gen.go index a6fe6c7b..b91bbf3c 100644 --- a/cmd/skopeo-sync-gen/skopeo-sync-gen.go +++ b/cmd/skopeo-sync-gen/skopeo-sync-gen.go @@ -119,7 +119,7 @@ func (o *Options) Run() error { return err } - files, err := file.ReadFiles(context.TODO(), exportDir) + files, err := file.ReadFilesWithLogger(context.TODO(), exportDir, o.globalFlags.GetLoggerOrDefault()) if err != nil { return err } diff --git a/cmd/transfer-pvc/transfer-pvc.go b/cmd/transfer-pvc/transfer-pvc.go index 320ab24f..c9fa8c2d 100644 --- a/cmd/transfer-pvc/transfer-pvc.go +++ b/cmd/transfer-pvc/transfer-pvc.go @@ -297,6 +297,10 @@ func (t *TransferPVCCommand) Validate() error { } func (t *TransferPVCCommand) Run() error { + if t.log == nil { + t.globalFlags.SetCmdName("transfer-pvc") + t.log = t.globalFlags.GetLoggerOrDefault() + } if t.Flags.CloudStorage != "" { return t.runIndirect() } diff --git a/cmd/transform/transform.go b/cmd/transform/transform.go index 18204740..143fc301 100644 --- a/cmd/transform/transform.go +++ b/cmd/transform/transform.go @@ -107,6 +107,7 @@ func getPluginCompletions(f *flags.GlobalFlags) func(cmd *cobra.Command, args [] } // Get plugin names using shared function + f.SetCmdName("transform") log := f.GetLoggerOrDefault() pluginNames, err := listplugins.GetPluginNames(pluginDir, skipPlugins, log) if err != nil { diff --git a/internal/audit/audit_logger_test.go b/internal/audit/audit_logger_test.go index 91f01f80..1372bc9e 100644 --- a/internal/audit/audit_logger_test.go +++ b/internal/audit/audit_logger_test.go @@ -65,7 +65,9 @@ func TestNewFileHook(t *testing.T) { t.Fatalf("unexpected error: %v", err) } if hook != nil { - hook.Close() + if err := hook.Close(); err != nil { + t.Errorf("Close: %v", err) + } } if !tt.wantErr { if _, statErr := os.Stat(tt.path); statErr != nil { @@ -82,7 +84,11 @@ func TestFileHook_Fire_WritesJSON(t *testing.T) { if err != nil { t.Fatalf("NewFileHook: %v", err) } - defer hook.Close() + defer func() { + if err := hook.Close(); err != nil { + t.Errorf("Close: %v", err) + } + }() if err := hook.Fire(makeEntry("test message", logrus.InfoLevel)); err != nil { t.Fatalf("Fire: %v", err) @@ -119,7 +125,9 @@ func TestFileHook_AppendMode(t *testing.T) { if err := hook.Fire(makeEntry(msg, logrus.InfoLevel)); err != nil { t.Fatalf("run %d Fire: %v", i, err) } - hook.Close() + if err := hook.Close(); err != nil { + t.Fatalf("run %d Close: %v", i, err) + } } data, err := os.ReadFile(path) @@ -154,7 +162,11 @@ func TestFileHook_Levels_ReturnsAll(t *testing.T) { if err != nil { t.Fatalf("NewFileHook: %v", err) } - defer hook.Close() + defer func() { + if err := hook.Close(); err != nil { + t.Errorf("Close: %v", err) + } + }() levels := hook.Levels() if len(levels) != len(logrus.AllLevels) { @@ -169,7 +181,11 @@ func TestFileHook_Fire_InjectsCmdField(t *testing.T) { if err != nil { t.Fatalf("NewFileHook: %v", err) } - defer hook.Close() + defer func() { + if err := hook.Close(); err != nil { + t.Errorf("Close: %v", err) + } + }() if err := hook.Fire(makeEntry("test", logrus.InfoLevel)); err != nil { t.Fatalf("Fire: %v", err) @@ -189,6 +205,24 @@ func TestFileHook_Fire_InjectsCmdField(t *testing.T) { } } +func TestNewFileHook_CustomPath(t *testing.T) { + dir := t.TempDir() + customPath := filepath.Join(dir, "custom", "crane.jsonl") + cmd := "export" + hook, err := NewFileHook(customPath, &cmd) + if err != nil { + t.Fatalf("NewFileHook with custom path: %v", err) + } + defer hook.Close() + + if err := hook.Fire(makeEntry("hello", logrus.InfoLevel)); err != nil { + t.Fatalf("Fire: %v", err) + } + if _, err := os.Stat(customPath); err != nil { + t.Fatalf("file not created at custom path %s: %v", customPath, err) + } +} + // --- ConsoleHook tests --- func TestNewConsoleHook_LevelsWithoutDebug(t *testing.T) { diff --git a/internal/file/file_helper.go b/internal/file/file_helper.go index 8b220c26..24a98773 100644 --- a/internal/file/file_helper.go +++ b/internal/file/file_helper.go @@ -21,8 +21,12 @@ type File struct { } func ReadFiles(ctx context.Context, dir string) ([]File, error) { - log := logrus.StandardLogger() + return ReadFilesWithLogger(ctx, dir, logrus.StandardLogger()) +} +// ReadFilesWithLogger reads Kubernetes resource files from dir using the provided logger. +// Use this instead of ReadFiles when the caller has an audit-hooked logger. +func ReadFilesWithLogger(ctx context.Context, dir string, log *logrus.Logger) ([]File, error) { files, err := ioutil.ReadDir(dir) if err != nil { return nil, fmt.Errorf("failed to read directory %q: %w", dir, err) diff --git a/internal/file/file_helper_test.go b/internal/file/file_helper_test.go index 052cb2df..a9ff6c46 100644 --- a/internal/file/file_helper_test.go +++ b/internal/file/file_helper_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/konveyor/crane/internal/file" + "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -32,6 +33,29 @@ func writeFile(t *testing.T, path, content string) { } } +func TestReadFilesWithLogger_UsesProvidedLogger(t *testing.T) { + dir := createTestDir(t) + validYAML := `apiVersion: v1 +kind: ConfigMap +metadata: + name: logger-test-cm + namespace: default +` + writeFile(t, filepath.Join(dir, "cm.yaml"), validYAML) + + log := logrus.New() + files, err := file.ReadFilesWithLogger(context.TODO(), dir, log) + if err != nil { + t.Fatalf("ReadFilesWithLogger: %v", err) + } + if len(files) != 1 { + t.Fatalf("expected 1 file, got %d", len(files)) + } + if files[0].Unstructured.GetName() != "logger-test-cm" { + t.Errorf("name = %q, want %q", files[0].Unstructured.GetName(), "logger-test-cm") + } +} + func TestReadFilesValidResource(t *testing.T) { dir := createTestDir(t) validYAML := `apiVersion: v1 diff --git a/internal/flags/global_flags.go b/internal/flags/global_flags.go index 3ca9b22f..d30a9221 100644 --- a/internal/flags/global_flags.go +++ b/internal/flags/global_flags.go @@ -60,10 +60,11 @@ func (g *GlobalFlags) GetLogger() *logrus.Logger { } // Close releases the audit log file. Call this when the program exits. -func (g *GlobalFlags) Close() { +func (g *GlobalFlags) Close() error { if g.fileHook != nil { - g.fileHook.Close() + return g.fileHook.Close() } + return nil } func (g *GlobalFlags) initConfig() { @@ -73,6 +74,7 @@ func (g *GlobalFlags) initConfig() { viper.AutomaticEnv() if err := viper.ReadInConfig(); err == nil { + viper.UnmarshalKey("audit-log", &g.AuditLogPath) g.GetLogger().Infof("Using config file: %v", viper.ConfigFileUsed()) } } diff --git a/internal/transform/orchestrator.go b/internal/transform/orchestrator.go index 6e8d74fe..9271d5f5 100644 --- a/internal/transform/orchestrator.go +++ b/internal/transform/orchestrator.go @@ -471,7 +471,7 @@ func (o *Orchestrator) applyStageTransforms(stageDir string) ([]unstructured.Uns // loadResourcesFromDirectory loads all Kubernetes resources from a directory func (o *Orchestrator) loadResourcesFromDirectory(dir string) ([]unstructured.Unstructured, error) { - files, err := file.ReadFiles(context.TODO(), dir) + files, err := file.ReadFilesWithLogger(context.TODO(), dir, o.Log) if err != nil { o.Log.Debugf("Failed to read directory %s: %v", dir, err) return nil, fmt.Errorf("failed to read directory %s: %w", dir, err) diff --git a/main.go b/main.go index 40e44c4d..5d432536 100644 --- a/main.go +++ b/main.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "os" "path/filepath" @@ -20,8 +21,16 @@ import ( ) func main() { + os.Exit(run()) +} + +func run() int { f := &flags.GlobalFlags{} - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + fmt.Fprintf(os.Stderr, "warning: failed to close audit log: %v\n", err) + } + }() root := cobra.Command{ Use: filepath.Base(os.Args[0]), } @@ -37,6 +46,7 @@ func main() { root.AddCommand(version.NewVersionCommand(f)) root.AddCommand(validate.NewValidateCommand(genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}, f)) if err := root.Execute(); err != nil { - os.Exit(1) + return 1 } + return 0 } From 0a2bdb272c8fd8f5070b0dbc113fc4d9413b6a89 Mon Sep 17 00:00:00 2001 From: Tamar-Dinavetsky Date: Wed, 26 Aug 2026 18:59:22 +0300 Subject: [PATCH 3/8] update some files --- internal/audit/audit_logger.go | 12 ++++++++++-- internal/audit/audit_logger_test.go | 24 ++++++++++++++++++++---- internal/file/file_helper.go | 3 +++ internal/file/file_helper_test.go | 18 ++++++++++++++++++ internal/flags/global_flags.go | 6 +++++- 5 files changed, 56 insertions(+), 7 deletions(-) diff --git a/internal/audit/audit_logger.go b/internal/audit/audit_logger.go index 08b6c7c4..269ad782 100644 --- a/internal/audit/audit_logger.go +++ b/internal/audit/audit_logger.go @@ -32,9 +32,17 @@ func NewFileHook(path string, cmd *string) (*FileHook, error) { }, nil } -// Levels returns all log levels so the hook captures everything, including Debug. +// Levels returns all log levels down to Debug. Trace is excluded because the +// configured logger is set to DebugLevel and will not emit Trace entries. func (h *FileHook) Levels() []logrus.Level { - return logrus.AllLevels + return []logrus.Level{ + logrus.PanicLevel, + logrus.FatalLevel, + logrus.ErrorLevel, + logrus.WarnLevel, + logrus.InfoLevel, + logrus.DebugLevel, + } } // Fire is called by logrus for every log entry. It formats the entry as JSON and writes it to the file. diff --git a/internal/audit/audit_logger_test.go b/internal/audit/audit_logger_test.go index 1372bc9e..ee8a53d3 100644 --- a/internal/audit/audit_logger_test.go +++ b/internal/audit/audit_logger_test.go @@ -156,7 +156,7 @@ func TestFileHook_Close_PreventsFurtherWrites(t *testing.T) { } } -func TestFileHook_Levels_ReturnsAll(t *testing.T) { +func TestFileHook_Levels_IncludesDebugExcludesTrace(t *testing.T) { path := filepath.Join(t.TempDir(), "audit.log") hook, err := NewFileHook(path, nil) if err != nil { @@ -169,8 +169,20 @@ func TestFileHook_Levels_ReturnsAll(t *testing.T) { }() levels := hook.Levels() - if len(levels) != len(logrus.AllLevels) { - t.Errorf("Levels() returned %d levels, want %d", len(levels), len(logrus.AllLevels)) + hasDebug, hasTrace := false, false + for _, l := range levels { + if l == logrus.DebugLevel { + hasDebug = true + } + if l == logrus.TraceLevel { + hasTrace = true + } + } + if !hasDebug { + t.Error("Levels() should include DebugLevel") + } + if hasTrace { + t.Error("Levels() should not include TraceLevel (logger is set to DebugLevel)") } } @@ -213,7 +225,11 @@ func TestNewFileHook_CustomPath(t *testing.T) { if err != nil { t.Fatalf("NewFileHook with custom path: %v", err) } - defer hook.Close() + defer func() { + if err := hook.Close(); err != nil { + t.Errorf("Close: %v", err) + } + }() if err := hook.Fire(makeEntry("hello", logrus.InfoLevel)); err != nil { t.Fatalf("Fire: %v", err) diff --git a/internal/file/file_helper.go b/internal/file/file_helper.go index 24a98773..df24df32 100644 --- a/internal/file/file_helper.go +++ b/internal/file/file_helper.go @@ -27,6 +27,9 @@ func ReadFiles(ctx context.Context, dir string) ([]File, error) { // ReadFilesWithLogger reads Kubernetes resource files from dir using the provided logger. // Use this instead of ReadFiles when the caller has an audit-hooked logger. func ReadFilesWithLogger(ctx context.Context, dir string, log *logrus.Logger) ([]File, error) { + if log == nil { + log = logrus.StandardLogger() + } files, err := ioutil.ReadDir(dir) if err != nil { return nil, fmt.Errorf("failed to read directory %q: %w", dir, err) diff --git a/internal/file/file_helper_test.go b/internal/file/file_helper_test.go index a9ff6c46..c0d19554 100644 --- a/internal/file/file_helper_test.go +++ b/internal/file/file_helper_test.go @@ -33,6 +33,24 @@ func writeFile(t *testing.T, path, content string) { } } +func TestReadFilesWithLogger_NilLoggerDoesNotPanic(t *testing.T) { + dir := createTestDir(t) + writeFile(t, filepath.Join(dir, "cm.yaml"), `apiVersion: v1 +kind: ConfigMap +metadata: + name: nil-logger-cm + namespace: default +`) + // passing nil should not panic + files, err := file.ReadFilesWithLogger(context.TODO(), dir, nil) + if err != nil { + t.Fatalf("unexpected error with nil logger: %v", err) + } + if len(files) != 1 { + t.Fatalf("expected 1 file, got %d", len(files)) + } +} + func TestReadFilesWithLogger_UsesProvidedLogger(t *testing.T) { dir := createTestDir(t) validYAML := `apiVersion: v1 diff --git a/internal/flags/global_flags.go b/internal/flags/global_flags.go index d30a9221..af900fac 100644 --- a/internal/flags/global_flags.go +++ b/internal/flags/global_flags.go @@ -1,7 +1,9 @@ package flags import ( + "fmt" "io" + "os" "github.com/konveyor/crane/internal/audit" "github.com/sirupsen/logrus" @@ -74,7 +76,9 @@ func (g *GlobalFlags) initConfig() { viper.AutomaticEnv() if err := viper.ReadInConfig(); err == nil { - viper.UnmarshalKey("audit-log", &g.AuditLogPath) + if err := viper.UnmarshalKey("audit-log", &g.AuditLogPath); err != nil { + fmt.Fprintf(os.Stderr, "warning: invalid audit-log value in %s: %v\n", viper.ConfigFileUsed(), err) + } g.GetLogger().Infof("Using config file: %v", viper.ConfigFileUsed()) } } From ecb93f731119fef27ed1f062ab9d4602db688a4a Mon Sep 17 00:00:00 2001 From: Tamar-Dinavetsky Date: Thu, 27 Aug 2026 13:55:12 +0300 Subject: [PATCH 4/8] update some files --- cmd/export/export_test.go | 3 +- internal/audit/audit_logger.go | 17 +++++---- internal/file/file_helper_test.go | 57 ++++++++++++++----------------- 3 files changed, 38 insertions(+), 39 deletions(-) diff --git a/cmd/export/export_test.go b/cmd/export/export_test.go index 393879b3..824da245 100644 --- a/cmd/export/export_test.go +++ b/cmd/export/export_test.go @@ -670,7 +670,8 @@ func TestValidate_GKFilter(t *testing.T) { t.Run(tt.name, func(t *testing.T) { o := &ExportOptions{ configFlags: genericclioptions.NewConfigFlags(true), - globalFlags: nil, // GetLoggerOrDefault handles nil + globalFlags: nil, + log: logrus.StandardLogger(), includeGK: tt.includeGK, excludeGK: tt.excludeGK, } diff --git a/internal/audit/audit_logger.go b/internal/audit/audit_logger.go index 269ad782..7dfed53f 100644 --- a/internal/audit/audit_logger.go +++ b/internal/audit/audit_logger.go @@ -1,6 +1,7 @@ package audit import ( + "fmt" "io" "os" "path/filepath" @@ -52,10 +53,12 @@ func (h *FileHook) Fire(entry *logrus.Entry) error { } line, err := h.formatter.Format(entry) if err != nil { - return err + return fmt.Errorf("audit file hook: format entry: %w", err) } - _, err = h.file.Write(line) - return err + if _, err := h.file.Write(line); err != nil { + return fmt.Errorf("audit file hook: write to %s: %w", h.file.Name(), err) + } + return nil } // Close closes the underlying file. Call this when the program exits. @@ -98,8 +101,10 @@ func (h *ConsoleHook) Levels() []logrus.Level { func (h *ConsoleHook) Fire(entry *logrus.Entry) error { line, err := h.formatter.Format(entry) if err != nil { - return err + return fmt.Errorf("audit console hook: format entry: %w", err) + } + if _, err := h.writer.Write(line); err != nil { + return fmt.Errorf("audit console hook: write to stderr: %w", err) } - _, err = h.writer.Write(line) - return err + return nil } diff --git a/internal/file/file_helper_test.go b/internal/file/file_helper_test.go index c0d19554..e787cc74 100644 --- a/internal/file/file_helper_test.go +++ b/internal/file/file_helper_test.go @@ -33,44 +33,37 @@ func writeFile(t *testing.T, path, content string) { } } -func TestReadFilesWithLogger_NilLoggerDoesNotPanic(t *testing.T) { - dir := createTestDir(t) - writeFile(t, filepath.Join(dir, "cm.yaml"), `apiVersion: v1 -kind: ConfigMap -metadata: - name: nil-logger-cm - namespace: default -`) - // passing nil should not panic - files, err := file.ReadFilesWithLogger(context.TODO(), dir, nil) - if err != nil { - t.Fatalf("unexpected error with nil logger: %v", err) - } - if len(files) != 1 { - t.Fatalf("expected 1 file, got %d", len(files)) - } -} - -func TestReadFilesWithLogger_UsesProvidedLogger(t *testing.T) { - dir := createTestDir(t) - validYAML := `apiVersion: v1 +func TestReadFilesWithLogger(t *testing.T) { + const validYAML = `apiVersion: v1 kind: ConfigMap metadata: name: logger-test-cm namespace: default ` - writeFile(t, filepath.Join(dir, "cm.yaml"), validYAML) - - log := logrus.New() - files, err := file.ReadFilesWithLogger(context.TODO(), dir, log) - if err != nil { - t.Fatalf("ReadFilesWithLogger: %v", err) - } - if len(files) != 1 { - t.Fatalf("expected 1 file, got %d", len(files)) + tests := []struct { + name string + logger *logrus.Logger + }{ + {name: "nil logger does not panic", logger: nil}, + {name: "non-nil logger", logger: logrus.New()}, } - if files[0].Unstructured.GetName() != "logger-test-cm" { - t.Errorf("name = %q, want %q", files[0].Unstructured.GetName(), "logger-test-cm") + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := createTestDir(t) + writeFile(t, filepath.Join(dir, "cm.yaml"), validYAML) + + files, err := file.ReadFilesWithLogger(context.TODO(), dir, tt.logger) + if err != nil { + t.Fatalf("ReadFilesWithLogger: %v", err) + } + if len(files) != 1 { + t.Fatalf("expected 1 file, got %d", len(files)) + } + if files[0].Unstructured.GetName() != "logger-test-cm" { + t.Errorf("name = %q, want %q", files[0].Unstructured.GetName(), "logger-test-cm") + } + }) } } From 85d1538f9544182e256912f1d3418f48a3885503 Mon Sep 17 00:00:00 2001 From: Tamar-Dinavetsky Date: Mon, 31 Aug 2026 22:13:51 +0300 Subject: [PATCH 5/8] Update by comments --- .gitignore | 4 +- cmd/apply/apply.go | 2 +- cmd/apply/apply_test.go | 4 +- cmd/convert/convert.go | 12 ++--- cmd/export/export.go | 5 ++- cmd/export/export_test.go | 34 +++++++------- cmd/plugin-manager/add/add.go | 8 ++-- cmd/plugin-manager/list/list.go | 8 +++- cmd/plugin-manager/remove/remove.go | 6 ++- cmd/skopeo-sync-gen/skopeo-sync-gen.go | 1 + cmd/transfer-pvc/transfer-pvc.go | 12 +---- cmd/transform/transform.go | 16 +++---- cmd/transform/transform_test.go | 62 +++++++++++++------------- cmd/tunnel-api/tunnel-api.go | 25 ++++++----- cmd/validate/validate.go | 2 +- cmd/validate/validate_test.go | 48 ++++++++++---------- internal/audit/audit_logger.go | 8 ++-- internal/flags/global_flags.go | 24 ++++++---- main.go | 4 +- 19 files changed, 152 insertions(+), 133 deletions(-) diff --git a/.gitignore b/.gitignore index 356959ca..42b4e389 100644 --- a/.gitignore +++ b/.gitignore @@ -16,4 +16,6 @@ vendor/ crane -.DS_Store \ No newline at end of file +.DS_Store + +audit/ \ No newline at end of file diff --git a/cmd/apply/apply.go b/cmd/apply/apply.go index ee8a8b3e..1e59b521 100644 --- a/cmd/apply/apply.go +++ b/cmd/apply/apply.go @@ -5,11 +5,11 @@ import ( "os" "path/filepath" - "github.com/sirupsen/logrus" "github.com/konveyor/crane/internal/apply" "github.com/konveyor/crane/internal/flags" "github.com/konveyor/crane/internal/kustomize" internalTransform "github.com/konveyor/crane/internal/transform" + "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/viper" ) diff --git a/cmd/apply/apply_test.go b/cmd/apply/apply_test.go index 00eb3160..9ced825a 100644 --- a/cmd/apply/apply_test.go +++ b/cmd/apply/apply_test.go @@ -148,7 +148,7 @@ func TestComplete(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { o := &Options{ - log: logrus.StandardLogger(),} + log: logrus.StandardLogger()} cmd := &cobra.Command{} err := o.Complete(cmd, tt.args) @@ -334,7 +334,7 @@ func TestValidate_MissingTransformDir_DoesNotCreateOutputDir(t *testing.T) { outputDir := filepath.Join(tmpDir, "output") o := &Options{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), Flags: Flags{ TransformDir: missingTransformDir, OutputDir: outputDir, diff --git a/cmd/convert/convert.go b/cmd/convert/convert.go index dd95cec3..dd067a90 100644 --- a/cmd/convert/convert.go +++ b/cmd/convert/convert.go @@ -2,6 +2,7 @@ package convert import ( "github.com/konveyor/crane-lib/convert" + "github.com/konveyor/crane/internal/flags" "github.com/sirupsen/logrus" "github.com/spf13/cobra" "k8s.io/cli-runtime/pkg/genericclioptions" @@ -16,6 +17,7 @@ import ( type ConvertOptions struct { configFlags *genericclioptions.ConfigFlags genericclioptions.IOStreams + globalFlags *flags.GlobalFlags SourceContext string Namespace string Logger logrus.FieldLogger @@ -27,15 +29,11 @@ type ConvertOptions struct { debug bool } -func NewConvertOptions(streams genericclioptions.IOStreams) *cobra.Command { - logger := logrus.New() - logger.SetOutput(streams.Out) - logger.SetFormatter(&logrus.TextFormatter{}) - +func NewConvertOptions(streams genericclioptions.IOStreams, f *flags.GlobalFlags) *cobra.Command { t := &ConvertOptions{ configFlags: genericclioptions.NewConfigFlags(false), IOStreams: streams, - Logger: logger, + globalFlags: f, } cmd := &cobra.Command{ @@ -72,6 +70,8 @@ func addFlagsForConvertOptions(t *ConvertOptions, cmd *cobra.Command) { } func (t *ConvertOptions) Complete(c *cobra.Command, args []string) error { + t.globalFlags.SetCmdName("convert") + t.Logger = t.globalFlags.GetLoggerOrDefault() if t.debug { if logger, ok := t.Logger.(*logrus.Logger); ok { logger.SetLevel(logrus.DebugLevel) diff --git a/cmd/export/export.go b/cmd/export/export.go index 4c6a7710..ffdfd097 100644 --- a/cmd/export/export.go +++ b/cmd/export/export.go @@ -108,6 +108,7 @@ func (o *ExportOptions) Complete(c *cobra.Command, args []string) error { // Users can override by explicitly using --include-gk Event or --exclude-gk if len(o.includeGK) == 0 && len(o.excludeGK) == 0 { o.excludeGK = []string{"Event"} + log.Debugf("No GK filters specified; applying default exclusion: Event") } return nil @@ -155,7 +156,7 @@ func (o *ExportOptions) Validate() error { } } if _, err := NewGKFilter(o.includeGK, o.excludeGK); err != nil { - log.Debugf("Invalid GK filter: %v", err) + log.Errorf("Invalid GK filter: %v", err) return err } return nil @@ -358,7 +359,7 @@ func (o *ExportOptions) Run() error { func NewExportCommand(streams genericclioptions.IOStreams, f *flags.GlobalFlags) *cobra.Command { o := &ExportOptions{ configFlags: genericclioptions.NewConfigFlags(true), - IOStreams: streams, + IOStreams: streams, cobraGlobalFlags: f, log: logrus.StandardLogger(), } diff --git a/cmd/export/export_test.go b/cmd/export/export_test.go index 824da245..81152661 100644 --- a/cmd/export/export_test.go +++ b/cmd/export/export_test.go @@ -85,7 +85,7 @@ func TestComplete_AsExtras(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { o := &ExportOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: genericclioptions.NewConfigFlags(true), asExtras: tt.asExtras, } @@ -338,7 +338,7 @@ func TestValidate_CRDGroupConflict(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { o := &ExportOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: genericclioptions.NewConfigFlags(true), crdSkipGroups: tt.crdSkipGroups, crdIncludeGroups: tt.crdIncludeGroups, @@ -689,33 +689,33 @@ func TestValidate_GKFilter(t *testing.T) { func TestComplete_DefaultExcludeEvent(t *testing.T) { tests := []struct { - name string - includeGK []string - excludeGK []string + name string + includeGK []string + excludeGK []string wantExclude []string }{ { - name: "no GK filters - defaults to exclude Event", - includeGK: nil, - excludeGK: nil, + name: "no GK filters - defaults to exclude Event", + includeGK: nil, + excludeGK: nil, wantExclude: []string{"Event"}, }, { - name: "explicit include - no default", - includeGK: []string{"Deployment"}, - excludeGK: nil, + name: "explicit include - no default", + includeGK: []string{"Deployment"}, + excludeGK: nil, wantExclude: nil, }, { - name: "explicit exclude - no default", - includeGK: nil, - excludeGK: []string{"Secret"}, + name: "explicit exclude - no default", + includeGK: nil, + excludeGK: []string{"Secret"}, wantExclude: []string{"Secret"}, }, { - name: "empty slices - defaults to exclude Event", - includeGK: []string{}, - excludeGK: []string{}, + name: "empty slices - defaults to exclude Event", + includeGK: []string{}, + excludeGK: []string{}, wantExclude: []string{"Event"}, }, } diff --git a/cmd/plugin-manager/add/add.go b/cmd/plugin-manager/add/add.go index 8c708fd1..34a1912f 100644 --- a/cmd/plugin-manager/add/add.go +++ b/cmd/plugin-manager/add/add.go @@ -26,6 +26,7 @@ type Options struct { cobraGlobalFlags *flags.GlobalFlags globalFlags *flags.GlobalFlags + log *logrus.Logger // Two Flags struct fields are needed // 1. cobraFlags for explicit CLI args parsed by cobra // 2. Flags for the args merged with values from the viper config file @@ -42,6 +43,8 @@ type Flags struct { func (o *Options) Complete(c *cobra.Command, args []string) error { // TODO: @jgabani + o.globalFlags.SetCmdName("plugin-manager add") + o.log = o.globalFlags.GetLoggerOrDefault() return nil } @@ -96,7 +99,6 @@ func NewAddCommand(f *flags.GlobalFlags) *cobra.Command { o := &Options{ globalFlags: f, } - log := o.globalFlags.GetLogger() cmd := &cobra.Command{ Use: "add ", Short: "installs the desired plugin", @@ -105,7 +107,7 @@ func NewAddCommand(f *flags.GlobalFlags) *cobra.Command { return err } if err := o.Validate(args); err != nil { - log.Errorf("%s", err.Error()) + o.log.Errorf("%s", err.Error()) return nil } if err := o.Run(args); err != nil { @@ -130,7 +132,7 @@ func addFlagsForOptions(o *Flags, cmd *cobra.Command) { } func (o *Options) run(args []string) error { - log := o.globalFlags.GetLogger() + log := o.log manifestMap, err := plugin.BuildManifestMap(log, args[0], o.Repo) if err != nil { diff --git a/cmd/plugin-manager/list/list.go b/cmd/plugin-manager/list/list.go index c98500d6..281126fc 100644 --- a/cmd/plugin-manager/list/list.go +++ b/cmd/plugin-manager/list/list.go @@ -10,6 +10,7 @@ import ( "github.com/konveyor/crane/internal/flags" "github.com/konveyor/crane/internal/plugin" "github.com/olekukonko/tablewriter" + "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/viper" ) @@ -20,6 +21,7 @@ type Options struct { // 2. globalFlags for the args merged with values from the viper config file cobraGlobalFlags *flags.GlobalFlags globalFlags *flags.GlobalFlags + log *logrus.Logger // Two Flags struct fields are needed // 1. cobraFlags for explicit CLI args parsed by cobra // 2. Flags for the args merged with values from the viper config file @@ -45,6 +47,8 @@ type AvailablePlugins struct { func (o *Options) Complete(c *cobra.Command, args []string) error { // TODO: @jgabani + o.globalFlags.SetCmdName("plugin-manager list") + o.log = o.globalFlags.GetLoggerOrDefault() return nil } @@ -96,7 +100,7 @@ func addFlagsForOptions(o *Flags, cmd *cobra.Command) { } func (o *Options) run() error { - log := o.globalFlags.GetLogger() + log := o.log if o.Installed { // retrieve list of all the plugins that are installed within plugin dir // TODO: differentiate between multiple repos @@ -163,7 +167,7 @@ func (o *Options) run() error { return nil } -//TODO: this can be merged with printParamsInformation +// TODO: this can be merged with printParamsInformation func printInstalledInformation(plugins []transform2.Plugin) { for _, thisPlugin := range plugins { printTable([][]string{ diff --git a/cmd/plugin-manager/remove/remove.go b/cmd/plugin-manager/remove/remove.go index 56ffd6d4..5f645863 100644 --- a/cmd/plugin-manager/remove/remove.go +++ b/cmd/plugin-manager/remove/remove.go @@ -8,6 +8,7 @@ import ( "github.com/konveyor/crane/internal/flags" "github.com/konveyor/crane/internal/plugin" + "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/viper" ) @@ -18,6 +19,7 @@ type Options struct { // 2. globalFlags for the args merged with values from the viper config file cobraGlobalFlags *flags.GlobalFlags globalFlags *flags.GlobalFlags + log *logrus.Logger // Two Flags struct fields are needed // 1. cobraFlags for explicit CLI args parsed by cobra // 2. Flags for the args merged with values from the viper config file @@ -32,6 +34,8 @@ type Flags struct { func (o *Options) Complete(c *cobra.Command, args []string) error { // TODO: @jgabani + o.globalFlags.SetCmdName("plugin-manager remove") + o.log = o.globalFlags.GetLoggerOrDefault() return nil } @@ -75,7 +79,7 @@ func NewRemoveCommand(f *flags.GlobalFlags) *cobra.Command { } func (o *Options) run(args []string) error { - log := o.globalFlags.GetLogger() + log := o.log pluginDir, err := filepath.Abs(fmt.Sprintf("%v/%v", o.PluginDir, o.Repo)) if err != nil { return err diff --git a/cmd/skopeo-sync-gen/skopeo-sync-gen.go b/cmd/skopeo-sync-gen/skopeo-sync-gen.go index b91bbf3c..adbc1a37 100644 --- a/cmd/skopeo-sync-gen/skopeo-sync-gen.go +++ b/cmd/skopeo-sync-gen/skopeo-sync-gen.go @@ -59,6 +59,7 @@ type registrySyncConfig struct { type sourceConfig map[string]registrySyncConfig func (o *Options) Complete(c *cobra.Command, args []string) error { + o.globalFlags.SetCmdName("skopeo-sync-gen") return nil } diff --git a/cmd/transfer-pvc/transfer-pvc.go b/cmd/transfer-pvc/transfer-pvc.go index 6376524d..6eb048ba 100644 --- a/cmd/transfer-pvc/transfer-pvc.go +++ b/cmd/transfer-pvc/transfer-pvc.go @@ -61,7 +61,6 @@ type TransferPVCCommand struct { genericclioptions.IOStreams globalFlags *flags.GlobalFlags log *logrus.Logger - logger logrus.FieldLogger sourceContext *clientcmdapi.Context destinationContext *clientcmdapi.Context @@ -154,9 +153,8 @@ func NewTransferPVCCommand(streams genericclioptions.IOStreams, f *flags.GlobalF StorageRequests: quantityVar{}, }, }, - IOStreams: streams, + IOStreams: streams, globalFlags: f, - logger: logrus.New(), } cmd := &cobra.Command{ @@ -313,10 +311,6 @@ func (t *TransferPVCCommand) Validate() error { } func (t *TransferPVCCommand) Run() error { - if t.log == nil { - t.globalFlags.SetCmdName("transfer-pvc") - t.log = t.globalFlags.GetLoggerOrDefault() - } if t.CloudStorage != "" { return t.runIndirect() } @@ -363,9 +357,7 @@ func (t *TransferPVCCommand) getRestConfigFromContext(ctx string) (*rest.Config, func (t *TransferPVCCommand) run() (retErr error) { log := t.log log.Infof("Starting PVC transfer: %s/%s -> %s/%s", t.PVC.Namespace.source, t.PVC.Name.source, t.PVC.Namespace.destination, t.PVC.Name.destination) - logrusLog := logrus.New() - logrusLog.SetFormatter(&logrus.JSONFormatter{}) - logger := logrusr.New(logrusLog).WithName("transfer-pvc") + logger := logrusr.New(log).WithName("transfer-pvc") totalPhases := 7 if t.isIntraClusterSameNamespace() { diff --git a/cmd/transform/transform.go b/cmd/transform/transform.go index 143fc301..95080bcd 100644 --- a/cmd/transform/transform.go +++ b/cmd/transform/transform.go @@ -41,13 +41,13 @@ type Options struct { } type Flags struct { - ExportDir string `mapstructure:"export-dir"` - PluginDir string `mapstructure:"plugin-dir"` - TransformDir string `mapstructure:"transform-dir"` - SkipPlugins []string `mapstructure:"skip-plugins"` - OptionalFlags string `mapstructure:"optional-flags"` - StageOptionals []string `mapstructure:"stage-optionals"` - Overwrite bool `mapstructure:"overwrite"` + ExportDir string `mapstructure:"export-dir"` + PluginDir string `mapstructure:"plugin-dir"` + TransformDir string `mapstructure:"transform-dir"` + SkipPlugins []string `mapstructure:"skip-plugins"` + OptionalFlags string `mapstructure:"optional-flags"` + StageOptionals []string `mapstructure:"stage-optionals"` + Overwrite bool `mapstructure:"overwrite"` // Kustomize arguments KustomizeArgs string `mapstructure:"kustomize-args"` // Instructions file @@ -206,7 +206,7 @@ func (o *Options) run() error { } if o.InstructionsFile != "" && len(o.RequestedStages) > 0 { - log.Debugf("Cannot use --instructions-file together with positional stage arguments") + log.Debugf("Cannot use --instructions-file together with positional stage arguments") return fmt.Errorf("use either --instructions-file or positional stage arguments, not both") } if o.InstructionsFile != "" && len(o.StageOptionals) > 0 { diff --git a/cmd/transform/transform_test.go b/cmd/transform/transform_test.go index 5c101393..9e5bef98 100644 --- a/cmd/transform/transform_test.go +++ b/cmd/transform/transform_test.go @@ -552,11 +552,11 @@ func TestResolveAndValidateStages_CustomStageCreation(t *testing.T) { expectError: false, }, { - name: "invalid custom stage name returns wrapped error", - requestedStage: "invalid stage name!", - shouldCreate: false, - expectError: true, - }, + name: "invalid custom stage name returns wrapped error", + requestedStage: "invalid stage name!", + shouldCreate: false, + expectError: true, + }, } for _, tt := range tests { @@ -598,24 +598,24 @@ func TestResolveAndValidateStages_CustomStageCreation(t *testing.T) { ) if tt.expectError { - if err == nil { - t.Fatalf("expected error but got none") - } - if !strings.Contains(err.Error(), "invalid custom stage name") || strings.Contains(err.Error(), "") { - t.Errorf("expected error to contain validation context, but got: %v", err) - } - - stageDir := filepath.Join(subTransformDir, tt.requestedStage) - if _, statErr := os.Stat(stageDir); statErr == nil { - t.Errorf("expected directory %s NOT to be created for invalid stage name", stageDir) - } - - return - } else { - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - } + if err == nil { + t.Fatalf("expected error but got none") + } + if !strings.Contains(err.Error(), "invalid custom stage name") || strings.Contains(err.Error(), "") { + t.Errorf("expected error to contain validation context, but got: %v", err) + } + + stageDir := filepath.Join(subTransformDir, tt.requestedStage) + if _, statErr := os.Stat(stageDir); statErr == nil { + t.Errorf("expected directory %s NOT to be created for invalid stage name", stageDir) + } + + return + } else { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + } if len(resolved) != 1 { t.Fatalf("expected 1 resolved stage, got %d", len(resolved)) @@ -681,7 +681,7 @@ func TestResolveAndValidateStages_MultipleCustomStages(t *testing.T) { log.SetOutput(os.Stderr) o := &Options{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), Flags: Flags{ SkipPlugins: []string{}, }, @@ -825,7 +825,7 @@ func TestResolveAndValidateStages_CustomStageWithPreviousStageOutput(t *testing. log.SetOutput(os.Stderr) o := &Options{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), Flags: Flags{ SkipPlugins: []string{}, }, @@ -1094,11 +1094,11 @@ func TestValidate_ExportDir(t *testing.T) { func TestParseStageOptionals(t *testing.T) { tests := []struct { - name string - values []string - wantErr bool - errMsg string - expected map[string]map[string]string + name string + values []string + wantErr bool + errMsg string + expected map[string]map[string]string }{ { name: "single stage", @@ -1258,7 +1258,7 @@ func TestValidate_MissingExportDir_FailsBeforeRun(t *testing.T) { transformDir := filepath.Join(tmpDir, "transform") o := &Options{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), Flags: Flags{ ExportDir: filepath.Join(tmpDir, "missing-export"), TransformDir: transformDir, diff --git a/cmd/tunnel-api/tunnel-api.go b/cmd/tunnel-api/tunnel-api.go index 2d449433..9ff1b9ee 100644 --- a/cmd/tunnel-api/tunnel-api.go +++ b/cmd/tunnel-api/tunnel-api.go @@ -2,9 +2,9 @@ package tunnel_api import ( "fmt" - "log" "github.com/konveyor/crane-lib/connect/tunnel_api" + "github.com/konveyor/crane/internal/flags" "github.com/sirupsen/logrus" "github.com/spf13/cobra" "k8s.io/cli-runtime/pkg/genericclioptions" @@ -17,8 +17,9 @@ import ( type TunnelAPIOptions struct { configFlags *genericclioptions.ConfigFlags genericclioptions.IOStreams + globalFlags *flags.GlobalFlags + logger logrus.FieldLogger - logger logrus.FieldLogger SourceContext string DestinationContext string Namespace string @@ -32,12 +33,11 @@ type TunnelAPIOptions struct { destinationContext *clientcmdapi.Context } -func NewTunnelAPIOptions(streams genericclioptions.IOStreams) *cobra.Command { +func NewTunnelAPIOptions(streams genericclioptions.IOStreams, f *flags.GlobalFlags) *cobra.Command { t := &TunnelAPIOptions{ configFlags: genericclioptions.NewConfigFlags(false), - - IOStreams: streams, - logger: logrus.New(), + IOStreams: streams, + globalFlags: f, } cmd := &cobra.Command{ @@ -75,6 +75,8 @@ func addFlagsForTunnelAPIOptions(t *TunnelAPIOptions, cmd *cobra.Command) { } func (t *TunnelAPIOptions) Complete(c *cobra.Command, args []string) error { + t.globalFlags.SetCmdName("tunnel-api") + t.logger = t.globalFlags.GetLoggerOrDefault() config := t.configFlags.ToRawKubeConfigLoader() rawConfig, err := config.RawConfig() if err != nil { @@ -134,6 +136,7 @@ func (t *TunnelAPIOptions) getRestConfigFromContext(ctx string) (*rest.Config, e } func (t *TunnelAPIOptions) run() error { + log := t.logger tunnel := tunnel_api.Tunnel{} fmt.Println("Generating SSL certificates. This may take several minutes.") @@ -151,21 +154,21 @@ func (t *TunnelAPIOptions) run() error { srcConfig, err := t.getRestConfigFromContext(t.SourceContext) if err != nil { - log.Fatal(err, "unable to get source config") + log.Fatalf("unable to get source config: %v", err) } dstConfig, err := t.getRestConfigFromContext(t.DestinationContext) if err != nil { - log.Fatal(err, "unable to get destination config") + log.Fatalf("unable to get destination config: %v", err) } _, err = t.getClientFromContext(t.SourceContext) if err != nil { - log.Fatal(err, "unable to get source client") + log.Fatalf("unable to get source client: %v", err) } _, err = t.getClientFromContext(t.DestinationContext) if err != nil { - log.Fatal(err, "unable to get destination client") + log.Fatalf("unable to get destination client: %v", err) } tunnel.SrcConfig = srcConfig @@ -180,7 +183,7 @@ func (t *TunnelAPIOptions) run() error { err = tunnel_api.Openvpn(tunnel) if err != nil { - log.Fatal(err, "Unable to create Tunnel") + log.Fatalf("unable to create tunnel: %v", err) } return nil diff --git a/cmd/validate/validate.go b/cmd/validate/validate.go index d33eb5fd..5135e9ee 100644 --- a/cmd/validate/validate.go +++ b/cmd/validate/validate.go @@ -264,7 +264,7 @@ func (o *ValidateOptions) Run() error { func NewValidateCommand(streams genericclioptions.IOStreams, f *flags.GlobalFlags) *cobra.Command { o := &ValidateOptions{ configFlags: genericclioptions.NewConfigFlags(true), - IOStreams: streams, + IOStreams: streams, cobraGlobalFlags: f, log: logrus.StandardLogger(), } diff --git a/cmd/validate/validate_test.go b/cmd/validate/validate_test.go index 0f9ef9bd..a3f88aa2 100644 --- a/cmd/validate/validate_test.go +++ b/cmd/validate/validate_test.go @@ -43,18 +43,18 @@ func TestNewValidateCommand(t *testing.T) { func TestValidate_Flags(t *testing.T) { tests := []struct { - name string - setup func(t *testing.T) *ValidateOptions - wantErr bool - errMatch string - setMutualExclusionFlags bool // If true, explicitly mark kubeconfig-related flags as changed + name string + setup func(t *testing.T) *ValidateOptions + wantErr bool + errMatch string + setMutualExclusionFlags bool // If true, explicitly mark kubeconfig-related flags as changed }{ { name: "missing input-dir", setup: func(t *testing.T) *ValidateOptions { missingDir := filepath.Join(t.TempDir(), "missing") return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), inputDir: missingDir, outputFormat: "yaml", } @@ -71,7 +71,7 @@ func TestValidate_Flags(t *testing.T) { t.Fatal(err) } return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), inputDir: f, outputFormat: "yaml", } @@ -83,7 +83,7 @@ func TestValidate_Flags(t *testing.T) { name: "invalid output format", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), inputDir: t.TempDir(), outputFormat: "xml", } @@ -95,7 +95,7 @@ func TestValidate_Flags(t *testing.T) { name: "valid yaml format", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), inputDir: t.TempDir(), outputFormat: "yaml", } @@ -106,7 +106,7 @@ func TestValidate_Flags(t *testing.T) { name: "valid json format", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), inputDir: t.TempDir(), outputFormat: "json", } @@ -117,7 +117,7 @@ func TestValidate_Flags(t *testing.T) { name: "uppercase JSON accepted", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), inputDir: t.TempDir(), outputFormat: "JSON", } @@ -128,7 +128,7 @@ func TestValidate_Flags(t *testing.T) { name: "uppercase YAML accepted", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), inputDir: t.TempDir(), outputFormat: "YAML", } @@ -139,7 +139,7 @@ func TestValidate_Flags(t *testing.T) { name: "mixed case Json accepted", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), inputDir: t.TempDir(), outputFormat: "Json", } @@ -150,7 +150,7 @@ func TestValidate_Flags(t *testing.T) { name: "api-resources file not found", setup: func(t *testing.T) *ValidateOptions { return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: genericclioptions.NewConfigFlags(true), inputDir: t.TempDir(), outputFormat: "json", @@ -172,7 +172,7 @@ func TestValidate_Flags(t *testing.T) { cf := genericclioptions.NewConfigFlags(true) cf.Context = &ctx return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: cf, inputDir: dir, outputFormat: "json", @@ -195,7 +195,7 @@ func TestValidate_Flags(t *testing.T) { cf := genericclioptions.NewConfigFlags(true) cf.KubeConfig = &kc return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: cf, inputDir: dir, outputFormat: "json", @@ -218,7 +218,7 @@ func TestValidate_Flags(t *testing.T) { cf := genericclioptions.NewConfigFlags(true) cf.APIServer = &server return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: cf, inputDir: dir, outputFormat: "json", @@ -241,7 +241,7 @@ func TestValidate_Flags(t *testing.T) { cf := genericclioptions.NewConfigFlags(true) cf.BearerToken = &token return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: cf, inputDir: dir, outputFormat: "json", @@ -264,7 +264,7 @@ func TestValidate_Flags(t *testing.T) { cf := genericclioptions.NewConfigFlags(true) cf.ClusterName = &cluster return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: cf, inputDir: dir, outputFormat: "json", @@ -287,7 +287,7 @@ func TestValidate_Flags(t *testing.T) { cf := genericclioptions.NewConfigFlags(true) cf.AuthInfoName = &user return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: cf, inputDir: dir, outputFormat: "json", @@ -307,7 +307,7 @@ func TestValidate_Flags(t *testing.T) { t.Fatal(err) } return &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: genericclioptions.NewConfigFlags(true), inputDir: dir, outputFormat: "json", @@ -395,7 +395,7 @@ func TestRun_EmptyInputDirReturnsError(t *testing.T) { } o := &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: genericclioptions.NewConfigFlags(true), IOStreams: genericclioptions.NewTestIOStreamsDiscard(), inputDir: emptyDir, @@ -502,7 +502,7 @@ current-context: existing-context cf.KubeConfig = &kc o := &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: cf, globalFlags: &flags.GlobalFlags{}, inputDir: t.TempDir(), @@ -525,7 +525,7 @@ current-context: existing-context func TestComplete_SkippedInOfflineMode(t *testing.T) { o := &ValidateOptions{ - log: logrus.StandardLogger(), + log: logrus.StandardLogger(), configFlags: genericclioptions.NewConfigFlags(true), globalFlags: &flags.GlobalFlags{}, inputDir: t.TempDir(), diff --git a/internal/audit/audit_logger.go b/internal/audit/audit_logger.go index 7dfed53f..0b87864f 100644 --- a/internal/audit/audit_logger.go +++ b/internal/audit/audit_logger.go @@ -22,7 +22,7 @@ func NewFileHook(path string, cmd *string) (*FileHook, error) { if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { return nil, err } - f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) if err != nil { return nil, err } @@ -48,10 +48,12 @@ func (h *FileHook) Levels() []logrus.Level { // Fire is called by logrus for every log entry. It formats the entry as JSON and writes it to the file. func (h *FileHook) Fire(entry *logrus.Entry) error { + e := entry if h.cmd != nil && *h.cmd != "" { - entry.Data["cmd"] = *h.cmd + e = entry.Dup() + e.Data["cmd"] = *h.cmd } - line, err := h.formatter.Format(entry) + line, err := h.formatter.Format(e) if err != nil { return fmt.Errorf("audit file hook: format entry: %w", err) } diff --git a/internal/flags/global_flags.go b/internal/flags/global_flags.go index af900fac..04f17752 100644 --- a/internal/flags/global_flags.go +++ b/internal/flags/global_flags.go @@ -15,7 +15,7 @@ type GlobalFlags struct { ConfigFile string Debug bool AuditLogPath string `mapstructure:"audit-log"` - CmdName string + cmdName string logger *logrus.Logger fileHook *audit.FileHook } @@ -31,7 +31,7 @@ func (g *GlobalFlags) ApplyFlags(cmd *cobra.Command) { // SetCmdName sets the command name used in audit log entries. Safe to call on a nil GlobalFlags. func (g *GlobalFlags) SetCmdName(name string) { if g != nil { - g.CmdName = name + g.cmdName = name } } @@ -43,19 +43,27 @@ func (g *GlobalFlags) GetLoggerOrDefault() *logrus.Logger { return g.GetLogger() } +// isCompletionMode returns true when cobra is running a shell completion request. +// In that case we skip the audit file hook to avoid creating audit/ during tab-completion. +func isCompletionMode() bool { + return len(os.Args) > 1 && (os.Args[1] == "__complete" || os.Args[1] == "__completeNoDesc") +} + func (g *GlobalFlags) GetLogger() *logrus.Logger { if g.logger == nil { g.logger = logrus.New() g.logger.SetLevel(logrus.DebugLevel) g.logger.SetOutput(io.Discard) consoleHook := audit.NewConsoleHook(g.Debug) - fileHook, err := audit.NewFileHook(g.AuditLogPath, &g.CmdName) g.logger.AddHook(consoleHook) - if err == nil { - g.fileHook = fileHook - g.logger.AddHook(fileHook) - } else { - g.logger.Warnf("Failed to open audit log file %s: %v", g.AuditLogPath, err) + if !isCompletionMode() { + fileHook, err := audit.NewFileHook(g.AuditLogPath, &g.cmdName) + if err == nil { + g.fileHook = fileHook + g.logger.AddHook(fileHook) + } else { + g.logger.Warnf("Failed to open audit log file %s: %v", g.AuditLogPath, err) + } } } return g.logger diff --git a/main.go b/main.go index 5d432536..8624cf37 100644 --- a/main.go +++ b/main.go @@ -37,8 +37,8 @@ func run() int { f.ApplyFlags(&root) root.AddCommand(export.NewExportCommand(genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}, f)) root.AddCommand(transfer_pvc.NewTransferPVCCommand(genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}, f)) - root.AddCommand(tunnel_api.NewTunnelAPIOptions(genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr})) - root.AddCommand(convert.NewConvertOptions(genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr})) + root.AddCommand(tunnel_api.NewTunnelAPIOptions(genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}, f)) + root.AddCommand(convert.NewConvertOptions(genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}, f)) root.AddCommand(transform.NewTransformCommand(f)) root.AddCommand(skopeo_sync_gen.NewSkopeoSyncGenCommand(f)) root.AddCommand(apply.NewApplyCommand(f)) From 3e7d70f12aa68fe9908214f47cda0974d4771207 Mon Sep 17 00:00:00 2001 From: Tamar-Dinavetsky Date: Tue, 1 Sep 2026 00:09:44 +0300 Subject: [PATCH 6/8] Update by comments --- cmd/transfer-pvc/transfer-pvc.go | 3 ++- cmd/tunnel-api/tunnel-api.go | 20 +++++++++++++++----- internal/audit/audit_logger.go | 11 +++++++---- internal/audit/audit_logger_test.go | 25 ++++++++++++++++++++++++- 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/cmd/transfer-pvc/transfer-pvc.go b/cmd/transfer-pvc/transfer-pvc.go index 6eb048ba..0fba7a8e 100644 --- a/cmd/transfer-pvc/transfer-pvc.go +++ b/cmd/transfer-pvc/transfer-pvc.go @@ -357,7 +357,8 @@ func (t *TransferPVCCommand) getRestConfigFromContext(ctx string) (*rest.Config, func (t *TransferPVCCommand) run() (retErr error) { log := t.log log.Infof("Starting PVC transfer: %s/%s -> %s/%s", t.PVC.Namespace.source, t.PVC.Name.source, t.PVC.Namespace.destination, t.PVC.Name.destination) - logger := logrusr.New(log).WithName("transfer-pvc") + ctrlLogger := logrus.New() + logger := logrusr.New(ctrlLogger).WithName("transfer-pvc") totalPhases := 7 if t.isIntraClusterSameNamespace() { diff --git a/cmd/tunnel-api/tunnel-api.go b/cmd/tunnel-api/tunnel-api.go index 9ff1b9ee..547c8773 100644 --- a/cmd/tunnel-api/tunnel-api.go +++ b/cmd/tunnel-api/tunnel-api.go @@ -100,15 +100,20 @@ func (t *TunnelAPIOptions) Complete(c *cobra.Command, args []string) error { } func (t *TunnelAPIOptions) Validate() error { + log := t.logger + if t.sourceContext == nil { + log.Debugf("Source context %q not found in kubeconfig", t.SourceContext) return fmt.Errorf("cannot evaluate source context") } if t.destinationContext == nil { + log.Debugf("Destination context %q not found in kubeconfig", t.DestinationContext) return fmt.Errorf("cannot evaluate destination context") } if t.sourceContext.Cluster == t.destinationContext.Cluster { + log.Debugf("Source and destination cluster are the same: %q", t.sourceContext.Cluster) return fmt.Errorf("both source and destination cluster are same, this is not supported") } @@ -154,21 +159,25 @@ func (t *TunnelAPIOptions) run() error { srcConfig, err := t.getRestConfigFromContext(t.SourceContext) if err != nil { - log.Fatalf("unable to get source config: %v", err) + log.Debugf("Unable to get source config: %v", err) + return fmt.Errorf("unable to get source config: %w", err) } dstConfig, err := t.getRestConfigFromContext(t.DestinationContext) if err != nil { - log.Fatalf("unable to get destination config: %v", err) + log.Debugf("Unable to get destination config: %v", err) + return fmt.Errorf("unable to get destination config: %w", err) } _, err = t.getClientFromContext(t.SourceContext) if err != nil { - log.Fatalf("unable to get source client: %v", err) + log.Debugf("Unable to get source client: %v", err) + return fmt.Errorf("unable to get source client: %w", err) } _, err = t.getClientFromContext(t.DestinationContext) if err != nil { - log.Fatalf("unable to get destination client: %v", err) + log.Debugf("Unable to get destination client: %v", err) + return fmt.Errorf("unable to get destination client: %w", err) } tunnel.SrcConfig = srcConfig @@ -183,7 +192,8 @@ func (t *TunnelAPIOptions) run() error { err = tunnel_api.Openvpn(tunnel) if err != nil { - log.Fatalf("unable to create tunnel: %v", err) + log.Debugf("Unable to create tunnel: %v", err) + return fmt.Errorf("unable to create tunnel: %w", err) } return nil diff --git a/internal/audit/audit_logger.go b/internal/audit/audit_logger.go index 0b87864f..1b20f605 100644 --- a/internal/audit/audit_logger.go +++ b/internal/audit/audit_logger.go @@ -26,6 +26,10 @@ func NewFileHook(path string, cmd *string) (*FileHook, error) { if err != nil { return nil, err } + if err := f.Chmod(0600); err != nil { + f.Close() + return nil, err + } return &FileHook{ file: f, cmd: cmd, @@ -48,12 +52,11 @@ func (h *FileHook) Levels() []logrus.Level { // Fire is called by logrus for every log entry. It formats the entry as JSON and writes it to the file. func (h *FileHook) Fire(entry *logrus.Entry) error { - e := entry if h.cmd != nil && *h.cmd != "" { - e = entry.Dup() - e.Data["cmd"] = *h.cmd + entry.Data["cmd"] = *h.cmd + defer delete(entry.Data, "cmd") } - line, err := h.formatter.Format(e) + line, err := h.formatter.Format(entry) if err != nil { return fmt.Errorf("audit file hook: format entry: %w", err) } diff --git a/internal/audit/audit_logger_test.go b/internal/audit/audit_logger_test.go index ee8a53d3..7029eadf 100644 --- a/internal/audit/audit_logger_test.go +++ b/internal/audit/audit_logger_test.go @@ -78,6 +78,28 @@ func TestNewFileHook(t *testing.T) { } } +func TestNewFileHook_EnforcesPermissionsOnExistingFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "existing.log") + // Create file with permissive 0644 permissions + if err := os.WriteFile(path, []byte("old content\n"), 0644); err != nil { + t.Fatalf("setup: %v", err) + } + hook, err := NewFileHook(path, nil) + if err != nil { + t.Fatalf("NewFileHook: %v", err) + } + if err := hook.Close(); err != nil { + t.Errorf("Close: %v", err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatalf("Stat: %v", err) + } + if got := info.Mode().Perm(); got != 0600 { + t.Errorf("permissions = %04o, want 0600", got) + } +} + func TestFileHook_Fire_WritesJSON(t *testing.T) { path := filepath.Join(t.TempDir(), "audit.log") hook, err := NewFileHook(path, nil) @@ -199,7 +221,8 @@ func TestFileHook_Fire_InjectsCmdField(t *testing.T) { } }() - if err := hook.Fire(makeEntry("test", logrus.InfoLevel)); err != nil { + entry := makeEntry("test", logrus.InfoLevel) + if err := hook.Fire(entry); err != nil { t.Fatalf("Fire: %v", err) } From 0b564461a524a588fe8a10ed054e23870c39bab1 Mon Sep 17 00:00:00 2001 From: Tamar-Dinavetsky Date: Tue, 1 Sep 2026 10:40:51 +0300 Subject: [PATCH 7/8] Update by comments --- .gitignore | 2 +- cmd/convert/convert.go | 6 ++---- internal/audit/audit_logger.go | 13 ++++++++++--- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index 42b4e389..680856e7 100644 --- a/.gitignore +++ b/.gitignore @@ -18,4 +18,4 @@ vendor/ crane .DS_Store -audit/ \ No newline at end of file +audit/ diff --git a/cmd/convert/convert.go b/cmd/convert/convert.go index dd067a90..da43ffb6 100644 --- a/cmd/convert/convert.go +++ b/cmd/convert/convert.go @@ -71,12 +71,10 @@ func addFlagsForConvertOptions(t *ConvertOptions, cmd *cobra.Command) { func (t *ConvertOptions) Complete(c *cobra.Command, args []string) error { t.globalFlags.SetCmdName("convert") - t.Logger = t.globalFlags.GetLoggerOrDefault() if t.debug { - if logger, ok := t.Logger.(*logrus.Logger); ok { - logger.SetLevel(logrus.DebugLevel) - } + t.globalFlags.Debug = true } + t.Logger = t.globalFlags.GetLoggerOrDefault() return nil } diff --git a/internal/audit/audit_logger.go b/internal/audit/audit_logger.go index 1b20f605..ecc68d15 100644 --- a/internal/audit/audit_logger.go +++ b/internal/audit/audit_logger.go @@ -52,11 +52,18 @@ func (h *FileHook) Levels() []logrus.Level { // Fire is called by logrus for every log entry. It formats the entry as JSON and writes it to the file. func (h *FileHook) Fire(entry *logrus.Entry) error { + e := entry if h.cmd != nil && *h.cmd != "" { - entry.Data["cmd"] = *h.cmd - defer delete(entry.Data, "cmd") + data := make(logrus.Fields, len(entry.Data)+1) + for k, v := range entry.Data { + data[k] = v + } + data["cmd"] = *h.cmd + clone := *entry + clone.Data = data + e = &clone } - line, err := h.formatter.Format(entry) + line, err := h.formatter.Format(e) if err != nil { return fmt.Errorf("audit file hook: format entry: %w", err) } From 737166ac566a1ca50ed55ae1cf513f4a5ac16216 Mon Sep 17 00:00:00 2001 From: Tamar-Dinavetsky Date: Tue, 1 Sep 2026 11:54:04 +0300 Subject: [PATCH 8/8] fix f.Close --- internal/audit/audit_logger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/audit/audit_logger.go b/internal/audit/audit_logger.go index ecc68d15..498ab36c 100644 --- a/internal/audit/audit_logger.go +++ b/internal/audit/audit_logger.go @@ -27,7 +27,7 @@ func NewFileHook(path string, cmd *string) (*FileHook, error) { return nil, err } if err := f.Chmod(0600); err != nil { - f.Close() + _ = f.Close() return nil, err } return &FileHook{