From ef357dc207c113dde7ebc96552b949c8daaa8cc5 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Mon, 20 Apr 2026 15:17:26 +0300 Subject: [PATCH 1/2] fix(module): avoid queue blocking on invalid module config Signed-off-by: Dmitry Lopatin --- .../hooks/cmd/module-config-reporter/main.go | 2 +- .../hook.go | 4 + .../hook_test.go | 1 + .../hooks/discovery-workload-nodes/hook.go | 4 + .../discovery-workload-nodes/hook_test.go | 84 +++++++++++++++ .../hooks/generate-secret-for-dvcr/hook.go | 4 + .../generate-secret-for-dvcr/hook_test.go | 1 + .../hooks/tls-certificates-api-proxy/main.go | 18 +++- .../tls-certificates-api-proxy/main_test.go | 37 +++++++ .../pkg/hooks/tls-certificates-api/main.go | 19 +++- .../hooks/tls-certificates-api/main_test.go | 37 +++++++ .../pkg/hooks/tls-certificates-audit/hook.go | 4 + .../hooks/tls-certificates-controller/hook.go | 19 +++- .../tls-certificates-controller/hook_test.go | 37 +++++++ .../pkg/hooks/tls-certificates-dvcr/hook.go | 4 + .../hooks/tls-certificates-dvcr/hook_test.go | 38 +++++++ .../pkg/hooks/validate-module-config/hook.go | 35 +++++- .../hooks/validate-module-config/hook_test.go | 102 +++++++++++++----- images/hooks/pkg/readiness/probe.go | 2 +- images/hooks/pkg/settings/module_config.go | 32 ++++++ .../hooks/pkg/settings/module_config_test.go | 68 ++++++++++++ templates/_helpers.tpl | 2 +- 22 files changed, 516 insertions(+), 38 deletions(-) create mode 100644 images/hooks/pkg/hooks/discovery-workload-nodes/hook_test.go create mode 100644 images/hooks/pkg/hooks/tls-certificates-api-proxy/main_test.go create mode 100644 images/hooks/pkg/hooks/tls-certificates-api/main_test.go create mode 100644 images/hooks/pkg/hooks/tls-certificates-controller/hook_test.go create mode 100644 images/hooks/pkg/hooks/tls-certificates-dvcr/hook_test.go create mode 100644 images/hooks/pkg/settings/module_config.go create mode 100644 images/hooks/pkg/settings/module_config_test.go diff --git a/images/hooks/cmd/module-config-reporter/main.go b/images/hooks/cmd/module-config-reporter/main.go index 7a1998012c..a6a8658481 100644 --- a/images/hooks/cmd/module-config-reporter/main.go +++ b/images/hooks/cmd/module-config-reporter/main.go @@ -102,7 +102,7 @@ func handle(values gjson.Result) error { if validationObj.IsObject() { validationErr := validationObj.Get("error") if validationErr.Exists() { - return fmt.Errorf(validationErr.String()) + return fmt.Errorf("%s", validationErr.String()) } } return nil diff --git a/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook.go b/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook.go index 5ca150f46b..9b386b39c7 100644 --- a/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook.go +++ b/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook.go @@ -66,6 +66,10 @@ var configDiscoveryService = &pkg.HookConfig{ } func handleDiscoveryService(_ context.Context, input *pkg.HookInput) error { + if !settings.HasModuleConfig(input) { + return nil + } + clusterIP := getClusterIP(input) if clusterIP == "" { diff --git a/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook_test.go b/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook_test.go index 35681c1404..4d289aeffc 100644 --- a/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook_test.go +++ b/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook_test.go @@ -64,6 +64,7 @@ var _ = Describe("DiscoveryClusterIPServiceForDVCR", func() { } newInput := func() *pkg.HookInput { + values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Parse(`{"dvcr":{},"virtualMachineCIDRs":["10.0.0.0/24"]}`)) return &pkg.HookInput{ Snapshots: snapshots, Values: values, diff --git a/images/hooks/pkg/hooks/discovery-workload-nodes/hook.go b/images/hooks/pkg/hooks/discovery-workload-nodes/hook.go index 61cf15004f..7254ce1d67 100644 --- a/images/hooks/pkg/hooks/discovery-workload-nodes/hook.go +++ b/images/hooks/pkg/hooks/discovery-workload-nodes/hook.go @@ -81,6 +81,10 @@ var configDiscoveryService = &pkg.HookConfig{ } func handleDiscoveryNodes(_ context.Context, input *pkg.HookInput) error { + if !settings.HasModuleConfig(input) { + return nil + } + nodeCount := len(input.Snapshots.Get(discoveryNodesSnapshot)) input.Values.Set(virtHandlerNodeCountPath, nodeCount) diff --git a/images/hooks/pkg/hooks/discovery-workload-nodes/hook_test.go b/images/hooks/pkg/hooks/discovery-workload-nodes/hook_test.go new file mode 100644 index 0000000000..de0cf28ce5 --- /dev/null +++ b/images/hooks/pkg/hooks/discovery-workload-nodes/hook_test.go @@ -0,0 +1,84 @@ +/* +Copyright 2026 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 discovery_workload_nodes + +import ( + "context" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/tidwall/gjson" + + "github.com/deckhouse/deckhouse/pkg/log" + "github.com/deckhouse/module-sdk/pkg" + "github.com/deckhouse/module-sdk/testing/mock" +) + +func TestDiscoveryWorkloadNodes(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "DiscoveryWorkloadNodes Suite") +} + +var _ = Describe("DiscoveryWorkloadNodes", func() { + var ( + snapshots *mock.SnapshotsMock + values *mock.OutputPatchableValuesCollectorMock + ) + + newInput := func() *pkg.HookInput { + return &pkg.HookInput{ + Snapshots: snapshots, + Values: values, + Logger: log.NewNop(), + } + } + + BeforeEach(func() { + snapshots = mock.NewSnapshotsMock(GinkgoT()) + values = mock.NewPatchableValuesCollectorMock(GinkgoT()) + }) + + AfterEach(func() { + snapshots = nil + values = nil + }) + + It("should do nothing when copied module config is absent", func() { + values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Result{}) + Expect(handleDiscoveryNodes(context.Background(), newInput())).To(Succeed()) + }) + + It("should set node count when copied module config exists", func() { + values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Parse(`{"dvcr":{},"virtualMachineCIDRs":["10.0.0.0/24"]}`)) + snapshots.GetMock.When(discoveryNodesSnapshot).Then([]pkg.Snapshot{ + mock.NewSnapshotMock(GinkgoT()), + mock.NewSnapshotMock(GinkgoT()), + }) + snapshots.GetMock.When(kubevirtConfigSnapshot).Then([]pkg.Snapshot{}) + + setCalls := 0 + values.SetMock.Set(func(path string, v any) { + setCalls++ + Expect(path).To(Equal(virtHandlerNodeCountPath)) + Expect(v).To(Equal(2)) + }) + + Expect(handleDiscoveryNodes(context.Background(), newInput())).To(Succeed()) + Expect(setCalls).To(Equal(1)) + }) +}) diff --git a/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook.go b/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook.go index 8eac4abd3b..bc7033f2db 100644 --- a/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook.go +++ b/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook.go @@ -75,6 +75,10 @@ var configDVCRSecrets = &pkg.HookConfig{ } func handlerDVCRSecrets(_ context.Context, input *pkg.HookInput) error { + if !settings.HasModuleConfig(input) { + return nil + } + dataFromSecret, err := getDVCRSecretsFromSecrets(input) if err != nil { return err diff --git a/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook_test.go b/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook_test.go index 11cc500d15..b08180e2dd 100644 --- a/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook_test.go +++ b/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook_test.go @@ -70,6 +70,7 @@ var _ = Describe("DVCR Secrets", func() { } newInput := func() *pkg.HookInput { + values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Parse(`{"dvcr":{},"virtualMachineCIDRs":["10.0.0.0/24"]}`)) return &pkg.HookInput{ Snapshots: snapshots, Values: values, diff --git a/images/hooks/pkg/hooks/tls-certificates-api-proxy/main.go b/images/hooks/pkg/hooks/tls-certificates-api-proxy/main.go index 82b9891719..ab591b93e3 100644 --- a/images/hooks/pkg/hooks/tls-certificates-api-proxy/main.go +++ b/images/hooks/pkg/hooks/tls-certificates-api-proxy/main.go @@ -14,6 +14,7 @@ limitations under the License. package tls_certificates_api_proxy import ( + "context" "fmt" "hooks/pkg/settings" @@ -22,9 +23,10 @@ import ( tlscertificate "github.com/deckhouse/module-sdk/common-hooks/tls-certificate" "github.com/deckhouse/module-sdk/pkg" + "github.com/deckhouse/module-sdk/pkg/registry" ) -var _ = tlscertificate.RegisterInternalTLSHookEM(tlscertificate.GenSelfSignedTLSHookConf{ +var conf = tlscertificate.GenSelfSignedTLSHookConf{ CN: settings.APIProxyCertCN, TLSSecretName: "virtualization-api-proxy-tls", Namespace: settings.ModuleNamespace, @@ -32,4 +34,16 @@ var _ = tlscertificate.RegisterInternalTLSHookEM(tlscertificate.GenSelfSignedTLS FullValuesPathPrefix: fmt.Sprintf("%s.internal.apiserver.proxyCert", settings.ModuleName), CommonCAValuesPath: fmt.Sprintf("%s.internal.rootCA", settings.ModuleName), Usages: []v1.KeyUsage{v1.UsageClientAuth}, -}) +} + +var reconcile = func(conf tlscertificate.GenSelfSignedTLSHookConf) pkg.ReconcileFunc { + return func(ctx context.Context, input *pkg.HookInput) error { + if !settings.HasModuleConfig(input) { + return nil + } + + return tlscertificate.GenSelfSignedTLS(conf)(ctx, input) + } +} + +var _ = registry.RegisterFunc(tlscertificate.GenSelfSignedTLSConfig(conf), reconcile(conf)) diff --git a/images/hooks/pkg/hooks/tls-certificates-api-proxy/main_test.go b/images/hooks/pkg/hooks/tls-certificates-api-proxy/main_test.go new file mode 100644 index 0000000000..5f0d4955f6 --- /dev/null +++ b/images/hooks/pkg/hooks/tls-certificates-api-proxy/main_test.go @@ -0,0 +1,37 @@ +/* +Copyright 2026 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 tls_certificates_api_proxy + +import ( + "context" + "testing" + + "github.com/tidwall/gjson" + + "github.com/deckhouse/module-sdk/pkg" + "github.com/deckhouse/module-sdk/testing/mock" +) + +func TestReconcileSkipsWithoutModuleConfig(t *testing.T) { + values := mock.NewPatchableValuesCollectorMock(t) + values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Result{}) + + input := &pkg.HookInput{Values: values} + if err := reconcile(conf)(context.Background(), input); err != nil { + t.Fatalf("expected nil error, got %v", err) + } +} diff --git a/images/hooks/pkg/hooks/tls-certificates-api/main.go b/images/hooks/pkg/hooks/tls-certificates-api/main.go index 9f25d04176..222c9e0e82 100644 --- a/images/hooks/pkg/hooks/tls-certificates-api/main.go +++ b/images/hooks/pkg/hooks/tls-certificates-api/main.go @@ -16,14 +16,17 @@ limitations under the License. package tls_certificates_api import ( + "context" "fmt" "hooks/pkg/settings" tlscertificate "github.com/deckhouse/module-sdk/common-hooks/tls-certificate" + "github.com/deckhouse/module-sdk/pkg" + "github.com/deckhouse/module-sdk/pkg/registry" ) -var _ = tlscertificate.RegisterInternalTLSHookEM(tlscertificate.GenSelfSignedTLSHookConf{ +var conf = tlscertificate.GenSelfSignedTLSHookConf{ CN: settings.APICertCN, TLSSecretName: "virtualization-api-tls", Namespace: settings.ModuleNamespace, @@ -40,4 +43,16 @@ var _ = tlscertificate.RegisterInternalTLSHookEM(tlscertificate.GenSelfSignedTLS FullValuesPathPrefix: fmt.Sprintf("%s.internal.apiserver.cert", settings.ModuleName), CommonCAValuesPath: fmt.Sprintf("%s.internal.rootCA", settings.ModuleName), -}) +} + +var reconcile = func(conf tlscertificate.GenSelfSignedTLSHookConf) pkg.ReconcileFunc { + return func(ctx context.Context, input *pkg.HookInput) error { + if !settings.HasModuleConfig(input) { + return nil + } + + return tlscertificate.GenSelfSignedTLS(conf)(ctx, input) + } +} + +var _ = registry.RegisterFunc(tlscertificate.GenSelfSignedTLSConfig(conf), reconcile(conf)) diff --git a/images/hooks/pkg/hooks/tls-certificates-api/main_test.go b/images/hooks/pkg/hooks/tls-certificates-api/main_test.go new file mode 100644 index 0000000000..a67ee6b909 --- /dev/null +++ b/images/hooks/pkg/hooks/tls-certificates-api/main_test.go @@ -0,0 +1,37 @@ +/* +Copyright 2026 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 tls_certificates_api + +import ( + "context" + "testing" + + "github.com/tidwall/gjson" + + "github.com/deckhouse/module-sdk/pkg" + "github.com/deckhouse/module-sdk/testing/mock" +) + +func TestReconcileSkipsWithoutModuleConfig(t *testing.T) { + values := mock.NewPatchableValuesCollectorMock(t) + values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Result{}) + + input := &pkg.HookInput{Values: values} + if err := reconcile(conf)(context.Background(), input); err != nil { + t.Fatalf("expected nil error, got %v", err) + } +} diff --git a/images/hooks/pkg/hooks/tls-certificates-audit/hook.go b/images/hooks/pkg/hooks/tls-certificates-audit/hook.go index 39027d8b79..9129562008 100644 --- a/images/hooks/pkg/hooks/tls-certificates-audit/hook.go +++ b/images/hooks/pkg/hooks/tls-certificates-audit/hook.go @@ -41,6 +41,10 @@ var conf = tlscertificate.GenSelfSignedTLSHookConf{ var genSelfSignedTLS = func(conf tlscertificate.GenSelfSignedTLSHookConf) pkg.ReconcileFunc { return func(ctx context.Context, input *pkg.HookInput) error { + if !settings.HasModuleConfig(input) { + return nil + } + if !input.Values.Get("virtualization.audit.enabled").Bool() { return nil } diff --git a/images/hooks/pkg/hooks/tls-certificates-controller/hook.go b/images/hooks/pkg/hooks/tls-certificates-controller/hook.go index 4d5230e654..1f0b8cb2a4 100644 --- a/images/hooks/pkg/hooks/tls-certificates-controller/hook.go +++ b/images/hooks/pkg/hooks/tls-certificates-controller/hook.go @@ -16,14 +16,17 @@ limitations under the License. package tls_certificates_controller import ( + "context" "fmt" "hooks/pkg/settings" tlscertificate "github.com/deckhouse/module-sdk/common-hooks/tls-certificate" + "github.com/deckhouse/module-sdk/pkg" + "github.com/deckhouse/module-sdk/pkg/registry" ) -var _ = tlscertificate.RegisterInternalTLSHookEM(tlscertificate.GenSelfSignedTLSHookConf{ +var conf = tlscertificate.GenSelfSignedTLSHookConf{ CN: settings.ControllerCertCN, TLSSecretName: "virtualization-controller-tls", Namespace: settings.ModuleNamespace, @@ -40,4 +43,16 @@ var _ = tlscertificate.RegisterInternalTLSHookEM(tlscertificate.GenSelfSignedTLS FullValuesPathPrefix: fmt.Sprintf("%s.internal.controller.cert", settings.ModuleName), CommonCAValuesPath: fmt.Sprintf("%s.internal.rootCA", settings.ModuleName), -}) +} + +var reconcile = func(conf tlscertificate.GenSelfSignedTLSHookConf) pkg.ReconcileFunc { + return func(ctx context.Context, input *pkg.HookInput) error { + if !settings.HasModuleConfig(input) { + return nil + } + + return tlscertificate.GenSelfSignedTLS(conf)(ctx, input) + } +} + +var _ = registry.RegisterFunc(tlscertificate.GenSelfSignedTLSConfig(conf), reconcile(conf)) diff --git a/images/hooks/pkg/hooks/tls-certificates-controller/hook_test.go b/images/hooks/pkg/hooks/tls-certificates-controller/hook_test.go new file mode 100644 index 0000000000..f3ef0ba2e3 --- /dev/null +++ b/images/hooks/pkg/hooks/tls-certificates-controller/hook_test.go @@ -0,0 +1,37 @@ +/* +Copyright 2026 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 tls_certificates_controller + +import ( + "context" + "testing" + + "github.com/tidwall/gjson" + + "github.com/deckhouse/module-sdk/pkg" + "github.com/deckhouse/module-sdk/testing/mock" +) + +func TestReconcileSkipsWithoutModuleConfig(t *testing.T) { + values := mock.NewPatchableValuesCollectorMock(t) + values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Result{}) + + input := &pkg.HookInput{Values: values} + if err := reconcile(conf)(context.Background(), input); err != nil { + t.Fatalf("expected nil error, got %v", err) + } +} diff --git a/images/hooks/pkg/hooks/tls-certificates-dvcr/hook.go b/images/hooks/pkg/hooks/tls-certificates-dvcr/hook.go index 9b6ce51e16..ebf69e7c59 100644 --- a/images/hooks/pkg/hooks/tls-certificates-dvcr/hook.go +++ b/images/hooks/pkg/hooks/tls-certificates-dvcr/hook.go @@ -51,6 +51,10 @@ var _ = tlscertificate.RegisterInternalTLSHookEM(tlscertificate.GenSelfSignedTLS CommonCAValuesPath: fmt.Sprintf("%s.internal.rootCA", settings.ModuleName), BeforeHookCheck: func(input *pkg.HookInput) bool { + if !settings.HasModuleConfig(input) { + return false + } + if dvcrGetServiceIP(input).Type == gjson.Null { return false } diff --git a/images/hooks/pkg/hooks/tls-certificates-dvcr/hook_test.go b/images/hooks/pkg/hooks/tls-certificates-dvcr/hook_test.go new file mode 100644 index 0000000000..db36aa94c2 --- /dev/null +++ b/images/hooks/pkg/hooks/tls-certificates-dvcr/hook_test.go @@ -0,0 +1,38 @@ +/* +Copyright 2026 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 tls_certificates_dvcr + +import ( + "testing" + + "github.com/tidwall/gjson" + + "hooks/pkg/settings" + + "github.com/deckhouse/module-sdk/pkg" + "github.com/deckhouse/module-sdk/testing/mock" +) + +func TestBeforeHookCheckSkipsWithoutModuleConfig(t *testing.T) { + values := mock.NewPatchableValuesCollectorMock(t) + values.GetMock.When(settings.InternalValuesConfigCopyPath).Then(gjson.Result{}) + + input := &pkg.HookInput{Values: values} + if settings.HasModuleConfig(input) { + t.Fatalf("expected copied module config to be absent") + } +} diff --git a/images/hooks/pkg/hooks/validate-module-config/hook.go b/images/hooks/pkg/hooks/validate-module-config/hook.go index 50e304d71e..912bb0ea7f 100644 --- a/images/hooks/pkg/hooks/validate-module-config/hook.go +++ b/images/hooks/pkg/hooks/validate-module-config/hook.go @@ -101,17 +101,22 @@ func Reconcile(_ context.Context, input *pkg.HookInput) error { input.Values.Set(settings.InternalValuesConfigValidationPath, map[string]string{ "error": fmt.Sprintf("ModuleConfig/virtualization is invalid: %v", err), }) - } else { - // Module is valid, remove moduleConfigValidation object to indicate valid state for the readiness probe. - input.Values.Remove(settings.InternalValuesConfigValidationPath) - // Copy valid settings from config values to apply them in helm templates. - copyModuleConfigSettingsIntoInternalValues(input) + return nil } + // Module is valid, remove moduleConfigValidation object to indicate valid state for the readiness probe. + input.Values.Remove(settings.InternalValuesConfigValidationPath) + // Copy valid settings from config values to apply them in helm templates. + copyModuleConfigSettingsIntoInternalValues(input) + return nil } func validateModuleConfigSettings(mc *mcapi.ModuleConfig, nodes []corev1.Node, podSubnet, serviceSubnet netip.Prefix) error { + if err := validateRequiredModuleConfigSettings(mc); err != nil { + return err + } + CIDRs, err := moduleconfig.ParseCIDRs(mc.Spec.Settings) if err != nil { return err @@ -147,6 +152,26 @@ func copyModuleConfigSettingsIntoInternalValues(input *pkg.HookInput) { input.Values.Set(settings.InternalValuesConfigCopyPath, cfg.Value()) } +func validateRequiredModuleConfigSettings(mc *mcapi.ModuleConfig) error { + if mc == nil { + return fmt.Errorf("moduleConfig is nil") + } + + if mc.Spec.Settings == nil { + return fmt.Errorf("spec.settings is empty") + } + + if _, ok := mc.Spec.Settings["virtualMachineCIDRs"]; !ok { + return fmt.Errorf("spec.settings.virtualMachineCIDRs is required") + } + + if _, ok := mc.Spec.Settings["dvcr"]; !ok { + return fmt.Errorf("spec.settings.dvcr is required") + } + + return nil +} + func moduleConfigFromSnapshot(input *pkg.HookInput) (*mcapi.ModuleConfig, error) { // Unmarshal ModuleConfig from jqFilter result. snap := input.Snapshots.Get(snapshotModuleConfig) diff --git a/images/hooks/pkg/hooks/validate-module-config/hook_test.go b/images/hooks/pkg/hooks/validate-module-config/hook_test.go index 62620c208c..af524ff0f3 100644 --- a/images/hooks/pkg/hooks/validate-module-config/hook_test.go +++ b/images/hooks/pkg/hooks/validate-module-config/hook_test.go @@ -55,20 +55,32 @@ var _ = Describe("ModuleConfig validation hook", func() { "192.168.0.3", } - validVirtualMachineCIDRs = []interface{}{ + validVirtualMachineCIDRs = []any{ "10.33.0.0/24", "10.35.0.0/24", } - virtualMachineCIDRsOverlapWithNodeAddresses = []interface{}{ + validSettings = map[string]any{ + "virtualMachineCIDRs": validVirtualMachineCIDRs, + "dvcr": map[string]any{ + "storage": map[string]any{ + "type": "PersistentVolumeClaim", + "persistentVolumeClaim": map[string]any{ + "size": "10Gi", + }, + }, + }, + } + + virtualMachineCIDRsOverlapWithNodeAddresses = []any{ "192.168.0.0/24", "10.35.0.0/24", } - virtualMachineCIDRsOverlapWithPodSubnet = []interface{}{ + virtualMachineCIDRsOverlapWithPodSubnet = []any{ "10.108.0.0/14", "10.35.0.0/24", } - virtualMachineCIDRsOverlapWithServiceSubnet = []interface{}{ + virtualMachineCIDRsOverlapWithServiceSubnet = []any{ "10.220.0.0/14", "10.35.0.0/24", } @@ -79,12 +91,8 @@ var _ = Describe("ModuleConfig validation hook", func() { values.GetMock.When(serviceSubnetCIDRPath).Then(gjson.Result{Type: gjson.String, Str: serviceSubnet}) } - prepareConfigValues := func(cidrs []interface{}) { - cfgValues := map[string]interface{}{ - "virtualMachineCIDRs": cidrs, - } - cfgValuesBytes, _ := json.Marshal(cfgValues) - + prepareConfigValues := func(cfg map[string]any) { + cfgValuesBytes, _ := json.Marshal(cfg) configValues.GetMock.When("virtualization").Then(gjson.ParseBytes(cfgValuesBytes)) } @@ -93,7 +101,7 @@ var _ = Describe("ModuleConfig validation hook", func() { snapshots.GetMock.When(snapshotNodes).Then(nodes) } - newModuleConfigSnapshot := func(cidrs []interface{}) []pkg.Snapshot { + newModuleConfigSnapshot := func(settingsMap map[string]any) []pkg.Snapshot { return []pkg.Snapshot{ mock.NewSnapshotMock(GinkgoT()).UnmarshalToMock.Set(func(v any) (err error) { GinkgoHelper() @@ -101,12 +109,7 @@ var _ = Describe("ModuleConfig validation hook", func() { Expect(ok).To(BeTrue()) mc.SetName("virtualization") - - if cidrs != nil { - mc.Spec.Settings = map[string]interface{}{ - "virtualMachineCIDRs": cidrs, - } - } + mc.Spec.Settings = settingsMap return nil }), @@ -161,9 +164,9 @@ var _ = Describe("ModuleConfig validation hook", func() { It("Should copy moduleconfig settings into internal object if config is valid", func() { prepareValues(defaultPodSubnet, defaultServiceSubnet) - prepareConfigValues(validVirtualMachineCIDRs) + prepareConfigValues(validSettings) prepareSnapshots( - newModuleConfigSnapshot(validVirtualMachineCIDRs), + newModuleConfigSnapshot(validSettings), newNodesSnapshot(defaultNodeAddresses), ) @@ -171,6 +174,7 @@ var _ = Describe("ModuleConfig validation hook", func() { switch path { case settings.InternalValuesConfigCopyPath: Expect(v).To(HaveKey("virtualMachineCIDRs")) + Expect(v).To(HaveKey("dvcr")) case settings.InternalValuesConfigValidationPath: Fail("Should not set validation error") default: @@ -185,29 +189,79 @@ var _ = Describe("ModuleConfig validation hook", func() { Expect(Reconcile(context.Background(), newInput())).To(Succeed()) }) - DescribeTable("Should set validation error if virtualMachineCIDRs overlap with other addresses in cluster", func(cidrs []interface{}) { + DescribeTable("Should set validation error if settings are incomplete", func(settingsMap map[string]any, expectedError string) { prepareValues(defaultPodSubnet, defaultServiceSubnet) prepareSnapshots( - newModuleConfigSnapshot(cidrs), + newModuleConfigSnapshot(settingsMap), newNodesSnapshot(defaultNodeAddresses), ) + validationErrorSet := false + values.SetMock.Set(func(path string, v any) { switch path { case settings.InternalValuesConfigCopyPath: Fail("Should not copy ModuleConfig settings") case settings.InternalValuesConfigValidationPath: - Expect(v).To(HaveKey("error")) + validationErrorSet = true + obj, ok := v.(map[string]string) + Expect(ok).To(BeTrue()) + Expect(obj).To(HaveKey("error")) + Expect(obj["error"]).To(ContainSubstring(expectedError)) default: Fail("unexpected path") } }) - values.RemoveMock.Optional() + values.RemoveMock.Set(func(path string) { + if path == settings.InternalValuesConfigCopyPath { + Fail("Should not remove previous valid copied ModuleConfig") + } + }) Expect(Reconcile(context.Background(), newInput())).To(Succeed()) + Expect(validationErrorSet).To(BeTrue(), "should set validation error") + }, + Entry("settings are nil", nil, "spec.settings is empty"), + Entry("settings are empty", map[string]any{}, "spec.settings.virtualMachineCIDRs is required"), + Entry("dvcr is missing", map[string]any{ + "virtualMachineCIDRs": validVirtualMachineCIDRs, + }, "spec.settings.dvcr is required"), + ) - Expect(values.RemoveMock.Calls()).To(HaveLen(0), "should not remove values") + DescribeTable("Should set validation error if virtualMachineCIDRs overlap with other addresses in cluster", func(cidrs []any) { + prepareValues(defaultPodSubnet, defaultServiceSubnet) + invalidSettings := map[string]any{ + "virtualMachineCIDRs": cidrs, + "dvcr": validSettings["dvcr"], + } + prepareSnapshots( + newModuleConfigSnapshot(invalidSettings), + newNodesSnapshot(defaultNodeAddresses), + ) + + validationErrorSet := false + + values.SetMock.Set(func(path string, v any) { + switch path { + case settings.InternalValuesConfigCopyPath: + Fail("Should not copy ModuleConfig settings") + case settings.InternalValuesConfigValidationPath: + validationErrorSet = true + Expect(v).To(HaveKey("error")) + default: + Fail("unexpected path") + } + }) + values.RemoveMock.Optional() + values.RemoveMock.Set(func(path string) { + if path == settings.InternalValuesConfigCopyPath { + Fail("Should not remove previous valid copied ModuleConfig") + } + }) + + Expect(Reconcile(context.Background(), newInput())).To(Succeed()) + Expect(validationErrorSet).To(BeTrue(), "should set validation error") }, Entry("contain node address", virtualMachineCIDRsOverlapWithNodeAddresses), Entry("overlap with podSubnet", virtualMachineCIDRsOverlapWithPodSubnet), diff --git a/images/hooks/pkg/readiness/probe.go b/images/hooks/pkg/readiness/probe.go index dc2f89493c..1ea0735397 100644 --- a/images/hooks/pkg/readiness/probe.go +++ b/images/hooks/pkg/readiness/probe.go @@ -34,7 +34,7 @@ func checkModuleReadiness(ctx context.Context, input *pkg.HookInput) error { if validationObj.IsObject() { validationErr := validationObj.Get("error") if validationErr.Exists() { - return fmt.Errorf(validationErr.String()) + return fmt.Errorf("%s", validationErr.String()) } // moduleConfigValidation is present, but no errors. Something wrong. return fmt.Errorf("module is not ready yet") diff --git a/images/hooks/pkg/settings/module_config.go b/images/hooks/pkg/settings/module_config.go new file mode 100644 index 0000000000..6ce6b0b66b --- /dev/null +++ b/images/hooks/pkg/settings/module_config.go @@ -0,0 +1,32 @@ +/* +Copyright 2026 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 settings + +import "github.com/deckhouse/module-sdk/pkg" + +func HasModuleConfig(input *pkg.HookInput) bool { + config := input.Values.Get(InternalValuesConfigCopyPath) + if !config.Exists() { + return false + } + + if !config.IsObject() { + return false + } + + return len(config.Map()) > 0 +} diff --git a/images/hooks/pkg/settings/module_config_test.go b/images/hooks/pkg/settings/module_config_test.go new file mode 100644 index 0000000000..7ce016ac6b --- /dev/null +++ b/images/hooks/pkg/settings/module_config_test.go @@ -0,0 +1,68 @@ +/* +Copyright 2026 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 settings + +import ( + "testing" + + "github.com/tidwall/gjson" + + "github.com/deckhouse/module-sdk/pkg" + "github.com/deckhouse/module-sdk/testing/mock" +) + +func TestHasModuleConfig(t *testing.T) { + newInput := func(values *mock.OutputPatchableValuesCollectorMock) *pkg.HookInput { + return &pkg.HookInput{Values: values} + } + + t.Run("returns false when moduleConfig is absent", func(t *testing.T) { + values := mock.NewPatchableValuesCollectorMock(t) + values.GetMock.When(InternalValuesConfigCopyPath).Then(gjson.Result{}) + + if HasModuleConfig(newInput(values)) { + t.Fatalf("expected HasModuleConfig to return false") + } + }) + + t.Run("returns false when moduleConfig is not an object", func(t *testing.T) { + values := mock.NewPatchableValuesCollectorMock(t) + values.GetMock.When(InternalValuesConfigCopyPath).Then(gjson.Result{Type: gjson.String, Str: "value"}) + + if HasModuleConfig(newInput(values)) { + t.Fatalf("expected HasModuleConfig to return false") + } + }) + + t.Run("returns false when moduleConfig is an empty object", func(t *testing.T) { + values := mock.NewPatchableValuesCollectorMock(t) + values.GetMock.When(InternalValuesConfigCopyPath).Then(gjson.Parse(`{}`)) + + if HasModuleConfig(newInput(values)) { + t.Fatalf("expected HasModuleConfig to return false") + } + }) + + t.Run("returns true when moduleConfig is a non-empty object", func(t *testing.T) { + values := mock.NewPatchableValuesCollectorMock(t) + values.GetMock.When(InternalValuesConfigCopyPath).Then(gjson.Parse(`{"dvcr":{},"virtualMachineCIDRs":["10.0.0.0/24"]}`)) + + if !HasModuleConfig(newInput(values)) { + t.Fatalf("expected HasModuleConfig to return true") + } + }) +} diff --git a/templates/_helpers.tpl b/templates/_helpers.tpl index b30fbc34e7..087efdf756 100644 --- a/templates/_helpers.tpl +++ b/templates/_helpers.tpl @@ -62,7 +62,7 @@ nodeSelector: {{- end }} {{- define "hasValidModuleConfig" -}} -{{- if (hasKey .Values.virtualization.internal "moduleConfig" ) -}} +{{- if (hasKey .Values.virtualization.internal "moduleConfig") -}} true {{- end }} {{- end }} From e2b60b40b73c8fc0624ae6fe7d8132fcb5c2bd66 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Tue, 21 Apr 2026 13:54:30 +0300 Subject: [PATCH 2/2] after review Signed-off-by: Dmitry Lopatin --- .../hook.go | 8 +- .../hook_test.go | 19 +++- .../hooks/discovery-workload-nodes/hook.go | 8 +- .../discovery-workload-nodes/hook_test.go | 39 +++++-- .../hooks/generate-secret-for-dvcr/hook.go | 8 +- .../generate-secret-for-dvcr/hook_test.go | 19 +++- .../hooks/tls-certificates-api-proxy/main.go | 6 +- .../tls-certificates-api-proxy/main_test.go | 25 ++++- .../pkg/hooks/tls-certificates-api/main.go | 6 +- .../hooks/tls-certificates-api/main_test.go | 25 ++++- .../pkg/hooks/tls-certificates-audit/hook.go | 6 +- .../hooks/tls-certificates-controller/hook.go | 6 +- .../tls-certificates-controller/hook_test.go | 25 ++++- .../pkg/hooks/tls-certificates-dvcr/hook.go | 8 +- .../hooks/tls-certificates-dvcr/hook_test.go | 34 ++++-- images/hooks/pkg/settings/module_config.go | 69 ++++++++++-- .../hooks/pkg/settings/module_config_test.go | 101 ++++++++++++++---- 17 files changed, 339 insertions(+), 73 deletions(-) diff --git a/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook.go b/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook.go index 9b386b39c7..a761a45c65 100644 --- a/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook.go +++ b/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook.go @@ -65,8 +65,12 @@ var configDiscoveryService = &pkg.HookConfig{ Queue: fmt.Sprintf("modules/%s", settings.ModuleName), } -func handleDiscoveryService(_ context.Context, input *pkg.HookInput) error { - if !settings.HasModuleConfig(input) { +func handleDiscoveryService(ctx context.Context, input *pkg.HookInput) error { + hasModuleConfig, err := settings.HasModuleConfig(ctx, input) + if err != nil { + return err + } + if !hasModuleConfig { return nil } diff --git a/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook_test.go b/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook_test.go index 4d289aeffc..2d3836a9db 100644 --- a/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook_test.go +++ b/images/hooks/pkg/hooks/discovery-clusterip-service-for-dvcr/hook_test.go @@ -23,10 +23,14 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/tidwall/gjson" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + "hooks/pkg/settings" "github.com/deckhouse/deckhouse/pkg/log" "github.com/deckhouse/module-sdk/pkg" "github.com/deckhouse/module-sdk/testing/mock" + mcapi "github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig/api" ) func TestDiscoveryClusterIPServiceForDVCR(t *testing.T) { @@ -64,7 +68,11 @@ var _ = Describe("DiscoveryClusterIPServiceForDVCR", func() { } newInput := func() *pkg.HookInput { - values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Parse(`{"dvcr":{},"virtualMachineCIDRs":["10.0.0.0/24"]}`)) + dc.GetK8sClientMock.Return(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error { + mc := obj.(*mcapi.ModuleConfig) + *mc = *settings.NewModuleConfigForTest(map[string]any{"dvcr": map[string]any{}, "virtualMachineCIDRs": []any{"10.0.0.0/24"}}) + return nil + }}, nil) return &pkg.HookInput{ Snapshots: snapshots, Values: values, @@ -91,3 +99,12 @@ var _ = Describe("DiscoveryClusterIPServiceForDVCR", func() { Expect(handleDiscoveryService(context.Background(), newInput())).To(Succeed()) }) }) + +type fakeKubernetesClient struct { + pkg.KubernetesClient + get func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error +} + +func (f *fakeKubernetesClient) Get(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object, _ ...ctrlclient.GetOption) error { + return f.get(ctx, key, obj) +} diff --git a/images/hooks/pkg/hooks/discovery-workload-nodes/hook.go b/images/hooks/pkg/hooks/discovery-workload-nodes/hook.go index 7254ce1d67..f01c7212c6 100644 --- a/images/hooks/pkg/hooks/discovery-workload-nodes/hook.go +++ b/images/hooks/pkg/hooks/discovery-workload-nodes/hook.go @@ -80,8 +80,12 @@ var configDiscoveryService = &pkg.HookConfig{ Queue: fmt.Sprintf("modules/%s", settings.ModuleName), } -func handleDiscoveryNodes(_ context.Context, input *pkg.HookInput) error { - if !settings.HasModuleConfig(input) { +func handleDiscoveryNodes(ctx context.Context, input *pkg.HookInput) error { + hasModuleConfig, err := settings.HasModuleConfig(ctx, input) + if err != nil { + return err + } + if !hasModuleConfig { return nil } diff --git a/images/hooks/pkg/hooks/discovery-workload-nodes/hook_test.go b/images/hooks/pkg/hooks/discovery-workload-nodes/hook_test.go index de0cf28ce5..7d0b6e5774 100644 --- a/images/hooks/pkg/hooks/discovery-workload-nodes/hook_test.go +++ b/images/hooks/pkg/hooks/discovery-workload-nodes/hook_test.go @@ -22,11 +22,14 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "github.com/tidwall/gjson" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + "hooks/pkg/settings" "github.com/deckhouse/deckhouse/pkg/log" "github.com/deckhouse/module-sdk/pkg" "github.com/deckhouse/module-sdk/testing/mock" + mcapi "github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig/api" ) func TestDiscoveryWorkloadNodes(t *testing.T) { @@ -34,14 +37,34 @@ func TestDiscoveryWorkloadNodes(t *testing.T) { RunSpecs(t, "DiscoveryWorkloadNodes Suite") } +type fakeKubernetesClient struct { + pkg.KubernetesClient + get func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error +} + +func (f *fakeKubernetesClient) Get(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object, _ ...ctrlclient.GetOption) error { + return f.get(ctx, key, obj) +} + var _ = Describe("DiscoveryWorkloadNodes", func() { var ( + dc *mock.DependencyContainerMock snapshots *mock.SnapshotsMock values *mock.OutputPatchableValuesCollectorMock ) - newInput := func() *pkg.HookInput { + newInput := func(withModuleConfig bool) *pkg.HookInput { + dc.GetK8sClientMock.Return(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error { + mc := obj.(*mcapi.ModuleConfig) + if withModuleConfig { + *mc = *settings.NewModuleConfigForTest(map[string]any{"dvcr": map[string]any{}, "virtualMachineCIDRs": []any{"10.0.0.0/24"}}) + } else { + *mc = *settings.NewModuleConfigForTest(nil) + } + return nil + }}, nil) return &pkg.HookInput{ + DC: dc, Snapshots: snapshots, Values: values, Logger: log.NewNop(), @@ -49,22 +72,22 @@ var _ = Describe("DiscoveryWorkloadNodes", func() { } BeforeEach(func() { + dc = mock.NewDependencyContainerMock(GinkgoT()) snapshots = mock.NewSnapshotsMock(GinkgoT()) values = mock.NewPatchableValuesCollectorMock(GinkgoT()) }) AfterEach(func() { + dc = nil snapshots = nil values = nil }) - It("should do nothing when copied module config is absent", func() { - values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Result{}) - Expect(handleDiscoveryNodes(context.Background(), newInput())).To(Succeed()) + It("should do nothing when module config is incomplete", func() { + Expect(handleDiscoveryNodes(context.Background(), newInput(false))).To(Succeed()) }) - It("should set node count when copied module config exists", func() { - values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Parse(`{"dvcr":{},"virtualMachineCIDRs":["10.0.0.0/24"]}`)) + It("should set node count when module config exists", func() { snapshots.GetMock.When(discoveryNodesSnapshot).Then([]pkg.Snapshot{ mock.NewSnapshotMock(GinkgoT()), mock.NewSnapshotMock(GinkgoT()), @@ -78,7 +101,7 @@ var _ = Describe("DiscoveryWorkloadNodes", func() { Expect(v).To(Equal(2)) }) - Expect(handleDiscoveryNodes(context.Background(), newInput())).To(Succeed()) + Expect(handleDiscoveryNodes(context.Background(), newInput(true))).To(Succeed()) Expect(setCalls).To(Equal(1)) }) }) diff --git a/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook.go b/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook.go index bc7033f2db..176a3d78c5 100644 --- a/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook.go +++ b/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook.go @@ -74,8 +74,12 @@ var configDVCRSecrets = &pkg.HookConfig{ Queue: fmt.Sprintf("modules/%s", settings.ModuleName), } -func handlerDVCRSecrets(_ context.Context, input *pkg.HookInput) error { - if !settings.HasModuleConfig(input) { +func handlerDVCRSecrets(ctx context.Context, input *pkg.HookInput) error { + hasModuleConfig, err := settings.HasModuleConfig(ctx, input) + if err != nil { + return err + } + if !hasModuleConfig { return nil } diff --git a/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook_test.go b/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook_test.go index b08180e2dd..429f68ca3a 100644 --- a/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook_test.go +++ b/images/hooks/pkg/hooks/generate-secret-for-dvcr/hook_test.go @@ -24,10 +24,14 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/tidwall/gjson" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + "hooks/pkg/settings" "github.com/deckhouse/deckhouse/pkg/log" "github.com/deckhouse/module-sdk/pkg" "github.com/deckhouse/module-sdk/testing/mock" + mcapi "github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig/api" ) func TestSetDVCRSecrets(t *testing.T) { @@ -70,7 +74,11 @@ var _ = Describe("DVCR Secrets", func() { } newInput := func() *pkg.HookInput { - values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Parse(`{"dvcr":{},"virtualMachineCIDRs":["10.0.0.0/24"]}`)) + dc.GetK8sClientMock.Return(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error { + mc := obj.(*mcapi.ModuleConfig) + *mc = *settings.NewModuleConfigForTest(map[string]any{"dvcr": map[string]any{}, "virtualMachineCIDRs": []any{"10.0.0.0/24"}}) + return nil + }}, nil) return &pkg.HookInput{ Snapshots: snapshots, Values: values, @@ -228,3 +236,12 @@ var _ = Describe("Generate secrets", func() { Expect(validateHtpasswd(password, htpasswd)).To(BeTrue()) }) }) + +type fakeKubernetesClient struct { + pkg.KubernetesClient + get func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error +} + +func (f *fakeKubernetesClient) Get(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object, _ ...ctrlclient.GetOption) error { + return f.get(ctx, key, obj) +} diff --git a/images/hooks/pkg/hooks/tls-certificates-api-proxy/main.go b/images/hooks/pkg/hooks/tls-certificates-api-proxy/main.go index ab591b93e3..8ee1daab3d 100644 --- a/images/hooks/pkg/hooks/tls-certificates-api-proxy/main.go +++ b/images/hooks/pkg/hooks/tls-certificates-api-proxy/main.go @@ -38,7 +38,11 @@ var conf = tlscertificate.GenSelfSignedTLSHookConf{ var reconcile = func(conf tlscertificate.GenSelfSignedTLSHookConf) pkg.ReconcileFunc { return func(ctx context.Context, input *pkg.HookInput) error { - if !settings.HasModuleConfig(input) { + hasModuleConfig, err := settings.HasModuleConfig(ctx, input) + if err != nil { + return err + } + if !hasModuleConfig { return nil } diff --git a/images/hooks/pkg/hooks/tls-certificates-api-proxy/main_test.go b/images/hooks/pkg/hooks/tls-certificates-api-proxy/main_test.go index 5f0d4955f6..d410046830 100644 --- a/images/hooks/pkg/hooks/tls-certificates-api-proxy/main_test.go +++ b/images/hooks/pkg/hooks/tls-certificates-api-proxy/main_test.go @@ -20,17 +20,32 @@ import ( "context" "testing" - "github.com/tidwall/gjson" + "hooks/pkg/settings" "github.com/deckhouse/module-sdk/pkg" "github.com/deckhouse/module-sdk/testing/mock" + mcapi "github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig/api" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" ) -func TestReconcileSkipsWithoutModuleConfig(t *testing.T) { - values := mock.NewPatchableValuesCollectorMock(t) - values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Result{}) +type fakeKubernetesClient struct { + pkg.KubernetesClient + get func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error +} - input := &pkg.HookInput{Values: values} +func (f *fakeKubernetesClient) Get(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object, _ ...ctrlclient.GetOption) error { + return f.get(ctx, key, obj) +} + +func TestReconcileSkipsWithoutModuleConfig(t *testing.T) { + dc := mock.NewDependencyContainerMock(t) + dc.GetK8sClientMock.Return(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error { + mc := obj.(*mcapi.ModuleConfig) + *mc = *settings.NewModuleConfigForTest(nil) + return nil + }}, nil) + + input := &pkg.HookInput{DC: dc} if err := reconcile(conf)(context.Background(), input); err != nil { t.Fatalf("expected nil error, got %v", err) } diff --git a/images/hooks/pkg/hooks/tls-certificates-api/main.go b/images/hooks/pkg/hooks/tls-certificates-api/main.go index 222c9e0e82..9b11668aff 100644 --- a/images/hooks/pkg/hooks/tls-certificates-api/main.go +++ b/images/hooks/pkg/hooks/tls-certificates-api/main.go @@ -47,7 +47,11 @@ var conf = tlscertificate.GenSelfSignedTLSHookConf{ var reconcile = func(conf tlscertificate.GenSelfSignedTLSHookConf) pkg.ReconcileFunc { return func(ctx context.Context, input *pkg.HookInput) error { - if !settings.HasModuleConfig(input) { + hasModuleConfig, err := settings.HasModuleConfig(ctx, input) + if err != nil { + return err + } + if !hasModuleConfig { return nil } diff --git a/images/hooks/pkg/hooks/tls-certificates-api/main_test.go b/images/hooks/pkg/hooks/tls-certificates-api/main_test.go index a67ee6b909..7914e906b5 100644 --- a/images/hooks/pkg/hooks/tls-certificates-api/main_test.go +++ b/images/hooks/pkg/hooks/tls-certificates-api/main_test.go @@ -20,17 +20,32 @@ import ( "context" "testing" - "github.com/tidwall/gjson" + "hooks/pkg/settings" "github.com/deckhouse/module-sdk/pkg" "github.com/deckhouse/module-sdk/testing/mock" + mcapi "github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig/api" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" ) -func TestReconcileSkipsWithoutModuleConfig(t *testing.T) { - values := mock.NewPatchableValuesCollectorMock(t) - values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Result{}) +type fakeKubernetesClient struct { + pkg.KubernetesClient + get func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error +} - input := &pkg.HookInput{Values: values} +func (f *fakeKubernetesClient) Get(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object, _ ...ctrlclient.GetOption) error { + return f.get(ctx, key, obj) +} + +func TestReconcileSkipsWithoutModuleConfig(t *testing.T) { + dc := mock.NewDependencyContainerMock(t) + dc.GetK8sClientMock.Return(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error { + mc := obj.(*mcapi.ModuleConfig) + *mc = *settings.NewModuleConfigForTest(nil) + return nil + }}, nil) + + input := &pkg.HookInput{DC: dc} if err := reconcile(conf)(context.Background(), input); err != nil { t.Fatalf("expected nil error, got %v", err) } diff --git a/images/hooks/pkg/hooks/tls-certificates-audit/hook.go b/images/hooks/pkg/hooks/tls-certificates-audit/hook.go index 9129562008..7ffe93b251 100644 --- a/images/hooks/pkg/hooks/tls-certificates-audit/hook.go +++ b/images/hooks/pkg/hooks/tls-certificates-audit/hook.go @@ -41,7 +41,11 @@ var conf = tlscertificate.GenSelfSignedTLSHookConf{ var genSelfSignedTLS = func(conf tlscertificate.GenSelfSignedTLSHookConf) pkg.ReconcileFunc { return func(ctx context.Context, input *pkg.HookInput) error { - if !settings.HasModuleConfig(input) { + hasModuleConfig, err := settings.HasModuleConfig(ctx, input) + if err != nil { + return err + } + if !hasModuleConfig { return nil } diff --git a/images/hooks/pkg/hooks/tls-certificates-controller/hook.go b/images/hooks/pkg/hooks/tls-certificates-controller/hook.go index 1f0b8cb2a4..2404712064 100644 --- a/images/hooks/pkg/hooks/tls-certificates-controller/hook.go +++ b/images/hooks/pkg/hooks/tls-certificates-controller/hook.go @@ -47,7 +47,11 @@ var conf = tlscertificate.GenSelfSignedTLSHookConf{ var reconcile = func(conf tlscertificate.GenSelfSignedTLSHookConf) pkg.ReconcileFunc { return func(ctx context.Context, input *pkg.HookInput) error { - if !settings.HasModuleConfig(input) { + hasModuleConfig, err := settings.HasModuleConfig(ctx, input) + if err != nil { + return err + } + if !hasModuleConfig { return nil } diff --git a/images/hooks/pkg/hooks/tls-certificates-controller/hook_test.go b/images/hooks/pkg/hooks/tls-certificates-controller/hook_test.go index f3ef0ba2e3..18a6beec7c 100644 --- a/images/hooks/pkg/hooks/tls-certificates-controller/hook_test.go +++ b/images/hooks/pkg/hooks/tls-certificates-controller/hook_test.go @@ -20,17 +20,32 @@ import ( "context" "testing" - "github.com/tidwall/gjson" + "hooks/pkg/settings" "github.com/deckhouse/module-sdk/pkg" "github.com/deckhouse/module-sdk/testing/mock" + mcapi "github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig/api" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" ) -func TestReconcileSkipsWithoutModuleConfig(t *testing.T) { - values := mock.NewPatchableValuesCollectorMock(t) - values.GetMock.When("virtualization.internal.moduleConfig").Then(gjson.Result{}) +type fakeKubernetesClient struct { + pkg.KubernetesClient + get func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error +} - input := &pkg.HookInput{Values: values} +func (f *fakeKubernetesClient) Get(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object, _ ...ctrlclient.GetOption) error { + return f.get(ctx, key, obj) +} + +func TestReconcileSkipsWithoutModuleConfig(t *testing.T) { + dc := mock.NewDependencyContainerMock(t) + dc.GetK8sClientMock.Return(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error { + mc := obj.(*mcapi.ModuleConfig) + *mc = *settings.NewModuleConfigForTest(nil) + return nil + }}, nil) + + input := &pkg.HookInput{DC: dc} if err := reconcile(conf)(context.Background(), input); err != nil { t.Fatalf("expected nil error, got %v", err) } diff --git a/images/hooks/pkg/hooks/tls-certificates-dvcr/hook.go b/images/hooks/pkg/hooks/tls-certificates-dvcr/hook.go index ebf69e7c59..594dfddec8 100644 --- a/images/hooks/pkg/hooks/tls-certificates-dvcr/hook.go +++ b/images/hooks/pkg/hooks/tls-certificates-dvcr/hook.go @@ -16,6 +16,7 @@ limitations under the License. package tls_certificates_dvcr import ( + "context" "fmt" "hooks/pkg/settings" @@ -51,7 +52,12 @@ var _ = tlscertificate.RegisterInternalTLSHookEM(tlscertificate.GenSelfSignedTLS CommonCAValuesPath: fmt.Sprintf("%s.internal.rootCA", settings.ModuleName), BeforeHookCheck: func(input *pkg.HookInput) bool { - if !settings.HasModuleConfig(input) { + hasModuleConfig, err := settings.HasModuleConfig(context.Background(), input) + if err != nil { + input.Logger.Error("Check module config before DVCR TLS hook", "error", err) + return false + } + if !hasModuleConfig { return false } diff --git a/images/hooks/pkg/hooks/tls-certificates-dvcr/hook_test.go b/images/hooks/pkg/hooks/tls-certificates-dvcr/hook_test.go index db36aa94c2..b69050f46b 100644 --- a/images/hooks/pkg/hooks/tls-certificates-dvcr/hook_test.go +++ b/images/hooks/pkg/hooks/tls-certificates-dvcr/hook_test.go @@ -17,22 +17,40 @@ limitations under the License. package tls_certificates_dvcr import ( + "context" "testing" - "github.com/tidwall/gjson" - "hooks/pkg/settings" "github.com/deckhouse/module-sdk/pkg" "github.com/deckhouse/module-sdk/testing/mock" + mcapi "github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig/api" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" ) -func TestBeforeHookCheckSkipsWithoutModuleConfig(t *testing.T) { - values := mock.NewPatchableValuesCollectorMock(t) - values.GetMock.When(settings.InternalValuesConfigCopyPath).Then(gjson.Result{}) +type fakeKubernetesClient struct { + pkg.KubernetesClient + get func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error +} - input := &pkg.HookInput{Values: values} - if settings.HasModuleConfig(input) { - t.Fatalf("expected copied module config to be absent") +func (f *fakeKubernetesClient) Get(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object, _ ...ctrlclient.GetOption) error { + return f.get(ctx, key, obj) +} + +func TestBeforeHookCheckSkipsWithoutModuleConfig(t *testing.T) { + dc := mock.NewDependencyContainerMock(t) + dc.GetK8sClientMock.Return(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error { + mc := obj.(*mcapi.ModuleConfig) + mc.Name = "virtualization" + mc.Spec.Settings = nil + return nil + }}, nil) + + ok, err := settings.HasModuleConfig(context.Background(), &pkg.HookInput{DC: dc}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ok { + t.Fatalf("expected module config check to return false") } } diff --git a/images/hooks/pkg/settings/module_config.go b/images/hooks/pkg/settings/module_config.go index 6ce6b0b66b..75616184c8 100644 --- a/images/hooks/pkg/settings/module_config.go +++ b/images/hooks/pkg/settings/module_config.go @@ -16,17 +16,70 @@ limitations under the License. package settings -import "github.com/deckhouse/module-sdk/pkg" +import ( + "context" + "fmt" -func HasModuleConfig(input *pkg.HookInput) bool { - config := input.Values.Get(InternalValuesConfigCopyPath) - if !config.Exists() { - return false + "github.com/deckhouse/module-sdk/pkg" + mcapi "github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig/api" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func HasModuleConfig(ctx context.Context, input *pkg.HookInput) (bool, error) { + if input == nil || input.DC == nil { + return false, fmt.Errorf("dependency container is nil") + } + + k8sClient, err := input.DC.GetK8sClient(addModuleConfigScheme()) + if err != nil { + return false, fmt.Errorf("get kubernetes client: %w", err) + } + + var moduleConfig mcapi.ModuleConfig + err = k8sClient.Get(ctx, client.ObjectKey{Name: ModuleName}, &moduleConfig) + if err != nil { + if apierrors.IsNotFound(err) { + return false, nil + } + return false, fmt.Errorf("get ModuleConfig/%s: %w", ModuleName, err) + } + + if moduleConfig.Spec.Settings == nil { + return false, nil + } + + if _, ok := moduleConfig.Spec.Settings["virtualMachineCIDRs"]; !ok { + return false, nil } - if !config.IsObject() { - return false + if _, ok := moduleConfig.Spec.Settings["dvcr"]; !ok { + return false, nil } - return len(config.Map()) > 0 + return true, nil +} + +type moduleConfigSchemeOption struct{} + +func (moduleConfigSchemeOption) Apply(optsApplier pkg.KubernetesOptionApplier) { + optsApplier.WithSchemeBuilder(mcapi.SchemeBuilder) +} + +func addModuleConfigScheme() pkg.KubernetesOption { + return moduleConfigSchemeOption{} +} + +func NewModuleConfigForTest(settings map[string]any) *mcapi.ModuleConfig { + return &mcapi.ModuleConfig{ + TypeMeta: metav1.TypeMeta{ + Kind: "ModuleConfig", + APIVersion: "deckhouse.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: ModuleName}, + Spec: mcapi.ModuleConfigSpec{ + Settings: settings, + }, + } } diff --git a/images/hooks/pkg/settings/module_config_test.go b/images/hooks/pkg/settings/module_config_test.go index 7ce016ac6b..4a69ef3b1b 100644 --- a/images/hooks/pkg/settings/module_config_test.go +++ b/images/hooks/pkg/settings/module_config_test.go @@ -17,52 +17,111 @@ limitations under the License. package settings import ( + "context" "testing" - "github.com/tidwall/gjson" - "github.com/deckhouse/module-sdk/pkg" "github.com/deckhouse/module-sdk/testing/mock" + mcapi "github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig/api" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" ) +type fakeKubernetesClient struct { + pkg.KubernetesClient + get func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error +} + +func (f *fakeKubernetesClient) Get(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object, _ ...ctrlclient.GetOption) error { + return f.get(ctx, key, obj) +} + func TestHasModuleConfig(t *testing.T) { - newInput := func(values *mock.OutputPatchableValuesCollectorMock) *pkg.HookInput { - return &pkg.HookInput{Values: values} + newInput := func(client pkg.KubernetesClient, err error) *pkg.HookInput { + dc := mock.NewDependencyContainerMock(t) + dc.GetK8sClientMock.Return(client, err) + return &pkg.HookInput{DC: dc} } - t.Run("returns false when moduleConfig is absent", func(t *testing.T) { - values := mock.NewPatchableValuesCollectorMock(t) - values.GetMock.When(InternalValuesConfigCopyPath).Then(gjson.Result{}) + t.Run("returns false when module config does not exist", func(t *testing.T) { + input := newInput(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error { + return apierrors.NewNotFound(schema.GroupResource{Group: "deckhouse.io", Resource: "moduleconfigs"}, ModuleName) + }}, nil) - if HasModuleConfig(newInput(values)) { + ok, err := HasModuleConfig(context.Background(), input) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ok { t.Fatalf("expected HasModuleConfig to return false") } }) - t.Run("returns false when moduleConfig is not an object", func(t *testing.T) { - values := mock.NewPatchableValuesCollectorMock(t) - values.GetMock.When(InternalValuesConfigCopyPath).Then(gjson.Result{Type: gjson.String, Str: "value"}) + t.Run("returns false when settings are nil", func(t *testing.T) { + input := newInput(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error { + mc := obj.(*mcapi.ModuleConfig) + *mc = *NewModuleConfigForTest(nil) + return nil + }}, nil) - if HasModuleConfig(newInput(values)) { + ok, err := HasModuleConfig(context.Background(), input) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ok { t.Fatalf("expected HasModuleConfig to return false") } }) - t.Run("returns false when moduleConfig is an empty object", func(t *testing.T) { - values := mock.NewPatchableValuesCollectorMock(t) - values.GetMock.When(InternalValuesConfigCopyPath).Then(gjson.Parse(`{}`)) + t.Run("returns false when required settings are absent", func(t *testing.T) { + input := newInput(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error { + mc := obj.(*mcapi.ModuleConfig) + *mc = *NewModuleConfigForTest(map[string]any{}) + return nil + }}, nil) - if HasModuleConfig(newInput(values)) { + ok, err := HasModuleConfig(context.Background(), input) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ok { t.Fatalf("expected HasModuleConfig to return false") } }) - t.Run("returns true when moduleConfig is a non-empty object", func(t *testing.T) { - values := mock.NewPatchableValuesCollectorMock(t) - values.GetMock.When(InternalValuesConfigCopyPath).Then(gjson.Parse(`{"dvcr":{},"virtualMachineCIDRs":["10.0.0.0/24"]}`)) - - if !HasModuleConfig(newInput(values)) { + t.Run("returns true when required settings exist", func(t *testing.T) { + input := newInput(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error { + mc := obj.(*mcapi.ModuleConfig) + *mc = *NewModuleConfigForTest(map[string]any{ + "virtualMachineCIDRs": []any{"10.0.0.0/24"}, + "dvcr": map[string]any{}, + }) + return nil + }}, nil) + + ok, err := HasModuleConfig(context.Background(), input) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !ok { t.Fatalf("expected HasModuleConfig to return true") } }) + + t.Run("returns error when kubernetes client cannot be created", func(t *testing.T) { + input := newInput(nil, staticErr("boom")) + + ok, err := HasModuleConfig(context.Background(), input) + if err == nil { + t.Fatalf("expected error") + } + if ok { + t.Fatalf("expected HasModuleConfig to return false") + } + }) } + +type staticErr string + +func (e staticErr) Error() string { return string(e) }