Branch rebased

hack/runtime-migrator/main.go

@@ -206,15 +206,14 @@ func createRuntime(ctx context.Context, shoot v1beta1.Shoot, cfg migrator.Config
 				},
 				Provider: v1.Provider{
 					Type:    shoot.Spec.Provider.Type,
-					Workers: shoot.Spec.Provider.Workers,
+					Workers: adjustWorkersConfig(shoot.Spec.Provider.Workers),
 				},
 				Networking: v1.Networking{
 					Type:     shoot.Spec.Networking.Type,
 					Pods:     *shoot.Spec.Networking.Pods,
 					Nodes:    *shoot.Spec.Networking.Nodes,
 					Services: *shoot.Spec.Networking.Services,
 				},
-				ControlPlane: getControlPlane(shoot),
 			},
 			Security: v1.Security{
 				Administrators: subjects,
@@ -235,9 +234,28 @@ func createRuntime(ctx context.Context, shoot v1beta1.Shoot, cfg migrator.Config
 			Conditions: nil, // deliberately left nil by our migrator to show that controller has not picked it yet
 		},
 	}
+
+	controlPlane := getControlPlane(shoot)
+	if controlPlane != nil {
+		runtime.Spec.Shoot.ControlPlane = controlPlane
+	}
+
 	return runtime, nil
 }
 
