From 2b8e327871e693c6a9be98662d9cad3797632b03 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Thu, 5 Sep 2024 09:28:12 +0200 Subject: [PATCH 1/9] Checking if AL need to be enabled in strict mode --- internal/auditlogging/auditlogging.go | 19 +++++++++++++----- internal/auditlogging/mocks/AuditLogging.go | 18 ++++++++--------- .../auditlogging/tests/auditlogging_test.go | 16 ++++++++------- .../fsm/runtime_fsm_configure_auditlog.go | 4 ++-- .../runtime_fsm_configure_auditlog_test.go | 20 ++++++++++++------- .../controller/runtime/fsm/utilz_for_test.go | 2 +- internal/gardener/shoot/converter.go | 1 + 7 files changed, 49 insertions(+), 31 deletions(-) diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index 5aaf150a..aeeabf6f 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -22,7 +22,7 @@ const ( //go:generate mockery --name=AuditLogging type AuditLogging interface { - Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) + Enable(ctx context.Context, shoot *gardener.Shoot, mandatory bool) (bool, error) } //go:generate mockery --name=AuditLogConfigurator @@ -93,7 +93,7 @@ func (a *auditLogConfig) GetSeedObj(ctx context.Context, seedKey types.Namespace return seed, nil } -func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) { +func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot, mandatory bool) (bool, error) { seedName := getSeedName(*shoot) if !al.CanEnableAuditLogsForShoot(seedName) { @@ -113,7 +113,7 @@ func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, er return false, errors.Wrap(err, "Cannot get Gardener Seed object") } - annotated, err := ApplyAuditLogConfig(shoot, auditConfigFromFile, seed.Spec.Provider.Type) + annotated, err := ApplyAuditLogConfig(shoot, auditConfigFromFile, seed.Spec.Provider.Type, mandatory) if err != nil { return false, errors.Wrap(err, "Error during enabling Audit Logs on shoot: "+shoot.Name) @@ -128,7 +128,7 @@ func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, er return annotated, nil } -func ApplyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string) (bool, error) { +func ApplyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string, mandatory bool) (bool, error) { providerConfig := auditConfigFromFile[providerType] if providerConfig == nil { return false, fmt.Errorf("cannot find config for provider %s", providerType) @@ -140,8 +140,11 @@ func ApplyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]m } tenant, ok := providerConfig[auditID] - if !ok { + if !ok && mandatory { return false, fmt.Errorf("auditlog config for region %s, provider %s is empty", auditID, providerType) + } else if !ok { + rollbackAuditPolicy(shoot) + return false, nil } changedExt, err := configureExtension(shoot, tenant) @@ -285,6 +288,12 @@ func newAuditPolicyConfig(policyConfigMapName string) *gardener.AuditConfig { } } +func rollbackAuditPolicy(shoot *gardener.Shoot) { + if shoot.Spec.Kubernetes.KubeAPIServer != nil { + shoot.Spec.Kubernetes.KubeAPIServer.AuditConfig = nil + } +} + func (a *auditLogConfig) UpdateShoot(ctx context.Context, shoot *gardener.Shoot) error { return a.client.Update(ctx, shoot) } diff --git a/internal/auditlogging/mocks/AuditLogging.go b/internal/auditlogging/mocks/AuditLogging.go index f670fd14..da6306d4 100644 --- a/internal/auditlogging/mocks/AuditLogging.go +++ b/internal/auditlogging/mocks/AuditLogging.go @@ -14,9 +14,9 @@ type AuditLogging struct { mock.Mock } -// Enable provides a mock function with given fields: ctx, shoot -func (_m *AuditLogging) Enable(ctx context.Context, shoot *v1beta1.Shoot) (bool, error) { - ret := _m.Called(ctx, shoot) +// Enable provides a mock function with given fields: ctx, shoot, mandatory +func (_m *AuditLogging) Enable(ctx context.Context, shoot *v1beta1.Shoot, mandatory bool) (bool, error) { + ret := _m.Called(ctx, shoot, mandatory) if len(ret) == 0 { panic("no return value specified for Enable") @@ -24,17 +24,17 @@ func (_m *AuditLogging) Enable(ctx context.Context, shoot *v1beta1.Shoot) (bool, var r0 bool var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *v1beta1.Shoot) (bool, error)); ok { - return rf(ctx, shoot) + if rf, ok := ret.Get(0).(func(context.Context, *v1beta1.Shoot, bool) (bool, error)); ok { + return rf(ctx, shoot, mandatory) } - if rf, ok := ret.Get(0).(func(context.Context, *v1beta1.Shoot) bool); ok { - r0 = rf(ctx, shoot) + if rf, ok := ret.Get(0).(func(context.Context, *v1beta1.Shoot, bool) bool); ok { + r0 = rf(ctx, shoot, mandatory) } else { r0 = ret.Get(0).(bool) } - if rf, ok := ret.Get(1).(func(context.Context, *v1beta1.Shoot) error); ok { - r1 = rf(ctx, shoot) + if rf, ok := ret.Get(1).(func(context.Context, *v1beta1.Shoot, bool) error); ok { + r1 = rf(ctx, shoot, mandatory) } else { r1 = ret.Error(1) } diff --git a/internal/auditlogging/tests/auditlogging_test.go b/internal/auditlogging/tests/auditlogging_test.go index 6a74d9b4..94d3a66b 100644 --- a/internal/auditlogging/tests/auditlogging_test.go +++ b/internal/auditlogging/tests/auditlogging_test.go @@ -13,6 +13,8 @@ import ( "testing" ) +var auditLogMandatory = true + func TestEnable(t *testing.T) { t.Run("Should successfully enable Audit Log for Shoot", func(t *testing.T) { // given @@ -31,7 +33,7 @@ func TestEnable(t *testing.T) { // when auditLog := &auditlogging.AuditLog{AuditLogConfigurator: configurator} - enable, err := auditLog.Enable(ctx, shoot) + enable, err := auditLog.Enable(ctx, shoot, auditLogMandatory) // then configurator.AssertExpectations(t) @@ -58,7 +60,7 @@ func TestEnable(t *testing.T) { // when auditLog := &auditlogging.AuditLog{AuditLogConfigurator: configurator} - enable, err := auditLog.Enable(ctx, shoot) + enable, err := auditLog.Enable(ctx, shoot, auditLogMandatory) // then configurator.AssertExpectations(t) @@ -78,7 +80,7 @@ func TestEnable(t *testing.T) { // when auditLog := &auditlogging.AuditLog{AuditLogConfigurator: configurator} - enable, err := auditLog.Enable(ctx, shoot) + enable, err := auditLog.Enable(ctx, shoot, auditLogMandatory) // then configurator.AssertExpectations(t) @@ -94,7 +96,7 @@ func TestApplyAuditLogConfig(t *testing.T) { configFromFile := fileConfigData() // when - annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws") + annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws", auditLogMandatory) // then require.True(t, annotated) @@ -106,7 +108,7 @@ func TestApplyAuditLogConfig(t *testing.T) { shoot := shootForTest() // when - annotated, err := auditlogging.ApplyAuditLogConfig(shoot, map[string]map[string]auditlogging.AuditLogData{}, "aws") + annotated, err := auditlogging.ApplyAuditLogConfig(shoot, map[string]map[string]auditlogging.AuditLogData{}, "aws", auditLogMandatory) // then require.False(t, annotated) @@ -120,7 +122,7 @@ func TestApplyAuditLogConfig(t *testing.T) { shoot.Spec.Region = "" // when - annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws") + annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws", auditLogMandatory) // then require.False(t, annotated) @@ -135,7 +137,7 @@ func TestApplyAuditLogConfig(t *testing.T) { } // when - annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws") + annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws", auditLogMandatory) // then require.False(t, annotated) diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index d63c57e9..899af557 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -10,7 +10,7 @@ import ( func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, *ctrl.Result, error) { m.log.Info("Configure Audit Log state") - wasAuditLogEnabled, err := m.AuditLogging.Enable(ctx, s.shoot) + wasAuditLogEnabled, err := m.AuditLogging.Enable(ctx, s.shoot, m.AuditLog.Mandatory) if wasAuditLogEnabled { m.log.Info("Audit Log configured for shoot: " + s.shoot.Name) @@ -36,7 +36,7 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, s.instance.UpdateStateReady( imv1.ConditionTypeAuditLogConfigured, imv1.ConditionReasonAuditLogConfigured, - "Audit Log configured successfully", + "Audit Log state completed successfully", ) } diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go index eab363b7..f27c0a99 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -33,10 +33,12 @@ func TestAuditLogState(t *testing.T) { }, } - auditLog.On("Enable", ctx, shoot).Return(true, nil).Once() + fsm := &fsm{AuditLogging: auditLog} + fsm.AuditLog.Mandatory = true + + auditLog.On("Enable", ctx, shoot, true).Return(true, nil).Once() // when - fsm := &fsm{AuditLogging: auditLog} stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) // set the time to its zero value for comparison purposes @@ -64,14 +66,16 @@ func TestAuditLogState(t *testing.T) { Type: string(v1.ConditionTypeAuditLogConfigured), Status: "True", Reason: string(v1.ConditionReasonAuditLogConfigured), - Message: "Audit Log configured successfully", + Message: "Audit Log state completed successfully", }, } - auditLog.On("Enable", ctx, shoot).Return(false, nil).Once() + fsm := &fsm{AuditLogging: auditLog} + fsm.AuditLog.Mandatory = true + + auditLog.On("Enable", ctx, shoot, true).Return(false, nil).Once() // when - fsm := &fsm{AuditLogging: auditLog} stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) // set the time to its zero value for comparison purposes @@ -103,10 +107,12 @@ func TestAuditLogState(t *testing.T) { }, } - auditLog.On("Enable", ctx, shoot).Return(false, errors.New("some error during configuration")).Once() + fsm := &fsm{AuditLogging: auditLog} + fsm.AuditLog.Mandatory = true + + auditLog.On("Enable", ctx, shoot, true).Return(false, errors.New("some error during configuration")).Once() // when - fsm := &fsm{AuditLogging: auditLog} stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) // set the time to its zero value for comparison purposes diff --git a/internal/controller/runtime/fsm/utilz_for_test.go b/internal/controller/runtime/fsm/utilz_for_test.go index 9e92a2ef..9a84b446 100644 --- a/internal/controller/runtime/fsm/utilz_for_test.go +++ b/internal/controller/runtime/fsm/utilz_for_test.go @@ -116,7 +116,7 @@ type stubAuditLogging struct { err error } -func (s *stubAuditLogging) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) { +func (s *stubAuditLogging) Enable(ctx context.Context, shoot *gardener.Shoot, mandatory bool) (bool, error) { return s.isEnabled, s.err } diff --git a/internal/gardener/shoot/converter.go b/internal/gardener/shoot/converter.go index dd1f0d3a..edf32cf1 100644 --- a/internal/gardener/shoot/converter.go +++ b/internal/gardener/shoot/converter.go @@ -39,6 +39,7 @@ type KubernetesConfig struct { } type AuditLogConfig struct { + Mandatory bool `json:"mandatory"` PolicyConfigMapName string `json:"policyConfigMapName"` TenantConfigPath string `json:"tenantConfigPath"` } From bea2e29442e29f233cadac8fbdc68ab14e989646 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Thu, 5 Sep 2024 14:33:06 +0200 Subject: [PATCH 2/9] Refactor --- cmd/main.go | 24 ++++++++++++++++++++++++ internal/gardener/shoot/converter.go | 7 +++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 51f42dc1..4ec9da03 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -17,8 +17,10 @@ limitations under the License. package main import ( + "encoding/json" "flag" "fmt" + "github.com/kyma-project/infrastructure-manager/internal/auditlogging" "io" "os" "time" @@ -171,6 +173,28 @@ func main() { os.Exit(1) } + // load Audit Log configuration + getReader = func() (io.Reader, error) { + return os.Open(converterConfig.AuditLog.TenantConfigPath) + } + var auditLogConfig map[string]map[string]auditlogging.AuditLogData + r, err := getReader() + if err != nil { + setupLog.Error(err, "unable to get reader") + os.Exit(1) + } + + if err = json.NewDecoder(r).Decode(&auditLogConfig); err != nil { + setupLog.Error(err, "unable to decode Audit Log configuration") + os.Exit(1) + } + + auditLogValidate := validator.New(validator.WithRequiredStructEnabled()) + if err = auditLogValidate.Struct(auditLogConfig); err != nil { + setupLog.Error(err, "invalid audit log configuration") + os.Exit(1) + } + cfg := fsm.RCCfg{ Finalizer: infrastructuremanagerv1.Finalizer, ShootNamesapace: gardenerNamespace, diff --git a/internal/gardener/shoot/converter.go b/internal/gardener/shoot/converter.go index edf32cf1..aa819a47 100644 --- a/internal/gardener/shoot/converter.go +++ b/internal/gardener/shoot/converter.go @@ -39,9 +39,8 @@ type KubernetesConfig struct { } type AuditLogConfig struct { - Mandatory bool `json:"mandatory"` - PolicyConfigMapName string `json:"policyConfigMapName"` - TenantConfigPath string `json:"tenantConfigPath"` + PolicyConfigMapName string `json:"policyConfigMapName" validate:"required"` + TenantConfigPath string `json:"tenantConfigPath" validate:"required"` } type ReaderGetter = func() (io.Reader, error) @@ -52,7 +51,7 @@ type ConverterConfig struct { Provider ProviderConfig `json:"provider"` MachineImage MachineImageConfig `json:"machineImage" validate:"required"` Gardener GardenerConfig `json:"gardener" validate:"required"` - AuditLog AuditLogConfig `json:"auditLogging"` + AuditLog AuditLogConfig `json:"auditLogging" validate:"required"` } func (c *ConverterConfig) Load(f ReaderGetter) error { From 1129976691343fd2536d3cc5a4fee0ef8e89c9e9 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 9 Sep 2024 09:33:41 +0200 Subject: [PATCH 3/9] Change the flow of Audit Log state --- api/v1/runtime_types.go | 7 +- cmd/main.go | 75 +++++++---- internal/auditlogging/auditlogging.go | 17 +-- .../controller/runtime/fsm/runtime_fsm.go | 7 +- .../fsm/runtime_fsm_configure_auditlog.go | 44 +++++-- .../runtime_fsm_configure_auditlog_test.go | 119 +++++++++++++++++- internal/gardener/shoot/converter_test.go | 8 ++ 7 files changed, 224 insertions(+), 53 deletions(-) diff --git a/api/v1/runtime_types.go b/api/v1/runtime_types.go index e4de9d24..dbbc7c83 100644 --- a/api/v1/runtime_types.go +++ b/api/v1/runtime_types.go @@ -96,9 +96,10 @@ const ( ConditionReasonSerializationError = RuntimeConditionReason("SerializationErr") ConditionReasonDeleted = RuntimeConditionReason("Deleted") - ConditionReasonAdministratorsConfigured = RuntimeConditionReason("AdministratorsConfigured") - ConditionReasonAuditLogConfigured = RuntimeConditionReason("AuditLogConfigured") - ConditionReasonAuditLogError = RuntimeConditionReason("AuditLogErr") + ConditionReasonAdministratorsConfigured = RuntimeConditionReason("AdministratorsConfigured") + ConditionReasonAuditLogConfigured = RuntimeConditionReason("AuditLogConfigured") + ConditionReasonAuditLogError = RuntimeConditionReason("AuditLogErr") + ConditionReasonAuditLogMissingRegionMapping = RuntimeConditionReason("AuditLogMissingRegionMappingErr") ) //+kubebuilder:object:root=true diff --git a/cmd/main.go b/cmd/main.go index 4ec9da03..0a51739f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -78,6 +78,7 @@ func main() { var enableRuntimeReconciler bool var converterConfigFilepath string var shootSpecDumpEnabled bool + var auditLogMandatory bool flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") @@ -92,6 +93,7 @@ func main() { flag.BoolVar(&enableRuntimeReconciler, "runtime-reconciler-enabled", defaultRuntimeReconcilerEnabled, "Feature flag for all runtime reconciler functionalities") flag.StringVar(&converterConfigFilepath, "converter-config-filepath", "/converter-config/converter_config.json", "A file path to the gardener shoot converter configuration.") flag.BoolVar(&shootSpecDumpEnabled, "shoot-spec-dump-enabled", false, "Feature flag to allow persisting specs of created shoots") + flag.BoolVar(&auditLogMandatory, "audit-log-mandatory", true, "Feature flag to enable strict mode for audit log configuration") opts := zap.Options{ Development: true, @@ -173,32 +175,17 @@ func main() { os.Exit(1) } - // load Audit Log configuration - getReader = func() (io.Reader, error) { - return os.Open(converterConfig.AuditLog.TenantConfigPath) - } - var auditLogConfig map[string]map[string]auditlogging.AuditLogData - r, err := getReader() + err = validateAuditLogConfiguration(converterConfig.AuditLog.TenantConfigPath) if err != nil { - setupLog.Error(err, "unable to get reader") - os.Exit(1) - } - - if err = json.NewDecoder(r).Decode(&auditLogConfig); err != nil { - setupLog.Error(err, "unable to decode Audit Log configuration") - os.Exit(1) - } - - auditLogValidate := validator.New(validator.WithRequiredStructEnabled()) - if err = auditLogValidate.Struct(auditLogConfig); err != nil { - setupLog.Error(err, "invalid audit log configuration") + setupLog.Error(err, "invalid Audit Log configuration") os.Exit(1) } cfg := fsm.RCCfg{ - Finalizer: infrastructuremanagerv1.Finalizer, - ShootNamesapace: gardenerNamespace, - ConverterConfig: converterConfig, + Finalizer: infrastructuremanagerv1.Finalizer, + ShootNamesapace: gardenerNamespace, + ConverterConfig: converterConfig, + AuditLogMandatory: auditLogMandatory, } if shootSpecDumpEnabled { cfg.PVCPath = "/testdata/kim" @@ -262,3 +249,49 @@ func initGardenerClients(kubeconfigPath string, namespace string) (client.Client } return gardenerClient, shootClient, dynamicKubeconfigAPI, nil } + +func validateAuditLogConfiguration(tenantConfigPath string) error { + + getReaderCloser := func() (io.ReadCloser, error) { + return os.Open(tenantConfigPath) + } + + f, err := getReaderCloser() + + defer func(f io.ReadCloser) { + _ = f.Close() + }(f) + + if err != nil { + setupLog.Error(err, "unable to open Audit Log configuration file") + return err + } + + var auditLogConfig map[string]map[string]auditlogging.AuditLogData + + if err = json.NewDecoder(f).Decode(&auditLogConfig); err != nil { + setupLog.Error(err, "unable to decode Audit Log configuration") + return err + } + + if err = validateAuditLogDataMap(auditLogConfig); err != nil { + setupLog.Error(err, "invalid audit log configuration") + return err + } + + return err +} + +func validateAuditLogDataMap(data map[string]map[string]auditlogging.AuditLogData) error { + validate := validator.New(validator.WithRequiredStructEnabled()) + + for _, nestedMap := range data { + for _, auditLogData := range nestedMap { + if err := validate.Struct(auditLogData); err != nil { + return err + } + } + } + + return nil +} diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index aeeabf6f..d551b46f 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -45,9 +45,9 @@ type auditLogConfig struct { } type AuditLogData struct { - TenantID string `json:"tenantID"` - ServiceURL string `json:"serviceURL"` - SecretName string `json:"secretName"` + TenantID string `json:"tenantID" validate:"required"` + ServiceURL string `json:"serviceURL" validate:"required,url"` + SecretName string `json:"secretName" validate:"required"` } type AuditlogExtensionConfig struct { @@ -140,11 +140,8 @@ func ApplyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]m } tenant, ok := providerConfig[auditID] - if !ok && mandatory { + if !ok { return false, fmt.Errorf("auditlog config for region %s, provider %s is empty", auditID, providerType) - } else if !ok { - rollbackAuditPolicy(shoot) - return false, nil } changedExt, err := configureExtension(shoot, tenant) @@ -288,12 +285,6 @@ func newAuditPolicyConfig(policyConfigMapName string) *gardener.AuditConfig { } } -func rollbackAuditPolicy(shoot *gardener.Shoot) { - if shoot.Spec.Kubernetes.KubeAPIServer != nil { - shoot.Spec.Kubernetes.KubeAPIServer.AuditConfig = nil - } -} - func (a *auditLogConfig) UpdateShoot(ctx context.Context, shoot *gardener.Shoot) error { return a.client.Update(ctx, shoot) } diff --git a/internal/controller/runtime/fsm/runtime_fsm.go b/internal/controller/runtime/fsm/runtime_fsm.go index 137a82ba..bc9d1a14 100644 --- a/internal/controller/runtime/fsm/runtime_fsm.go +++ b/internal/controller/runtime/fsm/runtime_fsm.go @@ -30,9 +30,10 @@ type writerGetter = func(filePath string) (io.Writer, error) // runtime reconciler specific configuration type RCCfg struct { - Finalizer string - PVCPath string - ShootNamesapace string + Finalizer string + PVCPath string + ShootNamesapace string + AuditLogMandatory bool shoot.ConverterConfig } diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index 899af557..71d8994b 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -2,15 +2,15 @@ package fsm import ( "context" - imv1 "github.com/kyma-project/infrastructure-manager/api/v1" ctrl "sigs.k8s.io/controller-runtime" + "strings" ) func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, *ctrl.Result, error) { m.log.Info("Configure Audit Log state") - wasAuditLogEnabled, err := m.AuditLogging.Enable(ctx, s.shoot, m.AuditLog.Mandatory) + wasAuditLogEnabled, err := m.AuditLogging.Enable(ctx, s.shoot, m.RCCfg.AuditLogMandatory) if wasAuditLogEnabled { m.log.Info("Audit Log configured for shoot: " + s.shoot.Name) @@ -25,13 +25,39 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, } if err != nil { - m.log.Error(err, "Failed to configure Audit Log") - s.instance.UpdateStatePending( - imv1.ConditionTypeAuditLogConfigured, - imv1.ConditionReasonAuditLogError, - "False", - err.Error(), - ) + errorMessage := err.Error() + if strings.Contains(errorMessage, "auditlog config for region") { + if m.RCCfg.AuditLogMandatory { + m.log.Error(err, "Failed to configure Audit Log, missing region mapping for this shoot") + s.instance.UpdateStatePending( + imv1.ConditionTypeAuditLogConfigured, + imv1.ConditionReasonAuditLogMissingRegionMapping, + "False", + errorMessage, + ) + } else { + m.log.Info(errorMessage, "Failed to configure Audit Log, missing region mapping for this shoot, but is not mandatory to be configured") + s.instance.UpdateStateReady( + imv1.ConditionTypeAuditLogConfigured, + imv1.ConditionReasonAuditLogMissingRegionMapping, + "Missing region mapping for this shoot. Audit Log is not mandatory. Skipping configuration") + } + } else { + if m.RCCfg.AuditLogMandatory { + m.log.Error(err, "Failed to configure Audit Log") + s.instance.UpdateStatePending( + imv1.ConditionTypeAuditLogConfigured, + imv1.ConditionReasonAuditLogError, + "False", + errorMessage) + } else { + m.log.Info(errorMessage, "Failed to configure Audit Log, but is not mandatory to be configured") + s.instance.UpdateStateReady( + imv1.ConditionTypeAuditLogConfigured, + imv1.ConditionReasonAuditLogError, + "Configuration of Audit Log is not mandatory, error for context: "+errorMessage) + } + } } else { s.instance.UpdateStateReady( imv1.ConditionTypeAuditLogConfigured, diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go index f27c0a99..3028e563 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -34,7 +34,7 @@ func TestAuditLogState(t *testing.T) { } fsm := &fsm{AuditLogging: auditLog} - fsm.AuditLog.Mandatory = true + fsm.RCCfg.AuditLogMandatory = true auditLog.On("Enable", ctx, shoot, true).Return(true, nil).Once() @@ -71,7 +71,7 @@ func TestAuditLogState(t *testing.T) { } fsm := &fsm{AuditLogging: auditLog} - fsm.AuditLog.Mandatory = true + fsm.RCCfg.AuditLogMandatory = true auditLog.On("Enable", ctx, shoot, true).Return(false, nil).Once() @@ -88,7 +88,81 @@ func TestAuditLogState(t *testing.T) { assert.Equal(t, expectedRuntimeConditions, systemState.instance.Status.Conditions) }) - t.Run("Should stop in case of error during configuration and set status on Runtime CR", func(t *testing.T) { + t.Run("Should set status on Runtime CR when region mapping is missing and Audit Log is mandatory (hard error)", func(t *testing.T) { + // given + ctx := context.Background() + auditLog := &mocks.AuditLogging{} + shoot := shootForTest() + instance := runtimeForTest() + systemState := &systemState{ + instance: instance, + shoot: shoot, + } + expectedRuntimeConditions := []metav1.Condition{ + { + Type: string(v1.ConditionTypeAuditLogConfigured), + Status: "False", + Reason: string(v1.ConditionReasonAuditLogMissingRegionMapping), + Message: "auditlog config for region", + }, + } + + fsm := &fsm{AuditLogging: auditLog} + fsm.RCCfg.AuditLogMandatory = true + + auditLog.On("Enable", ctx, shoot, true).Return(false, errors.New("auditlog config for region")).Once() + + // when + stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) + + // set the time to its zero value for comparison purposes + systemState.instance.Status.Conditions[0].LastTransitionTime = metav1.Time{} + + // then + auditLog.AssertExpectations(t) + require.Contains(t, stateFn.name(), "sFnUpdateStatus") + assert.Equal(t, v1.RuntimeStateFailed, string(systemState.instance.Status.State)) + assert.Equal(t, expectedRuntimeConditions, systemState.instance.Status.Conditions) + }) + + t.Run("Should set status on Runtime CR when region mapping is missing and Audit Log is not mandatory (proceed to next step)", func(t *testing.T) { + // given + ctx := context.Background() + auditLog := &mocks.AuditLogging{} + shoot := shootForTest() + instance := runtimeForTest() + systemState := &systemState{ + instance: instance, + shoot: shoot, + } + expectedRuntimeConditions := []metav1.Condition{ + { + Type: string(v1.ConditionTypeAuditLogConfigured), + Status: "True", + Reason: string(v1.ConditionReasonAuditLogMissingRegionMapping), + Message: "Missing region mapping for this shoot. Audit Log is not mandatory. Skipping configuration", + }, + } + + fsm := &fsm{AuditLogging: auditLog} + fsm.RCCfg.AuditLogMandatory = false + + auditLog.On("Enable", ctx, shoot, false).Return(false, errors.New("auditlog config for region")).Once() + + // when + stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) + + // set the time to its zero value for comparison purposes + systemState.instance.Status.Conditions[0].LastTransitionTime = metav1.Time{} + + // then + auditLog.AssertExpectations(t) + require.Contains(t, stateFn.name(), "sFnUpdateStatus") + assert.Equal(t, v1.RuntimeStateReady, string(systemState.instance.Status.State)) + assert.Equal(t, expectedRuntimeConditions, systemState.instance.Status.Conditions) + }) + + t.Run("Should stop in case of error during configuration if Audit Log is mandatory, and set status on Runtime CR (hard error)", func(t *testing.T) { // given ctx := context.Background() auditLog := &mocks.AuditLogging{} @@ -108,7 +182,7 @@ func TestAuditLogState(t *testing.T) { } fsm := &fsm{AuditLogging: auditLog} - fsm.AuditLog.Mandatory = true + fsm.RCCfg.AuditLogMandatory = true auditLog.On("Enable", ctx, shoot, true).Return(false, errors.New("some error during configuration")).Once() @@ -124,6 +198,43 @@ func TestAuditLogState(t *testing.T) { assert.Equal(t, v1.RuntimeStateFailed, string(systemState.instance.Status.State)) assert.Equal(t, expectedRuntimeConditions, systemState.instance.Status.Conditions) }) + + t.Run("Should stop in case of error during configuration if Audit Log is not mandatory, and set status on Runtime CR (proceed to next step)", func(t *testing.T) { + // given + ctx := context.Background() + auditLog := &mocks.AuditLogging{} + shoot := shootForTest() + instance := runtimeForTest() + systemState := &systemState{ + instance: instance, + shoot: shoot, + } + expectedRuntimeConditions := []metav1.Condition{ + { + Type: string(v1.ConditionTypeAuditLogConfigured), + Status: "True", + Reason: string(v1.ConditionReasonAuditLogError), + Message: "Configuration of Audit Log is not mandatory, error for context: some error during configuration", + }, + } + + fsm := &fsm{AuditLogging: auditLog} + fsm.RCCfg.AuditLogMandatory = false + + auditLog.On("Enable", ctx, shoot, false).Return(false, errors.New("some error during configuration")).Once() + + // when + stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) + + // set the time to its zero value for comparison purposes + systemState.instance.Status.Conditions[0].LastTransitionTime = metav1.Time{} + + // then + auditLog.AssertExpectations(t) + require.Contains(t, stateFn.name(), "sFnUpdateStatus") + assert.Equal(t, v1.RuntimeStateReady, string(systemState.instance.Status.State)) + assert.Equal(t, expectedRuntimeConditions, systemState.instance.Status.Conditions) + }) } func shootForTest() *gardener.Shoot { diff --git a/internal/gardener/shoot/converter_test.go b/internal/gardener/shoot/converter_test.go index d0be7181..51c1d4d3 100644 --- a/internal/gardener/shoot/converter_test.go +++ b/internal/gardener/shoot/converter_test.go @@ -155,6 +155,10 @@ var testReader io.Reader = strings.NewReader(`{ }, "gardener": { "projectName": "test-project" + }, + "auditLogging": { + "policyConfigMapName": "test-policy", + "tenantConfigPath": "test-path" } }`) @@ -189,6 +193,10 @@ func Test_ConverterConfig_Load_OK(t *testing.T) { Gardener: GardenerConfig{ ProjectName: "test-project", }, + AuditLog: AuditLogConfig{ + PolicyConfigMapName: "test-policy", + TenantConfigPath: "test-path", + }, } assert.Equal(t, expected, cfg) From 214ba676fe5591485c070fd53cf71042f309fe3d Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 9 Sep 2024 09:58:54 +0200 Subject: [PATCH 4/9] Fix linter, fix tests --- cmd/main.go | 1 - internal/auditlogging/auditlogging.go | 8 ++++---- internal/auditlogging/mocks/AuditLogging.go | 18 +++++++++--------- .../auditlogging/tests/auditlogging_test.go | 16 +++++++--------- .../fsm/runtime_fsm_configure_auditlog.go | 4 ++-- .../fsm/runtime_fsm_configure_auditlog_test.go | 12 ++++++------ .../controller/runtime/fsm/utilz_for_test.go | 2 +- 7 files changed, 29 insertions(+), 32 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 0a51739f..620a021e 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -251,7 +251,6 @@ func initGardenerClients(kubeconfigPath string, namespace string) (client.Client } func validateAuditLogConfiguration(tenantConfigPath string) error { - getReaderCloser := func() (io.ReadCloser, error) { return os.Open(tenantConfigPath) } diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index d551b46f..4350ddb6 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -22,7 +22,7 @@ const ( //go:generate mockery --name=AuditLogging type AuditLogging interface { - Enable(ctx context.Context, shoot *gardener.Shoot, mandatory bool) (bool, error) + Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) } //go:generate mockery --name=AuditLogConfigurator @@ -93,7 +93,7 @@ func (a *auditLogConfig) GetSeedObj(ctx context.Context, seedKey types.Namespace return seed, nil } -func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot, mandatory bool) (bool, error) { +func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) { seedName := getSeedName(*shoot) if !al.CanEnableAuditLogsForShoot(seedName) { @@ -113,7 +113,7 @@ func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot, mandatory return false, errors.Wrap(err, "Cannot get Gardener Seed object") } - annotated, err := ApplyAuditLogConfig(shoot, auditConfigFromFile, seed.Spec.Provider.Type, mandatory) + annotated, err := ApplyAuditLogConfig(shoot, auditConfigFromFile, seed.Spec.Provider.Type) if err != nil { return false, errors.Wrap(err, "Error during enabling Audit Logs on shoot: "+shoot.Name) @@ -128,7 +128,7 @@ func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot, mandatory return annotated, nil } -func ApplyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string, mandatory bool) (bool, error) { +func ApplyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string) (bool, error) { providerConfig := auditConfigFromFile[providerType] if providerConfig == nil { return false, fmt.Errorf("cannot find config for provider %s", providerType) diff --git a/internal/auditlogging/mocks/AuditLogging.go b/internal/auditlogging/mocks/AuditLogging.go index da6306d4..f670fd14 100644 --- a/internal/auditlogging/mocks/AuditLogging.go +++ b/internal/auditlogging/mocks/AuditLogging.go @@ -14,9 +14,9 @@ type AuditLogging struct { mock.Mock } -// Enable provides a mock function with given fields: ctx, shoot, mandatory -func (_m *AuditLogging) Enable(ctx context.Context, shoot *v1beta1.Shoot, mandatory bool) (bool, error) { - ret := _m.Called(ctx, shoot, mandatory) +// Enable provides a mock function with given fields: ctx, shoot +func (_m *AuditLogging) Enable(ctx context.Context, shoot *v1beta1.Shoot) (bool, error) { + ret := _m.Called(ctx, shoot) if len(ret) == 0 { panic("no return value specified for Enable") @@ -24,17 +24,17 @@ func (_m *AuditLogging) Enable(ctx context.Context, shoot *v1beta1.Shoot, mandat var r0 bool var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *v1beta1.Shoot, bool) (bool, error)); ok { - return rf(ctx, shoot, mandatory) + if rf, ok := ret.Get(0).(func(context.Context, *v1beta1.Shoot) (bool, error)); ok { + return rf(ctx, shoot) } - if rf, ok := ret.Get(0).(func(context.Context, *v1beta1.Shoot, bool) bool); ok { - r0 = rf(ctx, shoot, mandatory) + if rf, ok := ret.Get(0).(func(context.Context, *v1beta1.Shoot) bool); ok { + r0 = rf(ctx, shoot) } else { r0 = ret.Get(0).(bool) } - if rf, ok := ret.Get(1).(func(context.Context, *v1beta1.Shoot, bool) error); ok { - r1 = rf(ctx, shoot, mandatory) + if rf, ok := ret.Get(1).(func(context.Context, *v1beta1.Shoot) error); ok { + r1 = rf(ctx, shoot) } else { r1 = ret.Error(1) } diff --git a/internal/auditlogging/tests/auditlogging_test.go b/internal/auditlogging/tests/auditlogging_test.go index 94d3a66b..6a74d9b4 100644 --- a/internal/auditlogging/tests/auditlogging_test.go +++ b/internal/auditlogging/tests/auditlogging_test.go @@ -13,8 +13,6 @@ import ( "testing" ) -var auditLogMandatory = true - func TestEnable(t *testing.T) { t.Run("Should successfully enable Audit Log for Shoot", func(t *testing.T) { // given @@ -33,7 +31,7 @@ func TestEnable(t *testing.T) { // when auditLog := &auditlogging.AuditLog{AuditLogConfigurator: configurator} - enable, err := auditLog.Enable(ctx, shoot, auditLogMandatory) + enable, err := auditLog.Enable(ctx, shoot) // then configurator.AssertExpectations(t) @@ -60,7 +58,7 @@ func TestEnable(t *testing.T) { // when auditLog := &auditlogging.AuditLog{AuditLogConfigurator: configurator} - enable, err := auditLog.Enable(ctx, shoot, auditLogMandatory) + enable, err := auditLog.Enable(ctx, shoot) // then configurator.AssertExpectations(t) @@ -80,7 +78,7 @@ func TestEnable(t *testing.T) { // when auditLog := &auditlogging.AuditLog{AuditLogConfigurator: configurator} - enable, err := auditLog.Enable(ctx, shoot, auditLogMandatory) + enable, err := auditLog.Enable(ctx, shoot) // then configurator.AssertExpectations(t) @@ -96,7 +94,7 @@ func TestApplyAuditLogConfig(t *testing.T) { configFromFile := fileConfigData() // when - annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws", auditLogMandatory) + annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws") // then require.True(t, annotated) @@ -108,7 +106,7 @@ func TestApplyAuditLogConfig(t *testing.T) { shoot := shootForTest() // when - annotated, err := auditlogging.ApplyAuditLogConfig(shoot, map[string]map[string]auditlogging.AuditLogData{}, "aws", auditLogMandatory) + annotated, err := auditlogging.ApplyAuditLogConfig(shoot, map[string]map[string]auditlogging.AuditLogData{}, "aws") // then require.False(t, annotated) @@ -122,7 +120,7 @@ func TestApplyAuditLogConfig(t *testing.T) { shoot.Spec.Region = "" // when - annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws", auditLogMandatory) + annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws") // then require.False(t, annotated) @@ -137,7 +135,7 @@ func TestApplyAuditLogConfig(t *testing.T) { } // when - annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws", auditLogMandatory) + annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws") // then require.False(t, annotated) diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index 71d8994b..6d5a85d6 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -10,7 +10,7 @@ import ( func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, *ctrl.Result, error) { m.log.Info("Configure Audit Log state") - wasAuditLogEnabled, err := m.AuditLogging.Enable(ctx, s.shoot, m.RCCfg.AuditLogMandatory) + wasAuditLogEnabled, err := m.AuditLogging.Enable(ctx, s.shoot) if wasAuditLogEnabled { m.log.Info("Audit Log configured for shoot: " + s.shoot.Name) @@ -24,7 +24,7 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, return updateStatusAndRequeueAfter(gardenerRequeueDuration) } - if err != nil { + if err != nil { //nolint:nestif errorMessage := err.Error() if strings.Contains(errorMessage, "auditlog config for region") { if m.RCCfg.AuditLogMandatory { diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go index 3028e563..83ed6447 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -36,7 +36,7 @@ func TestAuditLogState(t *testing.T) { fsm := &fsm{AuditLogging: auditLog} fsm.RCCfg.AuditLogMandatory = true - auditLog.On("Enable", ctx, shoot, true).Return(true, nil).Once() + auditLog.On("Enable", ctx, shoot).Return(true, nil).Once() // when stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) @@ -73,7 +73,7 @@ func TestAuditLogState(t *testing.T) { fsm := &fsm{AuditLogging: auditLog} fsm.RCCfg.AuditLogMandatory = true - auditLog.On("Enable", ctx, shoot, true).Return(false, nil).Once() + auditLog.On("Enable", ctx, shoot).Return(false, nil).Once() // when stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) @@ -110,7 +110,7 @@ func TestAuditLogState(t *testing.T) { fsm := &fsm{AuditLogging: auditLog} fsm.RCCfg.AuditLogMandatory = true - auditLog.On("Enable", ctx, shoot, true).Return(false, errors.New("auditlog config for region")).Once() + auditLog.On("Enable", ctx, shoot).Return(false, errors.New("auditlog config for region")).Once() // when stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) @@ -147,7 +147,7 @@ func TestAuditLogState(t *testing.T) { fsm := &fsm{AuditLogging: auditLog} fsm.RCCfg.AuditLogMandatory = false - auditLog.On("Enable", ctx, shoot, false).Return(false, errors.New("auditlog config for region")).Once() + auditLog.On("Enable", ctx, shoot).Return(false, errors.New("auditlog config for region")).Once() // when stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) @@ -184,7 +184,7 @@ func TestAuditLogState(t *testing.T) { fsm := &fsm{AuditLogging: auditLog} fsm.RCCfg.AuditLogMandatory = true - auditLog.On("Enable", ctx, shoot, true).Return(false, errors.New("some error during configuration")).Once() + auditLog.On("Enable", ctx, shoot).Return(false, errors.New("some error during configuration")).Once() // when stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) @@ -221,7 +221,7 @@ func TestAuditLogState(t *testing.T) { fsm := &fsm{AuditLogging: auditLog} fsm.RCCfg.AuditLogMandatory = false - auditLog.On("Enable", ctx, shoot, false).Return(false, errors.New("some error during configuration")).Once() + auditLog.On("Enable", ctx, shoot).Return(false, errors.New("some error during configuration")).Once() // when stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) diff --git a/internal/controller/runtime/fsm/utilz_for_test.go b/internal/controller/runtime/fsm/utilz_for_test.go index 9a84b446..9e92a2ef 100644 --- a/internal/controller/runtime/fsm/utilz_for_test.go +++ b/internal/controller/runtime/fsm/utilz_for_test.go @@ -116,7 +116,7 @@ type stubAuditLogging struct { err error } -func (s *stubAuditLogging) Enable(ctx context.Context, shoot *gardener.Shoot, mandatory bool) (bool, error) { +func (s *stubAuditLogging) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) { return s.isEnabled, s.err } From a6e909e9bc00240a06333f424b052d327747437d Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 9 Sep 2024 10:03:22 +0200 Subject: [PATCH 5/9] Fix imports --- internal/auditlogging/tests/auditlogging_test.go | 3 ++- .../controller/runtime/fsm/runtime_fsm_configure_auditlog.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/auditlogging/tests/auditlogging_test.go b/internal/auditlogging/tests/auditlogging_test.go index 6a74d9b4..37e541b5 100644 --- a/internal/auditlogging/tests/auditlogging_test.go +++ b/internal/auditlogging/tests/auditlogging_test.go @@ -3,6 +3,8 @@ package tests import ( "context" "fmt" + "testing" + gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" "github.com/kyma-project/infrastructure-manager/internal/auditlogging" "github.com/kyma-project/infrastructure-manager/internal/auditlogging/mocks" @@ -10,7 +12,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" - "testing" ) func TestEnable(t *testing.T) { diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index 6d5a85d6..d37d735d 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -2,9 +2,10 @@ package fsm import ( "context" + "strings" + imv1 "github.com/kyma-project/infrastructure-manager/api/v1" ctrl "sigs.k8s.io/controller-runtime" - "strings" ) func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, *ctrl.Result, error) { From cf6ea0124787d5aa3c8017f8c641d459ca2c738b Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 9 Sep 2024 10:29:16 +0200 Subject: [PATCH 6/9] gci file --- cmd/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/main.go b/cmd/main.go index 620a021e..2f07e0a0 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -20,7 +20,6 @@ import ( "encoding/json" "flag" "fmt" - "github.com/kyma-project/infrastructure-manager/internal/auditlogging" "io" "os" "time" @@ -29,6 +28,7 @@ import ( gardener_apis "github.com/gardener/gardener/pkg/client/core/clientset/versioned/typed/core/v1beta1" "github.com/go-playground/validator/v10" infrastructuremanagerv1 "github.com/kyma-project/infrastructure-manager/api/v1" + "github.com/kyma-project/infrastructure-manager/internal/auditlogging" kubeconfig_controller "github.com/kyma-project/infrastructure-manager/internal/controller/kubeconfig" "github.com/kyma-project/infrastructure-manager/internal/controller/metrics" runtime_controller "github.com/kyma-project/infrastructure-manager/internal/controller/runtime" From 0ea85eef90a8352324aea9e130be83dbb0896c30 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 9 Sep 2024 13:11:00 +0200 Subject: [PATCH 7/9] Apply review sugestions --- docs/README.md | 1 + internal/auditlogging/auditlogging.go | 4 +++- internal/auditlogging/tests/auditlogging_test.go | 2 +- .../runtime/fsm/runtime_fsm_configure_auditlog.go | 6 +++--- .../runtime/fsm/runtime_fsm_configure_auditlog_test.go | 7 ++++--- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/README.md b/docs/README.md index 28e56bcd..ac8f7db1 100644 --- a/docs/README.md +++ b/docs/README.md @@ -18,6 +18,7 @@ You can configure the Infrastructure Manager deployment with the following argum 4. `gardener-request-timeout` - specifies the timeout for requests to Gardener. Default value is `60s`. 5. `runtime-reconciler-enabled` - feature flag responsible for enabling the runtime reconciler. Default value is `true`. 6. `shoot-spec-dump-enabled` - feature flag responsible for enabling the shoot spec dump. Default value is `false`. +7. `audit-log-mandatory` - feature flag responsible for enabling the Audit Log strict config. Default value is `true`. See [manager_gardener_secret_patch.yaml](../config/default/manager_gardener_secret_patch.yaml) for default values. diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index 4350ddb6..c4b47aec 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -20,6 +20,8 @@ const ( auditlogExtensionType = "shoot-auditlog-service" ) +var ErrMissingMapping = errors.New("missing mapping for selected region in provider config") + //go:generate mockery --name=AuditLogging type AuditLogging interface { Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) @@ -141,7 +143,7 @@ func ApplyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]m tenant, ok := providerConfig[auditID] if !ok { - return false, fmt.Errorf("auditlog config for region %s, provider %s is empty", auditID, providerType) + return false, ErrMissingMapping } changedExt, err := configureExtension(shoot, tenant) diff --git a/internal/auditlogging/tests/auditlogging_test.go b/internal/auditlogging/tests/auditlogging_test.go index 37e541b5..4dd18991 100644 --- a/internal/auditlogging/tests/auditlogging_test.go +++ b/internal/auditlogging/tests/auditlogging_test.go @@ -140,7 +140,7 @@ func TestApplyAuditLogConfig(t *testing.T) { // then require.False(t, annotated) - require.Equal(t, err, fmt.Errorf("auditlog config for region region, provider aws is empty")) + require.Equal(t, err, auditlogging.ErrMissingMapping) }) } diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index d37d735d..69cd32f9 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -2,9 +2,9 @@ package fsm import ( "context" - "strings" - imv1 "github.com/kyma-project/infrastructure-manager/api/v1" + "github.com/kyma-project/infrastructure-manager/internal/auditlogging" + "github.com/pkg/errors" ctrl "sigs.k8s.io/controller-runtime" ) @@ -27,7 +27,7 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, if err != nil { //nolint:nestif errorMessage := err.Error() - if strings.Contains(errorMessage, "auditlog config for region") { + if errors.Is(err, auditlogging.ErrMissingMapping) { if m.RCCfg.AuditLogMandatory { m.log.Error(err, "Failed to configure Audit Log, missing region mapping for this shoot") s.instance.UpdateStatePending( diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go index 83ed6447..925a93f2 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -2,6 +2,7 @@ package fsm import ( "context" + "github.com/kyma-project/infrastructure-manager/internal/auditlogging" "testing" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" @@ -103,14 +104,14 @@ func TestAuditLogState(t *testing.T) { Type: string(v1.ConditionTypeAuditLogConfigured), Status: "False", Reason: string(v1.ConditionReasonAuditLogMissingRegionMapping), - Message: "auditlog config for region", + Message: auditlogging.ErrMissingMapping.Error(), }, } fsm := &fsm{AuditLogging: auditLog} fsm.RCCfg.AuditLogMandatory = true - auditLog.On("Enable", ctx, shoot).Return(false, errors.New("auditlog config for region")).Once() + auditLog.On("Enable", ctx, shoot).Return(false, auditlogging.ErrMissingMapping).Once() // when stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) @@ -147,7 +148,7 @@ func TestAuditLogState(t *testing.T) { fsm := &fsm{AuditLogging: auditLog} fsm.RCCfg.AuditLogMandatory = false - auditLog.On("Enable", ctx, shoot).Return(false, errors.New("auditlog config for region")).Once() + auditLog.On("Enable", ctx, shoot).Return(false, auditlogging.ErrMissingMapping).Once() // when stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) From eea82e36c0a16ec1d85e500a12f0124bcc2b3b9c Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 9 Sep 2024 14:21:56 +0200 Subject: [PATCH 8/9] Fix linter --- .../controller/runtime/fsm/runtime_fsm_configure_auditlog.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index 69cd32f9..bd60ad0e 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -2,6 +2,7 @@ package fsm import ( "context" + imv1 "github.com/kyma-project/infrastructure-manager/api/v1" "github.com/kyma-project/infrastructure-manager/internal/auditlogging" "github.com/pkg/errors" From fd9388d377c805b1d77f255d9198d4fe9f4ac9ad Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Tue, 10 Sep 2024 09:17:32 +0200 Subject: [PATCH 9/9] Apply review sugestions --- .../runtime/fsm/runtime_fsm_configure_auditlog.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index bd60ad0e..d03cb5d6 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -30,7 +30,7 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, errorMessage := err.Error() if errors.Is(err, auditlogging.ErrMissingMapping) { if m.RCCfg.AuditLogMandatory { - m.log.Error(err, "Failed to configure Audit Log, missing region mapping for this shoot") + m.log.Error(err, "Failed to configure Audit Log, missing region mapping for this shoot", "AuditLogMandatory", m.RCCfg.AuditLogMandatory, "providerType", s.shoot.Spec.Provider.Type, "region", s.shoot.Spec.Region) s.instance.UpdateStatePending( imv1.ConditionTypeAuditLogConfigured, imv1.ConditionReasonAuditLogMissingRegionMapping, @@ -38,7 +38,7 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, errorMessage, ) } else { - m.log.Info(errorMessage, "Failed to configure Audit Log, missing region mapping for this shoot, but is not mandatory to be configured") + m.log.Info(errorMessage, "Audit Log was not configured, missing region mapping for this shoot.", "AuditLogMandatory", m.RCCfg.AuditLogMandatory, "providerType", s.shoot.Spec.Provider.Type, "region", s.shoot.Spec.Region) s.instance.UpdateStateReady( imv1.ConditionTypeAuditLogConfigured, imv1.ConditionReasonAuditLogMissingRegionMapping, @@ -46,14 +46,14 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, } } else { if m.RCCfg.AuditLogMandatory { - m.log.Error(err, "Failed to configure Audit Log") + m.log.Error(err, "Failed to configure Audit Log", "AuditLogMandatory", m.RCCfg.AuditLogMandatory) s.instance.UpdateStatePending( imv1.ConditionTypeAuditLogConfigured, imv1.ConditionReasonAuditLogError, "False", errorMessage) } else { - m.log.Info(errorMessage, "Failed to configure Audit Log, but is not mandatory to be configured") + m.log.Info(errorMessage, "AuditLogMandatory", m.RCCfg.AuditLogMandatory) s.instance.UpdateStateReady( imv1.ConditionTypeAuditLogConfigured, imv1.ConditionReasonAuditLogError,