diff --git a/internal/controller/controller.go b/internal/controller/controller.go index 39ba143..0a5dc00 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -175,7 +175,11 @@ func (c *HookController) PrintHookConfigs() error { configs := make([]gohook.HookConfig, 0, 1) for _, hook := range c.registry.Executors() { - configs = append(configs, *remapHookConfigToHookConfig(hook.Config())) + hookConfig, err := remapHookConfigToHookConfig(hook.Config()) + if err != nil { + return fmt.Errorf("failed to remap hook config for %s: %w", hook.Config().Metadata.Name, err) + } + configs = append(configs, *hookConfig) } cfg := &gohook.BatchHookConfig{ @@ -184,7 +188,11 @@ func (c *HookController) PrintHookConfigs() error { } if c.registry.Readiness() != nil { - cfg.Readiness = remapHookConfigToHookConfig(c.registry.Readiness().Config()) + readinessConfig, err := remapHookConfigToHookConfig(c.registry.Readiness().Config()) + if err != nil { + return fmt.Errorf("failed to remap readiness hook config: %w", err) + } + cfg.Readiness = readinessConfig } if c.settingsCheck != nil { @@ -229,7 +237,11 @@ func (c *HookController) WriteHookConfigsInFile() error { configs := make([]gohook.HookConfig, 0, 1) for _, hook := range c.registry.Executors() { - configs = append(configs, *remapHookConfigToHookConfig(hook.Config())) + hookConfig, err := remapHookConfigToHookConfig(hook.Config()) + if err != nil { + return fmt.Errorf("failed to remap hook config for %s: %w", hook.Config().Metadata.Name, err) + } + configs = append(configs, *hookConfig) } cfg := &gohook.BatchHookConfig{ @@ -238,7 +250,11 @@ func (c *HookController) WriteHookConfigsInFile() error { } if c.registry.Readiness() != nil { - cfg.Readiness = remapHookConfigToHookConfig(c.registry.Readiness().Config()) + readinessConfig, err := remapHookConfigToHookConfig(c.registry.Readiness().Config()) + if err != nil { + return fmt.Errorf("failed to remap readiness hook config: %w", err) + } + cfg.Readiness = readinessConfig } err = json.NewEncoder(f).Encode(cfg) @@ -249,7 +265,8 @@ func (c *HookController) WriteHookConfigsInFile() error { return nil } -func remapHookConfigToHookConfig(cfg *pkg.HookConfig) *gohook.HookConfig { +func remapHookConfigToHookConfig(cfg *pkg.HookConfig) (*gohook.HookConfig, error) { + isApplicationHook := cfg.HookType == pkg.HookTypeApplication newHookConfig := &gohook.HookConfig{ ConfigVersion: "v1", Metadata: gohook.GoHookMetadata(cfg.Metadata), @@ -290,8 +307,21 @@ func remapHookConfigToHookConfig(cfg *pkg.HookConfig) *gohook.HookConfig { } } - if shcfg.NamespaceSelector != nil { - newShCfg.NamespaceSelector = &gohook.NamespaceSelector{ + var targetNamespaceSelector *gohook.NamespaceSelector + // For application hooks, automatically add namespace selector to limit resources to the app's namespace + // For module hooks, use the configured namespace selector if present + if isApplicationHook { + appNs := os.Getenv(pkg.EnvApplicationNamespace) + if appNs == "" { + return nil, fmt.Errorf("application hook %q requires %s env var to be set", cfg.Metadata.Name, pkg.EnvApplicationNamespace) + } + targetNamespaceSelector = &gohook.NamespaceSelector{ + NameSelector: &gohook.NameSelector{ + MatchNames: []string{appNs}, + }, + } + } else if shcfg.NamespaceSelector != nil { + targetNamespaceSelector = &gohook.NamespaceSelector{ NameSelector: &gohook.NameSelector{ MatchNames: shcfg.NamespaceSelector.NameSelector.MatchNames, }, @@ -299,6 +329,8 @@ func remapHookConfigToHookConfig(cfg *pkg.HookConfig) *gohook.HookConfig { } } + newShCfg.NamespaceSelector = targetNamespaceSelector + if shcfg.FieldSelector != nil { fs := &gohook.FieldSelector{ MatchExpressions: make([]gohook.FieldSelectorRequirement, 0, len(shcfg.FieldSelector.MatchExpressions)), @@ -330,5 +362,5 @@ func remapHookConfigToHookConfig(cfg *pkg.HookConfig) *gohook.HookConfig { newHookConfig.OnAfterDeleteHelm = ptr.To(cfg.OnAfterDeleteHelm.Order) } - return newHookConfig + return newHookConfig, nil } diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go new file mode 100644 index 0000000..82eb68d --- /dev/null +++ b/internal/controller/controller_test.go @@ -0,0 +1,174 @@ +package controller + +import ( + "context" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/deckhouse/module-sdk/internal/executor" + "github.com/deckhouse/module-sdk/pkg" +) + +type mockExecutor struct { + isAppHook bool + config *pkg.HookConfig +} + +func (m *mockExecutor) Config() *pkg.HookConfig { + if m.isAppHook { + m.config.HookType = pkg.HookTypeApplication + } else { + m.config.HookType = pkg.HookTypeModule + } + return m.config +} + +func (m *mockExecutor) Execute(_ context.Context, _ executor.Request) (executor.Result, error) { + return nil, nil +} + +func (m *mockExecutor) IsApplicationHook() bool { + return m.isAppHook +} + +// Application Hook without namespace selector. +// Waiting: Namespace is automatically injected from the env variable. +func Test_remapHookConfigToHookConfig_ApplicationHook_InjectsNamespace(t *testing.T) { + appName := "my-test-app" + t.Setenv(pkg.EnvApplicationNamespace, appName) + + config := &pkg.HookConfig{ + Metadata: pkg.HookMetadata{Name: "app-hook-simple"}, + HookType: pkg.HookTypeApplication, + Kubernetes: []pkg.KubernetesConfig{ + {Name: "pods", APIVersion: "v1", Kind: "Pod"}, + }, + } + + mockExec := &mockExecutor{isAppHook: true, config: config} + + result, err := remapHookConfigToHookConfig(mockExec.Config()) + require.NoError(t, err) + + require.Len(t, result.Kubernetes, 1) + assert.NotNil(t, result.Kubernetes[0].NamespaceSelector) + assert.NotNil(t, result.Kubernetes[0].NamespaceSelector.NameSelector) + assert.Equal(t, []string{appName}, result.Kubernetes[0].NamespaceSelector.NameSelector.MatchNames) +} + +// Case 3: Application Hook, but forgot to set the environment variable. +// Waiting: The function returns an error (Fail Fast), the config is not generated. +func Test_remapHookConfigToHookConfig_ApplicationHook_ErrorsOnMissingEnv(t *testing.T) { + os.Unsetenv(pkg.EnvApplicationNamespace) + + config := &pkg.HookConfig{ + Metadata: pkg.HookMetadata{Name: "app-hook-broken"}, + HookType: pkg.HookTypeApplication, + Kubernetes: []pkg.KubernetesConfig{ + {Name: "pods", APIVersion: "v1", Kind: "Pod"}, + }, + } + + mockExec := &mockExecutor{isAppHook: true, config: config} + + result, err := remapHookConfigToHookConfig(mockExec.Config()) + + require.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "application hook \"app-hook-broken\" requires APPLICATION_NAMESPACE env var to be set") +} + +// Module Hook without the namespaceSelector. +// Waiting: The NamespaceSelector remains nil (monitors the entire cluster or works by default). +func Test_remapHookConfigToHookConfig_ModuleHook_PreservesNilSelector(t *testing.T) { + t.Setenv(pkg.EnvApplicationNamespace, "some-app-ns") + + config := &pkg.HookConfig{ + Metadata: pkg.HookMetadata{Name: "module-hook-global"}, + HookType: pkg.HookTypeModule, + Kubernetes: []pkg.KubernetesConfig{ + {Name: "nodes", APIVersion: "v1", Kind: "Node"}, + }, + } + + mockExec := &mockExecutor{isAppHook: false, config: config} + + result, err := remapHookConfigToHookConfig(mockExec.Config()) + require.NoError(t, err) + + require.Len(t, result.Kubernetes, 1) + assert.Nil(t, result.Kubernetes[0].NamespaceSelector) +} + +// Module Hook with an explicitly specified namespace. +// Waiting: The configuration is saved as it is. +func Test_remapHookConfigToHookConfig_ModuleHook_PreservesCustomSelector(t *testing.T) { + targetNs := "kube-system" + + config := &pkg.HookConfig{ + Metadata: pkg.HookMetadata{Name: "module-hook-system"}, + HookType: pkg.HookTypeModule, + Kubernetes: []pkg.KubernetesConfig{ + { + Name: "pods", + APIVersion: "v1", + Kind: "Pod", + NamespaceSelector: &pkg.NamespaceSelector{ + NameSelector: &pkg.NameSelector{ + MatchNames: []string{targetNs}, + }, + }, + }, + }, + } + + mockExec := &mockExecutor{isAppHook: false, config: config} + + result, err := remapHookConfigToHookConfig(mockExec.Config()) + require.NoError(t, err) + + require.Len(t, result.Kubernetes, 1) + assert.NotNil(t, result.Kubernetes[0].NamespaceSelector) + + assert.Equal(t, []string{targetNs}, result.Kubernetes[0].NamespaceSelector.NameSelector.MatchNames) +} + +// Application Hook with explicitly specified namespace selector. +// Waiting: The specified NamespaceSelector is ignored, and the application's namespace is used instead. +func Test_remapHookConfigToHookConfig_ApplicationHook_IgnoresCustomSelector(t *testing.T) { + appName := "my-test-app" + t.Setenv(pkg.EnvApplicationNamespace, appName) + + config := &pkg.HookConfig{ + Metadata: pkg.HookMetadata{Name: "app-hook-with-selector"}, + HookType: pkg.HookTypeApplication, + Kubernetes: []pkg.KubernetesConfig{ + { + Name: "pods", + APIVersion: "v1", + Kind: "Pod", + // Even if NamespaceSelector is specified, it should be ignored + NamespaceSelector: &pkg.NamespaceSelector{ + NameSelector: &pkg.NameSelector{ + MatchNames: []string{"wrong-namespace"}, + }, + }, + }, + }, + } + + mockExec := &mockExecutor{isAppHook: true, config: config} + + result, err := remapHookConfigToHookConfig(mockExec.Config()) + require.NoError(t, err) + + require.Len(t, result.Kubernetes, 1) + assert.NotNil(t, result.Kubernetes[0].NamespaceSelector) + assert.NotNil(t, result.Kubernetes[0].NamespaceSelector.NameSelector) + // Should use application namespace, not the specified one + assert.Equal(t, []string{appName}, result.Kubernetes[0].NamespaceSelector.NameSelector.MatchNames) + assert.NotEqual(t, []string{"wrong-namespace"}, result.Kubernetes[0].NamespaceSelector.NameSelector.MatchNames) +} diff --git a/internal/executor/application.go b/internal/executor/application.go index bc99d98..bf9d832 100644 --- a/internal/executor/application.go +++ b/internal/executor/application.go @@ -32,6 +32,10 @@ func (e *applicationExecutor) Config() *pkg.HookConfig { return e.hook.Config } +func (e *applicationExecutor) IsApplicationHook() bool { + return true +} + func (e *applicationExecutor) Execute(ctx context.Context, req Request) (Result, error) { // Values are patched in-place, so an error can occur. rawValues, err := req.GetValues() diff --git a/internal/executor/executor.go b/internal/executor/executor.go index 7f2331e..5218d27 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -11,6 +11,7 @@ import ( type Executor interface { Config() *pkg.HookConfig Execute(ctx context.Context, req Request) (Result, error) + IsApplicationHook() bool } type Request interface { diff --git a/internal/executor/module.go b/internal/executor/module.go index f54b701..2a2a286 100644 --- a/internal/executor/module.go +++ b/internal/executor/module.go @@ -31,6 +31,10 @@ func (e *moduleExecutor) Config() *pkg.HookConfig { return e.hook.Config } +func (e *moduleExecutor) IsApplicationHook() bool { + return false +} + func (e *moduleExecutor) Execute(ctx context.Context, req Request) (Result, error) { // Values are patched in-place, so an error can occur. rawValues, err := req.GetValues() diff --git a/pkg/hook.go b/pkg/hook.go index 28b0cde..f963f48 100644 --- a/pkg/hook.go +++ b/pkg/hook.go @@ -69,6 +69,14 @@ type HookMetadata struct { Path string } +// HookType defines the type of hook +type HookType string + +const ( + HookTypeModule HookType = "module" + HookTypeApplication HookType = "application" +) + type HookConfig struct { Metadata HookMetadata Schedule []ScheduleConfig @@ -85,10 +93,11 @@ type HookConfig struct { Queue string Settings *HookConfigSettings + + HookType HookType } var ( - kebabCaseRegexp = regexp.MustCompile(`^[a-z0-9]+(?:-[a-z0-9]+)*$`) camelCaseRegexp = regexp.MustCompile(`^[a-zA-Z]*$`) cronScheduleRegex = regexp.MustCompile(`^((((\d+,)+\d+|(\d+(\/|-|#)\d+)|\d+L?|\*(\/\d+)?|L(-\d+)?|\?|[A-Z]{3}(-[A-Z]{3})?) ?){5,7})|(@(annually|yearly|monthly|weekly|daily|hourly|reboot))|(@every (\d+(ns|us|µs|ms|s|m|h))+)$`) ) @@ -103,13 +112,17 @@ func (cfg *HookConfig) Validate() error { } } + isApplicationHook := cfg.HookType == HookTypeApplication for _, k := range cfg.Kubernetes { if err := k.Validate(); err != nil { errs = errors.Join(errs, fmt.Errorf("kubernetes config with name '%s': %w", k.Name, err)) } + if isApplicationHook && k.NamespaceSelector != nil { + errs = errors.Join(errs, fmt.Errorf("kubernetes config with name '%s': NamespaceSelector cannot be specified for application hooks, namespace is automatically set to the application's namespace", k.Name)) + } } - return nil + return errs } type OrderedConfig struct { @@ -130,10 +143,6 @@ type ScheduleConfig struct { func (cfg *ScheduleConfig) Validate() error { var errs error - if !camelCaseRegexp.Match([]byte(cfg.Name)) { - errs = errors.Join(errs, errors.New("name has not letter symbols")) - } - if !cronScheduleRegex.Match([]byte(cfg.Crontab)) { errs = errors.Join(errs, errors.New("crontab is not valid")) } @@ -174,22 +183,10 @@ type KubernetesConfig struct { func (cfg *KubernetesConfig) Validate() error { var errs error - if !kebabCaseRegexp.Match([]byte(cfg.Name)) { - errs = errors.Join(errs, errors.New("name is not kebab case")) - } - if !camelCaseRegexp.Match([]byte(cfg.Kind)) { errs = errors.Join(errs, errors.New("kind has not letter symbols")) } - if err := cfg.NameSelector.Validate(); err != nil { - errs = errors.Join(errs, fmt.Errorf("name selector: %w", err)) - } - - if err := cfg.NamespaceSelector.Validate(); err != nil { - errs = errors.Join(errs, fmt.Errorf("namespace selector: %w", err)) - } - return errs } @@ -197,22 +194,6 @@ type NameSelector struct { MatchNames []string } -func (cfg *NameSelector) Validate() error { - if cfg == nil { - return nil - } - - var errs error - - for _, sel := range cfg.MatchNames { - if !kebabCaseRegexp.Match([]byte(sel)) { - errs = errors.Join(errs, fmt.Errorf("selector is not kebab case '%s'", sel)) - } - } - - return errs -} - type FieldSelectorRequirement struct { Field string Operator string @@ -227,17 +208,3 @@ type NamespaceSelector struct { NameSelector *NameSelector LabelSelector *metav1.LabelSelector } - -func (cfg *NamespaceSelector) Validate() error { - if cfg == nil { - return nil - } - - var errs error - - if err := cfg.NameSelector.Validate(); err != nil { - errs = errors.Join(errs, fmt.Errorf("name selector: %w", err)) - } - - return errs -} diff --git a/pkg/registry/registry.go b/pkg/registry/registry.go index 220bf88..569cc6f 100644 --- a/pkg/registry/registry.go +++ b/pkg/registry/registry.go @@ -1,6 +1,7 @@ package registry import ( + "fmt" "regexp" "runtime" "strings" @@ -56,7 +57,9 @@ func registerHook[T pkg.Input](r *HookRegistry, cfg *pkg.HookConfig, f pkg.HookF panic(bindingsPanicMsg) } - cfg.Metadata = extractHookMetadata() + if cfg.Metadata.Name == "" { + cfg.Metadata = extractHookMetadata() + } r.mtx.Lock() defer r.mtx.Unlock() @@ -65,12 +68,18 @@ func registerHook[T pkg.Input](r *HookRegistry, cfg *pkg.HookConfig, f pkg.HookF switch any(hook).(type) { case pkg.Hook[*pkg.HookInput]: + cfg.HookType = pkg.HookTypeModule r.moduleHooks = append(r.moduleHooks, any(hook).(pkg.Hook[*pkg.HookInput])) case pkg.Hook[*pkg.ApplicationHookInput]: + cfg.HookType = pkg.HookTypeApplication r.applicationHooks = append(r.applicationHooks, any(hook).(pkg.Hook[*pkg.ApplicationHookInput])) default: panic("unknown hook input type") } + + if err := cfg.Validate(); err != nil { + panic(fmt.Sprintf("hook validation failed for %q: %v", cfg.Metadata.Name, err)) + } } func extractHookMetadata() pkg.HookMetadata { diff --git a/pkg/registry/registry_test.go b/pkg/registry/registry_test.go index 1763a5e..eb9965f 100644 --- a/pkg/registry/registry_test.go +++ b/pkg/registry/registry_test.go @@ -70,4 +70,64 @@ func TestRegister(t *testing.T) { return nil }) }) + + t.Run("Application hook with NamespaceSelector should panic", func(t *testing.T) { + hook := &pkg.HookConfig{ + Metadata: pkg.HookMetadata{ + Name: "test-hook", + Path: "test/hooks", + }, + Kubernetes: []pkg.KubernetesConfig{ + { + Name: "test", + APIVersion: "v1", + Kind: "Pod", + NamespaceSelector: &pkg.NamespaceSelector{ + NameSelector: &pkg.NameSelector{ + MatchNames: []string{"some-namespace"}, + }, + }, + }, + }, + } + + defer func() { + r := recover() + require.NotEmpty(t, r) + assert.Contains(t, r.(string), "NamespaceSelector cannot be specified for application hooks") + }() + + RegisterFunc(hook, func(_ context.Context, _ *pkg.ApplicationHookInput) error { + return nil + }) + }) + + t.Run("Application hook without NamespaceSelector should not panic", func(t *testing.T) { + hook := &pkg.HookConfig{ + Metadata: pkg.HookMetadata{ + Name: "test-hook", + Path: "test/hooks", + }, + Kubernetes: []pkg.KubernetesConfig{ + { + Name: "test", + APIVersion: "v1", + Kind: "Pod", + }, + }, + } + + defer func() { + r := recover() + assert.NotEqual(t, bindingsPanicMsg, r) + // Should not panic with validation error + if r != nil { + assert.NotContains(t, r.(string), "NamespaceSelector cannot be specified") + } + }() + + RegisterFunc(hook, func(_ context.Context, _ *pkg.ApplicationHookInput) error { + return nil + }) + }) }