From 9e0f2ab06a38364032f2812c7dc5bfd033dca3bc Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Wed, 11 Sep 2024 09:29:06 +0200 Subject: [PATCH 1/5] Refactor in audit log state --- .../fsm/runtime_fsm_configure_auditlog.go | 84 +++++++++++-------- 1 file changed, 47 insertions(+), 37 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index d03cb5d6..12d8979d 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -12,9 +12,9 @@ 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) + shootNeedsToBeReconciled, err := m.AuditLogging.Enable(ctx, s.shoot) - if wasAuditLogEnabled { + if err == nil && shootNeedsToBeReconciled { m.log.Info("Audit Log configured for shoot: " + s.shoot.Name) s.instance.UpdateStatePending( imv1.ConditionTypeAuditLogConfigured, @@ -26,46 +26,56 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, return updateStatusAndRequeueAfter(gardenerRequeueDuration) } - 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 { + if err == nil { s.instance.UpdateStateReady( imv1.ConditionTypeAuditLogConfigured, imv1.ConditionReasonAuditLogConfigured, "Audit Log state completed successfully", ) + + return updateStatusAndStop() + } + + setStateForAuditLogError := func(reason imv1.RuntimeConditionReason, pendingMsg string, readyMsg string) { + if m.RCCfg.AuditLogMandatory { + s.instance.UpdateStatePending( + imv1.ConditionTypeAuditLogConfigured, + reason, + "False", + pendingMsg, + ) + } else { + s.instance.UpdateStateReady( + imv1.ConditionTypeAuditLogConfigured, + reason, + readyMsg) + } + } + + logError := func(err error, errorMsg, infoMsg string) { + if m.RCCfg.AuditLogMandatory { + m.log.Error(err, errorMsg) + } else { + m.log.Info(err.Error(), "Failed to configure Audit Log, but is not mandatory to be configured") + } + } + + 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) } return updateStatusAndStop() From 8825b0718aa58766666366fda6a54081f908340f Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Wed, 11 Sep 2024 10:02:28 +0200 Subject: [PATCH 2/5] Refactor in audit log state --- .../controller/runtime/fsm/runtime_fsm_configure_auditlog.go | 2 +- 1 file changed, 1 insertion(+), 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 12d8979d..4637e738 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -56,7 +56,7 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, if m.RCCfg.AuditLogMandatory { m.log.Error(err, errorMsg) } else { - m.log.Info(err.Error(), "Failed to configure Audit Log, but is not mandatory to be configured") + m.log.Info(err.Error(), infoMsg) } } From 44877bfa56359b21c311e1ebb2354a94aa2c3536 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Wed, 18 Sep 2024 16:44:01 +0200 Subject: [PATCH 3/5] Fixed problem with requeue logic --- .../fsm/runtime_fsm_configure_auditlog.go | 68 +++---------------- 1 file changed, 9 insertions(+), 59 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index 4bfbbb0a..13ab05e6 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -14,9 +14,9 @@ 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) + shootNeedsToBeReconciled, err := m.AuditLogging.Enable(ctx, s.shoot) - if wasAuditLogEnabled && err == nil { + if shootNeedsToBeReconciled && err == nil { m.log.Info("Audit Log configured for shoot: " + s.shoot.Name) s.instance.UpdateStatePending( imv1.ConditionTypeAuditLogConfigured, @@ -39,52 +39,6 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, } 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) { @@ -104,11 +58,11 @@ func handleError(err error, m *fsm, s *systemState) (stateFn, *ctrl.Result, erro } } - logError := func(err error, errorMsg, infoMsg string, keysAndValues ...any) { + logError := func(err error, keysAndValues ...any) { if m.RCCfg.AuditLogMandatory { - m.log.Error(err, errorMsg, keysAndValues) + m.log.Error(nil, err.Error(), keysAndValues) } else { - m.log.Info(err.Error(), infoMsg) + m.log.Info(err.Error(), keysAndValues) } } @@ -131,20 +85,16 @@ func handleError(err error, m *fsm, s *systemState) (stateFn, *ctrl.Result, erro 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" - infoMsg := "Failed to configure Audit Log" - logError(err, errorMsg, infoMsg, "AuditLogMandatory", auditLogMandatoryString, "providerType", s.shoot.Spec.Provider.Type, "region", s.shoot.Spec.Region) + logError(err, "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) + logError(err, "AuditLogMandatory", auditLogMandatoryString) - return updateStatusAndRequeue() + return updateStatusAndStop() } From b05851ebf2c5e60b5c9ce445158994aef5aeacc5 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Wed, 18 Sep 2024 16:56:45 +0200 Subject: [PATCH 4/5] Linter --- .../controller/runtime/fsm/runtime_fsm_configure_auditlog.go | 4 ++-- 1 file changed, 2 insertions(+), 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 13ab05e6..81ccd276 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -60,9 +60,9 @@ func handleError(err error, m *fsm, s *systemState) (stateFn, *ctrl.Result, erro logError := func(err error, keysAndValues ...any) { if m.RCCfg.AuditLogMandatory { - m.log.Error(nil, err.Error(), keysAndValues) + m.log.Error(nil, err.Error(), keysAndValues...) } else { - m.log.Info(err.Error(), keysAndValues) + m.log.Info(err.Error(), keysAndValues...) } } From 26f8316230c47eba699c6eed9d1b593bb1176aa8 Mon Sep 17 00:00:00 2001 From: Wojciech Nawa Date: Thu, 19 Sep 2024 13:40:41 +0200 Subject: [PATCH 5/5] Add default machine image name (#382) * Add default image name * Run gci --------- Co-authored-by: Arkadiusz Galwas --- hack/shoot-comparator/cmd/comparator/directories.go | 5 +++-- hack/shoot-comparator/cmd/comparator/files.go | 1 + .../internal/directories/comparator.go | 3 ++- .../internal/directories/comparator_test.go | 6 +++--- hack/shoot-comparator/internal/files/comparator.go | 3 ++- hack/shoot-comparator/pkg/shoot/extensionmatcher.go | 7 ++++--- .../pkg/shoot/extensionmatcher_test.go | 1 + internal/auditlogging/mocks/AuditLogConfigurator.go | 5 +---- internal/controller/runtime/suite_test.go | 6 +++--- .../runtime/test_client_obj_tracker_test.go | 3 ++- internal/gardener/mocks/ShootClient.go | 7 ++----- internal/gardener/shoot/converter.go | 3 ++- internal/gardener/shoot/converter_test.go | 2 ++ internal/gardener/shoot/extender/oidc_test.go | 2 +- internal/gardener/shoot/extender/provider.go | 11 ++++++++--- internal/gardener/shoot/extender/provider_test.go | 11 +++++++---- 16 files changed, 44 insertions(+), 32 deletions(-) diff --git a/hack/shoot-comparator/cmd/comparator/directories.go b/hack/shoot-comparator/cmd/comparator/directories.go index ec4136de..41e9a138 100644 --- a/hack/shoot-comparator/cmd/comparator/directories.go +++ b/hack/shoot-comparator/cmd/comparator/directories.go @@ -2,10 +2,11 @@ package comparator import ( "fmt" - "github.com/kyma-project/infrastructure-manager/tools/shoot-comparator/internal/directories" - "github.com/spf13/cobra" "log/slog" "time" + + "github.com/kyma-project/infrastructure-manager/tools/shoot-comparator/internal/directories" + "github.com/spf13/cobra" ) func init() { diff --git a/hack/shoot-comparator/cmd/comparator/files.go b/hack/shoot-comparator/cmd/comparator/files.go index 4936af9b..4884dfee 100644 --- a/hack/shoot-comparator/cmd/comparator/files.go +++ b/hack/shoot-comparator/cmd/comparator/files.go @@ -2,6 +2,7 @@ package comparator import ( "fmt" + "github.com/kyma-project/infrastructure-manager/tools/shoot-comparator/internal/files" "github.com/spf13/cobra" ) diff --git a/hack/shoot-comparator/internal/directories/comparator.go b/hack/shoot-comparator/internal/directories/comparator.go index 1ef0a5f3..8af19105 100644 --- a/hack/shoot-comparator/internal/directories/comparator.go +++ b/hack/shoot-comparator/internal/directories/comparator.go @@ -1,11 +1,12 @@ package directories import ( - "github.com/kyma-project/infrastructure-manager/tools/shoot-comparator/internal/files" "os" "path" "slices" "time" + + "github.com/kyma-project/infrastructure-manager/tools/shoot-comparator/internal/files" ) type Result struct { diff --git a/hack/shoot-comparator/internal/directories/comparator_test.go b/hack/shoot-comparator/internal/directories/comparator_test.go index fcdf181b..b44828bf 100644 --- a/hack/shoot-comparator/internal/directories/comparator_test.go +++ b/hack/shoot-comparator/internal/directories/comparator_test.go @@ -1,16 +1,16 @@ package directories import ( - "github.com/gardener/gardener/pkg/apis/core/v1beta1" - "gopkg.in/yaml.v3" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "os" "path" "testing" "time" + "github.com/gardener/gardener/pkg/apis/core/v1beta1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const onlyLeftFilename = "onlyLeftFile.yaml" diff --git a/hack/shoot-comparator/internal/files/comparator.go b/hack/shoot-comparator/internal/files/comparator.go index 0c1acafe..7cb3ba51 100644 --- a/hack/shoot-comparator/internal/files/comparator.go +++ b/hack/shoot-comparator/internal/files/comparator.go @@ -1,9 +1,10 @@ package files import ( + "os" + "github.com/gardener/gardener/pkg/apis/core/v1beta1" "github.com/kyma-project/infrastructure-manager/tools/shoot-comparator/pkg/shoot" - "os" "sigs.k8s.io/yaml" ) diff --git a/hack/shoot-comparator/pkg/shoot/extensionmatcher.go b/hack/shoot-comparator/pkg/shoot/extensionmatcher.go index fa7a1179..c71f995d 100644 --- a/hack/shoot-comparator/pkg/shoot/extensionmatcher.go +++ b/hack/shoot-comparator/pkg/shoot/extensionmatcher.go @@ -2,12 +2,13 @@ package shoot import ( "fmt" - "github.com/gardener/gardener/pkg/apis/core/v1beta1" - "github.com/onsi/gomega" - "github.com/onsi/gomega/types" "reflect" "sort" "strings" + + "github.com/gardener/gardener/pkg/apis/core/v1beta1" + "github.com/onsi/gomega" + "github.com/onsi/gomega/types" ) type ExtensionMatcher struct { diff --git a/hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go b/hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go index ed8ffb36..652ec483 100644 --- a/hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go +++ b/hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go @@ -2,6 +2,7 @@ package shoot import ( "fmt" + "github.com/gardener/gardener/pkg/apis/core/v1beta1" . "github.com/onsi/ginkgo/v2" //nolint:revive . "github.com/onsi/gomega" //nolint:revive diff --git a/internal/auditlogging/mocks/AuditLogConfigurator.go b/internal/auditlogging/mocks/AuditLogConfigurator.go index 92016d0a..3977f019 100644 --- a/internal/auditlogging/mocks/AuditLogConfigurator.go +++ b/internal/auditlogging/mocks/AuditLogConfigurator.go @@ -5,13 +5,10 @@ package mocks import ( context "context" + v1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" auditlogging "github.com/kyma-project/infrastructure-manager/internal/auditlogging" - mock "github.com/stretchr/testify/mock" - types "k8s.io/apimachinery/pkg/types" - - v1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" ) // AuditLogConfigurator is an autogenerated mock type for the AuditLogConfigurator type diff --git a/internal/controller/runtime/suite_test.go b/internal/controller/runtime/suite_test.go index 22c538b2..f29815dd 100644 --- a/internal/controller/runtime/suite_test.go +++ b/internal/controller/runtime/suite_test.go @@ -19,19 +19,19 @@ package runtime import ( "context" "encoding/json" - "github.com/kyma-project/infrastructure-manager/internal/auditlogging" - v1 "k8s.io/api/autoscaling/v1" - v12 "k8s.io/api/core/v1" "path/filepath" "testing" "time" gardener_api "github.com/gardener/gardener/pkg/apis/core/v1beta1" infrastructuremanagerv1 "github.com/kyma-project/infrastructure-manager/api/v1" + "github.com/kyma-project/infrastructure-manager/internal/auditlogging" "github.com/kyma-project/infrastructure-manager/internal/controller/runtime/fsm" gardener_shoot "github.com/kyma-project/infrastructure-manager/internal/gardener/shoot" . "github.com/onsi/ginkgo/v2" //nolint:revive . "github.com/onsi/gomega" //nolint:revive + v1 "k8s.io/api/autoscaling/v1" + v12 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" //nolint:revive diff --git a/internal/controller/runtime/test_client_obj_tracker_test.go b/internal/controller/runtime/test_client_obj_tracker_test.go index 2604945a..5fe1388f 100644 --- a/internal/controller/runtime/test_client_obj_tracker_test.go +++ b/internal/controller/runtime/test_client_obj_tracker_test.go @@ -1,11 +1,12 @@ package runtime import ( + "testing" + gardener_api "github.com/gardener/gardener/pkg/apis/core/v1beta1" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "testing" ) func TestCustomTracker_Get(t *testing.T) { diff --git a/internal/gardener/mocks/ShootClient.go b/internal/gardener/mocks/ShootClient.go index ac582f90..4c85276e 100644 --- a/internal/gardener/mocks/ShootClient.go +++ b/internal/gardener/mocks/ShootClient.go @@ -5,13 +5,10 @@ package mocks import ( context "context" + v1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" mock "github.com/stretchr/testify/mock" - - types "k8s.io/apimachinery/pkg/types" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - v1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + types "k8s.io/apimachinery/pkg/types" ) // ShootClient is an autogenerated mock type for the ShootClient type diff --git a/internal/gardener/shoot/converter.go b/internal/gardener/shoot/converter.go index 4e6a891b..dcb81089 100644 --- a/internal/gardener/shoot/converter.go +++ b/internal/gardener/shoot/converter.go @@ -67,6 +67,7 @@ type GardenerConfig struct { } type MachineImageConfig struct { + DefaultName string `json:"defaultName" validate:"required"` DefaultVersion string `json:"defaultVersion" validate:"required"` } @@ -75,7 +76,7 @@ func NewConverter(config ConverterConfig) Converter { extender.ExtendWithAnnotations, extender.ExtendWithLabels, extender.NewKubernetesExtender(config.Kubernetes.DefaultVersion), - extender.NewProviderExtender(config.Provider.AWS.EnableIMDSv2, config.MachineImage.DefaultVersion), + extender.NewProviderExtender(config.Provider.AWS.EnableIMDSv2, config.MachineImage.DefaultName, config.MachineImage.DefaultVersion), extender.NewDNSExtender(config.DNS.SecretName, config.DNS.DomainPrefix, config.DNS.ProviderType), extender.ExtendWithOIDC, extender.ExtendWithCloudProfile, diff --git a/internal/gardener/shoot/converter_test.go b/internal/gardener/shoot/converter_test.go index 51c1d4d3..67389c12 100644 --- a/internal/gardener/shoot/converter_test.go +++ b/internal/gardener/shoot/converter_test.go @@ -151,6 +151,7 @@ var testReader io.Reader = strings.NewReader(`{ } }, "machineImage": { + "defaultName": "test-image-name", "defaultVersion": "0.1.2.3.4" }, "gardener": { @@ -188,6 +189,7 @@ func Test_ConverterConfig_Load_OK(t *testing.T) { }, }, MachineImage: MachineImageConfig{ + DefaultName: "test-image-name", DefaultVersion: "0.1.2.3.4", }, Gardener: GardenerConfig{ diff --git a/internal/gardener/shoot/extender/oidc_test.go b/internal/gardener/shoot/extender/oidc_test.go index 3fc0e683..88082cdf 100644 --- a/internal/gardener/shoot/extender/oidc_test.go +++ b/internal/gardener/shoot/extender/oidc_test.go @@ -1,13 +1,13 @@ package extender import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "testing" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestOidcExtender(t *testing.T) { diff --git a/internal/gardener/shoot/extender/provider.go b/internal/gardener/shoot/extender/provider.go index 5796b355..3f23a489 100644 --- a/internal/gardener/shoot/extender/provider.go +++ b/internal/gardener/shoot/extender/provider.go @@ -14,7 +14,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) -func NewProviderExtender(enableIMDSv2 bool, defaultMachineImageVersion string) func(runtime imv1.Runtime, shoot *gardener.Shoot) error { +func NewProviderExtender(enableIMDSv2 bool, defaultMachineImageName, defaultMachineImageVersion string) func(runtime imv1.Runtime, shoot *gardener.Shoot) error { return func(runtime imv1.Runtime, shoot *gardener.Shoot) error { provider := &shoot.Spec.Provider provider.Type = runtime.Spec.Shoot.Provider.Type @@ -26,7 +26,7 @@ func NewProviderExtender(enableIMDSv2 bool, defaultMachineImageVersion string) f return err } - setDefaultMachineImageVersion(provider, defaultMachineImageVersion) + setDefaultMachineImage(provider, defaultMachineImageName, defaultMachineImageVersion) err = setWorkerConfig(provider, provider.Type, enableIMDSv2) setWorkerSettings(provider) @@ -122,12 +122,13 @@ func setWorkerSettings(provider *gardener.Provider) { } } -func setDefaultMachineImageVersion(provider *gardener.Provider, defaultMachineImageVersion string) { +func setDefaultMachineImage(provider *gardener.Provider, defaultMachineImageName, defaultMachineImageVersion string) { for i := 0; i < len(provider.Workers); i++ { worker := &provider.Workers[i] if worker.Machine.Image == nil { worker.Machine.Image = &gardener.ShootMachineImage{ + Name: defaultMachineImageName, Version: &defaultMachineImageVersion, } @@ -138,6 +139,10 @@ func setDefaultMachineImageVersion(provider *gardener.Provider, defaultMachineIm machineImageVersion = &defaultMachineImageVersion } + if worker.Machine.Image.Name == "" { + worker.Machine.Image.Name = defaultMachineImageName + } + worker.Machine.Image.Version = machineImageVersion } } diff --git a/internal/gardener/shoot/extender/provider_test.go b/internal/gardener/shoot/extender/provider_test.go index 999790f8..1d24ff3f 100644 --- a/internal/gardener/shoot/extender/provider_test.go +++ b/internal/gardener/shoot/extender/provider_test.go @@ -18,6 +18,8 @@ func TestProviderExtender(t *testing.T) { EnableIMDSv2 bool DefaultMachineImageVersion string ExpectedMachineImageVersion string + DefaultMachineImageName string + ExpectedMachineImageName string ExpectedZonesCount int }{ "Create provider specific config for AWS without worker config": { @@ -65,13 +67,13 @@ func TestProviderExtender(t *testing.T) { shoot := fixEmptyGardenerShoot("cluster", "kcp-system") // when - extender := NewProviderExtender(testCase.EnableIMDSv2, testCase.DefaultMachineImageVersion) + extender := NewProviderExtender(testCase.EnableIMDSv2, testCase.DefaultMachineImageName, testCase.DefaultMachineImageVersion) err := extender(testCase.Runtime, &shoot) // then require.NoError(t, err) - assertProvider(t, testCase.Runtime.Spec.Shoot, shoot, testCase.EnableIMDSv2, testCase.ExpectedMachineImageVersion) + assertProvider(t, testCase.Runtime.Spec.Shoot, shoot, testCase.EnableIMDSv2, testCase.ExpectedMachineImageName, testCase.ExpectedMachineImageVersion) assertProviderSpecificConfig(t, shoot, testCase.ExpectedZonesCount) }) } @@ -90,7 +92,7 @@ func TestProviderExtender(t *testing.T) { } // when - extender := NewProviderExtender(false, "") + extender := NewProviderExtender(false, "", "") err := extender(runtime, &shoot) // then @@ -175,7 +177,7 @@ func fixAWSProviderWithMultipleWorkers() imv1.Provider { } } -func assertProvider(t *testing.T, runtimeShoot imv1.RuntimeShoot, shoot gardener.Shoot, expectWorkerConfig bool, expectedMachineImageVersion string) { +func assertProvider(t *testing.T, runtimeShoot imv1.RuntimeShoot, shoot gardener.Shoot, expectWorkerConfig bool, expectedMachineImageName, expectedMachineImageVersion string) { assert.Equal(t, runtimeShoot.Provider.Type, shoot.Spec.Provider.Type) assert.Equal(t, runtimeShoot.Provider.Workers, shoot.Spec.Provider.Workers) assert.Equal(t, false, shoot.Spec.Provider.WorkersSettings.SSHAccess.Enabled) @@ -192,6 +194,7 @@ func assertProvider(t *testing.T, runtimeShoot imv1.RuntimeShoot, shoot gardener assert.Empty(t, worker.ProviderConfig) } assert.Equal(t, expectedMachineImageVersion, *worker.Machine.Image.Version) + assert.Equal(t, expectedMachineImageName, worker.Machine.Image.Name) } }