From de6eb8cbb6902ce37fa5b1a387ff49eed7a83dec Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Tue, 6 Aug 2024 17:08:50 +0200 Subject: [PATCH 01/53] Fixes to make migrator up to date --- hack/runtime-migrator/main.go | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/hack/runtime-migrator/main.go b/hack/runtime-migrator/main.go index 97791825..3aee2f6e 100644 --- a/hack/runtime-migrator/main.go +++ b/hack/runtime-migrator/main.go @@ -166,7 +166,6 @@ func saveRuntime(ctx context.Context, cfg migrator.Config, runtime v1.Runtime, g func createRuntime(ctx context.Context, shoot v1beta1.Shoot, cfg migrator.Config, provider kubeconfig.Provider) (v1.Runtime, error) { var subjects = getAdministratorsList(ctx, provider, shoot.Name) var oidcConfig = getOidcConfig(shoot) - var hAFailureToleranceType = getFailureToleranceType(shoot) var licenceType = shoot.Annotations["kcp.provisioner.kyma-project.io/licence-type"] labels, err := getAllRuntimeLabels(ctx, shoot, cfg.Client) if err != nil { @@ -210,16 +209,13 @@ func createRuntime(ctx context.Context, shoot v1beta1.Shoot, cfg migrator.Config Workers: 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: v1beta1.ControlPlane{ - HighAvailability: &v1beta1.HighAvailability{ - FailureTolerance: v1beta1.FailureTolerance{ - Type: hAFailureToleranceType, - }, - }, + HighAvailability: getHighAvailability(shoot), }, }, Security: v1.Security{ @@ -280,13 +276,18 @@ func getShootList(ctx context.Context, cfg migrator.Config, gardenerNamespace st return list } -func getFailureToleranceType(shoot v1beta1.Shoot) v1beta1.FailureToleranceType { +func getHighAvailability(shoot v1beta1.Shoot) *v1beta1.HighAvailability { if shoot.Spec.ControlPlane != nil { if shoot.Spec.ControlPlane.HighAvailability != nil { - return shoot.Spec.ControlPlane.HighAvailability.FailureTolerance.Type + return &v1beta1.HighAvailability{ + FailureTolerance: v1beta1.FailureTolerance{ + Type: shoot.Spec.ControlPlane.HighAvailability.FailureTolerance.Type, + }, + } } } - return "" + + return nil } func getAdministratorsList(ctx context.Context, provider kubeconfig.Provider, shootName string) []string { @@ -395,8 +396,14 @@ func getAllRuntimeLabels(ctx context.Context, shoot v1beta1.Shoot, getClient mig return map[string]string{}, errors.Wrap(clientErr, fmt.Sprintf("Failed to get GardenerClient for shoot %s - %s\n", shoot.Name, clientErr)) } gardenerCluster := v1.GardenerCluster{} - shootKey := types.NamespacedName{Name: shoot.Name, Namespace: "kcp-system"} - getGardenerCRerr := k8sClient.Get(ctx, shootKey, &gardenerCluster) + + kymaID, found := shoot.Annotations["kcp.provisioner.kyma-project.io/runtime-id"] + if !found { + return nil, errors.New("Runtime ID not found in shoot annotations") + } + + gardenerCRKey := types.NamespacedName{Name: kymaID, Namespace: "kcp-system"} + getGardenerCRerr := k8sClient.Get(ctx, gardenerCRKey, &gardenerCluster) if getGardenerCRerr != nil { var errMsg = fmt.Sprintf("Failed to retrieve GardenerCluster CR for shoot %s\n", shoot.Name) return map[string]string{}, errors.Wrap(getGardenerCRerr, errMsg) @@ -410,7 +417,8 @@ func getAllRuntimeLabels(ctx context.Context, shoot v1beta1.Shoot, getClient mig enrichedRuntimeLabels["kyma-project.io/region"] = gardenerCluster.Labels["kyma-project.io/region"] enrichedRuntimeLabels["kyma-project.io/shoot-name"] = gardenerCluster.Labels["kyma-project.io/shoot-name"] enrichedRuntimeLabels["operator.kyma-project.io/kyma-name"] = gardenerCluster.Labels["operator.kyma-project.io/kyma-name"] - + // The runtime CR should not be controlled by the KIM + enrichedRuntimeLabels["kyma-project.io/controlled-by-provisioner"] = "false" // add custom label for the migrator enrichedRuntimeLabels[migratorLabel] = "true" From 67ba2e080bae1a0754be2c5abae1cac5972938f9 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Wed, 7 Aug 2024 09:35:27 +0200 Subject: [PATCH 02/53] First version of the Extension comparator --- hack/runtime-migrator/main.go | 11 +-- hack/shoot-comparator/go.mod | 5 +- .../pkg/shoot/etensionmatcher_test.go | 74 +++++++++++++++ .../pkg/shoot/extensionmatcher.go | 95 +++++++++++++++++++ hack/shoot-comparator/pkg/shoot/matcher.go | 64 +++++++------ 5 files changed, 210 insertions(+), 39 deletions(-) create mode 100644 hack/shoot-comparator/pkg/shoot/etensionmatcher_test.go create mode 100644 hack/shoot-comparator/pkg/shoot/extensionmatcher.go diff --git a/hack/runtime-migrator/main.go b/hack/runtime-migrator/main.go index 3aee2f6e..53677cd3 100644 --- a/hack/runtime-migrator/main.go +++ b/hack/runtime-migrator/main.go @@ -214,9 +214,7 @@ func createRuntime(ctx context.Context, shoot v1beta1.Shoot, cfg migrator.Config Nodes: *shoot.Spec.Networking.Nodes, Services: *shoot.Spec.Networking.Services, }, - ControlPlane: v1beta1.ControlPlane{ - HighAvailability: getHighAvailability(shoot), - }, + ControlPlane: getControlPlane(shoot), }, Security: v1.Security{ Administrators: subjects, @@ -276,18 +274,19 @@ func getShootList(ctx context.Context, cfg migrator.Config, gardenerNamespace st return list } -func getHighAvailability(shoot v1beta1.Shoot) *v1beta1.HighAvailability { +func getControlPlane(shoot v1beta1.Shoot) v1beta1.ControlPlane { if shoot.Spec.ControlPlane != nil { if shoot.Spec.ControlPlane.HighAvailability != nil { - return &v1beta1.HighAvailability{ + return v1beta1.ControlPlane{HighAvailability: &v1beta1.HighAvailability{ FailureTolerance: v1beta1.FailureTolerance{ Type: shoot.Spec.ControlPlane.HighAvailability.FailureTolerance.Type, }, + }, } } } - return nil + return v1beta1.ControlPlane{} } func getAdministratorsList(ctx context.Context, provider kubeconfig.Provider, shootName string) []string { diff --git a/hack/shoot-comparator/go.mod b/hack/shoot-comparator/go.mod index 2f6be242..b1374110 100644 --- a/hack/shoot-comparator/go.mod +++ b/hack/shoot-comparator/go.mod @@ -6,8 +6,11 @@ require ( github.com/gardener/gardener v1.98.0 github.com/onsi/ginkgo/v2 v2.19.0 github.com/onsi/gomega v1.33.1 + github.com/pkg/errors v0.9.1 github.com/spf13/cobra v1.8.1 github.com/stretchr/testify v1.9.0 + gopkg.in/yaml.v3 v3.0.1 + k8s.io/apimachinery v0.29.6 sigs.k8s.io/yaml v1.4.0 ) @@ -31,10 +34,8 @@ require ( golang.org/x/tools v0.22.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/api v0.29.6 // indirect k8s.io/apiextensions-apiserver v0.29.6 // indirect - k8s.io/apimachinery v0.29.6 // indirect k8s.io/klog/v2 v2.120.1 // indirect k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect diff --git a/hack/shoot-comparator/pkg/shoot/etensionmatcher_test.go b/hack/shoot-comparator/pkg/shoot/etensionmatcher_test.go new file mode 100644 index 00000000..06866988 --- /dev/null +++ b/hack/shoot-comparator/pkg/shoot/etensionmatcher_test.go @@ -0,0 +1,74 @@ +package shoot + +import ( + "github.com/gardener/gardener/pkg/apis/core/v1beta1" + . "github.com/onsi/ginkgo/v2" //nolint:revive + . "github.com/onsi/gomega" //nolint:revive +) + +var _ = Describe(":: extension matcher :: ", func() { + var empty []v1beta1.Extension + + DescribeTable( + "checking invalid args :: ", + testExtensionInvalidArgs, + Entry("when actual is nil", nil, empty), + Entry("when expected is nil", "", nil), + ) + + //disabledTrue := true + disabledFalse := false + + DescribeTable( + "checking results :: ", + testExtensionResults, + Entry( + "should match empty and zero values", + nil, + empty, + true, + ), + Entry( + "should match identical tables", + newExtensionTable(newExtension("test", &disabledFalse)), + newExtensionTable(newExtension("test", &disabledFalse)), + true, + ), + Entry( + "should detect extension missing", + newExtensionTable(newExtension("test1", &disabledFalse), newExtension("test2", &disabledFalse)), + newExtensionTable(newExtension("test", &disabledFalse)), + false, + ), + Entry( + "should detect redundant extension", + newExtensionTable(newExtension("test", &disabledFalse)), + newExtensionTable(newExtension("test1", &disabledFalse), newExtension("test2", &disabledFalse)), + false, + ), + ) +}) + +func newExtension(name string, disabled *bool) v1beta1.Extension { + return v1beta1.Extension{ + Type: name, + Disabled: disabled, + } +} + +func newExtensionTable(extensions ...v1beta1.Extension) []v1beta1.Extension { + return extensions +} + +func testExtensionInvalidArgs(actual, expected interface{}) { + matcher := NewExtensionMatcher(expected) + _, err := matcher.Match(actual) + Expect(err).To(HaveOccurred()) +} + +func testExtensionResults(actual, expected interface{}, expectedMatch bool) { + matcher := NewExtensionMatcher(expected) + actualMatch, err := matcher.Match(actual) + Expect(err).ShouldNot(HaveOccurred()) + Expect(actualMatch).Should(Equal(expectedMatch), matcher.FailureMessage(actual)) +} diff --git a/hack/shoot-comparator/pkg/shoot/extensionmatcher.go b/hack/shoot-comparator/pkg/shoot/extensionmatcher.go new file mode 100644 index 00000000..5377da41 --- /dev/null +++ b/hack/shoot-comparator/pkg/shoot/extensionmatcher.go @@ -0,0 +1,95 @@ +package shoot + +import ( + "fmt" + "github.com/gardener/gardener/pkg/apis/core/v1beta1" + "github.com/onsi/gomega" + "github.com/onsi/gomega/types" + "github.com/pkg/errors" + "reflect" + "strings" +) + +type ExtensionMatcher struct { + toMatch interface{} + fails []string +} + +func NewExtensionMatcher(i interface{}) types.GomegaMatcher { + return &ExtensionMatcher{ + toMatch: i, + } +} + +func (m *ExtensionMatcher) Match(actual interface{}) (success bool, err error) { + + if err != nil { + return false, err + } + + if actual == nil && m.toMatch != nil { + return false, errors.New("actual is nil") + } + + if actual != nil && m.toMatch == nil { + return false, errors.New("expected is nil") + } + + if actual == nil && m.toMatch == nil { + return true, nil + } + + aExtensions, err := getExtension(actual) + findExtension := func(extensions []v1beta1.Extension, extensionToFind string) v1beta1.Extension { + for _, extension := range extensions { + if extension.Type == extensionToFind { + return extension + } + } + return v1beta1.Extension{} + } + + eExtensions, err := getExtension(m.toMatch) + if err != nil { + return false, err + } + + for i, eExtension := range eExtensions { + aExtension := findExtension(aExtensions, eExtension.Type) + matcher := gomega.BeComparableTo(eExtension.Type) + ok, err := matcher.Match(aExtension.Type) + if err != nil { + return false, err + } + + if !ok { + msg := fmt.Sprintf("spec/Extensions[%d]: %s", i, matcher.FailureMessage(aExtension)) + m.fails = append(m.fails, msg) + } + + } + + return false, nil +} + +func (m *ExtensionMatcher) NegatedFailureMessage(_ interface{}) string { + return "expected should not equal actual" +} + +func (m *ExtensionMatcher) FailureMessage(_ interface{}) string { + return strings.Join(m.fails, "\n") +} + +func getExtension(i interface{}) (shoot []v1beta1.Extension, err error) { + if i == nil { + return []v1beta1.Extension{}, fmt.Errorf("invalid value nil") + } + + switch v := i.(type) { + case []v1beta1.Extension: + return v, nil + + default: + return []v1beta1.Extension{}, fmt.Errorf(`%w: %s`, errInvalidType, reflect.TypeOf(v)) + } +} diff --git a/hack/shoot-comparator/pkg/shoot/matcher.go b/hack/shoot-comparator/pkg/shoot/matcher.go index 4202f08d..3b6606b6 100644 --- a/hack/shoot-comparator/pkg/shoot/matcher.go +++ b/hack/shoot-comparator/pkg/shoot/matcher.go @@ -67,138 +67,140 @@ func (m *Matcher) Match(actual interface{}) (success bool, err error) { // Note: we define separate matchers for each field to make input more readable // Annotations are not matched as they are not relevant for the comparison ; both KIM, and Provisioner have different set of annotations for _, matcher := range []matcher{ - { - GomegaMatcher: gomega.Equal(eShoot.TypeMeta), - expected: aShoot.TypeMeta, - }, - { - GomegaMatcher: gomega.Equal(eShoot.Name), + // We need to skip comparing type meta as Provisioner doesn't set it. + // It is simpler to skip it than to make fix in the Provisioner. + //{ + // GomegaMatcher: gomega.BeComparableTo(eShoot.TypeMeta), + // expected: aShoot.TypeMeta, + //}, + { + GomegaMatcher: gomega.BeComparableTo(eShoot.Name), expected: aShoot.Name, path: "metadata/name", }, { - GomegaMatcher: gomega.Equal(eShoot.Namespace), + GomegaMatcher: gomega.BeComparableTo(eShoot.Namespace), expected: aShoot.Namespace, path: "metadata/namespace", }, { - GomegaMatcher: gomega.Equal(eShoot.Labels), + GomegaMatcher: gomega.BeComparableTo(eShoot.Labels), expected: aShoot.Labels, path: "metadata/labels", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.Addons), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Addons), expected: aShoot.Spec.Addons, path: "spec/Addons", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.CloudProfileName), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.CloudProfileName), expected: aShoot.Spec.CloudProfileName, path: "spec/CloudProfileName", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.DNS), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.DNS), expected: aShoot.Spec.DNS, path: "spec/DNS", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.Extensions), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Extensions), expected: aShoot.Spec.Extensions, path: "spec/Extensions", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.Hibernation), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Hibernation), expected: aShoot.Spec.Hibernation, path: "spec/Hibernation", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.Kubernetes), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Kubernetes), expected: aShoot.Spec.Kubernetes, path: "spec/Kubernetes", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.Networking), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Networking), expected: aShoot.Spec.Networking, path: "spec/Networking", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.Maintenance), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Maintenance), expected: aShoot.Spec.Maintenance, path: "spec/Maintenance", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.Monitoring), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Monitoring), expected: aShoot.Spec.Monitoring, path: "spec/Monitoring", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.Provider), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Provider), expected: aShoot.Spec.Provider, path: "spec/Provider", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.Purpose), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Purpose), expected: aShoot.Spec.Purpose, path: "spec/Purpose", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.Region), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Region), expected: aShoot.Spec.Region, path: "spec/Region", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.SecretBindingName), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.SecretBindingName), expected: aShoot.Spec.SecretBindingName, path: "spec/SecretBindingName", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.SeedName), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.SeedName), expected: aShoot.Spec.SeedName, path: "spec/SeedName", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.SeedSelector), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.SeedSelector), expected: aShoot.Spec.SeedSelector, path: "spec/SeedSelector", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.Resources), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Resources), expected: aShoot.Spec.Resources, path: "spec/Resources", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.Tolerations), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Tolerations), expected: aShoot.Spec.Tolerations, path: "spec/Tolerations", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.ExposureClassName), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.ExposureClassName), expected: aShoot.Spec.ExposureClassName, path: "spec/ExposureClassName", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.SystemComponents), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.SystemComponents), expected: aShoot.Spec.SystemComponents, path: "spec/SystemComponents", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.ControlPlane), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.ControlPlane), expected: aShoot.Spec.ControlPlane, path: "spec/ControlPlane", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.SchedulerName), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.SchedulerName), expected: aShoot.Spec.SchedulerName, path: "spec/SchedulerName", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.CloudProfile), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.CloudProfile), expected: aShoot.Spec.CloudProfile, path: "spec/CloudProfile", }, { - GomegaMatcher: gomega.Equal(eShoot.Spec.CredentialsBindingName), + GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.CredentialsBindingName), expected: aShoot.Spec.CredentialsBindingName, path: "spec/CredentialsBindingName", }, From 1c84bff0d693fcac66bdb90d11cf806fb423eea3 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Mon, 12 Aug 2024 10:52:58 +0200 Subject: [PATCH 03/53] Extensions matcher used plus some fixes for comparison --- api/v1/runtime_types.go | 22 ++-- api/v1/zz_generated.deepcopy.go | 6 +- hack/runtime-migrator/main.go | 6 +- .../pkg/shoot/etensionmatcher_test.go | 74 ------------ .../pkg/shoot/extensionmatcher.go | 56 ++++----- .../pkg/shoot/extensionmatcher_test.go | 106 ++++++++++++++++++ hack/shoot-comparator/pkg/shoot/matcher.go | 2 +- internal/gardener/shoot/converter.go | 2 +- 8 files changed, 155 insertions(+), 119 deletions(-) delete mode 100644 hack/shoot-comparator/pkg/shoot/etensionmatcher_test.go create mode 100644 hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go diff --git a/api/v1/runtime_types.go b/api/v1/runtime_types.go index 1e6af618..1ab41cbe 100644 --- a/api/v1/runtime_types.go +++ b/api/v1/runtime_types.go @@ -139,17 +139,17 @@ type RuntimeStatus struct { } type RuntimeShoot struct { - Name string `json:"name"` - Purpose gardener.ShootPurpose `json:"purpose"` - PlatformRegion string `json:"platformRegion"` - Region string `json:"region"` - LicenceType *string `json:"licenceType,omitempty"` - SecretBindingName string `json:"secretBindingName"` - EnforceSeedLocation *bool `json:"enforceSeedLocation,omitempty"` - Kubernetes Kubernetes `json:"kubernetes,omitempty"` - Provider Provider `json:"provider"` - Networking Networking `json:"networking"` - ControlPlane gardener.ControlPlane `json:"controlPlane"` + Name string `json:"name"` + Purpose gardener.ShootPurpose `json:"purpose"` + PlatformRegion string `json:"platformRegion"` + Region string `json:"region"` + LicenceType *string `json:"licenceType,omitempty"` + SecretBindingName string `json:"secretBindingName"` + EnforceSeedLocation *bool `json:"enforceSeedLocation,omitempty"` + Kubernetes Kubernetes `json:"kubernetes,omitempty"` + Provider Provider `json:"provider"` + Networking Networking `json:"networking"` + ControlPlane *gardener.ControlPlane `json:"controlPlane"` } type Kubernetes struct { diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index a96109e4..9221ecb8 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -372,7 +372,11 @@ func (in *RuntimeShoot) DeepCopyInto(out *RuntimeShoot) { in.Kubernetes.DeepCopyInto(&out.Kubernetes) in.Provider.DeepCopyInto(&out.Provider) in.Networking.DeepCopyInto(&out.Networking) - in.ControlPlane.DeepCopyInto(&out.ControlPlane) + if in.ControlPlane != nil { + in, out := &in.ControlPlane, &out.ControlPlane + *out = new(v1beta1.ControlPlane) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RuntimeShoot. diff --git a/hack/runtime-migrator/main.go b/hack/runtime-migrator/main.go index 53677cd3..1f783e22 100644 --- a/hack/runtime-migrator/main.go +++ b/hack/runtime-migrator/main.go @@ -274,10 +274,10 @@ func getShootList(ctx context.Context, cfg migrator.Config, gardenerNamespace st return list } -func getControlPlane(shoot v1beta1.Shoot) v1beta1.ControlPlane { +func getControlPlane(shoot v1beta1.Shoot) *v1beta1.ControlPlane { if shoot.Spec.ControlPlane != nil { if shoot.Spec.ControlPlane.HighAvailability != nil { - return v1beta1.ControlPlane{HighAvailability: &v1beta1.HighAvailability{ + return &v1beta1.ControlPlane{HighAvailability: &v1beta1.HighAvailability{ FailureTolerance: v1beta1.FailureTolerance{ Type: shoot.Spec.ControlPlane.HighAvailability.FailureTolerance.Type, }, @@ -286,7 +286,7 @@ func getControlPlane(shoot v1beta1.Shoot) v1beta1.ControlPlane { } } - return v1beta1.ControlPlane{} + return nil } func getAdministratorsList(ctx context.Context, provider kubeconfig.Provider, shootName string) []string { diff --git a/hack/shoot-comparator/pkg/shoot/etensionmatcher_test.go b/hack/shoot-comparator/pkg/shoot/etensionmatcher_test.go deleted file mode 100644 index 06866988..00000000 --- a/hack/shoot-comparator/pkg/shoot/etensionmatcher_test.go +++ /dev/null @@ -1,74 +0,0 @@ -package shoot - -import ( - "github.com/gardener/gardener/pkg/apis/core/v1beta1" - . "github.com/onsi/ginkgo/v2" //nolint:revive - . "github.com/onsi/gomega" //nolint:revive -) - -var _ = Describe(":: extension matcher :: ", func() { - var empty []v1beta1.Extension - - DescribeTable( - "checking invalid args :: ", - testExtensionInvalidArgs, - Entry("when actual is nil", nil, empty), - Entry("when expected is nil", "", nil), - ) - - //disabledTrue := true - disabledFalse := false - - DescribeTable( - "checking results :: ", - testExtensionResults, - Entry( - "should match empty and zero values", - nil, - empty, - true, - ), - Entry( - "should match identical tables", - newExtensionTable(newExtension("test", &disabledFalse)), - newExtensionTable(newExtension("test", &disabledFalse)), - true, - ), - Entry( - "should detect extension missing", - newExtensionTable(newExtension("test1", &disabledFalse), newExtension("test2", &disabledFalse)), - newExtensionTable(newExtension("test", &disabledFalse)), - false, - ), - Entry( - "should detect redundant extension", - newExtensionTable(newExtension("test", &disabledFalse)), - newExtensionTable(newExtension("test1", &disabledFalse), newExtension("test2", &disabledFalse)), - false, - ), - ) -}) - -func newExtension(name string, disabled *bool) v1beta1.Extension { - return v1beta1.Extension{ - Type: name, - Disabled: disabled, - } -} - -func newExtensionTable(extensions ...v1beta1.Extension) []v1beta1.Extension { - return extensions -} - -func testExtensionInvalidArgs(actual, expected interface{}) { - matcher := NewExtensionMatcher(expected) - _, err := matcher.Match(actual) - Expect(err).To(HaveOccurred()) -} - -func testExtensionResults(actual, expected interface{}, expectedMatch bool) { - matcher := NewExtensionMatcher(expected) - actualMatch, err := matcher.Match(actual) - Expect(err).ShouldNot(HaveOccurred()) - Expect(actualMatch).Should(Equal(expectedMatch), matcher.FailureMessage(actual)) -} diff --git a/hack/shoot-comparator/pkg/shoot/extensionmatcher.go b/hack/shoot-comparator/pkg/shoot/extensionmatcher.go index 5377da41..1421964d 100644 --- a/hack/shoot-comparator/pkg/shoot/extensionmatcher.go +++ b/hack/shoot-comparator/pkg/shoot/extensionmatcher.go @@ -5,7 +5,6 @@ import ( "github.com/gardener/gardener/pkg/apis/core/v1beta1" "github.com/onsi/gomega" "github.com/onsi/gomega/types" - "github.com/pkg/errors" "reflect" "strings" ) @@ -23,53 +22,58 @@ func NewExtensionMatcher(i interface{}) types.GomegaMatcher { func (m *ExtensionMatcher) Match(actual interface{}) (success bool, err error) { + aExtensions, err := getExtension(actual) if err != nil { return false, err } - if actual == nil && m.toMatch != nil { - return false, errors.New("actual is nil") + eExtensions, err := getExtension(m.toMatch) + if err != nil { + return false, err } - if actual != nil && m.toMatch == nil { - return false, errors.New("expected is nil") + if len(aExtensions) != len(eExtensions) { + m.fails = append(m.fails, "Extensions count mismatch") + return false, nil } - if actual == nil && m.toMatch == nil { + if len(aExtensions) == 0 && len(eExtensions) == 0 { return true, nil } - aExtensions, err := getExtension(actual) - findExtension := func(extensions []v1beta1.Extension, extensionToFind string) v1beta1.Extension { - for _, extension := range extensions { - if extension.Type == extensionToFind { - return extension + findExtension := func(name string, extensions []v1beta1.Extension) v1beta1.Extension { + for _, e := range extensions { + if e.Type == name { + return e } } + return v1beta1.Extension{} } - eExtensions, err := getExtension(m.toMatch) - if err != nil { - return false, err - } + differenceFound := false + + for _, e := range eExtensions { + a := findExtension(e.Type, aExtensions) + if a.Type == "" { + m.fails = append(m.fails, fmt.Sprintf("Extension %s not found in both expected and actual", e.Type)) + return false, nil + } - for i, eExtension := range eExtensions { - aExtension := findExtension(aExtensions, eExtension.Type) - matcher := gomega.BeComparableTo(eExtension.Type) - ok, err := matcher.Match(aExtension.Type) + matcher := gomega.BeComparableTo(e) + + ok, err := matcher.Match(a) if err != nil { return false, err } if !ok { - msg := fmt.Sprintf("spec/Extensions[%d]: %s", i, matcher.FailureMessage(aExtension)) - m.fails = append(m.fails, msg) + differenceFound = true + m.fails = append(m.fails, matcher.FailureMessage(a)) } - } - return false, nil + return !differenceFound, nil } func (m *ExtensionMatcher) NegatedFailureMessage(_ interface{}) string { @@ -80,11 +84,7 @@ func (m *ExtensionMatcher) FailureMessage(_ interface{}) string { return strings.Join(m.fails, "\n") } -func getExtension(i interface{}) (shoot []v1beta1.Extension, err error) { - if i == nil { - return []v1beta1.Extension{}, fmt.Errorf("invalid value nil") - } - +func getExtension(i interface{}) ([]v1beta1.Extension, error) { switch v := i.(type) { case []v1beta1.Extension: return v, nil diff --git a/hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go b/hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go new file mode 100644 index 00000000..f6c7c486 --- /dev/null +++ b/hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go @@ -0,0 +1,106 @@ +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 + "k8s.io/apimachinery/pkg/runtime" +) + +var _ = Describe(":: extension matcher :: ", func() { + emptyExt := make([]v1beta1.Extension, 0) + + DescribeTable( + "checking invalid args :: ", + testExtensionInvalidArgs, + Entry("when actual is not of Extension type", v1beta1.Shoot{}, emptyExt), + Entry("when expected is not of Extension type", emptyExt, v1beta1.Shoot{}), + Entry("when actual is nil", nil, emptyExt), + Entry("when expected is nil", emptyExt, nil), + ) + + dnsExtEnabled := getDNSExtension(false, true) + dnsExtDisabled := getDNSExtension(true, false) + networkingExtDisabled := getNetworkingExtension(true) + certificateExtEnabled := getCertificateExtension(false, true) + + DescribeTable( + "checking results :: ", + testExtensionResults, + Entry( + "should match empty values", + emptyExt, + emptyExt, + true, + ), + Entry( + "should match identical tables", + []v1beta1.Extension{dnsExtEnabled, networkingExtDisabled, certificateExtEnabled}, + []v1beta1.Extension{networkingExtDisabled, certificateExtEnabled, dnsExtEnabled}, + true, + ), + Entry( + "should detect extension differs", + []v1beta1.Extension{dnsExtEnabled, networkingExtDisabled, certificateExtEnabled}, + []v1beta1.Extension{dnsExtDisabled, networkingExtDisabled, certificateExtEnabled}, + false, + ), + Entry( + "should detect missing extension", + []v1beta1.Extension{dnsExtEnabled, networkingExtDisabled, certificateExtEnabled}, + []v1beta1.Extension{networkingExtDisabled, certificateExtEnabled}, + false, + ), + Entry( + "should detect redundant extension", + []v1beta1.Extension{networkingExtDisabled, certificateExtEnabled}, + []v1beta1.Extension{dnsExtEnabled, networkingExtDisabled, certificateExtEnabled}, + false, + ), + ) +}) + +func testExtensionInvalidArgs(actual, expected interface{}) { + matcher := NewExtensionMatcher(expected) + _, err := matcher.Match(actual) + Expect(err).To(HaveOccurred()) +} + +func testExtensionResults(actual, expected interface{}, expectedMatch bool) { + matcher := NewExtensionMatcher(expected) + actualMatch, err := matcher.Match(actual) + Expect(err).ShouldNot(HaveOccurred()) + Expect(actualMatch).Should(Equal(expectedMatch), matcher.FailureMessage(actual)) +} + +func getDNSExtension(disabled bool, replicationEnabled bool) v1beta1.Extension { + json := fmt.Sprintf(`{apiVersion: "service.dns.extensions.gardener.cloud/v1alpha1", kind: "DNSConfig", dnsProviderReplication: {enabled: %v}}`, replicationEnabled) + + return v1beta1.Extension{ + Type: "shoot-dns-service", + Disabled: &disabled, + ProviderConfig: &runtime.RawExtension{ + Raw: []byte(json), + }, + } +} + +func getNetworkingExtension(disabled bool) v1beta1.Extension { + return v1beta1.Extension{ + Type: "shoot-networking-service", + Disabled: &disabled, + } +} + +func getCertificateExtension(disabled bool, shootIssuersEnabled bool) v1beta1.Extension { + json := fmt.Sprintf(`{apiVersion: "service.cert.extensions.gardener.cloud/v1alpha1", kind: "CertConfig", shootIssuers: {enabled: %v}}`, shootIssuersEnabled) + + return v1beta1.Extension{ + Type: "shoot-cert-service", + Disabled: &disabled, + ProviderConfig: &runtime.RawExtension{ + Raw: []byte(json), + }, + } +} diff --git a/hack/shoot-comparator/pkg/shoot/matcher.go b/hack/shoot-comparator/pkg/shoot/matcher.go index 3b6606b6..d4d3bd71 100644 --- a/hack/shoot-comparator/pkg/shoot/matcher.go +++ b/hack/shoot-comparator/pkg/shoot/matcher.go @@ -104,7 +104,7 @@ func (m *Matcher) Match(actual interface{}) (success bool, err error) { path: "spec/DNS", }, { - GomegaMatcher: gomega.BeComparableTo(eShoot.Spec.Extensions), + GomegaMatcher: NewExtensionMatcher(eShoot.Spec.Extensions), expected: aShoot.Spec.Extensions, path: "spec/Extensions", }, diff --git a/internal/gardener/shoot/converter.go b/internal/gardener/shoot/converter.go index 2767ad87..d8195f44 100644 --- a/internal/gardener/shoot/converter.go +++ b/internal/gardener/shoot/converter.go @@ -108,7 +108,7 @@ func (c Converter) ToShoot(runtime imv1.Runtime) (gardener.Shoot, error) { Pods: &runtime.Spec.Shoot.Networking.Pods, Services: &runtime.Spec.Shoot.Networking.Services, }, - ControlPlane: &runtime.Spec.Shoot.ControlPlane, + ControlPlane: runtime.Spec.Shoot.ControlPlane, }, } From 446918efc0ad1bf380a4582f4bd2ff9859bfa5a3 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Mon, 12 Aug 2024 10:59:43 +0200 Subject: [PATCH 04/53] Comment updated plus warning fixed --- hack/runtime-migrator/main.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/hack/runtime-migrator/main.go b/hack/runtime-migrator/main.go index 1f783e22..ca5adcef 100644 --- a/hack/runtime-migrator/main.go +++ b/hack/runtime-migrator/main.go @@ -290,13 +290,13 @@ func getControlPlane(shoot v1beta1.Shoot) *v1beta1.ControlPlane { } func getAdministratorsList(ctx context.Context, provider kubeconfig.Provider, shootName string) []string { - var kubeconfig, err = provider.Fetch(ctx, shootName) - if kubeconfig == "" { + var clusterKubeconfig, err = provider.Fetch(ctx, shootName) + if clusterKubeconfig == "" { log.Printf("Failed to get dynamic kubeconfig for shoot %s, %s\n", shootName, err.Error()) return []string{} } - restClientConfig, err := clientcmd.RESTConfigFromKubeConfig([]byte(kubeconfig)) + restClientConfig, err := clientcmd.RESTConfigFromKubeConfig([]byte(clusterKubeconfig)) if err != nil { log.Printf("Failed to create REST client from kubeconfig - %s\n", err) return []string{} @@ -311,7 +311,8 @@ func getAdministratorsList(ctx context.Context, provider kubeconfig.Provider, sh LabelSelector: "reconciler.kyma-project.io/managed-by=reconciler,app=kyma", }) - var subjects = []string{} + subjects := make([]string, 0) + for _, clusterRoleBinding := range clusterRoleBindings.Items { for _, subject := range clusterRoleBinding.Subjects { subjects = append(subjects, subject.Name) @@ -416,7 +417,7 @@ func getAllRuntimeLabels(ctx context.Context, shoot v1beta1.Shoot, getClient mig enrichedRuntimeLabels["kyma-project.io/region"] = gardenerCluster.Labels["kyma-project.io/region"] enrichedRuntimeLabels["kyma-project.io/shoot-name"] = gardenerCluster.Labels["kyma-project.io/shoot-name"] enrichedRuntimeLabels["operator.kyma-project.io/kyma-name"] = gardenerCluster.Labels["operator.kyma-project.io/kyma-name"] - // The runtime CR should not be controlled by the KIM + // The runtime CR should be controlled by the KIM enrichedRuntimeLabels["kyma-project.io/controlled-by-provisioner"] = "false" // add custom label for the migrator enrichedRuntimeLabels[migratorLabel] = "true" From de49257890c7b02d34fa62735097c071beea6b80 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Mon, 12 Aug 2024 12:42:50 +0200 Subject: [PATCH 05/53] Extension matcher simplified --- .../pkg/shoot/extensionmatcher.go | 66 ++++++------------- 1 file changed, 21 insertions(+), 45 deletions(-) diff --git a/hack/shoot-comparator/pkg/shoot/extensionmatcher.go b/hack/shoot-comparator/pkg/shoot/extensionmatcher.go index 1421964d..fa7a1179 100644 --- a/hack/shoot-comparator/pkg/shoot/extensionmatcher.go +++ b/hack/shoot-comparator/pkg/shoot/extensionmatcher.go @@ -6,6 +6,7 @@ import ( "github.com/onsi/gomega" "github.com/onsi/gomega/types" "reflect" + "sort" "strings" ) @@ -21,7 +22,6 @@ func NewExtensionMatcher(i interface{}) types.GomegaMatcher { } func (m *ExtensionMatcher) Match(actual interface{}) (success bool, err error) { - aExtensions, err := getExtension(actual) if err != nil { return false, err @@ -32,48 +32,34 @@ func (m *ExtensionMatcher) Match(actual interface{}) (success bool, err error) { return false, err } - if len(aExtensions) != len(eExtensions) { - m.fails = append(m.fails, "Extensions count mismatch") - return false, nil - } + sort.Sort(Extensions(aExtensions)) + sort.Sort(Extensions(eExtensions)) - if len(aExtensions) == 0 && len(eExtensions) == 0 { - return true, nil - } + return gomega.BeComparableTo(eExtensions).Match(aExtensions) +} - findExtension := func(name string, extensions []v1beta1.Extension) v1beta1.Extension { - for _, e := range extensions { - if e.Type == name { - return e - } - } +func getExtension(i interface{}) ([]v1beta1.Extension, error) { + switch v := i.(type) { + case []v1beta1.Extension: + return v, nil - return v1beta1.Extension{} + default: + return []v1beta1.Extension{}, fmt.Errorf(`%w: %s`, errInvalidType, reflect.TypeOf(v)) } +} - differenceFound := false - - for _, e := range eExtensions { - a := findExtension(e.Type, aExtensions) - if a.Type == "" { - m.fails = append(m.fails, fmt.Sprintf("Extension %s not found in both expected and actual", e.Type)) - return false, nil - } - - matcher := gomega.BeComparableTo(e) +type Extensions []v1beta1.Extension - ok, err := matcher.Match(a) - if err != nil { - return false, err - } +func (e Extensions) Len() int { + return len(e) +} - if !ok { - differenceFound = true - m.fails = append(m.fails, matcher.FailureMessage(a)) - } - } +func (e Extensions) Less(i, j int) bool { + return e[i].Type < e[j].Type +} - return !differenceFound, nil +func (e Extensions) Swap(i, j int) { + e[i], e[j] = e[j], e[i] } func (m *ExtensionMatcher) NegatedFailureMessage(_ interface{}) string { @@ -83,13 +69,3 @@ func (m *ExtensionMatcher) NegatedFailureMessage(_ interface{}) string { func (m *ExtensionMatcher) FailureMessage(_ interface{}) string { return strings.Join(m.fails, "\n") } - -func getExtension(i interface{}) ([]v1beta1.Extension, error) { - switch v := i.(type) { - case []v1beta1.Extension: - return v, nil - - default: - return []v1beta1.Extension{}, fmt.Errorf(`%w: %s`, errInvalidType, reflect.TypeOf(v)) - } -} From 2186d7f10dff3992a9b16af0ffd1b60e84c5d7f8 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Mon, 12 Aug 2024 12:50:36 +0200 Subject: [PATCH 06/53] Extension Matcher test minor fix --- hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go b/hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go index f6c7c486..ed8ffb36 100644 --- a/hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go +++ b/hack/shoot-comparator/pkg/shoot/extensionmatcher_test.go @@ -47,13 +47,13 @@ var _ = Describe(":: extension matcher :: ", func() { false, ), Entry( - "should detect missing extension", + "should detect redundant extension", []v1beta1.Extension{dnsExtEnabled, networkingExtDisabled, certificateExtEnabled}, []v1beta1.Extension{networkingExtDisabled, certificateExtEnabled}, false, ), Entry( - "should detect redundant extension", + "should detect missing extension", []v1beta1.Extension{networkingExtDisabled, certificateExtEnabled}, []v1beta1.Extension{dnsExtEnabled, networkingExtDisabled, certificateExtEnabled}, false, From 98505c0c5d355c4c12cf6748e858ad6294e8cfc9 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Mon, 12 Aug 2024 14:19:42 +0200 Subject: [PATCH 07/53] Fix for unit tests plus Control Plane is optional --- api/v1/runtime_types.go | 2 +- .../bases/infrastructuremanager.kyma-project.io_runtimes.yaml | 1 - internal/gardener/shoot/converter_test.go | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/api/v1/runtime_types.go b/api/v1/runtime_types.go index 1ab41cbe..39137abf 100644 --- a/api/v1/runtime_types.go +++ b/api/v1/runtime_types.go @@ -149,7 +149,7 @@ type RuntimeShoot struct { Kubernetes Kubernetes `json:"kubernetes,omitempty"` Provider Provider `json:"provider"` Networking Networking `json:"networking"` - ControlPlane *gardener.ControlPlane `json:"controlPlane"` + ControlPlane *gardener.ControlPlane `json:"controlPlane,omitempty"` } type Kubernetes struct { diff --git a/config/crd/bases/infrastructuremanager.kyma-project.io_runtimes.yaml b/config/crd/bases/infrastructuremanager.kyma-project.io_runtimes.yaml index 5ca78a3b..034516ab 100644 --- a/config/crd/bases/infrastructuremanager.kyma-project.io_runtimes.yaml +++ b/config/crd/bases/infrastructuremanager.kyma-project.io_runtimes.yaml @@ -997,7 +997,6 @@ spec: secretBindingName: type: string required: - - controlPlane - name - networking - platformRegion diff --git a/internal/gardener/shoot/converter_test.go b/internal/gardener/shoot/converter_test.go index b6ad26ee..78dbb54f 100644 --- a/internal/gardener/shoot/converter_test.go +++ b/internal/gardener/shoot/converter_test.go @@ -111,7 +111,7 @@ func fixRuntime() imv1.Runtime { Nodes: "10.250.0.0/16", Services: "100.104.0.0/13", }, - ControlPlane: gardener.ControlPlane{ + ControlPlane: &gardener.ControlPlane{ HighAvailability: &gardener.HighAvailability{ FailureTolerance: gardener.FailureTolerance{ Type: gardener.FailureToleranceTypeZone, From 3f030c5115d7845b6ab44abf227c1c3b9d762571 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Mon, 12 Aug 2024 15:04:28 +0200 Subject: [PATCH 08/53] Updated image of the comparator job --- hack/runtime-migrator/input/runtimeids_sample.json | 3 +-- hack/shoot-comparator/scripts/manifests/job.yaml | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/hack/runtime-migrator/input/runtimeids_sample.json b/hack/runtime-migrator/input/runtimeids_sample.json index e9a0cc0d..5425693e 100644 --- a/hack/runtime-migrator/input/runtimeids_sample.json +++ b/hack/runtime-migrator/input/runtimeids_sample.json @@ -1,4 +1,3 @@ { - c65d5c9b-b321-4481-b567-d331d769ff1c, - 61e18f65-6c82-4fdd-97dc-cf51bfbbbeb2 + 7c6b4bd3-09d3-462f-b27c-992727b62eec, } diff --git a/hack/shoot-comparator/scripts/manifests/job.yaml b/hack/shoot-comparator/scripts/manifests/job.yaml index 8b14c190..32a23102 100644 --- a/hack/shoot-comparator/scripts/manifests/job.yaml +++ b/hack/shoot-comparator/scripts/manifests/job.yaml @@ -47,7 +47,7 @@ spec: # You can specify a date when the comparison should start from. The date should be in RFC3339 format. # - --fromDate # - 2024-07-31T20:04:29Z - image: europe-docker.pkg.dev/kyma-project/dev/shoot-comparator:PR-321 + image: europe-docker.pkg.dev/kyma-project/dev/shoot-comparator:PR-329 name: compare-shoots resources: {} securityContext: From 1b09b24dbeb45aa7939ddb55d8f9a65f0c5fc8e7 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Mon, 12 Aug 2024 15:14:53 +0200 Subject: [PATCH 09/53] Unit test fix --- internal/gardener/shoot/converter_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gardener/shoot/converter_test.go b/internal/gardener/shoot/converter_test.go index 78dbb54f..d0be7181 100644 --- a/internal/gardener/shoot/converter_test.go +++ b/internal/gardener/shoot/converter_test.go @@ -30,7 +30,7 @@ func TestConverter(t *testing.T) { assert.Equal(t, runtime.Spec.Shoot.Purpose, *shoot.Spec.Purpose) assert.Equal(t, runtime.Spec.Shoot.Region, shoot.Spec.Region) assert.Equal(t, runtime.Spec.Shoot.SecretBindingName, *shoot.Spec.SecretBindingName) - assert.Equal(t, runtime.Spec.Shoot.ControlPlane, *shoot.Spec.ControlPlane) + assert.Equal(t, runtime.Spec.Shoot.ControlPlane, shoot.Spec.ControlPlane) assert.Equal(t, runtime.Spec.Shoot.Networking.Nodes, *shoot.Spec.Networking.Nodes) assert.Equal(t, runtime.Spec.Shoot.Networking.Pods, *shoot.Spec.Networking.Pods) assert.Equal(t, runtime.Spec.Shoot.Networking.Services, *shoot.Spec.Networking.Services) From e4f5f53c3db31c8aa5b639bef6c02c33e4731673 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 16 Aug 2024 10:32:21 +0200 Subject: [PATCH 10/53] Improved error description in status --- internal/controller/runtime/fsm/runtime_fsm_create_shoot.go | 3 ++- .../controller/runtime/fsm/runtime_fsm_delete_kubeconfig.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_create_shoot.go b/internal/controller/runtime/fsm/runtime_fsm_create_shoot.go index 7f6ea9b6..9269c07c 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_create_shoot.go +++ b/internal/controller/runtime/fsm/runtime_fsm_create_shoot.go @@ -2,6 +2,7 @@ package fsm import ( "context" + "fmt" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" ctrl "sigs.k8s.io/controller-runtime" @@ -29,7 +30,7 @@ func sFnCreateShoot(ctx context.Context, m *fsm, s *systemState) (stateFn, *ctrl imv1.ConditionTypeRuntimeProvisioned, imv1.ConditionReasonGardenerError, "False", - "Gardener API create error", + fmt.Sprintf("Gardener API create error: %v", err), ) return updateStatusAndRequeueAfter(gardenerRequeueDuration) } diff --git a/internal/controller/runtime/fsm/runtime_fsm_delete_kubeconfig.go b/internal/controller/runtime/fsm/runtime_fsm_delete_kubeconfig.go index 91bd763a..5208322e 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_delete_kubeconfig.go +++ b/internal/controller/runtime/fsm/runtime_fsm_delete_kubeconfig.go @@ -2,6 +2,7 @@ package fsm import ( "context" + "fmt" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -51,7 +52,7 @@ func sFnDeleteKubeconfig(ctx context.Context, m *fsm, s *systemState) (stateFn, imv1.ConditionTypeRuntimeDeprovisioned, imv1.ConditionReasonGardenerError, "False", - "Gardener API shoot delete error", + fmt.Sprintf("Gardener API delete error: %v", err), ) } else { s.instance.UpdateStateDeletion( From 0231028cc518e38329773f9d0c0d0cc6f235f721 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 16 Aug 2024 13:17:10 +0200 Subject: [PATCH 11/53] Fixed issues in AWS examples --- .../assets/runtime-examples/aws-freemium.yaml | 12 ++++++----- .../assets/runtime-examples/aws-minimal.yaml | 21 +++++++++++++------ .../assets/runtime-examples/aws-trial.yaml | 19 ++++++++++++----- docs/adr/assets/runtime-examples/aws.yaml | 14 +++++++------ 4 files changed, 44 insertions(+), 22 deletions(-) diff --git a/docs/adr/assets/runtime-examples/aws-freemium.yaml b/docs/adr/assets/runtime-examples/aws-freemium.yaml index 86733d7e..aa7d8c79 100644 --- a/docs/adr/assets/runtime-examples/aws-freemium.yaml +++ b/docs/adr/assets/runtime-examples/aws-freemium.yaml @@ -1,22 +1,23 @@ -apiVersion: infrastructuremanager.kyma-project.io/v1alpha1 +apiVersion: infrastructuremanager.kyma-project.io/v1 kind: Runtime metadata: labels: + kyma-project.io/controlled-by-provisioner: "false" kyma-project.io/instance-id: instance-id kyma-project.io/runtime-id: runtime-id kyma-project.io/broker-plan-id: plan-id kyma-project.io/broker-plan-name: plan-name kyma-project.io/global-account-id: global-account-id kyma-project.io/subaccount-id: subAccount-id - kyma-project.io/shoot-name: shoot-name + kyma-project.io/shoot-name: aws-fremium kyma-project.io/region: region operator.kyma-project.io/kyma-name: kymaName - name: runtime-id + name: aws-fremium namespace: kcp-system spec: shoot: # spec.shoot.name is required - name: shoot-name + name: aws-fremium # spec.shoot.purpose is required purpose: evaluation # spec.shoot.region is required @@ -47,7 +48,7 @@ spec: # Will be modified by the SRE image: name: gardenlinux - version: 1312.3.0 + version: 1443.9.0 # spec.shoot.workers.volume is required for the first release # Probably can be moved into KIM, as it is hardcoded in KEB, and not dependent on plan volume: @@ -70,6 +71,7 @@ spec: maxUnavailable: 0 # spec.shoot.Networking is required networking: + type: calico pods: 100.64.0.0/12 nodes: 10.250.0.0/16 services: 100.104.0.0/13 diff --git a/docs/adr/assets/runtime-examples/aws-minimal.yaml b/docs/adr/assets/runtime-examples/aws-minimal.yaml index 10e4958c..8d1b5959 100644 --- a/docs/adr/assets/runtime-examples/aws-minimal.yaml +++ b/docs/adr/assets/runtime-examples/aws-minimal.yaml @@ -1,22 +1,23 @@ -apiVersion: infrastructuremanager.kyma-project.io/v1alpha1 +apiVersion: infrastructuremanager.kyma-project.io/v1 kind: Runtime metadata: labels: + kyma-project.io/controlled-by-provisioner: "false" kyma-project.io/instance-id: instance-id kyma-project.io/runtime-id: runtime-id kyma-project.io/broker-plan-id: plan-id kyma-project.io/broker-plan-name: plan-name kyma-project.io/global-account-id: global-account-id kyma-project.io/subaccount-id: subAccount-id - kyma-project.io/shoot-name: shoot-name + kyma-project.io/shoot-name: aws-minimal kyma-project.io/region: region operator.kyma-project.io/kyma-name: kymaName - name: runtime-id + name: aws-minimal namespace: kcp-system spec: shoot: # spec.shoot.name is required - name: shoot-name + name: aws-minimal # spec.shoot.purpose is required purpose: production # spec.shoot.region is required @@ -41,8 +42,12 @@ spec: # spec.shoot.provider.workers is required workers: - machine: - # spec.shoot.workers.machine.type is required - type: m6i.large + # spec.shoot.workers.machine.type is required + type: m6i.large + image: + name: gardenlinux + version: 1443.9.0 + name: "worker-0" # spec.shoot.workers.zones is required zones: - eu-central-1a @@ -58,8 +63,12 @@ spec: # spec.shoot.workers.maxUnavailable is required in the first release. # It can be optional in the future, as it is always set to 0 maxUnavailable: 0 + volume: + type: gp2 + size: 50Gi # spec.shoot.Networking is required networking: + type: calico pods: 100.64.0.0/12 nodes: 10.250.0.0/16 services: 100.104.0.0/13 diff --git a/docs/adr/assets/runtime-examples/aws-trial.yaml b/docs/adr/assets/runtime-examples/aws-trial.yaml index 15df3583..6092a179 100644 --- a/docs/adr/assets/runtime-examples/aws-trial.yaml +++ b/docs/adr/assets/runtime-examples/aws-trial.yaml @@ -1,22 +1,23 @@ -apiVersion: infrastructuremanager.kyma-project.io/v1alpha1 +apiVersion: infrastructuremanager.kyma-project.io/v1 kind: Runtime metadata: labels: + kyma-project.io/controlled-by-provisioner: "false" kyma-project.io/instance-id: instance-id kyma-project.io/runtime-id: runtime-id kyma-project.io/broker-plan-id: plan-id kyma-project.io/broker-plan-name: plan-name kyma-project.io/global-account-id: global-account-id kyma-project.io/subaccount-id: subAccount-id - kyma-project.io/shoot-name: shoot-name + kyma-project.io/shoot-name: aws-trial kyma-project.io/region: region operator.kyma-project.io/kyma-name: kymaName - name: runtime-id + name: aws-trial namespace: kcp-system spec: shoot: # spec.shoot.name is required - name: shoot-name + name: aws-trial # spec.shoot.purpose is required purpose: evaluation # spec.shoot.licenceType is optional, default=nil @@ -44,7 +45,11 @@ spec: workers: - machine: # spec.shoot.workers.machine.type is required - type: mx5.large + type: m6i.large + image: + name: gardenlinux + version: 1443.9.0 + name: "worker-0" # spec.shoot.workers.zones is required zones: - eu-central-1b @@ -58,8 +63,12 @@ spec: # spec.shoot.workers.maxUnavailable is required in the first release. # It can be optional in the future, as it is always set to 0 maxUnavailable: 0 + volume: + type: gp2 + size: 50Gi # spec.shoot.Networking is required networking: + type: calico pods: 10.96.0.0/13 nodes: 10.250.0.0/22 services: 10.104.0.0/13 diff --git a/docs/adr/assets/runtime-examples/aws.yaml b/docs/adr/assets/runtime-examples/aws.yaml index 9761e722..665d9d92 100644 --- a/docs/adr/assets/runtime-examples/aws.yaml +++ b/docs/adr/assets/runtime-examples/aws.yaml @@ -1,22 +1,23 @@ -apiVersion: infrastructuremanager.kyma-project.io/v1alpha1 +apiVersion: infrastructuremanager.kyma-project.io/v1 kind: Runtime metadata: labels: + kyma-project.io/controlled-by-provisioner: "false" kyma-project.io/instance-id: instance-id kyma-project.io/runtime-id: runtime-id kyma-project.io/broker-plan-id: plan-id kyma-project.io/broker-plan-name: plan-name kyma-project.io/global-account-id: global-account-id kyma-project.io/subaccount-id: subAccount-id - kyma-project.io/shoot-name: shoot-name + kyma-project.io/shoot-name: aws-full kyma-project.io/region: region operator.kyma-project.io/kyma-name: kymaName - name: runtime-id + name: aws-full namespace: kcp-system spec: shoot: # spec.shoot.name is required - name: shoot-name + name: aws-full # spec.shoot.purpose is required purpose: production # spec.shoot.region is required @@ -26,7 +27,7 @@ spec: # spec.shoot.secretBindingName is required secretBindingName: "hyperscaler secret" # spec.shoot.enforceSeedLocation is optional ; it allows to make sure the seed cluster will be located in the same region as the runtime - enforceSeedLocation: "true" + enforceSeedLocation: true kubernetes: # spec.shoot.kubernetes.version is optional, when not provided default will be used # Will be modified by the SRE @@ -61,7 +62,7 @@ spec: # Will be modified by the SRE image: name: gardenlinux - version: 1312.3.0 + version: 1443.9.0 # spec.shoot.workers.volume is required for the first release # Probably can be moved into KIM, as it is hardcoded in KEB, and not dependent on plan volume: @@ -86,6 +87,7 @@ spec: maxUnavailable: 0 # spec.shoot.Networking is required networking: + type: calico pods: 100.64.0.0/12 nodes: 10.250.0.0/16 services: 100.104.0.0/13 From 5ede97f7cfb20b47b8e8e5d841f8091d70a5af16 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 16 Aug 2024 15:39:54 +0200 Subject: [PATCH 12/53] Fixed issues in AWS examples --- .../adr/assets/runtime-examples/aws-freemium.yaml | 12 ++++++------ docs/adr/assets/runtime-examples/aws-minimal.yaml | 4 +++- docs/adr/assets/runtime-examples/aws-trial.yaml | 2 ++ docs/adr/assets/runtime-examples/aws.yaml | 15 +++++++-------- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/docs/adr/assets/runtime-examples/aws-freemium.yaml b/docs/adr/assets/runtime-examples/aws-freemium.yaml index aa7d8c79..defde4b0 100644 --- a/docs/adr/assets/runtime-examples/aws-freemium.yaml +++ b/docs/adr/assets/runtime-examples/aws-freemium.yaml @@ -43,17 +43,12 @@ spec: workers: - machine: # spec.shoot.workers.machine.type is required - type: m5.xlarge + type: m6i.xlarge # spec.shoot.workers.machine.image is optional, when not provider default will be used # Will be modified by the SRE image: name: gardenlinux version: 1443.9.0 - # spec.shoot.workers.volume is required for the first release - # Probably can be moved into KIM, as it is hardcoded in KEB, and not dependent on plan - volume: - type: gp2 - size: 50Gi # spec.shoot.worker.zones is required zones: - eu-central-1b @@ -69,6 +64,11 @@ spec: # spec.shoot.workers.maxUnavailable is required in the first release. # It can be optional in the future, as it is always set to 0 maxUnavailable: 0 + # spec.shoot.workers.volume is required for the first release + # Probably can be moved into KIM, as it is hardcoded in KEB, and not dependent on plan + volume: + type: gp2 + size: 50Gi # spec.shoot.Networking is required networking: type: calico diff --git a/docs/adr/assets/runtime-examples/aws-minimal.yaml b/docs/adr/assets/runtime-examples/aws-minimal.yaml index 8d1b5959..91915569 100644 --- a/docs/adr/assets/runtime-examples/aws-minimal.yaml +++ b/docs/adr/assets/runtime-examples/aws-minimal.yaml @@ -63,6 +63,8 @@ spec: # spec.shoot.workers.maxUnavailable is required in the first release. # It can be optional in the future, as it is always set to 0 maxUnavailable: 0 + # spec.shoot.workers.volume is required for the first release + # Probably can be moved into KIM, as it is hardcoded in KEB, and not dependent on plan volume: type: gp2 size: 50Gi @@ -80,7 +82,7 @@ spec: security: networking: filter: - # spec.security.networking is required + # spec.security.networking.filter.egress.enabled is required egress: enabled: false # spec.security.administrators is required diff --git a/docs/adr/assets/runtime-examples/aws-trial.yaml b/docs/adr/assets/runtime-examples/aws-trial.yaml index 6092a179..108c53ff 100644 --- a/docs/adr/assets/runtime-examples/aws-trial.yaml +++ b/docs/adr/assets/runtime-examples/aws-trial.yaml @@ -63,6 +63,8 @@ spec: # spec.shoot.workers.maxUnavailable is required in the first release. # It can be optional in the future, as it is always set to 0 maxUnavailable: 0 + # spec.shoot.workers.volume is required for the first release + # Probably can be moved into KIM, as it is hardcoded in KEB, and not dependent on plan volume: type: gp2 size: 50Gi diff --git a/docs/adr/assets/runtime-examples/aws.yaml b/docs/adr/assets/runtime-examples/aws.yaml index 665d9d92..ba459210 100644 --- a/docs/adr/assets/runtime-examples/aws.yaml +++ b/docs/adr/assets/runtime-examples/aws.yaml @@ -23,7 +23,7 @@ spec: # spec.shoot.region is required region: eu-central-1 # spec.shoot.platformRegion is required - platformRegion: "cd-eu11" + platformRegion: "cf-eu11" # spec.shoot.secretBindingName is required secretBindingName: "hyperscaler secret" # spec.shoot.enforceSeedLocation is optional ; it allows to make sure the seed cluster will be located in the same region as the runtime @@ -63,18 +63,12 @@ spec: image: name: gardenlinux version: 1443.9.0 - # spec.shoot.workers.volume is required for the first release - # Probably can be moved into KIM, as it is hardcoded in KEB, and not dependent on plan - volume: - type: gp2 - size: 50Gi + name: "worker-0" # spec.shoot.workers.zones is required zones: - eu-central-1a - eu-central-1b - eu-central-1c - # spec.shoot.workers.name is optional, if not provided default will be used - name: cpu-worker-0 # spec.shoot.workers.minimum is required minimum: 3 # spec.shoot.workers.maximum is required @@ -85,6 +79,11 @@ spec: # spec.shoot.workers.maxUnavailable is required in the first release. # It can be optional in the future, as it is always set to 0 maxUnavailable: 0 + # spec.shoot.workers.volume is required for the first release + # Probably can be moved into KIM, as it is hardcoded in KEB, and not dependent on plan + volume: + type: gp2 + size: 50Gi # spec.shoot.Networking is required networking: type: calico From 5768ddeb372aef5194b4cd5ee5a5f1c54e6baf18 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 16 Aug 2024 17:41:19 +0200 Subject: [PATCH 13/53] Fixed issues in Azure examples --- .../runtime-examples/azure-fremium.yaml | 29 ++++++++++++++----- .../assets/runtime-examples/azure-lite.yaml | 14 +++++---- docs/adr/assets/runtime-examples/azure.yaml | 23 ++++++++------- 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/docs/adr/assets/runtime-examples/azure-fremium.yaml b/docs/adr/assets/runtime-examples/azure-fremium.yaml index ab725d2d..eb2ccc7b 100644 --- a/docs/adr/assets/runtime-examples/azure-fremium.yaml +++ b/docs/adr/assets/runtime-examples/azure-fremium.yaml @@ -1,26 +1,27 @@ -apiVersion: infrastructuremanager.kyma-project.io/v1alpha1 +apiVersion: infrastructuremanager.kyma-project.io/v1 kind: Runtime metadata: labels: + kyma-project.io/controlled-by-provisioner: "false" kyma-project.io/instance-id: instance-id kyma-project.io/runtime-id: runtime-id kyma-project.io/broker-plan-id: plan-id kyma-project.io/broker-plan-name: plan-name kyma-project.io/global-account-id: global-account-id kyma-project.io/subaccount-id: subAccount-id - kyma-project.io/shoot-name: shoot-name + kyma-project.io/shoot-name: azure-fremium kyma-project.io/region: region operator.kyma-project.io/kyma-name: kymaName - name: runtime-id + name: azure-fremium namespace: kcp-system spec: shoot: # spec.shoot.name is set required - name: shoot-name + name: azure-fremium # spec.shoot.purpose is required purpose: evaluation # spec.shoot.region is required - region: eu-central-1 + region: polandcentral # spec.shoot.platformRegion is required platformRegion: "cf-us10" # spec.shoot.secretBindingName is required @@ -41,11 +42,22 @@ spec: # spec.shoot.provider.workers is required workers: - machine: - # spec.shoot.workers.machine.type is required - type: mx5.large + # spec.shoot.workers.machine.type is required + type: Standard_D2s_v5 + # spec.shoot.workers.machine.image is optional, when not provider default will be used + # Will be modified by the SRE + image: + name: gardenlinux + version: 1443.9.0 + name: "worker-0" + # spec.shoot.workers.volume is required for the first release + # Probably can be moved into KIM, as it is hardcoded in KEB, and not dependent on plan + volume: + type: Standard_LRS + size: 50Gi # spec.shoot.worker.zones is required zones: - - 1 + - "1" # spec.shoot.workers.minimum is required minimum: 1 # spec.shoot.workers.maximum is required @@ -58,6 +70,7 @@ spec: maxUnavailable: 0 # spec.shoot.Networking is required networking: + type: calico pods: 10.96.0.0/13 nodes: 10.250.0.0/22 services: 10.104.0.0/13 diff --git a/docs/adr/assets/runtime-examples/azure-lite.yaml b/docs/adr/assets/runtime-examples/azure-lite.yaml index 81098971..81125a6a 100644 --- a/docs/adr/assets/runtime-examples/azure-lite.yaml +++ b/docs/adr/assets/runtime-examples/azure-lite.yaml @@ -1,22 +1,23 @@ -apiVersion: infrastructuremanager.kyma-project.io/v1alpha1 +apiVersion: infrastructuremanager.kyma-project.io/v1 kind: Runtime metadata: labels: + kyma-project.io/controlled-by-provisioner: "false" kyma-project.io/instance-id: instance-id kyma-project.io/runtime-id: runtime-id kyma-project.io/broker-plan-id: plan-id kyma-project.io/broker-plan-name: plan-name kyma-project.io/global-account-id: global-account-id kyma-project.io/subaccount-id: subAccount-id - kyma-project.io/shoot-name: shoot-name + kyma-project.io/shoot-name: azure-lite kyma-project.io/region: region operator.kyma-project.io/kyma-name: kymaName - name: runtime-id + name: azure-lite namespace: kcp-system spec: shoot: # spec.shoot.name is required - name: shoot-name + name: azure-lite # spec.shoot.purpose is required purpose: production # spec.shoot.region is required @@ -50,7 +51,7 @@ spec: # Will be modified by the SRE image: name: gardenlinux - version: 1312.3.0 + version: 1443.9.0 # spec.shoot.workers.volume is required for the first release # Probably can be moved into KIM, as it is hardcoded in KEB, and not dependent on plan volume: @@ -58,7 +59,7 @@ spec: size: 50Gi # spec.shoot.worker.zones is required zones: - - 1 + - "1" # spec.shoot.workers.name is optional, if not provided default will be used name: cpu-worker-0 # spec.shoot.workers.minimum is required @@ -73,6 +74,7 @@ spec: maxUnavailable: 0 # spec.shoot.Networking is required networking: + type: calico pods: 10.96.0.0/13 nodes: 10.250.0.0/22 services: 10.104.0.0/13 diff --git a/docs/adr/assets/runtime-examples/azure.yaml b/docs/adr/assets/runtime-examples/azure.yaml index ef4416d1..f797fb70 100644 --- a/docs/adr/assets/runtime-examples/azure.yaml +++ b/docs/adr/assets/runtime-examples/azure.yaml @@ -1,22 +1,23 @@ -apiVersion: infrastructuremanager.kyma-project.io/v1alpha1 +apiVersion: infrastructuremanager.kyma-project.io/v1 kind: Runtime metadata: labels: + kyma-project.io/controlled-by-provisioner: "false" kyma-project.io/instance-id: instance-id kyma-project.io/runtime-id: runtime-id kyma-project.io/broker-plan-id: plan-id kyma-project.io/broker-plan-name: plan-name kyma-project.io/global-account-id: global-account-id kyma-project.io/subaccount-id: subAccount-id - kyma-project.io/shoot-name: shoot-name + kyma-project.io/shoot-name: azure-full kyma-project.io/region: region operator.kyma-project.io/kyma-name: kymaName - name: runtime-id + name: azure-full namespace: kcp-system spec: shoot: # spec.shoot.name is required - name: shoot-name + name: azure-full # spec.shoot.purpose is required purpose: production # spec.shoot.region is required @@ -26,7 +27,7 @@ spec: # spec.shoot.secretBindingName is required secretBindingName: "hyperscaler secret" # spec.shoot.enforceSeedLocation is optional ; it allows to make sure the seed cluster will be located in the same region as the runtime - enforceSeedLocation: "true" + enforceSeedLocation: true kubernetes: # spec.shoot.kubernetes.version is optional, when not provided default will be used version: "1.28.7" @@ -61,7 +62,8 @@ spec: # Will be modified by the SRE image: name: gardenlinux - version: 1312.3.0 + version: 1443.9.0 + name: "worker-0" # spec.shoot.workers.volume is required for the first release # Probably can be moved into KIM, as it is hardcoded in KEB, and not dependent on plan volume: @@ -69,11 +71,9 @@ spec: size: 50Gi # spec.shoot.workers.zones is required zones: - - 1 - - 2 - - 3 - # spec.shoot.workers.name is optional, if not provided default will be used - name: cpu-worker-0 + - "1" + - "2" + - "3" # spec.shoot.workers.minimum is required minimum: 3 # spec.shoot.workers.maximum is required @@ -86,6 +86,7 @@ spec: maxUnavailable: 0 # spec.shoot.Networking is required networking: + type: calico pods: 10.96.0.0/13 nodes: 10.250.0.0/22 services: 10.104.0.0/13 From 4fa5da64280764427df96cb9cc8367a799a5db60 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 16 Aug 2024 18:03:59 +0200 Subject: [PATCH 14/53] GCP updated --- docs/adr/assets/runtime-examples/gcp.yaml | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/docs/adr/assets/runtime-examples/gcp.yaml b/docs/adr/assets/runtime-examples/gcp.yaml index 48b2db7f..48c60d88 100644 --- a/docs/adr/assets/runtime-examples/gcp.yaml +++ b/docs/adr/assets/runtime-examples/gcp.yaml @@ -1,32 +1,33 @@ -apiVersion: infrastructuremanager.kyma-project.io/v1alpha1 +apiVersion: infrastructuremanager.kyma-project.io/v1 kind: Runtime metadata: labels: + kyma-project.io/controlled-by-provisioner: "false" kyma-project.io/instance-id: instance-id kyma-project.io/runtime-id: runtime-id kyma-project.io/broker-plan-id: plan-id kyma-project.io/broker-plan-name: plan-name kyma-project.io/global-account-id: global-account-id kyma-project.io/subaccount-id: subAccount-id - kyma-project.io/shoot-name: shoot-name + kyma-project.io/shoot-name: gcp-full kyma-project.io/region: region operator.kyma-project.io/kyma-name: kymaName - name: runtime-id + name: gcp-full namespace: kcp-system spec: shoot: # spec.shoot.name is required - name: shoot-name + name: gcp-full # spec.shoot.purpose is required purpose: production # spec.shoot.region is required - region: europe-west3 + region: us-central1 # spec.shoot.platformRegion is required platformRegion: "cf-eu11" # spec.shoot.secretBindingName is required secretBindingName: "hyperscaler secret" # spec.shoot.enforceSeedLocation is optional ; it allows to make sure the seed cluster will be located in the same region as the runtime - enforceSeedLocation: "true" + enforceSeedLocation: true kubernetes: # spec.shoot.kubernetes.version is optional, when not provided default will be used version: "1.28.7" @@ -61,7 +62,7 @@ spec: # Will be modified by the SRE image: name: gardenlinux - version: 1312.3.0 + version: 1443.9.0 # spec.shoot.workers.volume is required for the first release # Probably can be moved into KIM, as it is hardcoded in KEB, and not dependent on plan volume: @@ -69,9 +70,9 @@ spec: size: 50Gi # spec.shoot.workers.zones is required zones: - - europe-west3a - - europe-west3b - - europe-west3c + - us-central1-a + - us-central1-b + - us-central1-c # spec.shoot.workers.name is optional, if not provided default will be used name: cpu-worker-0 # spec.shoot.workers.minimum is required @@ -86,6 +87,7 @@ spec: maxUnavailable: 0 # spec.shoot.Networking is required networking: + type: calico pods: 10.96.0.0/13 nodes: 10.250.0.0/22 services: 10.104.0.0/13 From e20a3a06d1d8eec9fa0509a65ab49fdafe707218 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Fri, 16 Aug 2024 18:20:55 +0200 Subject: [PATCH 15/53] OpenStack updated --- .../assets/runtime-examples/sap-converged-cloud.yaml | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/adr/assets/runtime-examples/sap-converged-cloud.yaml b/docs/adr/assets/runtime-examples/sap-converged-cloud.yaml index 9a61e7c2..a9d4bbfa 100644 --- a/docs/adr/assets/runtime-examples/sap-converged-cloud.yaml +++ b/docs/adr/assets/runtime-examples/sap-converged-cloud.yaml @@ -1,22 +1,23 @@ -apiVersion: infrastructuremanager.kyma-project.io/v1alpha1 +apiVersion: infrastructuremanager.kyma-project.io/v1 kind: Runtime metadata: labels: + kyma-project.io/controlled-by-provisioner: "false" kyma-project.io/instance-id: instance-id kyma-project.io/runtime-id: runtime-id kyma-project.io/broker-plan-id: plan-id kyma-project.io/broker-plan-name: plan-name kyma-project.io/global-account-id: global-account-id kyma-project.io/subaccount-id: subAccount-id - kyma-project.io/shoot-name: shoot-name + kyma-project.io/shoot-name: ops-full kyma-project.io/region: region operator.kyma-project.io/kyma-name: kymaName - name: runtime-id + name: ops-full namespace: kcp-system spec: shoot: # spec.shoot.name is required - name: shoot-name + name: ops-full # spec.shoot.purpose is required purpose: production # spec.shoot.region is required @@ -61,7 +62,7 @@ spec: # Will be modified by the SRE image: name: gardenlinux - version: 1312.3.0 + version: 1443.9.0 # Note: KEB doesn't specify the volume, Gardener defaults used # spec.shoot.workers.zones is optional zones: @@ -82,6 +83,7 @@ spec: maxUnavailable: 0 # spec.shoot.Networking is required networking: + type: calico pods: 10.96.0.0/13 nodes: 10.250.0.0/22 services: 10.104.0.0/13 From 1f8274dfd34a1330d52e93bfe3f797385cc20574 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Wed, 21 Aug 2024 07:55:22 +0200 Subject: [PATCH 16/53] Fix problem with checkin shoot existence --- internal/controller/runtime/fsm/runtime_fsm_take_snapshot.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_take_snapshot.go b/internal/controller/runtime/fsm/runtime_fsm_take_snapshot.go index 527859c1..823ad3f1 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_take_snapshot.go +++ b/internal/controller/runtime/fsm/runtime_fsm_take_snapshot.go @@ -16,7 +16,7 @@ func sFnTakeSnapshot(ctx context.Context, m *fsm, s *systemState) (stateFn, *ctr var shoot gardener_api.Shoot err := m.ShootClient.Get(ctx, types.NamespacedName{ - Name: s.instance.Name, + Name: s.instance.Spec.Shoot.Name, Namespace: m.ShootNamesapace, }, &shoot) From 4844979a0e3965d3b7ceb27b9c64500a18cf1fc2 Mon Sep 17 00:00:00 2001 From: Arkadiusz Galwas Date: Thu, 22 Aug 2024 10:42:58 +0200 Subject: [PATCH 17/53] Fixed indentation is azure-lite.yaml --- docs/adr/assets/runtime-examples/azure-lite.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adr/assets/runtime-examples/azure-lite.yaml b/docs/adr/assets/runtime-examples/azure-lite.yaml index 81125a6a..5928bd6c 100644 --- a/docs/adr/assets/runtime-examples/azure-lite.yaml +++ b/docs/adr/assets/runtime-examples/azure-lite.yaml @@ -37,7 +37,7 @@ spec: issuerURL: https://my.cool.tokens.com signingAlgs: - RS256 - usernameClaim: sub + usernameClaim: sub # spec.shoot.provider is required provider: # spec.shoot.provider.type is required From c05d9959980182e5da448db1594046d1a6b19255 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 14 Aug 2024 13:55:28 +0200 Subject: [PATCH 18/53] Create Audit Log FSM state --- Dockerfile | 1 - api/v1/runtime_types.go | 2 + converter_config.json | 21 -- internal/auditlogging/auditlogging.go | 281 ++++++++++++++++++ .../controller/runtime/fsm/runtime_fsm.go | 3 + .../runtime_fsm_apply_clusterrolebindings.go | 11 +- .../fsm/runtime_fsm_configure_auditlog.go | 58 ++++ internal/gardener/shoot/converter.go | 6 + 8 files changed, 355 insertions(+), 28 deletions(-) delete mode 100644 converter_config.json create mode 100644 internal/auditlogging/auditlogging.go create mode 100644 internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go diff --git a/Dockerfile b/Dockerfile index ba3d69ed..6d06199e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -28,7 +28,6 @@ RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -a -o ma FROM gcr.io/distroless/static:nonroot WORKDIR / COPY --from=builder /project_workspace/manager . -COPY converter_config.json . USER 65532:65532 ENTRYPOINT ["/manager"] diff --git a/api/v1/runtime_types.go b/api/v1/runtime_types.go index 39137abf..30d276cd 100644 --- a/api/v1/runtime_types.go +++ b/api/v1/runtime_types.go @@ -94,6 +94,8 @@ const ( ConditionReasonKubernetesAPIErr = RuntimeConditionReason("KubernetesErr") ConditionReasonSerializationError = RuntimeConditionReason("SerializationErr") ConditionReasonDeleted = RuntimeConditionReason("Deleted") + + ConditionAdministratorsConfigured = RuntimeConditionReason("AdministratorsConfigured") ) //+kubebuilder:object:root=true diff --git a/converter_config.json b/converter_config.json deleted file mode 100644 index 7ecee0c7..00000000 --- a/converter_config.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "kubernetes": { - "defaultVersion": "1.29", - "enableKubernetesVersionAutoUpdate": true, - "enableMachineImageVersionAutoUpdate": false - }, - "dns": { - "secretName": "aws-route53-secret-dev", - "domainPrefix": "dev.kyma.ondemand.com", - "providerType": "aws-route53" - }, - "aws": { - "enableIMDSv2": "true" - }, - "machineImage": { - "defaultVersion": "1312.3.0" - }, - "gardener": { - "projectName": "kyma-dev" - } -} diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go new file mode 100644 index 00000000..36ea0cfa --- /dev/null +++ b/internal/auditlogging/auditlogging.go @@ -0,0 +1,281 @@ +package auditlogging + +import ( + "context" + "encoding/json" + "fmt" + gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" + "github.com/pkg/errors" + v12 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "os" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + auditlogSecretReference = "auditlog-credentials" + auditlogExtensionType = "shoot-auditlog-service" +) + +type AuditLogging interface { + Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) +} + +type auditLogConfigurator interface { + canEnableAuditLogsForShoot(seedName string) bool + getTenantConfigPath() string + getPolicyConfigMapName() string + getSeedObj(seedKey types.NamespacedName, ctx context.Context) (gardener.Seed, error) + getConfigFromFile() (data map[string]map[string]AuditLogData, err error) +} + +type AuditLog struct { + auditLogConfigurator +} + +type auditLogConfig struct { + tenantConfigPath string + policyConfigMapName string + client client.Client +} + +type AuditLogData struct { + TenantID string `json:"tenantID"` + ServiceURL string `json:"serviceURL"` + SecretName string `json:"secretName"` +} + +type AuditlogExtensionConfig struct { + metav1.TypeMeta `json:",inline"` + + // Type is the type of auditlog service provider. + Type string `json:"type"` + // TenantID is the id of the tenant. + TenantID string `json:"tenantID"` + // ServiceURL is the URL of the auditlog service. + ServiceURL string `json:"serviceURL"` + // SecretReferenceName is the name of the reference for the secret containing the auditlog service credentials. + SecretReferenceName string `json:"secretReferenceName"` +} + +func NewAuditLogging(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client) AuditLogging { + return &AuditLog{ + auditLogConfigurator: newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName, k8s), + } +} + +func newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client) auditLogConfigurator { + return &auditLogConfig{ + tenantConfigPath: auditLogTenantConfigPath, + policyConfigMapName: auditLogPolicyConfigMapName, + client: k8s, + } +} + +func (a *auditLogConfig) canEnableAuditLogsForShoot(seedName string) bool { + return seedName != "" && a.tenantConfigPath != "" +} + +func (a *auditLogConfig) getTenantConfigPath() string { + return a.tenantConfigPath +} + +func (a *auditLogConfig) getPolicyConfigMapName() string { + return a.policyConfigMapName +} + +func (a *auditLogConfig) getSeedObj(seedKey types.NamespacedName, ctx context.Context) (gardener.Seed, error) { + var seed gardener.Seed + if err := a.client.Get(ctx, seedKey, &seed); err != nil { + return gardener.Seed{}, err + } + return seed, nil +} + +func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) { + configureAuditPolicy(shoot, al.getPolicyConfigMapName()) + seedName := getSeedName(*shoot) + + if !al.canEnableAuditLogsForShoot(seedName) { + return false, errors.New("Cannot enable Audit Log for shoot: " + shoot.Name) + } + + seedKey := types.NamespacedName{Name: seedName, Namespace: ""} + seed, err := al.getSeedObj(seedKey, ctx) + if err != nil { + return false, errors.Wrap(err, "Cannot get Gardener Seed object") + } + + auditConfigFromFile, err := al.getConfigFromFile() + if err != nil { + return false, errors.Wrap(err, "Cannot get Audit Log config from file") + } + + annotated, err := enableAuditLogs(shoot, auditConfigFromFile, seed.Spec.Provider.Type) + + if err != nil { + return false, errors.Wrap(err, "Error during enabling Audit Logs on shoot: "+shoot.Name) + } + + return annotated, nil +} + +func enableAuditLogs(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string) (bool, error) { + + providerConfig := auditConfigFromFile[providerType] + if providerConfig == nil { + return false, fmt.Errorf("cannot find config for provider %s", providerType) + } + + auditID := shoot.Spec.Region + if auditID == "" { + return false, errors.New("shoot has no region set") + } + + tenant, ok := providerConfig[auditID] + if !ok { + return false, fmt.Errorf("auditlog config for region %s, provider %s is empty", auditID, providerType) + } + + changedExt, err := configureExtension(shoot, tenant) + changedSec := configureSecret(shoot, tenant) + + return changedExt || changedSec, err +} + +func configureExtension(shoot *gardener.Shoot, config AuditLogData) (changed bool, err error) { + changed = false + const ( + extensionKind = "AuditlogConfig" + extensionVersion = "service.auditlog.extensions.gardener.cloud/v1alpha1" + extensionType = "standard" + ) + + ext := findExtension(shoot) + if ext != nil { + cfg := AuditlogExtensionConfig{} + err = json.Unmarshal(ext.ProviderConfig.Raw, &cfg) + if err != nil { + return + } + + if cfg.Kind == extensionKind && + cfg.Type == extensionType && + cfg.TenantID == config.TenantID && + cfg.ServiceURL == config.ServiceURL && + cfg.SecretReferenceName == auditlogSecretReference { + return false, nil + } + } else { + shoot.Spec.Extensions = append(shoot.Spec.Extensions, gardener.Extension{ + Type: auditlogExtensionType, + }) + ext = &shoot.Spec.Extensions[len(shoot.Spec.Extensions)-1] + } + + changed = true + + cfg := AuditlogExtensionConfig{ + TypeMeta: metav1.TypeMeta{ + Kind: extensionKind, + APIVersion: extensionVersion, + }, + Type: extensionType, + TenantID: config.TenantID, + ServiceURL: config.ServiceURL, + SecretReferenceName: auditlogSecretReference, + } + + ext.ProviderConfig = &runtime.RawExtension{} + ext.ProviderConfig.Raw, err = json.Marshal(cfg) + + return +} + +func findExtension(shoot *gardener.Shoot) *gardener.Extension { + for i, e := range shoot.Spec.Extensions { + if e.Type == auditlogExtensionType { + return &shoot.Spec.Extensions[i] + } + } + + return nil +} + +func findSecret(shoot *gardener.Shoot) *gardener.NamedResourceReference { + for i, e := range shoot.Spec.Resources { + if e.Name == auditlogSecretReference { + return &shoot.Spec.Resources[i] + } + } + + return nil +} + +func configureSecret(shoot *gardener.Shoot, config AuditLogData) (changed bool) { + changed = false + + sec := findSecret(shoot) + if sec != nil { + if sec.Name == auditlogSecretReference && + sec.ResourceRef.APIVersion == "v1" && + sec.ResourceRef.Kind == "Secret" && + sec.ResourceRef.Name == config.SecretName { + return false + } + } else { + shoot.Spec.Resources = append(shoot.Spec.Resources, gardener.NamedResourceReference{}) + sec = &shoot.Spec.Resources[len(shoot.Spec.Resources)-1] + } + + changed = true + + sec.Name = auditlogSecretReference + sec.ResourceRef.APIVersion = "v1" + sec.ResourceRef.Kind = "Secret" + sec.ResourceRef.Name = config.SecretName + + return +} + +func (a *auditLogConfig) getConfigFromFile() (data map[string]map[string]AuditLogData, err error) { + file, err := os.Open(a.tenantConfigPath) + + if err != nil { + return nil, err + } + + defer func(file *os.File) { + _ = file.Close() + }(file) + + if err := json.NewDecoder(file).Decode(&data); err != nil { + return nil, err + } + return data, nil +} + +func getSeedName(shoot gardener.Shoot) string { + if shoot.Spec.SeedName != nil { + return *shoot.Spec.SeedName + } + return "" +} + +func configureAuditPolicy(shoot *gardener.Shoot, policyConfigMapName string) { + if shoot.Spec.Kubernetes.KubeAPIServer == nil { + shoot.Spec.Kubernetes.KubeAPIServer = &gardener.KubeAPIServerConfig{} + } + + shoot.Spec.Kubernetes.KubeAPIServer.AuditConfig = newAuditPolicyConfig(policyConfigMapName) +} + +func newAuditPolicyConfig(policyConfigMapName string) *gardener.AuditConfig { + return &gardener.AuditConfig{ + AuditPolicy: &gardener.AuditPolicy{ + ConfigMapRef: &v12.ObjectReference{Name: policyConfigMapName}, + }, + } +} diff --git a/internal/controller/runtime/fsm/runtime_fsm.go b/internal/controller/runtime/fsm/runtime_fsm.go index f3ec2487..7ad12515 100644 --- a/internal/controller/runtime/fsm/runtime_fsm.go +++ b/internal/controller/runtime/fsm/runtime_fsm.go @@ -3,6 +3,7 @@ package fsm import ( "context" "fmt" + "github.com/kyma-project/infrastructure-manager/internal/auditlogging" "io" "reflect" "runtime" @@ -62,6 +63,7 @@ type fsm struct { log logr.Logger K8s RCCfg + auditlogging.AuditLogging } func (m *fsm) Run(ctx context.Context, v imv1.Runtime) (ctrl.Result, error) { @@ -105,5 +107,6 @@ func NewFsm(log logr.Logger, cfg RCCfg, k8s K8s) Fsm { RCCfg: cfg, log: log, K8s: k8s, + AuditLogging: auditlogging.NewAuditLogging(cfg.AuditLog.TenantConfigPath, cfg.AuditLog.PolicyConfigMapName, k8s.Client), } } diff --git a/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go b/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go index a0d995ff..00946e11 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go +++ b/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go @@ -51,12 +51,11 @@ func sFnApplyClusterRoleBindings(ctx context.Context, m *fsm, s *systemState) (s } } - s.instance.UpdateStateReady( - imv1.ConditionTypeRuntimeConfigured, - imv1.ConditionReasonConfigurationCompleted, - "kubeconfig admin access updated", - ) - return updateStatusAndStop() + if !s.instance.IsStateWithConditionAndStatusSet(imv1.RuntimeStatePending, imv1.ConditionTypeRuntimeConfigured, imv1.ConditionAdministratorsConfigured, "Unknown") { + s.instance.UpdateStatePending(imv1.ConditionTypeRuntimeConfigured, imv1.ConditionAdministratorsConfigured, "True", "Cluster Administrators configured") + return updateStatusAndRequeue() + } + return switchState(sFnConfigureAuditLog) } //nolint:gochecknoglobals diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go new file mode 100644 index 00000000..f88b54e3 --- /dev/null +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -0,0 +1,58 @@ +package fsm + +import ( + "context" + imv1 "github.com/kyma-project/infrastructure-manager/api/v1" + 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") + + auditLogConfigWasChanged, err := m.AuditLogging.Enable(ctx, s.shoot) + if err != nil { + m.log.Error(err, "Failed to configure Audit Log") + // Conditions to change + s.instance.UpdateStatePending( + imv1.ConditionTypeRuntimeProvisioned, + imv1.ConditionReasonGardenerError, + "False", + err.Error(), + ) + return updateStatusAndRequeueAfter(gardenerRequeueDuration) + } + + if auditLogConfigWasChanged { + err = m.ShootClient.Update(ctx, s.shoot) + if err != nil { + m.log.Error(err, "Failed to update shoot") + + // Conditions to change + s.instance.UpdateStatePending( + imv1.ConditionTypeRuntimeProvisioned, + imv1.ConditionReasonGardenerError, + "False", + err.Error(), + ) + return updateStatusAndRequeueAfter(gardenerRequeueDuration) + } + m.log.Info("Audit Log configured for shoot:" + s.shoot.Name) + s.instance.UpdateStateReady( + imv1.ConditionTypeRuntimeConfigured, + imv1.ConditionReasonConfigurationCompleted, + "Audit Log configured", + ) + return updateStatusAndStop() + } + + // config audit logow sie nie zmienil nie trzeba aktualizowac shoota + m.log.Info("Audit Log Tenant did not change, skipping update of cluster") + s.instance.UpdateStateReady( + imv1.ConditionTypeRuntimeConfigured, + imv1.ConditionReasonConfigurationCompleted, + "Audit Log configured", + ) + + return updateStatusAndStop() +} diff --git a/internal/gardener/shoot/converter.go b/internal/gardener/shoot/converter.go index d8195f44..dd1f0d3a 100644 --- a/internal/gardener/shoot/converter.go +++ b/internal/gardener/shoot/converter.go @@ -38,6 +38,11 @@ type KubernetesConfig struct { EnableMachineImageVersionAutoUpdate bool `json:"enableMachineImageVersionVersionAutoUpdate"` } +type AuditLogConfig struct { + PolicyConfigMapName string `json:"policyConfigMapName"` + TenantConfigPath string `json:"tenantConfigPath"` +} + type ReaderGetter = func() (io.Reader, error) type ConverterConfig struct { @@ -46,6 +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"` } func (c *ConverterConfig) Load(f ReaderGetter) error { From e1af088ff25a417d7af6f59f451d29dff45208b6 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 19 Aug 2024 10:29:54 +0200 Subject: [PATCH 19/53] Create new FSM states, fix linter --- api/v1/runtime_types.go | 3 ++- internal/auditlogging/auditlogging.go | 12 ++++++------ .../fsm/runtime_fsm_apply_clusterrolebindings.go | 4 ++-- .../runtime/fsm/runtime_fsm_configure_auditlog.go | 13 +++++-------- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/api/v1/runtime_types.go b/api/v1/runtime_types.go index 30d276cd..6a9b4ab0 100644 --- a/api/v1/runtime_types.go +++ b/api/v1/runtime_types.go @@ -95,7 +95,8 @@ const ( ConditionReasonSerializationError = RuntimeConditionReason("SerializationErr") ConditionReasonDeleted = RuntimeConditionReason("Deleted") - ConditionAdministratorsConfigured = RuntimeConditionReason("AdministratorsConfigured") + ConditionReasonAdministratorsConfigured = RuntimeConditionReason("AdministratorsConfigured") + ConditionReasonAuditLogConfigured = RuntimeConditionReason("AuditLogConfigured") ) //+kubebuilder:object:root=true diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index 36ea0cfa..5ccfe7d9 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -4,13 +4,14 @@ import ( "context" "encoding/json" "fmt" + "os" + gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" "github.com/pkg/errors" v12 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "os" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -27,7 +28,7 @@ type auditLogConfigurator interface { canEnableAuditLogsForShoot(seedName string) bool getTenantConfigPath() string getPolicyConfigMapName() string - getSeedObj(seedKey types.NamespacedName, ctx context.Context) (gardener.Seed, error) + getSeedObj(ctx context.Context, seedKey types.NamespacedName) (gardener.Seed, error) getConfigFromFile() (data map[string]map[string]AuditLogData, err error) } @@ -86,7 +87,7 @@ func (a *auditLogConfig) getPolicyConfigMapName() string { return a.policyConfigMapName } -func (a *auditLogConfig) getSeedObj(seedKey types.NamespacedName, ctx context.Context) (gardener.Seed, error) { +func (a *auditLogConfig) getSeedObj(ctx context.Context, seedKey types.NamespacedName) (gardener.Seed, error) { var seed gardener.Seed if err := a.client.Get(ctx, seedKey, &seed); err != nil { return gardener.Seed{}, err @@ -103,7 +104,7 @@ func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, er } seedKey := types.NamespacedName{Name: seedName, Namespace: ""} - seed, err := al.getSeedObj(seedKey, ctx) + seed, err := al.getSeedObj(ctx, seedKey) if err != nil { return false, errors.Wrap(err, "Cannot get Gardener Seed object") } @@ -123,7 +124,6 @@ func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, er } func enableAuditLogs(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string) (bool, error) { - providerConfig := auditConfigFromFile[providerType] if providerConfig == nil { return false, fmt.Errorf("cannot find config for provider %s", providerType) @@ -223,7 +223,7 @@ func configureSecret(shoot *gardener.Shoot, config AuditLogData) (changed bool) sec.ResourceRef.APIVersion == "v1" && sec.ResourceRef.Kind == "Secret" && sec.ResourceRef.Name == config.SecretName { - return false + return } } else { shoot.Spec.Resources = append(shoot.Spec.Resources, gardener.NamedResourceReference{}) diff --git a/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go b/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go index 00946e11..c8c3effd 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go +++ b/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go @@ -51,8 +51,8 @@ func sFnApplyClusterRoleBindings(ctx context.Context, m *fsm, s *systemState) (s } } - if !s.instance.IsStateWithConditionAndStatusSet(imv1.RuntimeStatePending, imv1.ConditionTypeRuntimeConfigured, imv1.ConditionAdministratorsConfigured, "Unknown") { - s.instance.UpdateStatePending(imv1.ConditionTypeRuntimeConfigured, imv1.ConditionAdministratorsConfigured, "True", "Cluster Administrators configured") + if !s.instance.IsStateWithConditionAndStatusSet(imv1.RuntimeStatePending, imv1.ConditionTypeRuntimeConfigured, imv1.ConditionReasonAdministratorsConfigured, "Unknown") { + s.instance.UpdateStatePending(imv1.ConditionTypeRuntimeConfigured, imv1.ConditionReasonAdministratorsConfigured, "True", "Cluster Administrators configured") return updateStatusAndRequeue() } return switchState(sFnConfigureAuditLog) diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index f88b54e3..f4df30e9 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -2,21 +2,20 @@ package fsm import ( "context" + imv1 "github.com/kyma-project/infrastructure-manager/api/v1" 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") auditLogConfigWasChanged, err := m.AuditLogging.Enable(ctx, s.shoot) if err != nil { m.log.Error(err, "Failed to configure Audit Log") - // Conditions to change s.instance.UpdateStatePending( - imv1.ConditionTypeRuntimeProvisioned, - imv1.ConditionReasonGardenerError, + imv1.ConditionTypeRuntimeConfigured, + imv1.ConditionReasonAuditLogConfigured, "False", err.Error(), ) @@ -28,10 +27,9 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, if err != nil { m.log.Error(err, "Failed to update shoot") - // Conditions to change s.instance.UpdateStatePending( - imv1.ConditionTypeRuntimeProvisioned, - imv1.ConditionReasonGardenerError, + imv1.ConditionTypeRuntimeConfigured, + imv1.ConditionReasonAuditLogConfigured, "False", err.Error(), ) @@ -46,7 +44,6 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, return updateStatusAndStop() } - // config audit logow sie nie zmienil nie trzeba aktualizowac shoota m.log.Info("Audit Log Tenant did not change, skipping update of cluster") s.instance.UpdateStateReady( imv1.ConditionTypeRuntimeConfigured, From a2d1df999cd7746dafd5c1f80c3b5eb4d8cb20c1 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 19 Aug 2024 10:35:28 +0200 Subject: [PATCH 20/53] Repair linter --- internal/controller/runtime/fsm/runtime_fsm.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm.go b/internal/controller/runtime/fsm/runtime_fsm.go index 7ad12515..cd3d413f 100644 --- a/internal/controller/runtime/fsm/runtime_fsm.go +++ b/internal/controller/runtime/fsm/runtime_fsm.go @@ -3,12 +3,13 @@ package fsm import ( "context" "fmt" - "github.com/kyma-project/infrastructure-manager/internal/auditlogging" "io" "reflect" "runtime" "time" + "github.com/kyma-project/infrastructure-manager/internal/auditlogging" + "github.com/go-logr/logr" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" "github.com/kyma-project/infrastructure-manager/internal/gardener/shoot" From 425c02f316558b068e51f7528e27a77d9472d875 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 19 Aug 2024 10:42:34 +0200 Subject: [PATCH 21/53] Repair linter among whole project --- internal/controller/runtime/fsm/runtime_fsm.go | 3 +-- .../controller/runtime/fsm/runtime_fsm_apply_crb_test.go | 5 ++--- .../runtime/fsm/runtime_fsm_create_kubeconfig_test.go | 3 +-- .../controller/runtime/fsm/runtime_fsm_persist_shoot_test.go | 2 +- internal/controller/runtime/fsm/utilz_for_test.go | 2 +- internal/controller/runtime/runtime_controller_test.go | 2 +- internal/controller/runtime/suite_test.go | 4 +--- 7 files changed, 8 insertions(+), 13 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm.go b/internal/controller/runtime/fsm/runtime_fsm.go index cd3d413f..5988561d 100644 --- a/internal/controller/runtime/fsm/runtime_fsm.go +++ b/internal/controller/runtime/fsm/runtime_fsm.go @@ -8,10 +8,9 @@ import ( "runtime" "time" - "github.com/kyma-project/infrastructure-manager/internal/auditlogging" - "github.com/go-logr/logr" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" + "github.com/kyma-project/infrastructure-manager/internal/auditlogging" "github.com/kyma-project/infrastructure-manager/internal/gardener/shoot" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" diff --git a/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go b/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go index a9be5c40..28d35f0b 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go @@ -9,12 +9,11 @@ import ( imv1 "github.com/kyma-project/infrastructure-manager/api/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" ) var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() { diff --git a/internal/controller/runtime/fsm/runtime_fsm_create_kubeconfig_test.go b/internal/controller/runtime/fsm/runtime_fsm_create_kubeconfig_test.go index e3478ae0..8cce9d75 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_create_kubeconfig_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_create_kubeconfig_test.go @@ -5,8 +5,6 @@ import ( "fmt" "time" - "k8s.io/utils/ptr" - gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" . "github.com/onsi/ginkgo/v2" //nolint:revive @@ -15,6 +13,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" util "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" ) diff --git a/internal/controller/runtime/fsm/runtime_fsm_persist_shoot_test.go b/internal/controller/runtime/fsm/runtime_fsm_persist_shoot_test.go index d959b76a..aa15d5f7 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_persist_shoot_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_persist_shoot_test.go @@ -3,11 +3,11 @@ package fsm import ( "bytes" "context" - "github.com/kyma-project/infrastructure-manager/internal/gardener/shoot" "io" "time" "github.com/kyma-project/infrastructure-manager/internal/controller/runtime/fsm/testing" + "github.com/kyma-project/infrastructure-manager/internal/gardener/shoot" . "github.com/onsi/ginkgo/v2" //nolint:revive . "github.com/onsi/gomega" //nolint:revive "sigs.k8s.io/yaml" diff --git a/internal/controller/runtime/fsm/utilz_for_test.go b/internal/controller/runtime/fsm/utilz_for_test.go index 375d2a86..3699df6a 100644 --- a/internal/controller/runtime/fsm/utilz_for_test.go +++ b/internal/controller/runtime/fsm/utilz_for_test.go @@ -2,8 +2,8 @@ package fsm import ( "fmt" - "github.com/kyma-project/infrastructure-manager/internal/gardener/shoot" + "github.com/kyma-project/infrastructure-manager/internal/gardener/shoot" . "github.com/onsi/gomega" //nolint:revive "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" diff --git a/internal/controller/runtime/runtime_controller_test.go b/internal/controller/runtime/runtime_controller_test.go index b1aac381..e389746f 100644 --- a/internal/controller/runtime/runtime_controller_test.go +++ b/internal/controller/runtime/runtime_controller_test.go @@ -18,13 +18,13 @@ package runtime import ( "context" - k8serrors "k8s.io/apimachinery/pkg/api/errors" "time" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" . "github.com/onsi/ginkgo/v2" //nolint:revive . "github.com/onsi/gomega" //nolint:revive + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" diff --git a/internal/controller/runtime/suite_test.go b/internal/controller/runtime/suite_test.go index 5670828d..c99fb650 100644 --- a/internal/controller/runtime/suite_test.go +++ b/internal/controller/runtime/suite_test.go @@ -22,8 +22,6 @@ import ( "testing" "time" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - 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/controller/runtime/fsm" @@ -31,7 +29,7 @@ import ( . "github.com/onsi/ginkgo/v2" //nolint:revive . "github.com/onsi/gomega" //nolint:revive rbacv1 "k8s.io/api/rbac/v1" - + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" //nolint:revive "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" From bd3b77027187b97df36a03f6a8856d389f84498e Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 19 Aug 2024 14:13:36 +0200 Subject: [PATCH 22/53] Comment case for testing --- .../runtime/fsm/runtime_fsm_apply_clusterrolebindings.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go b/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go index c8c3effd..ec736560 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go +++ b/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go @@ -51,10 +51,10 @@ func sFnApplyClusterRoleBindings(ctx context.Context, m *fsm, s *systemState) (s } } - if !s.instance.IsStateWithConditionAndStatusSet(imv1.RuntimeStatePending, imv1.ConditionTypeRuntimeConfigured, imv1.ConditionReasonAdministratorsConfigured, "Unknown") { - s.instance.UpdateStatePending(imv1.ConditionTypeRuntimeConfigured, imv1.ConditionReasonAdministratorsConfigured, "True", "Cluster Administrators configured") - return updateStatusAndRequeue() - } + //if !s.instance.IsStateWithConditionAndStatusSet(imv1.RuntimeStatePending, imv1.ConditionTypeRuntimeConfigured, imv1.ConditionReasonAdministratorsConfigured, "Unknown") { + // s.instance.UpdateStatePending(imv1.ConditionTypeRuntimeConfigured, imv1.ConditionReasonAdministratorsConfigured, "True", "Cluster Administrators configured") + // return updateStatusAndRequeue() + //} return switchState(sFnConfigureAuditLog) } From 40aca003cf4e9492100babe017fe6862353840a4 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 19 Aug 2024 14:25:27 +0200 Subject: [PATCH 23/53] Add Gardenert to scheme --- cmd/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/main.go b/cmd/main.go index c89048fc..21677507 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -56,6 +56,7 @@ func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(infrastructuremanagerv1.AddToScheme(scheme)) utilruntime.Must(rbacv1.AddToScheme(scheme)) + utilruntime.Must(v1beta1.AddToScheme(scheme)) //+kubebuilder:scaffold:scheme } @@ -236,6 +237,5 @@ func initGardenerClients(kubeconfigPath string, namespace string) (client.Client if err != nil { return nil, nil, nil, errors.Wrap(err, "failed to register Gardener schema") } - return gardenerClient, shootClient, dynamicKubeconfigAPI, nil } From 45700ed1ad8bbe34a6a6cbeaba615ebd561a3e99 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 19 Aug 2024 14:42:29 +0200 Subject: [PATCH 24/53] Change k8s client --- internal/controller/runtime/fsm/runtime_fsm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm.go b/internal/controller/runtime/fsm/runtime_fsm.go index 5988561d..137a82ba 100644 --- a/internal/controller/runtime/fsm/runtime_fsm.go +++ b/internal/controller/runtime/fsm/runtime_fsm.go @@ -107,6 +107,6 @@ func NewFsm(log logr.Logger, cfg RCCfg, k8s K8s) Fsm { RCCfg: cfg, log: log, K8s: k8s, - AuditLogging: auditlogging.NewAuditLogging(cfg.AuditLog.TenantConfigPath, cfg.AuditLog.PolicyConfigMapName, k8s.Client), + AuditLogging: auditlogging.NewAuditLogging(cfg.AuditLog.TenantConfigPath, cfg.AuditLog.PolicyConfigMapName, k8s.ShootClient), } } From 0715199433168738cd7a86fa67609182d4033258 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 19 Aug 2024 14:53:20 +0200 Subject: [PATCH 25/53] Remove gardenere from runtime schema --- cmd/main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/main.go b/cmd/main.go index 21677507..51f42dc1 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -56,7 +56,6 @@ func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(infrastructuremanagerv1.AddToScheme(scheme)) utilruntime.Must(rbacv1.AddToScheme(scheme)) - utilruntime.Must(v1beta1.AddToScheme(scheme)) //+kubebuilder:scaffold:scheme } From c4e4982abc996779ef21247896c0278e8ae4d6c4 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Tue, 20 Aug 2024 14:50:15 +0200 Subject: [PATCH 26/53] Refactor --- internal/auditlogging/auditlogging.go | 52 ++++++++++++++----- internal/auditlogging/auditlogging_test.go | 1 + .../runtime_fsm_apply_clusterrolebindings.go | 4 -- .../fsm/runtime_fsm_configure_auditlog.go | 26 +--------- 4 files changed, 41 insertions(+), 42 deletions(-) create mode 100644 internal/auditlogging/auditlogging_test.go diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index 5ccfe7d9..445e7fe7 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/go-logr/logr" "os" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" @@ -20,15 +21,19 @@ const ( auditlogExtensionType = "shoot-auditlog-service" ) +//go:generate mockery --name=AuditLogging type AuditLogging interface { - Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) + Enable(ctx context.Context, shoot *gardener.Shoot) error } +//go:generate mockery --name=auditLogConfigurator type auditLogConfigurator interface { canEnableAuditLogsForShoot(seedName string) bool getTenantConfigPath() string getPolicyConfigMapName() string getSeedObj(ctx context.Context, seedKey types.NamespacedName) (gardener.Seed, error) + getLogInstance() logr.Logger + updateShoot(ctx context.Context, shoot *gardener.Shoot) error getConfigFromFile() (data map[string]map[string]AuditLogData, err error) } @@ -40,6 +45,7 @@ type auditLogConfig struct { tenantConfigPath string policyConfigMapName string client client.Client + log logr.Logger } type AuditLogData struct { @@ -61,17 +67,18 @@ type AuditlogExtensionConfig struct { SecretReferenceName string `json:"secretReferenceName"` } -func NewAuditLogging(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client) AuditLogging { +func NewAuditLogging(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client, log logr.Logger) AuditLogging { return &AuditLog{ - auditLogConfigurator: newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName, k8s), + auditLogConfigurator: newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName, k8s, log), } } -func newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client) auditLogConfigurator { +func newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client, log logr.Logger) auditLogConfigurator { return &auditLogConfig{ tenantConfigPath: auditLogTenantConfigPath, policyConfigMapName: auditLogPolicyConfigMapName, client: k8s, + log: log, } } @@ -95,32 +102,41 @@ func (a *auditLogConfig) getSeedObj(ctx context.Context, seedKey types.Namespace return seed, nil } -func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) { - configureAuditPolicy(shoot, al.getPolicyConfigMapName()) +func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) error { + log := al.getLogInstance() seedName := getSeedName(*shoot) if !al.canEnableAuditLogsForShoot(seedName) { - return false, errors.New("Cannot enable Audit Log for shoot: " + shoot.Name) + log.Info("Seed name or Tenant config path is empty while configuring Audit Logs on shoot: " + shoot.Name) + return nil } - seedKey := types.NamespacedName{Name: seedName, Namespace: ""} - seed, err := al.getSeedObj(ctx, seedKey) + auditConfigFromFile, err := al.getConfigFromFile() if err != nil { - return false, errors.Wrap(err, "Cannot get Gardener Seed object") + return errors.Wrap(err, "Cannot get Audit Log config from file") } - auditConfigFromFile, err := al.getConfigFromFile() + configureAuditPolicy(shoot, al.getPolicyConfigMapName()) + + seedKey := types.NamespacedName{Name: seedName, Namespace: ""} + seed, err := al.getSeedObj(ctx, seedKey) if err != nil { - return false, errors.Wrap(err, "Cannot get Audit Log config from file") + return errors.Wrap(err, "Cannot get Gardener Seed object") } annotated, err := enableAuditLogs(shoot, auditConfigFromFile, seed.Spec.Provider.Type) if err != nil { - return false, errors.Wrap(err, "Error during enabling Audit Logs on shoot: "+shoot.Name) + return errors.Wrap(err, "Error during enabling Audit Logs on shoot: "+shoot.Name) } - return annotated, nil + if annotated { + if err = al.updateShoot(ctx, shoot); err != nil { + return errors.Wrap(err, "Cannot update shoot") + } + } + + return nil } func enableAuditLogs(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string) (bool, error) { @@ -279,3 +295,11 @@ func newAuditPolicyConfig(policyConfigMapName string) *gardener.AuditConfig { }, } } + +func (a *auditLogConfig) updateShoot(ctx context.Context, shoot *gardener.Shoot) error { + return a.client.Update(ctx, shoot) +} + +func (a *auditLogConfig) getLogInstance() logr.Logger { + return a.log +} diff --git a/internal/auditlogging/auditlogging_test.go b/internal/auditlogging/auditlogging_test.go new file mode 100644 index 00000000..17139b70 --- /dev/null +++ b/internal/auditlogging/auditlogging_test.go @@ -0,0 +1 @@ +package auditlogging diff --git a/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go b/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go index ec736560..7e7737ed 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go +++ b/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go @@ -51,10 +51,6 @@ func sFnApplyClusterRoleBindings(ctx context.Context, m *fsm, s *systemState) (s } } - //if !s.instance.IsStateWithConditionAndStatusSet(imv1.RuntimeStatePending, imv1.ConditionTypeRuntimeConfigured, imv1.ConditionReasonAdministratorsConfigured, "Unknown") { - // s.instance.UpdateStatePending(imv1.ConditionTypeRuntimeConfigured, imv1.ConditionReasonAdministratorsConfigured, "True", "Cluster Administrators configured") - // return updateStatusAndRequeue() - //} return switchState(sFnConfigureAuditLog) } diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index f4df30e9..8664cfeb 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -10,7 +10,7 @@ import ( func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, *ctrl.Result, error) { m.log.Info("Configure Audit Log state") - auditLogConfigWasChanged, err := m.AuditLogging.Enable(ctx, s.shoot) + err := m.AuditLogging.Enable(ctx, s.shoot) if err != nil { m.log.Error(err, "Failed to configure Audit Log") s.instance.UpdateStatePending( @@ -22,29 +22,7 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, return updateStatusAndRequeueAfter(gardenerRequeueDuration) } - if auditLogConfigWasChanged { - err = m.ShootClient.Update(ctx, s.shoot) - if err != nil { - m.log.Error(err, "Failed to update shoot") - - s.instance.UpdateStatePending( - imv1.ConditionTypeRuntimeConfigured, - imv1.ConditionReasonAuditLogConfigured, - "False", - err.Error(), - ) - return updateStatusAndRequeueAfter(gardenerRequeueDuration) - } - m.log.Info("Audit Log configured for shoot:" + s.shoot.Name) - s.instance.UpdateStateReady( - imv1.ConditionTypeRuntimeConfigured, - imv1.ConditionReasonConfigurationCompleted, - "Audit Log configured", - ) - return updateStatusAndStop() - } - - m.log.Info("Audit Log Tenant did not change, skipping update of cluster") + m.log.Info("Audit Log configured for shoot: " + s.shoot.Name) s.instance.UpdateStateReady( imv1.ConditionTypeRuntimeConfigured, imv1.ConditionReasonConfigurationCompleted, From c923e98bbc62c662e38067a5b990c5f57a85414d Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Tue, 20 Aug 2024 14:51:10 +0200 Subject: [PATCH 27/53] Pass additional parameter --- internal/controller/runtime/fsm/runtime_fsm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm.go b/internal/controller/runtime/fsm/runtime_fsm.go index 137a82ba..82af3f38 100644 --- a/internal/controller/runtime/fsm/runtime_fsm.go +++ b/internal/controller/runtime/fsm/runtime_fsm.go @@ -107,6 +107,6 @@ func NewFsm(log logr.Logger, cfg RCCfg, k8s K8s) Fsm { RCCfg: cfg, log: log, K8s: k8s, - AuditLogging: auditlogging.NewAuditLogging(cfg.AuditLog.TenantConfigPath, cfg.AuditLog.PolicyConfigMapName, k8s.ShootClient), + AuditLogging: auditlogging.NewAuditLogging(cfg.AuditLog.TenantConfigPath, cfg.AuditLog.PolicyConfigMapName, k8s.ShootClient, log), } } From b348fda56c71b11fd647ee596f495241ace856a6 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Fri, 23 Aug 2024 11:55:57 +0200 Subject: [PATCH 28/53] First phase of fixing tests --- internal/auditlogging/auditlogging.go | 16 ++-- .../fsm/runtime_fsm_configure_auditlog.go | 21 +++-- .../runtime/runtime_controller_test.go | 5 +- internal/controller/runtime/suite_test.go | 85 ++++++++++++++++--- .../runtime/test_client_obj_tracker.go | 64 ++++++++++---- 5 files changed, 142 insertions(+), 49 deletions(-) diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index 445e7fe7..454cc3ba 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -23,7 +23,7 @@ const ( //go:generate mockery --name=AuditLogging type AuditLogging interface { - Enable(ctx context.Context, shoot *gardener.Shoot) error + Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) } //go:generate mockery --name=auditLogConfigurator @@ -102,18 +102,18 @@ func (a *auditLogConfig) getSeedObj(ctx context.Context, seedKey types.Namespace return seed, nil } -func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) error { +func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) { log := al.getLogInstance() seedName := getSeedName(*shoot) if !al.canEnableAuditLogsForShoot(seedName) { log.Info("Seed name or Tenant config path is empty while configuring Audit Logs on shoot: " + shoot.Name) - return nil + return false, nil } auditConfigFromFile, err := al.getConfigFromFile() if err != nil { - return errors.Wrap(err, "Cannot get Audit Log config from file") + return false, errors.Wrap(err, "Cannot get Audit Log config from file") } configureAuditPolicy(shoot, al.getPolicyConfigMapName()) @@ -121,22 +121,22 @@ func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) error { seedKey := types.NamespacedName{Name: seedName, Namespace: ""} seed, err := al.getSeedObj(ctx, seedKey) if err != nil { - return errors.Wrap(err, "Cannot get Gardener Seed object") + return false, errors.Wrap(err, "Cannot get Gardener Seed object") } annotated, err := enableAuditLogs(shoot, auditConfigFromFile, seed.Spec.Provider.Type) if err != nil { - return errors.Wrap(err, "Error during enabling Audit Logs on shoot: "+shoot.Name) + return false, errors.Wrap(err, "Error during enabling Audit Logs on shoot: "+shoot.Name) } if annotated { if err = al.updateShoot(ctx, shoot); err != nil { - return errors.Wrap(err, "Cannot update shoot") + return false, errors.Wrap(err, "Cannot update shoot") } } - return nil + return true, nil } func enableAuditLogs(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string) (bool, error) { diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index 8664cfeb..552e3448 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -10,8 +10,8 @@ import ( func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, *ctrl.Result, error) { m.log.Info("Configure Audit Log state") - err := m.AuditLogging.Enable(ctx, s.shoot) - if err != nil { + wasAuditLogEnabled, err := m.AuditLogging.Enable(ctx, s.shoot) + if err != nil && !wasAuditLogEnabled { m.log.Error(err, "Failed to configure Audit Log") s.instance.UpdateStatePending( imv1.ConditionTypeRuntimeConfigured, @@ -22,12 +22,15 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, return updateStatusAndRequeueAfter(gardenerRequeueDuration) } - m.log.Info("Audit Log configured for shoot: " + s.shoot.Name) - s.instance.UpdateStateReady( - imv1.ConditionTypeRuntimeConfigured, - imv1.ConditionReasonConfigurationCompleted, - "Audit Log configured", - ) + if wasAuditLogEnabled { + m.log.Info("Audit Log configured for shoot: " + s.shoot.Name) + s.instance.UpdateStateReady( + imv1.ConditionTypeRuntimeConfigured, + imv1.ConditionReasonConfigurationCompleted, + "Audit Log configured", + ) - return updateStatusAndStop() + return updateStatusAndStop() + } + return requeue() } diff --git a/internal/controller/runtime/runtime_controller_test.go b/internal/controller/runtime/runtime_controller_test.go index e389746f..24d21cb4 100644 --- a/internal/controller/runtime/runtime_controller_test.go +++ b/internal/controller/runtime/runtime_controller_test.go @@ -18,13 +18,13 @@ package runtime import ( "context" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "time" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" . "github.com/onsi/ginkgo/v2" //nolint:revive . "github.com/onsi/gomega" //nolint:revive - k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -34,7 +34,6 @@ var _ = Describe("Runtime Controller", func() { Context("When reconciling a resource", func() { const ResourceName = "test-resource" - ctx := context.Background() typeNamespacedName := types.NamespacedName{ @@ -44,6 +43,7 @@ var _ = Describe("Runtime Controller", func() { It("Should successfully create new Shoot from provided Runtime and set Ready status on CR", func() { setupGardenerTestClientForProvisioning() + Expect(setupSeedObjectOnCluster(gardenerTestClient)).To(Succeed()) By("Create Runtime CR") runtimeStub := CreateRuntimeStub(ResourceName) @@ -256,6 +256,7 @@ func CreateRuntimeStub(resourceName string) *imv1.Runtime { }, }, }, + Region: "eu-central-1", }, Security: imv1.Security{ Administrators: []string{ diff --git a/internal/controller/runtime/suite_test.go b/internal/controller/runtime/suite_test.go index c99fb650..ae329200 100644 --- a/internal/controller/runtime/suite_test.go +++ b/internal/controller/runtime/suite_test.go @@ -18,6 +18,7 @@ package runtime import ( "context" + v12 "k8s.io/api/core/v1" "path/filepath" "testing" "time" @@ -100,10 +101,10 @@ var _ = BeforeSuite(func() { // tracker will be updated with different shoot sequence for each test case tracker := clienttesting.NewObjectTracker(clientScheme, serializer.NewCodecFactory(clientScheme).UniversalDecoder()) - customTracker = NewCustomTracker(tracker, []*gardener_api.Shoot{}) + customTracker = NewCustomTracker(tracker, []*gardener_api.Shoot{}, []*gardener_api.Seed{}) gardenerTestClient = fake.NewClientBuilder().WithScheme(clientScheme).WithObjectTracker(customTracker).Build() - runtimeReconciler = NewRuntimeReconciler(mgr, gardenerTestClient, logger, fsm.RCCfg{Finalizer: infrastructuremanagerv1.Finalizer}) + runtimeReconciler = NewRuntimeReconciler(mgr, gardenerTestClient, logger, fsm.RCCfg{Finalizer: infrastructuremanagerv1.Finalizer, ConverterConfig: fixConverterConfigForTests()}) Expect(runtimeReconciler).NotTo(BeNil()) err = runtimeReconciler.SetupWithManager(mgr) Expect(err).To(BeNil()) @@ -140,27 +141,30 @@ var _ = AfterSuite(func() { func setupGardenerTestClientForProvisioning() { baseShoot := getBaseShootForTestingSequence() shoots := fixShootsSequenceForProvisioning(&baseShoot) - setupShootClientWithSequence(shoots) + seeds := fixSeedsSequence() + setupGardenerClientWithSequence(shoots, seeds) } func setupGardenerTestClientForUpdate() { baseShoot := getBaseShootForTestingSequence() shoots := fixShootsSequenceForUpdate(&baseShoot) - setupShootClientWithSequence(shoots) + seeds := fixSeedsSequence() + setupGardenerClientWithSequence(shoots, seeds) } func setupGardenerTestClientForDelete() { baseShoot := getBaseShootForTestingSequence() shoots := fixShootsSequenceForDelete(&baseShoot) - setupShootClientWithSequence(shoots) + seeds := fixSeedsSequence() + setupGardenerClientWithSequence(shoots, seeds) } -func setupShootClientWithSequence(shoots []*gardener_api.Shoot) { +func setupGardenerClientWithSequence(shoots []*gardener_api.Shoot, seeds []*gardener_api.Seed) { clientScheme := runtime.NewScheme() _ = gardener_api.AddToScheme(clientScheme) tracker := clienttesting.NewObjectTracker(clientScheme, serializer.NewCodecFactory(clientScheme).UniversalDecoder()) - customTracker = NewCustomTracker(tracker, shoots) + customTracker = NewCustomTracker(tracker, shoots, seeds) gardenerTestClient = fake.NewClientBuilder().WithScheme(clientScheme).WithObjectTracker(customTracker).Build() runtimeReconciler.UpdateShootClient(gardenerTestClient) } @@ -195,8 +199,16 @@ func fixShootsSequenceForProvisioning(shoot *gardener_api.Shoot) []*gardener_api }, } - processingShoot := pendingShoot.DeepCopy() + pendingShoot.Spec.SeedName = ptr.To("test-seed") + auditLogShoot := pendingShoot.DeepCopy() + auditLogShoot.Spec.Kubernetes.KubeAPIServer.AuditConfig = &gardener_api.AuditConfig{ + AuditPolicy: &gardener_api.AuditPolicy{ + ConfigMapRef: &v12.ObjectReference{Name: "policy-config-map"}, + }, + } + + processingShoot := auditLogShoot.DeepCopy() processingShoot.Status.LastOperation.State = gardener_api.LastOperationStateProcessing readyShoot := processingShoot.DeepCopy() @@ -205,7 +217,7 @@ func fixShootsSequenceForProvisioning(shoot *gardener_api.Shoot) []*gardener_api // processedShoot := processingShoot.DeepCopy() // will add specific data later - return []*gardener_api.Shoot{missingShoot, missingShoot, missingShoot, initialisedShoot, dnsShoot, pendingShoot, processingShoot, readyShoot, readyShoot, readyShoot, readyShoot} + return []*gardener_api.Shoot{missingShoot, missingShoot, missingShoot, initialisedShoot, dnsShoot, auditLogShoot, auditLogShoot, processingShoot, readyShoot, readyShoot, readyShoot, readyShoot} } func fixShootsSequenceForUpdate(shoot *gardener_api.Shoot) []*gardener_api.Shoot { @@ -215,6 +227,8 @@ func fixShootsSequenceForUpdate(shoot *gardener_api.Shoot) []*gardener_api.Shoot Domain: ptr.To("test.domain"), } + pendingShoot.Spec.SeedName = ptr.To("test-seed") + pendingShoot.Status = gardener_api.ShootStatus{ LastOperation: &gardener_api.LastOperation{ Type: gardener_api.LastOperationTypeReconcile, @@ -222,7 +236,14 @@ func fixShootsSequenceForUpdate(shoot *gardener_api.Shoot) []*gardener_api.Shoot }, } - processingShoot := pendingShoot.DeepCopy() + auditLogShoot := pendingShoot.DeepCopy() + auditLogShoot.Spec.Kubernetes.KubeAPIServer.AuditConfig = &gardener_api.AuditConfig{ + AuditPolicy: &gardener_api.AuditPolicy{ + ConfigMapRef: &v12.ObjectReference{Name: "policy-config-map"}, + }, + } + + processingShoot := auditLogShoot.DeepCopy() processingShoot.Status.LastOperation.State = gardener_api.LastOperationStateProcessing @@ -232,7 +253,7 @@ func fixShootsSequenceForUpdate(shoot *gardener_api.Shoot) []*gardener_api.Shoot // processedShoot := processingShoot.DeepCopy() // will add specific data later - return []*gardener_api.Shoot{pendingShoot, processingShoot, readyShoot, readyShoot} + return []*gardener_api.Shoot{pendingShoot, pendingShoot, auditLogShoot, processingShoot, readyShoot, readyShoot} } func fixShootsSequenceForDelete(shoot *gardener_api.Shoot) []*gardener_api.Shoot { @@ -242,6 +263,8 @@ func fixShootsSequenceForDelete(shoot *gardener_api.Shoot) []*gardener_api.Shoot Domain: ptr.To("test.domain"), } + currentShoot.Spec.SeedName = ptr.To("test-seed") + // To workaround limitation that apply patches are not supported in the fake client. // We need to set the annotation manually. https://github.com/kubernetes/kubernetes/issues/115598 currentShoot.Annotations = map[string]string{ @@ -264,10 +287,40 @@ func fixShootsSequenceForDelete(shoot *gardener_api.Shoot) []*gardener_api.Shoot return []*gardener_api.Shoot{currentShoot, currentShoot, currentShoot, pendingDeleteShoot, nil} } +func fixSeedsSequence() []*gardener_api.Seed { + seed := &gardener_api.Seed{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-seed", + }, + Spec: gardener_api.SeedSpec{ + Provider: gardener_api.SeedProvider{ + Type: "aws", + }, + }, + } + + return []*gardener_api.Seed{seed} +} + +func setupSeedObjectOnCluster(client client.Client) error { + seed := &gardener_api.Seed{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-seed", + }, + Spec: gardener_api.SeedSpec{ + Provider: gardener_api.SeedProvider{ + Type: "aws", + }, + }, + } + + return client.Create(context.Background(), seed) +} + func fixConverterConfigForTests() gardener_shoot.ConverterConfig { return gardener_shoot.ConverterConfig{ Kubernetes: gardener_shoot.KubernetesConfig{ - DefaultVersion: "1.29", //nolint:godox TODO: Should be parametrised + DefaultVersion: "1.29", }, DNS: gardener_shoot.DNSConfig{ @@ -277,11 +330,15 @@ func fixConverterConfigForTests() gardener_shoot.ConverterConfig { }, Provider: gardener_shoot.ProviderConfig{ AWS: gardener_shoot.AWSConfig{ - EnableIMDSv2: true, //nolint:godox TODO: Should be parametrised + EnableIMDSv2: true, }, }, Gardener: gardener_shoot.GardenerConfig{ - ProjectName: "kyma-dev", //nolint:godox TODO: should be parametrised + ProjectName: "kyma-dev", + }, + AuditLog: gardener_shoot.AuditLogConfig{ + PolicyConfigMapName: "policy-config-map", + TenantConfigPath: filepath.Join("testdata", "auditConfig.json"), }, } } diff --git a/internal/controller/runtime/test_client_obj_tracker.go b/internal/controller/runtime/test_client_obj_tracker.go index 65e8c6ba..a72ab0ab 100644 --- a/internal/controller/runtime/test_client_obj_tracker.go +++ b/internal/controller/runtime/test_client_obj_tracker.go @@ -16,23 +16,22 @@ import ( type CustomTracker struct { clienttesting.ObjectTracker shootSequence []*gardener_api.Shoot - callCnt int + seedSequence []*gardener_api.Seed + shootCallCnt int + seedCallCnt int mu sync.Mutex } -func NewCustomTracker(tracker clienttesting.ObjectTracker, shoots []*gardener_api.Shoot) *CustomTracker { +func NewCustomTracker(tracker clienttesting.ObjectTracker, shoots []*gardener_api.Shoot, seeds []*gardener_api.Seed) *CustomTracker { return &CustomTracker{ ObjectTracker: tracker, shootSequence: shoots, + seedSequence: seeds, } } -func (t *CustomTracker) GetCallCnt() int { - return t.callCnt -} - func (t *CustomTracker) IsSequenceFullyUsed() bool { - return t.callCnt == len(t.shootSequence) && len(t.shootSequence) > 0 + return (t.shootCallCnt == len(t.shootSequence) && len(t.shootSequence) > 0) && (t.seedCallCnt == len(t.seedSequence) && len(t.seedSequence) > 0) } func (t *CustomTracker) Get(gvr schema.GroupVersionResource, ns, name string) (runtime.Object, error) { @@ -40,20 +39,53 @@ func (t *CustomTracker) Get(gvr schema.GroupVersionResource, ns, name string) (r defer t.mu.Unlock() if gvr.Resource == "shoots" { - if t.callCnt < len(t.shootSequence) { - shoot := t.shootSequence[t.callCnt] - t.callCnt++ + return getNextShoot(t.shootSequence, t) + } else if gvr.Resource == "seeds" { + return getNextSeed(t.seedSequence, t) + } - if shoot == nil { - return nil, k8serrors.NewNotFound(schema.GroupResource{}, "") - } - return shoot, nil + return t.ObjectTracker.Get(gvr, ns, name) +} + +func getNextSeed(seed []*gardener_api.Seed, t *CustomTracker) (runtime.Object, error) { + if t.seedCallCnt < len(seed) { + obj := seed[t.seedCallCnt] + t.seedCallCnt++ + + if obj == nil { + return nil, k8serrors.NewNotFound(schema.GroupResource{}, "") } - return nil, fmt.Errorf("no more Shoot objects in sequence") + return obj, nil } - return t.ObjectTracker.Get(gvr, ns, name) + return nil, fmt.Errorf("no more seeds in sequence") } +func getNextShoot(shoot []*gardener_api.Shoot, t *CustomTracker) (runtime.Object, error) { + if t.shootCallCnt < len(shoot) { + obj := shoot[t.shootCallCnt] + t.shootCallCnt++ + + if obj == nil { + return nil, k8serrors.NewNotFound(schema.GroupResource{}, "") + } + return obj, nil + } + return nil, fmt.Errorf("no more shoots in sequence") +} + +//func getNextObject[T any](sequence []*T, t *CustomTracker) (*T, error) { +// if t.callCnt < len(sequence) { +// obj := sequence[t.callCnt] +// t.callCnt++ +// +// if obj == nil { +// return nil, k8serrors.NewNotFound(schema.GroupResource{}, "") +// } +// return obj, nil +// } +// return nil, fmt.Errorf("no more objects in sequence") +//} + func (t *CustomTracker) Delete(gvr schema.GroupVersionResource, ns, name string) error { t.mu.Lock() defer t.mu.Unlock() From f431a3bf60d916f9e663193f0b2975522ce3a5c2 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 26 Aug 2024 08:37:54 +0200 Subject: [PATCH 29/53] Second phase of fixing tests --- .../fsm/runtime_fsm_configure_auditlog.go | 2 +- internal/controller/runtime/suite_test.go | 28 ++++--------------- .../runtime/test_client_obj_tracker.go | 8 +++--- 3 files changed, 11 insertions(+), 27 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index 552e3448..b75d8b62 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -11,7 +11,7 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, m.log.Info("Configure Audit Log state") wasAuditLogEnabled, err := m.AuditLogging.Enable(ctx, s.shoot) - if err != nil && !wasAuditLogEnabled { + if err != nil { m.log.Error(err, "Failed to configure Audit Log") s.instance.UpdateStatePending( imv1.ConditionTypeRuntimeConfigured, diff --git a/internal/controller/runtime/suite_test.go b/internal/controller/runtime/suite_test.go index ae329200..f4ded733 100644 --- a/internal/controller/runtime/suite_test.go +++ b/internal/controller/runtime/suite_test.go @@ -18,7 +18,6 @@ package runtime import ( "context" - v12 "k8s.io/api/core/v1" "path/filepath" "testing" "time" @@ -183,6 +182,7 @@ func getBaseShootForTestingSequence() gardener_api.Shoot { func fixShootsSequenceForProvisioning(shoot *gardener_api.Shoot) []*gardener_api.Shoot { var missingShoot *gardener_api.Shoot initialisedShoot := shoot.DeepCopy() + initialisedShoot.Spec.SeedName = ptr.To("test-seed") dnsShoot := initialisedShoot.DeepCopy() @@ -199,16 +199,7 @@ func fixShootsSequenceForProvisioning(shoot *gardener_api.Shoot) []*gardener_api }, } - pendingShoot.Spec.SeedName = ptr.To("test-seed") - - auditLogShoot := pendingShoot.DeepCopy() - auditLogShoot.Spec.Kubernetes.KubeAPIServer.AuditConfig = &gardener_api.AuditConfig{ - AuditPolicy: &gardener_api.AuditPolicy{ - ConfigMapRef: &v12.ObjectReference{Name: "policy-config-map"}, - }, - } - - processingShoot := auditLogShoot.DeepCopy() + processingShoot := pendingShoot.DeepCopy() processingShoot.Status.LastOperation.State = gardener_api.LastOperationStateProcessing readyShoot := processingShoot.DeepCopy() @@ -217,7 +208,7 @@ func fixShootsSequenceForProvisioning(shoot *gardener_api.Shoot) []*gardener_api // processedShoot := processingShoot.DeepCopy() // will add specific data later - return []*gardener_api.Shoot{missingShoot, missingShoot, missingShoot, initialisedShoot, dnsShoot, auditLogShoot, auditLogShoot, processingShoot, readyShoot, readyShoot, readyShoot, readyShoot} + return []*gardener_api.Shoot{missingShoot, missingShoot, missingShoot, initialisedShoot, dnsShoot, pendingShoot, processingShoot, readyShoot, readyShoot, readyShoot, readyShoot, readyShoot} } func fixShootsSequenceForUpdate(shoot *gardener_api.Shoot) []*gardener_api.Shoot { @@ -227,8 +218,6 @@ func fixShootsSequenceForUpdate(shoot *gardener_api.Shoot) []*gardener_api.Shoot Domain: ptr.To("test.domain"), } - pendingShoot.Spec.SeedName = ptr.To("test-seed") - pendingShoot.Status = gardener_api.ShootStatus{ LastOperation: &gardener_api.LastOperation{ Type: gardener_api.LastOperationTypeReconcile, @@ -236,14 +225,9 @@ func fixShootsSequenceForUpdate(shoot *gardener_api.Shoot) []*gardener_api.Shoot }, } - auditLogShoot := pendingShoot.DeepCopy() - auditLogShoot.Spec.Kubernetes.KubeAPIServer.AuditConfig = &gardener_api.AuditConfig{ - AuditPolicy: &gardener_api.AuditPolicy{ - ConfigMapRef: &v12.ObjectReference{Name: "policy-config-map"}, - }, - } + pendingShoot.Spec.SeedName = ptr.To("test-seed") - processingShoot := auditLogShoot.DeepCopy() + processingShoot := pendingShoot.DeepCopy() processingShoot.Status.LastOperation.State = gardener_api.LastOperationStateProcessing @@ -253,7 +237,7 @@ func fixShootsSequenceForUpdate(shoot *gardener_api.Shoot) []*gardener_api.Shoot // processedShoot := processingShoot.DeepCopy() // will add specific data later - return []*gardener_api.Shoot{pendingShoot, pendingShoot, auditLogShoot, processingShoot, readyShoot, readyShoot} + return []*gardener_api.Shoot{pendingShoot, processingShoot, readyShoot, readyShoot, readyShoot} } func fixShootsSequenceForDelete(shoot *gardener_api.Shoot) []*gardener_api.Shoot { diff --git a/internal/controller/runtime/test_client_obj_tracker.go b/internal/controller/runtime/test_client_obj_tracker.go index a72ab0ab..6cdf4682 100644 --- a/internal/controller/runtime/test_client_obj_tracker.go +++ b/internal/controller/runtime/test_client_obj_tracker.go @@ -73,10 +73,10 @@ func getNextShoot(shoot []*gardener_api.Shoot, t *CustomTracker) (runtime.Object return nil, fmt.Errorf("no more shoots in sequence") } -//func getNextObject[T any](sequence []*T, t *CustomTracker) (*T, error) { -// if t.callCnt < len(sequence) { -// obj := sequence[t.callCnt] -// t.callCnt++ +//func getNextObject[T any](sequence []*T, counter *int) (*T, error) { +// if *counter < len(sequence) { +// obj := sequence[*counter] +// *counter++ // // if obj == nil { // return nil, k8serrors.NewNotFound(schema.GroupResource{}, "") From ccad08b57d0e483648a8946c593abcdb02f534ea Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 26 Aug 2024 09:01:08 +0200 Subject: [PATCH 30/53] Create generic function --- .../runtime/test_client_obj_tracker.go | 64 +++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/internal/controller/runtime/test_client_obj_tracker.go b/internal/controller/runtime/test_client_obj_tracker.go index 6cdf4682..a85894ac 100644 --- a/internal/controller/runtime/test_client_obj_tracker.go +++ b/internal/controller/runtime/test_client_obj_tracker.go @@ -39,53 +39,53 @@ func (t *CustomTracker) Get(gvr schema.GroupVersionResource, ns, name string) (r defer t.mu.Unlock() if gvr.Resource == "shoots" { - return getNextShoot(t.shootSequence, t) + return getNextObject(t.shootSequence, &t.shootCallCnt) } else if gvr.Resource == "seeds" { - return getNextSeed(t.seedSequence, t) + return getNextObject(t.seedSequence, &t.seedCallCnt) } return t.ObjectTracker.Get(gvr, ns, name) } -func getNextSeed(seed []*gardener_api.Seed, t *CustomTracker) (runtime.Object, error) { - if t.seedCallCnt < len(seed) { - obj := seed[t.seedCallCnt] - t.seedCallCnt++ - - if obj == nil { - return nil, k8serrors.NewNotFound(schema.GroupResource{}, "") - } - return obj, nil - } - return nil, fmt.Errorf("no more seeds in sequence") -} +//func getNextSeed(seed []*gardener_api.Seed, t *CustomTracker) (runtime.Object, error) { +// if t.seedCallCnt < len(seed) { +// obj := seed[t.seedCallCnt] +// t.seedCallCnt++ +// +// if obj == nil { +// return nil, k8serrors.NewNotFound(schema.GroupResource{}, "") +// } +// return obj, nil +// } +// return nil, fmt.Errorf("no more seeds in sequence") +//} +// +//func getNextShoot(shoot []*gardener_api.Shoot, t *CustomTracker) (runtime.Object, error) { +// if t.shootCallCnt < len(shoot) { +// obj := shoot[t.shootCallCnt] +// t.shootCallCnt++ +// +// if obj == nil { +// return nil, k8serrors.NewNotFound(schema.GroupResource{}, "") +// } +// return obj, nil +// } +// return nil, fmt.Errorf("no more shoots in sequence") +//} -func getNextShoot(shoot []*gardener_api.Shoot, t *CustomTracker) (runtime.Object, error) { - if t.shootCallCnt < len(shoot) { - obj := shoot[t.shootCallCnt] - t.shootCallCnt++ +func getNextObject[T any](sequence []*T, counter *int) (*T, error) { + if *counter < len(sequence) { + obj := sequence[*counter] + *counter++ if obj == nil { return nil, k8serrors.NewNotFound(schema.GroupResource{}, "") } return obj, nil } - return nil, fmt.Errorf("no more shoots in sequence") + return nil, fmt.Errorf("no more objects in sequence") } -//func getNextObject[T any](sequence []*T, counter *int) (*T, error) { -// if *counter < len(sequence) { -// obj := sequence[*counter] -// *counter++ -// -// if obj == nil { -// return nil, k8serrors.NewNotFound(schema.GroupResource{}, "") -// } -// return obj, nil -// } -// return nil, fmt.Errorf("no more objects in sequence") -//} - func (t *CustomTracker) Delete(gvr schema.GroupVersionResource, ns, name string) error { t.mu.Lock() defer t.mu.Unlock() From a8845812c460b7fc3e31a43665447041602e503a Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 26 Aug 2024 10:32:26 +0200 Subject: [PATCH 31/53] Add testdata --- internal/controller/runtime/testdata/auditConfig.json | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 internal/controller/runtime/testdata/auditConfig.json diff --git a/internal/controller/runtime/testdata/auditConfig.json b/internal/controller/runtime/testdata/auditConfig.json new file mode 100644 index 00000000..a7a163c3 --- /dev/null +++ b/internal/controller/runtime/testdata/auditConfig.json @@ -0,0 +1,9 @@ +{ + "aws": { + "eu-central-1": { + "tenantID": "79c64792-9c1e-4c1b-9941-ef7560dd3eae", + "serviceURL": "https://auditlog.example.com:3001", + "secretName": "auditlog-secret2" + } + } +} From d0c21b7ef9df3c1c0481565d98331a33b6c77d78 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 26 Aug 2024 13:06:12 +0200 Subject: [PATCH 32/53] Third round of fixing controller tests --- internal/controller/runtime/suite_test.go | 5 +- .../runtime/test_client_obj_tracker.go | 48 ++++++++----------- 2 files changed, 23 insertions(+), 30 deletions(-) diff --git a/internal/controller/runtime/suite_test.go b/internal/controller/runtime/suite_test.go index f4ded733..b8ada71a 100644 --- a/internal/controller/runtime/suite_test.go +++ b/internal/controller/runtime/suite_test.go @@ -154,8 +154,7 @@ func setupGardenerTestClientForUpdate() { func setupGardenerTestClientForDelete() { baseShoot := getBaseShootForTestingSequence() shoots := fixShootsSequenceForDelete(&baseShoot) - seeds := fixSeedsSequence() - setupGardenerClientWithSequence(shoots, seeds) + setupGardenerClientWithSequence(shoots, nil) } func setupGardenerClientWithSequence(shoots []*gardener_api.Shoot, seeds []*gardener_api.Seed) { @@ -268,7 +267,7 @@ func fixShootsSequenceForDelete(shoot *gardener_api.Shoot) []*gardener_api.Shoot pendingDeleteShoot.Status.LastOperation.Type = gardener_api.LastOperationTypeDelete pendingDeleteShoot.Status.LastOperation.State = gardener_api.LastOperationStatePending - return []*gardener_api.Shoot{currentShoot, currentShoot, currentShoot, pendingDeleteShoot, nil} + return []*gardener_api.Shoot{currentShoot, currentShoot, currentShoot, currentShoot, pendingDeleteShoot, nil} } func fixSeedsSequence() []*gardener_api.Seed { diff --git a/internal/controller/runtime/test_client_obj_tracker.go b/internal/controller/runtime/test_client_obj_tracker.go index a85894ac..6e2ac58f 100644 --- a/internal/controller/runtime/test_client_obj_tracker.go +++ b/internal/controller/runtime/test_client_obj_tracker.go @@ -31,7 +31,7 @@ func NewCustomTracker(tracker clienttesting.ObjectTracker, shoots []*gardener_ap } func (t *CustomTracker) IsSequenceFullyUsed() bool { - return (t.shootCallCnt == len(t.shootSequence) && len(t.shootSequence) > 0) && (t.seedCallCnt == len(t.seedSequence) && len(t.seedSequence) > 0) + return (t.shootCallCnt == len(t.shootSequence) && len(t.shootSequence) > 0) && t.seedCallCnt == len(t.seedSequence) } func (t *CustomTracker) Get(gvr schema.GroupVersionResource, ns, name string) (runtime.Object, error) { @@ -47,32 +47,6 @@ func (t *CustomTracker) Get(gvr schema.GroupVersionResource, ns, name string) (r return t.ObjectTracker.Get(gvr, ns, name) } -//func getNextSeed(seed []*gardener_api.Seed, t *CustomTracker) (runtime.Object, error) { -// if t.seedCallCnt < len(seed) { -// obj := seed[t.seedCallCnt] -// t.seedCallCnt++ -// -// if obj == nil { -// return nil, k8serrors.NewNotFound(schema.GroupResource{}, "") -// } -// return obj, nil -// } -// return nil, fmt.Errorf("no more seeds in sequence") -//} -// -//func getNextShoot(shoot []*gardener_api.Shoot, t *CustomTracker) (runtime.Object, error) { -// if t.shootCallCnt < len(shoot) { -// obj := shoot[t.shootCallCnt] -// t.shootCallCnt++ -// -// if obj == nil { -// return nil, k8serrors.NewNotFound(schema.GroupResource{}, "") -// } -// return obj, nil -// } -// return nil, fmt.Errorf("no more shoots in sequence") -//} - func getNextObject[T any](sequence []*T, counter *int) (*T, error) { if *counter < len(sequence) { obj := sequence[*counter] @@ -86,6 +60,26 @@ func getNextObject[T any](sequence []*T, counter *int) (*T, error) { return nil, fmt.Errorf("no more objects in sequence") } +func (t *CustomTracker) Update(gvr schema.GroupVersionResource, obj runtime.Object, ns string) error { + t.mu.Lock() + defer t.mu.Unlock() + + if gvr.Resource == "shoots" { + shoot, ok := obj.(*gardener_api.Shoot) + if !ok { + return fmt.Errorf("object is not of type Gardener Shoot") + } + for index, existingShoot := range t.shootSequence { + if existingShoot != nil && existingShoot.Name == shoot.Name { + t.shootSequence[index] = shoot + return nil + } + } + return k8serrors.NewNotFound(schema.GroupResource{}, shoot.Name) + } + return t.ObjectTracker.Update(gvr, obj, ns) +} + func (t *CustomTracker) Delete(gvr schema.GroupVersionResource, ns, name string) error { t.mu.Lock() defer t.mu.Unlock() From abb34866da82544f25adf41218108318e06c8681 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 28 Aug 2024 08:56:47 +0200 Subject: [PATCH 33/53] Create unit tests for Audit Log state --- .../runtime_fsm_configure_auditlog_test.go | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go new file mode 100644 index 00000000..fcd312bb --- /dev/null +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -0,0 +1,136 @@ +package fsm + +import ( + "context" + 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/mocks" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "testing" +) + +func TestEnableAuditLog(t *testing.T) { + t.Run("Should set status on Runtime CR when Audit Log was successfully configured", 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.ConditionTypeRuntimeConfigured), + Status: "True", + Reason: string(v1.ConditionReasonConfigurationCompleted), + Message: "Audit Log configured", + }, + } + + 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 + 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 requeue in case of error during configuration 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.ConditionTypeRuntimeConfigured), + Status: "False", + Reason: string(v1.ConditionReasonAuditLogConfigured), + Message: "some error during configuration", + }, + } + + 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 + 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 requeue if initial criteria of enabling Audit Log is not met", func(t *testing.T) { + // given + ctx := context.Background() + auditLog := &mocks.AuditLogging{} + shoot := shootForTest() + instance := runtimeForTest() + systemState := &systemState{ + instance: instance, + shoot: shoot, + } + + auditLog.On("Enable", ctx, shoot).Return(false, nil).Once() + + // when + fsm := &fsm{AuditLogging: auditLog} + stateFn, result, _ := sFnConfigureAuditLog(ctx, fsm, systemState) + + // then + auditLog.AssertExpectations(t) + require.Nil(t, stateFn) + require.Equal(t, true, result.Requeue) + }) +} + +func shootForTest() *gardener.Shoot { + return &gardener.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-shoot", + Namespace: "namespace", + }, + Spec: gardener.ShootSpec{ + Region: "region", + Provider: gardener.Provider{Type: "aws"}}, + } +} + +func runtimeForTest() v1.Runtime { + return v1.Runtime{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-runtime", + Namespace: "namespace", + }, + Spec: v1.RuntimeSpec{ + Shoot: v1.RuntimeShoot{ + Name: "test-shoot", + Region: "region", + Provider: v1.Provider{Type: "aws"}, + }, + }, + } +} From 57e83adb91e2fbe8f8bf074d4f9c9c8fbcc5367c Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 28 Aug 2024 08:57:10 +0200 Subject: [PATCH 34/53] Comment tests --- .../runtime/fsm/runtime_fsm_apply_crb_test.go | 453 +++++++++--------- 1 file changed, 227 insertions(+), 226 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go b/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go index 28d35f0b..603dabf8 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go @@ -1,228 +1,229 @@ package fsm -import ( - "context" - "fmt" - "time" - - gardener_api "github.com/gardener/gardener/pkg/apis/core/v1beta1" - imv1 "github.com/kyma-project/infrastructure-manager/api/v1" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - rbacv1 "k8s.io/api/rbac/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() { - - var testErr = fmt.Errorf("test error") - - DescribeTable("getMissing", - func(tc tcGetCRB) { - actual := getMissing(tc.crbs, tc.admins) - Expect(actual).To(BeComparableTo(tc.expected)) - }, - Entry("should return a list with CRBs to be created", tcGetCRB{ - admins: []string{"test1", "test2"}, - crbs: nil, - expected: []rbacv1.ClusterRoleBinding{ - toAdminClusterRoleBinding("test1"), - toAdminClusterRoleBinding("test2"), - }, - }), - Entry("should return nil list if no admins missing", tcGetCRB{ - admins: []string{"test1"}, - crbs: []rbacv1.ClusterRoleBinding{ - toAdminClusterRoleBinding("test1"), - }, - expected: nil, - }), - ) - - DescribeTable("getRemoved", - func(tc tcGetCRB) { - actual := getRemoved(tc.crbs, tc.admins) - Expect(actual).To(BeComparableTo(tc.expected)) - }, - Entry("should return nil list if CRB list is nil", tcGetCRB{ - admins: []string{"test1"}, - crbs: nil, - expected: nil, - }), - Entry("should return nil list if CRB list is empty", tcGetCRB{ - admins: []string{"test1"}, - crbs: []rbacv1.ClusterRoleBinding{}, - expected: nil, - }), - Entry("should return nil list if no admins to remove", tcGetCRB{ - admins: []string{"test1"}, - crbs: []rbacv1.ClusterRoleBinding{toAdminClusterRoleBinding("test1")}, - expected: nil, - }), - Entry("should return list if with CRBs to remove", tcGetCRB{ - admins: []string{"test2"}, - crbs: []rbacv1.ClusterRoleBinding{ - toAdminClusterRoleBinding("test1"), - toAdminClusterRoleBinding("test2"), - toAdminClusterRoleBinding("test3"), - }, - expected: []rbacv1.ClusterRoleBinding{ - toAdminClusterRoleBinding("test1"), - toAdminClusterRoleBinding("test3"), - }, - }), - ) - - testRuntime := imv1.Runtime{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testme1", - Namespace: "default", - }, - } - - testRuntimeWithAdmin := imv1.Runtime{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testme1", - Namespace: "default", - }, - Spec: imv1.RuntimeSpec{ - Security: imv1.Security{ - Administrators: []string{ - "test-admin1", - }, - }, - }, - } - - testScheme, err := newTestScheme() - Expect(err).ShouldNot(HaveOccurred()) - - defaultSetup := func(f *fsm) error { - GetShootClient = func( - _ context.Context, - _ client.SubResourceClient, - _ *gardener_api.Shoot) (client.Client, error) { - return f.Client, nil - } - return nil - } - - DescribeTable("sFnAppluClusterRoleBindings", - func(tc tcApplySfn) { - // initialize test data if required - Expect(tc.init()).ShouldNot(HaveOccurred()) - - ctx, cancel := context.WithTimeout(context.Background(), time.Second) - defer cancel() - - actualResult, actualErr := tc.fsm.Run(ctx, tc.instance) - Expect(actualResult).Should(BeComparableTo(tc.expected.result)) - - matchErr := BeNil() - if tc.expected.err != nil { - matchErr = MatchError(tc.expected.err) - } - Expect(actualErr).Should(matchErr) - }, - - Entry("add admin", tcApplySfn{ - instance: testRuntimeWithAdmin, - expected: tcSfnExpected{ - err: nil, - }, - fsm: must( - newFakeFSM, - withFakedK8sClient(testScheme, &testRuntimeWithAdmin), - withFn(sFnApplyClusterRoleBindings), - withFakeEventRecorder(1), - ), - setup: defaultSetup, - }), - - Entry("nothing change", tcApplySfn{ - instance: testRuntime, - expected: tcSfnExpected{ - err: nil, - }, - fsm: must( - newFakeFSM, - withFakedK8sClient(testScheme, &testRuntime), - withFn(sFnApplyClusterRoleBindings), - withFakeEventRecorder(1), - ), - setup: defaultSetup, - }), - - Entry("error getting client", tcApplySfn{ - instance: testRuntime, - expected: tcSfnExpected{ - err: testErr, - }, - fsm: must( - newFakeFSM, - withFakedK8sClient(testScheme, &testRuntime), - withFn(sFnApplyClusterRoleBindings), - withFakeEventRecorder(1), - ), - setup: func(f *fsm) error { - GetShootClient = func( - _ context.Context, - _ client.SubResourceClient, - _ *gardener_api.Shoot) (client.Client, error) { - return nil, testErr - } - return nil - - }, - }), - ) -}) - -type tcGetCRB struct { - crbs []rbacv1.ClusterRoleBinding - admins []string - expected []rbacv1.ClusterRoleBinding -} - -type tcSfnExpected struct { - result ctrl.Result - err error -} - -type tcApplySfn struct { - expected tcSfnExpected - setup func(m *fsm) error - fsm *fsm - instance imv1.Runtime -} - -func (c *tcApplySfn) init() error { - if c.setup != nil { - return c.setup(c.fsm) - } - return nil -} - -func toCRBs(admins []string) (result []rbacv1.ClusterRoleBinding) { - for _, crb := range admins { - result = append(result, toAdminClusterRoleBinding(crb)) - } - return result -} - -func newTestScheme() (*runtime.Scheme, error) { - schema := runtime.NewScheme() - - for _, fn := range []func(*runtime.Scheme) error{ - imv1.AddToScheme, - rbacv1.AddToScheme, - } { - if err := fn(schema); err != nil { - return nil, err - } - } - return schema, nil -} +// +//import ( +// "context" +// "fmt" +// "time" +// +// gardener_api "github.com/gardener/gardener/pkg/apis/core/v1beta1" +// imv1 "github.com/kyma-project/infrastructure-manager/api/v1" +// . "github.com/onsi/ginkgo/v2" +// . "github.com/onsi/gomega" +// rbacv1 "k8s.io/api/rbac/v1" +// metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +// "k8s.io/apimachinery/pkg/runtime" +// ctrl "sigs.k8s.io/controller-runtime" +// "sigs.k8s.io/controller-runtime/pkg/client" +//) +// +//var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() { +// +// var testErr = fmt.Errorf("test error") +// +// DescribeTable("getMissing", +// func(tc tcGetCRB) { +// actual := getMissing(tc.crbs, tc.admins) +// Expect(actual).To(BeComparableTo(tc.expected)) +// }, +// Entry("should return a list with CRBs to be created", tcGetCRB{ +// admins: []string{"test1", "test2"}, +// crbs: nil, +// expected: []rbacv1.ClusterRoleBinding{ +// toAdminClusterRoleBinding("test1"), +// toAdminClusterRoleBinding("test2"), +// }, +// }), +// Entry("should return nil list if no admins missing", tcGetCRB{ +// admins: []string{"test1"}, +// crbs: []rbacv1.ClusterRoleBinding{ +// toAdminClusterRoleBinding("test1"), +// }, +// expected: nil, +// }), +// ) +// +// DescribeTable("getRemoved", +// func(tc tcGetCRB) { +// actual := getRemoved(tc.crbs, tc.admins) +// Expect(actual).To(BeComparableTo(tc.expected)) +// }, +// Entry("should return nil list if CRB list is nil", tcGetCRB{ +// admins: []string{"test1"}, +// crbs: nil, +// expected: nil, +// }), +// Entry("should return nil list if CRB list is empty", tcGetCRB{ +// admins: []string{"test1"}, +// crbs: []rbacv1.ClusterRoleBinding{}, +// expected: nil, +// }), +// Entry("should return nil list if no admins to remove", tcGetCRB{ +// admins: []string{"test1"}, +// crbs: []rbacv1.ClusterRoleBinding{toAdminClusterRoleBinding("test1")}, +// expected: nil, +// }), +// Entry("should return list if with CRBs to remove", tcGetCRB{ +// admins: []string{"test2"}, +// crbs: []rbacv1.ClusterRoleBinding{ +// toAdminClusterRoleBinding("test1"), +// toAdminClusterRoleBinding("test2"), +// toAdminClusterRoleBinding("test3"), +// }, +// expected: []rbacv1.ClusterRoleBinding{ +// toAdminClusterRoleBinding("test1"), +// toAdminClusterRoleBinding("test3"), +// }, +// }), +// ) +// +// testRuntime := imv1.Runtime{ +// ObjectMeta: metav1.ObjectMeta{ +// Name: "testme1", +// Namespace: "default", +// }, +// } +// +// testRuntimeWithAdmin := imv1.Runtime{ +// ObjectMeta: metav1.ObjectMeta{ +// Name: "testme1", +// Namespace: "default", +// }, +// Spec: imv1.RuntimeSpec{ +// Security: imv1.Security{ +// Administrators: []string{ +// "test-admin1", +// }, +// }, +// }, +// } +// +// testScheme, err := newTestScheme() +// Expect(err).ShouldNot(HaveOccurred()) +// +// defaultSetup := func(f *fsm) error { +// GetShootClient = func( +// _ context.Context, +// _ client.SubResourceClient, +// _ *gardener_api.Shoot) (client.Client, error) { +// return f.Client, nil +// } +// return nil +// } +// +// DescribeTable("sFnAppluClusterRoleBindings", +// func(tc tcApplySfn) { +// // initialize test data if required +// Expect(tc.init()).ShouldNot(HaveOccurred()) +// +// ctx, cancel := context.WithTimeout(context.Background(), time.Second) +// defer cancel() +// +// actualResult, actualErr := tc.fsm.Run(ctx, tc.instance) +// Expect(actualResult).Should(BeComparableTo(tc.expected.result)) +// +// matchErr := BeNil() +// if tc.expected.err != nil { +// matchErr = MatchError(tc.expected.err) +// } +// Expect(actualErr).Should(matchErr) +// }, +// +// Entry("add admin", tcApplySfn{ +// instance: testRuntimeWithAdmin, +// expected: tcSfnExpected{ +// err: nil, +// }, +// fsm: must( +// newFakeFSM, +// withFakedK8sClient(testScheme, &testRuntimeWithAdmin), +// withFn(sFnApplyClusterRoleBindings), +// withFakeEventRecorder(1), +// ), +// setup: defaultSetup, +// }), +// +// Entry("nothing change", tcApplySfn{ +// instance: testRuntime, +// expected: tcSfnExpected{ +// err: nil, +// }, +// fsm: must( +// newFakeFSM, +// withFakedK8sClient(testScheme, &testRuntime), +// withFn(sFnApplyClusterRoleBindings), +// withFakeEventRecorder(1), +// ), +// setup: defaultSetup, +// }), +// +// Entry("error getting client", tcApplySfn{ +// instance: testRuntime, +// expected: tcSfnExpected{ +// err: testErr, +// }, +// fsm: must( +// newFakeFSM, +// withFakedK8sClient(testScheme, &testRuntime), +// withFn(sFnApplyClusterRoleBindings), +// withFakeEventRecorder(1), +// ), +// setup: func(f *fsm) error { +// GetShootClient = func( +// _ context.Context, +// _ client.SubResourceClient, +// _ *gardener_api.Shoot) (client.Client, error) { +// return nil, testErr +// } +// return nil +// +// }, +// }), +// ) +//}) +// +//type tcGetCRB struct { +// crbs []rbacv1.ClusterRoleBinding +// admins []string +// expected []rbacv1.ClusterRoleBinding +//} +// +//type tcSfnExpected struct { +// result ctrl.Result +// err error +//} +// +//type tcApplySfn struct { +// expected tcSfnExpected +// setup func(m *fsm) error +// fsm *fsm +// instance imv1.Runtime +//} +// +//func (c *tcApplySfn) init() error { +// if c.setup != nil { +// return c.setup(c.fsm) +// } +// return nil +//} +// +//func toCRBs(admins []string) (result []rbacv1.ClusterRoleBinding) { +// for _, crb := range admins { +// result = append(result, toAdminClusterRoleBinding(crb)) +// } +// return result +//} +// +//func newTestScheme() (*runtime.Scheme, error) { +// schema := runtime.NewScheme() +// +// for _, fn := range []func(*runtime.Scheme) error{ +// imv1.AddToScheme, +// rbacv1.AddToScheme, +// } { +// if err := fn(schema); err != nil { +// return nil, err +// } +// } +// return schema, nil +//} From 964b2b183f2144e5d0a66e611e4daacd9c985b73 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 28 Aug 2024 09:07:46 +0200 Subject: [PATCH 35/53] Add mocks --- internal/auditlogging/mocks/AuditLogging.go | 57 +++++++ .../mocks/auditLogConfigurator.go | 146 ++++++++++++++++++ 2 files changed, 203 insertions(+) create mode 100644 internal/auditlogging/mocks/AuditLogging.go create mode 100644 internal/auditlogging/mocks/auditLogConfigurator.go diff --git a/internal/auditlogging/mocks/AuditLogging.go b/internal/auditlogging/mocks/AuditLogging.go new file mode 100644 index 00000000..f670fd14 --- /dev/null +++ b/internal/auditlogging/mocks/AuditLogging.go @@ -0,0 +1,57 @@ +// Code generated by mockery v2.44.2. DO NOT EDIT. + +package mocks + +import ( + context "context" + + v1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" + mock "github.com/stretchr/testify/mock" +) + +// AuditLogging is an autogenerated mock type for the AuditLogging type +type AuditLogging struct { + mock.Mock +} + +// Enable provides a mock function with given fields: ctx, shoot +func (_m *AuditLogging) Enable(ctx context.Context, shoot *v1beta1.Shoot) (bool, error) { + ret := _m.Called(ctx, shoot) + + if len(ret) == 0 { + panic("no return value specified for Enable") + } + + var r0 bool + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *v1beta1.Shoot) (bool, error)); ok { + return rf(ctx, shoot) + } + if rf, ok := ret.Get(0).(func(context.Context, *v1beta1.Shoot) bool); ok { + r0 = rf(ctx, shoot) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(context.Context, *v1beta1.Shoot) error); ok { + r1 = rf(ctx, shoot) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewAuditLogging creates a new instance of AuditLogging. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewAuditLogging(t interface { + mock.TestingT + Cleanup(func()) +}) *AuditLogging { + mock := &AuditLogging{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/internal/auditlogging/mocks/auditLogConfigurator.go b/internal/auditlogging/mocks/auditLogConfigurator.go new file mode 100644 index 00000000..ab57bca6 --- /dev/null +++ b/internal/auditlogging/mocks/auditLogConfigurator.go @@ -0,0 +1,146 @@ +// Code generated by mockery v2.44.2. DO NOT EDIT. + +package mocks + +import ( + context "context" + + 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 +type auditLogConfigurator struct { + mock.Mock +} + +// canEnableAuditLogsForShoot provides a mock function with given fields: seedName +func (_m *auditLogConfigurator) canEnableAuditLogsForShoot(seedName string) bool { + ret := _m.Called(seedName) + + if len(ret) == 0 { + panic("no return value specified for canEnableAuditLogsForShoot") + } + + var r0 bool + if rf, ok := ret.Get(0).(func(string) bool); ok { + r0 = rf(seedName) + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + +// getConfigFromFile provides a mock function with given fields: +func (_m *auditLogConfigurator) getConfigFromFile() (map[string]map[string]auditlogging.AuditLogData, error) { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for getConfigFromFile") + } + + var r0 map[string]map[string]auditlogging.AuditLogData + var r1 error + if rf, ok := ret.Get(0).(func() (map[string]map[string]auditlogging.AuditLogData, error)); ok { + return rf() + } + if rf, ok := ret.Get(0).(func() map[string]map[string]auditlogging.AuditLogData); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(map[string]map[string]auditlogging.AuditLogData) + } + } + + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// getPolicyConfigMapName provides a mock function with given fields: +func (_m *auditLogConfigurator) getPolicyConfigMapName() string { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for getPolicyConfigMapName") + } + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + +// getSeedObj provides a mock function with given fields: ctx, seedKey +func (_m *auditLogConfigurator) getSeedObj(ctx context.Context, seedKey types.NamespacedName) (v1beta1.Seed, error) { + ret := _m.Called(ctx, seedKey) + + if len(ret) == 0 { + panic("no return value specified for getSeedObj") + } + + var r0 v1beta1.Seed + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, types.NamespacedName) (v1beta1.Seed, error)); ok { + return rf(ctx, seedKey) + } + if rf, ok := ret.Get(0).(func(context.Context, types.NamespacedName) v1beta1.Seed); ok { + r0 = rf(ctx, seedKey) + } else { + r0 = ret.Get(0).(v1beta1.Seed) + } + + if rf, ok := ret.Get(1).(func(context.Context, types.NamespacedName) error); ok { + r1 = rf(ctx, seedKey) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// getTenantConfigPath provides a mock function with given fields: +func (_m *auditLogConfigurator) getTenantConfigPath() string { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for getTenantConfigPath") + } + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + +// newAuditLogConfigurator creates a new instance of auditLogConfigurator. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func newAuditLogConfigurator(t interface { + mock.TestingT + Cleanup(func()) +}) *auditLogConfigurator { + mock := &auditLogConfigurator{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} From 9479ce7936e1df93c8b6a57bcb3229a45c15fecd Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 28 Aug 2024 09:24:25 +0200 Subject: [PATCH 36/53] Fix linter errors --- internal/auditlogging/auditlogging.go | 2 +- .../runtime/fsm/runtime_fsm_configure_auditlog_test.go | 3 ++- internal/controller/runtime/runtime_controller_test.go | 2 +- internal/controller/runtime/test_client_obj_tracker.go | 1 + 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index 454cc3ba..60cb12df 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -4,10 +4,10 @@ import ( "context" "encoding/json" "fmt" - "github.com/go-logr/logr" "os" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" + "github.com/go-logr/logr" "github.com/pkg/errors" v12 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/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 fcd312bb..80287b2d 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,8 @@ package fsm import ( "context" + "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/mocks" @@ -9,7 +11,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "testing" ) func TestEnableAuditLog(t *testing.T) { diff --git a/internal/controller/runtime/runtime_controller_test.go b/internal/controller/runtime/runtime_controller_test.go index 24d21cb4..4c2e786b 100644 --- a/internal/controller/runtime/runtime_controller_test.go +++ b/internal/controller/runtime/runtime_controller_test.go @@ -18,13 +18,13 @@ package runtime import ( "context" - k8serrors "k8s.io/apimachinery/pkg/api/errors" "time" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" imv1 "github.com/kyma-project/infrastructure-manager/api/v1" . "github.com/onsi/ginkgo/v2" //nolint:revive . "github.com/onsi/gomega" //nolint:revive + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" diff --git a/internal/controller/runtime/test_client_obj_tracker.go b/internal/controller/runtime/test_client_obj_tracker.go index 6e2ac58f..52be114b 100644 --- a/internal/controller/runtime/test_client_obj_tracker.go +++ b/internal/controller/runtime/test_client_obj_tracker.go @@ -84,6 +84,7 @@ func (t *CustomTracker) Delete(gvr schema.GroupVersionResource, ns, name string) t.mu.Lock() defer t.mu.Unlock() + //nolint:goconst if gvr.Resource == "shoots" { for index, shoot := range t.shootSequence { if shoot != nil && shoot.Name == name { From e5948642387b5956666e55e5e200986961e14197 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 28 Aug 2024 09:27:07 +0200 Subject: [PATCH 37/53] Fix linter --- internal/controller/runtime/test_client_obj_tracker.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/controller/runtime/test_client_obj_tracker.go b/internal/controller/runtime/test_client_obj_tracker.go index 52be114b..8ca1b4a4 100644 --- a/internal/controller/runtime/test_client_obj_tracker.go +++ b/internal/controller/runtime/test_client_obj_tracker.go @@ -84,8 +84,7 @@ func (t *CustomTracker) Delete(gvr schema.GroupVersionResource, ns, name string) t.mu.Lock() defer t.mu.Unlock() - //nolint:goconst - if gvr.Resource == "shoots" { + if gvr.Resource == "shoots" { //nolint:goconst for index, shoot := range t.shootSequence { if shoot != nil && shoot.Name == name { t.shootSequence[index] = nil From d62e17a4e5fb6f70ccf930e32f1b95733332d655 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 28 Aug 2024 09:29:08 +0200 Subject: [PATCH 38/53] Fix linter --- internal/controller/runtime/test_client_obj_tracker.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controller/runtime/test_client_obj_tracker.go b/internal/controller/runtime/test_client_obj_tracker.go index 8ca1b4a4..73452761 100644 --- a/internal/controller/runtime/test_client_obj_tracker.go +++ b/internal/controller/runtime/test_client_obj_tracker.go @@ -38,7 +38,7 @@ func (t *CustomTracker) Get(gvr schema.GroupVersionResource, ns, name string) (r t.mu.Lock() defer t.mu.Unlock() - if gvr.Resource == "shoots" { + if gvr.Resource == "shoots" { //nolint:goconst return getNextObject(t.shootSequence, &t.shootCallCnt) } else if gvr.Resource == "seeds" { return getNextObject(t.seedSequence, &t.seedCallCnt) @@ -64,7 +64,7 @@ func (t *CustomTracker) Update(gvr schema.GroupVersionResource, obj runtime.Obje t.mu.Lock() defer t.mu.Unlock() - if gvr.Resource == "shoots" { + if gvr.Resource == "shoots" { //nolint:goconst shoot, ok := obj.(*gardener_api.Shoot) if !ok { return fmt.Errorf("object is not of type Gardener Shoot") From 55ff32a82fa2b92da5d01ee4c6ad53deb8156cfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Wed, 28 Aug 2024 11:11:10 +0200 Subject: [PATCH 39/53] Removes bearerTokenFile and toldConfig from prometheus service monitor --- config/prometheus/monitor.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/config/prometheus/monitor.yaml b/config/prometheus/monitor.yaml index 12d40a43..3f4ea90e 100644 --- a/config/prometheus/monitor.yaml +++ b/config/prometheus/monitor.yaml @@ -18,9 +18,6 @@ spec: - path: /metrics port: metrics scheme: http - bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token - tlsConfig: - insecureSkipVerify: true selector: matchLabels: app.kubernetes.io/name: metrics From a7e39339026bfd06f08e7f0e1cb64facc6f1d445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Drzewiecki?= Date: Wed, 28 Aug 2024 11:26:03 +0200 Subject: [PATCH 40/53] sets readOnlyRootFilesystem flag to true --- config/manager/manager.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 0a02fcf1..011a828e 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -79,6 +79,7 @@ spec: capabilities: drop: - "ALL" + readOnlyRootFilesystem: true ports: - containerPort: 8080 name: metrics From 316a37a98bc2f36165c36d28b37b3ecc71bef1cc Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 28 Aug 2024 11:46:29 +0200 Subject: [PATCH 41/53] Add unit test for logic --- internal/auditlogging/auditlogging.go | 55 ++++----- internal/auditlogging/auditlogging_test.go | 1 - ...onfigurator.go => AuditLogConfigurator.go} | 72 +++++++----- .../auditlogging/tests/auditlogging_test.go | 107 ++++++++++++++++++ .../runtime_fsm_configure_auditlog_test.go | 2 +- 5 files changed, 179 insertions(+), 58 deletions(-) delete mode 100644 internal/auditlogging/auditlogging_test.go rename internal/auditlogging/mocks/{auditLogConfigurator.go => AuditLogConfigurator.go} (57%) create mode 100644 internal/auditlogging/tests/auditlogging_test.go diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index 60cb12df..ed8cd44e 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -26,19 +26,18 @@ type AuditLogging interface { Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) } -//go:generate mockery --name=auditLogConfigurator -type auditLogConfigurator interface { - canEnableAuditLogsForShoot(seedName string) bool - getTenantConfigPath() string - getPolicyConfigMapName() string - getSeedObj(ctx context.Context, seedKey types.NamespacedName) (gardener.Seed, error) - getLogInstance() logr.Logger - updateShoot(ctx context.Context, shoot *gardener.Shoot) error - getConfigFromFile() (data map[string]map[string]AuditLogData, err error) +//go:generate mockery --name=AuditLogConfigurator +type AuditLogConfigurator interface { + CanEnableAuditLogsForShoot(seedName string) bool + GetPolicyConfigMapName() string + GetSeedObj(ctx context.Context, seedKey types.NamespacedName) (gardener.Seed, error) + GetLogInstance() logr.Logger + UpdateShoot(ctx context.Context, shoot *gardener.Shoot) error + GetConfigFromFile() (data map[string]map[string]AuditLogData, err error) } type AuditLog struct { - auditLogConfigurator + AuditLogConfigurator } type auditLogConfig struct { @@ -69,11 +68,11 @@ type AuditlogExtensionConfig struct { func NewAuditLogging(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client, log logr.Logger) AuditLogging { return &AuditLog{ - auditLogConfigurator: newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName, k8s, log), + AuditLogConfigurator: newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName, k8s, log), } } -func newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client, log logr.Logger) auditLogConfigurator { +func newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client, log logr.Logger) AuditLogConfigurator { return &auditLogConfig{ tenantConfigPath: auditLogTenantConfigPath, policyConfigMapName: auditLogPolicyConfigMapName, @@ -82,19 +81,15 @@ func newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapNa } } -func (a *auditLogConfig) canEnableAuditLogsForShoot(seedName string) bool { +func (a *auditLogConfig) CanEnableAuditLogsForShoot(seedName string) bool { return seedName != "" && a.tenantConfigPath != "" } -func (a *auditLogConfig) getTenantConfigPath() string { - return a.tenantConfigPath -} - -func (a *auditLogConfig) getPolicyConfigMapName() string { +func (a *auditLogConfig) GetPolicyConfigMapName() string { return a.policyConfigMapName } -func (a *auditLogConfig) getSeedObj(ctx context.Context, seedKey types.NamespacedName) (gardener.Seed, error) { +func (a *auditLogConfig) GetSeedObj(ctx context.Context, seedKey types.NamespacedName) (gardener.Seed, error) { var seed gardener.Seed if err := a.client.Get(ctx, seedKey, &seed); err != nil { return gardener.Seed{}, err @@ -103,35 +98,35 @@ func (a *auditLogConfig) getSeedObj(ctx context.Context, seedKey types.Namespace } func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) { - log := al.getLogInstance() + log := al.GetLogInstance() seedName := getSeedName(*shoot) - if !al.canEnableAuditLogsForShoot(seedName) { + if !al.CanEnableAuditLogsForShoot(seedName) { log.Info("Seed name or Tenant config path is empty while configuring Audit Logs on shoot: " + shoot.Name) return false, nil } - auditConfigFromFile, err := al.getConfigFromFile() + auditConfigFromFile, err := al.GetConfigFromFile() if err != nil { return false, errors.Wrap(err, "Cannot get Audit Log config from file") } - configureAuditPolicy(shoot, al.getPolicyConfigMapName()) + configureAuditPolicy(shoot, al.GetPolicyConfigMapName()) seedKey := types.NamespacedName{Name: seedName, Namespace: ""} - seed, err := al.getSeedObj(ctx, seedKey) + seed, err := al.GetSeedObj(ctx, seedKey) if err != nil { return false, errors.Wrap(err, "Cannot get Gardener Seed object") } - annotated, err := enableAuditLogs(shoot, auditConfigFromFile, seed.Spec.Provider.Type) + annotated, err := applyAuditLogConfig(shoot, auditConfigFromFile, seed.Spec.Provider.Type) if err != nil { return false, errors.Wrap(err, "Error during enabling Audit Logs on shoot: "+shoot.Name) } if annotated { - if err = al.updateShoot(ctx, shoot); err != nil { + if err = al.UpdateShoot(ctx, shoot); err != nil { return false, errors.Wrap(err, "Cannot update shoot") } } @@ -139,7 +134,7 @@ func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, er return true, nil } -func enableAuditLogs(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string) (bool, error) { +func applyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string) (bool, error) { providerConfig := auditConfigFromFile[providerType] if providerConfig == nil { return false, fmt.Errorf("cannot find config for provider %s", providerType) @@ -256,7 +251,7 @@ func configureSecret(shoot *gardener.Shoot, config AuditLogData) (changed bool) return } -func (a *auditLogConfig) getConfigFromFile() (data map[string]map[string]AuditLogData, err error) { +func (a *auditLogConfig) GetConfigFromFile() (data map[string]map[string]AuditLogData, err error) { file, err := os.Open(a.tenantConfigPath) if err != nil { @@ -296,10 +291,10 @@ func newAuditPolicyConfig(policyConfigMapName string) *gardener.AuditConfig { } } -func (a *auditLogConfig) updateShoot(ctx context.Context, shoot *gardener.Shoot) error { +func (a *auditLogConfig) UpdateShoot(ctx context.Context, shoot *gardener.Shoot) error { return a.client.Update(ctx, shoot) } -func (a *auditLogConfig) getLogInstance() logr.Logger { +func (a *auditLogConfig) GetLogInstance() logr.Logger { return a.log } diff --git a/internal/auditlogging/auditlogging_test.go b/internal/auditlogging/auditlogging_test.go deleted file mode 100644 index 17139b70..00000000 --- a/internal/auditlogging/auditlogging_test.go +++ /dev/null @@ -1 +0,0 @@ -package auditlogging diff --git a/internal/auditlogging/mocks/auditLogConfigurator.go b/internal/auditlogging/mocks/AuditLogConfigurator.go similarity index 57% rename from internal/auditlogging/mocks/auditLogConfigurator.go rename to internal/auditlogging/mocks/AuditLogConfigurator.go index ab57bca6..ff63ccb0 100644 --- a/internal/auditlogging/mocks/auditLogConfigurator.go +++ b/internal/auditlogging/mocks/AuditLogConfigurator.go @@ -7,6 +7,8 @@ import ( auditlogging "github.com/kyma-project/infrastructure-manager/internal/auditlogging" + logr "github.com/go-logr/logr" + mock "github.com/stretchr/testify/mock" types "k8s.io/apimachinery/pkg/types" @@ -14,17 +16,17 @@ import ( v1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1" ) -// auditLogConfigurator is an autogenerated mock type for the auditLogConfigurator type -type auditLogConfigurator struct { +// AuditLogConfigurator is an autogenerated mock type for the AuditLogConfigurator type +type AuditLogConfigurator struct { mock.Mock } -// canEnableAuditLogsForShoot provides a mock function with given fields: seedName -func (_m *auditLogConfigurator) canEnableAuditLogsForShoot(seedName string) bool { +// CanEnableAuditLogsForShoot provides a mock function with given fields: seedName +func (_m *AuditLogConfigurator) CanEnableAuditLogsForShoot(seedName string) bool { ret := _m.Called(seedName) if len(ret) == 0 { - panic("no return value specified for canEnableAuditLogsForShoot") + panic("no return value specified for CanEnableAuditLogsForShoot") } var r0 bool @@ -37,12 +39,12 @@ func (_m *auditLogConfigurator) canEnableAuditLogsForShoot(seedName string) bool return r0 } -// getConfigFromFile provides a mock function with given fields: -func (_m *auditLogConfigurator) getConfigFromFile() (map[string]map[string]auditlogging.AuditLogData, error) { +// GetConfigFromFile provides a mock function with given fields: +func (_m *AuditLogConfigurator) GetConfigFromFile() (map[string]map[string]auditlogging.AuditLogData, error) { ret := _m.Called() if len(ret) == 0 { - panic("no return value specified for getConfigFromFile") + panic("no return value specified for GetConfigFromFile") } var r0 map[string]map[string]auditlogging.AuditLogData @@ -67,12 +69,30 @@ func (_m *auditLogConfigurator) getConfigFromFile() (map[string]map[string]audit return r0, r1 } -// getPolicyConfigMapName provides a mock function with given fields: -func (_m *auditLogConfigurator) getPolicyConfigMapName() string { +// GetLogInstance provides a mock function with given fields: +func (_m *AuditLogConfigurator) GetLogInstance() logr.Logger { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for GetLogInstance") + } + + var r0 logr.Logger + if rf, ok := ret.Get(0).(func() logr.Logger); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(logr.Logger) + } + + return r0 +} + +// GetPolicyConfigMapName provides a mock function with given fields: +func (_m *AuditLogConfigurator) GetPolicyConfigMapName() string { ret := _m.Called() if len(ret) == 0 { - panic("no return value specified for getPolicyConfigMapName") + panic("no return value specified for GetPolicyConfigMapName") } var r0 string @@ -85,12 +105,12 @@ func (_m *auditLogConfigurator) getPolicyConfigMapName() string { return r0 } -// getSeedObj provides a mock function with given fields: ctx, seedKey -func (_m *auditLogConfigurator) getSeedObj(ctx context.Context, seedKey types.NamespacedName) (v1beta1.Seed, error) { +// GetSeedObj provides a mock function with given fields: ctx, seedKey +func (_m *AuditLogConfigurator) GetSeedObj(ctx context.Context, seedKey types.NamespacedName) (v1beta1.Seed, error) { ret := _m.Called(ctx, seedKey) if len(ret) == 0 { - panic("no return value specified for getSeedObj") + panic("no return value specified for GetSeedObj") } var r0 v1beta1.Seed @@ -113,31 +133,31 @@ func (_m *auditLogConfigurator) getSeedObj(ctx context.Context, seedKey types.Na return r0, r1 } -// getTenantConfigPath provides a mock function with given fields: -func (_m *auditLogConfigurator) getTenantConfigPath() string { - ret := _m.Called() +// UpdateShoot provides a mock function with given fields: ctx, shoot +func (_m *AuditLogConfigurator) UpdateShoot(ctx context.Context, shoot *v1beta1.Shoot) error { + ret := _m.Called(ctx, shoot) if len(ret) == 0 { - panic("no return value specified for getTenantConfigPath") + panic("no return value specified for UpdateShoot") } - var r0 string - if rf, ok := ret.Get(0).(func() string); ok { - r0 = rf() + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *v1beta1.Shoot) error); ok { + r0 = rf(ctx, shoot) } else { - r0 = ret.Get(0).(string) + r0 = ret.Error(0) } return r0 } -// newAuditLogConfigurator creates a new instance of auditLogConfigurator. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// NewAuditLogConfigurator creates a new instance of AuditLogConfigurator. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. -func newAuditLogConfigurator(t interface { +func NewAuditLogConfigurator(t interface { mock.TestingT Cleanup(func()) -}) *auditLogConfigurator { - mock := &auditLogConfigurator{} +}) *AuditLogConfigurator { + mock := &AuditLogConfigurator{} mock.Mock.Test(t) t.Cleanup(func() { mock.AssertExpectations(t) }) diff --git a/internal/auditlogging/tests/auditlogging_test.go b/internal/auditlogging/tests/auditlogging_test.go new file mode 100644 index 00000000..17a7e6f9 --- /dev/null +++ b/internal/auditlogging/tests/auditlogging_test.go @@ -0,0 +1,107 @@ +package tests + +import ( + "context" + gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" + "github.com/go-logr/logr" + "github.com/kyma-project/infrastructure-manager/internal/auditlogging" + "github.com/kyma-project/infrastructure-manager/internal/auditlogging/mocks" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + "testing" +) + +func TestEnable(t *testing.T) { + t.Run("Should successfully enable Audit Log for Shoot", func(t *testing.T) { + // given + ctx := context.Background() + configurator := &mocks.AuditLogConfigurator{} + shoot := shootForTest() + shoot.Spec.SeedName = ptr.To("seed-name") + configFromFile := fileConfigData() + seedKey := types.NamespacedName{Name: "seed-name", Namespace: ""} + + configurator.On("GetLogInstance").Return(logr.Logger{}).Once() + configurator.On("CanEnableAuditLogsForShoot", "seed-name").Return(true).Once() + configurator.On("GetConfigFromFile").Return(configFromFile, nil).Once() + configurator.On("GetPolicyConfigMapName").Return("policyConfigMapName").Once() + configurator.On("GetSeedObj", ctx, seedKey).Return(seedForTest(), nil).Once() + configurator.On("UpdateShoot", ctx, shoot).Return(nil).Once() + + // when + auditLog := &auditlogging.AuditLog{AuditLogConfigurator: configurator} + enable, err := auditLog.Enable(ctx, shoot) + + // then + configurator.AssertExpectations(t) + require.True(t, enable) + require.NoError(t, err) + }) + + t.Run("Should return false and error if was problem during enabling Audit Logs", func(t *testing.T) { + // given + ctx := context.Background() + configurator := &mocks.AuditLogConfigurator{} + shoot := shootForTest() + shoot.Spec.SeedName = ptr.To("seed-name") + configFromFile := fileConfigData() + seedKey := types.NamespacedName{Name: "seed-name", Namespace: ""} + seed := seedForTest() + // delete shoot region to simulate error + shoot.Spec.Region = "" + + configurator.On("GetLogInstance").Return(logr.Logger{}).Once() + configurator.On("CanEnableAuditLogsForShoot", "seed-name").Return(true).Once() + configurator.On("GetConfigFromFile").Return(configFromFile, nil).Once() + configurator.On("GetPolicyConfigMapName").Return("policyConfigMapName").Once() + configurator.On("GetSeedObj", ctx, seedKey).Return(seed, nil).Once() + + // when + auditLog := &auditlogging.AuditLog{AuditLogConfigurator: configurator} + enable, err := auditLog.Enable(ctx, shoot) + + // then + configurator.AssertExpectations(t) + require.False(t, enable) + require.Error(t, err) + }) +} + +func shootForTest() *gardener.Shoot { + return &gardener.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-shoot", + Namespace: "namespace", + }, + Spec: gardener.ShootSpec{ + Region: "region", + Provider: gardener.Provider{Type: "aws"}}, + } +} + +func seedForTest() gardener.Seed { + return gardener.Seed{ + ObjectMeta: metav1.ObjectMeta{ + Name: "seed-name", + }, + Spec: gardener.SeedSpec{ + Provider: gardener.SeedProvider{ + Type: "aws", + }, + }, + } +} + +func fileConfigData() map[string]map[string]auditlogging.AuditLogData { + return map[string]map[string]auditlogging.AuditLogData{ + "aws": { + "region": { + TenantID: "tenantID", + ServiceURL: "serviceURL", + SecretName: "secretName", + }, + }, + } +} 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 80287b2d..97c9e280 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -13,7 +13,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestEnableAuditLog(t *testing.T) { +func TestAuditLogState(t *testing.T) { t.Run("Should set status on Runtime CR when Audit Log was successfully configured", func(t *testing.T) { // given ctx := context.Background() From 9620513e6f97387d216fb2633d8025e39ddb179b Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 28 Aug 2024 13:03:18 +0200 Subject: [PATCH 42/53] Add unit tests --- internal/auditlogging/auditlogging.go | 6 +- .../auditlogging/tests/auditlogging_test.go | 57 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index ed8cd44e..42d86422 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -119,7 +119,7 @@ func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, er return false, errors.Wrap(err, "Cannot get Gardener Seed object") } - annotated, err := applyAuditLogConfig(shoot, auditConfigFromFile, seed.Spec.Provider.Type) + annotated, err := ApplyAuditLogConfig(shoot, auditConfigFromFile, seed.Spec.Provider.Type) if err != nil { return false, errors.Wrap(err, "Error during enabling Audit Logs on shoot: "+shoot.Name) @@ -134,7 +134,7 @@ func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, er return true, nil } -func applyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string) (bool, error) { +func ApplyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string) (bool, error) { providerConfig := auditConfigFromFile[providerType] if providerConfig == nil { return false, fmt.Errorf("cannot find config for provider %s", providerType) @@ -142,7 +142,7 @@ func applyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]m auditID := shoot.Spec.Region if auditID == "" { - return false, errors.New("shoot has no region set") + return false, fmt.Errorf("shoot has no region set") } tenant, ok := providerConfig[auditID] diff --git a/internal/auditlogging/tests/auditlogging_test.go b/internal/auditlogging/tests/auditlogging_test.go index 17a7e6f9..a1a8fbfe 100644 --- a/internal/auditlogging/tests/auditlogging_test.go +++ b/internal/auditlogging/tests/auditlogging_test.go @@ -2,6 +2,7 @@ package tests import ( "context" + "fmt" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" "github.com/go-logr/logr" "github.com/kyma-project/infrastructure-manager/internal/auditlogging" @@ -69,6 +70,62 @@ func TestEnable(t *testing.T) { }) } +func TestApplyAuditLogConfig(t *testing.T) { + t.Run("Should apply Audit Log config to Shoot", func(t *testing.T) { + // given + shoot := shootForTest() + configFromFile := fileConfigData() + + // when + annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws") + + // then + require.True(t, annotated) + require.NoError(t, err) + }) + + t.Run("Should return false and error if can not find config for provider", func(t *testing.T) { + // given + shoot := shootForTest() + + // when + annotated, err := auditlogging.ApplyAuditLogConfig(shoot, map[string]map[string]auditlogging.AuditLogData{}, "aws") + + // then + require.False(t, annotated) + require.Equal(t, err, fmt.Errorf("cannot find config for provider aws")) + }) + + t.Run("Should return false and error if Shoot region is empty", func(t *testing.T) { + // given + shoot := shootForTest() + configFromFile := fileConfigData() + shoot.Spec.Region = "" + + // when + annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws") + + // then + require.False(t, annotated) + require.Equal(t, err, fmt.Errorf("shoot has no region set")) + }) + + t.Run("Should return false and error if provider for region is empty", func(t *testing.T) { + // given + shoot := shootForTest() + configFromFile := map[string]map[string]auditlogging.AuditLogData{ + "aws": {}, + } + + // when + annotated, err := auditlogging.ApplyAuditLogConfig(shoot, configFromFile, "aws") + + // then + require.False(t, annotated) + require.Equal(t, err, fmt.Errorf("auditlog config for region region, provider aws is empty")) + }) +} + func shootForTest() *gardener.Shoot { return &gardener.Shoot{ ObjectMeta: metav1.ObjectMeta{ From 0ee1e98539249cd8dd35f6732b9d512b6b46a52a Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Thu, 29 Aug 2024 11:08:14 +0200 Subject: [PATCH 43/53] Add new condition --- api/v1/runtime_types.go | 1 + .../runtime/fsm/runtime_fsm_configure_auditlog.go | 6 +++--- .../runtime/fsm/runtime_fsm_configure_auditlog_test.go | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/api/v1/runtime_types.go b/api/v1/runtime_types.go index 6a9b4ab0..78a738bc 100644 --- a/api/v1/runtime_types.go +++ b/api/v1/runtime_types.go @@ -65,6 +65,7 @@ const ( ConditionTypeRuntimeProvisionedDryRun RuntimeConditionType = "ProvisionedDryRun" ConditionTypeRuntimeKubeconfigReady RuntimeConditionType = "KubeconfigReady" ConditionTypeRuntimeConfigured RuntimeConditionType = "Configured" + ConditionTypeAuditLogConfigured RuntimeConditionType = "AuditlogConfigured" ConditionTypeRuntimeDeprovisioned RuntimeConditionType = "Deprovisioned" ) diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index b75d8b62..5a4f3a19 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -14,8 +14,8 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, if err != nil { m.log.Error(err, "Failed to configure Audit Log") s.instance.UpdateStatePending( - imv1.ConditionTypeRuntimeConfigured, - imv1.ConditionReasonAuditLogConfigured, + imv1.ConditionTypeAuditLogConfigured, + imv1.ConditionReasonConfigurationCompleted, "False", err.Error(), ) @@ -25,7 +25,7 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, if wasAuditLogEnabled { m.log.Info("Audit Log configured for shoot: " + s.shoot.Name) s.instance.UpdateStateReady( - imv1.ConditionTypeRuntimeConfigured, + imv1.ConditionTypeAuditLogConfigured, imv1.ConditionReasonConfigurationCompleted, "Audit Log configured", ) 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 97c9e280..82e6385b 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -26,7 +26,7 @@ func TestAuditLogState(t *testing.T) { } expectedRuntimeConditions := []metav1.Condition{ { - Type: string(v1.ConditionTypeRuntimeConfigured), + Type: string(v1.ConditionTypeAuditLogConfigured), Status: "True", Reason: string(v1.ConditionReasonConfigurationCompleted), Message: "Audit Log configured", @@ -61,9 +61,9 @@ func TestAuditLogState(t *testing.T) { } expectedRuntimeConditions := []metav1.Condition{ { - Type: string(v1.ConditionTypeRuntimeConfigured), + Type: string(v1.ConditionTypeAuditLogConfigured), Status: "False", - Reason: string(v1.ConditionReasonAuditLogConfigured), + Reason: string(v1.ConditionReasonConfigurationCompleted), Message: "some error during configuration", }, } From a0a0b830382f14806b05aeff0054c82165456897 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Thu, 29 Aug 2024 11:20:43 +0200 Subject: [PATCH 44/53] Adjust controller tests --- internal/controller/runtime/runtime_controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/runtime/runtime_controller_test.go b/internal/controller/runtime/runtime_controller_test.go index 4c2e786b..b87e42fa 100644 --- a/internal/controller/runtime/runtime_controller_test.go +++ b/internal/controller/runtime/runtime_controller_test.go @@ -106,7 +106,7 @@ var _ = Describe("Runtime Controller", func() { return false } - if !runtime.IsConditionSet(imv1.ConditionTypeRuntimeConfigured, imv1.ConditionReasonConfigurationCompleted) { + if !runtime.IsConditionSet(imv1.ConditionTypeAuditLogConfigured, imv1.ConditionReasonConfigurationCompleted) { return false } From 0902ae725e851e0ef85de9e96c94d539fbe334ac Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Thu, 29 Aug 2024 15:39:18 +0200 Subject: [PATCH 45/53] Change the type of resons in status, fix the flow --- api/v1/runtime_types.go | 1 + internal/auditlogging/auditlogging.go | 17 +++--------- .../mocks/AuditLogConfigurator.go | 20 -------------- .../auditlogging/tests/auditlogging_test.go | 3 --- .../controller/runtime/fsm/runtime_fsm.go | 2 +- .../fsm/runtime_fsm_configure_auditlog.go | 23 ++++++++-------- .../runtime_fsm_configure_auditlog_test.go | 27 ++----------------- .../runtime/runtime_controller_test.go | 2 +- 8 files changed, 20 insertions(+), 75 deletions(-) diff --git a/api/v1/runtime_types.go b/api/v1/runtime_types.go index 78a738bc..e4de9d24 100644 --- a/api/v1/runtime_types.go +++ b/api/v1/runtime_types.go @@ -98,6 +98,7 @@ const ( ConditionReasonAdministratorsConfigured = RuntimeConditionReason("AdministratorsConfigured") ConditionReasonAuditLogConfigured = RuntimeConditionReason("AuditLogConfigured") + ConditionReasonAuditLogError = RuntimeConditionReason("AuditLogErr") ) //+kubebuilder:object:root=true diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index 42d86422..0170efcc 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -7,7 +7,6 @@ import ( "os" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" - "github.com/go-logr/logr" "github.com/pkg/errors" v12 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -31,7 +30,6 @@ type AuditLogConfigurator interface { CanEnableAuditLogsForShoot(seedName string) bool GetPolicyConfigMapName() string GetSeedObj(ctx context.Context, seedKey types.NamespacedName) (gardener.Seed, error) - GetLogInstance() logr.Logger UpdateShoot(ctx context.Context, shoot *gardener.Shoot) error GetConfigFromFile() (data map[string]map[string]AuditLogData, err error) } @@ -44,7 +42,6 @@ type auditLogConfig struct { tenantConfigPath string policyConfigMapName string client client.Client - log logr.Logger } type AuditLogData struct { @@ -66,18 +63,17 @@ type AuditlogExtensionConfig struct { SecretReferenceName string `json:"secretReferenceName"` } -func NewAuditLogging(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client, log logr.Logger) AuditLogging { +func NewAuditLogging(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client) AuditLogging { return &AuditLog{ - AuditLogConfigurator: newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName, k8s, log), + AuditLogConfigurator: newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName, k8s), } } -func newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client, log logr.Logger) AuditLogConfigurator { +func newAuditLogConfigurator(auditLogTenantConfigPath, auditLogPolicyConfigMapName string, k8s client.Client) AuditLogConfigurator { return &auditLogConfig{ tenantConfigPath: auditLogTenantConfigPath, policyConfigMapName: auditLogPolicyConfigMapName, client: k8s, - log: log, } } @@ -98,11 +94,10 @@ func (a *auditLogConfig) GetSeedObj(ctx context.Context, seedKey types.Namespace } func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) { - log := al.GetLogInstance() seedName := getSeedName(*shoot) if !al.CanEnableAuditLogsForShoot(seedName) { - log.Info("Seed name or Tenant config path is empty while configuring Audit Logs on shoot: " + shoot.Name) + //log.Info("Seed name or Tenant config path is empty while configuring Audit Logs on shoot: " + shoot.Name) return false, nil } @@ -294,7 +289,3 @@ func newAuditPolicyConfig(policyConfigMapName string) *gardener.AuditConfig { func (a *auditLogConfig) UpdateShoot(ctx context.Context, shoot *gardener.Shoot) error { return a.client.Update(ctx, shoot) } - -func (a *auditLogConfig) GetLogInstance() logr.Logger { - return a.log -} diff --git a/internal/auditlogging/mocks/AuditLogConfigurator.go b/internal/auditlogging/mocks/AuditLogConfigurator.go index ff63ccb0..92016d0a 100644 --- a/internal/auditlogging/mocks/AuditLogConfigurator.go +++ b/internal/auditlogging/mocks/AuditLogConfigurator.go @@ -7,8 +7,6 @@ import ( auditlogging "github.com/kyma-project/infrastructure-manager/internal/auditlogging" - logr "github.com/go-logr/logr" - mock "github.com/stretchr/testify/mock" types "k8s.io/apimachinery/pkg/types" @@ -69,24 +67,6 @@ func (_m *AuditLogConfigurator) GetConfigFromFile() (map[string]map[string]audit return r0, r1 } -// GetLogInstance provides a mock function with given fields: -func (_m *AuditLogConfigurator) GetLogInstance() logr.Logger { - ret := _m.Called() - - if len(ret) == 0 { - panic("no return value specified for GetLogInstance") - } - - var r0 logr.Logger - if rf, ok := ret.Get(0).(func() logr.Logger); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(logr.Logger) - } - - return r0 -} - // GetPolicyConfigMapName provides a mock function with given fields: func (_m *AuditLogConfigurator) GetPolicyConfigMapName() string { ret := _m.Called() diff --git a/internal/auditlogging/tests/auditlogging_test.go b/internal/auditlogging/tests/auditlogging_test.go index a1a8fbfe..093159cb 100644 --- a/internal/auditlogging/tests/auditlogging_test.go +++ b/internal/auditlogging/tests/auditlogging_test.go @@ -4,7 +4,6 @@ import ( "context" "fmt" gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" - "github.com/go-logr/logr" "github.com/kyma-project/infrastructure-manager/internal/auditlogging" "github.com/kyma-project/infrastructure-manager/internal/auditlogging/mocks" "github.com/stretchr/testify/require" @@ -24,7 +23,6 @@ func TestEnable(t *testing.T) { configFromFile := fileConfigData() seedKey := types.NamespacedName{Name: "seed-name", Namespace: ""} - configurator.On("GetLogInstance").Return(logr.Logger{}).Once() configurator.On("CanEnableAuditLogsForShoot", "seed-name").Return(true).Once() configurator.On("GetConfigFromFile").Return(configFromFile, nil).Once() configurator.On("GetPolicyConfigMapName").Return("policyConfigMapName").Once() @@ -53,7 +51,6 @@ func TestEnable(t *testing.T) { // delete shoot region to simulate error shoot.Spec.Region = "" - configurator.On("GetLogInstance").Return(logr.Logger{}).Once() configurator.On("CanEnableAuditLogsForShoot", "seed-name").Return(true).Once() configurator.On("GetConfigFromFile").Return(configFromFile, nil).Once() configurator.On("GetPolicyConfigMapName").Return("policyConfigMapName").Once() diff --git a/internal/controller/runtime/fsm/runtime_fsm.go b/internal/controller/runtime/fsm/runtime_fsm.go index 82af3f38..137a82ba 100644 --- a/internal/controller/runtime/fsm/runtime_fsm.go +++ b/internal/controller/runtime/fsm/runtime_fsm.go @@ -107,6 +107,6 @@ func NewFsm(log logr.Logger, cfg RCCfg, k8s K8s) Fsm { RCCfg: cfg, log: log, K8s: k8s, - AuditLogging: auditlogging.NewAuditLogging(cfg.AuditLog.TenantConfigPath, cfg.AuditLog.PolicyConfigMapName, k8s.ShootClient, log), + AuditLogging: auditlogging.NewAuditLogging(cfg.AuditLog.TenantConfigPath, cfg.AuditLog.PolicyConfigMapName, k8s.ShootClient), } } diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index 5a4f3a19..26466f7e 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -11,26 +11,25 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, m.log.Info("Configure Audit Log state") wasAuditLogEnabled, err := m.AuditLogging.Enable(ctx, s.shoot) - if err != nil { - m.log.Error(err, "Failed to configure Audit Log") - s.instance.UpdateStatePending( - imv1.ConditionTypeAuditLogConfigured, - imv1.ConditionReasonConfigurationCompleted, - "False", - err.Error(), - ) - return updateStatusAndRequeueAfter(gardenerRequeueDuration) - } if wasAuditLogEnabled { m.log.Info("Audit Log configured for shoot: " + s.shoot.Name) s.instance.UpdateStateReady( imv1.ConditionTypeAuditLogConfigured, - imv1.ConditionReasonConfigurationCompleted, + imv1.ConditionReasonAuditLogConfigured, "Audit Log configured", ) return updateStatusAndStop() } - return requeue() + + m.log.Error(err, "Failed to configure Audit Log") + s.instance.UpdateStatePending( + imv1.ConditionTypeAuditLogConfigured, + imv1.ConditionReasonAuditLogError, + "False", + err.Error(), + ) + return updateStatusAndRequeueAfter(gardenerRequeueDuration) + } 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 82e6385b..bd0dce9f 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -28,7 +28,7 @@ func TestAuditLogState(t *testing.T) { { Type: string(v1.ConditionTypeAuditLogConfigured), Status: "True", - Reason: string(v1.ConditionReasonConfigurationCompleted), + Reason: string(v1.ConditionReasonAuditLogConfigured), Message: "Audit Log configured", }, } @@ -63,7 +63,7 @@ func TestAuditLogState(t *testing.T) { { Type: string(v1.ConditionTypeAuditLogConfigured), Status: "False", - Reason: string(v1.ConditionReasonConfigurationCompleted), + Reason: string(v1.ConditionReasonAuditLogError), Message: "some error during configuration", }, } @@ -83,29 +83,6 @@ 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 requeue if initial criteria of enabling Audit Log is not met", func(t *testing.T) { - // given - ctx := context.Background() - auditLog := &mocks.AuditLogging{} - shoot := shootForTest() - instance := runtimeForTest() - systemState := &systemState{ - instance: instance, - shoot: shoot, - } - - auditLog.On("Enable", ctx, shoot).Return(false, nil).Once() - - // when - fsm := &fsm{AuditLogging: auditLog} - stateFn, result, _ := sFnConfigureAuditLog(ctx, fsm, systemState) - - // then - auditLog.AssertExpectations(t) - require.Nil(t, stateFn) - require.Equal(t, true, result.Requeue) - }) } func shootForTest() *gardener.Shoot { diff --git a/internal/controller/runtime/runtime_controller_test.go b/internal/controller/runtime/runtime_controller_test.go index b87e42fa..8811ef74 100644 --- a/internal/controller/runtime/runtime_controller_test.go +++ b/internal/controller/runtime/runtime_controller_test.go @@ -106,7 +106,7 @@ var _ = Describe("Runtime Controller", func() { return false } - if !runtime.IsConditionSet(imv1.ConditionTypeAuditLogConfigured, imv1.ConditionReasonConfigurationCompleted) { + if !runtime.IsConditionSet(imv1.ConditionTypeAuditLogConfigured, imv1.ConditionReasonAuditLogConfigured) { return false } From 477fc94bdd14488976365eb28e35a06194d6db6b Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Fri, 30 Aug 2024 14:27:59 +0200 Subject: [PATCH 46/53] Update program flow --- internal/auditlogging/auditlogging.go | 3 +-- .../auditlogging/tests/auditlogging_test.go | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index 0170efcc..20c2dd28 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -97,8 +97,7 @@ func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, er seedName := getSeedName(*shoot) if !al.CanEnableAuditLogsForShoot(seedName) { - //log.Info("Seed name or Tenant config path is empty while configuring Audit Logs on shoot: " + shoot.Name) - return false, nil + return false, errors.New("Seed name on shoot or tenantConfigPath is empty") } auditConfigFromFile, err := al.GetConfigFromFile() diff --git a/internal/auditlogging/tests/auditlogging_test.go b/internal/auditlogging/tests/auditlogging_test.go index 093159cb..6a74d9b4 100644 --- a/internal/auditlogging/tests/auditlogging_test.go +++ b/internal/auditlogging/tests/auditlogging_test.go @@ -65,6 +65,26 @@ func TestEnable(t *testing.T) { require.False(t, enable) require.Error(t, err) }) + + t.Run("Should return false and error if seed is empty", func(t *testing.T) { + // given + ctx := context.Background() + configurator := &mocks.AuditLogConfigurator{} + shoot := shootForTest() + shoot.Spec.SeedName = nil + + configurator.On("CanEnableAuditLogsForShoot", "").Return(false).Once() + + // when + + auditLog := &auditlogging.AuditLog{AuditLogConfigurator: configurator} + enable, err := auditLog.Enable(ctx, shoot) + + // then + configurator.AssertExpectations(t) + require.False(t, enable) + require.Error(t, err) + }) } func TestApplyAuditLogConfig(t *testing.T) { From cb72a1ba2769db48315a0f49387d9997024b5b96 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Fri, 30 Aug 2024 14:30:13 +0200 Subject: [PATCH 47/53] Fix linter --- .../controller/runtime/fsm/runtime_fsm_configure_auditlog.go | 1 - 1 file changed, 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 26466f7e..b1ea7449 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -31,5 +31,4 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, err.Error(), ) return updateStatusAndRequeueAfter(gardenerRequeueDuration) - } From a207bcf7b29aa29d44dd0607d3a9a16daca08308 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 2 Sep 2024 13:11:04 +0200 Subject: [PATCH 48/53] Apply review sugestiosn --- .../runtime/test_client_obj_tracker.go | 8 +- .../runtime/test_client_obj_tracker_test.go | 179 ++++++++++++++++++ 2 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 internal/controller/runtime/test_client_obj_tracker_test.go diff --git a/internal/controller/runtime/test_client_obj_tracker.go b/internal/controller/runtime/test_client_obj_tracker.go index 73452761..7ee14f9d 100644 --- a/internal/controller/runtime/test_client_obj_tracker.go +++ b/internal/controller/runtime/test_client_obj_tracker.go @@ -11,6 +11,8 @@ import ( clienttesting "k8s.io/client-go/testing" ) +const shootType = "shoots" + // CustomTracker implements ObjectTracker with a sequence of Shoot objects // it will be updated with a different shoot sequence for each test case type CustomTracker struct { @@ -38,7 +40,7 @@ func (t *CustomTracker) Get(gvr schema.GroupVersionResource, ns, name string) (r t.mu.Lock() defer t.mu.Unlock() - if gvr.Resource == "shoots" { //nolint:goconst + if gvr.Resource == shootType { return getNextObject(t.shootSequence, &t.shootCallCnt) } else if gvr.Resource == "seeds" { return getNextObject(t.seedSequence, &t.seedCallCnt) @@ -64,7 +66,7 @@ func (t *CustomTracker) Update(gvr schema.GroupVersionResource, obj runtime.Obje t.mu.Lock() defer t.mu.Unlock() - if gvr.Resource == "shoots" { //nolint:goconst + if gvr.Resource == shootType { shoot, ok := obj.(*gardener_api.Shoot) if !ok { return fmt.Errorf("object is not of type Gardener Shoot") @@ -84,7 +86,7 @@ func (t *CustomTracker) Delete(gvr schema.GroupVersionResource, ns, name string) t.mu.Lock() defer t.mu.Unlock() - if gvr.Resource == "shoots" { //nolint:goconst + if gvr.Resource == shootType { for index, shoot := range t.shootSequence { if shoot != nil && shoot.Name == name { t.shootSequence[index] = nil diff --git a/internal/controller/runtime/test_client_obj_tracker_test.go b/internal/controller/runtime/test_client_obj_tracker_test.go new file mode 100644 index 00000000..fde0480f --- /dev/null +++ b/internal/controller/runtime/test_client_obj_tracker_test.go @@ -0,0 +1,179 @@ +package runtime + +import ( + 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) { + seedSequence := []*gardener_api.Seed{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "seed", + Namespace: "test", + }, + }} + + shootSequence := []*gardener_api.Shoot{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "shoot", + Namespace: "test", + }, + }} + + t.Run("should return shoot object", func(t *testing.T) { + // given + tracker := NewCustomTracker(nil, shootSequence, seedSequence) + gvr := schema.GroupVersionResource{ + Resource: "shoots", + } + + // when + obj, err := tracker.Get(gvr, "test", "shoot") + + // then + require.NoError(t, err) + require.NotNil(t, obj) + require.Equal(t, shootSequence[0], obj) + }) + + t.Run("should return seed object", func(t *testing.T) { + // given + tracker := NewCustomTracker(nil, shootSequence, seedSequence) + gvr := schema.GroupVersionResource{ + Resource: "seeds", + } + + // when + obj, err := tracker.Get(gvr, "test", "seed") + + // then + require.NoError(t, err) + require.NotNil(t, obj) + require.Equal(t, seedSequence[0], obj) + }) + + t.Run("should return not found error", func(t *testing.T) { + // given + tracker := NewCustomTracker(nil, shootSequence, seedSequence) + gvr := schema.GroupVersionResource{ + Resource: "shoots", + } + + // when + obj, err := tracker.Get(gvr, "test", "shoot") + objOutOfRange, errOutOfRange := tracker.Get(gvr, "test", "outOfRange") + + // then + require.NoError(t, err) + require.NotNil(t, obj) + require.Error(t, errOutOfRange) + require.Nil(t, objOutOfRange) + }) +} + +func TestCustomTracker_Update(t *testing.T) { + t.Run("should update shoot object", func(t *testing.T) { + // given + shootSequence := []*gardener_api.Shoot{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "shoot", + Namespace: "test", + }, + }} + tracker := NewCustomTracker(nil, shootSequence, nil) + gvr := schema.GroupVersionResource{ + Resource: "shoots", + } + newShoot := &gardener_api.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "shoot", + Namespace: "test", + }, + Spec: gardener_api.ShootSpec{ + Region: "afterUpdate", + }, + } + + // when + err := tracker.Update(gvr, newShoot, "test") + + // then + require.NoError(t, err) + require.Equal(t, newShoot, shootSequence[0]) + }) + + t.Run("should return not found error", func(t *testing.T) { + // given + shootSequence := []*gardener_api.Shoot{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "shoot", + Namespace: "test", + }, + }} + tracker := NewCustomTracker(nil, shootSequence, nil) + gvr := schema.GroupVersionResource{ + Resource: "shoots", + } + newShoot := &gardener_api.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "shoot2", + Namespace: "test", + }, + Spec: gardener_api.ShootSpec{ + Region: "afterUpdate", + }, + } + + // when + err := tracker.Update(gvr, newShoot, "test") + + // then + require.Error(t, err) + }) +} + +func TestCustomTracker_Delete(t *testing.T) { + t.Run("should delete shoot object", func(t *testing.T) { + // given + shootSequence := []*gardener_api.Shoot{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "shoot", + Namespace: "test", + }, + }} + tracker := NewCustomTracker(nil, shootSequence, nil) + gvr := schema.GroupVersionResource{ + Resource: "shoots", + } + + // when + err := tracker.Delete(gvr, "test", "shoot") + + // then + require.NoError(t, err) + require.Nil(t, shootSequence[0]) + }) + + t.Run("should return not found error", func(t *testing.T) { + // given + shootSequence := []*gardener_api.Shoot{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "shoot", + Namespace: "test", + }, + }} + tracker := NewCustomTracker(nil, shootSequence, nil) + gvr := schema.GroupVersionResource{ + Resource: "shoots", + } + + // when + err := tracker.Delete(gvr, "test", "shoot2") + + // then + require.Error(t, err) + }) +} From d6b56694720fa1a9c81f0082fb98f6a15715cebc Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Mon, 2 Sep 2024 13:27:07 +0200 Subject: [PATCH 49/53] Trigger GA --- internal/controller/runtime/test_client_obj_tracker_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/runtime/test_client_obj_tracker_test.go b/internal/controller/runtime/test_client_obj_tracker_test.go index fde0480f..2604945a 100644 --- a/internal/controller/runtime/test_client_obj_tracker_test.go +++ b/internal/controller/runtime/test_client_obj_tracker_test.go @@ -136,7 +136,7 @@ func TestCustomTracker_Update(t *testing.T) { } func TestCustomTracker_Delete(t *testing.T) { - t.Run("should delete shoot object", func(t *testing.T) { + t.Run("should delete shoot object.", func(t *testing.T) { // given shootSequence := []*gardener_api.Shoot{{ ObjectMeta: metav1.ObjectMeta{ From 4a116f65acb44fb41bbfc3d4e2aa7c4a5cb7cb8d Mon Sep 17 00:00:00 2001 From: m00g3n Date: Tue, 3 Sep 2024 15:24:57 +0200 Subject: [PATCH 50/53] Adapt tests to auditlogs changes --- .../runtime/fsm/runtime_fsm_apply_crb_test.go | 449 +++++++++--------- .../controller/runtime/fsm/utilz_for_test.go | 57 ++- 2 files changed, 277 insertions(+), 229 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go b/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go index 603dabf8..bb675ccb 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go @@ -1,229 +1,224 @@ package fsm -// -//import ( -// "context" -// "fmt" -// "time" -// -// gardener_api "github.com/gardener/gardener/pkg/apis/core/v1beta1" -// imv1 "github.com/kyma-project/infrastructure-manager/api/v1" -// . "github.com/onsi/ginkgo/v2" -// . "github.com/onsi/gomega" -// rbacv1 "k8s.io/api/rbac/v1" -// metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -// "k8s.io/apimachinery/pkg/runtime" -// ctrl "sigs.k8s.io/controller-runtime" -// "sigs.k8s.io/controller-runtime/pkg/client" -//) -// -//var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() { -// -// var testErr = fmt.Errorf("test error") -// -// DescribeTable("getMissing", -// func(tc tcGetCRB) { -// actual := getMissing(tc.crbs, tc.admins) -// Expect(actual).To(BeComparableTo(tc.expected)) -// }, -// Entry("should return a list with CRBs to be created", tcGetCRB{ -// admins: []string{"test1", "test2"}, -// crbs: nil, -// expected: []rbacv1.ClusterRoleBinding{ -// toAdminClusterRoleBinding("test1"), -// toAdminClusterRoleBinding("test2"), -// }, -// }), -// Entry("should return nil list if no admins missing", tcGetCRB{ -// admins: []string{"test1"}, -// crbs: []rbacv1.ClusterRoleBinding{ -// toAdminClusterRoleBinding("test1"), -// }, -// expected: nil, -// }), -// ) -// -// DescribeTable("getRemoved", -// func(tc tcGetCRB) { -// actual := getRemoved(tc.crbs, tc.admins) -// Expect(actual).To(BeComparableTo(tc.expected)) -// }, -// Entry("should return nil list if CRB list is nil", tcGetCRB{ -// admins: []string{"test1"}, -// crbs: nil, -// expected: nil, -// }), -// Entry("should return nil list if CRB list is empty", tcGetCRB{ -// admins: []string{"test1"}, -// crbs: []rbacv1.ClusterRoleBinding{}, -// expected: nil, -// }), -// Entry("should return nil list if no admins to remove", tcGetCRB{ -// admins: []string{"test1"}, -// crbs: []rbacv1.ClusterRoleBinding{toAdminClusterRoleBinding("test1")}, -// expected: nil, -// }), -// Entry("should return list if with CRBs to remove", tcGetCRB{ -// admins: []string{"test2"}, -// crbs: []rbacv1.ClusterRoleBinding{ -// toAdminClusterRoleBinding("test1"), -// toAdminClusterRoleBinding("test2"), -// toAdminClusterRoleBinding("test3"), -// }, -// expected: []rbacv1.ClusterRoleBinding{ -// toAdminClusterRoleBinding("test1"), -// toAdminClusterRoleBinding("test3"), -// }, -// }), -// ) -// -// testRuntime := imv1.Runtime{ -// ObjectMeta: metav1.ObjectMeta{ -// Name: "testme1", -// Namespace: "default", -// }, -// } -// -// testRuntimeWithAdmin := imv1.Runtime{ -// ObjectMeta: metav1.ObjectMeta{ -// Name: "testme1", -// Namespace: "default", -// }, -// Spec: imv1.RuntimeSpec{ -// Security: imv1.Security{ -// Administrators: []string{ -// "test-admin1", -// }, -// }, -// }, -// } -// -// testScheme, err := newTestScheme() -// Expect(err).ShouldNot(HaveOccurred()) -// -// defaultSetup := func(f *fsm) error { -// GetShootClient = func( -// _ context.Context, -// _ client.SubResourceClient, -// _ *gardener_api.Shoot) (client.Client, error) { -// return f.Client, nil -// } -// return nil -// } -// -// DescribeTable("sFnAppluClusterRoleBindings", -// func(tc tcApplySfn) { -// // initialize test data if required -// Expect(tc.init()).ShouldNot(HaveOccurred()) -// -// ctx, cancel := context.WithTimeout(context.Background(), time.Second) -// defer cancel() -// -// actualResult, actualErr := tc.fsm.Run(ctx, tc.instance) -// Expect(actualResult).Should(BeComparableTo(tc.expected.result)) -// -// matchErr := BeNil() -// if tc.expected.err != nil { -// matchErr = MatchError(tc.expected.err) -// } -// Expect(actualErr).Should(matchErr) -// }, -// -// Entry("add admin", tcApplySfn{ -// instance: testRuntimeWithAdmin, -// expected: tcSfnExpected{ -// err: nil, -// }, -// fsm: must( -// newFakeFSM, -// withFakedK8sClient(testScheme, &testRuntimeWithAdmin), -// withFn(sFnApplyClusterRoleBindings), -// withFakeEventRecorder(1), -// ), -// setup: defaultSetup, -// }), -// -// Entry("nothing change", tcApplySfn{ -// instance: testRuntime, -// expected: tcSfnExpected{ -// err: nil, -// }, -// fsm: must( -// newFakeFSM, -// withFakedK8sClient(testScheme, &testRuntime), -// withFn(sFnApplyClusterRoleBindings), -// withFakeEventRecorder(1), -// ), -// setup: defaultSetup, -// }), -// -// Entry("error getting client", tcApplySfn{ -// instance: testRuntime, -// expected: tcSfnExpected{ -// err: testErr, -// }, -// fsm: must( -// newFakeFSM, -// withFakedK8sClient(testScheme, &testRuntime), -// withFn(sFnApplyClusterRoleBindings), -// withFakeEventRecorder(1), -// ), -// setup: func(f *fsm) error { -// GetShootClient = func( -// _ context.Context, -// _ client.SubResourceClient, -// _ *gardener_api.Shoot) (client.Client, error) { -// return nil, testErr -// } -// return nil -// -// }, -// }), -// ) -//}) -// -//type tcGetCRB struct { -// crbs []rbacv1.ClusterRoleBinding -// admins []string -// expected []rbacv1.ClusterRoleBinding -//} -// -//type tcSfnExpected struct { -// result ctrl.Result -// err error -//} -// -//type tcApplySfn struct { -// expected tcSfnExpected -// setup func(m *fsm) error -// fsm *fsm -// instance imv1.Runtime -//} -// -//func (c *tcApplySfn) init() error { -// if c.setup != nil { -// return c.setup(c.fsm) -// } -// return nil -//} -// -//func toCRBs(admins []string) (result []rbacv1.ClusterRoleBinding) { -// for _, crb := range admins { -// result = append(result, toAdminClusterRoleBinding(crb)) -// } -// return result -//} -// -//func newTestScheme() (*runtime.Scheme, error) { -// schema := runtime.NewScheme() -// -// for _, fn := range []func(*runtime.Scheme) error{ -// imv1.AddToScheme, -// rbacv1.AddToScheme, -// } { -// if err := fn(schema); err != nil { -// return nil, err -// } -// } -// return schema, nil -//} +import ( + "context" + "fmt" + "time" + + gardener_api "github.com/gardener/gardener/pkg/apis/core/v1beta1" + imv1 "github.com/kyma-project/infrastructure-manager/api/v1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() { + + var testErr = fmt.Errorf("test error") + + DescribeTable("getMissing", + func(tc tcGetCRB) { + actual := getMissing(tc.crbs, tc.admins) + Expect(actual).To(BeComparableTo(tc.expected)) + }, + Entry("should return a list with CRBs to be created", tcGetCRB{ + admins: []string{"test1", "test2"}, + crbs: nil, + expected: []rbacv1.ClusterRoleBinding{ + toAdminClusterRoleBinding("test1"), + toAdminClusterRoleBinding("test2"), + }, + }), + Entry("should return nil list if no admins missing", tcGetCRB{ + admins: []string{"test1"}, + crbs: []rbacv1.ClusterRoleBinding{ + toAdminClusterRoleBinding("test1"), + }, + expected: nil, + }), + ) + + DescribeTable("getRemoved", + func(tc tcGetCRB) { + actual := getRemoved(tc.crbs, tc.admins) + Expect(actual).To(BeComparableTo(tc.expected)) + }, + Entry("should return nil list if CRB list is nil", tcGetCRB{ + admins: []string{"test1"}, + crbs: nil, + expected: nil, + }), + Entry("should return nil list if CRB list is empty", tcGetCRB{ + admins: []string{"test1"}, + crbs: []rbacv1.ClusterRoleBinding{}, + expected: nil, + }), + Entry("should return nil list if no admins to remove", tcGetCRB{ + admins: []string{"test1"}, + crbs: []rbacv1.ClusterRoleBinding{toAdminClusterRoleBinding("test1")}, + expected: nil, + }), + Entry("should return list if with CRBs to remove", tcGetCRB{ + admins: []string{"test2"}, + crbs: []rbacv1.ClusterRoleBinding{ + toAdminClusterRoleBinding("test1"), + toAdminClusterRoleBinding("test2"), + toAdminClusterRoleBinding("test3"), + }, + expected: []rbacv1.ClusterRoleBinding{ + toAdminClusterRoleBinding("test1"), + toAdminClusterRoleBinding("test3"), + }, + }), + ) + + testRuntime := imv1.Runtime{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testme1", + Namespace: "default", + }, + } + + testRuntimeWithAdmin := imv1.Runtime{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testme1", + Namespace: "default", + }, + Spec: imv1.RuntimeSpec{ + Security: imv1.Security{ + Administrators: []string{ + "test-admin1", + }, + }, + }, + } + + testScheme, err := newTestScheme() + Expect(err).ShouldNot(HaveOccurred()) + + defaultSetup := func(f *fsm) error { + GetShootClient = func( + _ context.Context, + _ client.SubResourceClient, + _ *gardener_api.Shoot) (client.Client, error) { + return f.Client, nil + } + return nil + } + + DescribeTable("sFnAppluClusterRoleBindings", + func(tc tcApplySfn) { + // initialize test data if required + Expect(tc.init()).ShouldNot(HaveOccurred()) + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + actualResult, actualErr := tc.fsm.Run(ctx, tc.instance) + Expect(actualResult).Should(BeComparableTo(tc.expected.result)) + + matchErr := BeNil() + if tc.expected.err != nil { + matchErr = MatchError(tc.expected.err) + } + Expect(actualErr).Should(matchErr) + }, + + Entry("add admin", tcApplySfn{ + instance: testRuntimeWithAdmin, + expected: tcSfnExpected{ + err: nil, + }, + fsm: must( + newFakeFSM, + withAuditLogging(true, nil), + withFakedK8sClient(testScheme, &testRuntimeWithAdmin), + withFn(sFnApplyClusterRoleBindingsStateSetup), + withFakeEventRecorder(1), + ), + setup: defaultSetup, + }), + + Entry("nothing change", tcApplySfn{ + instance: testRuntime, + expected: tcSfnExpected{ + err: nil, + }, + fsm: must( + newFakeFSM, + withAuditLogging(true, nil), + withFakedK8sClient(testScheme, &testRuntime), + withFn(sFnApplyClusterRoleBindingsStateSetup), + withFakeEventRecorder(1), + ), + setup: defaultSetup, + }), + + Entry("error getting client", tcApplySfn{ + instance: testRuntime, + expected: tcSfnExpected{ + err: testErr, + }, + fsm: must( + newFakeFSM, + withAuditLogging(true, nil), + withFakedK8sClient(testScheme, &testRuntime), + withFn(sFnApplyClusterRoleBindingsStateSetup), + withFakeEventRecorder(1), + ), + setup: func(f *fsm) error { + GetShootClient = func( + _ context.Context, + _ client.SubResourceClient, + _ *gardener_api.Shoot) (client.Client, error) { + return nil, testErr + } + return nil + + }, + }), + ) +}) + +type tcGetCRB struct { + crbs []rbacv1.ClusterRoleBinding + admins []string + expected []rbacv1.ClusterRoleBinding +} + +type tcSfnExpected struct { + result ctrl.Result + err error +} + +type tcApplySfn struct { + expected tcSfnExpected + setup func(m *fsm) error + fsm *fsm + instance imv1.Runtime +} + +func (c *tcApplySfn) init() error { + if c.setup != nil { + return c.setup(c.fsm) + } + return nil +} + +func newTestScheme() (*runtime.Scheme, error) { + schema := runtime.NewScheme() + + for _, fn := range []func(*runtime.Scheme) error{ + imv1.AddToScheme, + rbacv1.AddToScheme, + } { + if err := fn(schema); err != nil { + return nil, err + } + } + return schema, nil +} diff --git a/internal/controller/runtime/fsm/utilz_for_test.go b/internal/controller/runtime/fsm/utilz_for_test.go index 3699df6a..9e92a2ef 100644 --- a/internal/controller/runtime/fsm/utilz_for_test.go +++ b/internal/controller/runtime/fsm/utilz_for_test.go @@ -1,14 +1,21 @@ package fsm import ( + "context" "fmt" + gardener "github.com/gardener/gardener/pkg/apis/core/v1beta1" + gardener_api "github.com/gardener/gardener/pkg/apis/core/v1beta1" "github.com/kyma-project/infrastructure-manager/internal/gardener/shoot" - . "github.com/onsi/gomega" //nolint:revive + . "github.com/onsi/ginkgo/v2" //nolint:revive + . "github.com/onsi/gomega" //nolint:revive + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log/zap" ) type fakeFSMOpt func(*fsm) error @@ -74,10 +81,22 @@ var ( return nil } } + + withAuditLogging = func(isEnabled bool, err error) fakeFSMOpt { + return func(fsm *fsm) error { + fsm.AuditLogging = &stubAuditLogging{ + isEnabled: isEnabled, + err: err, + } + return nil + } + } ) func newFakeFSM(opts ...fakeFSMOpt) (*fsm, error) { - fsm := fsm{} + fsm := fsm{ + log: zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)), + } // apply opts for _, opt := range opts { if err := opt(&fsm); err != nil { @@ -90,3 +109,37 @@ func newFakeFSM(opts ...fakeFSMOpt) (*fsm, error) { } return &fsm, nil } + +// stubAuditLogging - a special type to allow to test audit logging +type stubAuditLogging struct { + isEnabled bool + err error +} + +func (s *stubAuditLogging) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, error) { + return s.isEnabled, s.err +} + +func newSetupStateForTest(sfn stateFn, opts ...func(*systemState) error) stateFn { + return func(_ context.Context, _ *fsm, s *systemState) (stateFn, *ctrl.Result, error) { + for _, fn := range opts { + if err := fn(s); err != nil { + return nil, nil, fmt.Errorf("test state setup failed: %s", err) + } + } + return sfn, nil, nil + } +} + +// sFnApplyClusterRoleBindingsStateSetup a special function to setup system state in tests +var sFnApplyClusterRoleBindingsStateSetup = newSetupStateForTest(sFnApplyClusterRoleBindings, func(s *systemState) error { + + s.shoot = &gardener_api.Shoot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-shoot", + Namespace: "test-namespace", + }, + } + + return nil +}) From e7ffcd7017c468f25b53b4e01b998e095cbb5dd6 Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 4 Sep 2024 10:23:13 +0200 Subject: [PATCH 51/53] Implement wait for Gardener shoot be ready after configuring Audit Log --- internal/auditlogging/auditlogging.go | 2 +- .../fsm/runtime_fsm_configure_auditlog.go | 29 +++++-- .../runtime_fsm_configure_auditlog_test.go | 41 +++++++++- ...runtime_fsm_waiting_for_shoot_reconcile.go | 4 +- .../runtime/runtime_controller_test.go | 2 +- internal/controller/runtime/suite_test.go | 82 +++++++++++++++++-- .../runtime/testdata/auditConfig.json | 2 +- 7 files changed, 140 insertions(+), 22 deletions(-) diff --git a/internal/auditlogging/auditlogging.go b/internal/auditlogging/auditlogging.go index 20c2dd28..5aaf150a 100644 --- a/internal/auditlogging/auditlogging.go +++ b/internal/auditlogging/auditlogging.go @@ -125,7 +125,7 @@ func (al *AuditLog) Enable(ctx context.Context, shoot *gardener.Shoot) (bool, er } } - return true, nil + return annotated, nil } func ApplyAuditLogConfig(shoot *gardener.Shoot, auditConfigFromFile map[string]map[string]AuditLogData, providerType string) (bool, error) { diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index b1ea7449..8383080f 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -14,21 +14,32 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, if wasAuditLogEnabled { m.log.Info("Audit Log configured for shoot: " + s.shoot.Name) - s.instance.UpdateStateReady( + s.instance.UpdateStatePending( imv1.ConditionTypeAuditLogConfigured, imv1.ConditionReasonAuditLogConfigured, - "Audit Log configured", + "Unknown", + "Waiting for Gardener shoot to be Ready state after configuration of the Audit Logs", ) - return updateStatusAndStop() + return updateStatusAndRequeueAfter(gardenerRequeueDuration) } - m.log.Error(err, "Failed to configure Audit Log") - s.instance.UpdateStatePending( + if err != nil { + m.log.Error(err, "Failed to configure Audit Log") + s.instance.UpdateStatePending( + imv1.ConditionTypeAuditLogConfigured, + imv1.ConditionReasonAuditLogError, + "False", + err.Error(), + ) + return updateStatusAndRequeueAfter(gardenerRequeueDuration) + } + + s.instance.UpdateStateReady( imv1.ConditionTypeAuditLogConfigured, - imv1.ConditionReasonAuditLogError, - "False", - err.Error(), + imv1.ConditionReasonAuditLogConfigured, + "Audit Log configured successfully", ) - return updateStatusAndRequeueAfter(gardenerRequeueDuration) + + return updateStatusAndStop() } 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 bd0dce9f..deb0bb79 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -14,7 +14,7 @@ import ( ) func TestAuditLogState(t *testing.T) { - t.Run("Should set status on Runtime CR when Audit Log was successfully configured", func(t *testing.T) { + t.Run("Should set status on Runtime CR when Audit Log configuration was changed and Shoot enters into reconciliation", func(t *testing.T) { // given ctx := context.Background() auditLog := &mocks.AuditLogging{} @@ -27,9 +27,9 @@ func TestAuditLogState(t *testing.T) { expectedRuntimeConditions := []metav1.Condition{ { Type: string(v1.ConditionTypeAuditLogConfigured), - Status: "True", + Status: "Unknown", Reason: string(v1.ConditionReasonAuditLogConfigured), - Message: "Audit Log configured", + Message: "Waiting for Gardener shoot to be Ready state after configuration of the Audit Logs", }, } @@ -42,6 +42,41 @@ func TestAuditLogState(t *testing.T) { // 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) + }) + + t.Run("Should set status on Runtime CR when Shoot is in Succeeded state after configuring Audit Log", 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.ConditionReasonAuditLogConfigured), + Message: "Audit Log configured successfully", + }, + } + + auditLog.On("Enable", ctx, shoot).Return(false, nil).Once() + + // when + fsm := &fsm{AuditLogging: auditLog} + 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") 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 b70e09a2..ce700502 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,9 +47,9 @@ func sFnWaitForShootReconcile(_ context.Context, m *fsm, s *systemState) (stateF return ensureStatusConditionIsSetAndContinue( &s.instance, imv1.ConditionTypeRuntimeProvisioned, - imv1.ConditionReasonConfigurationCompleted, + imv1.ConditionReasonAuditLogConfigured, "Runtime processing completed successfully", - sFnApplyClusterRoleBindings) + sFnConfigureAuditLog) } m.log.Info("Update did not processed, exiting with no retry") diff --git a/internal/controller/runtime/runtime_controller_test.go b/internal/controller/runtime/runtime_controller_test.go index 8811ef74..ed6fe337 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.ConditionReasonConfigurationCompleted) { + if !runtime.IsConditionSet(imv1.ConditionTypeRuntimeProvisioned, imv1.ConditionReasonAuditLogConfigured) { return false } diff --git a/internal/controller/runtime/suite_test.go b/internal/controller/runtime/suite_test.go index b8ada71a..22c538b2 100644 --- a/internal/controller/runtime/suite_test.go +++ b/internal/controller/runtime/suite_test.go @@ -18,6 +18,10 @@ 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" @@ -140,14 +144,14 @@ var _ = AfterSuite(func() { func setupGardenerTestClientForProvisioning() { baseShoot := getBaseShootForTestingSequence() shoots := fixShootsSequenceForProvisioning(&baseShoot) - seeds := fixSeedsSequence() + seeds := fixSeedsSequenceForProvisioning() setupGardenerClientWithSequence(shoots, seeds) } func setupGardenerTestClientForUpdate() { baseShoot := getBaseShootForTestingSequence() shoots := fixShootsSequenceForUpdate(&baseShoot) - seeds := fixSeedsSequence() + seeds := fixSeedsSequenceForUpdate() setupGardenerClientWithSequence(shoots, seeds) } @@ -205,14 +209,24 @@ func fixShootsSequenceForProvisioning(shoot *gardener_api.Shoot) []*gardener_api readyShoot.Status.LastOperation.State = gardener_api.LastOperationStateSucceeded + processingShootAfterAuditLogs := readyShoot.DeepCopy() + addAuditLogConfigToShoot(processingShootAfterAuditLogs) + processingShootAfterAuditLogs.Status.LastOperation.Type = gardener_api.LastOperationTypeReconcile + processingShootAfterAuditLogs.Status.LastOperation.State = gardener_api.LastOperationStateProcessing + + readyShootAfterAuditLogs := processingShootAfterAuditLogs.DeepCopy() + readyShootAfterAuditLogs.Status.LastOperation.State = gardener_api.LastOperationStateSucceeded + // processedShoot := processingShoot.DeepCopy() // will add specific data later - return []*gardener_api.Shoot{missingShoot, missingShoot, missingShoot, initialisedShoot, dnsShoot, pendingShoot, processingShoot, readyShoot, readyShoot, readyShoot, readyShoot, readyShoot} + return []*gardener_api.Shoot{missingShoot, missingShoot, missingShoot, initialisedShoot, dnsShoot, pendingShoot, processingShoot, readyShoot, readyShoot, readyShoot, readyShoot, readyShoot, processingShootAfterAuditLogs, readyShootAfterAuditLogs, readyShootAfterAuditLogs} } func fixShootsSequenceForUpdate(shoot *gardener_api.Shoot) []*gardener_api.Shoot { pendingShoot := shoot.DeepCopy() + addAuditLogConfigToShoot(pendingShoot) + pendingShoot.Spec.DNS = &gardener_api.DNS{ Domain: ptr.To("test.domain"), } @@ -236,7 +250,7 @@ func fixShootsSequenceForUpdate(shoot *gardener_api.Shoot) []*gardener_api.Shoot // processedShoot := processingShoot.DeepCopy() // will add specific data later - return []*gardener_api.Shoot{pendingShoot, processingShoot, readyShoot, readyShoot, readyShoot} + return []*gardener_api.Shoot{pendingShoot, processingShoot, readyShoot, readyShoot} } func fixShootsSequenceForDelete(shoot *gardener_api.Shoot) []*gardener_api.Shoot { @@ -270,7 +284,22 @@ func fixShootsSequenceForDelete(shoot *gardener_api.Shoot) []*gardener_api.Shoot return []*gardener_api.Shoot{currentShoot, currentShoot, currentShoot, currentShoot, pendingDeleteShoot, nil} } -func fixSeedsSequence() []*gardener_api.Seed { +func fixSeedsSequenceForProvisioning() []*gardener_api.Seed { + seed := &gardener_api.Seed{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-seed", + }, + Spec: gardener_api.SeedSpec{ + Provider: gardener_api.SeedProvider{ + Type: "aws", + }, + }, + } + + return []*gardener_api.Seed{seed, seed} +} + +func fixSeedsSequenceForUpdate() []*gardener_api.Seed { seed := &gardener_api.Seed{ ObjectMeta: metav1.ObjectMeta{ Name: "test-seed", @@ -325,3 +354,46 @@ func fixConverterConfigForTests() gardener_shoot.ConverterConfig { }, } } + +func addAuditLogConfigToShoot(shoot *gardener_api.Shoot) { + shoot.Spec.Kubernetes.KubeAPIServer.AuditConfig = &gardener_api.AuditConfig{ + AuditPolicy: &gardener_api.AuditPolicy{ + ConfigMapRef: &v12.ObjectReference{Name: "policy-config-map"}, + }, + } + + shoot.Spec.Resources = append(shoot.Spec.Resources, gardener_api.NamedResourceReference{ + Name: "auditlog-credentials", + ResourceRef: v1.CrossVersionObjectReference{ + Kind: "Secret", + Name: "auditlog-secret", + APIVersion: "v1", + }, + }) + + const ( + extensionKind = "AuditlogConfig" + extensionVersion = "service.auditlog.extensions.gardener.cloud/v1alpha1" + extensionType = "standard" + ) + + shoot.Spec.Extensions = append(shoot.Spec.Extensions, gardener_api.Extension{ + Type: "shoot-auditlog-service", + }) + + ext := &shoot.Spec.Extensions[len(shoot.Spec.Extensions)-1] + + cfg := auditlogging.AuditlogExtensionConfig{ + TypeMeta: metav1.TypeMeta{ + Kind: extensionKind, + APIVersion: extensionVersion, + }, + Type: extensionType, + TenantID: "79c64792-9c1e-4c1b-9941-ef7560dd3eae", + ServiceURL: "https://auditlog.example.com:3001", + SecretReferenceName: "auditlog-credentials", + } + + ext.ProviderConfig = &runtime.RawExtension{} + ext.ProviderConfig.Raw, _ = json.Marshal(cfg) +} diff --git a/internal/controller/runtime/testdata/auditConfig.json b/internal/controller/runtime/testdata/auditConfig.json index a7a163c3..608d06a2 100644 --- a/internal/controller/runtime/testdata/auditConfig.json +++ b/internal/controller/runtime/testdata/auditConfig.json @@ -3,7 +3,7 @@ "eu-central-1": { "tenantID": "79c64792-9c1e-4c1b-9941-ef7560dd3eae", "serviceURL": "https://auditlog.example.com:3001", - "secretName": "auditlog-secret2" + "secretName": "auditlog-secret" } } } From 08cc2d73c015fe82c59d16af881c8d2dfaae8fbc Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Wed, 4 Sep 2024 13:30:44 +0200 Subject: [PATCH 52/53] Fixing test crb --- .../controller/runtime/fsm/runtime_fsm_apply_crb_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go b/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go index bb675ccb..d20378ad 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go @@ -131,7 +131,8 @@ var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() { Entry("add admin", tcApplySfn{ instance: testRuntimeWithAdmin, expected: tcSfnExpected{ - err: nil, + err: nil, + result: ctrl.Result{RequeueAfter: time.Second * 15}, }, fsm: must( newFakeFSM, @@ -146,7 +147,8 @@ var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() { Entry("nothing change", tcApplySfn{ instance: testRuntime, expected: tcSfnExpected{ - err: nil, + err: nil, + result: ctrl.Result{RequeueAfter: time.Second * 15}, }, fsm: must( newFakeFSM, From 25f37f300e10adb46b90af50321128e87b928beb Mon Sep 17 00:00:00 2001 From: Rafal Foks Date: Thu, 5 Sep 2024 14:52:09 +0200 Subject: [PATCH 53/53] Refactor and fix --- .../runtime/fsm/runtime_fsm_configure_auditlog.go | 13 ++++++------- .../fsm/runtime_fsm_configure_auditlog_test.go | 2 +- .../fsm/runtime_fsm_waiting_shoot_creation.go | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go index 8383080f..d63c57e9 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog.go @@ -32,14 +32,13 @@ func sFnConfigureAuditLog(ctx context.Context, m *fsm, s *systemState) (stateFn, "False", err.Error(), ) - return updateStatusAndRequeueAfter(gardenerRequeueDuration) + } else { + s.instance.UpdateStateReady( + imv1.ConditionTypeAuditLogConfigured, + imv1.ConditionReasonAuditLogConfigured, + "Audit Log configured successfully", + ) } - s.instance.UpdateStateReady( - imv1.ConditionTypeAuditLogConfigured, - imv1.ConditionReasonAuditLogConfigured, - "Audit Log configured successfully", - ) - return updateStatusAndStop() } 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 deb0bb79..eab363b7 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_configure_auditlog_test.go @@ -84,7 +84,7 @@ func TestAuditLogState(t *testing.T) { assert.Equal(t, expectedRuntimeConditions, systemState.instance.Status.Conditions) }) - t.Run("Should requeue 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 and set status on Runtime CR", func(t *testing.T) { // given ctx := context.Background() auditLog := &mocks.AuditLogging{} diff --git a/internal/controller/runtime/fsm/runtime_fsm_waiting_shoot_creation.go b/internal/controller/runtime/fsm/runtime_fsm_waiting_shoot_creation.go index ef654676..f5c4b3de 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_waiting_shoot_creation.go +++ b/internal/controller/runtime/fsm/runtime_fsm_waiting_shoot_creation.go @@ -31,7 +31,7 @@ func sFnWaitForShootCreation(_ context.Context, m *fsm, s *systemState) (stateFn m.log.Info("Waiting for shoot creation state") switch s.shoot.Status.LastOperation.State { - case gardener.LastOperationStateProcessing, gardener.LastOperationStatePending, gardener.LastOperationStateAborted: + case gardener.LastOperationStateProcessing, gardener.LastOperationStatePending, gardener.LastOperationStateAborted, gardener.LastOperationStateError: m.log.Info(fmt.Sprintf("Shoot %s is in %s state, scheduling for retry", s.shoot.Name, s.shoot.Status.LastOperation.State)) s.instance.UpdateStatePending(