+// The goal of this function is to make the migrator output equal to the shoot created by the converter
+// As a result we can automatically verify the correctness of the migrator output
+func adjustWorkersConfig(workers []v1beta1.Worker) []v1beta1.Worker {
+	// We need to set the following fields to nil, as they are not set by the KIM converter
+	for i := 0; i < len(workers); i++ {
+		workers[i].Machine.Architecture = nil
+		workers[i].SystemComponents = nil
+		workers[i].CRI = nil
+	}
+
+	return workers
+}
+
 func getOidcConfig(shoot v1beta1.Shoot) v1beta1.OIDCConfig {
 	var oidcConfig = v1beta1.OIDCConfig{
 		CABundle:             nil, // deliberately left empty

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) {

internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go

@@ -2,19 +2,21 @@ package fsm
 
 import (
 	"context"
+	"strconv"
 
 	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"
 )
 
 func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, *ctrl.Result, error) {
 	m.log.Info("Configure Audit Log state")
 
-	shootNeedsToBeReconciled, err := m.AuditLogging.Enable(ctx, s.shoot)
+	wasAuditLogEnabled, err := m.AuditLogging.Enable(ctx, s.shoot)
 
-	if err == nil && shootNeedsToBeReconciled {
+	if wasAuditLogEnabled && err == nil {
 		m.log.Info("Audit Log configured for shoot: " + s.shoot.Name)
 		s.instance.UpdateStatePending(
 			imv1.ConditionTypeAuditLogConfigured,
@@ -36,6 +38,56 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn,
 		return updateStatusAndStop()
 	}
 
+	return handleError(err, m, s)
+	//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 {
+	//			m.log.Error(err, "AuditLogMandatory", auditLogMandatoryString, "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, "AuditLogMandatory", auditLogMandatoryString, "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, "AuditLogMandatory", auditLogMandatoryString)
+	//			s.instance.UpdateStatePending(
+	//				imv1.ConditionTypeAuditLogConfigured,
+	//				imv1.ConditionReasonAuditLogError,
+	//				"False",
+	//				errorMessage)
+	//		} else {
+	//			m.log.Info(errorMessage, "AuditLogMandatory", auditLogMandatoryString)
+	//			s.instance.UpdateStateReady(
+	//				imv1.ConditionTypeAuditLogConfigured,
+	//				imv1.ConditionReasonAuditLogError,
+	//				"Configuration of Audit Log is not mandatory, error for context: "+errorMessage)
+	//		}
+	//	}
+	//}
+}
+
+func handleError(err error, m *fsm, s *systemState) (stateFn, *ctrl.Result, error) {
 	setStateForAuditLogError := func(reason imv1.RuntimeConditionReason, pendingMsg string, readyMsg string) {
 		if m.RCCfg.AuditLogMandatory {
 			s.instance.UpdateStatePending(
@@ -52,31 +104,47 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn,
 		}
 	}
 
-	logError := func(err error, errorMsg, infoMsg string) {
+	logError := func(err error, errorMsg, infoMsg string, keysAndValues ...any) {
 		if m.RCCfg.AuditLogMandatory {
-			m.log.Error(err, errorMsg)
+			m.log.Error(err, errorMsg, keysAndValues)
 		} else {
 			m.log.Info(err.Error(), infoMsg)
 		}
 	}
 
+	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()
+	}
+
+	auditLogMandatoryString := strconv.FormatBool(m.RCCfg.AuditLogMandatory)
+
 	if errors.Is(err, auditlogging.ErrMissingMapping) {
 		pendingStatusMsg := err.Error()
 		readyStatusMsg := "Missing region mapping for this shoot. Audit Log is not mandatory. Skipping configuration"
 		setStateForAuditLogError(imv1.ConditionReasonAuditLogMissingRegionMapping, pendingStatusMsg, readyStatusMsg)
 
-		errorMsg := "Failed to configure Audit Log, missing region mapping for this shoot"
-		infoMsg := "Failed to configure Audit Log, missing region mapping for this shoot, but is not mandatory to be configured"
-		logError(err, errorMsg, infoMsg)
-	} else {
-		pendingStatusMsg := err.Error()
-		readyStatusMsg := "Configuration of Audit Log is not mandatory, error for context: " + err.Error()
-		setStateForAuditLogError(imv1.ConditionReasonAuditLogError, pendingStatusMsg, readyStatusMsg)
-
 		errorMsg := "Failed to configure Audit Log"
-		infoMsg := "Failed to configure Audit Log, but is not mandatory to be configured"
-		logError(err, errorMsg, infoMsg)
+		infoMsg := "Failed to configure Audit Log"
+		logError(err, errorMsg, infoMsg, "AuditLogMandatory", auditLogMandatoryString, "providerType", s.shoot.Spec.Provider.Type, "region", s.shoot.Spec.Region)
+
+		return updateStatusAndRequeue()
 	}
 
-	return updateStatusAndStop()
+	pendingStatusMsg := err.Error()
+	readyStatusMsg := "Configuration of Audit Log is not mandatory, error for context: " + err.Error()
+	setStateForAuditLogError(imv1.ConditionReasonAuditLogError, pendingStatusMsg, readyStatusMsg)
+
+	errorMsg := "Failed to configure Audit Log"
+	infoMsg := "Failed to configure Audit Log"
+	logError(err, errorMsg, infoMsg, "AuditLogMandatory", auditLogMandatoryString)
+
+	return updateStatusAndRequeue()
 }

internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go

@@ -2,15 +2,16 @@ package fsm
 
 import (
 	"context"
-	"github.com/kyma-project/infrastructure-manager/internal/auditlogging"
 	"testing"
 
 	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"
 )
 
@@ -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 {

internal/controller/runtime/fsm/runtime_fsm_waiting_for_shoot_reconcile.go

@@ -47,9 +47,9 @@ func sFnWaitForShootReconcile(_ context.Context, m *fsm, s *systemState) (stateF
 		return ensureStatusConditionIsSetAndContinue(
 			&s.instance,
 			imv1.ConditionTypeRuntimeProvisioned,
-			imv1.ConditionReasonAuditLogConfigured,
+			imv1.ConditionReasonConfigurationCompleted,
 			"Runtime processing completed successfully",
-			sFnConfigureAuditLog)
+			sFnApplyClusterRoleBindings)
 	}
 
 	m.log.Info("Update did not processed, exiting with no retry")

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
 				}
 

internal/gardener/shoot/converter.go

@@ -74,7 +74,7 @@ func NewConverter(config ConverterConfig) Converter {
 	extenders := []Extend{
 		extender.ExtendWithAnnotations,
 		extender.ExtendWithLabels,
-		extender.NewKubernetesVersionExtender(config.Kubernetes.DefaultVersion),
+		extender.NewKubernetesExtender(config.Kubernetes.DefaultVersion),
 		extender.NewProviderExtender(config.Provider.AWS.EnableIMDSv2, config.MachineImage.DefaultVersion),
 		extender.NewDNSExtender(config.DNS.SecretName, config.DNS.DomainPrefix, config.DNS.ProviderType),
 		extender.ExtendWithOIDC,

internal/gardener/shoot/extender/dns_test.go

@@ -58,6 +58,7 @@ func fixEmptyGardenerShoot(name, namespace string) gardener.Shoot {
 		ObjectMeta: v1.ObjectMeta{
 			Name:      name,
 			Namespace: namespace,
+			Labels:    map[string]string{},
 		},
 		Spec: gardener.ShootSpec{},
 	}

internal/gardener/shoot/extender/kubernetes.go

@@ -0,0 +1,25 @@
+package extender
+
+import (
+	gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1"
+	imv1 "github.com/kyma-project/infrastructure-manager/api/v1"
+	"k8s.io/utils/ptr"
+)
+
+// NewKubernetesExtender creates a new Kubernetes extender function.
+// It sets the Kubernetes version of the Shoot to the version specified in the Runtime.
+// If the version is not specified in the Runtime, it sets the version to the `defaultKubernetesVersion`, set in `converter_config.json`.
+// It sets the EnableStaticTokenKubeconfig field of the Shoot to false.
+func NewKubernetesExtender(defaultKubernetesVersion string) func(runtime imv1.Runtime, shoot *gardener.Shoot) error {
+	return func(runtime imv1.Runtime, shoot *gardener.Shoot) error {
+		kubernetesVersion := runtime.Spec.Shoot.Kubernetes.Version
+		if kubernetesVersion == nil || *kubernetesVersion == "" {
+			kubernetesVersion = &defaultKubernetesVersion
+		}
+
+		shoot.Spec.Kubernetes.Version = *kubernetesVersion
+		shoot.Spec.Kubernetes.EnableStaticTokenKubeconfig = ptr.To(false)
+
+		return nil
+	}
+}

internal/gardener/shoot/extender/kubernetes_version_test.go → internal/gardener/shoot/extender/kubernetes_test.go

@@ -16,14 +16,28 @@ func TestKubernetesVersionExtender(t *testing.T) {
 		runtime := imv1.Runtime{}
 
 		// when
-		kubernetesVersionExtender := NewKubernetesVersionExtender("1.99")
+		kubernetesVersionExtender := NewKubernetesExtender("1.99")
 		err := kubernetesVersionExtender(runtime, &shoot)
 
 		// then
 		require.NoError(t, err)
 		assert.Equal(t, "1.99", shoot.Spec.Kubernetes.Version)
 	})
 
+	t.Run("Disable static token kubeconfig", func(t *testing.T) {
+		// given
+		shoot := fixEmptyGardenerShoot("test", "kcp-system")
+		runtime := imv1.Runtime{}
+
+		// when
+		kubernetesVersionExtender := NewKubernetesExtender("1.99")
+		err := kubernetesVersionExtender(runtime, &shoot)
+
+		// then
+		require.NoError(t, err)
+		assert.Equal(t, false, *shoot.Spec.Kubernetes.EnableStaticTokenKubeconfig)
+	})
+
 	t.Run("Use version provided in the Runtime CR", func(t *testing.T) {
 		// given
 		shoot := fixEmptyGardenerShoot("test", "kcp-system")
@@ -38,7 +52,7 @@ func TestKubernetesVersionExtender(t *testing.T) {
 		}
 
 		// when
-		kubernetesVersionExtender := NewKubernetesVersionExtender("1.99")
+		kubernetesVersionExtender := NewKubernetesExtender("1.99")
 		err := kubernetesVersionExtender(runtime, &shoot)
 
 		// then