diff --git a/pkg/app/app.go b/pkg/app/app.go index 4d5561d3..c4717dd1 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -28,10 +28,9 @@ var ( var PrometheusMetricsPrefix = "shell_operator_" type FlagInfo struct { - Name string - Help string - Envar string - Define bool + Name string + Help string + Envar string } var CommonFlagsInfo = map[string]FlagInfo{ @@ -39,97 +38,76 @@ var CommonFlagsInfo = map[string]FlagInfo{ "hooks-dir", "A path to a hooks file structure. Can be set with $SHELL_OPERATOR_HOOKS_DIR.", "SHELL_OPERATOR_HOOKS_DIR", - true, }, "tmp-dir": { "tmp-dir", "A path to store temporary files with data for hooks. Can be set with $SHELL_OPERATOR_TMP_DIR.", "SHELL_OPERATOR_TMP_DIR", - true, }, "listen-address": { "listen-address", "Address to use to serve metrics to Prometheus. Can be set with $SHELL_OPERATOR_LISTEN_ADDRESS.", "SHELL_OPERATOR_LISTEN_ADDRESS", - true, }, "listen-port": { "listen-port", "Port to use to serve metrics to Prometheus. Can be set with $SHELL_OPERATOR_LISTEN_PORT.", "SHELL_OPERATOR_LISTEN_PORT", - true, }, "prometheus-metrics-prefix": { "prometheus-metrics-prefix", "Prefix for Prometheus metrics. Can be set with $SHELL_OPERATOR_PROMETHEUS_METRICS_PREFIX.", "SHELL_OPERATOR_PROMETHEUS_METRICS_PREFIX", - true, }, "hook-metrics-listen-port": { "hook-metrics-listen-port", "Port to use to serve hooks’ custom metrics to Prometheus. Can be set with $SHELL_OPERATOR_HOOK_METRICS_LISTEN_PORT. Equal to listen-port if empty.", "SHELL_OPERATOR_HOOK_METRICS_LISTEN_PORT", - true, }, "namespace": { "namespace", "A namespace of a shell-operator. Used to setup validating webhooks. Can be set with $SHELL_OPERATOR_NAMESPACE.", "SHELL_OPERATOR_NAMESPACE", - true, }, } // DefineStartCommandFlags set shell-operator flags for cmd func DefineStartCommandFlags(kpApp *kingpin.Application, cmd *kingpin.CmdClause) { - var flag FlagInfo - - flag = CommonFlagsInfo["hooks-dir"] - if flag.Define { - cmd.Flag(flag.Name, flag.Help). - Envar(flag.Envar). - Default(HooksDir). - StringVar(&HooksDir) - } + flag := CommonFlagsInfo["hooks-dir"] + cmd.Flag(flag.Name, flag.Help). + Envar(flag.Envar). + Default(HooksDir). + StringVar(&HooksDir) flag = CommonFlagsInfo["tmp-dir"] - if flag.Define { - cmd.Flag(flag.Name, flag.Help). - Envar(flag.Envar). - Default(TempDir). - StringVar(&TempDir) - } + cmd.Flag(flag.Name, flag.Help). + Envar(flag.Envar). + Default(TempDir). + StringVar(&TempDir) flag = CommonFlagsInfo["listen-address"] - if flag.Define { - cmd.Flag(flag.Name, flag.Help). - Envar(flag.Envar). - Default(ListenAddress). - StringVar(&ListenAddress) - } + cmd.Flag(flag.Name, flag.Help). + Envar(flag.Envar). + Default(ListenAddress). + StringVar(&ListenAddress) flag = CommonFlagsInfo["listen-port"] - if flag.Define { - cmd.Flag(flag.Name, flag.Help). - Envar(flag.Envar). - Default(ListenPort). - StringVar(&ListenPort) - } + cmd.Flag(flag.Name, flag.Help). + Envar(flag.Envar). + Default(ListenPort). + StringVar(&ListenPort) flag = CommonFlagsInfo["prometheus-metrics-prefix"] - if flag.Define { - cmd.Flag(flag.Name, flag.Help). - Envar(flag.Envar). - Default(PrometheusMetricsPrefix). - StringVar(&PrometheusMetricsPrefix) - } + cmd.Flag(flag.Name, flag.Help). + Envar(flag.Envar). + Default(PrometheusMetricsPrefix). + StringVar(&PrometheusMetricsPrefix) flag = CommonFlagsInfo["namespace"] - if flag.Define { - cmd.Flag(flag.Name, flag.Help). - Envar(flag.Envar). - Default(Namespace). - StringVar(&Namespace) - } + cmd.Flag(flag.Name, flag.Help). + Envar(flag.Envar). + Default(Namespace). + StringVar(&Namespace) DefineKubeClientFlags(cmd) DefineValidatingWebhookFlags(cmd) diff --git a/pkg/hook/hook.go b/pkg/hook/hook.go index cac75b45..782c1229 100644 --- a/pkg/hook/hook.go +++ b/pkg/hook/hook.go @@ -29,10 +29,6 @@ const ( serviceName = "hook" ) -type CommonHook interface { - Name() string -} - type Result struct { Usage *executor.CmdUsage Metrics []operation.MetricOperation diff --git a/pkg/hook/hook_discovery_test.go b/pkg/hook/hook_discovery_test.go new file mode 100644 index 00000000..558b8d2b --- /dev/null +++ b/pkg/hook/hook_discovery_test.go @@ -0,0 +1,143 @@ +package hook + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestFileSystemHookDiscovery_Discover_emptyDir returns no paths for an empty directory. +func TestFileSystemHookDiscovery_Discover_emptyDir(t *testing.T) { + dir := t.TempDir() + d := FileSystemHookDiscovery{} + paths, err := d.Discover(dir) + require.NoError(t, err) + assert.Empty(t, paths) +} + +// TestFileSystemHookDiscovery_Discover_findsExecutables returns executable files. +func TestFileSystemHookDiscovery_Discover_findsExecutables(t *testing.T) { + dir := t.TempDir() + + // Create an executable file. + execPath := filepath.Join(dir, "my-hook.sh") + err := os.WriteFile(execPath, []byte("#!/bin/sh\necho ok\n"), 0o755) + require.NoError(t, err) + + d := FileSystemHookDiscovery{} + paths, err := d.Discover(dir) + require.NoError(t, err) + assert.Contains(t, paths, execPath) +} + +// TestFileSystemHookDiscovery_Discover_ignoresNonExecutables skips regular files. +func TestFileSystemHookDiscovery_Discover_ignoresNonExecutables(t *testing.T) { + dir := t.TempDir() + + // Non-executable file. + nonExecPath := filepath.Join(dir, "README.md") + err := os.WriteFile(nonExecPath, []byte("docs"), 0o644) + require.NoError(t, err) + + d := FileSystemHookDiscovery{} + paths, err := d.Discover(dir) + require.NoError(t, err) + assert.NotContains(t, paths, nonExecPath) +} + +// TestFileSystemHookDiscovery_Discover_recursive finds executables in subdirectories. +func TestFileSystemHookDiscovery_Discover_recursive(t *testing.T) { + dir := t.TempDir() + subdir := filepath.Join(dir, "subdir") + require.NoError(t, os.MkdirAll(subdir, 0o755)) + + hookPath := filepath.Join(subdir, "hook.sh") + require.NoError(t, os.WriteFile(hookPath, []byte("#!/bin/sh\n"), 0o755)) + + d := FileSystemHookDiscovery{} + paths, err := d.Discover(dir) + require.NoError(t, err) + assert.Contains(t, paths, hookPath) +} + +// TestNewHookManager_defaultDiscovery verifies that a nil HookDiscovery in +// ManagerConfig defaults to FileSystemHookDiscovery inside the Manager. +func TestNewHookManager_defaultDiscovery(t *testing.T) { + hm := newHookManager(t, t.TempDir()) + hm.hookDiscovery = nil // reset to exercise the nil-default path via NewHookManager + cfg := &ManagerConfig{ + WorkingDir: t.TempDir(), + TempDir: t.TempDir(), + HookDiscovery: nil, // explicitly nil → should default + AdmissionWebhookManager: hm.admissionWebhookManager, + ConversionWebhookManager: hm.conversionWebhookManager, + Logger: hm.logger, + } + hm2 := NewHookManager(cfg) + require.NotNil(t, hm2) + assert.IsType(t, FileSystemHookDiscovery{}, hm2.hookDiscovery) +} + +// TestNewHookManager_injectedDiscovery verifies that a custom HookDiscovery is +// stored as-is in the Manager. +func TestNewHookManager_injectedDiscovery(t *testing.T) { + stub := &stubDiscovery{} + hm := newHookManager(t, t.TempDir()) + cfg := &ManagerConfig{ + WorkingDir: t.TempDir(), + TempDir: t.TempDir(), + HookDiscovery: stub, + AdmissionWebhookManager: hm.admissionWebhookManager, + ConversionWebhookManager: hm.conversionWebhookManager, + Logger: hm.logger, + } + hm2 := NewHookManager(cfg) + assert.Equal(t, stub, hm2.hookDiscovery) +} + +// TestManager_Init_usesInjectedDiscovery verifies that Manager.Init calls +// HookDiscovery.Discover rather than the filesystem when a custom discovery is +// injected. An empty result means Init succeeds with zero hooks loaded. +func TestManager_Init_usesInjectedDiscovery(t *testing.T) { + stub := &stubDiscovery{paths: []string{}} // returns nothing + hm := newHookManager(t, t.TempDir()) + hm.hookDiscovery = stub + + err := hm.Init() + require.NoError(t, err) + assert.True(t, stub.called, "Discover should have been called") + assert.Equal(t, []string{}, hm.GetHookNames()) +} + +// TestManager_Init_discoveryError propagates discovery errors. +func TestManager_Init_discoveryError(t *testing.T) { + stub := &stubDiscovery{err: errStubDiscovery} + hm := newHookManager(t, t.TempDir()) + hm.hookDiscovery = stub + + err := hm.Init() + require.Error(t, err) + assert.ErrorIs(t, err, errStubDiscovery) +} + +// ---- helpers ---- + +var errStubDiscovery = &testError{"stub discovery error"} + +type testError struct{ msg string } + +func (e *testError) Error() string { return e.msg } + +type stubDiscovery struct { + paths []string + err error + called bool +} + +func (s *stubDiscovery) Discover(_ string) ([]string, error) { + s.called = true + return s.paths, s.err +} diff --git a/pkg/hook/hook_manager.go b/pkg/hook/hook_manager.go index f05548c8..1f56ff2a 100644 --- a/pkg/hook/hook_manager.go +++ b/pkg/hook/hook_manager.go @@ -36,6 +36,10 @@ type Manager struct { conversionWebhookManager *conversion.WebhookManager admissionWebhookManager *admission.WebhookManager + // hookDiscovery resolves the set of hook executables to load. + // Defaults to FileSystemHookDiscovery; tests may inject a stub. + hookDiscovery HookDiscovery + // hook execution options keepTemporaryHookFiles bool logProxyHookJSON bool @@ -64,6 +68,10 @@ type ManagerConfig struct { AdmissionWebhookManager *admission.WebhookManager ConversionWebhookManager *conversion.WebhookManager + // HookDiscovery overrides the default filesystem-based hook discovery. + // When nil, FileSystemHookDiscovery is used. + HookDiscovery HookDiscovery + KeepTemporaryHookFiles bool LogProxyHookJSON bool LogProxyHookJSONKey string @@ -72,6 +80,10 @@ type ManagerConfig struct { } func NewHookManager(config *ManagerConfig) *Manager { + disc := config.HookDiscovery + if disc == nil { + disc = FileSystemHookDiscovery{} + } return &Manager{ hooksByName: make(map[string]*Hook), hookNamesInOrder: make([]string, 0), @@ -84,6 +96,7 @@ func NewHookManager(config *ManagerConfig) *Manager { scheduleManager: config.ScheduleManager, admissionWebhookManager: config.AdmissionWebhookManager, conversionWebhookManager: config.ConversionWebhookManager, + hookDiscovery: disc, keepTemporaryHookFiles: config.KeepTemporaryHookFiles, logProxyHookJSON: config.LogProxyHookJSON, @@ -114,16 +127,16 @@ func (hm *Manager) Init() error { log.Err(err)) } - hooksRelativePaths, err := utils_file.RecursiveGetExecutablePaths(hm.workingDir) + hookPaths, err := hm.hookDiscovery.Discover(hm.workingDir) if err != nil { return err } // sort hooks by path - sort.Strings(hooksRelativePaths) - hm.logger.Debug("Search hooks in paths", slog.Any(pkg.LogKeyPaths, hooksRelativePaths)) + sort.Strings(hookPaths) + hm.logger.Debug("Search hooks in paths", slog.Any(pkg.LogKeyPaths, hookPaths)) - for _, hookPath := range hooksRelativePaths { + for _, hookPath := range hookPaths { hook, err := hm.loadHook(hookPath) if err != nil { return err @@ -449,3 +462,37 @@ func (hm *Manager) UpdateConversionChains() error { func (hm *Manager) FindConversionChain(crdName string, rule conversion.Rule) []conversion.Rule { return hm.conversionChains.FindConversionChain(crdName, rule) } + +// HookManager is the interface for the hook manager used by the operator. +// It allows substituting test doubles in unit tests. +type HookManager interface { + Init() error + GetHook(name string) *Hook + GetHookNames() []string + GetHooksInOrder(bindingType htypes.BindingType) ([]string, error) + CreateTasksFromKubeEvent(kubeEvent kemtypes.KubeEvent, createTaskFn func(*Hook, controller.BindingExecutionInfo) task.Task) []task.Task + HandleCreateTasksFromScheduleEvent(crontab string, createTaskFn func(*Hook, controller.BindingExecutionInfo) task.Task) []task.Task + HandleAdmissionEvent(ctx context.Context, event admission.Event, createTaskFn func(*Hook, controller.BindingExecutionInfo)) + DetectAdmissionEventType(event admission.Event) htypes.BindingType + HandleConversionEvent(ctx context.Context, crdName string, request *v1.ConversionRequest, rule conversion.Rule, createTaskFn func(*Hook, controller.BindingExecutionInfo)) + FindConversionChain(crdName string, rule conversion.Rule) []conversion.Rule +} + +// HookDiscovery discovers hook executables to be loaded by the Manager. +// The default implementation scans the filesystem; tests and alternative +// runtimes can supply their own. +type HookDiscovery interface { + // Discover returns a sorted list of absolute paths to hook executables. + Discover(workingDir string) ([]string, error) +} + +// FileSystemHookDiscovery discovers hooks by recursively scanning workingDir +// for executable files. +type FileSystemHookDiscovery struct{} + +func (FileSystemHookDiscovery) Discover(workingDir string) ([]string, error) { + return utils_file.RecursiveGetExecutablePaths(workingDir) +} + +// compile-time assertion +var _ HookManager = (*Manager)(nil) diff --git a/pkg/shell-operator/bootstrap.go b/pkg/shell-operator/bootstrap.go index 4ae038aa..743f694b 100644 --- a/pkg/shell-operator/bootstrap.go +++ b/pkg/shell-operator/bootstrap.go @@ -164,6 +164,10 @@ func (op *ShellOperator) assembleShellOperator(hooksDir string, tempDir string, // Create webhookManagers with dependencies. op.setupHookManagers(hooksDir, tempDir) + // Register the three built-in task type handlers. Extenders may add more + // handlers via op.taskHandlerRegistry.Register() after this call. + op.RegisterBuiltinTaskHandlers() + // Search and configure all hooks. err = op.initHookManager() if err != nil { diff --git a/pkg/shell-operator/combine_binding_context_test.go b/pkg/shell-operator/combine_binding_context_test.go index de69eae8..70501c7a 100644 --- a/pkg/shell-operator/combine_binding_context_test.go +++ b/pkg/shell-operator/combine_binding_context_test.go @@ -39,7 +39,7 @@ func Test_CombineBindingContext_MultipleHooks(t *testing.T) { TaskQueues.NewNamedQueue("test_multiple_hooks", func(_ context.Context, _ task.Task) queue.TaskResult { return queue.TaskResult{ - Status: "Success", + Status: queue.Success, } }, queue.WithCompactableTypes(task_metadata.HookRun), @@ -149,7 +149,7 @@ func Test_CombineBindingContext_Nil_On_NoCombine(t *testing.T) { TaskQueues.NewNamedQueue("test_no_combine", func(_ context.Context, _ task.Task) queue.TaskResult { return queue.TaskResult{ - Status: "Success", + Status: queue.Success, } }, queue.WithCompactableTypes(task_metadata.HookRun), @@ -224,7 +224,7 @@ func Test_CombineBindingContext_Group_Compaction(t *testing.T) { TaskQueues.NewNamedQueue("test_multiple_hooks", func(_ context.Context, _ task.Task) queue.TaskResult { return queue.TaskResult{ - Status: "Success", + Status: queue.Success, } }, queue.WithCompactableTypes(task_metadata.HookRun), @@ -342,7 +342,7 @@ func Test_CombineBindingContext_Group_Type(t *testing.T) { TaskQueues.NewNamedQueue("test_multiple_hooks", func(_ context.Context, _ task.Task) queue.TaskResult { return queue.TaskResult{ - Status: "Success", + Status: queue.Success, } }, queue.WithCompactableTypes(task_metadata.HookRun), diff --git a/pkg/shell-operator/operator.go b/pkg/shell-operator/operator.go index b6daa018..a028bf91 100644 --- a/pkg/shell-operator/operator.go +++ b/pkg/shell-operator/operator.go @@ -24,7 +24,6 @@ import ( metricsstorage "github.com/deckhouse/deckhouse/pkg/metrics-storage" "github.com/gofrs/uuid/v5" "go.opentelemetry.io/otel" - v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" klient "github.com/flant/kube-client/client" pkg "github.com/flant/shell-operator/pkg" @@ -76,10 +75,14 @@ type ShellOperator struct { ManagerEventsHandler *ManagerEventsHandler - HookManager *hook.Manager + HookManager hook.HookManager AdmissionWebhookManager *admission.WebhookManager ConversionWebhookManager *conversion.WebhookManager + + // taskHandlerRegistry maps task types to their executor functions. + // Extenders (addon-operator) may register additional task types. + taskHandlerRegistry *TaskHandlerRegistry } type Option func(operator *ShellOperator) @@ -223,49 +226,9 @@ func (op *ShellOperator) initValidatingWebhookManager() error { h.HookController.EnableAdmissionBindings() } - // Define handler for AdmissionEvent - op.AdmissionWebhookManager.WithAdmissionEventHandler(func(ctx context.Context, event admission.Event) (*admission.Response, error) { - eventBindingType := op.HookManager.DetectAdmissionEventType(event) - logLabels := map[string]string{ - pkg.LogKeyEventID: uuid.Must(uuid.NewV4()).String(), - pkg.LogKeyEvent: string(eventBindingType), - } - logEntry := utils.EnrichLoggerWithLabels(op.logger, logLabels) - logEntry = logEntry.With( - slog.String(pkg.LogKeyType, string(eventBindingType)), - slog.String(pkg.LogKeyConfigurationId, event.ConfigurationId), - slog.String(pkg.LogKeyWebhookID, event.WebhookId)) - logEntry.Debug("Handle event") - - var admissionTask task.Task - op.HookManager.HandleAdmissionEvent(ctx, event, func(hook *hook.Hook, info controller.BindingExecutionInfo) { - admissionTask = globalHookTaskFactory.NewHookRunTask(hook.Name, eventBindingType, info, logLabels) - }) - - // Assert exactly one task is created. - if admissionTask == nil { - logEntry.Error("Possible bug!!! No hook found for event") - return nil, fmt.Errorf("no hook found for '%s' '%s'", event.ConfigurationId, event.WebhookId) - } - - res := op.taskHandler(ctx, admissionTask) - - if res.Status == "Fail" { - return &admission.Response{ - Allowed: false, - Message: "Hook failed", - }, nil - } - - admissionProp := admissionTask.GetProp("admissionResponse") - admissionResponse, ok := admissionProp.(*admission.Response) - if !ok { - logEntry.Error("'admissionResponse' task prop is not of type *AdmissionResponse", - slog.String(pkg.LogKeyType, fmt.Sprintf("%T", admissionProp))) - return nil, fmt.Errorf("hook task prop error") - } - return admissionResponse, nil - }) + // Delegate to the dedicated named type instead of an inlined closure. + admissionHandler := NewAdmissionEventHandler(op.HookManager, op.taskHandler, op.logger) + op.AdmissionWebhookManager.WithAdmissionEventHandler(admissionHandler.Handle) if err := op.AdmissionWebhookManager.Start(); err != nil { return fmt.Errorf("ValidatingWebhookManager start: %w", err) @@ -286,8 +249,9 @@ func (op *ShellOperator) initConversionWebhookManager() error { return nil } - // This handler is called when Kubernetes requests a conversion. - op.ConversionWebhookManager.EventHandlerFn = op.conversionEventHandler + // Assign the dedicated handler type instead of an inlined method. + conversionHandler := NewConversionEventHandler(op.HookManager, op.taskHandler, op.logger) + op.ConversionWebhookManager.EventHandlerFn = conversionHandler.Handle err := op.ConversionWebhookManager.Init() if err != nil { @@ -306,125 +270,51 @@ func (op *ShellOperator) initConversionWebhookManager() error { return nil } -// conversionEventHandler is called when Kubernetes requests a conversion. -func (op *ShellOperator) conversionEventHandler(ctx context.Context, crdName string, request *v1.ConversionRequest) (*conversion.Response, error) { - logLabels := map[string]string{ - pkg.LogKeyEventID: uuid.Must(uuid.NewV4()).String(), - pkg.LogKeyBinding: string(types.KubernetesConversion), - } - logEntry := utils.EnrichLoggerWithLabels(op.logger, logLabels) - - sourceVersions := conversion.ExtractAPIVersions(request.Objects) - logEntry.Info("Handle kubernetesCustomResourceConversion event for crd", - slog.String(pkg.LogKeyName, crdName), - slog.Int(pkg.LogKeyLen, len(request.Objects)), - slog.Any(pkg.LogKeyVersions, sourceVersions)) - - done := false - for _, srcVer := range sourceVersions { - rule := conversion.Rule{ - FromVersion: srcVer, - ToVersion: request.DesiredAPIVersion, - } - convPath := op.HookManager.FindConversionChain(crdName, rule) - if len(convPath) == 0 { - continue - } - logEntry.Info("Find conversion path for rule", - slog.String(pkg.LogKeyName, rule.String()), - slog.Any(pkg.LogKeyValue, convPath)) - - for _, convRule := range convPath { - var convTask task.Task - op.HookManager.HandleConversionEvent(ctx, crdName, request, convRule, func(hook *hook.Hook, info controller.BindingExecutionInfo) { - convTask = globalHookTaskFactory.NewHookRunTask(hook.Name, types.KubernetesConversion, info, logLabels) - }) - - if convTask == nil { - return nil, fmt.Errorf("no hook found for '%s' event for crd/%s", string(types.KubernetesConversion), crdName) - } - - res := op.taskHandler(ctx, convTask) - - if res.Status == "Fail" { - return &conversion.Response{ - FailedMessage: fmt.Sprintf("Hook failed to convert to %s", request.DesiredAPIVersion), - ConvertedObjects: nil, - }, nil - } - - prop := convTask.GetProp("conversionResponse") - response, ok := prop.(*conversion.Response) - if !ok { - logEntry.Error("'conversionResponse' task prop is not of type *conversion.Response", - slog.String(pkg.LogKeyType, fmt.Sprintf("%T", prop))) - return nil, fmt.Errorf("hook task prop error") - } - - // Set response objects as new objects for a next round. - request.Objects = response.ConvertedObjects - - // Stop iterating if hook has converted all objects to a desiredAPIVersions. - newSourceVersions := conversion.ExtractAPIVersions(request.Objects) - // logEntry.Infof("Hook return conversion response: failMsg=%s, %d convertedObjects, versions:%v, desired: %s", response.FailedMessage, len(response.ConvertedObjects), newSourceVersions, event.Review.Request.DesiredAPIVersion) - - if len(newSourceVersions) == 1 && newSourceVersions[0] == request.DesiredAPIVersion { - // success - done = true - break - } - } - - if done { - break - } - } - - if done { - return &conversion.Response{ - ConvertedObjects: request.Objects, - }, nil +// taskHandler dispatches a task to the registered handler in the registry. +// It is the function passed to each task queue as its handler. +func (op *ShellOperator) taskHandler(ctx context.Context, t task.Task) queue.TaskResult { + res, handled := op.taskHandlerRegistry.Handle(ctx, t) + if !handled { + op.logger.Error("Possible Bug! Unknown task type", + slog.String(pkg.LogKeyType, string(t.GetType()))) + return queue.TaskResult{Status: queue.Fail} } - - return &conversion.Response{ - FailedMessage: fmt.Sprintf("Conversion %s to %s was not successful", crdName, request.DesiredAPIVersion), - }, nil + return res } -// taskHandler -func (op *ShellOperator) taskHandler(ctx context.Context, t task.Task) queue.TaskResult { +// taskHandleEnableScheduleBindings enables schedule bindings for the hook referenced by the task. +func (op *ShellOperator) taskHandleEnableScheduleBindings(_ context.Context, t task.Task) queue.TaskResult { logEntry := op.logger.With(pkg.LogKeyOperatorComponent, "taskRunner") hookMeta, ok := task_metadata.HookMetadataAccessor(t) if !ok { - logEntry.Error("Possible Bug! cannot access hook metadata for task", + logEntry.Error("Possible Bug! cannot access hook metadata", slog.String(pkg.LogKeyType, string(t.GetType()))) - return queue.TaskResult{Status: "Fail"} + return queue.TaskResult{Status: queue.Fail} } - var res queue.TaskResult - - switch t.GetType() { - case task_metadata.HookRun: - res = op.taskHandleHookRun(ctx, t) - - case task_metadata.EnableKubernetesBindings: - res = op.taskHandleEnableKubernetesBindings(ctx, t) - case task_metadata.EnableScheduleBindings: - hookLogLabels := map[string]string{} - hookLogLabels[pkg.LogKeyHook] = hookMeta.HookName - hookLogLabels[pkg.LogKeyBinding] = string(types.Schedule) - hookLogLabels[pkg.LogKeyTask] = "EnableScheduleBindings" - hookLogLabels[pkg.LogKeyQueue] = "main" + hookLogLabels := map[string]string{ + pkg.LogKeyHook: hookMeta.HookName, + pkg.LogKeyBinding: string(types.Schedule), + pkg.LogKeyTask: "EnableScheduleBindings", + pkg.LogKeyQueue: "main", + } + taskLogEntry := utils.EnrichLoggerWithLabels(logEntry, hookLogLabels) - taskLogEntry := utils.EnrichLoggerWithLabels(logEntry, hookLogLabels) + taskHook := op.HookManager.GetHook(hookMeta.HookName) + taskHook.HookController.EnableScheduleBindings() + taskLogEntry.Info("Schedule binding for hook enabled successfully") - taskHook := op.HookManager.GetHook(hookMeta.HookName) - taskHook.HookController.EnableScheduleBindings() - taskLogEntry.Info("Schedule binding for hook enabled successfully") - res.Status = "Success" - } + return queue.TaskResult{Status: queue.Success} +} - return res +// RegisterBuiltinTaskHandlers populates the registry with the three core task types. +// It must be called after HookManager is set. Extenders may call Register() afterwards +// to add extra task types without touching this method. +func (op *ShellOperator) RegisterBuiltinTaskHandlers() { + op.taskHandlerRegistry = NewTaskHandlerRegistry() + op.taskHandlerRegistry.Register(task_metadata.HookRun, op.taskHandleHookRun) + op.taskHandlerRegistry.Register(task_metadata.EnableKubernetesBindings, op.taskHandleEnableKubernetesBindings) + op.taskHandlerRegistry.Register(task_metadata.EnableScheduleBindings, op.taskHandleEnableScheduleBindings) } // taskHandleEnableKubernetesBindings creates task for each Kubernetes binding in the hook and queues them. @@ -436,7 +326,7 @@ func (op *ShellOperator) taskHandleEnableKubernetesBindings(ctx context.Context, if !ok { op.logger.Error("Possible Bug! cannot access hook metadata", slog.String(pkg.LogKeyType, string(t.GetType()))) - return queue.TaskResult{Status: "Fail"} + return queue.TaskResult{Status: queue.Fail} } metricLabels := map[string]string{ @@ -474,12 +364,12 @@ func (op *ShellOperator) taskHandleEnableKubernetesBindings(ctx context.Context, taskLogEntry.Error("Enable Kubernetes binding for hook failed. Will retry after delay.", slog.Int(pkg.LogKeyFailedCount, t.GetFailureCount()+1), log.Err(err)) - res.Status = "Fail" + res.Status = queue.Fail } else { success = 1.0 taskLogEntry.Info("Kubernetes bindings for hook are enabled successfully", slog.Int(pkg.LogKeyCount, len(hookRunTasks))) - res.Status = "Success" + res.Status = queue.Success now := time.Now() for _, t := range hookRunTasks { t.WithQueuedAt(now) @@ -502,7 +392,7 @@ func (op *ShellOperator) taskHandleHookRun(ctx context.Context, t task.Task) que if !ok { op.logger.Error("Possible Bug! cannot access hook metadata", slog.String(pkg.LogKeyType, string(t.GetType()))) - return queue.TaskResult{Status: "Fail"} + return queue.TaskResult{Status: queue.Fail} } taskHook := op.HookManager.GetHook(hookMeta.HookName) @@ -510,7 +400,7 @@ func (op *ShellOperator) taskHandleHookRun(ctx context.Context, t task.Task) que if err != nil { // This could happen when the Context is canceled, so just repeat the task until the queue is stopped. return queue.TaskResult{ - Status: "Repeat", + Status: queue.Repeat, } } @@ -572,7 +462,7 @@ func (op *ShellOperator) taskHandleHookRun(ctx context.Context, t task.Task) que var res queue.TaskResult // Default when shouldRunHook is false. - res.Status = "Success" + res.Status = queue.Success if shouldRunHook { taskLogEntry.Info("Execute hook") @@ -585,7 +475,7 @@ func (op *ShellOperator) taskHandleHookRun(ctx context.Context, t task.Task) que if hookMeta.AllowFailure { allowed = 1.0 taskLogEntry.Info("Hook failed, but allowed to fail", log.Err(err)) - res.Status = "Success" + res.Status = queue.Success } else { errors = 1.0 t.UpdateFailureMessage(err.Error()) @@ -593,12 +483,12 @@ func (op *ShellOperator) taskHandleHookRun(ctx context.Context, t task.Task) que taskLogEntry.Error("Hook failed. Will retry after delay.", slog.Int(pkg.LogKeyFailedCount, t.GetFailureCount()+1), log.Err(err)) - res.Status = "Fail" + res.Status = queue.Fail } } else { success = 1.0 taskLogEntry.Info("Hook executed successfully") - res.Status = "Success" + res.Status = queue.Success } op.MetricStorage.CounterAdd(metrics.HookRunAllowedErrorsTotal, allowed, metricLabels) op.MetricStorage.CounterAdd(metrics.HookRunErrorsTotal, errors, metricLabels) @@ -606,7 +496,7 @@ func (op *ShellOperator) taskHandleHookRun(ctx context.Context, t task.Task) que } // Unlock Kubernetes events for all monitors when Synchronization task is done. - if isSynchronization && res.Status == "Success" { + if isSynchronization && res.Status == queue.Success { taskLogEntry.Info("Unlock kubernetes.Event tasks") for _, monitorID := range hookMeta.MonitorIDs { taskHook.HookController.UnlockKubernetesEventsFor(monitorID) diff --git a/pkg/shell-operator/task_handler_registry.go b/pkg/shell-operator/task_handler_registry.go new file mode 100644 index 00000000..e465f29c --- /dev/null +++ b/pkg/shell-operator/task_handler_registry.go @@ -0,0 +1,54 @@ +// Copyright 2025 Flant JSC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package shell_operator + +import ( + "context" + + "github.com/flant/shell-operator/pkg/task" + "github.com/flant/shell-operator/pkg/task/queue" +) + +// TaskHandlerFunc is the function type for handling a single task. +type TaskHandlerFunc func(ctx context.Context, t task.Task) queue.TaskResult + +// TaskHandlerRegistry maps task types to their handlers. +// New task types can be registered without modifying the dispatcher. +type TaskHandlerRegistry struct { + handlers map[task.TaskType]TaskHandlerFunc +} + +// NewTaskHandlerRegistry creates an empty registry. +func NewTaskHandlerRegistry() *TaskHandlerRegistry { + return &TaskHandlerRegistry{ + handlers: make(map[task.TaskType]TaskHandlerFunc), + } +} + +// Register associates a handler with a task type. +// Registering the same type twice overwrites the previous handler. +func (r *TaskHandlerRegistry) Register(taskType task.TaskType, handler TaskHandlerFunc) { + r.handlers[taskType] = handler +} + +// Handle dispatches the task to the registered handler. +// Returns a Fail result when no handler is found. +func (r *TaskHandlerRegistry) Handle(ctx context.Context, t task.Task) (queue.TaskResult, bool) { + h, ok := r.handlers[t.GetType()] + if !ok { + return queue.TaskResult{Status: "Fail"}, false + } + return h(ctx, t), true +} diff --git a/pkg/shell-operator/task_handler_registry_test.go b/pkg/shell-operator/task_handler_registry_test.go new file mode 100644 index 00000000..40053d1d --- /dev/null +++ b/pkg/shell-operator/task_handler_registry_test.go @@ -0,0 +1,99 @@ +package shell_operator + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/flant/shell-operator/pkg/task" + "github.com/flant/shell-operator/pkg/task/queue" +) + +func makeTask(typ task.TaskType) task.Task { + return task.NewTask(typ) +} + +func TestTaskHandlerRegistry_NewTaskHandlerRegistry_empty(t *testing.T) { + r := NewTaskHandlerRegistry() + require.NotNil(t, r) + _, handled := r.Handle(context.Background(), makeTask("any")) + assert.False(t, handled, "empty registry should not handle any task") +} + +func TestTaskHandlerRegistry_Register_and_Handle_found(t *testing.T) { + r := NewTaskHandlerRegistry() + called := false + want := queue.TaskResult{Status: queue.Success} + + r.Register("my-task", func(_ context.Context, _ task.Task) queue.TaskResult { + called = true + return want + }) + + got, handled := r.Handle(context.Background(), makeTask("my-task")) + assert.True(t, handled) + assert.True(t, called) + assert.Equal(t, want, got) +} + +func TestTaskHandlerRegistry_Handle_notFound_returnsFail(t *testing.T) { + r := NewTaskHandlerRegistry() + r.Register("registered", func(_ context.Context, _ task.Task) queue.TaskResult { + return queue.TaskResult{Status: queue.Success} + }) + + got, handled := r.Handle(context.Background(), makeTask("unregistered")) + assert.False(t, handled) + assert.Equal(t, queue.Fail, got.Status) +} + +func TestTaskHandlerRegistry_Register_overwrites(t *testing.T) { + r := NewTaskHandlerRegistry() + r.Register("t", func(_ context.Context, _ task.Task) queue.TaskResult { + return queue.TaskResult{Status: queue.Success} + }) + r.Register("t", func(_ context.Context, _ task.Task) queue.TaskResult { + return queue.TaskResult{Status: queue.Fail} + }) + + got, handled := r.Handle(context.Background(), makeTask("t")) + assert.True(t, handled) + assert.Equal(t, queue.Fail, got.Status) +} + +func TestTaskHandlerRegistry_MultipleTypes(t *testing.T) { + r := NewTaskHandlerRegistry() + r.Register("type-a", func(_ context.Context, _ task.Task) queue.TaskResult { + return queue.TaskResult{Status: queue.Success} + }) + r.Register("type-b", func(_ context.Context, _ task.Task) queue.TaskResult { + return queue.TaskResult{Status: queue.Repeat} + }) + + gotA, handledA := r.Handle(context.Background(), makeTask("type-a")) + gotB, handledB := r.Handle(context.Background(), makeTask("type-b")) + _, handledC := r.Handle(context.Background(), makeTask("type-c")) + + assert.True(t, handledA) + assert.Equal(t, queue.Success, gotA.Status) + assert.True(t, handledB) + assert.Equal(t, queue.Repeat, gotB.Status) + assert.False(t, handledC) +} + +func TestTaskHandlerRegistry_Handle_passesTaskToHandler(t *testing.T) { + r := NewTaskHandlerRegistry() + var receivedTask task.Task + + r.Register("my-task", func(_ context.Context, t task.Task) queue.TaskResult { + receivedTask = t + return queue.TaskResult{Status: queue.Success} + }) + + input := makeTask("my-task") + r.Handle(context.Background(), input) + + assert.Equal(t, input.GetId(), receivedTask.GetId()) +} diff --git a/pkg/shell-operator/webhook_handlers.go b/pkg/shell-operator/webhook_handlers.go new file mode 100644 index 00000000..4cd5d712 --- /dev/null +++ b/pkg/shell-operator/webhook_handlers.go @@ -0,0 +1,201 @@ +// Copyright 2025 Flant JSC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package shell_operator + +import ( + "context" + "fmt" + "log/slog" + + "github.com/deckhouse/deckhouse/pkg/log" + "github.com/gofrs/uuid/v5" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + + pkg "github.com/flant/shell-operator/pkg" + "github.com/flant/shell-operator/pkg/hook" + "github.com/flant/shell-operator/pkg/hook/controller" + "github.com/flant/shell-operator/pkg/hook/types" + "github.com/flant/shell-operator/pkg/task" + "github.com/flant/shell-operator/pkg/task/queue" + utils "github.com/flant/shell-operator/pkg/utils/labels" + "github.com/flant/shell-operator/pkg/webhook/admission" + "github.com/flant/shell-operator/pkg/webhook/conversion" +) + +// TaskRunner executes a task synchronously and returns its result. +// ShellOperator.taskHandler satisfies this type. +type TaskRunner func(ctx context.Context, t task.Task) queue.TaskResult + +// AdmissionEventHandler handles admission webhook events by creating a hook task, +// running it synchronously, and returning the hook's admission response. +type AdmissionEventHandler struct { + hookManager hook.HookManager + taskRunner TaskRunner + logger *log.Logger +} + +// NewAdmissionEventHandler creates a new AdmissionEventHandler. +func NewAdmissionEventHandler(hm hook.HookManager, runner TaskRunner, logger *log.Logger) *AdmissionEventHandler { + return &AdmissionEventHandler{ + hookManager: hm, + taskRunner: runner, + logger: logger, + } +} + +// Handle implements the admission event handler func signature expected by +// admission.WebhookManager.WithAdmissionEventHandler. +func (h *AdmissionEventHandler) Handle(ctx context.Context, event admission.Event) (*admission.Response, error) { + eventBindingType := h.hookManager.DetectAdmissionEventType(event) + logLabels := map[string]string{ + pkg.LogKeyEventID: uuid.Must(uuid.NewV4()).String(), + pkg.LogKeyEvent: string(eventBindingType), + } + logEntry := utils.EnrichLoggerWithLabels(h.logger, logLabels) + logEntry = logEntry.With( + slog.String(pkg.LogKeyType, string(eventBindingType)), + slog.String(pkg.LogKeyConfigurationId, event.ConfigurationId), + slog.String(pkg.LogKeyWebhookID, event.WebhookId)) + logEntry.Debug("Handle event") + + var admissionTask task.Task + h.hookManager.HandleAdmissionEvent(ctx, event, func(h *hook.Hook, info controller.BindingExecutionInfo) { + admissionTask = globalHookTaskFactory.NewHookRunTask(h.Name, eventBindingType, info, logLabels) + }) + + if admissionTask == nil { + logEntry.Error("Possible bug!!! No hook found for event") + return nil, fmt.Errorf("no hook found for '%s' '%s'", event.ConfigurationId, event.WebhookId) + } + + res := h.taskRunner(ctx, admissionTask) + + if res.Status == "Fail" { + return &admission.Response{ + Allowed: false, + Message: "Hook failed", + }, nil + } + + admissionProp := admissionTask.GetProp("admissionResponse") + admissionResponse, ok := admissionProp.(*admission.Response) + if !ok { + logEntry.Error("'admissionResponse' task prop is not of type *AdmissionResponse", + slog.String(pkg.LogKeyType, fmt.Sprintf("%T", admissionProp))) + return nil, fmt.Errorf("hook task prop error") + } + return admissionResponse, nil +} + +// ConversionEventHandler handles conversion webhook events by finding the conversion +// path, running the appropriate hook tasks, and returning the converted objects. +type ConversionEventHandler struct { + hookManager hook.HookManager + taskRunner TaskRunner + logger *log.Logger +} + +// NewConversionEventHandler creates a new ConversionEventHandler. +func NewConversionEventHandler(hm hook.HookManager, runner TaskRunner, logger *log.Logger) *ConversionEventHandler { + return &ConversionEventHandler{ + hookManager: hm, + taskRunner: runner, + logger: logger, + } +} + +// Handle implements the conversion event handler func signature expected by +// conversion.WebhookManager.EventHandlerFn. +func (h *ConversionEventHandler) Handle(ctx context.Context, crdName string, request *v1.ConversionRequest) (*conversion.Response, error) { + logLabels := map[string]string{ + pkg.LogKeyEventID: uuid.Must(uuid.NewV4()).String(), + pkg.LogKeyBinding: string(types.KubernetesConversion), + } + logEntry := utils.EnrichLoggerWithLabels(h.logger, logLabels) + + sourceVersions := conversion.ExtractAPIVersions(request.Objects) + logEntry.Info("Handle kubernetesCustomResourceConversion event for crd", + slog.String(pkg.LogKeyName, crdName), + slog.Int(pkg.LogKeyLen, len(request.Objects)), + slog.Any(pkg.LogKeyVersions, sourceVersions)) + + done := false + for _, srcVer := range sourceVersions { + rule := conversion.Rule{ + FromVersion: srcVer, + ToVersion: request.DesiredAPIVersion, + } + convPath := h.hookManager.FindConversionChain(crdName, rule) + if len(convPath) == 0 { + continue + } + logEntry.Info("Find conversion path for rule", + slog.String(pkg.LogKeyName, rule.String()), + slog.Any(pkg.LogKeyValue, convPath)) + + for _, convRule := range convPath { + var convTask task.Task + h.hookManager.HandleConversionEvent(ctx, crdName, request, convRule, func(hk *hook.Hook, info controller.BindingExecutionInfo) { + convTask = globalHookTaskFactory.NewHookRunTask(hk.Name, types.KubernetesConversion, info, logLabels) + }) + + if convTask == nil { + return nil, fmt.Errorf("no hook found for '%s' event for crd/%s", string(types.KubernetesConversion), crdName) + } + + res := h.taskRunner(ctx, convTask) + + if res.Status == "Fail" { + return &conversion.Response{ + FailedMessage: fmt.Sprintf("Hook failed to convert to %s", request.DesiredAPIVersion), + ConvertedObjects: nil, + }, nil + } + + prop := convTask.GetProp("conversionResponse") + response, ok := prop.(*conversion.Response) + if !ok { + logEntry.Error("'conversionResponse' task prop is not of type *conversion.Response", + slog.String(pkg.LogKeyType, fmt.Sprintf("%T", prop))) + return nil, fmt.Errorf("hook task prop error") + } + + // Set response objects as new objects for a next round. + request.Objects = response.ConvertedObjects + + // Stop iterating if hook has converted all objects to a desiredAPIVersion. + newSourceVersions := conversion.ExtractAPIVersions(request.Objects) + + if len(newSourceVersions) == 1 && newSourceVersions[0] == request.DesiredAPIVersion { + done = true + break + } + } + + if done { + break + } + } + + if done { + return &conversion.Response{ + ConvertedObjects: request.Objects, + }, nil + } + + return &conversion.Response{ + FailedMessage: fmt.Sprintf("Conversion %s to %s was not successful", crdName, request.DesiredAPIVersion), + }, nil +} diff --git a/pkg/shell-operator/webhook_handlers_test.go b/pkg/shell-operator/webhook_handlers_test.go new file mode 100644 index 00000000..760aa728 --- /dev/null +++ b/pkg/shell-operator/webhook_handlers_test.go @@ -0,0 +1,301 @@ +package shell_operator + +import ( + "context" + "fmt" + "testing" + + "github.com/deckhouse/deckhouse/pkg/log" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + admissionv1 "k8s.io/api/admission/v1" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/runtime" + + "github.com/flant/shell-operator/pkg/hook" + "github.com/flant/shell-operator/pkg/hook/controller" + htypes "github.com/flant/shell-operator/pkg/hook/types" + kemtypes "github.com/flant/shell-operator/pkg/kube_events_manager/types" + "github.com/flant/shell-operator/pkg/task" + "github.com/flant/shell-operator/pkg/task/queue" + "github.com/flant/shell-operator/pkg/webhook/admission" + "github.com/flant/shell-operator/pkg/webhook/conversion" +) + +// stubHookManager implements hook.HookManager for testing. +// Only the methods used by AdmissionEventHandler and ConversionEventHandler are meaningful. +type stubHookManager struct { + detectAdmissionEventTypeFunc func(event admission.Event) htypes.BindingType + handleAdmissionEventFunc func(ctx context.Context, event admission.Event, createTaskFn func(*hook.Hook, controller.BindingExecutionInfo)) + findConversionChainFunc func(crdName string, rule conversion.Rule) []conversion.Rule + handleConversionEventFunc func(ctx context.Context, crdName string, request *v1.ConversionRequest, rule conversion.Rule, createTaskFn func(*hook.Hook, controller.BindingExecutionInfo)) +} + +func (s *stubHookManager) Init() error { return nil } +func (s *stubHookManager) GetHook(name string) *hook.Hook { return nil } +func (s *stubHookManager) GetHookNames() []string { return nil } +func (s *stubHookManager) GetHooksInOrder(htypes.BindingType) ([]string, error) { + return nil, nil +} +func (s *stubHookManager) CreateTasksFromKubeEvent(_ kemtypes.KubeEvent, _ func(*hook.Hook, controller.BindingExecutionInfo) task.Task) []task.Task { + return nil +} +func (s *stubHookManager) HandleCreateTasksFromScheduleEvent(_ string, _ func(*hook.Hook, controller.BindingExecutionInfo) task.Task) []task.Task { + return nil +} +func (s *stubHookManager) HandleAdmissionEvent(ctx context.Context, event admission.Event, createTaskFn func(*hook.Hook, controller.BindingExecutionInfo)) { + if s.handleAdmissionEventFunc != nil { + s.handleAdmissionEventFunc(ctx, event, createTaskFn) + } +} +func (s *stubHookManager) DetectAdmissionEventType(event admission.Event) htypes.BindingType { + if s.detectAdmissionEventTypeFunc != nil { + return s.detectAdmissionEventTypeFunc(event) + } + return htypes.KubernetesValidating +} +func (s *stubHookManager) HandleConversionEvent(ctx context.Context, crdName string, request *v1.ConversionRequest, rule conversion.Rule, createTaskFn func(*hook.Hook, controller.BindingExecutionInfo)) { + if s.handleConversionEventFunc != nil { + s.handleConversionEventFunc(ctx, crdName, request, rule, createTaskFn) + } +} +func (s *stubHookManager) FindConversionChain(crdName string, rule conversion.Rule) []conversion.Rule { + if s.findConversionChainFunc != nil { + return s.findConversionChainFunc(crdName, rule) + } + return nil +} + +// makeMinimalHook returns a hook with just the Name set, sufficient for factory calls. +func makeMinimalHook(name string) *hook.Hook { + return hook.NewHook(name, "/stub/path/"+name, false, false, "", log.NewNop()) +} + +// stubTaskRunner is a TaskRunner that sets an admissionResponse or conversionResponse prop +// and then returns Success. +type stubTaskRunnerWithProp struct { + propKey string + propValue interface{} + status queue.TaskStatus +} + +func (s *stubTaskRunnerWithProp) run(_ context.Context, t task.Task) queue.TaskResult { + if s.propKey != "" { + t.SetProp(s.propKey, s.propValue) + } + return queue.TaskResult{Status: s.status} +} + +// ---- AdmissionEventHandler tests ---- + +func TestAdmissionEventHandler_Handle_noHookFound_returnsError(t *testing.T) { + hm := &stubHookManager{ + handleAdmissionEventFunc: func(_ context.Context, _ admission.Event, _ func(*hook.Hook, controller.BindingExecutionInfo)) { + // deliberate no-op: does not call createTaskFn → admissionTask stays nil + }, + } + runner := func(_ context.Context, _ task.Task) queue.TaskResult { + return queue.TaskResult{Status: queue.Success} + } + h := NewAdmissionEventHandler(hm, runner, log.NewNop()) + event := admission.Event{ + WebhookId: "wh1", + ConfigurationId: "cfg1", + Request: &admissionv1.AdmissionRequest{}, + } + + resp, err := h.Handle(context.Background(), event) + assert.Nil(t, resp) + require.Error(t, err) +} + +func TestAdmissionEventHandler_Handle_taskRunnerFail_returnsDenied(t *testing.T) { + hm := &stubHookManager{ + handleAdmissionEventFunc: func(_ context.Context, _ admission.Event, fn func(*hook.Hook, controller.BindingExecutionInfo)) { + fn(makeMinimalHook("test-hook"), controller.BindingExecutionInfo{}) + }, + } + runner := func(_ context.Context, _ task.Task) queue.TaskResult { + return queue.TaskResult{Status: queue.Fail} + } + h := NewAdmissionEventHandler(hm, runner, log.NewNop()) + event := admission.Event{ + WebhookId: "wh1", + ConfigurationId: "cfg1", + Request: &admissionv1.AdmissionRequest{}, + } + + resp, err := h.Handle(context.Background(), event) + require.NoError(t, err) + require.NotNil(t, resp) + assert.False(t, resp.Allowed) + assert.Equal(t, "Hook failed", resp.Message) +} + +func TestAdmissionEventHandler_Handle_taskRunnerSuccess_returnsProp(t *testing.T) { + want := &admission.Response{Allowed: true, Message: "ok"} + hm := &stubHookManager{ + handleAdmissionEventFunc: func(_ context.Context, _ admission.Event, fn func(*hook.Hook, controller.BindingExecutionInfo)) { + fn(makeMinimalHook("test-hook"), controller.BindingExecutionInfo{}) + }, + } + runner := (&stubTaskRunnerWithProp{propKey: "admissionResponse", propValue: want, status: queue.Success}).run + h := NewAdmissionEventHandler(hm, runner, log.NewNop()) + event := admission.Event{ + WebhookId: "wh1", + ConfigurationId: "cfg1", + Request: &admissionv1.AdmissionRequest{}, + } + + resp, err := h.Handle(context.Background(), event) + require.NoError(t, err) + assert.Equal(t, want, resp) +} + +func TestAdmissionEventHandler_Handle_badPropType_returnsError(t *testing.T) { + hm := &stubHookManager{ + handleAdmissionEventFunc: func(_ context.Context, _ admission.Event, fn func(*hook.Hook, controller.BindingExecutionInfo)) { + fn(makeMinimalHook("test-hook"), controller.BindingExecutionInfo{}) + }, + } + runner := (&stubTaskRunnerWithProp{propKey: "admissionResponse", propValue: "wrong-type", status: queue.Success}).run + h := NewAdmissionEventHandler(hm, runner, log.NewNop()) + event := admission.Event{ + WebhookId: "wh1", + ConfigurationId: "cfg1", + Request: &admissionv1.AdmissionRequest{}, + } + + resp, err := h.Handle(context.Background(), event) + assert.Nil(t, resp) + require.Error(t, err) +} + +// ---- ConversionEventHandler tests ---- + +func TestConversionEventHandler_Handle_noConversionPath_returnsNilError(t *testing.T) { + hm := &stubHookManager{ + findConversionChainFunc: func(_ string, _ conversion.Rule) []conversion.Rule { + return nil // empty → no path found + }, + } + runner := func(_ context.Context, _ task.Task) queue.TaskResult { + return queue.TaskResult{Status: queue.Success} + } + h := NewConversionEventHandler(hm, runner, log.NewNop()) + + req := &v1.ConversionRequest{ + DesiredAPIVersion: "v2", + Objects: []runtime.RawExtension{{Raw: []byte(`{"apiVersion":"v1","kind":"Foo"}`)}}, + } + resp, err := h.Handle(context.Background(), "foo.example.com", req) + // No chain found → done == false → returns success response with original objects + require.NoError(t, err) + require.NotNil(t, resp) +} + +func TestConversionEventHandler_Handle_taskRunnerFail_returnsFailedResponse(t *testing.T) { + convRule := conversion.Rule{FromVersion: "v1", ToVersion: "v2"} + hm := &stubHookManager{ + findConversionChainFunc: func(_ string, _ conversion.Rule) []conversion.Rule { + return []conversion.Rule{convRule} + }, + handleConversionEventFunc: func(_ context.Context, _ string, _ *v1.ConversionRequest, _ conversion.Rule, fn func(*hook.Hook, controller.BindingExecutionInfo)) { + fn(makeMinimalHook("conv-hook"), controller.BindingExecutionInfo{}) + }, + } + runner := func(_ context.Context, _ task.Task) queue.TaskResult { + return queue.TaskResult{Status: queue.Fail} + } + h := NewConversionEventHandler(hm, runner, log.NewNop()) + + req := &v1.ConversionRequest{ + DesiredAPIVersion: "v2", + Objects: []runtime.RawExtension{{Raw: []byte(`{"apiVersion":"v1","kind":"Foo"}`)}}, + } + resp, err := h.Handle(context.Background(), "foo.example.com", req) + require.NoError(t, err) + require.NotNil(t, resp) + assert.NotEmpty(t, resp.FailedMessage) + assert.Nil(t, resp.ConvertedObjects) +} + +func TestConversionEventHandler_Handle_noHookForPath_returnsError(t *testing.T) { + convRule := conversion.Rule{FromVersion: "v1", ToVersion: "v2"} + hm := &stubHookManager{ + findConversionChainFunc: func(_ string, _ conversion.Rule) []conversion.Rule { + return []conversion.Rule{convRule} + }, + handleConversionEventFunc: func(_ context.Context, _ string, _ *v1.ConversionRequest, _ conversion.Rule, _ func(*hook.Hook, controller.BindingExecutionInfo)) { + // no-op: does not call createTaskFn → convTask stays nil + }, + } + runner := func(_ context.Context, _ task.Task) queue.TaskResult { + return queue.TaskResult{Status: queue.Success} + } + h := NewConversionEventHandler(hm, runner, log.NewNop()) + + req := &v1.ConversionRequest{ + DesiredAPIVersion: "v2", + Objects: []runtime.RawExtension{{Raw: []byte(`{"apiVersion":"v1","kind":"Foo"}`)}}, + } + _, err := h.Handle(context.Background(), "foo.example.com", req) + require.Error(t, err) +} + +func TestConversionEventHandler_Handle_badPropType_returnsError(t *testing.T) { + convRule := conversion.Rule{FromVersion: "v1", ToVersion: "v2"} + hm := &stubHookManager{ + findConversionChainFunc: func(_ string, _ conversion.Rule) []conversion.Rule { + return []conversion.Rule{convRule} + }, + handleConversionEventFunc: func(_ context.Context, _ string, _ *v1.ConversionRequest, _ conversion.Rule, fn func(*hook.Hook, controller.BindingExecutionInfo)) { + fn(makeMinimalHook("conv-hook"), controller.BindingExecutionInfo{}) + }, + } + runner := (&stubTaskRunnerWithProp{propKey: "conversionResponse", propValue: "wrong-type", status: queue.Success}).run + h := NewConversionEventHandler(hm, runner, log.NewNop()) + + req := &v1.ConversionRequest{ + DesiredAPIVersion: "v2", + Objects: []runtime.RawExtension{{Raw: []byte(`{"apiVersion":"v1","kind":"Foo"}`)}}, + } + _, err := h.Handle(context.Background(), "foo.example.com", req) + require.Error(t, err) + assert.Contains(t, err.Error(), "hook task prop error") +} + +func TestConversionEventHandler_Handle_success_returnsProp(t *testing.T) { + convRule := conversion.Rule{FromVersion: "v1", ToVersion: "v2"} + want := &conversion.Response{ + ConvertedObjects: []runtime.RawExtension{{Raw: []byte(`{"apiVersion":"v2","kind":"Foo"}`)}}, + } + hm := &stubHookManager{ + findConversionChainFunc: func(_ string, _ conversion.Rule) []conversion.Rule { + return []conversion.Rule{convRule} + }, + handleConversionEventFunc: func(_ context.Context, _ string, _ *v1.ConversionRequest, _ conversion.Rule, fn func(*hook.Hook, controller.BindingExecutionInfo)) { + fn(makeMinimalHook("conv-hook"), controller.BindingExecutionInfo{}) + }, + } + runner := func(_ context.Context, t task.Task) queue.TaskResult { + t.SetProp("conversionResponse", want) + return queue.TaskResult{Status: queue.Success} + } + h := NewConversionEventHandler(hm, runner, log.NewNop()) + + req := &v1.ConversionRequest{ + DesiredAPIVersion: "v2", + Objects: []runtime.RawExtension{{Raw: []byte(`{"apiVersion":"v1","kind":"Foo"}`)}}, + } + resp, err := h.Handle(context.Background(), "foo.example.com", req) + require.NoError(t, err) + require.NotNil(t, resp) + assert.Equal(t, want.ConvertedObjects, resp.ConvertedObjects) +} + +// compile-time check that stubHookManager satisfies the interface +var _ hook.HookManager = (*stubHookManager)(nil) + +// Ensure fmt is used to avoid unused import errors. +var _ = fmt.Sprintf