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 51f42dc1..2f07e0a0 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -17,6 +17,7 @@ limitations under the License. package main import ( + "encoding/json" "flag" "fmt" "io" @@ -27,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" @@ -76,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.") @@ -90,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, @@ -171,10 +175,17 @@ func main() { os.Exit(1) } + err = validateAuditLogConfiguration(converterConfig.AuditLog.TenantConfigPath) + if err != nil { + 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" @@ -238,3 +249,48 @@ 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/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 5aaf150a..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) @@ -45,9 +47,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 { @@ -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 6a74d9b4..4dd18991 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) { @@ -139,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.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 d63c57e9..d03cb5d6 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -4,6 +4,8 @@ import ( "context" 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" ) @@ -24,19 +26,45 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, return updateStatusAndRequeueAfter(gardenerRequeueDuration) } - if err != nil { - m.log.Error(err, "Failed to configure Audit Log") - s.instance.UpdateStatePending( - imv1.ConditionTypeAuditLogConfigured, - imv1.ConditionReasonAuditLogError, - "False", - err.Error(), - ) + if err != nil { //nolint:nestif + 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", "AuditLogMandatory", m.RCCfg.AuditLogMandatory, "providerType", s.shoot.Spec.Provider.Type, "region", s.shoot.Spec.Region) + s.instance.UpdateStatePending( + imv1.ConditionTypeAuditLogConfigured, + imv1.ConditionReasonAuditLogMissingRegionMapping, + "False", + errorMessage, + ) + } else { + 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, + "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", "AuditLogMandatory", m.RCCfg.AuditLogMandatory) + s.instance.UpdateStatePending( + imv1.ConditionTypeAuditLogConfigured, + imv1.ConditionReasonAuditLogError, + "False", + errorMessage) + } else { + m.log.Info(errorMessage, "AuditLogMandatory", m.RCCfg.AuditLogMandatory) + s.instance.UpdateStateReady( + imv1.ConditionTypeAuditLogConfigured, + imv1.ConditionReasonAuditLogError, + "Configuration of Audit Log is not mandatory, error for context: "+errorMessage) + } + } } else { 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..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" @@ -33,10 +34,12 @@ func TestAuditLogState(t *testing.T) { }, } + fsm := &fsm{AuditLogging: auditLog} + fsm.RCCfg.AuditLogMandatory = true + auditLog.On("Enable", ctx, shoot).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 +67,90 @@ 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", }, } + fsm := &fsm{AuditLogging: auditLog} + fsm.RCCfg.AuditLogMandatory = true + auditLog.On("Enable", ctx, shoot).Return(false, nil).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 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: auditlogging.ErrMissingMapping.Error(), + }, + } + fsm := &fsm{AuditLogging: auditLog} + fsm.RCCfg.AuditLogMandatory = true + + auditLog.On("Enable", ctx, shoot).Return(false, auditlogging.ErrMissingMapping).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).Return(false, auditlogging.ErrMissingMapping).Once() + + // when stateFn, _, _ := sFnConfigureAuditLog(ctx, fsm, systemState) // set the time to its zero value for comparison purposes @@ -84,7 +163,7 @@ 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 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{} @@ -103,10 +182,12 @@ func TestAuditLogState(t *testing.T) { }, } + fsm := &fsm{AuditLogging: auditLog} + fsm.RCCfg.AuditLogMandatory = true + auditLog.On("Enable", ctx, shoot).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 @@ -118,6 +199,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).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.go b/internal/gardener/shoot/converter.go index dd1f0d3a..aa819a47 100644 --- a/internal/gardener/shoot/converter.go +++ b/internal/gardener/shoot/converter.go @@ -39,8 +39,8 @@ type KubernetesConfig struct { } type AuditLogConfig struct { - 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) @@ -51,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 { 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)