Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 39 additions & 8 deletions internal/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(), hook.IsApplicationHook())
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{
Expand All @@ -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(), false) // Readiness is always a module hook
if err != nil {
return fmt.Errorf("failed to remap readiness hook config: %w", err)
}
cfg.Readiness = readinessConfig
}

if c.settingsCheck != nil {
Expand Down Expand Up @@ -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(), hook.IsApplicationHook())
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{
Expand All @@ -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(), false) // Readiness is always a module hook
if err != nil {
return fmt.Errorf("failed to remap readiness hook config: %w", err)
}
cfg.Readiness = readinessConfig
}

err = json.NewEncoder(f).Encode(cfg)
Expand All @@ -249,7 +265,7 @@ func (c *HookController) WriteHookConfigsInFile() error {
return nil
}

func remapHookConfigToHookConfig(cfg *pkg.HookConfig) *gohook.HookConfig {
func remapHookConfigToHookConfig(cfg *pkg.HookConfig, isApplicationHook bool) (*gohook.HookConfig, error) {
newHookConfig := &gohook.HookConfig{
ConfigVersion: "v1",
Metadata: gohook.GoHookMetadata(cfg.Metadata),
Expand Down Expand Up @@ -290,15 +306,30 @@ 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
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,
},
LabelSelector: shcfg.NamespaceSelector.LabelSelector,
}
}

newShCfg.NamespaceSelector = targetNamespaceSelector

if shcfg.FieldSelector != nil {
fs := &gohook.FieldSelector{
MatchExpressions: make([]gohook.FieldSelectorRequirement, 0, len(shcfg.FieldSelector.MatchExpressions)),
Expand Down Expand Up @@ -330,5 +361,5 @@ func remapHookConfigToHookConfig(cfg *pkg.HookConfig) *gohook.HookConfig {
newHookConfig.OnAfterDeleteHelm = ptr.To(cfg.OnAfterDeleteHelm.Order)
}

return newHookConfig
return newHookConfig, nil
}
163 changes: 163 additions & 0 deletions internal/controller/controller_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
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 {
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
}

// Case 1: Application Hook without the specified namespace.
// Waiting: Namespace is automatically inserted 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"},
Kubernetes: []pkg.KubernetesConfig{
{Name: "pods", APIVersion: "v1", Kind: "Pod"},
},
}

mockExec := &mockExecutor{isAppHook: true, config: config}

result, err := remapHookConfigToHookConfig(mockExec.Config(), mockExec.IsApplicationHook())
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 2: Application Hook with an attempt to specify a "foreign" namespace (for example, kube-system).
// Waiting: The user config is IGNORED, the application namespace is forced.
func Test_remapHookConfigToHookConfig_ApplicationHook_OverwritesMaliciousNamespace(t *testing.T) {
appName := "my-safe-app"
maliciousNamespace := "kube-system"
t.Setenv(pkg.EnvApplicationNamespace, appName)

config := &pkg.HookConfig{
Metadata: pkg.HookMetadata{Name: "app-hook-malicious"},
Kubernetes: []pkg.KubernetesConfig{
{
Name: "secrets",
APIVersion: "v1",
Kind: "Secret",
NamespaceSelector: &pkg.NamespaceSelector{
NameSelector: &pkg.NameSelector{
MatchNames: []string{maliciousNamespace},
},
},
},
},
}

mockExec := &mockExecutor{isAppHook: true, config: config}

result, err := remapHookConfigToHookConfig(mockExec.Config(), mockExec.IsApplicationHook())
require.NoError(t, err)

require.Len(t, result.Kubernetes, 1)
assert.NotNil(t, result.Kubernetes[0].NamespaceSelector)

assert.Equal(t, []string{appName}, result.Kubernetes[0].NamespaceSelector.NameSelector.MatchNames)
assert.NotContains(t, result.Kubernetes[0].NamespaceSelector.NameSelector.MatchNames, maliciousNamespace)
}

// 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"},
Kubernetes: []pkg.KubernetesConfig{
{Name: "pods", APIVersion: "v1", Kind: "Pod"},
},
}

mockExec := &mockExecutor{isAppHook: true, config: config}

result, err := remapHookConfigToHookConfig(mockExec.Config(), mockExec.IsApplicationHook())

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 set1")
}

// Case 4: 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"},
Kubernetes: []pkg.KubernetesConfig{
{Name: "nodes", APIVersion: "v1", Kind: "Node"},
},
}

mockExec := &mockExecutor{isAppHook: false, config: config}

result, err := remapHookConfigToHookConfig(mockExec.Config(), mockExec.IsApplicationHook())
require.NoError(t, err)

require.Len(t, result.Kubernetes, 1)
assert.Nil(t, result.Kubernetes[0].NamespaceSelector)
}

// Case 5: 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"},
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(), mockExec.IsApplicationHook())
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)
}
4 changes: 4 additions & 0 deletions internal/executor/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions internal/executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
type Executor interface {
Config() *pkg.HookConfig
Execute(ctx context.Context, req Request) (Result, error)
IsApplicationHook() bool
}

type Request interface {
Expand Down
4 changes: 4 additions & 0 deletions internal/executor/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading