From cdd127f2e916dad83461ae9a477050521e894848 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Tue, 10 Sep 2024 12:00:50 +0200 Subject: [PATCH 1/5] oidc extension added only if controlled-by-provisioner value is disabled --- internal/gardener/shoot/extender/dns_test.go | 3 +++ internal/gardener/shoot/extender/oidc.go | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/internal/gardener/shoot/extender/dns_test.go b/internal/gardener/shoot/extender/dns_test.go index 802c52be..7d90ce05 100644 --- a/internal/gardener/shoot/extender/dns_test.go +++ b/internal/gardener/shoot/extender/dns_test.go @@ -58,6 +58,9 @@ func fixEmptyGardenerShoot(name, namespace string) gardener.Shoot { ObjectMeta: v1.ObjectMeta{ Name: name, Namespace: namespace, + Labels: map[string]string{ + "kyma-project.io/controlled-by-provisioner": "false", + }, }, Spec: gardener.ShootSpec{}, } diff --git a/internal/gardener/shoot/extender/oidc.go b/internal/gardener/shoot/extender/oidc.go index 8e8da762..90e7e5e8 100644 --- a/internal/gardener/shoot/extender/oidc.go +++ b/internal/gardener/shoot/extender/oidc.go @@ -13,12 +13,25 @@ const ( func ExtendWithOIDC(runtime imv1.Runtime, shoot *gardener.Shoot) error { oidcConfig := runtime.Spec.Shoot.Kubernetes.KubeAPIServer.OidcConfig - setOIDCExtension(shoot) + if CanEnableExtension(shoot) { + setOIDCExtension(shoot) + } setKubeAPIServerOIDCConfig(shoot, oidcConfig) return nil } +func CanEnableExtension(shoot *gardener.Shoot) bool { + controlledByProvisionerLabelValue := shoot.Labels[imv1.LabelControlledByProvisioner] + canEnable := false + + if controlledByProvisionerLabelValue == "false" { + canEnable = true + } + + return canEnable +} + func setOIDCExtension(shoot *gardener.Shoot) { oidcService := gardener.Extension{ Type: OidcExtensionType, From 4908db32739be02f1ac5303166de36517219dd3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Wed, 11 Sep 2024 09:10:53 +0200 Subject: [PATCH 2/5] oidc is not being configured for migrated clusters --- internal/gardener/shoot/extender/oidc.go | 8 +- internal/gardener/shoot/extender/oidc_test.go | 84 ++++++++++++------- 2 files changed, 59 insertions(+), 33 deletions(-) diff --git a/internal/gardener/shoot/extender/oidc.go b/internal/gardener/shoot/extender/oidc.go index 90e7e5e8..ae93b6d8 100644 --- a/internal/gardener/shoot/extender/oidc.go +++ b/internal/gardener/shoot/extender/oidc.go @@ -22,11 +22,11 @@ func ExtendWithOIDC(runtime imv1.Runtime, shoot *gardener.Shoot) error { } func CanEnableExtension(shoot *gardener.Shoot) bool { - controlledByProvisionerLabelValue := shoot.Labels[imv1.LabelControlledByProvisioner] - canEnable := false + canEnable := true + createdByMigrator := shoot.Labels["operator.kyma-project.io/created-by-migrator"] - if controlledByProvisionerLabelValue == "false" { - canEnable = true + if createdByMigrator == "true" { + canEnable = false } return canEnable diff --git a/internal/gardener/shoot/extender/oidc_test.go b/internal/gardener/shoot/extender/oidc_test.go index 2fda2db5..75e8cbe6 100644 --- a/internal/gardener/shoot/extender/oidc_test.go +++ b/internal/gardener/shoot/extender/oidc_test.go @@ -10,42 +10,68 @@ import ( ) func TestOidcExtender(t *testing.T) { - t.Run("Create kubernetes config", func(t *testing.T) { - // given - clientID := "client-id" - groupsClaim := "groups" - issuerURL := "https://my.cool.tokens.com" - usernameClaim := "sub" + const MigratorLabel = "operator.kyma-project.io/created-by-migrator" + for _, testCase := range []struct { + name string + migratorLabel map[string]string + expectedOidcExtensionEnabled bool + }{ + { + name: "label created-by-migrator=true should not configure OIDC", + migratorLabel: map[string]string{MigratorLabel: "true"}, + expectedOidcExtensionEnabled: false, + }, + { + name: "label created-by-migrator=false should configure OIDC", + migratorLabel: map[string]string{MigratorLabel: "false"}, + expectedOidcExtensionEnabled: true, + }, + { + name: "label created-by-migrator unset should configure OIDC", + migratorLabel: nil, + expectedOidcExtensionEnabled: true, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + // given + clientID := "client-id" + groupsClaim := "groups" + issuerURL := "https://my.cool.tokens.com" + usernameClaim := "sub" - shoot := fixEmptyGardenerShoot("test", "kcp-system") - runtimeShoot := imv1.Runtime{ - Spec: imv1.RuntimeSpec{ - Shoot: imv1.RuntimeShoot{ - Kubernetes: imv1.Kubernetes{ - KubeAPIServer: imv1.APIServer{ - OidcConfig: gardener.OIDCConfig{ - ClientID: &clientID, - GroupsClaim: &groupsClaim, - IssuerURL: &issuerURL, - SigningAlgs: []string{ - "RS256", + shoot := fixEmptyGardenerShoot("test", "kcp-system") + shoot.Labels[MigratorLabel] = testCase.migratorLabel[MigratorLabel] + runtimeShoot := imv1.Runtime{ + Spec: imv1.RuntimeSpec{ + Shoot: imv1.RuntimeShoot{ + Kubernetes: imv1.Kubernetes{ + KubeAPIServer: imv1.APIServer{ + OidcConfig: gardener.OIDCConfig{ + ClientID: &clientID, + GroupsClaim: &groupsClaim, + IssuerURL: &issuerURL, + SigningAlgs: []string{ + "RS256", + }, + UsernameClaim: &usernameClaim, }, - UsernameClaim: &usernameClaim, }, }, }, }, - }, - } + } - // when - err := ExtendWithOIDC(runtimeShoot, &shoot) + // when + err := ExtendWithOIDC(runtimeShoot, &shoot) - // then - require.NoError(t, err) + // then + require.NoError(t, err) - assert.Equal(t, runtimeShoot.Spec.Shoot.Kubernetes.KubeAPIServer.OidcConfig, *shoot.Spec.Kubernetes.KubeAPIServer.OIDCConfig) - assert.Equal(t, false, *shoot.Spec.Extensions[0].Disabled) - assert.Equal(t, "shoot-oidc-service", shoot.Spec.Extensions[0].Type) - }) + assert.Equal(t, runtimeShoot.Spec.Shoot.Kubernetes.KubeAPIServer.OidcConfig, *shoot.Spec.Kubernetes.KubeAPIServer.OIDCConfig) + if testCase.expectedOidcExtensionEnabled { + assert.Equal(t, testCase.expectedOidcExtensionEnabled, !*shoot.Spec.Extensions[0].Disabled) + assert.Equal(t, "shoot-oidc-service", shoot.Spec.Extensions[0].Type) + } + }) + } } From cdf28343f813f947cee4c4d345ce6289d3336def Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Wed, 11 Sep 2024 09:15:05 +0200 Subject: [PATCH 3/5] removes bootstraping of controlled-by-provisioner label in tests --- internal/gardener/shoot/extender/dns_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/gardener/shoot/extender/dns_test.go b/internal/gardener/shoot/extender/dns_test.go index 7d90ce05..f0e4c032 100644 --- a/internal/gardener/shoot/extender/dns_test.go +++ b/internal/gardener/shoot/extender/dns_test.go @@ -58,9 +58,7 @@ func fixEmptyGardenerShoot(name, namespace string) gardener.Shoot { ObjectMeta: v1.ObjectMeta{ Name: name, Namespace: namespace, - Labels: map[string]string{ - "kyma-project.io/controlled-by-provisioner": "false", - }, + Labels: map[string]string{}, }, Spec: gardener.ShootSpec{}, } From aca7e6fd664ca8df8b2102f2bc92220a5ee46586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Wed, 11 Sep 2024 11:51:34 +0200 Subject: [PATCH 4/5] adds oidc disabled extender test case --- internal/gardener/shoot/extender/oidc_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/gardener/shoot/extender/oidc_test.go b/internal/gardener/shoot/extender/oidc_test.go index 75e8cbe6..941443b1 100644 --- a/internal/gardener/shoot/extender/oidc_test.go +++ b/internal/gardener/shoot/extender/oidc_test.go @@ -71,6 +71,8 @@ func TestOidcExtender(t *testing.T) { if testCase.expectedOidcExtensionEnabled { assert.Equal(t, testCase.expectedOidcExtensionEnabled, !*shoot.Spec.Extensions[0].Disabled) assert.Equal(t, "shoot-oidc-service", shoot.Spec.Extensions[0].Type) + } else { + assert.Equal(t, 0, len(shoot.Spec.Extensions)) } }) } From 8161e84adfb7906eeffd4288be907387f2c983a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Fri, 13 Sep 2024 15:19:13 +0200 Subject: [PATCH 5/5] relies on Runtime CR label instead of Shoot CR --- internal/gardener/shoot/extender/oidc.go | 6 +++--- internal/gardener/shoot/extender/oidc_test.go | 7 ++++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/gardener/shoot/extender/oidc.go b/internal/gardener/shoot/extender/oidc.go index ae93b6d8..b776cf71 100644 --- a/internal/gardener/shoot/extender/oidc.go +++ b/internal/gardener/shoot/extender/oidc.go @@ -13,7 +13,7 @@ const ( func ExtendWithOIDC(runtime imv1.Runtime, shoot *gardener.Shoot) error { oidcConfig := runtime.Spec.Shoot.Kubernetes.KubeAPIServer.OidcConfig - if CanEnableExtension(shoot) { + if CanEnableExtension(runtime) { setOIDCExtension(shoot) } setKubeAPIServerOIDCConfig(shoot, oidcConfig) @@ -21,9 +21,9 @@ func ExtendWithOIDC(runtime imv1.Runtime, shoot *gardener.Shoot) error { return nil } -func CanEnableExtension(shoot *gardener.Shoot) bool { +func CanEnableExtension(runtime imv1.Runtime) bool { canEnable := true - createdByMigrator := shoot.Labels["operator.kyma-project.io/created-by-migrator"] + createdByMigrator := runtime.Labels["operator.kyma-project.io/created-by-migrator"] if createdByMigrator == "true" { canEnable = false diff --git a/internal/gardener/shoot/extender/oidc_test.go b/internal/gardener/shoot/extender/oidc_test.go index 941443b1..3fc0e683 100644 --- a/internal/gardener/shoot/extender/oidc_test.go +++ b/internal/gardener/shoot/extender/oidc_test.go @@ -1,6 +1,7 @@ package extender import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "testing" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" @@ -40,8 +41,12 @@ func TestOidcExtender(t *testing.T) { usernameClaim := "sub" shoot := fixEmptyGardenerShoot("test", "kcp-system") - shoot.Labels[MigratorLabel] = testCase.migratorLabel[MigratorLabel] runtimeShoot := imv1.Runtime{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + MigratorLabel: testCase.migratorLabel[MigratorLabel], + }, + }, Spec: imv1.RuntimeSpec{ Shoot: imv1.RuntimeShoot{ Kubernetes: imv1.Kubernetes{