diff --git a/pkg/app/app_config.go b/pkg/app/app_config.go index 64c47a63..da5f19e0 100644 --- a/pkg/app/app_config.go +++ b/pkg/app/app_config.go @@ -16,6 +16,7 @@ type AppSettings struct { ListenPort string `env:"LISTEN_PORT"` PrometheusMetricsPrefix string `env:"PROMETHEUS_METRICS_PREFIX"` Namespace string `env:"NAMESPACE"` + SkipInvalidHooks bool `env:"SKIP_INVALID_HOOKS"` } // KubeSettings holds Kubernetes connection parameters. diff --git a/pkg/app/flags.go b/pkg/app/flags.go index 8c174362..b217a363 100644 --- a/pkg/app/flags.go +++ b/pkg/app/flags.go @@ -38,6 +38,7 @@ func bindAppFlags(cfg *Config, cmd *cobra.Command) { f.StringVar(&cfg.App.ListenPort, "listen-port", cfg.App.ListenPort, "Port to use to serve metrics to Prometheus. Can be set with $SHELL_OPERATOR_LISTEN_PORT.") f.StringVar(&cfg.App.PrometheusMetricsPrefix, "prometheus-metrics-prefix", cfg.App.PrometheusMetricsPrefix, "Prefix for Prometheus metrics. Can be set with $SHELL_OPERATOR_PROMETHEUS_METRICS_PREFIX.") f.StringVar(&cfg.App.Namespace, "namespace", cfg.App.Namespace, "A namespace of a shell-operator. Used to set up validating webhooks. Can be set with $SHELL_OPERATOR_NAMESPACE.") + f.BoolVar(&cfg.App.SkipInvalidHooks, "skip-invalid-hooks", cfg.App.SkipInvalidHooks, "If true, log a warning and skip hooks that fail --config instead of aborting startup. Can be set with $SHELL_OPERATOR_SKIP_INVALID_HOOKS.") } func bindKubeFlags(cfg *Config, cmd *cobra.Command) { diff --git a/pkg/hook/hook_manager.go b/pkg/hook/hook_manager.go index 1f56ff2a..c6ae56c6 100644 --- a/pkg/hook/hook_manager.go +++ b/pkg/hook/hook_manager.go @@ -44,6 +44,7 @@ type Manager struct { keepTemporaryHookFiles bool logProxyHookJSON bool logProxyHookJSONKey string + skipInvalidHooks bool // sorted hook names hookNamesInOrder []string @@ -75,6 +76,7 @@ type ManagerConfig struct { KeepTemporaryHookFiles bool LogProxyHookJSON bool LogProxyHookJSONKey string + SkipInvalidHooks bool Logger *log.Logger } @@ -101,6 +103,7 @@ func NewHookManager(config *ManagerConfig) *Manager { keepTemporaryHookFiles: config.KeepTemporaryHookFiles, logProxyHookJSON: config.LogProxyHookJSON, logProxyHookJSONKey: config.LogProxyHookJSONKey, + skipInvalidHooks: config.SkipInvalidHooks, logger: config.Logger, } @@ -139,6 +142,10 @@ func (hm *Manager) Init() error { for _, hookPath := range hookPaths { hook, err := hm.loadHook(hookPath) if err != nil { + if hm.skipInvalidHooks { + hm.logger.Warn("Skipping hook with invalid config", slog.String("hook", hookPath), log.Err(err)) + continue + } return err } diff --git a/pkg/hook/hook_manager_discrete_test.go b/pkg/hook/hook_manager_discrete_test.go index ccdb9336..bd45595b 100644 --- a/pkg/hook/hook_manager_discrete_test.go +++ b/pkg/hook/hook_manager_discrete_test.go @@ -1,6 +1,7 @@ package hook import ( + "path/filepath" "testing" "github.com/deckhouse/deckhouse/pkg/log" @@ -70,6 +71,43 @@ func TestLoadHook_allHooksHaveNonNilConfig(t *testing.T) { } } +// TestInit_skipInvalidHooks verifies that when SkipInvalidHooks is true, a hook +// that fails --config is silently skipped while valid hooks are still registered. +func TestInit_skipInvalidHooks(t *testing.T) { + conversionManager := conversion.NewWebhookManager() + conversionManager.Settings = conversion.DefaultSettings + admissionManager := admission.NewWebhookManager(nil) + admissionManager.Settings = admission.DefaultSettings + + // Use the existing valid hook from testdata alongside a nonexistent binary. + // WorkingDir must be the testdata dir so filepath.Rel produces "hook.sh" as the name. + workingDir, _ := filepath.Abs("testdata/hook_manager") + validHookPath := filepath.Join(workingDir, "hook.sh") + cfg := &ManagerConfig{ + WorkingDir: workingDir, + TempDir: t.TempDir(), + AdmissionWebhookManager: admissionManager, + ConversionWebhookManager: conversionManager, + SkipInvalidHooks: true, + HookDiscovery: staticHookDiscovery{paths: []string{validHookPath, "/nonexistent-binary"}}, + Logger: log.NewNop(), + } + hm := NewHookManager(cfg) + + err := hm.Init() + require.NoError(t, err, "Init should not fail when SkipInvalidHooks is true") + assert.Equal(t, []string{"hook.sh"}, hm.GetHookNames(), "only the valid hook should be registered") +} + +// staticHookDiscovery is a HookDiscovery stub that returns a fixed list of paths. +type staticHookDiscovery struct { + paths []string +} + +func (s staticHookDiscovery) Discover(_ string) ([]string, error) { + return s.paths, nil +} + // TestFetchHookConfig_returnsErrorForNonExecutable verifies that fetchHookConfig // returns an error when the hook binary doesn't exist or fails. func TestFetchHookConfig_returnsErrorForNonExecutable(t *testing.T) { diff --git a/pkg/shell-operator/bootstrap.go b/pkg/shell-operator/bootstrap.go index d1313488..494e1949 100644 --- a/pkg/shell-operator/bootstrap.go +++ b/pkg/shell-operator/bootstrap.go @@ -248,6 +248,7 @@ func (op *ShellOperator) setupHookManagers(cfg *app.Config, hooksDir string, tem KeepTemporaryHookFiles: cfg.Debug.KeepTempFiles, LogProxyHookJSON: cfg.Log.ProxyHookJSON, LogProxyHookJSONKey: app.ProxyJsonLogKey, + SkipInvalidHooks: cfg.App.SkipInvalidHooks, Logger: op.logger.Named("hook-manager"), } op.HookManager = hook.NewHookManager(hookCfg)