From d46d22f49f77c208f4a2246d8b143ff630d47f7e Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 11 Sep 2024 13:03:31 +0200 Subject: [PATCH 1/5] Handle the Kubernetes conflict error in KIM Audit Log --- internal/auditlogging/auditlogging.go | 7 +++- .../fsm/runtime_fsm_configure_auditlog.go | 13 ++++++- .../runtime_fsm_configure_auditlog_test.go | 38 +++++++++++++++++++ ...runtime_fsm_waiting_for_shoot_reconcile.go | 2 +- 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index c4b47aec..feac599d 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -147,9 +147,14 @@ func ApplyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]m } changedExt, err := configureExtension(shoot, tenant) + + if err != nil { + return false, err + } + changedSec := configureSecret(shoot, tenant) - return changedExt || changedSec, err + return changedExt || changedSec, nil } func configureExtension(shoot *gardener.Shoot, config AuditLogData) (changed bool, err error) { diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index d03cb5d6..04f3a43c 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" + k8serrors "k8s.io/apimachinery/pkg/api/errors" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" "github.com/kyma-project/infrastructure-manager/internal/auditlogging" @@ -14,7 +15,7 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, wasAuditLogEnabled, err := m.AuditLogging.Enable(ctx, s.shoot) - if wasAuditLogEnabled { + if wasAuditLogEnabled && err == nil { m.log.Info("Audit Log configured for shoot: " + s.shoot.Name) s.instance.UpdateStatePending( imv1.ConditionTypeAuditLogConfigured, @@ -27,6 +28,16 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, } if err != nil { //nolint:nestif + if k8serrors.IsConflict(err) { + m.log.Error(err, "Conflict while updating Shoot object after applying Audit Log configuration, retrying") + s.instance.UpdateStatePending( + imv1.ConditionTypeAuditLogConfigured, + imv1.ConditionReasonAuditLogError, + "True", + err.Error(), + ) + return updateStatusAndRequeue() + } errorMessage := err.Error() if errors.Is(err, auditlogging.ErrMissingMapping) { 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 925a93f2..b8bad745 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -3,6 +3,7 @@ package fsm import ( "context" "github.com/kyma-project/infrastructure-manager/internal/auditlogging" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "testing" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" @@ -236,6 +237,43 @@ func TestAuditLogState(t *testing.T) { assert.Equal(t, v1.RuntimeStateReady, string(systemState.instance.Status.State)) assert.Equal(t, expectedRuntimeConditions, systemState.instance.Status.Conditions) }) + + t.Run("Should requeue in case of Kubernetes object conflict error during update of Shoot object and set status on Runtime CR", 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: k8serrors.NewConflict(gardener.Resource("shoots"), shoot.Name, errors.New("k8s conflict on update error")).Error(), + }, + } + + fsm := &fsm{AuditLogging: auditLog} + fsm.RCCfg.AuditLogMandatory = true + + auditLog.On("Enable", ctx, shoot).Return(false, k8serrors.NewConflict(gardener.Resource("shoots"), shoot.Name, errors.New("k8s conflict on update error"))).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.RuntimeStatePending, string(systemState.instance.Status.State)) + assert.Equal(t, expectedRuntimeConditions, systemState.instance.Status.Conditions) + }) } func shootForTest() *gardener.Shoot { diff --git a/internal/controller/runtime/fsm/runtime_fsm_waiting_for_shoot_reconcile.go b/internal/controller/runtime/fsm/runtime_fsm_waiting_for_shoot_reconcile.go index ce700502..a4a8fb5f 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_waiting_for_shoot_reconcile.go +++ b/internal/controller/runtime/fsm/runtime_fsm_waiting_for_shoot_reconcile.go @@ -49,7 +49,7 @@ func sFnWaitForShootReconcile(_ context.Context, m *fsm, s *systemState) (stateF imv1.ConditionTypeRuntimeProvisioned, imv1.ConditionReasonAuditLogConfigured, "Runtime processing completed successfully", - sFnConfigureAuditLog) + sFnApplyClusterRoleBindings) } m.log.Info("Update did not processed, exiting with no retry") From bbd4ddbb4d69112d6e676680518331434ba67522 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 11 Sep 2024 13:10:12 +0200 Subject: [PATCH 2/5] Fix linter --- .../controller/runtime/fsm/runtime_fsm_configure_auditlog.go | 1 + .../runtime/fsm/runtime_fsm_configure_auditlog_test.go | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index 04f3a43c..368a9dd0 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" + k8serrors "k8s.io/apimachinery/pkg/api/errors" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" 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 b8bad745..7fb57e56 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -2,9 +2,10 @@ package fsm import ( "context" + "testing" + "github.com/kyma-project/infrastructure-manager/internal/auditlogging" k8serrors "k8s.io/apimachinery/pkg/api/errors" - "testing" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" v1 "github.com/kyma-project/infrastructure-manager/api/v1" From 1229e7d5b05034cfcca40c171c4131bfd7f2fc66 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 11 Sep 2024 13:20:19 +0200 Subject: [PATCH 3/5] Fix gci --- .../controller/runtime/fsm/runtime_fsm_configure_auditlog.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index 368a9dd0..1491f549 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -3,11 +3,10 @@ package fsm import ( "context" - k8serrors "k8s.io/apimachinery/pkg/api/errors" - imv1 "github.com/kyma-project/infrastructure-manager/api/v1" "github.com/kyma-project/infrastructure-manager/internal/auditlogging" "github.com/pkg/errors" + k8serrors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" ) From fa8284dc633c590950d8857527817d6c98f5a5cb Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 11 Sep 2024 13:20:38 +0200 Subject: [PATCH 4/5] Fix gci in tests --- .../runtime/fsm/runtime_fsm_configure_auditlog_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 7fb57e56..cf48b935 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -4,15 +4,14 @@ import ( "context" "testing" - "github.com/kyma-project/infrastructure-manager/internal/auditlogging" - k8serrors "k8s.io/apimachinery/pkg/api/errors" - gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" v1 "github.com/kyma-project/infrastructure-manager/api/v1" + "github.com/kyma-project/infrastructure-manager/internal/auditlogging" "github.com/kyma-project/infrastructure-manager/internal/auditlogging/mocks" "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) From e0e245be591feaf2e9aadfee40bb2f3229d7801c Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Fri, 13 Sep 2024 15:27:37 +0200 Subject: [PATCH 5/5] Apply review sugestions --- .../runtime/fsm/runtime_fsm_waiting_for_shoot_reconcile.go | 2 +- internal/controller/runtime/runtime_controller_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_waiting_for_shoot_reconcile.go b/internal/controller/runtime/fsm/runtime_fsm_waiting_for_shoot_reconcile.go index a4a8fb5f..b70e09a2 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_waiting_for_shoot_reconcile.go +++ b/internal/controller/runtime/fsm/runtime_fsm_waiting_for_shoot_reconcile.go @@ -47,7 +47,7 @@ func sFnWaitForShootReconcile(_ context.Context, m *fsm, s *systemState) (stateF return ensureStatusConditionIsSetAndContinue( &s.instance, imv1.ConditionTypeRuntimeProvisioned, - imv1.ConditionReasonAuditLogConfigured, + imv1.ConditionReasonConfigurationCompleted, "Runtime processing completed successfully", sFnApplyClusterRoleBindings) } diff --git a/internal/controller/runtime/runtime_controller_test.go b/internal/controller/runtime/runtime_controller_test.go index ed6fe337..8811ef74 100644 --- a/internal/controller/runtime/runtime_controller_test.go +++ b/internal/controller/runtime/runtime_controller_test.go @@ -157,7 +157,7 @@ var _ = Describe("Runtime Controller", func() { return false } - if !runtime.IsConditionSet(imv1.ConditionTypeRuntimeProvisioned, imv1.ConditionReasonAuditLogConfigured) { + if !runtime.IsConditionSet(imv1.ConditionTypeRuntimeProvisioned, imv1.ConditionReasonConfigurationCompleted) { return false }